Not reporting right message

[Xtremer360]

I"m trying to find out why if both my variables are set to NULL why is it not showing the correct if statement.  It should be showing the TRUE if statement  message but its displaying a message further down in my controller.

 

if ((!is_numeric($user_id)) || ($registration_key == NULL))
        {
            $message = 'One or both parameters were not entered!';
        } 
        else if (!(($user_id > 0) && (preg_match('/^[A-Za-z0-9]+$/', $registration_key)))) 
        {   
            $message = 'The parameters do not meet the validation criteria!';
        }

[MMDE]

I'm not entirely sure what you mean, but if $registration_key is set to NULL, it will show the first message, because:

$registration_key == NULL

part in:

if ((!is_numeric($user_id)) || ($registration_key == NULL))

would be true. Since you use ||, which means or and returns true if one or both parts are true.

 

Read more about it in the manual:

http://php.net/manual/en/language.operators.logical.php

[Xtremer360]

But it doesn't

[MMDE]

When I did this:

<?php
if ((!is_numeric($user_id)) || ($registration_key == NULL))
        {
            $message = 'One or both parameters were not entered!';
        } 
        else if (!(($user_id > 0) && (preg_match('/^[A-Za-z0-9]+$/', $registration_key)))) 
        {   
            $message = 'The parameters do not meet the validation criteria!';
        }
echo $message;
?>

 

I got this:

<br />
<b>Notice</b>:  Undefined variable: user_id in <b>x:\pathtofile.php</b> on line <b>2</b><br />
One or both parameters were not entered!

 

With other words, you need to check if $user_id is set, isset().

 

You may also want to do this before line 2:

var_dump($user_id);
var_dump($registration_key);

so you know what they are for sure.

[Pikachu2000]

Post the code where you set the variables to NULL.

[Xtremer360]

Here's my full code. Notice I don't have any values in there now but you can put a number in there and then a string for the second and none of my validation is working.

 

http://pastebin.com/M2PpyMy7

 

http://www.kansasoutlawwrestling.com/kowmanager/activation

 

 

[Pikachu2000]

Which error message are you getting?

[Xtremer360]

The user is now activated! You may now proceed to the login page.

KOW Manager Login Page

 

 

[MMDE]

Well, one problem is it that you keep re-assigning $message all the time.

 

You should do it like this:

 

$message = '';
if ((!is_numeric($user_id)) || ($registration_key == NULL))
        {
            $message .= 'One or both parameters were not entered!<br />';
        } 
        else if (!(($user_id > 0) && (preg_match('/^[A-Za-z0-9]+$/', $registration_key)))) 
        {   
            $message .= 'The parameters do not meet the validation criteria!<br />';
        }

 

You see the dot? Now it will add each message to $message, instead of just the last message.

You may also want to print something to the screen everywhere you want to check if it gets to in the script.

 

 

Another thing to be noted, even if it fails to find an id, you later just keep using id as if it was valid.

 

if (!$this->users_model->is_registered($user_id))

 

Use return to exit the method/function early.

[Xtremer360]

Hmm what do you suggest with that second part?

[MMDE]

Hmm what do you suggest with that second part?

I actually suggest you break it into more methods/functions.

 

Way too big method/function. Try to keep it at under 10 lines or so.

    public function index($user_id = NULL, $registration_key = NULL)
    {
        $message_box_messages = array();
        $css_page_addons = '';
        $js_page_addons =  '';
        $meta_tag_addons = '';
        $site_title = 'KOW Manager Account Activation';
   
        var_dump($user_id);
        var_dump($registration_key);
       
        if ((!is_numeric($user_id)) || ($registration_key == NULL))
        {
            $message = 'One or both parameters were not entered!';
        }
        else if (!(($user_id > 0) && (preg_match('/^[A-Za-z0-9]+$/', $registration_key))))
        {  
            $message = 'The parameters do not meet the validation criteria!';
        }
       
        if (!$this->users_model->is_registered($user_id))
        {
            $message = 'The user specified does not exist in the database!';
        }
       
        $user_status_id = $this->users_model->get_user_status_id($user_id);
       
        if ($user_status_id != 1)
        {
            $message = 'The user specified is already activated! You may now proceed to the login page.<br /><br /><a href="' . base_url() . 'login">KOW Manager Login Page</a>';
        }
        else if (!$this->users_model->check_user_registration_key($user_id, $registration_key))
        {
            $message = 'The registration key did not match the user specified!';
        }
       
        $this->db->trans_start();
       
        if ($this->users_model->change_account_type($user_id, 2))
        {
            if ($this->users_model->erase_registration_key($user_id, $registration_key))
            {
                $activation_date = gmdate('Y-m-d H:i:s');
                if ($this->users->insert_activation_date($user_id, $activation_date))
                {
                    $message = 'The user is now activated! You may now proceed to the login page.<br /><br /><a href="' . base_url() . 'login">KOW Manager Login Page</a>';
                }
                else
                {
                    $message = 'The activation date was not able to be inserted properly!';
                }
            }
            else
            {
                $message = 'The registration key failed to erase for the user specified!';        
            }
        }
        else
        {
            $message = 'The user specified was not able to have his account changed!';
        }
   
        $this->db->trans_complete();
   
        if ($this->db->trans_status())
        {
            $message = 'The user is now activated! You may now proceed to the login page.<br /><a href="' . base_url() . 'login">KOW Manager Login Page</a>';
        }
        else
        {
            $message = 'The user was not able to be activated. The account specified was not able to be changed into a user!';  
        }
   
       
        $body_content = $this->config->item('themes_path').'/'.$this->config->item('default_theme').'/usermanagement/general_message';
        $body_type = 'full';
       
        if (count($message_box_messages) !== 0)
        {
            $message_boxes = $this->functions_model->build_message_boxes_output(array('display' => 'show', 'messages' => $message_box_messages));
        }
        else
        {
            $message_boxes = array('display' => 'none');
        }
       
        $meta_tags = $this->functions_model->meta_tags();
       
        if (isset($site_title) && (empty($site_title)))
        {
            $site_title = $this->functions_model->site_title();
        }
       
        $this->data['message_boxes'] = $message_boxes;
        $this->data['css_page_addons'] = $css_page_addons;
        $this->data['js_page_addons'] = $js_page_addons;
        $this->data['site_title'] = $site_title;
        $this->data['body_content'] = $body_content;
        $this->data['body_type'] = $body_type;
        $this->data['meta_tags'] = $meta_tags;
        $this->data['message'] = $message;
        $this->load->view($this->config->item('themes_path').'/'.$this->config->item('default_theme').'/usermanagement/index', $this->data );
    }
}

 

I suspect you didn't really write this entire code.

 

Why do you create $message_box_messages = array(); without using it? I suspect that is where you were supposed to store all the messages, and not like you've done it where they overlap each other.

 

What I tried to say at the end is that you need to use "return" to end the method when for example $user_id is not set. This is because you some few lines later try to use something you've probably already made sure isn't set. Let me put it in other words, you are aware something doesn't exist, but then ignore that fact and attempt to use it anyways.

 

return is used to make a function return a value, but it can be set alone like this: return; if you don't want it to return anything (void), and by that canceling executing the rest of the function.