Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Limit Attachments Downloads Per Day Review

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: Limit Attachments Downloads Per Day
Plugin Version: 1.0 (last updated 12th July 2011)
Plugin Author: Mohammad Zangeneh
Author Message
ZiNgA BuRgA Offline
Fag
*******
Posts: 3,357
Joined: Jan 2008
Post: #1
Limit Attachments Downloads Per Day Review
This is a plugin "review" requested by hamed.ramzi.

Firstly, I don't see any major issues with this plugin, which is good.  My main criticisms are thus rather minor.  But anyway, here goes:
  • I have a feeling that duplicating this hook was a mistake:

    PHP Code:
    $plugins->add_hook("attachment_start", "at_limit_on");
    $plugins->add_hook("attachment_end", "at_limit_on");

    (I'm guessing the second call is redundant)

  • Ideally, use $db->write_query but try to avoid mixing it with $db->query:

    PHP Code:
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    $db->write_query ("CREATE TABLE `".TABLE_PREFIX."atdl` (
    	  `did` smallint(8) UNSIGNED NOT NULL auto_increment,
    	  `uid` bigint(30) UNSIGNED NOT NULL default '0',
    	  `dateline` bigint(30) UNSIGNED NOT NULL default '0',
    	  PRIMARY KEY  (`did`)
    		) TYPE=MyISAM");
    		
    // Add Field
    
    	$db->query("ALTER TABLE ".TABLE_PREFIX."usergroups ADD attachdllimit INT(5) NOT NULL DEFAULT '5'");

  • PHP Code:
    1
    2
    3
    4
    5
    6
    	$q = $db->simple_select("atdl", "COUNT(*) AS dl_num", "uid='{$mybb->user['uid']}' AND dateline >='".(TIME_NOW - (60*60*24))."'");
    	$atdls = $db->fetch_field($q, 'dl_num');
    	if($mybb->input['thumbnail'])
    	{
    	return;
    	}

    Firstly, I'd do the thumbnail check before the query.  Secondly, I'd recommend a key on uid and dateline (during CREATE TABLE).

  • PHP Code:
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    11
    12
    function at_limit_acp_ug($pluginargs)
    {
    global $db, $mybb, $form, $lang;
    
    $lang->load('at_limit');
    
    if($pluginargs['title'] == $lang->misc && $lang->misc)
    {
    			$pluginargs['content']= "{$lang->acp_atdl_title}<br /><small class=\"input\">{$lang->acp_atdl_desc}</small><br />".$form->generate_text_box('attachdllimit', $mybb->input['attachdllimit'], array('id' => 'attachdllimit', 'class' => 'field50'));
    
    }
    }

    Firstly, it seems unnecessary to load the language every time a formcontainer row is output (then again, it's the ACP, so performance isn't a big issue, plus it's a small file).  Secondly, I don't quite feel comfortable with the check, since, potentially, places other than the usergroup editor may use $lang->misc.  The biggest problem, however, is that it removes all other content in the Miscellaneous section of the edit usergroup page.  Content should be appended with ".=", not assigned with "=" operator.

  • It's unclear what happens if a user is in more than one usergroup - which limit applies?  Perhaps consider adding to the 'attachdllimit' to the $groupzerogreater array (see bottom of inc/class_core.php).

My Blog
07-17-2011 11:24 AM
Find all posts by this user Quote this message in a reply


Messages In This Thread
Limit Attachments Downloads Per Day Review - ZiNgA BuRgA - 07-17-2011 11:24 AM

Forum Jump: