lukep11a Posted June 22, 2012 Share Posted June 22, 2012 Hi, I have a form which posts team_id[1], team_id[2], team_id[3] etc.. I have tried to create it in a function. The code I have is inserting the data correctly into the table but it is bringing up the error Team Registration failed from the code, I think it's because I am not passing the array correctly in the following line of code: <?php if (submitNewTeam($_POST['user_id'], $_POST['user_team_name'], $_POST['team_id'])) ?> This is the full code: <?php if (isset($_POST['submit_team'])){ if (submitNewTeam($_POST['user_id'], $_POST['user_team_name'], $_POST['team_id'])){ echo "<p class='normal'>Thank you for submitting a new team.</p>"; }else { echo "<p class='fail'>Team registration failed! Please try again.</p>"; show_team_selections(); } } else { // has not pressed the submit button show_team_selections(); } ?> <?php function submitNewTeam($user_id, $user_team_name, $valuesAry) { $query1 = "INSERT INTO user_teams (user_team_id, user_id, team_name, date)VALUES ('', '{$user_id}', '{$user_team_name}', NOW())"; $result = mysql_query($query1) or die("Query: {$query1}<br>Error: ".mysql_error()); $user_team_id = mysql_insert_id(); //Put group selections into arry in MySQL INSERT value format $valuesAry = array(); foreach($_POST['team_id'] as $team_id) { $team_id = mysql_real_escape_string(trim($team_id)); if($team_id != '') { //Only add if not empty $valuesAry[] = "('{$user_team_id}', '{$team_id}', NOW(), '2100-1-1 00:00:01')"; } } $query = "INSERT INTO team_selections (user_team_id, team_id, transfer_in, transfer_out)VALUES " . implode(', ', $valuesAry); $result = mysql_query($query) or die("Query: {$query}<br>Error: ".mysql_error()); } ?> Quote Link to comment Share on other sites More sharing options...
boompa Posted June 22, 2012 Share Posted June 22, 2012 f (submitNewTeam($_POST['user_id'], $_POST['user_team_name'], $_POST['team_id'])) You seem to be expecting the function to return a boolean value, but nowhere do I see you actually returning a value from said function. Why do you accept and insert team_name and user_id without SQL injection sanitation, but actually sanitize the other values prior to insert. You must consistently protect against SQL injection. Quote Link to comment Share on other sites More sharing options...
Psycho Posted June 22, 2012 Share Posted June 22, 2012 There is nothing inherently wrong with how you are passing the array. Let's trace back through the code to figure out what may be wrong. The error you are getting is triggered here: if (submitNewTeam($_POST['user_id'], $_POST['user_team_name'], $_POST['team_id'])) { echo "<p class='normal'>Thank you for submitting a new team.</p>"; } else { echo "<p class='fail'>Team registration failed! Please try again.</p>"; show_team_selections(); } That tells us that submitNewTeam() is not returning a true value. So, now we need to look at that function. function submitNewTeam($user_id, $user_team_name, $valuesAry) { $query1 = "INSERT INTO user_teams (user_team_id, user_id, team_name, date)VALUES ('', '{$user_id}', '{$user_team_name}', NOW())"; $result = mysql_query($query1) or die("Query: {$query1}<br>Error: ".mysql_error()); $user_team_id = mysql_insert_id(); //Put group selections into arry in MySQL INSERT value format $valuesAry = array(); foreach($_POST['team_id'] as $team_id) { $team_id = mysql_real_escape_string(trim($team_id)); if($team_id != '') { //Only add if not empty $valuesAry[] = "('{$user_team_id}', '{$team_id}', NOW(), '2100-1-1 00:00:01')"; } } $query = "INSERT INTO team_selections (user_team_id, team_id, transfer_in, transfer_out)VALUES " . implode(', ', $valuesAry); $result = mysql_query($query) or die("Query: {$query}<br>Error: ".mysql_error()); } Well, that code looks pretty good. You have some error handling in there and it is logically constructed. But I see a couple problem and have some suggestions: 1. The first ptoblem is that you are passing the array of IDs to the function, but then inside the function you are using $_POST['team_id'] instead of the passed value! 2. Don't use mysql_real_escape_string() for ID values. I would assume those should be integer values so you should use intval() or something similar. Because if a non numeric value was passed it would cause an error. 3. Also, you can easily process all the fields in the array without a foreach() loop by using array_map() to apply intval() to all the values and array_filter() to remove empty values. $valuesAry = array_filter(array_map('intval', $valuesAry)); 4. Your queries are using NOW() for the dates. If those fields are supposed to be the creation date or a modification date you should set the fields up in the database so they will be auto-populated/updated. Then you can remove that from your code and let the database do that work. 5. It looks like 'user_team_id' is an auto-increment field. If so, it should be removed from the query. 6. You are running two queries and assigning the results of both to a variable called $result. In both instances you never use that variable. In the first instance that's not a problem. But, in the second you need to be returning that result at the end of the function so your previous if() condition will work! Give this a try <?php function submitNewTeam($user_id, $user_team_name, $teamIDs) { $user_id = intval($user_id); $user_team_name = mysql_real_escape_string($user_team_name); $query = "INSERT INTO user_teams (user_id, team_name, date) VALUES ('{$user_id}', '{$user_team_name}', NOW())"; mysql_query($query1) or die("Query: {$query}<br>Error: ".mysql_error()); $user_team_id = mysql_insert_id(); //Preprocess the array $teamIDs = array_filter(array_map('intval', $teamIDs)); //Put group selections into arry in MySQL INSERT value format $valuesAry = array(); foreach($teamIDs as $team_id) { $valuesAry[] = "('{$user_team_id}', '{$team_id}', NOW(), '2100-1-1 00:00:01')"; } $query = "INSERT INTO team_selections (user_team_id, team_id, transfer_in, transfer_out) VALUES " . implode(', ', $valuesAry); $result = mysql_query($query) or die("Query: {$query}<br>Error: ".mysql_error()); return $result; } ?> Quote Link to comment Share on other sites More sharing options...
lukep11a Posted June 23, 2012 Author Share Posted June 23, 2012 Thank you for that, it now seems to work as I wanted, will keep your suggestions in mind for future use. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.