Update database, refresh the page to show changes, show a success message

Question

Follow up question for:

Update database, reload page then show a success message

I have a table of articles, in each row is a button to activate / deactivate the article, along with other actions shown here:

The problem I was having:

If I did a standard AJAX call, I could append a success message to a div but it would not refresh the page. That's where document.location.reload(true); comes in, but now the page refreshes before the success message can show. Sure you could setTimeOut, but it seems a bit backwards to show a success message before you can see the result, so on to my method.

Note: All I/O is filtered with a function which I chose to leave out for ease of reading and error handling has not yet been implemented

The Ajax in articlelist.php:

This will post the data to articlecontrol.php to be processed, I've added a selector here because the controller also handles other requests i.e. Categories.

// ACTIVATE / DEACTIVATE WITH LINK
$(document).on("click",".active_button",function() {

    var articleID = $(this).attr("name");
    var active = $(this).val();
    var selector = $("#selector").attr("name");

    $.ajax({
    type: "POST",
    url: "controllers/articlecontrol.php",
    data: { articleID: articleID, active:active, selector: selector },
    success: function(data){
        document.location.reload(true);
    }
    });
});

I'll use the activate / deactivate PHP as the example in articlecontrol.php

If the database updated set the success info in $_SESSION['status'] ready for output on our table page:

if(isset($_POST['selector'])) {
    $selector = $_POST['selector'];

// ACTIVATE / DEACTIVATE ARTICLE w/Button
if(isset($_POST['active']) && $selector === 'article') {
        $where = array('article_id' => $_POST['articleID']);

        $active = ($_POST['active'] == 'YES') ? '0' : '1';

        $update = array('article_active' => $active);

    if ($database->update('wcx_articles', $update, $where)) {
        if ($active == '0') {
            $_SESSION['status'] = '<div class="success">Article Deactivated Successfully</div>';
        } else {
            $_SESSION['status'] = '<div class="success">Article Activated Successfully</div>';
        }
    }
}

This is how I'm showing the success message in articlelist.php:

<?php
if(isset($_SESSION['status'])) {
    echo $_SESSION['status'];
    $_SESSION['status'] = '';
}
?>

I'm also checking if there is anything in the .successmessage div on page load and fading it out:

 $(document).ready(function(){
   if ($(".successmessage").html().length > 0) {
       $('.successmessage').fadeOut(3000);
   }                                           
 });

Answer 1

Storing HTML in session leads to tight-coupling

Having the HTML stored in the server- side session is a violation of the Single-Responsibility principle. It couples the front-end display output in the back-end code.

Consider the scenario where translation/localization is needed. A simpler approach would be to store a simple dictionary in the session like $_SESSION['status'] = 'deactivated’; or $_SESSION['status'] = 'activated’; depending on the value of $_POST['active']. Then the response from articlelist.php can be a JSON form of $_SESSION - with at least the value of $_SESSION['status'] and the front end code can add the HTML accordingly.

If the structure of the HTML needs to be updated then the front-end code can be updated without requiring an update to the back-end code. Other benefits include:

• the size required to store each session would be decreased
• the size of network requests could be smaller

jQuery shorthand method could be used to simplify JavaScript

 $.ajax({
 type: "POST",
 url: "controllers/articlecontrol.php",
 data: { articleID: articleID, active:active, selector: selector },
 success: function(data){
     document.location.reload(true);
 }
 });

Could be simplified using jQuery.post() shorthand method and The shorthand property definition notation

$.post('controllers/articlecontrol.php', { articleID, active, selector },
   function(data){
        document.location.reload(true);
    }
});

Answer 2

• In your submission processing code, you have 3 required variables. Use isset() once to check that all 3 are declared.

• Try to avoid single-use variables. If you aren't altering a $_POST variable, there's no reason to re-declare one of its elements as a new variable. Declaring $where and $update or $set can be helpful, but in a good IDE those parameter names can/should be faintly presented anyhow in the method call.

• It may be overkill for such a short snippet, but an early conditional break, return, or exit can be helpful in preventing deeply nested structures (sometimes referred to as "arrowhead code" e.g. conditions and loops). Minimizing nested structures will keep your code more left and allow more visible characters per line before reaching the advised PSR-12 character limit per line.

• Only use inline comments in your code when your code is not intuitive. Without any commenting at all, I can easily determine that your code updates the status of an article as "Activated" or "Deactivated".

• As a general rule try to avoid "magic numbers" in projects. While it can be justified to use integers in the database, those 0 and 1 values have no intuitive meaning to someone unfamiliar with your project. In fact, as a new reader of your code, I would assume 0 (a falsey value) would mean not active and 1 (a truthy value) would mean active. Food for thought.

• We cannot see what your update() method does, but ideally it should return the number of affected rows (not merely a boolean indicating that the query was free from errors). In my projects, I prefer to return the affected rows from update() methods and the new auto-incremented id (if possible) from insert()/create() methods. This gives a truer sense of the database action's outcome.

• Using the printf() family of functions is a good habit to get into. It affords a clean way to blend static text with variables and conditions. This often improves script readability and maintainability while reducing script bloat.

New version of submission receiving code:

const STATUSES = [
    'activated' => '0',
    'deactivated' => '1'
];
if (
    isset($_POST['selector'], $_POST['active'], $_POST['articleID'])
    && $selector === 'article'
) {
    $newStatus = $_POST['active'] === 'YES' ? 'activated' : 'deactivated';

    $affRows = $database->update(
        'wcx_articles',
        ['article_active' => STATUSES[$newStatus]],
        ['article_id' => $_POST['articleID']]
    );

    $_SESSION['status'] = sprintf(
        '<div class="%s">Article %s $s</div>',
        $affRows ? 'success' : 'error',
        $affRows ? 'successfully' : 'was not',
        $newStatus
    );
}