Simple Karma Challenge

[Highway of Life]

Yesterday I started considering the possibility of porting a karma mod I use(d) on my Arpia boards to phpBB3. I started doing modifications here and there, but now, I've hit a wall: my karma.php file never seems to work, no matter how much I try to alter it. I just get a blank page.

Just as means of a quick intro, the idea behind this karma mod is that moderators & admin can change user karma, from -8 to +8. I won't start a discussion about the merits of this karma system, in comparison with others (*me winks at HoL*), but basically, it's fine for the use I have for it (i.e. thanking people who have helped me for my plug, ?).

Anyway, you call the karma.php file (karma.php?t= topic ID &u= target user ID &x= applaud/smite ), and this is karma.php:

Spoiler:

Code: Select all

<?php
/***************************************************************************
 *                               karma.php
 *                            -------------------
 *   begin                : Thursday, Jan 24, 2004
 *   copyright            : (C) Nome
 *   email                : nome@bk.ru
 *
 ***************************************************************************/

/***************************************************************************
 *
 *   This program is free software; you can redistribute it and/or modify
 *   it under the terms of the GNU General Public License as published by
 *   the Free Software Foundation; either version 2 of the License, or
 *   (at your option) any later version.
 *
 ***************************************************************************/

define('IN_PHPBB', true);
// PaceNote: I used to have an "include?? common.php", but removed it to see if it solved the problem. It didn't?


// Some extra script settings. You can modify them
// How long do we have to wait before giving karma points again?
$hours_past = 1; // In hours
// Do admins and moders have the permission to give karma points any time they like&
$allow_up = 1; // NOTE THAT 0 = yes, 1 (or above)= no and not otherwize!  

// Start session management => PaceNote: taken from ucp.php. Used to be two other lines around it, removed to see if it helped.
$auth->acl($user->data);

    $sql = "SELECT karma_time FROM ' . USERS_TABLE . ' WHERE user_id = ' . $user->data['user_id']' "; //get last time user tried a karma vote
    $result = $db->sql_query($sql);
    $array = mysql_fetch_array($result);
    $time_old = $array[0];
    $sql = "SELECT user_id FROM ' . USERS_TABLE . ' WHERE user_id = ' . $user->data['user_id']' ";//make sure no one votes for themselves
    $result = $db->sql_query($sql);
    $array = mysql_fetch_array($result);
    $voter_id = $array[0];

// $_GET variables
if ( isset($_GET['t']) ) { $topic_id = $_GET['t']; } else { die("Hacking attempt"); }
if ( isset($_GET['u']) ) { $user = $_GET['u']; } else { die("Hacking attempt"); }  // PaceNote: this is the thing I'm most annoyed with? Can I use $user without any worries?
if ( isset($_GET['x']) ) { $x = $_GET['x']; } else { die("Hacking attempt"); }

    if($voter_id == $user)
    {
        message_die(CRITICAL_MESSAGE, $user->lang['No_Self_Karma'] . '<br /><a href="' . append_sid("{$phpbb_root_path}viewtopic.$phpEx", "f=$forum_id&amp;t=$topic_id") . '"> ' . $user->lang['Return_To_Topic'] . ' </a>');
    }
    else
    {
        if(!$auth->acl_gets('a_', 'm_') && !$auth->acl_getf_global('m_'))  // PaceNote: i.e., non-mods
        {
            message_die(CRITICAL_MESSAGE, $user->lang['No_User_Karma'] . '<br /><a href="' . append_sid("{$phpbb_root_path}viewtopic.$phpEx", "f=$forum_id&amp;t=$topic_id") . '"> ' . $user->lang['Return_To_Topic'] . ' </a>');

        }
        else
        {
            $time = time();
            $diff = $time - $time_old;
            if($diff >= 3600 * $hours_past) //make sure they haven't voted in the last hour or if they're a mod or admin, they can continue
            {
                if ($x == 'applaud')
                {
                    $sql = "SELECT karma FROM ' . USERS_TABLE . ' WHERE user_id = '$user' "; //Find the good guy
                    $result = $db->sql_query($sql); 
                    $array = mysql_fetch_array($result);
                     $karma = $array[0];
                    // We only up karma by one
                    $karma = $karma + 1;
                    // Here comes the db update 
                    $karma_update = "UPDATE ' . USERS_TABLE . ' SET karma = '$karma' WHERE user_id = '$user' ";
                    
                }
                else
                // If someone tries to fake the x input, that someone will get bad karma ;)
                {
                    $sql = "SELECT karma FROM ' . USERS_TABLE . ' WHERE user_id = '$user' "; //Find the bad guy
                    $result = $db->sql_query($sql); 
                    $array = mysql_fetch_array($result);
                     $karma = $array[0];
                    // We only up karma by one
                    $karma = $karma - 1;
                    // Add bans on reaching -8 karma
                    if ($karma <= -8)
                    {
                        $sql = "INSERT INTO phpbb3_banlist VALUES (0, $user, '', NULL)";
                        $db->sql_query($sql);
                    }
                    // Here comes the db update 
                    $karma_update = "UPDATE ' . USERS_TABLE . ' SET karma = '$karma' WHERE user_id = '$user' ";
                }

    
                //update the database with current time() for voter
                $time_update = "UPDATE ' . USERS_TABLE . ' SET karma_time = '$time' where user_id = '$user->data['user_id']' ";
                $result = $db->sql_query($karma_update);
                $time_result = $db->sql_query($time_update);
    
                if($result&&$time_result) //Both gotta happen...
                {       
                    header('Location:' . append_sid("{$phpbb_root_path}ucp.$phpEx", 'i=pm&amp;mode=compose&amp;u=' . $row['user_id']));
                    $template->assign_vars(array(
                        'L_SUBJECT' => $user->lang['Karma_Change'])
                    );
                }
                else
                {
                    message_die(GENERAL_ERROR, $user->lang['Critical_Error'] . '<br /><a href="' . append_sid("{$phpbb_root_path}viewtopic.$phpEx", "f=$forum_id&amp;t=$topic_id") . '"> ' . $user->lang['Return_To_Topic'] . ' </a>', __LINE__, __FILE__, $sql);
                }
            }
            else
            {
            message_die(CRITICAL_MESSAGE, $user->lang['Too_Soon'] . '<br /><a href="' . append_sid("{$phpbb_root_path}viewtopic.$phpEx", "f=$forum_id&amp;t=$topic_id") . '"> ' . $user->lang['Return_To_Topic'] . ' </a>');
            }
        }
    }

?>

What causes this blank page?

[Handyman]

It could be because you forgot to include the common.php at the start of the page.
Here is how the first part of the page should look:

Code: Select all

define('IN_PHPBB', true);
$phpbb_root_path = './';
$phpEx = substr(strrchr(__FILE__, '.'), 1);
include($phpbb_root_path . 'common.' . $phpEx);

// Start session management
$user->session_begin();
$auth->acl($user->data);
$user->setup();


[Pace]

I had it before, but removed it. In fact, I had all of those elements. I'll see if somehow I'd managed to badly write them down/copy them from other pages.

Weird, it now works. Probably because I didn't put all of those things together at the top (I had a bit between them). Now I get a SQL error, but at least, it means I see what's happening and where the mistakes are

Thanks!

[Handyman]

[Highway of Life]

message_die is no longer used... use trigger_error instead.
Don't use die();

Don't use $_GET, use request_var();
Wrong:

Code: Select all

if ( isset($_GET['t']) ) { $topic_id = $_GET['t']; } else { die("Hacking attempt"); }

Correct:

Code: Select all

$topic_id = request_var('t', '');

All formatting should use the normal phpBB coding guidelines:
Wrong:

Code: Select all

if (isset($variable)) { $topic_id = $variable; } else { die("Hacking attempt"); }

Correct:

Code: Select all

if (isset($variable))
{
    $topic_id = $variable;
}
else
{
    die("Hacking attempt");
}

"L_" should not be used in the PHP file, but it should be defined in the template file whenever possible.

Conditional statements should have proper spacing,
e.g.:

Code: Select all

if($result&&$time_result)

should be:

Code: Select all

if ($result && $time_result)

Query text is incorrect:

Code: Select all

$time_update = "UPDATE ' . USERS_TABLE . ' SET karma_time = '$time' where user_id = '$user->data['user_id']' ";

That's going to give you an error because your string begin is a double quote... which means you need a double quote to break out to the USERS_TABLE... and you also need to break out of the string for the user->data variable...
Here is that string in the correct format.

Code: Select all

$time_update = "UPDATE " . USERS_TABLE . " SET karma_time = '$time' where user_id = '" . $user->data['user_id'] . "'";

An update or insert SQL statement don't need to be defined as variables... They just need to run the query, this is the correct example:

Code: Select all

$sql = "UPDATE " . TABLE_NAME . " SET variable = '" . $variable . "' WHERE other_var = '" . $other_variable . "'";
$db->sql_query($sql);

Don't specify a header in the PHP file unless you are NOT going to use a template, or NOT going to use the overall_header.html in your template.

Don't use a Database specific function... use the phpBB pre-defined functions.
Example: Do NOT use:

Code: Select all

$array = mysql_fetch_array($result);

Instead, use:

Code: Select all

$karma_ary = $db->sql_fetchrowset($result);

The "hours_past" should be a config value...

Code: Select all

$config['karma_time_limit'];

Another example why phpBB2 MODs can't and should not be ported to phpBB3.
You're going to do a complete re-write, so might as well make the copyright your own.

[Handyman]

the karma mod was a mod from phpbbhacks and was not approved by the phpBB mod team

[Pace]

Funny how, for most of those errors, I figured them out on my own

I have two questions though:

- what do you mean,

Code: Select all

$config['karma_time_limit'];

?

- and concerning the

Code: Select all

$karma_ary = $db->sql_fetchrowset($result);

does that mean I put

Code: Select all

                    $sql = "SELECT karma FROM " . USERS_TABLE . " WHERE user_id = '$user' "; //Find the bad guy
                    $result = $db->sql_query($sql); 
                    $karma_ary = $db->sql_fetchrowset($result);
                    // We only up karma by one
                    $karma = $karma_ary - 1;


instead of

Code: Select all

                    $sql = "SELECT karma FROM " . USERS_TABLE . " WHERE user_id = '$user' "; //Find the bad guy
                    $result = $db->sql_query($sql); 
                    $array = mysql_fetch_array($result);
                     $karma = $array[0];
                    // We only up karma by one
                    $karma = $karma - 1;


(for example)

[Handyman]

$config['karma_time_limit'];

he's saying that should go into the phpbb_config database table? make it a config value.

Code: Select all

  $sql = "SELECT karma FROM " . USERS_TABLE . " WHERE user_id = '$user' "; //Find the bad guy
                    $result = $db->sql_query($sql); 
                    $array = mysql_fetch_array($result);
                     $karma = $array[0];
                    // We only up karma by one
                    $karma = $karma - 1;

That's not a very good piece of code anyways.
If you are trying to subtract karma from an user by 1, just do this

Code: Select all

$sql = 'UPDATE ' . USERS_TABLE . " SET karma = karma - 1 WHERE user_id = $user";
$db->sql_query($sql);

That's all you need to subtract karma from an user? you can also do just the opposite for adding karma.

Code: Select all

$sql = 'UPDATE ' . USERS_TABLE . " SET karma = karma + 1 WHERE user_id = $user";
$db->sql_query($sql);[/code]


[Highway of Life]

Excellent! (that you figured most of them out on your own)

It would be more like:

Code: Select all

$karma = $karma_ary['karma'][0] - 1;

But if you are only fetching one row, and one field from that row, then you should just use:

Code: Select all

$result = $db->sql_query($sql);
$karma_result = $db->sql_fetchfield('karma');
$db->sql_freeresult($result);

$karma = $karma_result - 1;

As far as the config, I'm referring to this line:

Code: Select all

if($diff >= 3600 * $hours_past)

It should be something more like:

Code: Select all

if ($diff >= 60 * $config['karma_time_limit'])

And the karma_time_limit would be added to the config table, and the acp_board.php
Where you would set the karma time limit in minutes.

Note, there is a problem with about 90-95% of the code, so be sure to do a complete rewrite... I would start from scratch and just use the "ideas" from that MOD... er... hack...
The phpBB2 version doesn't deserve the "MOD" status.

[Handyman]

Code: Select all

$karma = $karma_ary['karma'][0] - 1;

It shouldn't be putting 1 result into an array... and if it pulls more than 1 result, there is a problem because its query only selects one? so use the code I posted above.