Reputation: 50
Initially, I was going to have 3 separate files for creating, update and delete but to cut down the number of files I decided to make it all in 1 file. The delete action was working when the href was going to a page that was only handling the delete process. The issue is my $_GET doesn't seem to be working in the processor when it shares a file with create and update. Currently, if you click on Delete it creates a null record in the table so it's going to the create function.
viewCountries.php
$CountryID = $row['COUNTRYID'];
<tr>
<td><?php echo $row['COUNTRYNAME']; ?></td>
<td><?php echo $row['GDP']; ?></td>
<td><a href="editCountry.php?CountryID=<?php echo $CountryID; ?>" class="btn btn-warning">Edit</a></td>
<td><a href="processor/countryProcessor.php?CountryID=<?php echo $CountryID; ?>" class="btn btn-danger">Delete</a></td>
countryProcessor.php
<?php
require('../scripts/x_connect.php');
if(isset($_POST)) {
// Create Country
if(!isset($_POST['CountryID'])) {
if(isset($_POST['CountryName'])) {
$CountryName = $_POST['CountryName'];
}
if(isset($_POST['Gross'])){
$Gross = $_POST['Gross'];
}
$stmt = oci_parse($conn, "INSERT INTO COUNTRY (COUNTRYNAME, GDP) VALUES (:CountryName, :GDP)");
oci_bind_by_name($stmt, ":CountryName", $CountryName);
oci_bind_by_name($stmt, ":GDP", $Gross);
oci_execute($stmt);
$Affected = oci_num_rows($stmt);
oci_commit($conn);
oci_free_statement($stmt);
oci_close($conn);
// echo $Gross;
// echo $CountryName;
if(count($Affected) > 0){
header("Location: ../viewCountries.php?Success=$CountryName has been created!");
} else {
header("Location: ../viewCountries.php?Danger=$CountryName hasn't been created!");
}
// Update Country
} else {
if(isset($_POST['CountryID'])) {
$CountryID = $_POST['CountryID'];
}
if(isset($_POST['CountryName'])) {
$CountryName = $_POST['CountryName'];
}
if(isset($_POST['Gross'])){
$Gross = $_POST['Gross'];
}
$stmt = oci_parse($conn, "UPDATE COUNTRY SET COUNTRYNAME = :CountryName, GDP = :GDP WHERE COUNTRYID = :CountryID");
oci_bind_by_name($stmt, ":CountryID", $CountryID);
oci_bind_by_name($stmt, ":CountryName", $CountryName);
oci_bind_by_name($stmt, ":GDP", $Gross);
oci_execute($stmt);
$Affected = oci_num_rows($stmt);
oci_commit($conn);
oci_free_statement($stmt);
oci_close($conn);
// echo "CountryID" . ' ' . $CountryID . "<br>";
// echo "GDP" . ' ' . $Gross . "<br>";
// echo "Country Name" . ' ' . $CountryName . "<br>";
// echo "Rows Affected" . ' ' . $Affected;
if(count($Affected) > 0){
header("Location: ../viewCountries.php?Success=$CountryName has been updated!");
} else {
header("Location: ../viewCountries.php?Danger=$CountryName hasn't been updated!");
}
}
} else {
// Delete Country
if(isset($_GET['CountryID'])) {
$CountryID = $_GET['CountryID'];
$stmt = oci_parse($conn, "DELETE FROM COUNTRY WHERE COUNTRYID = :CountryID");
ocibindbyname($stmt, ":CountryID", $CountryID);
oci_execute($stmt);
$Affected = oci_num_rows($stmt);
oci_commit($conn);
oci_free_statement($stmt);
oci_close($conn);
if(count($Affected) > 0){
header("Location: ../viewCountries.php?Success=Country has been deleted!");
} else {
header("Location: ../viewCountries.php?Danger=Country hasn't been deleted!");
}
}
}
?>
Upvotes: 1
Views: 1259
Reputation: 50
I've decided to make the processor look less confusing by adding a hidden input tag in the view to differentiate between create and update that way. So viewCountries.php stays the same.
createCountry.php
<input type="hidden" name="Action" value="Create">
editCountry.php
<input type="hidden" name="Action" value="Update">
countryProcessor.php
// Create Country
if($_POST['Action'] == "Create") {
if(isset($_POST['CountryName'])) {
$CountryName = $_POST['CountryName'];
}
if(isset($_POST['Gross'])){
$Gross = $_POST['Gross'];
}
$stmt = oci_parse($conn, "INSERT INTO COUNTRY (COUNTRYNAME, GDP) VALUES (:CountryName, :GDP)");
oci_bind_by_name($stmt, ":CountryName", $CountryName);
oci_bind_by_name($stmt, ":GDP", $Gross);
oci_execute($stmt);
$Affected = oci_num_rows($stmt);
oci_commit($conn);
oci_free_statement($stmt);
oci_close($conn);
// echo $Gross;
// echo $CountryName;
if(count($Affected) > 0){
header("Location: ../viewCountries.php?Success=$CountryName has been created!");
} else {
header("Location: ../viewCountries.php?Danger=$CountryName hasn't been created!");
}
// Update Country
} elseif($_POST['Action'] == "Update") {
if(isset($_POST['CountryID'])) {
$CountryID = $_POST['CountryID'];
}
if(isset($_POST['CountryName'])) {
$CountryName = $_POST['CountryName'];
}
if(isset($_POST['Gross'])){
$Gross = $_POST['Gross'];
}
$stmt = oci_parse($conn, "UPDATE COUNTRY SET COUNTRYNAME = :CountryName, GDP = :GDP WHERE COUNTRYID = :CountryID");
oci_bind_by_name($stmt, ":CountryID", $CountryID);
oci_bind_by_name($stmt, ":CountryName", $CountryName);
oci_bind_by_name($stmt, ":GDP", $Gross);
oci_execute($stmt);
$Affected = oci_num_rows($stmt);
oci_commit($conn);
oci_free_statement($stmt);
oci_close($conn);
// echo "CountryID" . ' ' . $CountryID . "<br>";
// echo "GDP" . ' ' . $Gross . "<br>";
// echo "Country Name" . ' ' . $CountryName . "<br>";
// echo "Rows Affected" . ' ' . $Affected;
if(count($Affected) > 0){
header("Location: ../viewCountries.php?Success=$CountryName has been updated!");
} else {
header("Location: ../viewCountries.php?Danger=$CountryName hasn't been updated!");
}
} else {
// Delete Country
if(isset($_GET['CountryID'])) {
$CountryID = $_GET['CountryID'];
$stmt = oci_parse($conn, "DELETE FROM COUNTRY WHERE COUNTRYID = :CountryID");
ocibindbyname($stmt, ":CountryID", $CountryID);
oci_execute($stmt);
$Affected = oci_num_rows($stmt);
oci_commit($conn);
oci_free_statement($stmt);
oci_close($conn);
if(count($Affected) > 0){
header("Location: ../viewCountries.php?Success=Country has been deleted!");
} else {
header("Location: ../viewCountries.php?Danger=Country hasn't been deleted!");
}
}
}
Upvotes: 0
Reputation: 21681
Instead of:
if(isset($_POST)) {
It's better to do this:
if ('POST' === $_SERVER['REQUEST_METHOD']) {
#its prefered to put constants, values, or function calls on the left (but its less intuitive to do)
#this is because you can do this if($_SERVER['REQUEST_METHOD'] = 'POST')
#which will assign `POST` to `$_SERVER['REQUEST_METHOD']` and return true on every request
#without an error and it can be very hard to debug, but the other way around will throw an error.
#so if we do our conditions this way, we can avoid that completely
This tells you if the request to the server was an actual post or not. Where the Super Global $_POST
is always set, it's just empty.
By the way $_SERVER
is another super global like $_COOKIE
,$_POST
or $_GET
. But it contains information about the server and the request headers etc...
There are some considerations when using information from $_SERVER
you should treat most of it as you treat $_POST
or $_GET
in that you cannot trust it to be safe as some of it can be edited by the client or comes from the client.
But that is a whole other topic for another day..
Good Luck!
Upvotes: 2
Reputation: 3399
As others have already mentioned it, if (isset($_POST))
will always return true.
IMHO, I believe that you should consider doing things the proper way while you are at it. Consider reading about CRUD at https://www.codecademy.com/articles/what-is-crud.
You will learn there that a GET call is not usually what you should use for deleting stuff. Deleting should use the DELETE verb/method. Adding should use the POST method and updating should use the PUT method, etc.
Additionally, with your current setup, code can be deleted from your database when a search engine Bot crawls your URLs especially given the fact that there is no confirmation needed before deleting a row from the DB.
At least, make sure that your CountryID is not a sequential ID and that it is some sort of UUID hash that is much more difficult to guess.
Upvotes: 0
Reputation: 26
Check for posted parameter name such isset($_POST['CountryID']) not just isset($_POST).
So the following should work.
<?php
require('../scripts/x_connect.php');
if(isset($_POST['CountryID'])) {
// Create Country
if(!isset($_POST['CountryID'])) {
if(isset($_POST['CountryName'])) {
$CountryName = $_POST['CountryName'];
}
if(isset($_POST['Gross'])){
$Gross = $_POST['Gross'];
}
$stmt = oci_parse($conn, "INSERT INTO COUNTRY (COUNTRYNAME, GDP) VALUES (:CountryName, :GDP)");
oci_bind_by_name($stmt, ":CountryName", $CountryName);
oci_bind_by_name($stmt, ":GDP", $Gross);
oci_execute($stmt);
$Affected = oci_num_rows($stmt);
oci_commit($conn);
oci_free_statement($stmt);
oci_close($conn);
// echo $Gross;
// echo $CountryName;
if(count($Affected) > 0){
header("Location: ../viewCountries.php?Success=$CountryName has been created!");
} else {
header("Location: ../viewCountries.php?Danger=$CountryName hasn't been created!");
}
// Update Country
} else {
if(isset($_POST['CountryID'])) {
$CountryID = $_POST['CountryID'];
}
if(isset($_POST['CountryName'])) {
$CountryName = $_POST['CountryName'];
}
if(isset($_POST['Gross'])){
$Gross = $_POST['Gross'];
}
$stmt = oci_parse($conn, "UPDATE COUNTRY SET COUNTRYNAME = :CountryName, GDP = :GDP WHERE COUNTRYID = :CountryID");
oci_bind_by_name($stmt, ":CountryID", $CountryID);
oci_bind_by_name($stmt, ":CountryName", $CountryName);
oci_bind_by_name($stmt, ":GDP", $Gross);
oci_execute($stmt);
$Affected = oci_num_rows($stmt);
oci_commit($conn);
oci_free_statement($stmt);
oci_close($conn);
// echo "CountryID" . ' ' . $CountryID . "<br>";
// echo "GDP" . ' ' . $Gross . "<br>";
// echo "Country Name" . ' ' . $CountryName . "<br>";
// echo "Rows Affected" . ' ' . $Affected;
if(count($Affected) > 0){
header("Location: ../viewCountries.php?Success=$CountryName has been updated!");
} else {
header("Location: ../viewCountries.php?Danger=$CountryName hasn't been updated!");
}
}
} else {
// Delete Country
if(isset($_GET['CountryID'])) {
$CountryID = $_GET['CountryID'];
$stmt = oci_parse($conn, "DELETE FROM COUNTRY WHERE COUNTRYID = :CountryID");
ocibindbyname($stmt, ":CountryID", $CountryID);
oci_execute($stmt);
$Affected = oci_num_rows($stmt);
oci_commit($conn);
oci_free_statement($stmt);
oci_close($conn);
if(count($Affected) > 0){
header("Location: ../viewCountries.php?Success=Country has been deleted!");
} else {
header("Location: ../viewCountries.php?Danger=Country hasn't been deleted!");
}
}
}
?>
Upvotes: 0
Reputation: 174
As a few others have mentioned $_POST
is always set, so your script is never going into your else{}
clause.
Try this; change your first instance of:
if(isset($_POST))
to
if(sizeof($_POST))
Upvotes: -1
Reputation: 33413
isset()
checks if a variable exists and is not null. In case of $_POST
or $_GET
, they are superglobals and will always exist in every PHP script. Even if the HTTP method was not POST the superglobal will exists and will contain empty array. This means that isset($_POST)
will always return true.
There is another way you can compare if $_POST
contains any values and it is this:
if($_POST){
// your code
}
You could also check what was the request method used with $_SERVER['REQUEST_METHOD']
Upvotes: 0
Reputation: 897
The problem is that the POST super global is always set, it's just empty. If you var_dump post you'll see this. Either check that the post variable is not empty, or check that a specific value is set, e.g. isset(post['submit'])
Upvotes: 1