Simple while loop help DOH!

Hi all

I been having some issues with a function I am coding for a eCommerce site, and this custom task needed is nearly done bar sorting out if they ordered more than one.

Basically you order a product and the transaction email pulls a license key from a table and posts it below the product title in the email then deletes that key from the DB

But I need to check that if the quantity is more than one post an additional key for every extra one. Easier said than done it seems...

Hope you guys can help, its one of those where I know it can be done but cant find out how too



then echos into html

you inner while loop needs to have an array i.e.

not this: $license = $row_product_license['licenseid'];
but thus: $license[] = $row_product_license['licenseid'];

otherwise if you are only expecting ONE licence then dont loop its pointless.

Hi Paul

Thanks for response, so your saying I need to have the LicenseID fed into an array so I can output numerous licenses

I added this to a While loop as I thought it was needed as the other values were looped (not the best developer in town lol)

How would I do this to check against the quantity amount?

use LIMIT in your SQL
something like this:


Below are some points that can be an improvement to your code:
- Combine the product and license query by using JOIN instead.
- Prevent possible SQL injections. (maybe you do have this as I cant see how you receive the user input)

When you only want to fetch one row, then you only need one mysql_fetch_array (or whatever you use).

This is somehow a common misunderstanding. People get so used to the "while (... fetch ...) {...}" that they'll use it everywhere, even when it's completely unnecessary.

And an addition to MrFujin's list:

Always specify the columns explicitly, don't use "SELECT *". This "fetch all" pattern is bad practice, because it retrieves more data than you actually need and might even fetch internal columns that you certainly don't want to output (like credit card numbers or whatever).

No offense, but I'm not sure if it's a good idea to work on eCommerce code when you don't seem to have much PHP experience. There's a lot you can do wrong, which will lead to massive security holes.

Thank you for your input and no offence taken at the end of the day I am learning and you guys know more than me that is why I am here.

In regards to security holes for ecommerce the license keys are within a separate table from other details and they are only being outputted to a html email that is all so I dont feel it will create and security holes. As you said I would not dream of touching any of the ecommerce "internals".

I have taken the array out of a while loop and tried the LIMIT but I dont see how this will count what the quantity is and output the amount of keys referenced to the quantity

Well, unfortunately it's not that simple. Security problems are likely to affect the whole page, not just your specific section. For example, if your queries are vulnerable to SQL injections, an attacker might be able to rewrite the query and fetch critical data from a completely different table.

So you should at least read up on current best practices and security precautions before you start programming. For example, every value you insert into a query or HTML string must be escaped carefully with the right method (mostly mysql_real_escape_string and htmlentities). It would be better to drop the obsolete "mysql_" functions completely and switch to a modern library which supports prepared statements, but I guess that's hard to integrate into Wordpress.

And if you know anybody with solid (and up to date) knowledge about PHP, ask him to review your final code. That's not really something for a forum.





The "LIMIT" clause by MrFujin will limit the number of selected licences to the quantity. You can then output those licences and afterwards check if there are licences missing.

So you do in fact need a "while" loop, since you have multiple rows. The licences must either be saved in an array (like paulh1983 suggested) or appended to an output string. In your original code you just kept overwriting the same variable, so you ended up with only one license (namely the last one from the query).


(completely untested!)

Thank you Jacques1 much appreciate your advice and will take them on board!

I have tested the code and tried to understand how you have done it...

heres what I have updated with my values. It gives a mysql_num_rows(): supplied argument is not a valid MySQL result resource warning at the if statement line. and the result out puts the please contact.... line

is it because of the == $product_quantity

Why did you change the query code? Since you replaced all backticks `` (for identifiers) with single quotes '' (for values), you'll get a syntax error. You now have values instead of column and table names.

Secondly, there's a reason why I put the query on multiple lines with indentation. This makes the query much more readable and less prone to errors.

Please accept my apologies I changed the back ticks as I thought it was how they were copied over as sometimes copying code makes the '' into ``. But I never knew there was a difference so thank you for that totally understand the code now

plus I have whacked in the code with a few extra } and works perfectly! thank you for your help it is much appreciated.

Hi all

am back again

After the success with the code I thought I'd be able to do this simple task but cant find a solution.

Basically want to delete the keys that are in the loop from the db once they have been outputted.

I found a solution to delete the values that have only one quantity but the one that gives 2 doesn't delete and am also worried I am not doing it securely.

I have got it to work by adding the DELETE within the while and using the $license['licenseid']

is this secure?

No, that's not secure, because again you didn't escape the values before inserting them into the query string.

Never insert a raw value into anything that's evaluated (like an SQL query or HTML markup or a console command). If you do, an attacker will be able to manipulate the query/markup/command and do all kind of bad things like logging in as the administrator, stealing credit card numbers, destroying data and whatnot (depending on how well the system is configured). Ask Sony how bad an SQL injection can get.

So whenever this situation occurs, you must be extremely careful. Every value must be escaped according to the specific context (as I already said above). The ideal solution would be to completely separate code and values, but unfortunately, that's not always possible.

Again: Never insert unescaped values into code. Always use mysql_real_escape_string() or whatever is the right method for the current context. I must admit that I did the same mistake by not escaping $license['licenseid'] while constructing the $output string.

Apart from that, I wouldn't delete the keys one by one. It's probably better to first collect all keys in an array and then send to single query to delete them all at once:

Before you delete the products keys, I really hope you have the products keys stored in another place, at least in a backup or similar.

What if something unexpected happens and the user didn't get the keys?

Yeah. You should actually use a kind of "deleted" flag instead of actually deleting the keys. There are many situation where you might have to get void keys.