I can't seem to find a working solution in stackoverflow. I wish to update the form values with the updated data that is submitted in the form post. The database updates but after the refresh, the values are still the same. Any help would be appreciated. Thanks!
<?php
global $wpdb;
$user_id = get_current_user_id();
$org_id = get_user_meta($user_id, '_org_id', true);
$orgs = $wpdb->get_results("select * from wp_organization");
$user_org = null;
foreach($orgs as $struct) {
if ($org_id == $struct->id) {
$user_org = $struct;
break;
}
}
?>
<?php
$connection = mysqli_connect("address", "login", 'password');
$db = mysqli_select_db($connection, 'databasename');
if(isset($_POST['update'])) {
$id = $org_id;
$query = "UPDATE `wp_organization` SET
name = '$_POST[org_name]',
shortname = '$_POST[shortname]',
industry = '$_POST[industry]',
description = '$_POST[description]'
where id = $id ";
$query_run = mysqli_query($connection, $query);
}
}
?>
<!-- organization info form -->
<form method="post" action="">
<h2 >Organization Information</h2>
<label >Name</label>
<input type="text" required name="org_name" value="<?php echo $user_org-> name; ?>"
/>
<label >Industry</label>
<input type="text" name="industry" value="<?php echo $user_org-> industry; ?>" />
<label >Short name (4 characters)</label>
<input type="text" name="shortname" required maxlength="4" value="<?php echo $user_org-> shortname; ?>" />
<label >Description</label>
<textarea name="description" rows="5" cols="50"><?php echo trim(stripslashes($user_org-> description)); ?></textarea>
<div >
<input type="submit" name="update" value='Update' class='btn btn-primary'>
</div>
</form>
CodePudding user response:
The main issue that's causing old data to appear is that you are querying the database before you update it. The data displayed in the form is queried at the beginning of the page's execution, and later, if there's POST data, the database is updated. As a result, the data used to render the form is the old data, not the recently updated data. So in a very minimal way, you could simply switch the database query with the database update, and your data flow would work as intended.
However, there's several other issues with this code, most notably the SQL attack vulnerability. As a result, I'll address all of these issues in creating a complete and safe answer to this question.
- This is WordPress!
Based on the presence of the $wpdb
variable, as well as some of the table names, it appears that this is running as part of a WordPress site (or else alongside a WordPress install). As a result, you never need to make a any of the raw mysqli_
calls. Instead, use the the methods documented in the $wpdb
class which provide safe access to the database, and also will help ensure that any future changes to the WordPress database system don't break your code.
Result: Remove mysqli_
methods.
- Watch out for unnecessary syntax
There's several unnecessary bits of syntax which may have undesirable side effects. For instance, in the middle of the code you have
?>
<?php
What this code snippet actually does is insert a phantom bit of whitespace (in this case one literal newline character or \n
) into the output of your page. If, for instance, you tried to set a header after this value you would get a difficult and mysterious error about output already having been sent. A good practice is that all of your business code lives in a single, contiguous block with just a single <?php
at the top of your file.
In addition, your block that begins if(isset($_POST['update']))
ends with an extra dangling closing bracket }
.
Furthermore, inside your actual update section, the calls to the $_POST
superglobal are not actual strings but raw tokens (such as $_POST[shortname]
. I suspect you did this in order to take advantage of PHPs string interpolation. While this raw format does work for legacy reasons, it's much clearer and safer when interpolating a string like this to use something like {$_POST['shortname']}
inside your string.
Result: Remove and clean up unnecessary syntax.
- SQL Injection! Oh My!
The biggest scary bit of this code is wide open SQL injection attack that's enabled by passing un-trusted strings (in the form of the $_POST
variables) to SQL. This has been the bane of thousands (if not millions) of websites. Anyone who can access your web page can pass any data they want into the form field, including SQL commands which can delete your database and compromise your server. Using raw SQL like this is wildly dangerous and should never be used. PHP includes a great library PDO
for this, and there are many other options. However, as discussed above, since you're using WordPress, you can just use the $wpdb
, which also has protections against SQL injection built in and provides further compatibility benefits with the WordPress database.
Result: Don't use raw SQL
- Update then Query (but Query first, too)
The core problem that caused you to post the question was data not updating in the web page when a form is submitted until the next load. This is because you currently query the data from the database, then update the database after that. Since you use the result from the early query to display values in the web page, they're naturally not the new values.
I suspect you did this because you need the value of $org_id
in order to create and query and as a result all your querying got clumped together. Nonetheless, this primary fetch of the $org_id
needs to be in the beginning, but the rest of the query after the update.
Result: Fix query and update ordering.
- No data validation
You blindly use the value of $_POST
values, but these may not be set. In your example this will probably only lead to empty database values, but in other context this could be catastrophic. As mentioned above, anyone can request your page with any $_POST
data they desire. Therefore, it's good practice to ensure that you provide default values in case the data sent to the server is missing some fields.
Result: Validate that all expected data is present.
A version of this code that updates the database before querying, so the data is correct on each page load and update, as well as incorporating the other fixes mentioned, might look something like this:
<?php
/// 1. Get the global WordPress Database Object
global $wpdb;
/// 2. Use WP helper functiosn to query the current user and org IDs
$user_id = get_current_user_id();
$org_id = get_user_meta($user_id, '_org_id', true);
/// 3. Update the database if necessary
if (isset($_POST['update'])) {
/// 3.i Prepare the variables
// Note: In new versions of PHP, each of the isset ? : lines below could be replaced
// with something like:
//
// $name = $_POST['org_name'] ?? "default org name";
//
// I've used the long version here, so you can really see what ?? means. You can also
// put any value you want in the final string of these expressions, so for example
//
// $name = isset($_POST['org_name']) ? $_POST['org_name'] : "unknown";
//
// would provide the value "unknown" into the database. You could inline all of these
// checks into the following call to $wpdb, but that would make for a very hard to read
// line. Setting local variables to these values makes the code easier to follow.
$name = isset($_POST['org_name']) ? $_POST['org_name'] : "";
$short = isset($_POST['shortname']) ? $_POST['shortname'] : "";
$ind = isset($_POST['industry']) ? $_POST['industry'] : "";
$desc = isset($_POST['description']) ? $_POST['description'] : "";
/// 3.ii Execute the query
// Note: I assume that $org_id is an integer. If it's actually a string, then that
// line in the query should read
//
// where id = %s
$wpdb->query(
$wpdb->prepare("
UPDATE `wp_organization` SET
name = %s,
shortname = %s,
industry = %s,
description = %s
where id = %d
",
$name, $short, $ind, $desc, $org_id
)
);
// Alternatively to the raw $wpdb->query above, you could use the built in update method
// of $wpdb, like so
//
// $wpdb->update('wp_organization', array(
// 'name' => $name,
// 'shortname' => $short,
// 'industry' => $ind,
// 'description' => $desc,
// ),
// array( 'id' => $org_id ),
// array('%s', '%s', '%s', '%s'),
// array('%d')
// );
//
// This has the advantage that you don't have to write any SQL, but may be harder to understand
// if you come back to maintain this code far in the future.
}
/// 4. Load the current value of $user_org, this will reflect any database changes made above
$orgs = $wpdb->get_results("SELECT * FROM `wp_organization`");
$user_org = null;
foreach($orgs as $struct) {
if ($org_id == $struct->id) {
$user_org = $struct;
break;
}
}
/// 5. Render the form
?>
<!-- organization info form -->
<form method="post" action="">
<h2 >Organization Information
<label >Name
<input type="text" required name="org_name" value="<?php echo $user_org->name; ?>" />
<label >Industry
<input type="text" name="industry" value="<?php echo $user_org->industry; ?>" />
<label >Short name (4 characters)
<input type="text" name="shortname" required maxlength="4" value="<?php echo $user_org->shortname; ?>" />
<label >Description
<textarea name="description" rows="5" cols="50"><?php echo trim(stripslashes($user_org->description)); ?>
<div >
<input type="submit" name="update" value='Update' class='btn btn-primary'>
</div>
</form>