Help needed for checkbox issue

[macytan1988]

I'm currently developing a website which allows users to register and log into the system to read news articles in RSS form.

A subscription page with a list of checkboxes (like CNN, BBC etc) is provided for the users to choose which news site they want to include in their customized RSS feed page.

 

I successfully stored the checkbox values in array form into the database. For example, if the user ticks "CNN", "BBC", "ABC News", the values that will be stored in database will be: cnn, bbc, abc

 

Since I'm a beginner in web coding, especially in PHP and MYSQL, I have two questions here.

 

Q1. How can I retrieve the stored values and display it on the subscription page with the specific checkboxes checked based on user's subscription? For instance, if the user already subscribed to CNN, when he logs into the system next time, he should be able to see the already-ticked CNN checkbox.

 

Q2. How can I retrieve the stored values and put it onto the customised RSS page? It's something like: if cnn is selected, then add http://rss.cnn.com/rss/edition_world.rss. Is there any tutorial on that?

 

Last but not least, forgive me for asking so many questions in one go in this lengthy piece and thanks for all the replies in advance! =)

 

Table structure:

 

HTML for checkboxes:

 

        <input type="checkbox" name="subs[]" value="mal-malins"> The Malaysian Insider</option>
        <input type="checkbox" name="subs[]" value="mal-malaymo"> The Malay Mail Online</option>
        <input type="checkbox" name="subs[]" value="mal-star"> The Star</option>
        <input type="checkbox" name="subs[]" value="mal-nst"> New Straits Times</option>

 

Code for storing the checkbox values:

 

//START: Subscription 

if(isset($_POST['subscribe'])){
if(isset($_POST['subs'])){
try {

$data= $_POST['subs'];

$subs = implode(",", (array)$data);

$stmt = $db->prepare("UPDATE members SET subs = :subs WHERE username = '$username' ");
$stmt->execute(array(
':subs' => $subs
));
//show success message
echo '<div class="alert alert-success"><a class="close" data-dismiss="alert">×</a>Subscription updated!</div>';
//else catch the exception and show the error.
} 
catch(PDOException $e) {
$error[] = $e->getMessage();
}
}
else {
echo '<div class="alert alert-danger"><a class="close" data-dismiss="alert">×</a>You have not selected any subscription!</div>';
}
}

//END: Subscription

 

[Jacques1]

Do not store comma-separated values in your database. This violates the First Normal Form (values must be atomic) and makes it practically impossible to do any complex queries. For example, you couldn't even select all users which have subscribed the CNN feed, because that information is buried within a long string.

 

In the relational model, data is stored in rows. So what you want is one table for the users, one table for the various RSS feeds and one table which assigns users to RSS feeds (many-to-many relationship). The latter table would look something like this:

user_id | feed_id
--------+---------
1       | 3
1       | 5
2       | 1
3       | 3

You also need to avoid “mixedCase” column names. While this may be pretty, it can lead to a lot of confusion and bugs, because some parts of your application are case-sensitive while others are not. MySQL itself ignores character case, so “memberid” is the same as “memberID”. But when you fetch the columns with PHP, character case does matter, which means it may not be able to find the column.

 

Use the “underscore_notation” instead.

 

As to your questions:

 

1) When you query the database for all possible feeds, also ask whether the feed has been subscribed by the user. This requires a left join. If you're not familiar with joins, now would be a good time to learn them. Otherwise, just fetch all feeds and all subscriptions separately. When you generate the list of checkboxes, you add the checked attribute to the feeds which are already subscribed.

 

2) Not sure what you mean. Are you asking how to merge the different feeds into one custom feed? Anyway, I think this can wait until the first task is finished.

[macytan1988]

Hi, Jacques1. Thanks for your reply!
I will now separate different feeds with different columns in the database for easy configuration in the future according to your suggestion.
I will be back soon.

[macytan1988]

Hi, Jacques1 again.
I have already edited my code accordingly based on your suggestion. The checkbox input is, thus far, successfully.

Can you give me some guidance on how to generate the list of checkboxes and add the checked attrbute to the feeds which are already subscribed?

Attached herewith are my php and HTML codes.

 

PHP

//START: Subscription
            
    if(isset($_POST['subscribe'])){
        try{
            $subsTMI =  (isset($_POST['subsTMI'])) ? '1' : '0';
            $subsTMO =  (isset($_POST['subsTMO'])) ? '1' : '0';
            $subsSTAR =  (isset($_POST['subsSTAR'])) ? '1' : '0';
            
            $stmt = $db->prepare("UPDATE members SET
            subsTMI = :subsTMI,
            subsTMO = :subsTMO,
            subsSTAR = :subsSTAR
            WHERE username = '$username' ");
            $stmt->execute(array(
            ':subsTMI' => $subsTMI,
            ':subsTMO' => $subsTMO,
            ':subsSTAR' => $subsSTAR
        ));
        }
        catch(PDOException $e) {
            $error[] = $e->getMessage();
        }    
    echo '<div class="alert alert-success"><a class="close" data-dismiss="alert">×</a>Subscription updated!</div>';    
}

//END: Subscription

 

 

        <input type="checkbox" name="subsTMI" value="1" > The Malaysian Insider</input>
        <input type="checkbox" name="subsTMO" value="1"> The Malay Mail Online</input>
        <input type="checkbox" name="subsSTAR" value="1"> The Star</input>

[Jacques1]

I fear you've misunderstood my reply. I did not say that you should create a column for each possible subscription. While this is not quite as bad as stuffing everything into one column, it still leads to major problems:

• Whenever you want to add, rename or delete an RSS service, you need to change the entire table structure and possibly update a lot of queries. This is a dead giveaway that there's something wrong with the data model.
• Some queries are extremely difficult to do. For example, you cannot even count the subscriptions of one user unless you hard-code all the possible feeds in the query (which means that the query has to be updated whenever the feeds change).

So, no, this is not a solution. Instead, you need an extra table as I've described in my reply. This table assigns users to RSS feeds. For example, if the user #3 has subscribed the feed #2, you insert the row (3, 2) into the table.

[macytan1988]

Hi. I have created three tables (below) based on your suggestion and renamed all the variables to avoid case-sensitive issues.

Everything goes smooth until an error message reads "Notice: Undefined index: member_ID in C:\xampp\htdocs\memberpage.php on line 17" pops up.
It seems that I'm unable to retrieve the user ID but I'm able to do so for username.
Can you teach me how to edit this file to allow the display of member_ID?

My user file is as follows:

 

<?php
include('password.php');

class User extends Password{

    private $_db;

    function __construct($db){
        parent::__construct();
    
        $this->_db = $db;
    }

    private function get_user_hash($username){    

        try {
            $stmt = $this->_db->prepare('SELECT password FROM members WHERE username = :username AND active="Yes" ');
            $stmt->execute(array('username' => $username));
            
            $row = $stmt->fetch();
            return $row['password'];

        } catch(PDOException $e) {
            echo '<p class="bg-danger">'.$e->getMessage().'</p>';
        }
    }
    

    public function login($username,$password){

        $hashed = $this->get_user_hash($username);
        
        if($this->password_verify($password,$hashed) == 1){
            $_SESSION['loggedin'] = true;
            $_SESSION['username'] = $username; //This addition works and it allows me to output the username.
            $_SESSION['member_ID'] = $member_ID; //I'm trying to add this line, but it fails to display the ID.
            return true;
        }     
    }
        
    public function logout(){
        session_destroy();
    }

    public function is_logged_in(){
        if(isset($_SESSION['loggedin']) && $_SESSION['loggedin'] == true){
            return true;
        }        
    }
    
    
    
}
    
?>

[Jacques1]

Why do you write “member_ID” with uppercase letters? The whole point of renaming the columns was to make them all-lowercase. So it's “member_id”.

[macytan1988]

 

Why do you write “member_ID” with uppercase letters? The whole point of renaming the columns was to make them all-lowercase. So it's “member_id”.

 

Done with the editing, but I'm still unable to get the id display

[macytan1988]

Ok, I tried again just now. No more error message like the one I mentioned in my previous post, but instead of inserting the correct id number, it inserts "0" into database.

PHP code for user.php:

 

<?php
include('password.php');

class User extends Password{

    private $_db;

    function __construct($db){
        parent::__construct();
    
        $this->_db = $db;
    }

    private function get_user_hash($username){    

        try {
            $stmt = $this->_db->prepare('SELECT password FROM members WHERE username = :username AND active="Yes" ');
            $stmt->execute(array('username' => $username));
            
            $row = $stmt->fetch();
            return $row['password'];

        } catch(PDOException $e) {
            echo '<p class="bg-danger">'.$e->getMessage().'</p>';
        }
    }
    

    public function login($username,$password){

        $hashed = $this->get_user_hash($username);
        
        if($this->password_verify($password,$hashed) == 1){
            $_SESSION['loggedin'] = true;                    
            $_SESSION['username'] = $username;
            $_SESSION['member_id'] = $member_id;
            return true;
        }     
    }
        
    public function logout(){
        session_destroy();
    }

    public function is_logged_in(){
        if(isset($_SESSION['loggedin']) && $_SESSION['loggedin'] == true){
            return true;
        }        
    }
    
    
    
}
    
?>

PHP for Subscription part:

 

//START: Subscription
            
    if(isset($_POST['subscribe'])){

        try{    
            $checkbox_values = $_POST['subs'];
            for ($i=0; $i<sizeof($checkbox_values);$i++){        
                $stmt = $db->prepare("INSERT INTO member_feed(member_id, feed_id) VALUES (':member_id', '".$checkbox_values[$i]."')");        
                $stmt->execute(array(
                        ':member_id' => $member_id
                        ));
            }
        }
        catch(PDOException $e) {
            $error[] = $e->getMessage();
        }    
    echo '<div class="alert alert-success"><a class="close" data-dismiss="alert">×</a>Subscription updated!</div>';    
}

//END: Subscription
 

[Jacques1]

Several things. I don't see the definition of $member_id anywhere, neither in the subscription part nor in the login() method.

 

But what's much worse is that now you have a major SQL injection vulnerability in your subscription query, because you just drop the raw subs parameter into the query string. An attacker is now able to manipulate the query in any way they want. For example, they could fetch the password hash of the admin and try to break it offline. If your database isn't configured correctly, they may even be able to compromise the entire server.

 

Never insert raw user input into a query string. I'm not even sure why you're doing this. You have a prepared statement right there. All you need to do is add a second parameter for the subscription ID.

 

You also shouldn't constantly create the same prepared statement in the loop. One important property of prepared statements is that they can be reused: You create them once and execute them as often as you want. So create the prepared statement outside of the loop and execute it within the loop.

<?php

$subscription_stmt = $db->prepare('
    INSERT INTO
        member_feed
    SET
        member_id = :member_id
        , feed_id = :feed_id
')
foreach ($_POST['subs'] as $subscribed_feed)
{
    $subscription_stmt->execute(array(
        'member_id' => $_SESSION['member_id'],
        'feed_id' => $subscribed_feed,
    ));
}

Last but not least, you need to delete the subscriptions which are no longer active. The easiest solution is to delete all current subscriptions of the member prior to the INSERT queries.

[macytan1988]

To be honest, I don't know how to edit the login file (user.php) to make member_id available for display. Can I get some hints on that please?

Also, I too don't know how to edit the HTML part for checkboxes in order for them to be "connected" to the database table, especially the feed_id. For now, I just entered the feed_id myself in this way:

 

        <input type="checkbox" name="subs[]" value="1" > The Malaysian Insider</input>
        <input type="checkbox" name="subs[]" value="2"> The Malay Mail Online</input>
        <input type="checkbox" name="subs[]" value="3"> The Star</input>

My user.php is still the same as above, but I have made some changes, according to your hints, on the PHP code for subscription.

//START: Subscription     
    $member_id = $_SESSION['member_id'];
    $subscription_stmt = $db->prepare('
        INSERT INTO
            member_feed
        SET
            member_id = :member_id,
            feed_id = :feed_id
    ');
    
    foreach ($_POST['subs'] as $subscribed_feed)
    {
    $subscription_stmt->execute(array(
        'member_id' => $_SESSION['member_id'],
        'feed_id' => $subscribed_feed
        ));
    }
    

//END: Subscription

 

However, these chunks of code give me two errors again

Notice: Undefined index: subs in C:\xampp\htdocs\memberpage.php on line 76

Warning: Invalid argument supplied for foreach() in C:\xampp\htdocs\memberpage.php on line 76

 

[macytan1988]

If I edited the code by including isset function, then the code runs without any error, but I'm still unable to get it work by inserting required data into the database. (When I click the button, it runs but inserts NOTHING into the database)

The member_id issue as I mentioned in my last post still exists.

 

//START: Subscription     
    $member_id = $_SESSION['member_id'];
    $subscription_stmt = $db->prepare('
        INSERT INTO
            member_feed
        SET
            member_id = :member_id,
            feed_id = :feed_id
    ');
    if(isset($_POST['invite'])){
        foreach ( (array)$subs as $subscribed_feed)
        {
            $subscription_stmt->execute(array(
            'member_id' => $_SESSION['member_id'],
            'feed_id' => $subscribed_feed
            ));
        }
    }

//END: Subscription

[mogosselin]

The error probably comes from the fact that your condition isn't true:

if(isset($_POST['invite'])){
        foreach ( (array)$subs as $subscribed_feed)
        {
            $subscription_stmt->execute(array(
            'member_id' => $_SESSION['member_id'],
            'feed_id' => $subscribed_feed
            ));
        }
    }

What this IF does, is it check if 'invite' (which is an index), exists in the array $_POST. My guess is that when you post your form, it's always false anyway.

 

do a var_dump($_POST) to see what's inside the $_POST variable. If 'invite' never exists, that's your problem.  Also, checks if the code goes into your 'foreach'. You could do a var_dump($subs) just to be sure.

[macytan1988]

Thanks for your reply mogosselin!

 

 

I have edited the code but it still has a minor error. This is my latest version of the code.

//START: Subscription
            
    if(isset($_POST['subscribe'])){
        if(isset($_POST['subs'])){
            $subscription_stmt = $db->prepare('
            INSERT INTO
                member_feed
            SET
            member_id = :member_id,
            feed_id = :feed_id
            ');
            

            foreach ( (array)$subs as $subscribed_feed)
            {
                $subscription_stmt->execute(array(
                'member_id' => $member_id,
                'feed_id' => $subscribed_feed
                ));
            }

            
            echo '<div class="alert alert-success"><a class="close" data-dismiss="alert">×</a>Subscription updated!</div>';  
        }
        
        else
        {
            echo '<div class="alert alert-danger"><a class="close" data-dismiss="alert">×</a>You have not selected any subscription!</div>';
        }
        

    }


//END: Subscription
 

When I tick a random checkbox and click the subscribe button, it shows this error:

Notice: Undefined variable: subs in C:\xampp\htdocs\memberpage.php on line 76 (I highlighted line 76 in red colour as shown above)

My HTML code for the checkboxes:

 

        <input type="checkbox" name="subs[]" value="1"> The Malaysian Insider</input>
        <input type="checkbox" name="subs[]" value="2"> The Malay Mail Online</input>
        <input type="checkbox" name="subs[]" value="3"> The Star</input>

 

Besides, it inputs nothing into the database . By the way, my previous issue on the display of member_id has been resolved but thus far, I still can't get it stored into the database.

Sorry for asking you so many questions in one go, but if you don't mind, can I get some hints on:

1. How should I get my checkboxes ticked according to the user's subscription, so when he/she logs into the system next time, he/she will be able to view it.

2. How and where should I code my DELETE part to allow users to remove the subscription by unticking the checkbox?

 

Thank you so much in advance and have a nice day ahead!

Edited August 5, 2014 by macytan1988

[macytan1988]

Oh wait! It's working now!

 

 

//START: Subscription
            
    if(isset($_POST['subscribe'])){
        if(isset($_POST['subs'])){
            $subscription_stmt = $db->prepare('
            INSERT INTO
                member_feed
            SET
            member_id = :member_id,
            feed_id = :feed_id
            ');
            
            if (!empty($_POST['subs'])){
            foreach ( $_POST['subs'] as $subscribed_feed)
            {
                $subscription_stmt->execute(array(
                'member_id' => $member_id,
                'feed_id' => $subscribed_feed
                ));
            }
            }
            
            echo '<div class="alert alert-success"><a class="close" data-dismiss="alert">×</a>Subscription updated!</div>';  
        }
        
        else
        {
            echo '<div class="alert alert-danger"><a class="close" data-dismiss="alert">×</a>You have not selected any subscription!</div>';
        }
        

    }


//END: Subscription

 

However, there is one more error here. Let say if the user has already subscribed to feed 1, when he clicks it again, the error message reads

 

Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '28-1' for key 'PRIMARY'' in C:\xampp\htdocs\memberpage.php:81 Stack trace: #0 C:\xampp\htdocs\memberpage.php(81): PDOStatement->execute(Array) #1 {main} thrown in C:\xampp\htdocs\memberpage.php on line 81

 

My question is, how can I set my own custom message like "You have already subscribed to this feed"?

OR

Should I recreate member_feed table and not set any of the fields as PRIMARY KEY to avoid this issue?
 

[Jacques1]

As I already said above, you need to delete all current subscriptions before you insert the new ones.

 

 

 

Should I recreate member_feed table and not set any of the fields as PRIMARY KEY to avoid this issue?

 

If the “check engine” light of your car is on, do you solve the problem by turning it off?

 

You need to fix the duplicate entries, not get rid of the error message. The message actually means something, it tells you that you have a bug in your code. And that's indeed the case: You're not deleting the current subscriptions.

 

If you only removed the PRIMARY KEY constraint, then you'd end up with tons of wrong and duplicate subscriptions.

[mogosselin]

Ljike @Jacques1 said, you need to DELETE everything related to one user and his subscription, it's the easiest way to do it.

 

For example, imagine that I have a table with these entries:

idUser / idSubscription

1 - 1

1 - 2

1 - 3

2 - 2

3 - 1

3 - 3

 

So, it basically  means that the user #1 is subscribed to the feed #1, 2 and 3.

The user #2 subscribed to the feed #2 only.

And the user #3, subscribed to the feed #3.

 

Now, imagine that you are logged in as the user #2 and you have those checkboxes on the page:

Feed #1 [  ]

Feed #2 [X]

Feed #3 [  ]

 

The Feed #2 is already 'ticked'. But lets say that I want to subscribe to the Feed #3 also, I'll get this form:

Feed #1 [  ]

Feed #2 [X]

Feed #3 [X]

 

And that's what I'm going to submit to the server.

Then, on the server side in your PHP code, you loop trough all the checkboxes you received and try to insert all of them. When you get to Feed #2 for the current user, you get this error message because the entry 'user #2 / feed #2' already exists in the subscription table.

 

Now, before inserting the data, you could check if it already exists. Imagine something like this:

For each subscription received

   Check if already exists

      If not, insert the new row

 

This would work, but it's not efficient. The easiest and fastest way would be something like this:

Delete every subscriptions for user #2

For each subscription received

   Insert new subscription

 

So, you have to run this query first:

DELETE FROM member_feed WHERE member_id = 2

 

Then, after this, you can loop trough all the subscription and insert them again.