Raj R96
Raj R96

Reputation: 50

PHP - $_GET variable not set

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

Answers (7)

Raj R96
Raj R96

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

ArtisticPhoenix
ArtisticPhoenix

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

asiby
asiby

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

Cygital
Cygital

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

AgileFox
AgileFox

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

Dharman
Dharman

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

imposterSyndrome
imposterSyndrome

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

Related Questions