0
\$\begingroup\$

Code below successfully processes and submits data to database. Is there anything else that needs to be included / considered?

PHP (note-process.php):

$data = array();

if ( (empty($_POST['name'])) || (empty($_POST['note'])) ) {
    $data['success'] = FALSE;
    $data['message'] = 'Name and note is required.';

} else {

   // Process inputs and send to database using prepared statements
    
    if ($stmt->execute()) 
    {

      $data['success'] = TRUE;
      $data['message'] = 'Success! Name and note saved!';

    } else
    {

      $data['success'] = FALSE;
      $data['message'] = 'Note not saved. Try again later';
      
    }

    $stmt->close(); 


}

echo json_encode($data);

JS:

// Refresh note section every 2 seconds

function loadlink(){
  $('#notes-box').load('note-refresh.php',function () {
    $(this).unwrap();
  });
}

loadlink(); // This will run on page load
setInterval(function(){
  loadlink() // This will run after every 2 seconds
}, 2000);

// Submits form without refreshing page

$(document).ready(function () {

    $("form").submit(function (event) {

    $(".help-block").remove();

    var formData = {
      name: $("#name").val(),
      note: $("#note").val()
    };

    $.ajax({
      type: "POST",
      url: "note-process.php",
      data: formData,
      dataType: "json",
      encode: true,
    })

    .done(function (data) {
    console.log(data);

    if (!data.success) {

      $("#error-box").append(
        '<div class="help-block"><b>' + data.message + "</b></div>"
      );
      
    } else {

      $("#error-box").append(
        '<div class="help-block">' + data.message + "</div>"
      );
      
    }

  })

  <!-- How does the below get called? -->

  .fail(function (data) {
    $("#error-box").append(
      '<div class="help-block">Could not reach server, please try again later.</div>'
    );
  });

  event.preventDefault(); // Prevents the default action of an element from happening
  });
  
});

HTML:

<div id="notes-box"></div>

<div id="error-box"></div>

<form action="note-process.php" method="POST">
    <label for="name">Name</label>
    <input type="text" id="name" name="name" placeholder="Full Name" />

    <label for="note">Note</label>
    <input type="text" id="note" name="note" placeholder="Note" />

  <button type="submit" class="btn btn-success">Submit</button>
</form>
\$\endgroup\$
3
  • \$\begingroup\$ I've noticed that no attempt has been made to make this code reusable. You will have to edit all of this code for each new form you'll be making. See: Code reusability. \$\endgroup\$ Jun 30, 2023 at 14:14
  • \$\begingroup\$ This is a good point. I am planning to reuse this code across multiple forms. I think the only thing that will really vary will be the formData section, with some forms with more inputs, others with less. Do you have any resources on how to best approach? \$\endgroup\$
    – Matt
    Jul 3, 2023 at 10:45
  • \$\begingroup\$ To create reusable code you need a programming language. Out of the three pieces of code, in your question, only PHP is capable of incorporating the other two. To reuse the PHP code you need to encapsulate it, either in a function or a class, and then put it in a separate include file. Reusability is a basic feature of all programming languages, the trick is to actually make use of it. \$\endgroup\$ Jul 3, 2023 at 11:01

2 Answers 2

1
\$\begingroup\$

This is unfortunately almost off-topic, since it - especially the PHP - is unrunnable stub code, but I think the JavaScript is reviewable, which is what I'll look at.

First, the indentation is off. Your editor should format the code for you. Make sure to use that function. Correct indentation may help you with your question "How does the below get called?". More about that later.

As a beginner you should consider learning and writing plain JavaScript instead of relying on jQuery. I think it's important to understand the intricacies of front-end development without having things hidden by a library. Additionally jQuery is generally considered obsolete. Its original goals are not relevant in today's web environment.

You should be using arrow functions for callbacks.

The call of loadlink() should either be moved into the document.ready() callback, or alternatively don't use document.ready() at all, if your script is being included using defer or at the end of the HTML document.

The setInterval call can be simplified to

setInterval(loadlink, 2000);

I'm not quite sure what you are asking with "How does the below get called?" (or if it's even addressing us), but I'll attempt to answer it: jQuery's ajax function returns an object that is called a Promise[*], which will return a value at a later time (in this case waiting for a server response). Generally one provides the promise with two callback functions. One passed to the done method, which is called when the value has arrived, and a second one passed to the fail method, which is called if something goes wrong, for example, the server reports a HTTP error.

[*] As far as I remember in case of jQuery it is actually more/different than that, but I'll just consider it a "Promise" for now.

Finally you should avoid building HTML inside your JavaScript, especially when displaying external text like the error message, because if that message (which may have been manipulated by forces outside your control) contains invalid HTML (for example, a < character which the browser may try to interpret as an opening HTML tag and which can break the display). Instead in your case consider setting the error message using .textContent (or jQuery's .text() method).

\$\endgroup\$
3
  • \$\begingroup\$ Thank you so much for your reply. Appreciate the time and effort that went into it. Re jQuery: I keep reading it's dead, but then it's still being maintained and widely used - what are your thoughts? \$\endgroup\$
    – Matt
    Jun 28, 2023 at 8:57
  • \$\begingroup\$ @Mark The idea that JQuery should not be used is simply an opinion. Javascript in particular seems to attract language purists. The more obscure the code the better. Any programming language is just a tool to reach an objective, and if for you JQuery is part of that, then that's fine. A point could be made to rely on as little extra libraries as possible, because this could complicate the maintenance of the code, and that could be an argument to avoid JQuery. \$\endgroup\$ Jun 30, 2023 at 14:12
  • \$\begingroup\$ @Mark think of it as of using a wood-fed stove. An indisputable achievement for its time. but nowadays? In a modern house? It's just obsoleted. So JQuery, when JS was infant, it made you able to do many things that raw JS was unable to do. But nowadays it matured and can do everything with the same ease. It is only used because of the momentum, but there is not a single reason to prefer it over native solutions. \$\endgroup\$ Jul 28, 2023 at 5:50
1
\$\begingroup\$

Regarding PHP part, the error handling is rather inconsistent. To make it solid and useful, I would suggest

  1. Use appropriate HTTP status codes.
    • an error in the input data is a problem with request, and hence it should return a 400 status code.
    • an error in the database, on the other hand, is a server error and therefore should be returned with a status 500
  2. Use consistent error reporting for PDO. By setting the error mode to exceptions, you will be able handle all errors automatically, without a single line of manually written code.
  3. Set up a site-wide error handler, that would uniformly handle all system errors, to satisfy both kinds of customers - a programmer and a site user, who need completely different approach in error reporting.

To implement this you can create a simple function,

function json_response($data, $status = 200) {
    http_response_code($status);
    header('Content-Type: application/json; charset=utf-8');
    echo json_encode($data);
    die;
}

that will make your code concise and meaningful, letting the code to exit earlier

if ( (empty($_POST['name'])) || (empty($_POST['note'])) ) {
    $data = [
        'success' => false,
        'message' => 'Name and note is required.',
    ];
    json_response($data, 400);
}

// Process inputs and send to database using prepared statements
$stmt = $pdo->prepare("INSERT ...");
$stmt->execute([...input...]);
$data = [
    'success' => true,
    'message' => 'Success! Name and note saved!',
];
json_response($data);

While in case of error, you can add a simple error handler, such as one featured in my article on PHP error reporting:

set_exception_handler(function ($e)
{
    // log the actual error
    error_log($e);
    // set a generic error message for a live environment        
    if (!filter_var(ini_get('display_errors'),FILTER_VALIDATE_BOOLEAN)) {
        $e = "An internal server error has been occurred. Try again later.";
    }
    // differ the output for direct and AJAX calls
    if ($_SERVER['HTTP_X_REQUESTED_WITH'] ?? '') == 'xmlhttprequest') {
        $data = [
            'success' => false,
            'message' => $e,
        ];   
        json_response($data, 500);
    } else {
        http_response_code(500);
        echo $e;
    }
});
\$\endgroup\$
2
  • \$\begingroup\$ Thanks (big fan BTW). Works ok when there is no error i.e. json_response($data), but Ajax returns undefined when I provide a $status. Headers are fine when I inspect with Development Tools \$\endgroup\$
    – Matt
    Jul 29, 2023 at 12:18
  • \$\begingroup\$ @Mark that should be handled with your Ajax code code. If you check examples, there is always a handling code for getting error statuses. But if you want to keep it simple, you can omit sending statuses from PHP. Which is not right, but currently will make your life a bit easier \$\endgroup\$ Aug 19, 2023 at 7:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.