OUGC Better Portal News

[ZiNgA BuRgA]

This is a plugin "review" requested by Sama34.

This is an interesting plugin which gives moderators an option to make a copy of the thread (into a specified forum) when it is created (that is, the option only appears when making a new thread and won't show up on edit thread).
I personally would've probably taken a completely different approach - just simply parsing a {SPLIT} tag on the portal page would be suffice, and doesn't suffer issues with having to manage two threads / duplicate data (such as having to update two threads instead of one), but maybe you have a reason to have a copy instead.

Plugin in general does a good job, but I'm feeling a little pedantic at the moment.

• Typo (2 instances):
  
  PHP Code:

  $active_plugins = $cache->read('plugins'); if(!$pscom_plugins['active']['default_post_icon'])

  ---

• I'd recommend a mb_stripos check rather than this
  
  PHP Code:

  1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23

  $message = preg_split("#{SPLIT}#ims", $mybb->input['message']); if($modoptions['portalnews'] == 1) { if(!$message[0]) { error($lang->ougc_portalnews_newthreaderror); } else { $count = my_strlen($message[0]); $mincount = intval($mybb->settings['minmessagelength']); if($count < $mincount) { error($lang->sprintf($lang->ougc_portalnews_newthreaderror3, $mincount)); } else { $ougc_message = $mybb->input['message']; $mybb->input['message'] = $message[0].$message[1]; $GLOBALS['plugins']->add_hook("newthread_do_newthread_end", "ougc_portalnews_end"); } } }

  ---

  Slightly faster and won't fall over with multiple instances of {SPLIT} in the message, although I guess it's not a big deal.

• PHP Code:

  $ougc_message = $mybb->input['message'];

  ---

  Since only $message[0] is used later on, why not just pass that?  Less processing code, and less memory usage.

• I suspect the following is incorrect
  
  PHP Code:

  1 2 3 4 5 6 7 8 9 10 11 12 13

  //Cache our template. if(my_strpos($_SERVER['PHP_SELF'], 'newthread.php')) { global $templatelist; if(isset($templatelist)) { $templatelist .= ', newreply_modoptions'; } else { $templatelist = 'newreply_modoptions'; } }

  ---

  as newreply_modoptions is already cached on newthread.php, whilst the added template ougc_portalnews_input isn't.  Although not particularly important, I might suggest using THIS_SCRIPT constant rather than PHP_SELF, as it's a bit more reliable.

• Querying the forum is unnecessary here, as you can simply check with $thread['fid']
  
  PHP Code:

  $thread = get_thread(intval($mybb->input['tid'])); $forum = get_forum($thread['fid']); if($forum['fid'] == intval($mybb->settings['ougc_portalnews_newsforum']))

  ---

• Replacement 'read more' text is in bbcode, so may not work if you have that disabled (ie you prefer HTML)
  

[Sama34]

The reason to use this way was so that moderators could copy any thread to the portal forum, otherwise everybody threads will appear in portal, even if they were no mods.

Thanks for the review, I edited many parts of the plugin and I´m reading about that function you mentioned.

Thanks again!

[ZiNgA BuRgA]

(04-11-2012 01:51 PM)Sama34 Wrote:  The reason to use this way was so that moderators could copy any thread to the portal forum, otherwise everybody threads will appear in portal, even if they were no mods.

In which case, I'd modify the query that the portal uses, to pull marked threads instead.
Generally should try not to duplicate data when possible (although, again, there may be a purpose).

[Sama34]

Yes, maybe I will try to find a better way using portal hooks and drop this plugin.