Threaded Mode | Linear Mode

***ZiNgA BuRgA*** · 11-26-2012 04:04 PM

Adding to our collection is the 5th OUGC plugin! The nice prefix littered virtually everything, which I struggle to pronounce, demonstrates the author's intent on trying self improve!

The plugin is called something to do with reputations, but I'm going to call it post ratings, as it seems to be a more accurate description (ratings are associated with posts & users, not just users).

So one of the first things you notice is that this package includes screenshots. Now before you moan about a >500KB download package, this one happens to be under 100KB - nice one!
Nonetheless, there's a fair bit more code than I'm willing to go through, so yeah...

How to use the rating interface isn't obvious in any way - you've got an icon, then a "x [number]"; not only is this difficult to see that it's meant to be clicked on, the fact that the icon and number links and their respective functions isn't obvious. You can toggle the rating you've chosen by clicking on the icon, although this isn't exactly obvious either (there's no visual indication (not even in a tooltip) on whether clicking the icon will add to the reputation or remove from it). Note that, however, you can't change your rating by clicking on another icon - you need to 'unrate' the post by clicking on the rating you originally specified (and this can be difficult to tell since there's no visual indication) and then click the other rating. For example, if there's two ratings, "Good" and "Bad", and you originally rated a post as "Bad" but now want to change it to "Good", you have to click "Bad" first (to unrate the post), then click "Good" (to rate the post) - you can't simply click "Good" and have the rating carry across.
Rating interface does not work with Javascript disabled (seems like a bug as opposed to intentional behaviour). Interestingly, turning off the AJAX setting makes it fail to work as well, regardless of whether Javascript is enabled or not

PHP Code:

	$PL->settings('ougc_customrep', $lang->ougc_customrep, $lang->ougc_customrep_d, array(
		'groups'	=> array(
			'title'			=> $lang->ougc_customrep_s_groups,
			'description'	=> $lang->ougc_customrep_s_groups,
			'optionscode'	=> 'text',
			'value'			=> '7,1,5',
		),
		'forums'	=> array(
			'title'			=> $lang->ougc_customrep_s_forums,
			'description'	=> $lang->ougc_customrep_s_forums,

Having the same description as the title really doesn't help people understand what you mean...

Code:
`groups` text NOT NULL DEFAULT '', `forums` text NOT NULL DEFAULT '',
You can't specify default values for a text field in MySQL (although MySQL will probably just ignore the error)
I question the use of bigint's in most database columns added, but I guess it really doesn't matter...
PHP Code:
1 2 3 4 5 6 7 8

if($customrep->firstpost_only) { $pids = "pid='{$thread['firstpost']}'"; } else { $pids = $GLOBALS['pids']; }
Breaks if showthread is in threaded mode because $GLOBALS['pids'] isn't set

PHP Code:

	if(!isset($customrep->cache['query']))
	{
// ...
	if($customrep->firstpost_only)
	{
		global $thread;

		if($post['pid'] != $thread['firstpost'])
		{
			return;
		}

		global $plugins;

		$plugins->remove_hook('postbit', 'ougc_customrep_postbit');
	}

I'd perform the first post check before running the query. I'm not entirely sure of the reason behind checking whether "$post['pid'] != $thread['firstpost']" as this will always be true if not on the first page of the thread.

The ougc_customrep_log table seems to mostly query on the rid,pid columns - there really should be indexes on these; one would presume that pid,uid should be a unique key as you can't vote for more than once for each post. The good thing about identifying these uniques is that it could help one simplify something like
PHP Code:
$customrep->cache['query'][$rep['rid']][$rep['pid']][$rep['lid']][$rep['uid']] = 1; //TODO
where the 'lid' key seems to be unnecessary
On postbit, the plugin loads ALL votes, and does a summation + check for current user voting on this. This can potentially be problematic if there happens to be a large number of votes.
PHP Code:
$lang->ougc_customrep_popup_date = $lang->sprintf($lang->ougc_customrep_popup_date, $log['date'], $log['time']);
Overwrites $lang var in a loop = broken behaviour
~~What does Reputation Type do?~~

A decent post rating (classification perhaps?) plugin, but improvement on the user interface needs to be done IMO.

Sama34 · 11-26-2012 06:00 PM

Quote:ratings are associated with posts & users, not just users

Wish I knew before naming my plugin xD

Quote:Rating interface does not work with Javascript disabled (seems like a bug as opposed to intentional behaviour). Interestingly, turning off the AJAX setting makes it fail to work as well, regardless of whether Javascript is enabled or not

hm, I tested on a clean board before releasing and it was fully functional, no sure what happened.

Quote:The ougc_customrep_log table seems to mostly query on the rid,pid columns - there really should be indexes on these; one would presume that pid,uid should be a unique key as you can't vote for more than once for each post. The good thing about identifying these uniques is that it could help one simplify something lik

May you please give me an MyBB example of how to do such thing while modifying/creating tables?

Quote:What does Reputation Type do?

You can select Negative, Neutral, or Positive reputation for each rate, and every time a post is rated reputation will be added to the user depending on that rate reputation type.

Will carefully read tomorrow, thanks!

BTW, OUGC = Omar U*** Gonzalez C***, it is my name acronym (I can re-use plugin names this way xD). Just so anyone knows Tongue

.

***ZiNgA BuRgA*** · 11-27-2012 02:35 PM

(11-26-2012 06:00 PM)Sama34 Wrote: hm, I tested on a clean board before releasing and it was fully functional, no sure what happened.

set_url() isn't being called. Though that doesn't seem to be the only issue.

(11-26-2012 06:00 PM)Sama34 Wrote: May you please give me an MyBB example of how to do such thing while modifying/creating tables?

Can be defined in the CREATE TABLE statement (for examples, see install/resources/mysql_tables.php or whatever it's called).
Worthwhile reading a bit about MySQL's indexes: http://dev.mysql.com/doc/refman/5.5/en/c...http://dev.mysql.com/doc/refman/5.5/en/create-
In general, you just need to specify which columns you want in an index.

(11-26-2012 06:00 PM)Sama34 Wrote: You can select Negative, Neutral, or Positive reputation for each rate, and every time a post is rated reputation will be added to the user depending on that rate reputation type.

Ah I see. You may wish to put that it's a +1/0/-1 (since reputations can have different weights). Also I suppose that changing the type won't update existing reputations?

Sama34 · 11-30-2012 07:44 PM

(11-26-2012 04:04 PM)ZiNgA BuRgA Wrote: Having the same description as the title really doesn't help people understand what you mean...

Bug. Language variables were actually added.

(11-26-2012 04:04 PM)ZiNgA BuRgA Wrote: I question the use of bigint's in most database columns added, but I guess it really doesn't matter...

Would using int (not int(n)) be any better?

(11-26-2012 04:04 PM)ZiNgA BuRgA Wrote: Breaks if showthread is in threaded mode because $GLOBALS['pids'] isn't set

Bug. Who really uses threaded mode anyways ¬_¬

(11-26-2012 04:04 PM)ZiNgA BuRgA Wrote: I'd perform the first post check before running the query. I'm not entirely sure of the reason behind checking whether "$post['pid'] != $thread['firstpost']" as this will always be true if not on the first page of the thread.

Makes sense.

(11-26-2012 04:04 PM)ZiNgA BuRgA Wrote: Overwrites $lang var in a loop = broken behaviour

Bug.

Quote:On postbit, the plugin loads ALL votes, and does a summation + check for current user voting on this. This can potentially be problematic if there happens to be a large number of votes.

What do you suggest? I avoided to add a new column to the posts table since I hate doing so. If I understood correctly.

Quote:Can be defined in the CREATE TABLE statement (for examples, see install/resources/mysql_tables.php or whatever it's called).
Worthwhile reading a bit about MySQL's indexes: http://dev.mysql.com/doc/refman/5.5/en/c...index.html
In general, you just need to specify which columns you want in an index.

Do you think the following is correct?

SQL Code

	$db->write_query("CREATE TABLE `".TABLE_PREFIX."ougc_customrep_log` (
			`lid` int UNSIGNED NOT NULL AUTO_INCREMENT,
			`pid` int NOT NULL DEFAULT '0',
			`uid` int NOT NULL DEFAULT '0',
			`rid` int NOT NULL DEFAULT '0',
			`dateline` int(10) NOT NULL DEFAULT '0',
			PRIMARY KEY (`lid`),
			UNIQUE KEY piduid (pid,uid),
			CREATE INDEX pidrid (pid,rid)
		) ENGINE=MyISAM{$collation};"
	);

///---

	$db->write_query('ALTER TABLE '.TABLE_PREFIX.'ougc_customrep_log ADD UNIQUE KEY piduid (pid,uid)');
	if(!$db->index_exists('ougc_customrep_log', 'pidrid'))
	{
		$db->write_query('CREATE INDEX pidrid ON '.TABLE_PREFIX.'ougc_customrep_log (pid,rid)');
	}

Quote:Ah I see. You may wish to put that it's a +1/0/-1 (since reputations can have different weights). Also I suppose that changing the type won't update existing reputations?

The idea was to add a text box later so that it was not limited to just -1, 0, or +1.

Quote:Rating interface does not work with Javascript disabled (seems like a bug as opposed to intentional behaviour). Interestingly, turning off the AJAX setting makes it fail to work as well, regardless of whether Javascript is enabled or not

Quote:How to use the rating interface isn't obvious in any way - you've got an icon, then a "x [number]"; not only is this difficult to see that it's meant to be clicked on, the fact that the icon and number links and their respective functions isn't obvious. You can toggle the rating you've chosen by clicking on the icon, although this isn't exactly obvious either (there's no visual indication (not even in a tooltip) on whether clicking the icon will add to the reputation or remove from it).

I see what you mean there, it is everything but obvious. I assumed that since it was obvious to me (the one coding it), it would be to anyone.

Quote:Note that, however, you can't change your rating by clicking on another icon - you need to 'unrate' the post by clicking on the rating you originally specified (and this can be difficult to tell since there's no visual indication) and then click the other rating.

Nice idea, never though of such a thing.

Quote:set_url() isn't being called. Though that doesn't seem to be the only issue.

Seems to be working fine in my latest build, thanks!

Seems like this is full of bugs, I also noticed that moderation actions weren't being taken into consideration while coding it so some data may be not deleted when it should/is no necessary anymore.

***ZiNgA BuRgA*** · 12-01-2012 09:26 AM

(11-30-2012 07:44 PM)Sama34 Wrote: Would using int (not int(n)) be any better?

I don't think there's really a difference.

(11-30-2012 07:44 PM)Sama34 Wrote: What do you suggest? I avoided to add a new column to the posts table since I hate doing so. If I understood correctly.

I don't really have one - it's still a concern nonetheless.
You could cache the counts since that's all that you really need to display, but if you don't want to be modding the posts table, I guess that's out of the question (I suppose you could skirt around it by creating another table, but I wouldn't consider it any better than modding the posts table).

(11-30-2012 07:44 PM)Sama34 Wrote: Do you think the following is correct?

SQL Code

	$db->write_query("CREATE TABLE `".TABLE_PREFIX."ougc_customrep_log` (
			`lid` int UNSIGNED NOT NULL AUTO_INCREMENT,
			`pid` int NOT NULL DEFAULT '0',
			`uid` int NOT NULL DEFAULT '0',
			`rid` int NOT NULL DEFAULT '0',
			`dateline` int(10) NOT NULL DEFAULT '0',
			PRIMARY KEY (`lid`),
			UNIQUE KEY piduid (pid,uid),
			CREATE INDEX pidrid (pid,rid)
		) ENGINE=MyISAM{$collation};"
	);

///---

	$db->write_query('ALTER TABLE '.TABLE_PREFIX.'ougc_customrep_log ADD UNIQUE KEY piduid (pid,uid)');
	if(!$db->index_exists('ougc_customrep_log', 'pidrid'))
	{
		$db->write_query('CREATE INDEX pidrid ON '.TABLE_PREFIX.'ougc_customrep_log (pid,rid)');
	}

Keys and indexes are the same thing.
In the first, replace "CREATE INDEX " with "KEY " (or "INDEX " if you prefer).

(11-30-2012 07:44 PM)Sama34 Wrote: I assumed that since it was obvious to me (the one coding it), it would be to anyone.

A common problem in user interface design you'll find. The developer will always know where everything is and how it's supposed to work Tongue

Sama34 · 12-01-2012 05:28 PM

Quote:I don't think there's really a difference.

Would you recommend anything over the use of bigint then?

Quote:You could cache the counts since that's all that you really need to display, but if you don't want to be modding the posts table, I guess that's out of the question (I suppose you could skirt around it by creating another table, but I wouldn't consider it any better than modding the posts table).

I will come to it when I have the time/need to. My concern is possible posts with outdated data possibly leading to different issues.

***ZiNgA BuRgA*** · 12-01-2012 10:20 PM

(12-01-2012 05:28 PM)Sama34 Wrote: Would you recommend anything over the use of bigint then?

bigint is a 64-bit integer which is pretty pointless considering that IDs in MyBB are typically int, and most PHP installations are 32-bit.
It's honestly not really an issue - a little pointless perhaps, but not a problem.

Sama34 · 12-03-2012 04:18 PM

Thank you for your review Zinga Burga. Can we request reviews for already reviewed plugins? (Major updates at least?)

***ZiNgA BuRgA*** · 12-03-2012 04:30 PM

Sure.

Threaded Mode \| Linear Mode OUGC Custom Reputation
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 Custom Reputation Plugin Version: 1.0.1 (last updated 25th November 2012) Plugin Author: Omar G.
Author	Message