I seriously cannot figure out what's wrong with this!

[Craptacular]

Hey guys,

 

So I am adding to an already existing Shipping quote form and backend reply system. I basically just added one new item, tested if it was working on the front and backend (which it was), and then proceeded to add the other new items. BUT for some reason the database is not being populated. HOWEVER when the automated email is sent after the customer asks for a quote via this system, it shows all the correct information, but yet the database isn't showing anything, and the backend reply system doesn't show that they selected an item to get a quote on (if they select one, or all of the 6 new items). I know it's something silly I'm missing, but I am just not seeing it, and I have spent at least two days trying to find the issue! PLEASE HELP me! Thanks so much! (Sorry it's so much code in this post, I wanted to make sure I included it all so you can see what's all going on. I am only having issues with the Gun Vault Products GV2000S, GVB2000, GV3000, GVB3000, NV300, BB3000, the rest of the items work perfectly. I have attached the scripts.)

 

So here is the HTML: 

<div class="ship-wrap">
<div class="checkbox"><label for="gv2000s"><input type="checkbox" name="gv2000s" id="gv2000s" /> Model GV2000S</label></div>
<div id="quantity-25" class="quantity"><label for="quantity-gv2000s">Quantity: </label>
<select name="quantity-gv2000s" id="quantity-gv2000s">
<option value="1">1</option>
<option value="2">2</option>
<option value="3">3</option>
<option value="4">4</option>
<option value="5+">5+</option>
</select></div>
</div>
<div class="ship-wrap">
<div class="checkbox"><label for="gvb2000"><input type="checkbox" name="gvb2000" id="gvb2000" /> Model GVB2000</label></div>
<div id="quantity-26" class="quantity"><label for="quantity-gvb2000">Quantity: </label>
<select name="quantity-gvb2000" id="quantity-gvb2000">
<option value="1">1</option>
<option value="2">2</option>
<option value="3">3</option>
<option value="4">4</option>
<option value="5+">5+</option>
</select></div>
</div>

<div class="ship-wrap">
<div class="checkbox"><label for="gv3000s"><input type="checkbox" name="gv3000s" id="gv3000s" /> Model GV3000S</label></div>
<div id="quantity-27" class="quantity"><label for="quantity-gv3000s">Quantity: </label>
<select name="quantity-gv3000s" id="quantity-gv3000s">
<option value="1">1</option>
<option value="2">2</option>
<option value="3">3</option>
<option value="4">4</option>
<option value="5+">5+</option>
</select></div>
</div>

<div class="ship-wrap">
<div class="checkbox"><label for="gvb3000"><input type="checkbox" name="gvb3000" id="gvb3000" /> Model GVB3000</label></div>
<div id="quantity-28" class="quantity"><label for="quantity-gvb3000">Quantity: </label>
<select name="quantity-gvb3000" id="quantity-gvb3000">
<option value="1">1</option>
<option value="2">2</option>
<option value="3">3</option>
<option value="4">4</option>
<option value="5+">5+</option>
</select></div>
</div>
<div class="ship-wrap">
<div class="checkbox"><label for="nv300"><input type="checkbox" name="nv300" id="nv300" /> Model NV300</label></div>
<div id="quantity-29" class="quantity"><label for="quantity-nv300">Quantity: </label>
<select name="quantity-nv300" id="quantity-nv300">
<option value="1">1</option>
<option value="2">2</option>
<option value="3">3</option>
<option value="4">4</option>
<option value="5+">5+</option>
</select></div>
</div>

<div class="ship-wrap">
<div class="checkbox"><label for="bb3000"><input type="checkbox" name="bb3000" id="bb3000" /> Model BB3000</label></div>
<div id="quantity-30" class="quantity"><label for="quantity-bb3000">Quantity: </label>
<select name="quantity-bb3000" id="quantity-bb3000">
<option value="1">1</option>
<option value="2">2</option>
<option value="3">3</option>
<option value="4">4</option>
<option value="5+">5+</option>
</select></div>

The script that talks with the shipping form (the above html) is called single.php and the script for displaying the requested items (the backend quoting system) is called reply.php. (Now the following scripts have ALL products included, but I am only having issues with the Gun Vault Products: GV2000S, GVB2000, GV3000, GVB3000, NV300, BB3000).

single.php

reply.php

[mac_gyver]

i realize this reply doesn't directly address the problem, but you have too much hard-code logic, repeated for each product that has been added, and in the case of some of the products, wasn't added correctly. i doubt anyone is going to try and find in your 200+/1600+ line files why some of the data isn't doing what you expect.

 

you need to GREATLY simplify your logic by letting the computer do the repetitive work, not you. you should have a list of product information stored somewhere (database table/array) and you use this information to dynamically produce your form. your form would use array(s) for the fields. this would allow you to use array functions (a foreach(){} loop) to process all the submitted data in the same way. nothing would get left out because all the submitted values, no matter how many of them there are, will be processed by the looping code. i would estimate all your code added together could be reduced to less then 200 lines and adding, subtracting, or changing any of the product information would take place in the database table/array where you have defined it, not by editing the actual logic in your code.

[Psycho]

Um, posting all that code is a sure way to get people to NOT respond. We aren't familiar with your code so we expect the poster to identify the section(s) of code where the problem is and to state what the expected results are and what the actual results are.

 

EDIT: I was going to state the same things that mac_gyver stated above about the code not being written in an intelligent way. You could probably reduce that code to less than 10% of its current size if done correctly. For, example, there is a ton of code that does nothing but create a new variable for one that already exists

$t14 = $newArray['T14'];

Why do that? Just use $newArray['T14'] where you need to reference that value.

 

 

I'll also add that the database design is poorly constructed as well. It looks like their is a single table with a heck of a lot of columns. There should instead be at least one second table for the products.

 

EDIT#2: Also, if the emails are being sent but the database INSERT is not taking place, then the logic is flawed. You should not sent a confirmation email if you haven't verified that everything succeeded.

Edited August 19, 2013 by Psycho

[Craptacular]

Thanks you Psycho and mac_gyver. I inherited this code and haven't gotten around to the simplification you are talking about, because I totally agree!

[.josh]

yeah.. sorry that you've been pullin' your hair out for 2 days now, but not gonna sort through tons of code. 

 

But in general, here are some tips to help:

 

1) You said it works for some of your items, but not all.  This usually means that you've typoed a variable or form field name somewhere.  Make sure the form field names line up with your $_POST variable and whatever other variable(s) you push it to.  Also make sure they are not typoed in your query string, columns in your table are right, etc..

 

2) make sure your form is actually pointing to your single.php.  I know this is probably dumb but you never know. Maybe you have several scripts and forms floating around and you're pointing to the wrong one

 

3) Add the following to your single.php

echo "<pre>";print_r($_POST);echo "</pre>"; exit();

This will dump what was received from your form.  Make sure the expected variables/values are there

 

4) echo out your db query string (whatever variable you put your query into):

echo $query; exit();

Make sure it looks right.  Copy and paste it directly into your database via phpmyadmin or commandline or however else you directly interact with your database.  Does it throw an error?

 

5) add the following to your query execution (not sure what you are using, but hopefully you can figure out the equivalent for you) :

$result = mysqli_query($conn, $query) or die (mysqi_error());

Does this produce an error?

[Psycho]

Thanks you Psycho and mac_gyver. I inherited this code and haven't gotten around to the simplification you are talking about, because I totally agree!

 

Well, you are seeing now why that should be the first priority. When you have 50 sections of code that do the same thing then it is very easy for a bug to be in one of those sections and be difficult to find/fix. Whereas, if you have a single process to do that process 50 times, then any bug should be easy to find and fix.

[Craptacular]

Well, you are seeing now why that should be the first priority. When you have 50 sections of code that do the same thing then it is very easy for a bug to be in one of those sections and be difficult to find/fix. Whereas, if you have a single process to do that process 50 times, then any bug should be easy to find and fix.

Yeah... I was retarded and pasted the code twice. I am uploading the updated files now. Derp on me! It's been a long few days!

[mac_gyver]

and to get you started, here is what all that mass/mess of posted html would look like -

<?php
// define the products (somewhere)
$products['gv2000s'] = array('description'=>'');
$products['gvb2000'] = array('description'=>'');
$products['gv3000s'] = array('description'=>'');
$products['gvb3000'] = array('description'=>'');
$products['nv300'] = array('description'=>'');
$products['bb3000'] = array('description'=>'');


$i = 1; // incrementing value for dom id's
foreach($products as $key=>$value){
    $model = strtoupper($key);
    echo "<div class='ship-wrap'>
        <div class='checkbox'>
            <label for='chk-$i'><input type='checkbox' name='chk[$key]' id='chk-$i' /> Model $model</label>
        </div>
        <div id='quantity-$i' class='quantity'><label for='qty-$i'>Quantity: </label>
        <select name='qty[$key]' id='qty-$i'>
        <option value='1'>1</option>
        <option value='2'>2</option>
        <option value='3'>3</option>
        <option value='4'>4</option>
        <option value='5+'>5+</option>
        </select>
        </div>
    </div>";
}
?>

[Craptacular]

 

and to get you started, here is what all that mass/mess of posted html would look like -

<?php
// define the products (somewhere)
$products['gv2000s'] = array('description'=>'');
$products['gvb2000'] = array('description'=>'');
$products['gv3000s'] = array('description'=>'');
$products['gvb3000'] = array('description'=>'');
$products['nv300'] = array('description'=>'');
$products['bb3000'] = array('description'=>'');


$i = 1; // incrementing value for dom id's
foreach($products as $key=>$value){
    $model = strtoupper($key);
    echo "<div class='ship-wrap'>
        <div class='checkbox'>
            <label for='chk-$i'><input type='checkbox' name='chk[$key]' id='chk-$i' /> Model $model</label>
        </div>
        <div id='quantity-$i' class='quantity'><label for='qty-$i'>Quantity: </label>
        <select name='qty[$key]' id='qty-$i'>
        <option value='1'>1</option>
        <option value='2'>2</option>
        <option value='3'>3</option>
        <option value='4'>4</option>
        <option value='5+'>5+</option>
        </select>
        </div>
    </div>";
}
?>

Nice! Thanks a bunch!

[mac_gyver]

and the processing code would simply be -

// define the products (somewhere)
$products['gv2000s'] = array('description'=>'');
$products['gvb2000'] = array('description'=>'');
$products['gv3000s'] = array('description'=>'');
$products['gvb3000'] = array('description'=>'');
$products['nv300'] = array('description'=>'');
$products['bb3000'] = array('description'=>'');

if($_SERVER['REQUEST_METHOD'] == 'POST'){
    if(!isset($_POST['chk'])){
        echo 'No products were picked.';
    } else {
        foreach($products as $key=>$value){
            if(isset($_POST['chk'][$key])){
                // a checked checkbox was found, use the submitted quantity any way you want.
                $model = strtoupper($key);
                echo "You picked Qty: {$_POST['qty'][$key]} of Model $model<br>";
            }
        }
    }
}

where i am just echoing the qty/model, you would validate and store the submitted values to be inserted into your database table and put into the email.

[Psycho]

OK, here are some suggestions:

 

1. To find the problem check for errors on the query. After you run a query, check to see if there are errors. E.g.

 

$res = mysqli_query($mysqli, $sql);
if(!$res)
{
    die("The query failed. Error:<br> " . mysql_error() . "<br>Query: $sql");
}

Note: don't do this in your production environment, else customers will see the error. That is just a down-and-dirty debug process. You can build better debugging process so users get a friendly message while you get a better description.

 

2. I see that all the values from the DB are being run through strip_tags - that tells me the data isn't being properly escaped before being saved.

 

2. Although rebuilding the DB schema is a definite thing to do, in the interim you can reduce/improve the code as it stands. I see there is some sort of number associated with each product (which should be in the DB). So, for now, create an array of all the product codes and the associated number.  Then create another array of the ship methods and the associated index name. Then use loops to run the same processes for each product and ship method.

 

For example, you have about 400 lines of code that do this:

 

if (!empty($t8))
    {$model = $model . "Shipping quote for $qt8 of Model T8.<br />";
    $model_quote = $model_quote . printInvoice("Residential - Curbside", "T8", 399, $qt8, $t8_quote_residence, $total);
    $model_quote = $model_quote . printInvoice("Residential - Garage", "T8", 399, $qt8, $t8_quote_residence2, $total);
    $model_quote = $model_quote . printInvoice("Residential - Curbside No Lift Gate", "T8", 399, $qt8, $t8_quote_residence3, $total);
    $model_quote = $model_quote . printInvoice("Business - Lift Gate Required", "T8", 399, $qt8, $t8_quote_business1, $total);
    $model_quote = $model_quote . printInvoice("Business - No Lift Gate Required", "T8", 399, $qt8, $t8_quote_business, $total);
    $model_quote = $model_quote . printInvoice("Truck Terminal*", "T8", 399, $qt8, $t8_quote_terminal, $total);
    $model_quote = $model_quote . printInvoice("Local Pickup - Vancouver, WA", "T8", 399, $qt8, 0.01, $total);
    $model_quote = $model_quote . "<tr><td> </td><td> </td><td> </td><td> </td><td> </td></tr>";
    }

 

You can replace all of that with about 20 lines of code - not counting the arrays you would create. (Note: I used the values returned from the DB query rather than creating a bunch of unnecessary variables.

 

//Create array of all product codes and the associated numbers
$prod_codes = ('t8', 't8e', 't14',  . . . );
//Create array of the ship methods and the partial index name that goes with each
$ship_types = array(
    'Residential - Curbside' => 'quote-residence',
    'Residential - Garage' => 'quote-residence2',
    'Residential - Curbside No Lift Gate' => 'quote-residence3',
    'Business - Lift Gate Required' => 'quote-business1',
    'Business - No Lift Gate Required' => 'quote-business',
    'Truck Terminal*' => 'quote-terminal',
    'Local Pickup - Vancouver, WA' => false
    );

//Run loops to process each product and each ship methos
foreach($prod_codes as $code => $num)
{
    $upperCode = strtoupper($code);
    if (!empty($row[$upperCode]))
    {
        $model .= "Shipping quote for {$row['Q{$code}']} of Model {$upperCode}.<br />";
        foreach($ship_types as $shipLabel => $shipIndex)
        {
            $shipPrice = ($shipIndex == false) ? '0.01' : $row["{$code}-{$shipIndex}"];
            $model_quote .= printInvoice($shipLabel, $upperCode, $num, $code, $shipPrice, $total);
        }
        $model_quote .= "<tr><td> </td><td> </td><td> </td><td> </td><td> </td></tr>";
    }
}

 

Now, this won't work as written, but is just to give you an idea of how you can approach this.. The problem is that in some places you only include part of the code. So, to reference the correct value you have to "know" the start of the code. E.g. some places you have 'qt8' and in others just 't8'. Since it is not consistent, it makes it difficult to code it appropriately. So, there is quite a bit of renaming in the database that may need to occur. But, really, all that info should be in a separate table!

[Craptacular]

and the processing code would simply be -

// define the products (somewhere)
$products['gv2000s'] = array('description'=>'');
$products['gvb2000'] = array('description'=>'');
$products['gv3000s'] = array('description'=>'');
$products['gvb3000'] = array('description'=>'');
$products['nv300'] = array('description'=>'');
$products['bb3000'] = array('description'=>'');

if($_SERVER['REQUEST_METHOD'] == 'POST'){
    if(!isset($_POST['chk'])){
        echo 'No products were picked.';
    } else {
        foreach($products as $key=>$value){
            if(isset($_POST['chk'][$key])){
                // a checked checkbox was found, use the submitted quantity any way you want.
                $model = strtoupper($key);
                echo "You picked Qty: {$_POST['qty'][$key]} of Model $model<br>";
            }
        }
    }
}

where i am just echoing the qty/model, you would validate and store the submitted values to be inserted into your database table and put into the email.

Got it! Thanks for the help on cleaning this thing up!

[Craptacular]

OK, here are some suggestions:

 

1. To find the problem check for errors on the query. After you run a query, check to see if there are errors. E.g.

$res = mysqli_query($mysqli, $sql);
if(!$res)
{
    die("The query failed. Error:<br> " . mysql_error() . "<br>Query: $sql");
}

Note: don't do this in your production environment, else customers will see the error. That is just a down-and-dirty debug process. You can build better debugging process so users get a friendly message while you get a better description.

 

2. I see that all the values from the DB are being run through strip_tags - that tells me the data isn't being properly escaped before being saved.

 

2. Although rebuilding the DB schema is a definite thing to do, in the interim you can reduce/improve the code as it stands. I see there is some sort of number associated with each product (which should be in the DB). So, for now, create an array of all the product codes and the associated number.  Then create another array of the ship methods and the associated index name. Then use loops to run the same processes for each product and ship method.

 

For example, you have about 400 lines of code that do this:

if (!empty($t8))
    {$model = $model . "Shipping quote for $qt8 of Model T8.<br />";
    $model_quote = $model_quote . printInvoice("Residential - Curbside", "T8", 399, $qt8, $t8_quote_residence, $total);
    $model_quote = $model_quote . printInvoice("Residential - Garage", "T8", 399, $qt8, $t8_quote_residence2, $total);
    $model_quote = $model_quote . printInvoice("Residential - Curbside No Lift Gate", "T8", 399, $qt8, $t8_quote_residence3, $total);
    $model_quote = $model_quote . printInvoice("Business - Lift Gate Required", "T8", 399, $qt8, $t8_quote_business1, $total);
    $model_quote = $model_quote . printInvoice("Business - No Lift Gate Required", "T8", 399, $qt8, $t8_quote_business, $total);
    $model_quote = $model_quote . printInvoice("Truck Terminal*", "T8", 399, $qt8, $t8_quote_terminal, $total);
    $model_quote = $model_quote . printInvoice("Local Pickup - Vancouver, WA", "T8", 399, $qt8, 0.01, $total);
    $model_quote = $model_quote . "<tr><td> </td><td> </td><td> </td><td> </td><td> </td></tr>";
    }

You can replace all of that with about 20 lines of code - not counting the arrays you would create. (Note: I used the values returned from the DB query rather than creating a bunch of unnecessary variables.

//Create array of all product codes and the associated numbers
$prod_codes = ('t8', 't8e', 't14',  . . . );
//Create array of the ship methods and the partial index name that goes with each
$ship_types = array(
    'Residential - Curbside' => 'quote-residence',
    'Residential - Garage' => 'quote-residence2',
    'Residential - Curbside No Lift Gate' => 'quote-residence3',
    'Business - Lift Gate Required' => 'quote-business1',
    'Business - No Lift Gate Required' => 'quote-business',
    'Truck Terminal*' => 'quote-terminal',
    'Local Pickup - Vancouver, WA' => false
    );

//Run loops to process each product and each ship methos
foreach($prod_codes as $code => $num)
{
    $upperCode = strtoupper($code);
    if (!empty($row[$upperCode]))
    {
        $model .= "Shipping quote for {$row['Q{$code}']} of Model {$upperCode}.<br />";
        foreach($ship_types as $shipLabel => $shipIndex)
        {
            $shipPrice = ($shipIndex == false) ? '0.01' : $row["{$code}-{$shipIndex}"];
            $model_quote .= printInvoice($shipLabel, $upperCode, $num, $code, $shipPrice, $total);
        }
        $model_quote .= "<tr><td> </td><td> </td><td> </td><td> </td><td> </td></tr>";
    }
}

Now, this won't work as written, but is just to give you an idea of how you can approach this.. The problem is that in some places you only include part of the code. So, to reference the correct value you have to "know" the start of the code. E.g. some places you have 'qt8' and in others just 't8'. Since it is not consistent, it makes it difficult to code it appropriately. So, there is quite a bit of renaming in the database that may need to occur. But, really, all that info should be in a separate table!

Okay. But when I put the debugging code in there, nothing happens so it should be populating the DB. Hmmm. Well thanks for all the work and help with getting this thing looking less cluttered and running more efficiently!

[Craptacular]

Here are some updated files. I haven't applied the edits suggested yet but have reduced the extra lines of code by keeping only the relevant code in the files.

Thanks for having a look!

shippingQuote.php

reply.php

single.php

[Psycho]

Okay. But when I put the debugging code in there, nothing happens so it should be populating the DB. Hmmm. Well thanks for all the work and help with getting this thing looking less cluttered and running more efficiently!

 

OK, so take the debugging a step further. Put some echos in there to tell you EXACTLY what is happening instead of just assuming what is happening and getting frustrated.

$res = mysqli_query($mysqli, $sql);
if(!$res)
{
    die("The query failed. Error:<br> " . mysql_error() . "<br>Query: $sql");
}
elseif(!mysql_affected_rows())
{
    die("Query did not INSERT/UPDAT any rows!<br>Query: $sql");   
}

Debugging can be challenging, but it is not terribly difficult. Don't just look at the final results, see there is a problem and then read through the code trying "see" the error. Determine a point in your code where you think the problem may be occurring and test it.

 

1. Verify the the data coming into that process is what you expect. If the data coming into the process is not what you expect, then the error is occurring at an earlier point and you need to move your debugging efforts to an earlier point in the logic of the code.

 

2. Assuming the input data is correct, next verify that the output/results from that process are what you expect.If not, then you've found the source of the problem. Now you can analyze that section of code to determine and fix the problem.

 

3. If the input and output at that section of code is as you expect, then the error is occurring somewhere after that in the logic and you need to move your debugging efforts further down in the logic.

 

Also, for complicated processes, it's not a bad idea to insert debugging code and leaving it. Just make sure that it won't be displayed in a production environment (unless you can turn it on selectively). For example, you can create a "debug()" function that is called at certain points in the code to output the value of certain values or data about the results of certain operations. You could create that function so, the first thing it does is check a constant (that you define) to determine whether it should do the debugging or not.

 

Example:

define("DEBUG_MODE", TRUE); //Change this to turn debugging on/off
function debug($debugText)
{
    //If debug mode is not on - do nothing
    if(!DEBUG_MODE) { return false; }
 
    //Debug mode is on
    //Insert code to output debug message to screen or database
    echo "Debug: {$debugText}<br>\n";
}

Then in your code add statements that will help you later if there is a problem

debug("Quantity: {$quantity}, Price: {$price}"); //Check input values
$total = shippingCost($quantity, $price);
debug("Total: {$total}"); //Check results

Edited August 19, 2013 by Psycho

[Craptacular]

It was a stupid DB issue. The script wasn't breaking anything. Thanks for all the help and suggestions for cleaning this feature up!