Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

[ticket/10857] Adding information about items requiring a moderators attention #780

Closed
wants to merge 5 commits into from

8 participants

@michaelcullum michaelcullum [ticket/10857] Adding information about items requiring a moderators …
…attention

With thanks to RMcGirr83 for his Moderator Needed MOD.

PHPBB3-10857
56474da
phpBB/includes/functions_display.php
((84 lines not shown))
+ $total_unapproved_topics = sprintf($l_unapproved_topics_count, $unapproved_topics_count);
+ $l_unapproved_posts_count = $unapproved_posts_count ? (($unapproved_posts_count == 1) ? $user->lang['MODERATOR_NEEDED_APPROVE_POST'] : $user->lang['MODERATOR_NEEDED_APPROVE_POSTS']) : '';
+ $total_unapproved_posts = sprintf($l_unapproved_posts_count, $unapproved_posts_count);
+
+ // And export variables to the template
+ $template->assign_vars(array(
+ 'TOTAL_MODERATOR_REPORTS' => $total_reported_posts,
+ 'TOTAL_MODERATOR_POSTS' => $total_unapproved_posts,
+ 'TOTAL_MODERATOR_TOPICS' => $total_unapproved_topics,
+
+ 'U_MODERATOR_REPORTS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=reports&mode=reports', true, $user->session_id),
+ 'U_MODERATOR_APPROVE_POSTS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&mode=unapproved_posts', true, $user->session_id),
+ 'U_MODERATOR_APPROVE_TOPICS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&mode=unapproved_topics', true, $user->session_id),
+ ));
+ }
+ }

Indentation

@michaelcullum Owner

Fixed. It shouldn't have been there anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
phpBB/language/en/common.php
@@ -358,6 +358,14 @@
'MINUTES' => 'Minutes',
'MODERATE' => 'Moderate',
'MODERATOR' => 'Moderator',
+ 'MODERATOR_REPORTED_POST' => '<strong>%d</strong> Post is reported',
+ 'MODERATOR_REPORTED_POSTS' => '<strong>%d</strong> Posts are reported',
+ 'MODERATOR_APPROVE_POST' => '<strong>%d</strong> Post needs approval',
+ 'MODERATOR_APPROVE_POSTS' => '<strong>%d</strong> Posts need approval',
+ 'MODERATOR_APPROVE_TOPIC' => '<strong>%d</strong> Topic needs approval',
+ 'MODERATOR_APPROVE_TOPICS' => '<strong>%d</strong> Topics need approval',

Post and topic are not proper nouns, so they should not be capitalized.

Also, I think better language would be like:
"%d reported posts" (same with topics).
and
"%d posts awaiting approval"

@michaelcullum Owner

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@imkingdavid imkingdavid commented on the diff
phpBB/includes/functions_display.php
((107 lines not shown))
+ {
+ $sql = 'SELECT COUNT(msg_id) AS pms_count
+ FROM ' . PRIVMSGS_TABLE . '
+ WHERE message_reported = 1';
+ $result = $db->sql_query($sql);
+ $reported_pms = (int) $db->sql_fetchfield('pms_count');
+ $db->sql_freeresult($result);
+
+ // Cache for a few minutes to optimize. Might add to topics/posts later
+ $cache->put('_reported_pms', $reported_pms, 300);
+ }
+
+ // Dump to the template again
+ if ($reported_pms)
+ {
+ $l_reported_pms_count = $reported_pms ? (($reported_pms == 1) ? $user->lang['MODERATOR_NEEDED_REPORTED_PM'] : $user->lang['MODERATOR_NEEDED_REPORTED_PMS']) : '';

Use the lang() method rather than the lang array. Applies to any other similar uses of $user->lang[]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@imkingdavid imkingdavid commented on the diff
phpBB/language/en/common.php
@@ -358,6 +358,14 @@
'MINUTES' => 'Minutes',
'MODERATE' => 'Moderate',
'MODERATOR' => 'Moderator',
+ 'MODERATOR_REPORTED_POST' => '<strong>%d</strong> reported post',
+ 'MODERATOR_REPORTED_POSTS' => '<strong>%d</strong> reported posts',
+ 'MODERATOR_APPROVE_POST' => '<strong>%d</strong> post awaiting approval',
+ 'MODERATOR_APPROVE_POSTS' => '<strong>%d</strong> posts awaiting approve',
+ 'MODERATOR_APPROVE_TOPIC' => '<strong>%d</strong> topic awaiting approval',
+ 'MODERATOR_APPROVE_TOPICS' => '<strong>%d</strong> topics awaiting approval',
+ 'MODERATOR_REPORTED_PM' => '<strong>%d</strong> PM is reported',
+ 'MODERATOR_REPORTED_PMS' => '<strong>%d</strong> PMs are reported',

probably "%d reported PMs" like you did with posts and topics would be better for consistency.

+1

I think the new system for plural form should be used instead.

@michaelcullum Owner

@brunoais That has already been said.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@callumacrae callumacrae commented on the diff
phpBB/language/en/common.php
@@ -358,6 +358,14 @@
'MINUTES' => 'Minutes',
'MODERATE' => 'Moderate',
'MODERATOR' => 'Moderator',
+ 'MODERATOR_REPORTED_POST' => '<strong>%d</strong> reported post',
+ 'MODERATOR_REPORTED_POSTS' => '<strong>%d</strong> reported posts',
+ 'MODERATOR_APPROVE_POST' => '<strong>%d</strong> post awaiting approval',
+ 'MODERATOR_APPROVE_POSTS' => '<strong>%d</strong> posts awaiting approve',

approval

@michaelcullum Owner

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@callumacrae callumacrae commented on the diff
phpBB/language/en/common.php
@@ -358,6 +358,14 @@
'MINUTES' => 'Minutes',
'MODERATE' => 'Moderate',
'MODERATOR' => 'Moderator',
+ 'MODERATOR_REPORTED_POST' => '<strong>%d</strong> reported post',

Is this every going to be anything but 1?

@michaelcullum Owner

No, hence the point of having post and posts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@imkingdavid imkingdavid commented on the diff
phpBB/includes/functions_display.php
((71 lines not shown))
+ if (!$row['post_approved'])
+ {
+ $unapproved_posts_count++;
+ }
+ }
+ $db->sql_freeresult($result);
+
+ // Lets use this data
+ if ($reported_posts_count || $unapproved_posts_count || $unapproved_topics_count)
+ {
+ $l_reported_posts_count = $reported_posts_count ? (($reported_posts_count == 1) ? $user->lang['MODERATOR_NEEDED_REPORTED_POST'] : $user->lang['MODERATOR_NEEDED_REPORTED_POSTS']) : '';
+ $total_reported_posts = sprintf($l_reported_posts_count, $reported_posts_count);
+ $l_unapproved_topics_count = $unapproved_topics_count ? (($unapproved_topics_count == 1) ? $user->lang['MODERATOR_NEEDED_APPROVE_TOPIC'] : $user->lang['MODERATOR_NEEDED_APPROVE_TOPICS']) : '';
+ $total_unapproved_topics = sprintf($l_unapproved_topics_count, $unapproved_topics_count);
+ $l_unapproved_posts_count = $unapproved_posts_count ? (($unapproved_posts_count == 1) ? $user->lang['MODERATOR_NEEDED_APPROVE_POST'] : $user->lang['MODERATOR_NEEDED_APPROVE_POSTS']) : '';
+ $total_unapproved_posts = sprintf($l_unapproved_posts_count, $unapproved_posts_count);

Use $user->lang() instead of $user->lang[] so don't have to use sprintf, and you can condense this by 3 lines if you do so, I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@p p commented on the diff
phpBB/language/en/common.php
@@ -358,6 +358,14 @@
'MINUTES' => 'Minutes',
'MODERATE' => 'Moderate',
'MODERATOR' => 'Moderator',
+ 'MODERATOR_REPORTED_POST' => '<strong>%d</strong> reported post',
+ 'MODERATOR_REPORTED_POSTS' => '<strong>%d</strong> reported posts',
+ 'MODERATOR_APPROVE_POST' => '<strong>%d</strong> post awaiting approval',
+ 'MODERATOR_APPROVE_POSTS' => '<strong>%d</strong> posts awaiting approve',
+ 'MODERATOR_APPROVE_TOPIC' => '<strong>%d</strong> topic awaiting approval',
+ 'MODERATOR_APPROVE_TOPICS' => '<strong>%d</strong> topics awaiting approval',
@p
p added a note

Plurals need to be done via the 3.1 plural system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@p p commented on the diff
phpBB/includes/functions_display.php
@@ -1322,3 +1322,133 @@ function get_user_avatar($avatar, $avatar_type, $avatar_width, $avatar_height, $
$avatar_img .= $avatar;
return '<img src="' . (str_replace(' ', '%20', $avatar_img)) . '" width="' . $avatar_width . '" height="' . $avatar_height . '" alt="' . ((!empty($user->lang[$alt])) ? $user->lang[$alt] : $alt) . '" />';
}
+
+/**
+* Fetches the number of items which need a moderators attention
+*/
+function mcp_quick_info()
@p
p added a note

Needs a phpbb prefix.

@brunoais
brunoais added a note

Found the problem.
This function is never called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@p
p commented

You are adding 3 queries for each page view for moderators/admins? Have you checked that all of them use indexes we have?

phpBB/includes/functions_display.php
((86 lines not shown))
+ $total_unapproved_posts = sprintf($l_unapproved_posts_count, $unapproved_posts_count);
+
+ // And export variables to the template
+ $template->assign_vars(array(
+ 'TOTAL_MODERATOR_REPORTS' => $total_reported_posts,
+ 'TOTAL_MODERATOR_POSTS' => $total_unapproved_posts,
+ 'TOTAL_MODERATOR_TOPICS' => $total_unapproved_topics,
+
+ 'U_MODERATOR_REPORTS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=reports&amp;mode=reports', true, $user->session_id),
+ 'U_MODERATOR_APPROVE_POSTS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&amp;mode=unapproved_posts', true, $user->session_id),
+ 'U_MODERATOR_APPROVE_TOPICS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&amp;mode=unapproved_topics', true, $user->session_id),
+ ));
+ }
+
+ // Don't forget PMs
+ if ($auth->acl_getf_global('m_'))
@p
p added a note

Indentation looks wrong here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@p p commented on the diff
phpBB/includes/functions_display.php
((66 lines not shown))
+ if ($row['post_reported'])
+ {
+ $reported_posts_count++;
+ }
+ // count the unapproved posts
+ if (!$row['post_approved'])
+ {
+ $unapproved_posts_count++;
+ }
+ }
+ $db->sql_freeresult($result);
+
+ // Lets use this data
+ if ($reported_posts_count || $unapproved_posts_count || $unapproved_topics_count)
+ {
+ $l_reported_posts_count = $reported_posts_count ? (($reported_posts_count == 1) ? $user->lang['MODERATOR_NEEDED_REPORTED_POST'] : $user->lang['MODERATOR_NEEDED_REPORTED_POSTS']) : '';
@p
p added a note

The plurals logic here should be replaced with the 3.1 plurals stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bantu bantu commented on the diff
phpBB/includes/functions_display.php
@@ -1322,3 +1322,133 @@ function get_user_avatar($avatar, $avatar_type, $avatar_width, $avatar_height, $
$avatar_img .= $avatar;
return '<img src="' . (str_replace(' ', '%20', $avatar_img)) . '" width="' . $avatar_width . '" height="' . $avatar_height . '" alt="' . ((!empty($user->lang[$alt])) ? $user->lang[$alt] : $alt) . '" />';
}
+
+/**
+* Fetches the number of items which need a moderators attention
+*/
+function mcp_quick_info()
+{
+ global $auth, $cache, $db, $template, $user, $phpEx, $phpbb_root_path;
+
+ $allow = false;
+
+ // Check for approve and report permissions
+ if ($auth->acl_getf_global('m_approve') && $auth->acl_getf_global('m_report'))
+ {
+ $sql_where = ' WHERE (' . $db->sql_in_set('forum_id', get_forum_list('m_approve')) . ' or ' . $db->sql_in_set('forum_id', get_forum_list('m_report')) . ') AND (post_reported = 1 or post_approved = 0)';
@bantu Owner
bantu added a note

"or" should be upper case.

@bantu Owner
bantu added a note

Query should be broken into multiple lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bantu bantu commented on the diff
phpBB/includes/functions_display.php
((5 lines not shown))
+/**
+* Fetches the number of items which need a moderators attention
+*/
+function mcp_quick_info()
+{
+ global $auth, $cache, $db, $template, $user, $phpEx, $phpbb_root_path;
+
+ $allow = false;
+
+ // Check for approve and report permissions
+ if ($auth->acl_getf_global('m_approve') && $auth->acl_getf_global('m_report'))
+ {
+ $sql_where = ' WHERE (' . $db->sql_in_set('forum_id', get_forum_list('m_approve')) . ' or ' . $db->sql_in_set('forum_id', get_forum_list('m_report')) . ') AND (post_reported = 1 or post_approved = 0)';
+ $allow = true;
+ }
+ elseif ($auth->acl_getf_global('m_approve'))
@bantu Owner
bantu added a note

Space between else and if

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bantu bantu commented on the diff
phpBB/includes/functions_display.php
((36 lines not shown))
+ if ($auth->acl_getf_global('m_approve'))
+ {
+ $sql = 'SELECT topic_first_post_id
+ FROM ' . TOPICS_TABLE . '
+ WHERE ' . $db->sql_in_set('forum_id', get_forum_list('m_approve')) . ' AND topic_approved = 0';
+ $result = $db->sql_query($sql);
+
+ $unapproved_topics_array = array();
+ while ($row = $db->sql_fetchrow($result))
+ {
+ $unapproved_topics_array[] = (int) $row['topic_first_post_id'];
+ $unapproved_topics_count++;
+ }
+ $db->sql_freeresult($result);
+
+ if(sizeof($unapproved_topics_array))
@bantu Owner
bantu added a note

Space after if.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bantu bantu commented on the diff
phpBB/includes/functions_display.php
((32 lines not shown))
+ {
+ $reported_posts_count = $unapproved_posts_count = $unapproved_topics_count = $reported_pms = 0;
+
+ // Topic Approval
+ if ($auth->acl_getf_global('m_approve'))
+ {
+ $sql = 'SELECT topic_first_post_id
+ FROM ' . TOPICS_TABLE . '
+ WHERE ' . $db->sql_in_set('forum_id', get_forum_list('m_approve')) . ' AND topic_approved = 0';
+ $result = $db->sql_query($sql);
+
+ $unapproved_topics_array = array();
+ while ($row = $db->sql_fetchrow($result))
+ {
+ $unapproved_topics_array[] = (int) $row['topic_first_post_id'];
+ $unapproved_topics_count++;
@bantu Owner
bantu added a note

Counting seems unnecessary here as the array seems to count for you. You can use $unapproved_topics_count = sizeof($unapproved_topics_array); after the loop.

@bantu Owner
bantu added a note

The array data structure in php carries a counter. sizeof() returns that counter. when you add an element using [], the counter is incremented. So php already does the counting work for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bantu bantu commented on the diff
phpBB/includes/functions_display.php
@@ -1322,3 +1322,133 @@ function get_user_avatar($avatar, $avatar_type, $avatar_width, $avatar_height, $
$avatar_img .= $avatar;
return '<img src="' . (str_replace(' ', '%20', $avatar_img)) . '" width="' . $avatar_width . '" height="' . $avatar_height . '" alt="' . ((!empty($user->lang[$alt])) ? $user->lang[$alt] : $alt) . '" />';
}
+
+/**
+* Fetches the number of items which need a moderators attention
+*/
+function mcp_quick_info()
+{
+ global $auth, $cache, $db, $template, $user, $phpEx, $phpbb_root_path;
+
+ $allow = false;
+
+ // Check for approve and report permissions
+ if ($auth->acl_getf_global('m_approve') && $auth->acl_getf_global('m_report'))
@bantu Owner
bantu added a note

This seems to be unnecessarily complex. it should be possible to build a query using the two individual cases m_approve and m_report with two independent ifs below

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bantu bantu commented on the diff
phpBB/includes/functions_display.php
((45 lines not shown))
+ {
+ $unapproved_topics_array[] = (int) $row['topic_first_post_id'];
+ $unapproved_topics_count++;
+ }
+ $db->sql_freeresult($result);
+
+ if(sizeof($unapproved_topics_array))
+ {
+ $sql_where .= ' AND ' . $db->sql_in_set('post_id', $unapproved_topics_array, true);
+ }
+ }
+
+ // Reported Posts and Approved Posts
+ $sql = 'SELECT post_reported, post_approved
+ FROM ' . POSTS_TABLE .
+ $sql_where . ' OR (forum_id = 0 and post_reported = 1)';
@bantu Owner
bantu added a note

Can forum_id = 0 on develop?

@bantu Owner
bantu added a note

Upper case AND

@bantu Owner
bantu added a note

Has to be handled by the convertor.

@nickvergessen Collaborator

forum_id can not be 0 anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bantu bantu commented on the diff
phpBB/includes/functions_display.php
((48 lines not shown))
+ }
+ $db->sql_freeresult($result);
+
+ if(sizeof($unapproved_topics_array))
+ {
+ $sql_where .= ' AND ' . $db->sql_in_set('post_id', $unapproved_topics_array, true);
+ }
+ }
+
+ // Reported Posts and Approved Posts
+ $sql = 'SELECT post_reported, post_approved
+ FROM ' . POSTS_TABLE .
+ $sql_where . ' OR (forum_id = 0 and post_reported = 1)';
+ $result = $db->sql_query($sql);
+
+ while ($row = $db->sql_fetchrow($result))
@bantu Owner
bantu added a note

Wouldn't it make more sense to do this logic in SQL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bantu bantu commented on the diff
phpBB/includes/functions_display.php
((91 lines not shown))
+ 'TOTAL_MODERATOR_POSTS' => $total_unapproved_posts,
+ 'TOTAL_MODERATOR_TOPICS' => $total_unapproved_topics,
+
+ 'U_MODERATOR_REPORTS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=reports&amp;mode=reports', true, $user->session_id),
+ 'U_MODERATOR_APPROVE_POSTS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&amp;mode=unapproved_posts', true, $user->session_id),
+ 'U_MODERATOR_APPROVE_TOPICS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&amp;mode=unapproved_topics', true, $user->session_id),
+ ));
+ }
+
+ // Don't forget PMs
+ if ($auth->acl_getf_global('m_'))
+ {
+ $reported_pms = 0;
+
+ // Lets have a cache
+ if (($reported_pms = $cache->get('_reported_pms')) === false)
@bantu Owner
bantu added a note

Maybe this should be in cache.php or at least its own function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nickvergessen nickvergessen commented on the diff
phpBB/styles/prosilver/template/index_body.html
@@ -1,7 +1,7 @@
<!-- INCLUDE overall_header.html -->
<p class="{S_CONTENT_FLOW_END}<!-- IF S_USER_LOGGED_IN --> rightside<!-- ENDIF -->"><!-- IF S_USER_LOGGED_IN -->{LAST_VISIT_DATE}<!-- ELSE -->{CURRENT_TIME}<!-- ENDIF --></p>
-<!-- IF U_MCP --><p>{CURRENT_TIME} <br />[&nbsp;<a href="{U_MCP}">{L_MCP}</a>&nbsp;]</p><!-- ELSEIF S_USER_LOGGED_IN --><p>{CURRENT_TIME}</p><!-- ENDIF -->
+<!-- IF U_MCP --><p>{CURRENT_TIME} <br />[&nbsp;<a href="{U_MCP}">{L_MCP}</a><!-- IF TOTAL_MODERATOR_REPORTS --> &bull; <a href="{U_MODERATOR_REPORTS}">{TOTAL_MODERATOR_REPORTS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_POSTS --> &bull; <a href="{U_MODERATOR_APPROVE_POSTS}">{TOTAL_MODERATOR_POSTS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_TOPICS --> &bull; <a href="{U_MODERATOR_APPROVE_TOPICS}">{TOTAL_MODERATOR_TOPICS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_PMS --> &bull; <a href="{U_MODERATOR_PMS}">{TOTAL_MODERATOR_PMS}</a><!-- ENDIF -->&nbsp;]</p><!-- ELSEIF S_USER_LOGGED_IN --><p>{CURRENT_TIME}</p><!-- ENDIF -->
@nickvergessen Collaborator

I'd prefer | over the bull here.
That's the way we use do it inside of [ ]:
Example in profile:
Warnings:
0 [ View user notes | Warn user ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@brunoais

Does not work. It gives unexpected $end.
I still don't know where it is failing. norepad++ finds an if that is closing at near bottom.
Indentation as is makes the job harder.
I'm just unable to make this work.

@michaelcullum

Ah this is fixed locally but I forgot to push.

@michaelcullum

There you are.

@brunoais What do you mean indentation as-is? It looks fine to me but if you notice something wrong with it please add an in-line comment....

@travisbot

This pull request fails (merged 83057f1 into 2a92fee).

@brunoais

Where should this appear? Near: [ Moderator Control Panel ] ex: in index.php, right?
It's not there :(

@bantu
Owner

Since this is for develop, maybe the implementation should be more object oriented and "plugin-aware". Maybe it could be changed so you can just drop in files/classes to add new items requiring moderator attention. Maybe also with the ability to display some information on the index page like http://www.phpbb.com/customise/db/mod/mcp_info_on_index The exising code could be decomposed into two plugins: a) reported posts b) posts in moderation queue

@callumacrae callumacrae commented on the diff
phpBB/includes/functions_display.php
((20 lines not shown))
+ elseif ($auth->acl_getf_global('m_approve'))
+ {
+ $sql_where = ' WHERE ' . $db->sql_in_set('forum_id', get_forum_list('m_approve')) . ' AND post_approved = 0';
+ $allow = true;
+ }
+ elseif ($auth->acl_getf_global('m_report'))
+ {
+ $sql_where = ' WHERE ' . $db->sql_in_set('forum_id', get_forum_list('m_report')) . ' AND post_reported = 1';
+ $allow = true;
+ }
+
+ if ($allow)
+ {
+ $reported_posts_count = $unapproved_posts_count = $unapproved_topics_count = $reported_pms = 0;
+
+ // Topic Approval

Not that it is hugely significant, but shouldn't this be "Topic approval"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@callumacrae callumacrae commented on the diff
phpBB/includes/functions_display.php
((25 lines not shown))
+ elseif ($auth->acl_getf_global('m_report'))
+ {
+ $sql_where = ' WHERE ' . $db->sql_in_set('forum_id', get_forum_list('m_report')) . ' AND post_reported = 1';
+ $allow = true;
+ }
+
+ if ($allow)
+ {
+ $reported_posts_count = $unapproved_posts_count = $unapproved_topics_count = $reported_pms = 0;
+
+ // Topic Approval
+ if ($auth->acl_getf_global('m_approve'))
+ {
+ $sql = 'SELECT topic_first_post_id
+ FROM ' . TOPICS_TABLE . '
+ WHERE ' . $db->sql_in_set('forum_id', get_forum_list('m_approve')) . ' AND topic_approved = 0';

Spacing is weird here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@callumacrae callumacrae commented on the diff
phpBB/includes/functions_display.php
((109 lines not shown))
+ $sql = 'SELECT COUNT(msg_id) AS pms_count
+ FROM ' . PRIVMSGS_TABLE . '
+ WHERE message_reported = 1';
+ $result = $db->sql_query($sql);
+ $reported_pms = (int) $db->sql_fetchfield('pms_count');
+ $db->sql_freeresult($result);
+
+ // Cache for a few minutes to optimize. Might add to topics/posts later
+ $cache->put('_reported_pms', $reported_pms, 300);
+ }
+
+ // Dump to the template again
+ if ($reported_pms)
+ {
+ $l_reported_pms_count = $reported_pms ? (($reported_pms == 1) ? $user->lang['MODERATOR_NEEDED_REPORTED_PM'] : $user->lang['MODERATOR_NEEDED_REPORTED_PMS']) : '';
+ $total_reported_pms = sprintf($l_reported_pms_count, $reported_pms);

Random extra spaces after =

Not completely random, but padding wrongly applied. IMO, if there's a search for a padding, then it should be before the = and not after.

@michaelcullum Owner

@brunoais It was a mistake, not on purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@imkingdavid

New PR: #809

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 30, 2012
  1. @michaelcullum

    [ticket/10857] Adding information about items requiring a moderators …

    michaelcullum authored
    …attention
    
    With thanks to RMcGirr83 for his Moderator Needed MOD.
    
    PHPBB3-10857
  2. @michaelcullum
  3. @michaelcullum
  4. @michaelcullum
Commits on May 7, 2012
  1. @michaelcullum

    [ticket/10857] Fixing a php fatal error

    michaelcullum authored
    PHPBB3-10857
This page is out of date. Refresh to see the latest.
View
130 phpBB/includes/functions_display.php
@@ -1322,3 +1322,133 @@ function get_user_avatar($avatar, $avatar_type, $avatar_width, $avatar_height, $
$avatar_img .= $avatar;
return '<img src="' . (str_replace(' ', '%20', $avatar_img)) . '" width="' . $avatar_width . '" height="' . $avatar_height . '" alt="' . ((!empty($user->lang[$alt])) ? $user->lang[$alt] : $alt) . '" />';
}
+
+/**
+* Fetches the number of items which need a moderators attention
+*/
+function mcp_quick_info()
@p
p added a note

Needs a phpbb prefix.

@brunoais
brunoais added a note

Found the problem.
This function is never called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+{
+ global $auth, $cache, $db, $template, $user, $phpEx, $phpbb_root_path;
+
+ $allow = false;
+
+ // Check for approve and report permissions
+ if ($auth->acl_getf_global('m_approve') && $auth->acl_getf_global('m_report'))
@bantu Owner
bantu added a note

This seems to be unnecessarily complex. it should be possible to build a query using the two individual cases m_approve and m_report with two independent ifs below

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ $sql_where = ' WHERE (' . $db->sql_in_set('forum_id', get_forum_list('m_approve')) . ' or ' . $db->sql_in_set('forum_id', get_forum_list('m_report')) . ') AND (post_reported = 1 or post_approved = 0)';
@bantu Owner
bantu added a note

"or" should be upper case.

@bantu Owner
bantu added a note

Query should be broken into multiple lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $allow = true;
+ }
+ elseif ($auth->acl_getf_global('m_approve'))
@bantu Owner
bantu added a note

Space between else and if

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ $sql_where = ' WHERE ' . $db->sql_in_set('forum_id', get_forum_list('m_approve')) . ' AND post_approved = 0';
+ $allow = true;
+ }
+ elseif ($auth->acl_getf_global('m_report'))
+ {
+ $sql_where = ' WHERE ' . $db->sql_in_set('forum_id', get_forum_list('m_report')) . ' AND post_reported = 1';
+ $allow = true;
+ }
+
+ if ($allow)
+ {
+ $reported_posts_count = $unapproved_posts_count = $unapproved_topics_count = $reported_pms = 0;
+
+ // Topic Approval

Not that it is hugely significant, but shouldn't this be "Topic approval"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if ($auth->acl_getf_global('m_approve'))
+ {
+ $sql = 'SELECT topic_first_post_id
+ FROM ' . TOPICS_TABLE . '
+ WHERE ' . $db->sql_in_set('forum_id', get_forum_list('m_approve')) . ' AND topic_approved = 0';

Spacing is weird here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $result = $db->sql_query($sql);
+
+ $unapproved_topics_array = array();
+ while ($row = $db->sql_fetchrow($result))
+ {
+ $unapproved_topics_array[] = (int) $row['topic_first_post_id'];
+ $unapproved_topics_count++;
@bantu Owner
bantu added a note

Counting seems unnecessary here as the array seems to count for you. You can use $unapproved_topics_count = sizeof($unapproved_topics_array); after the loop.

@bantu Owner
bantu added a note

The array data structure in php carries a counter. sizeof() returns that counter. when you add an element using [], the counter is incremented. So php already does the counting work for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+ $db->sql_freeresult($result);
+
+ if(sizeof($unapproved_topics_array))
@bantu Owner
bantu added a note

Space after if.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ $sql_where .= ' AND ' . $db->sql_in_set('post_id', $unapproved_topics_array, true);
+ }
+ }
+
+ // Reported Posts and Approved Posts
+ $sql = 'SELECT post_reported, post_approved
+ FROM ' . POSTS_TABLE .
+ $sql_where . ' OR (forum_id = 0 and post_reported = 1)';
@bantu Owner
bantu added a note

Can forum_id = 0 on develop?

@bantu Owner
bantu added a note

Upper case AND

@bantu Owner
bantu added a note

Has to be handled by the convertor.

@nickvergessen Collaborator

forum_id can not be 0 anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $result = $db->sql_query($sql);
+
+ while ($row = $db->sql_fetchrow($result))
@bantu Owner
bantu added a note

Wouldn't it make more sense to do this logic in SQL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ // count the reported posts
+ if ($row['post_reported'])
+ {
+ $reported_posts_count++;
+ }
+ // count the unapproved posts
+ if (!$row['post_approved'])
+ {
+ $unapproved_posts_count++;
+ }
+ }
+ $db->sql_freeresult($result);
+
+ // Lets use this data
+ if ($reported_posts_count || $unapproved_posts_count || $unapproved_topics_count)
+ {
+ $l_reported_posts_count = $reported_posts_count ? (($reported_posts_count == 1) ? $user->lang['MODERATOR_NEEDED_REPORTED_POST'] : $user->lang['MODERATOR_NEEDED_REPORTED_POSTS']) : '';
@p
p added a note

The plurals logic here should be replaced with the 3.1 plurals stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $total_reported_posts = sprintf($l_reported_posts_count, $reported_posts_count);
+ $l_unapproved_topics_count = $unapproved_topics_count ? (($unapproved_topics_count == 1) ? $user->lang['MODERATOR_NEEDED_APPROVE_TOPIC'] : $user->lang['MODERATOR_NEEDED_APPROVE_TOPICS']) : '';
+ $total_unapproved_topics = sprintf($l_unapproved_topics_count, $unapproved_topics_count);
+ $l_unapproved_posts_count = $unapproved_posts_count ? (($unapproved_posts_count == 1) ? $user->lang['MODERATOR_NEEDED_APPROVE_POST'] : $user->lang['MODERATOR_NEEDED_APPROVE_POSTS']) : '';
+ $total_unapproved_posts = sprintf($l_unapproved_posts_count, $unapproved_posts_count);

Use $user->lang() instead of $user->lang[] so don't have to use sprintf, and you can condense this by 3 lines if you do so, I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ // And export variables to the template
+ $template->assign_vars(array(
+ 'TOTAL_MODERATOR_REPORTS' => $total_reported_posts,
+ 'TOTAL_MODERATOR_POSTS' => $total_unapproved_posts,
+ 'TOTAL_MODERATOR_TOPICS' => $total_unapproved_topics,
+
+ 'U_MODERATOR_REPORTS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=reports&amp;mode=reports', true, $user->session_id),
+ 'U_MODERATOR_APPROVE_POSTS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&amp;mode=unapproved_posts', true, $user->session_id),
+ 'U_MODERATOR_APPROVE_TOPICS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&amp;mode=unapproved_topics', true, $user->session_id),
+ ));
+ }
+ }
+
+ // Don't forget PMs
+ if ($auth->acl_getf_global('m_'))
+ {
+ $reported_pms = 0;
+
+ // Lets have a cache
+ if (($reported_pms = $cache->get('_reported_pms')) === false)
@bantu Owner
bantu added a note

Maybe this should be in cache.php or at least its own function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ $sql = 'SELECT COUNT(msg_id) AS pms_count
+ FROM ' . PRIVMSGS_TABLE . '
+ WHERE message_reported = 1';
+ $result = $db->sql_query($sql);
+ $reported_pms = (int) $db->sql_fetchfield('pms_count');
+ $db->sql_freeresult($result);
+
+ // Cache for a few minutes to optimize. Might add to topics/posts later
+ $cache->put('_reported_pms', $reported_pms, 300);
+ }
+
+ // Dump to the template again
+ if ($reported_pms)
+ {
+ $l_reported_pms_count = $reported_pms ? (($reported_pms == 1) ? $user->lang['MODERATOR_NEEDED_REPORTED_PM'] : $user->lang['MODERATOR_NEEDED_REPORTED_PMS']) : '';

Use the lang() method rather than the lang array. Applies to any other similar uses of $user->lang[]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $total_reported_pms = sprintf($l_reported_pms_count, $reported_pms);

Random extra spaces after =

Not completely random, but padding wrongly applied. IMO, if there's a search for a padding, then it should be before the = and not after.

@michaelcullum Owner

@brunoais It was a mistake, not on purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ $template->assign_vars(array(
+ 'TOTAL_MODERATOR_PMS' => $total_reported_pms,
+ 'U_MODERATOR_PMS' => append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=pm_reports&amp;mode=pm_reports', true, $user->session_id),
+ ));
+ }
+ }
+ return;
+}
View
1  phpBB/includes/mcp/mcp_pm_reports.php
@@ -61,6 +61,7 @@ function main($id, $mode)
}
close_report($report_id_list, $mode, $action, true);
+ $cache->purge('_reported_pms');
break;
}
View
8 phpBB/language/en/common.php
@@ -358,6 +358,14 @@
'MINUTES' => 'Minutes',
'MODERATE' => 'Moderate',
'MODERATOR' => 'Moderator',
+ 'MODERATOR_REPORTED_POST' => '<strong>%d</strong> reported post',

Is this every going to be anything but 1?

@michaelcullum Owner

No, hence the point of having post and posts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ 'MODERATOR_REPORTED_POSTS' => '<strong>%d</strong> reported posts',
+ 'MODERATOR_APPROVE_POST' => '<strong>%d</strong> post awaiting approval',
+ 'MODERATOR_APPROVE_POSTS' => '<strong>%d</strong> posts awaiting approve',

approval

@michaelcullum Owner

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ 'MODERATOR_APPROVE_TOPIC' => '<strong>%d</strong> topic awaiting approval',
+ 'MODERATOR_APPROVE_TOPICS' => '<strong>%d</strong> topics awaiting approval',
@p
p added a note

Plurals need to be done via the 3.1 plural system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ 'MODERATOR_REPORTED_PM' => '<strong>%d</strong> PM is reported',
+ 'MODERATOR_REPORTED_PMS' => '<strong>%d</strong> PMs are reported',

probably "%d reported PMs" like you did with posts and topics would be better for consistency.

+1

I think the new system for plural form should be used instead.

@michaelcullum Owner

@brunoais That has already been said.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
'MODERATORS' => 'Moderators',
'MONTH' => 'Month',
'MOVE' => 'Move',
View
2  phpBB/styles/prosilver/template/index_body.html
@@ -1,7 +1,7 @@
<!-- INCLUDE overall_header.html -->
<p class="{S_CONTENT_FLOW_END}<!-- IF S_USER_LOGGED_IN --> rightside<!-- ENDIF -->"><!-- IF S_USER_LOGGED_IN -->{LAST_VISIT_DATE}<!-- ELSE -->{CURRENT_TIME}<!-- ENDIF --></p>
-<!-- IF U_MCP --><p>{CURRENT_TIME} <br />[&nbsp;<a href="{U_MCP}">{L_MCP}</a>&nbsp;]</p><!-- ELSEIF S_USER_LOGGED_IN --><p>{CURRENT_TIME}</p><!-- ENDIF -->
+<!-- IF U_MCP --><p>{CURRENT_TIME} <br />[&nbsp;<a href="{U_MCP}">{L_MCP}</a><!-- IF TOTAL_MODERATOR_REPORTS --> &bull; <a href="{U_MODERATOR_REPORTS}">{TOTAL_MODERATOR_REPORTS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_POSTS --> &bull; <a href="{U_MODERATOR_APPROVE_POSTS}">{TOTAL_MODERATOR_POSTS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_TOPICS --> &bull; <a href="{U_MODERATOR_APPROVE_TOPICS}">{TOTAL_MODERATOR_TOPICS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_PMS --> &bull; <a href="{U_MODERATOR_PMS}">{TOTAL_MODERATOR_PMS}</a><!-- ENDIF -->&nbsp;]</p><!-- ELSEIF S_USER_LOGGED_IN --><p>{CURRENT_TIME}</p><!-- ENDIF -->
@nickvergessen Collaborator

I'd prefer | over the bull here.
That's the way we use do it inside of [ ]:
Example in profile:
Warnings:
0 [ View user notes | Warn user ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
<!-- IF S_DISPLAY_SEARCH or (S_USER_LOGGED_IN and not S_IS_BOT) -->
<ul class="linklist">
View
2  phpBB/styles/prosilver/template/viewforum_body.html
@@ -1,5 +1,5 @@
<!-- INCLUDE overall_header.html -->
-<!-- IF U_MCP --><p>[&nbsp;<a href="{U_MCP}">{L_MCP}</a>&nbsp;]</p><!-- ENDIF -->
+<!-- IF U_MCP --><p>[&nbsp;<a href="{U_MCP}">{L_MCP}</a><!-- IF TOTAL_MODERATOR_REPORTS --> &bull; <a href="{U_MODERATOR_REPORTS}">{TOTAL_MODERATOR_REPORTS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_POSTS --> &bull; <a href="{U_MODERATOR_APPROVE_POSTS}">{TOTAL_MODERATOR_POSTS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_TOPICS --> &bull; <a href="{U_MODERATOR_APPROVE_TOPICS}">{TOTAL_MODERATOR_TOPICS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_PMS --> &bull; <a href="{U_MODERATOR_PMS}">{TOTAL_MODERATOR_PMS}</a><!-- ENDIF -->&nbsp;]</p><!-- ENDIF -->
<h2><a href="{U_VIEW_FORUM}">{FORUM_NAME}</a></h2>
<!-- IF FORUM_DESC or MODERATORS or U_MCP -->
View
2  phpBB/styles/prosilver/template/viewtopic_body.html
@@ -1,5 +1,5 @@
<!-- INCLUDE overall_header.html -->
-<!-- IF U_MCP --><p>[&nbsp;<a href="{U_MCP}">{L_MCP}</a>&nbsp;]</p><!-- ENDIF -->
+<!-- IF U_MCP --><p>[&nbsp;<a href="{U_MCP}">{L_MCP}</a><!-- IF TOTAL_MODERATOR_REPORTS --> &bull; <a href="{U_MODERATOR_REPORTS}">{TOTAL_MODERATOR_REPORTS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_POSTS --> &bull; <a href="{U_MODERATOR_APPROVE_POSTS}">{TOTAL_MODERATOR_POSTS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_TOPICS --> &bull; <a href="{U_MODERATOR_APPROVE_TOPICS}">{TOTAL_MODERATOR_TOPICS}</a><!-- ENDIF --><!-- IF TOTAL_MODERATOR_PMS --> &bull; <a href="{U_MODERATOR_PMS}">{TOTAL_MODERATOR_PMS}</a><!-- ENDIF -->&nbsp;]</p><!-- ENDIF -->
<h2><a href="{U_VIEW_TOPIC}">{TOPIC_TITLE}</a></h2>
<!-- NOTE: remove the style="display: none" when you want to have the forum description on the topic body -->
<!-- IF FORUM_DESC --><div style="display: none !important;">{FORUM_DESC}<br /></div><!-- ENDIF -->
Something went wrong with that request. Please try again.