Thread Rating:
  • 0 Votes - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
OUGC Pages 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: OUGC Pages
Plugin Version: v1.0.0 (last updated 12th September 2014)
Plugin Author: Omar G.
Author Message
ZiNgA BuRgA Offline
Fag
*******
Posts: 3,357
Joined: Jan 2008
Post: #1
OUGC Pages Review
This "review" was requested by Sama34.

Rather late with this one, but I managed to get around it.  I haven't been doing PHP coding much lately, so my memory is a bit weak, meaning that I've likely glossed over much more than I usually would do.

Anyway, the plugin generally seems to be fine.  Some suggestions I'd make:
  • It's probably not clear what one needs to exactly do to integrate Pages into the forum.  By default, no links are added.
  • PHP Code:
    $is_page = $mybb->get_input('page') && !empty($mybb->cache->cache['ougc_pages']['pages'][$mybb->get_input('page')]);

    Variable not used anywhere, consider removing.

  • PHP Code:
    $query = $db->simple_select('ougc_pages', 'pid, url', 'visible=\'1\''.(!empty($update['categories']) ? ' AND cid NOT IN (\''.implode('\', \'', $update['categories']).'\')' : ''), array('order_by' => 'disporder'));

    The attempt to filter out categories doesn't make a lot of sense, since the rest of the code seems to assume that all pages are stored in the cache.
    The filter doesn't do anything anyway - I assume that the implode part was supposed to be

    PHP Code:
    implode('\', \'', array_keys($update['categories']))

    (though this obviously breaks things)
    My guess is that it's legacy code that forgot to be removed.

  • The categories cache (MyBB level cache, not the cache that is tied to the class) doesn't appear to be used.  Perhaps if it was keyed by URL, like the pages cache, it could be more useful.
  • My mistake with this one:

    PHP Code:
    		// convert \xA0 to spaces (reverse  )
    		$message = trim(preg_replace(array('~ {2,}~', "~\n{2,}~"), array(' ', "\n"), strtr($message, array("\xA0" => ' ', "\r" => '', "\t" => ' '))));

    "\xA0" should be utf8_encode("\xA0")
    (though I can't actually see this code being used anywhere in the plugin - my guess is that it's just a generic include across all OUGC plugins?)

  • I personally don't agree with stuffing all the code into the ougc_pages.php file.  For example, the ougc_pages_show function is only ever called from pages.php, so it may as well be placed there so that it doesn't need to be loaded on every page load.  I suppose this is more of a design decision though.

My Blog
(This post was last modified: 11-23-2014 07:25 PM by ZiNgA BuRgA.)
11-23-2014 07:23 PM
Find all posts by this user Quote this message in a reply
nier3 Offline
Member
***
Posts: 125
Joined: Jul 2012
Post: #2
RE: OUGC Pages Review
Thank you for review! I'm looking for a plugin to easily and quickly add many custom pages, without create php page each time. I'll try it, thanks
11-30-2014 12:34 AM
Find all posts by this user Quote this message in a reply
Sama34 Offline
Senior Member
****
Posts: 490
Joined: May 2011
Post: #3
RE: OUGC Pages Review
Quote:It's probably not clear what one needs to exactly do to integrate Pages into the forum.  By default, no links are added.

There I'm once again assuming things, since I do use the Global Menu plugin by RateU to manage my own links and I expect some minimum understanding on the template system and therefore HTML I never though about this. I will probably add a feature to do this less manually.

Quote:The attempt to filter out categories doesn't make a lot of sense, since the rest of the code seems to assume that all pages are stored in the cache.

Hidden categories should be not visible nor its pages, that is why I only intend to cache visible pages within visible categories. Though yeah the implode fortunately seems to be wrong because from a quick look the query should be an 'IN' not 'NOT IN', haha..

And yeah, the categories cache is not used even when that was the plan. Unsure about the reason though.

Quote:my guess is that it's just a generic include across all OUGC plugins?

Correct. I will need to update all of my plugins that use that function at some point. All of them use the same named function...

Quote: personally don't agree with stuffing all the code into the ougc_pages.php file.  For example, the ougc_pages_show function is only ever called from pages.php, so it may as well be placed there so that it doesn't need to be loaded on every page load.

This was to return a value instead of directly printing the page to use within the portal. I removed this functionality before releasing because of lack of time for testing it. Commented code can be seen within all the script.

Thanks for your review Zinga Burga.

Support PM's will be ignored. Yipi
Plugins: Announcement Bars - Custom Reputation - Mark PM As Unread
11-30-2014 07:56 PM
Visit this user's website Find all posts by this user Quote this message in a reply
ZiNgA BuRgA Offline
Fag
*******
Posts: 3,357
Joined: Jan 2008
Post: #4
RE: OUGC Pages Review
Ah I see.
Thanks for the reply.

My Blog
12-01-2014 04:24 PM
Find all posts by this user Quote this message in a reply


Forum Jump: