0

Im having an issue where i can't seem to assign an ID from a column in the database to each DIV respectively.

The Scenario: I have a page full of clients and next to each one is a vote up and vote down button. When i click one of them it simply adds +1 to either the positive or negative column.

What is the Issue? The issue is no matter which client i vote for it only updates the ClientID '1'

What have i done so far?

  • I am using a while loop to retrieve all the clients from the database. I have given each div that comes through a data-clientid for $_POST['ClientID'].
  • I have classes called voteup and votedown so i can reference them in my JS script.
  • In my script i assign the clientid from data-clientid to variables using the classes voteup and votedown
  • In the voteup/down php files i then use this clientid to use the update sql to update the column.

What is my code?

My main page that displays the clients and the voting buttons (Shortened to the bit you need to see, connection to the database is included and all of that works fine)

<?php

$clientInfo = "SELECT * FROM Clients ORDER BY Client ASC";
$stmt = sqlsrv_query($conn, $clientInfo);

echo "<div style='width: 100%; display: inline-block;'>";

while ($client = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC))
{
    echo "<div class='clientid' style='height: 50px; font-size: 18px; vertical-align: middle; display: inline-block; width: 100%'>" . 

            "
            <div style='width: 35%;'></div>

            <div onclick=\"voteUp()\" style='display: inline-block; width: 5%;'>      
                <span style='font-size: 20px;' class='hover-cursor fa fa-chevron-up vote-up'></span>
            </div>" .

                 "<div class='hover-cursor hvr-underline-reveal voteup votedown' data-clientid='{$client['ClientID']}' style='width: 20%; display: inline-block;'>" . $client['Client'] . "</div>" . 

            "<div onclick=\"voteDown()\" style='display: inline-block; width: 5%;'>
                <span style='font-size: 20px; text-align: right;' class='hover-cursor fa fa-chevron-down vote-down'></span>
            </div>

            <div style='width: 35%;'></div>

         </div> 
        <br />";
}

echo "</div>";

?>

This then links to my scripts file with my JQuery (I set it up to print the ID in the console so i can see which ID it is hitting. Im just going to take Vote Up as an example)

window.voteUp = function() {
    var clientid = $(".voteup").data("clientid");

    $.post("voteup.php", {clientid: clientid}, function(data) {
       console.log("Data:" + clientid) 
    });
    return false;
}

and then there is the voteup.php that i am referring to in my $.post

<?php

if (isset($_POST['clientid'])) {
    $voteup = "UPDATE Clients SET Pos = (SELECT MAX(Pos) FROM Clients) + 1 WHERE ClientID = " . $_POST['clientid'];

    $stmt = sqlsrv_query($conn, $voteup);
} else {
    echo "Failed";
}

?>
Ivan
  • 2,463
  • 1
  • 20
  • 28
MagicRecon
  • 53
  • 2
  • 9
  • 1
    `$(".voteup").data("clientid")` will always return the `clientid` of the first element with class `voteup` (see [`.data()`](https://api.jquery.com/data/)). Remove the inline JavaScript and use jQuery to [attach the event handlers](https://learn.jquery.com/events/). – Andreas Jan 11 '17 at 10:47
  • Your script is at risk of [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) Have a look at what happened to [Little Bobby Tables](http://bobby-tables.com/) Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) Use [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) – RiggsFolly Jan 11 '17 at 10:48

1 Answers1

0

Javascript issue

As stated in comments, your Javascript code is getting only the first voteup element in the DOM Tree.

you need to use a dynamic way to find the specific voteup div for the client that you are voting up (or down), try this:

while ($client = sqlsrv_fetch_array($stmt, SQLSRV_FETCH_ASSOC))
{
    echo "<div class='clientid' style='height: 50px; font-size: 18px; vertical-align: middle; display: inline-block; width: 100%'>" . 

            "
            <div style='width: 35%;'></div>

            <div style='display: inline-block; width: 5%;'>      
                <span style='font-size: 20px;' class='hover-cursor fa fa-chevron-up vote-up'></span>
            </div>" .

                 "<div class='hover-cursor hvr-underline-reveal voteup votedown' data-clientid='{$client['ClientID']}' style='width: 20%; display: inline-block;'>" . $client['Client'] . "</div>" . 

            "<div style='display: inline-block; width: 5%;'>
                <span style='font-size: 20px; text-align: right;' class='hover-cursor fa fa-chevron-down vote-down'></span>
            </div>

            <div style='width: 35%;'></div>

         </div> 
        <br />";
}

I've just removed the onclick event of your vote-up and vote-down buttons.

Now you need only one script:

<script>
    $(".vote-up").click(function(){
        var clientId = $(this).parents(".clientid").find(".voteup").data("clientid");

        $.post("voteup.php", {clientid: clientid}, function(data) {
            console.log("Data:" + clientid)
        });
        return false;
    });

    $(".vote-down").click(function(){
        var clientId = $(this).parents(".clientid").find(".votedown").data("clientid");

        $.post("votedown.php", {clientid: clientid}, function(data) {
            console.log("Data:" + clientid)
        });
        return false;
    });
</script>

Basically we are getting the top parent of vote-up button, the div.clientid, and with it, we search for the div.voteup that contains the clientid data


PHP Issue

Also, your UPDATE query is wrong, you are selecting the MAX(pox) from Clients through a subquery without the where clause, try this:

$voteup = "UPDATE Clients SET Pos = (SELECT MAX(Pos) FROM Clients WHERE ClientID = " . $_POST['clientid']. ") + 1 WHERE ClientID = " . $_POST['clientid'];

Later, try to transform your queries into parametrized queries, to avoid sql injection issues.

leoap
  • 1,684
  • 3
  • 24
  • 32
  • Hmm, this works, so its getting the differnent ID so thank you! however its taking the last Rating and adding plus 1, so say i have client a and client b, both are 0 rating, i vote up client A which now has 1 rating. when i vote up client B it gives them a rating of 2, whereas they should also be 1 because they were on 0 before – MagicRecon Jan 11 '17 at 11:27
  • yep i jsut noticed that, i added in an extra where clause to the sub query in the 'select max' – MagicRecon Jan 11 '17 at 11:35
  • ok, I've updated my answer too. You may want to accept the answer if it resolves your problem – leoap Jan 11 '17 at 11:37
  • Yep just sorted that, i shall accept your answer now, thank you for the help! – MagicRecon Jan 11 '17 at 11:40