Simple AJAX and PHP | POST 500 issue [Resolved]

Hi all

I’m trying to update my database using AJAX to execute a PHP file when somebody closes the modal. Sounds simple :neutral_face:

I keep seeing the below error

jquery-3.3.1.js:9600 POST includes/update-photo.inc.php 500

JS

$('#imageId').on("click", function () {
        $.post("includes/update-photo.inc.php");
        modal.close();
});

update-photo.inc.php

include_once('includes/mysqli_connect.inc.php');

$photo_id = 01031901;

if(isset($photo_id)) {
  $mysqli->query("UPDATE photos SET views = views + 1 WHERE photo_id = $photo_id");
}

Any ideas what is causing this and why the database table is not updating?

side note
The SQL works independently as I’ve used this with other snippets before.

Thanks,
Barry

What about the PHP itself, if you call it directly from the browser rather than via your ajax call?

What’s in your database include? Presumably you’ve missed out the php open and close tags for brevity in the forum post?

I’ve basically copied the code from an already active page which has worked for a long time. It’s only now I need to run the code using the javascript modal windows with AJAX.

I’ll double check everything again and run this PHP snippet on its own.

Yes just for viewing purposes.
The include file is basic db connect username password etc.

I’ve just download the error_log file and searching to see if I can spopt the issue.
Report back shortly once I have further information :slight_smile:

Barry

One issue that keeps showing in the logs…

PHP Parse error: Invalid numeric literal in /includes/update-photo.inc.php on line 11

Line 11
$photo_id = 01031901;

Reading this could be something to do with PHP 7

I’ll carry on checking.
Let me know if this is something familiar to yourself?

update
Another warning

PHP Warning: include_once(): https:// wrapper is disabled in the server configuration by allow_url_include=0 in includes/update-photo.inc.php on line 5

Should I use require instead of include_once ?

Barry

If I run that line, I get " Line : 2 – Invalid numeric literal"

Shouldn’t it be in quotes, if it needs a preceding zero?

I was going to mention in my first post that if I put the number in quotes the 500 error disappears - meaning the 500 issue is not shown.

I didn’t know I needed quotes if the number had a preceding zero? (common knowledge :upside_down_face:)

The database still doesn’t update if I use quotes or not that’s why I didn’t think it was an issue :thinking:

Did you see the other warning?

Barry

FYI
The photo_id column is varchar(11)

I might of used varchar because of this naming convention - was sometime ago when I built the table.

So does it matter if I use quotes?

int = no quotes
varchar = quotes

?

Barry

Fixed it! :smiley:

The include issue

include_once('includes/mysqli_connect.inc.php');
//should of been (../)
include_once('../includes/mysqli_connect.inc.php');

Working solution with quoted photo_id

include_once('../includes/mysqli_connect.inc.php');

$photo_id = "01031901";

if(isset($photo_id)) {
  $mysqli->query("UPDATE photos SET views = views + 1 WHERE photo_id = $photo_id");
}

Good stuff @droopsnoot

Got there in the end :nerd_face:

Cheers, Barry

1 Like

I would imagine that if you assign a number with leading zeroes then var_dump it, the leading zeroes will be gone.

Great that you’ve got it working. Now you need to convert that to use a prepared statement instead of concatenating the id into the query. :slight_smile:

You mean if I was not to use VARCHAR and I was using INT?
The DB design is years old - I’ve only just released about the leading zero issue :upside_down_face:

Yes I did think about this - more secure with prepared statement.

Cool thanks for the reminder!
I’m going to work on a prepared draft and post back.

Not resolved just yet :nerd_face:

Barry

Something like the below @droopsnoot - not tested yet?

I also now have the photo_id coming from the AJAX POST
$photo_id = $_POST['imgVal'];

require_once('../includes/mysqli_connect.inc.php');

$photo_id = $_POST['imgVal'];

if(isset($photo_id)) {

  $stmt = $mysqli->prepare("
    UPDATE photos
    SET views = views + 1
    WHERE photo_id = ?
  ");

  // Should I be using s here?
  $stmt->bind_param('i', $photo_id);

  if (!$stmt) {
    throw new Exception($conn->error, $conn->errno);
  }

}

Updated AJAX
I wasn’t quite sure how to add the data to the .post so used .ajax instead.

       let myImgVal  = $('p#imageVal').html();

        $.ajax({
            type: 'POST',
            url: 'includes/update-photo.inc.php',
            data: { imgVal : imgVal}
        });

What do you think?

Barry

Another question @droopsnoot

I will be executing two prepared statements at the same time using the same parameter ($photo_id).

Could I execute both as shown below or maybe combine these into one?

if(isset($photo_id)) {

  $stmt = $mysqli->prepare("
    UPDATE photos
    SET views = views + 1
    WHERE photo_id = ?
  ");

  $stmt = $mysqli->prepare("
    INSERT INTO hits (photo_id, dtstamp)
    VALUES ((
    SELECT id
    FROM photos
    WHERE photo_id = ?), NOW())
  ");

  $stmt->bind_param('i', $photo_id);

  if (!$stmt) {
    throw new Exception($conn->error, $conn->errno);
  }

}

Is this the correct way, allowed?

Barry

Think for a moment about the contents of $stmt after you have run the second prepare() pointing to the same variable. The second overwrites the first, of course, so you’ll have to do it separately. Maybe you can do it with a combined query, my sql knowledge isn’t up to it without lots of referring to samples, and you can do that as well as I can.

In this subquery as part of your second query

SELECT id FROM photos WHERE photo_id = ?

what’s the different between photo_id and id ? Wouldn’t it be more sensible to use the same id everywhere, to identify your photo?

In the main part of that query, if you’re always going to insert NOW() as the datestamp, why not set that to be the default for that column of your table, leave it out of the query and let the database just insert that default value? If you used a consistent id, and set the default, all you need would be

INSERT INTO hits (photo_id) VALUES (?)

I’m not sure it’s a great idea to have two tables, one which is effectively a “transactions” file (your ‘hits’ table) and another which contains the total number of entries in ‘hits’ for each photo - you can achieve the same by querying the ‘hits’ table when you need totals. By having the two, you’re just asking for the two to drift apart. If you are going to have the two separate tables, you need to run these as part of a transaction so they can’t get out of sync.

No, I was thinking only of the PHP code. In any language I’ve used, leading zeroes are not retained in numeric variables, only in strings. So there’s no functional difference between these two lines:

$photo_id = 01031901;
$photo_id = 1031901;

Yes, I’ve become aware of this and decided to create two separate statements (showing in next post).

I also read, for clarity:

mysql::prepare can only prepare a single statement

I think it is possible with some sort of JOIN, though in this instance I don’t want to make things more complicated, right now anyhow :slight_smile:

I have a FK and other indexes between the two tables which associates the id to the photo_id so everything is referenced and recorded to the same photo_id.

You might be onto something :thinking:
Yes this makes sense good catch! I’ll maybe update my code to reflect this.

I only introduced the second hits table well after the photos table was created. I understand not the best design, works as needed for now without a full refactor.

The idea was to query the hits table based on dates - example most viewed this month. The photos table view count is more of a total for each photo for quick referencing.

Yes but only within a certain date. The photos table had accumulated many views before the HITS table was created. Though nowadays it’s used just more for a reference.

I only started reading about transaction last night :grin:
Now I understand the process I understand what you mean. I’ll look into having this setup for accuracy and nice to know for future work.

I might even phase out the photos view count eventually.

Thanks for the recap, as mentioned in a previous post. I think this was the reason behind me using VARCHAR in the DB. Certainly make a note of this.

Barry

Here is the full script I’m currently using @droopsnoot
Works ok :slight_smile:

Moving forward…
I’ll need to refactor in the transaction and removal of the NOW()

require_once('../includes/mysqli_connect.inc.php');

$photo_id = $_POST['imgVal'];

if(isset($photo_id)) {

  $stmt1 = $mysqli->prepare("
    UPDATE photos
    SET views = views + 1
    WHERE photo_id = ?
  ");

  $stmt2 = $mysqli->prepare("
    INSERT INTO hits (photo_id, dtstamp)
    VALUES ((
    SELECT id
    FROM photos
    WHERE photo_id = ?), NOW())");

  $stmt1->bind_param('i', $photo_id);
  $stmt2->bind_param('i', $photo_id);
  $stmt1->execute();
  $stmt2->execute();

  if (!$stmt1 || !$stmt2) {
    throw new Exception($conn->error, $conn->errno);
  }

  $stmt1->close();
  $stmt2->close();

}

Anything to add what do you think?

Barry

Here is the refactored snippet - untested.

Are you familiar with $mysqli->autocommit and how it works with transactions, is this correct?

And good reference I found multiple-prepared-statements-in-transactions
Which also talks about try and catch with a rollback - for development purposes only I’m sure?

if(isset($photo_id)) {

  $mysqli->autocommit(FALSE); //turn on transactions

  $stmt1 = $mysqli->prepare("
    UPDATE photos
    SET views = views + 1
    WHERE photo_id = ?
  ");

  $stmt2 = $mysqli->prepare("
    INSERT INTO hits (photo_id)
    VALUES ((
    SELECT id
    FROM photos
    WHERE photo_id = ?))");

  $stmt1->bind_param('i', $photo_id);
  $stmt2->bind_param('i', $photo_id);
  $stmt1->execute();
  $stmt2->execute();

  if (!$stmt1 || !$stmt2) {
    throw new Exception($conn->error, $conn->errno);
  }

  $stmt1->close();
  $stmt2->close();

  $mysqli->autocommit(TRUE); //turn off transactions + commit queued queries
}

Barry

Sorry, I’m not familiar with mysqli at all - I started learning with the old-style mysql calls, then came on here and saw they were out of date so switched directly to PDO.

Or perhaps a trigger. Not something I’ve tried personally, but I’ve read bits about them. Maybe something a bit like this

DELIMITER $$
 
CREATE TRIGGER after_hits_insert
AFTER INSERT
ON hits FOR EACH ROW
BEGIN
   UPDATE photos
     SET views = views + 1
     WHERE photo_id = new.photo_id
END$$
 
DELIMITER ;

I think this would mean you only need to run your second query to insert the new row in hits.

Likewise, I’ve been meaning to get on board with PDO :nerd_face:

I’ll look into this, not a major concern though be good to reduce code and get things running a bit smoother.

Ok I think we’ve covered as much as we can with this thread.

Thanks, Barry

1 Like