-
Bug
-
Resolution: Fixed
-
Major
-
3.1.10, 3.2.0
Events in the content_visibility class are ill-defined.
In some/most situations, trying to add a condition through those events will result in total failure. And in a lot of others, it is simply impossible due to grouping and priorities.
This needs some serious fixing.
Also, the use of content_visibility across the phpbb software is random. In some places it is used, while in others it is not. And the concept of "visible" is, in a number of situations, treated as synonym for "approved", which extensions may need/want to redefine.
One example: trying to make the unapproved posts visible by the poster, the code in the listener would be something like:
```
/**
* SQL to get content visibility for forum
*
* @param object $event The event object
* @return void
* @event core.content_visibility_get_visibility_sql_before
*/
public function get_visibility_sql_before($event)
{
$event['where_sql'] = '(' . $event['table_alias'] . $event['mode'] . '_visibility = ' . ITEM_UNAPPROVED . ' AND ' .
$event['table_alias'] . (($event['mode'] == 'post') ? 'poster_id = ' : 'topic_poster = ') .
(int) $this->user->data['user_id'] . ') OR ';
}
```
The event description says:
```
/**
* Allow changing the result of calling get_visibility_sql
*
* @event core.phpbb_content_visibility_get_visibility_sql_before
* @var string where_sql Extra visibility conditions. It must end with either an SQL "AND" or an "OR"
* @var string mode Either "topic" or "post" depending on the query this is being used in
* @var array forum_id The forum id in which the search is made.
* @var string table_alias Table alias to prefix in SQL queries
* @var mixed get_visibility_sql_overwrite If a string, forces the function to return get_forums_visibility_sql_overwrite after executing the event
* If false, get_visibility_sql continues normally
* It must be either boolean or string
* @since 3.1.4-RC1
*/
```
With this, the function correctly? returns (p.post_visibility = 0 AND p.poster_id = 59) OR p.post_visibility = 1, without surrounding parenthesis.
But in case an OR is used (like the case above) the finally generated SQL when accessing a topic looks like:
SELECT p.post_id
FROM phpbb_posts p
WHERE p.topic_id = 165 AND
(p.post_visibility = 0 AND p.poster_id = 59) OR p.post_visibility = 1
ORDER BY p.post_time ASC, p.post_id ASC
That given the operator priorities, will give back the list of ALL approved posts in any topic.
Another example, in the core.phpbb_content_visibility_get_global_visibility_before event. Event description:
```
/**
* Allow changing the result of calling get_global_visibility_sql
*
* @event core.phpbb_content_visibility_get_global_visibility_before
* @var array where_sqls The action the user tried to execute
* @var string mode Either "topic" or "post" depending on the query this is being used in
* @var array exclude_forum_ids Array of forum ids the current user doesn't have access to
* @var string table_alias Table alias to prefix in SQL queries
* @var array approve_forums Array of forums where the user has m_approve permissions
* @var string visibility_sql_overwrite Forces the function to return an implosion of where_sqls (joined by "OR")
* @since 3.1.3-RC1
*/
```
Apart that the documentation is totally wrong (visibility_sql_overwrite MUST be a string, and does not force an implosion or anything, while where_sqls is not an action, but an empty array, that MUST be filled in taking into account that will be converted to string by imploding it with "OR"), also the code is wrong: in case an extension adds a where_sqls element, and there are no moderated forum, the function returns:
// There is only one element, so we just return that one
return $where_sqls[0];
Wrong, because it will be returning whatever the extension has put into the $where_sqls array, WITHOUT taking into consideration the rest of the function.
There are more examples (in core.phpbb_content_visibility_get_forums_visibility_before it is impossible to add the condition above due to grouping and ordering of the code), but I don't want to write a Bible here.