07-17-2011, 11:24 AM
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:
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:$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:
$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:
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).