Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
OUGC Awards

Please note that this is pretty much a negative criticism post, rather than a balanced review as mentioned in this thread. Also be aware that stuff posted here may be highly subjective.
Please feel free to criticise this post, however.

Plugin Reviewed: OUGC Awards
Plugin Version: 1.0.2 (last updated 15th April 2012)
Plugin Author: Omar Gonzalez
Author Message
ZiNgA BuRgA Offline
Fag
*******
Posts: 3,357
Joined: Jan 2008
Post: #1
OUGC Awards
This is a plugin "review" requested by Sama34.

Quite a fair bit of code, so I won't bother going through everything.

Plugin allows users to be awarded through the ModCP or AdminCP.  Awards displayed in profile/postbit.  An AdminCP interface for managing awards is also provided.
I'm somewhat intrigued by the 'proxy' inc/plugins/ougc_awards.php file - can't quite grasp the idea behind it - maybe putting all code into one folder?

Anyway...

awards.php
  • The check on whether the plugin is installed is a little pointless, due to the fact that the language load call will fail if that's the case

    PHP Code:
    1
    2
    3
    4
    5
    6
    7
    8
    ougc_awards_lang_load();
    $awards = $cache->read('ougc_awards');
    
    // If plugin no active or user has no useid then stop.
    if(!$mybb->user['uid'] || $mybb->settings['ougc_awards_power'] != 1 || !function_exists('ougc_awards_is_installed') || !is_array($awards) || !$awards)
    {
    	error_no_permission();
    }

    I suggest putting both that and the cache load after the check.

  • In-loop query used here - consider using a JOIN in the query

    PHP Code:
    		$users = $db->simple_select('ougc_awards_users', '*', "aid IN ({$award['aid']})", array('order_by' => 'date', 'order_dir' => 'desc'));
    		while($gived = $db->fetch_array($users))
    		{
    			$trow = alt_trow();
    			$user = get_user($gived['uid']);


inc/plugins/ougc_awards/plugin.php
  • PHP Code:
    1
    2
    3
    4
    5
    6
    7
    8
    9
    		$db->write_query("CREATE TABLE `".TABLE_PREFIX."ougc_awards_users` (
    				`gid` bigint(30) UNSIGNED NOT NULL AUTO_INCREMENT,
    				`uid` bigint(30) NOT NULL DEFAULT '0',
    				`aid` bigint(30) NOT NULL DEFAULT '0',
    				`reason` varchar(255) NOT NULL DEFAULT '',
    				`date` int(10) NOT NULL DEFAULT '0',
    				PRIMARY KEY (`gid`)
    			) ENGINE=MyISAM;"
    		);

    I'd consider putting a unique key over (uid,aid) as it seems that it should be.  Perhaps also consider sticking indexes on uid as well as aid.

  • ougc_awards_lang_load is sometimes called multiple times on a single page load.  Perhaps consider restricting this to one call per page.
  • Over-sanitisation used here (and possibly in other places):

    PHP Code:
    1
    2
    3
    4
    5
    6
    7
    8
    			if($mybb->input['reason'])
    			{
    				$mybb->input['reason'] = htmlspecialchars_uni($mybb->input['reason']);
    			}
    			if($mybb->request_method == 'post')
    			{
    			...
    					ougc_awards_give_award($award, $user['uid'], $mybb->input['reason']);

    (a htmlspecialchar'd version of the reason is being saved to the DB)

  • PHP Code:
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    11
    12
    	while($award = $db->fetch_array($q1))
    	{
    		$q2 = $db->simple_select('ougc_awards_users', 'uid', "aid='{$award['aid']}'");
    		$users = array();
    		while($user = $db->fetch_array($q2))
    		{
    			$users[] = intval($user['uid']);
    		}
    		$db->free_result($q2);
    		$update = implode(',', $users);
    		ougc_awards_update_award($award['aid'], array('users' => $update));
    	}

    ...noting that the users column that is being saved to is defined as varchar(100) - ie, a large amount of users will easily overflow the column.  I recommend rethinking this behaviour altogether - do you really want to potentially cache such a huge amount of data?


Some files (language files, possible others) are using the short opening tag (<?) as opposed to the full tag (<?php) - I suggest ensuring all files use the latter as short tags may be disabled.

My Blog
04-19-2012 05:16 PM
Find all posts by this user Quote this message in a reply


Messages In This Thread
OUGC Awards - ZiNgA BuRgA - 04-19-2012 05:16 PM
RE: OUGC Awards - Sama34 - 04-22-2012, 09:04 AM
RE: OUGC Awards - ZiNgA BuRgA - 04-22-2012, 08:34 PM
RE: OUGC Awards - Sama34 - 04-23-2012, 01:30 PM
RE: OUGC Awards - ZiNgA BuRgA - 04-23-2012, 04:54 PM
RE: OUGC Awards - Sama34 - 04-24-2012, 01:59 PM

Forum Jump: