4
\$\begingroup\$

I made this to-do list since I never seriously used jQuery (in particular way AJAX) and I need this kind of script for a project that I'm working on. Principal features:

  • Use of a database to manage to-do list values (using PDO).
  • Possibility to add/delete to-do list values.
  • Possibility to edit to-do list value dblclicking on the text value.
  • Possibility to set a "status" for a specific value (completed and tested).
  • To-do list values can be separeted for category/type.
  • AJAX functionality.

I want to know if all my methods applied to achieve this result are right and mostly well written, particularly for the jQuery/AJAX part. Could I write better some logic of the PHP code? Could I avoid some useless code in the jQuery/AJAX part? Every comment on the code would be appreciated.

load.initialize.php

<?php       
$database = [
    'name'  => "name",
    'user'  => "user",
    'pass'  => "idk",
];

try
{
    $dsn = "mysql:host=127.0.0.1;dbname={$database['name']};";
    $dbh = new PDO($dsn, $database['user'], $database['pass']);

    # Disable emulation of prepared statements, use REAL prepared statements instead.
    $dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

    # Set charset to uf8 (using query, that's the only way that make work SET NAMES).
    $dbh->query("SET NAMES utf8mb4");
}
catch (PDOException $e)
{
    echo 'An error occured: ' . $e->getMessage();
}

index.php [top of the file]

<?php
require_once('load.initialize.php');

// Delete to-do list value.
if (isset($_POST['delete']) && is_numeric($_POST['delete']))
{
    $stmt = $dbh->query("DELETE FROM tl_main WHERE id = {$_POST['delete']}");

    if ($stmt)
    {
        $json_data['action'] = true; # true;
    }
}

// Edit to-do list value.
if (isset($_POST['edited_text']) && isset($_POST['value']) && is_numeric($_POST['value']))
{
    $stmt = $dbh->prepare("UPDATE tl_main SET text = :text, edit_date = NOW() WHERE id = {$_POST['value']}");
    $stmt->bindParam(':text', $_POST['edited_text'], PDO::PARAM_STR);

    if ($stmt->execute())
    {
        $stmt = $dbh->query("SELECT edit_date FROM tl_main WHERE id = {$_POST['value']} LIMIT 1");

        $json_data['action'] = true; # true;
        $json_data['edit_date'] = date("d/m/y H:i", strtotime($stmt->fetchColumn())); # Send it directly formatted as we setted in the page (instead of formatting it in jQuery)
    }
}

// Add value to the to-do list
if (isset($_POST['value_text']) && !empty($_POST['value_text']) && isset($_POST['type']) && is_numeric($_POST['type']))
{
    $stmt = $dbh->prepare("INSERT INTO tl_main (type, text, added_date) VALUES({$_POST['type']}, :text, NOW())");
    $stmt->bindParam(':text', $_POST['value_text'], PDO::PARAM_STR);

    if ($stmt->execute())
    {
        $json_data['action'] = true; # true;
    }
}

// Set to-do list status to specific value.
if (isset($_POST['status']) && isset($_POST['value']) && is_numeric($_POST['value']))
{
    switch ($_POST['status'])
    {
        case "completed":
            $column_date = "completed_date";
            $status = "completed = 1, tested = 0, completed_date = NOW()";
            break;
        case "tested":
            $column_date = "tested_date";
            $status = "completed = 0, tested = 1, tested_date = NOW()";
            break;
        case "indev":
            $status = "completed = 0, tested = 0, completed_date = DEFAULT, tested_date = DEFAULT";
    }

    if ($status) {
        $stmt = $dbh->query("UPDATE tl_main SET {$status} WHERE id = {$_POST['value']}");

        if ($stmt)
        {
            if (isset($column_date))
            {
                $stmt = $dbh->query("SELECT {$column_date} FROM tl_main WHERE id = {$_POST['value']} LIMIT 1");
                $json_data[$column_date] = date("d/m/y H:i", strtotime($stmt->fetchColumn())); # Send it directly formatted as we setted in the page (instead of formatting it in jQuery)
            }

            $json_data['action'] = true; # true;
        }
    }
}

// Display json infos for AJAX call for to-do list actions (delete, edit, add, status).
if (isset($_POST['delete']) || isset($_POST['edited_text']) || isset($_POST['value_text']) || isset($_POST['status']))
{
    if (!isset($json_data))
        $json_data['action'] = false;

    header('Content-type: application/json');
    echo json_encode($json_data);
    exit;
}

// Fetch to-do list types.
$sql = "SELECT * FROM tl_types";
$types = $dbh->query($sql)->fetchAll();

// Fetch to-do list content.
$sql = "SELECT * FROM tl_main ORDER BY added_date DESC";
$todolist = $dbh->query($sql)->fetchAll();

// Apply/Fetch some operations to the todolist array.
foreach ($todolist as &$value)
{
    // Formatting the text content.
    // Catching http/https links.
    $pattern = "@(http(s)?)?(://)+(([a-zA-Z])([-\w]+\.)+([^\s\.]+[^\s]*)+[^,).\s])@";
    $value['text'] = preg_replace($pattern, '<a href="$0" target="_blank">$0</a>', $value['text']);

    // Formatting datetimes.
    $datetimes = [
        'added_date'        => $value['added_date'],
        'edit_date'         => $value['edit_date'],
        'completed_date'    => $value['completed_date'],
        'tested_date'       => $value['tested_date']
    ];
    foreach ($datetimes as $key => $datetime)
    {
        $value[$key] = strtotime($value[$key]);
    }

    // Build an array with type => array(edit_dates).
    // Needed after for get the highest last edit for each type.
    $type_edits[$value['type']][] = $value['edit_date'];
}

// Get in an array the highest edit date of the to-do list for every type in order to determine the last edit for each type.
ksort($type_edits); # A whim just to have the array with edit dates ordered by type id.
foreach ($type_edits as $type => $edits)
{
    $last_edit[$type] = date("d F H:i", max($edits));
}

// Check if last_edit array have missing types due to no content for the specific type (and so couldn't catch the last edit).
foreach ($types as $type)
{
    if (!array_key_exists($type['id'], $last_edit))
    {
        $last_edit[$type['id']] = "foo";
    }
}

index.php [bottom of the file]

<body>
    <div class="background"></div>
    <div id="container">
        <ul id="menu">
<?php foreach ($types as $type): ?>
            <li><a href="#<?= $type['name'] ?>"><?= $type['name'] ?></a></li>
<?php endforeach; ?>
            <li id="special">[ <a class="toggle_all" style="cursor:pointer">toggle all</a> ]</li>
        </ul>
        <fieldset id="legend">
            <legend>Legend</legend>
            <div id="completed" class="row">
                <span class="appearance"></span>
                <span class="explanation">Completed</span>
            </div>
            <div id="tested" class="row">
                <span class="appearance"></span>
                <span class="explanation">Tested</span>
            </div>
            <div id="indev" class="row">
                <span class="appearance"></span>
                <span class="explanation">In development</span>
            </div>
            <div id="edited" class="row">
                <span class="explanation">edited recently <span style="font-size:12px">(2 days)</span></span>
            </div>
        </fieldset>

<?php foreach ($types as $type): ?>
        <div id="<?= $type['name'] ?>" class="tab">
            <div class="title group">
                <div class="float_left">
                    <span class="main"><?= $type['name'] ?></span>
                    <span class="link">[ <a class="toggle" style="cursor:pointer">toggle</a> ]</span>
                    <span class="link">[ <a href="#<?= $type['name'] ?>">redirect</a> ]</span>
                </div>
                <div class="float_right">
                    <span class="last_edit">Last edit: <?= $last_edit[$type['id']] ?></span>
                </div>
            </div>
            <table>
                <tr>
                    <th class="subject"><span>Subject</span></th>
                    <th>Added</th>
                    <th>Mod</th>
                </tr>
                <tr>
                    <td class="subject"><textarea placeholder="Add new value..."></textarea></td>
                    <td class="date">00/00/00 00:00</td>
                    <td class="mod"><a id="add" data-type="<?= $type['id'] ?>"><i class="fas fa-plus"></i></a></td>
                </tr>
<?php foreach ($todolist as $content): ?>
<?php if ($content['type'] === $type['id']): ?>
<?php if ($content['completed']): ?>
                <tr class="completed">
<?php elseif ($content['tested']): ?>
                <tr class="tested">
<?php else: ?>
                <tr>
<?php endif; ?>
                    <td class="subject">
                        <div<?= ($content['edit_date'] > strtotime('-2 days')) ? ' style="font-style:italic"' : "" ?> data-value="<?= $content['id'] ?>" data-editable><?= $content['text'] ?></div>
                    </td>
                    <td class="date">
                        <span class="showhim"><?= date("d/m/y H:i", $content['added_date']) ?></span>
                        <div class="showme">
                            <ul>
                                <li><i class="fas fa-pencil-alt"></i> <span id="edit_date"><?= date("d/m/y H:i", $content['edit_date']) ?></span></li>
                                <li><i class="far fa-thumbs-up"></i> <span id="completed_date"><?= date("d/m/y H:i", $content['completed_date']) ?></span></li>
                                <li><i class="fas fa-check"></i> <span id="tested_date"><?= date("d/m/y H:i", $content['tested_date']) ?></span></li>
                            </ul>
                        </div>
                    </td>
                    <td class="mod">
<?php if ($content['completed']): ?>
                        <a id="status" data-status="tested" data-value="<?= $content['id'] ?>"><i class="fas fa-check"></i></a>
<?php elseif ($content['tested']): ?>
                        <a id="status" data-status="indev" data-value="<?= $content['id'] ?>"><i class="fas fa-undo-alt"></i></a>
<?php else: ?>
                        <a id="status" data-status="completed" data-value="<?= $content['id'] ?>"><i class="far fa-thumbs-up"></i></a>
<?php endif; ?>
                        <a id="delete" data-value="<?= $content['id'] ?>"><i class="far fa-trash-alt"></i></a>
                    </td>
                </tr>
<?php endif; ?>
<?php endforeach; ?>
            </table>
        </div>
<?php endforeach; ?>
    </div>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<script src="assets/general.js"></script>
</body>

general.js [all AJAX requests]

// Return window confirm() method for ajax requests.
function ajax_confirm(request) {
    return confirm("Are you sure?!\nAJAX request: " + request);
}

// Toggle specific table.
$('.toggle').on('click', function() {
    $(this).closest('.title').next('table').toggle();
});

// Toggle all tables.
$('.toggle_all').on('click', function() {
    $('table').toggle();
});

// AJAX POST request in order to delete a specific value.
$('td.mod #delete').on('click', function() {
    // Denied confirm alert doesn't run the script.
    if (!ajax_confirm('delete')) return;

    var $this = $(this);

    var request = {
        'delete': $this.data('value')
    };

    var posting = $.post(window.location, request, 'json');

    posting.done(function(data) {
        // Check if error query occurs.
        if (!data.action) return;

        $this.closest('tr').hide('fast', function() {
            $this.closest('tr').remove();
        });
    })
});

// AJAX POST request in order to edit a specific value.
$('table').on('dblclick', 'td div[data-editable]', function() {
    var $this = $(this);

    var $textarea = $('<textarea />').height($this.height()).val($this.text());
    $this.replaceWith($textarea);

    var save = function() {
        // Denied confirm alert doesn't send the AJAX POST request.
        // Text has not changed doesn't send the AJAX POST request.
        if (!ajax_confirm('edit') || $textarea.val() == $this.text()) {
            $textarea.replaceWith($this);
            return;
        }

        var request = {
            'edited_text': $textarea.val(),
            'value': $this.data('value')
        };

        var posting = $.post(window.location, request, 'json');

        posting.done(function(data) {
            // Check if error query occurs.
            if (!data.action) {
                alert(data);
                $textarea.replaceWith($this);
                return;
            }

            $div = $this.text($textarea.val()).css('font-style', 'italic')
            $textarea.replaceWith($div);

            $this.closest('tr').find('span#edit_date').text(data.edit_date);
        }); 
    };

    /**
        We're defining the callback with `one`, because we know that
        the element will be gone just after that, and we don't want 
        any callbacks leftovers take memory. 
        Next time `div` turns into `textarea` this single callback 
        will be applied again.
    */
    $textarea.one('blur', save).focus();
});

// AJAX POST request in order to add a value.
$('td.mod #add').on('click', function() {
    // Denied confirm alert doesn't run the script.
    if (!ajax_confirm('add')) return;

    var $this = $(this);
    var $textarea = $this.closest('tr').find('textarea');

    // Check if textarea is empty
    if (!$.trim($textarea.val())) {
        $this.closest('table').before('<div id="error" class="notice" style="display:none">Please fill out the textarea value.</div>');
        $('#error').stop(true).fadeIn().delay(5000).fadeOut(function() {
            $('#error').remove();
        });
        return;
    }

    var request = {
        'value_text': $textarea.val(),
        'type': $this.data('type')
    };

    var posting = $.post(window.location, request, 'json');

    posting.done(function(data) {
        // Check if error query occurs.
        if (!data.action) return;

        $this.closest('table').before('<div id="success" class="notice" style="display:none">Value added succesfully, refresh the page to view it.</div>');
        $('#success').stop(true).fadeIn().delay(5000).fadeOut(function() {
            $('#success').remove();
        });

        // Reset textarea value
        $textarea.val('');
    });
});

// AJAX POST request in order to set the status of a specific value.
$('td.mod #status').on('click', function() {
    // Denied confirm alert doesn't run the script.
    if (!ajax_confirm('status')) return;

    var $this = $(this);

    var request = {
        'status': $this.data('status'),
        'value': $this.data('value')
    };

    var posting = $.post(window.location, request, 'json');

    posting.done(function(data) {
        // Check if error query occurs.
        if (!data.action) return;

        switch (request.status) {
            case "completed":
                /*
                 * If completed:
                 * Add completed class to tr.
                 * Update data-status to the next status (tested).
                 * Update icon class to the next status (tested) icon.
                 * Update completed datetime.
                 */
                $this.closest('tr').addClass('completed');

                $this.data('status', 'tested');

                $this.children('svg').attr('data-prefix', 'fas').attr('data-icon', 'check');

                $this.closest('tr').find('span#completed_date').text(data.completed_date);
                break;
            case "tested":
                /*
                 * If tested:
                 * Remove completed class from tr. / Add tested class to tr.
                 * Update data-status to the next status (indev).
                 * Update icon class to the next status (indev) icon.
                 * Update tested datetime.
                 */
                $this.closest('tr').removeClass('completed');
                $this.closest('tr').addClass('tested');

                $this.data('status', 'indev');

                $this.children('svg').attr('data-prefix', 'fas').attr('data-icon', 'undo-alt');

                $this.closest('tr').find('span#tested_date').text(data.tested_date);
                break;
            case "indev":
                /*
                 * If in-dev:
                 * Remove tested class from tr. / No need to add class since indev take default background-color.
                 * Update data-status to the next status (completed).
                 * Update icon class to the next status (completed) icon.
                 * Remove completed and tested datetime.
                 */
                $this.closest('tr').removeClass('tested');

                $this.data('status', 'completed');

                $this.children('svg').attr('data-prefix', 'far').attr('data-icon', 'thumbs-up');

                $this.closest('tr').find('span#completed_date').text("foo");
                $this.closest('tr').find('span#tested_date').text("foo");
                break;
        }
    })
});

If can be useful, the two tables that I used. tl_main to store all the values, tl_types to list the categories.

CREATE TABLE IF NOT EXISTS `tl_main` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `type` tinyint(3) unsigned NOT NULL,
  `text` mediumtext CHARACTER SET utf8 NOT NULL,
  `added_date` datetime NOT NULL DEFAULT '0000-00-00 00:00:00',
  `edit_date` datetime NOT NULL DEFAULT '0000-00-00 00:00:00',
  `completed_date` datetime NOT NULL DEFAULT '0000-00-00 00:00:00',
  `tested_date` datetime NOT NULL DEFAULT '0000-00-00 00:00:00',
  `completed` tinyint(1) NOT NULL DEFAULT '0',
  `tested` tinyint(1) NOT NULL DEFAULT '0',
  PRIMARY KEY (`id`)
) ENGINE=MyISAM  DEFAULT CHARSET=utf8mb4 AUTO_INCREMENT=87 ;

CREATE TABLE IF NOT EXISTS `tl_types` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `name` varchar(45) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=MyISAM  DEFAULT CHARSET=latin1 AUTO_INCREMENT=7 ;
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

General feedback

Overall this code looks a bit repetitive - especially the PHP in index.php [top of the file] and the Javascript in general.js. Common lines could be abstracted into functions, and OOP approaches might make things easier to clean up, as well as allow for unit testing.

More specific Feedback

Below are some specific points I noticed in different parts of the code, but not a thorough list.

PHP

HTML

  • id attributes "must be unique in the whole document"1 - so elements like the delete links should have a unique value for id. Perhaps a class name of delete would suffice. The same is true for other links like those with id value add and status.

JS

  • it is better to store DOM references in a variable once instead of querying each time, unless the structure continues to change. So for example, $('table') can be stored in a variable and the variable can be referenced instead of querying the DOM each time.

1https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id

\$\endgroup\$
2
  • \$\begingroup\$ Got the HTML part and the JS one too, didn't know about this thing, thanks! For the PHP part, I have some doubts: I always thought that if you don't have something to really escape and you sanitize it properly, you can also avoid to bind the parameter. For example, in the first part (delete process), I just need to specify the ID, a numeric value: since I sanitized it with is_numeric, do I really need to bind it? I mean, what risks do I run if I'm going to accept just a number? \$\endgroup\$
    – gomd
    Jan 31, 2019 at 20:07
  • \$\begingroup\$ Instead, for data input filtering, what should I have filtered? Text values should be filtered? There's actually a difference between is_numeric and FILTER_VALIDATE_INT? I agree that I can rewrite it in OOP and I think that I will do it as soon as I can since I'm approaching this world recently. Thanks again for the answer! \$\endgroup\$
    – gomd
    Jan 31, 2019 at 20:07

Your Answer

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

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