1
\$\begingroup\$

I'm trying to implement upvote/downvote system.The user clicks on the upvote or downvote button its triggers the AJAX code the votes is inserted or updated in the database.

This triggers AJAX request on click.

    echo '<i id="'.$row['postID'].'" class="fa fa-chevron-up vote_up" aria-hidden="true"></i>';
    echo '<i id="'.$row['postID'].'" class="fa fa-chevron-down vote_down" aria-hidden="true"></i>';

This is the ajax code -

$(document).ready(function () {
    $("i.vote_up").click(function () {
        the_id = $(this).attr('id');
        $.ajax({
            type: "POST",
            data: "action=upvote&id=" + $(this).attr("id"),
            url: "vote.php",
            success: function (msg) {
                alert("Success");
            },
            error: function () {
                alert("Error");
            }
        });
    });
});

$(document).ready(function () {
    $("i.vote_down").click(function () {
        the_id = $(this).attr('id');
        $.ajax({
            type: "POST",
            data: "action=downvote&id=" + $(this).attr("id"),
            url: "vote.php",
            success: function (msg) {
                alert("Success");
            },
            error: function () {
                alert("Error");
            }
        });
    });
}); 

This is the code that handles votes.

<?php
require('includes/config.php');
$action   = $_POST['action'];
$postID   = $_POST['id'];
$memberID = $_SESSION['memberID'];
switch ($action) {
    case "upvote":
        try {
            $stmt = $db->prepare('SELECT memberID,postID,voteType FROM votes WHERE postID = :postID AND memberID =:memberID');
            $stmt->execute(array(
                ':postID' => $postID,
                ':memberID' => $memberID
            ));
            $row = $stmt->fetch();
            if ($row['voteType'] == "up") {
                $stmt = $db->prepare('DELETE FROM votes WHERE postID = :postID AND memberID =:memberID');
                $stmt->execute(array(
                    ':postID' => $postID,
                    ':memberID' => $memberID
                ));
                $stmt = $db->prepare('UPDATE posts SET upVote = upVote -1 WHERE postID = :postID');
                $stmt->execute(array(
                    ':postID' => $postID
                ));
            }

            else if ($row['voteType'] == "down") {
                $voteType = "up";
                $stmt     = $db->prepare('UPDATE votes SET voteType = :voteType WHERE postID = :postID AND memberID =:memberID');
                $stmt->execute(array(
                    ':voteType' => $voteType,
                    ':postID' => $postID,
                    ':memberID' => $memberID
                ));
                $stmt = $db->prepare('UPDATE posts SET upVote = upVote +1, downVote = downVote -1 WHERE postID = :postID');
                $stmt->execute(array(
                    ':postID' => $postID
                ));
            } else if ($row['memberID'] == null) {
                $voteType = "up";
                $stmt     = $db->prepare('INSERT INTO votes (postID,memberID,voteType) VALUES (:postID, :memberID, :voteType)');
                $stmt->execute(array(
                    ':postID' => $postID,
                    ':memberID' => $memberID,
                    ':voteType' => $voteType
                ));
                $stmt = $db->prepare('UPDATE posts SET upVote = upVote +1 WHERE postID = :postID');
                $stmt->execute(array(
                    ':postID' => $postID
                ));
            }
        }
        catch (PDOException $e) {
            echo $e->getMessage();
        }
        break;
    case "downvote":
        try {
            $stmt = $db->prepare('SELECT memberID,postID,voteType FROM votes WHERE postID = :postID AND memberID =:memberID');
            $stmt->execute(array(
                ':postID' => $postID,
                ':memberID' => $memberID
            ));
            $row = $stmt->fetch();
            if ($row['voteType'] == "down") {
                $stmt = $db->prepare('DELETE FROM votes WHERE postID = :postID AND memberID =:memberID');
                $stmt->execute(array(
                    ':postID' => $postID,
                    ':memberID' => $memberID
                ));
                $stmt = $db->prepare('UPDATE posts SET downVote = downVote -1 WHERE postID = :postID');
                $stmt->execute(array(
                    ':postID' => $postID
                ));
            } else if ($row['voteType'] == "up") {
                $voteType = "down";
                $stmt     = $db->prepare('UPDATE votes SET voteType = :voteType WHERE postID = :postID AND memberID =:memberID');
                $stmt->execute(array(
                    ':voteType' => $voteType,
                    ':postID' => $postID,
                    ':memberID' => $memberID
                ));
                $stmt = $db->prepare('UPDATE posts SET downVote = downVote +1, upVote = upVote -1 WHERE postID = :postID');
                $stmt->execute(array(
                    ':postID' => $postID
                ));
            } else if ($row['memberID'] == null) {
                $voteType = "down";
                $stmt     = $db->prepare('INSERT INTO votes (postID,memberID,voteType) VALUES (:postID, :memberID, :voteType)');
                $stmt->execute(array(
                    ':postID' => $postID,
                    ':memberID' => $memberID,
                    ':voteType' => $voteType
                ));
                $stmt = $db->prepare('UPDATE posts SET downVote = downVote +1 WHERE postID = :postID');
                $stmt->execute(array(
                    ':postID' => $postID
                ));
            }
        }
        catch (PDOException $e) {
            echo $e->getMessage();
        }
        break;
}
?> 

This is all working fine, but I want to know if there is any other way to do this? And also how can I make this code safe to use on a website ?

\$\endgroup\$
2
\$\begingroup\$
  1. When using ids in HTML elements, there should only be one per page. Consider using a custom attribute, e.g. <i data-post-id="<?=$row['postID']?>" ...></i>. The JQuery part will look something like this the_id = $(this).attr('data-post-id');.
  2. When the success function of the AJAX call is called, this does not necessarily mean that the vote was successful. You need to check the attribute passed into the function to determine whether the vote was actually successful or not.
  3. For a better user experience, I would recommend using something other than alerts to communicate success or failure.
  4. In the PHP code, the else if ($row['memberID'] == null) { can just be else { assuming there are only 2 types of votes, 'up' and 'down'.
  5. Your logic in your vote handling section seems a little off. If I understood it correctly, if the user has already cast a vote, it is removed and the current vote is discarded. If they have not, you store the vote. I think what you may have wanted was to remove the conditional statement mentioned in my previous point entirely so that it always runs.
  6. It would be better to create some functions for the duplicate code parts (SQL queries).
  7. You should echo a result of the vote. Whether the vote was successfully cast or (if that was your intention) their previous vote was removed and this one discarded, etc. At the moment, your only output is if there is an error with the database. JSON encode the output for ease of use.
\$\endgroup\$
2
  • \$\begingroup\$ I made changes according to your suggestion. Can you give an another example of using custom attribute ? \$\endgroup\$ – Mahim Parsai Aug 20 '17 at 9:29
  • \$\begingroup\$ I have expanded the answer a little to show you how JQuery would use it. When using a custom attribute, we add data- in front of the attribute so that we can have valid markup. \$\endgroup\$ – OwChallie Aug 20 '17 at 10:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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