Commit bbca7a55 authored by Yuliya Bozhko's avatar Yuliya Bozhko
Browse files

Refactor the way objectionable items are stored (Bug #1298646)



- Created 'objectionable' table
- Migrated existing objectionable records to a new table
- Added is_objectionable() method to View class
- Fixed can_view_view() and elasticsearch to use new table
- Fixed artefact.tpl style to match view page when displaying "Not objectionable" form
- Make forum reports use new 'objectionable' table

Change-Id: Ib3759e86c4003216ad57f3cf68911b13e50e9c67
Signed-off-by: default avatarYuliya Bozhko <yuliya.bozhko@totaralms.com>
parent 21d5712f
......@@ -39,8 +39,6 @@
<FIELD NAME="body" TYPE="text" NOTNULL="true" />
<FIELD NAME="ctime" TYPE="datetime" NOTNULL="true" />
<FIELD NAME="deleted" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" />
<FIELD NAME="reported" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" />
<FIELD NAME="reportedreason" TYPE="text" NOTNULL="false" />
<FIELD NAME="sent" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" />
<FIELD NAME="path" TYPE="char" LENGTH="2048" NOTNULL="false" />
</FIELDS>
......
......@@ -143,5 +143,18 @@ function xmldb_interaction_forum_upgrade($oldversion=0) {
activity_add_admin_defaults($adminusers);
}
if ($oldversion < 2014060500) {
// Drop unused fields.
$table = new XMLDBTable('interaction_forum_post');
$field = new XMLDBField('reported');
if (field_exists($table, $field)) {
drop_field($table, $field, true);
}
$field = new XMLDBField('reportedreason');
if (field_exists($table, $field)) {
drop_field($table, $field, true);
}
}
return true;
}
......@@ -836,9 +836,10 @@ class InteractionForumInstance extends InteractionInstance {
*/
public function has_objectionable() {
$reported = count_records_sql(
'SELECT count(fp.id) FROM {interaction_forum_topic} ft
"SELECT count(fp.id) FROM {interaction_forum_topic} ft
JOIN {interaction_forum_post} fp ON (ft.id = fp.topic)
WHERE fp.deleted = 0 AND fp.reported = 1 AND ft.forum = ?', array($this->id)
JOIN {objectionable} o ON (o.objecttype = 'forum' AND o.objectid = fp.id)
WHERE fp.deleted = 0 AND o.resolvedby IS NULL AND o.resolvedtime IS NULL AND ft.forum = ?", array($this->id)
);
return (bool) $reported;
}
......
......@@ -23,7 +23,8 @@ require_once('pieforms/pieform.php');
$postid = param_integer('id');
$post = get_record_sql(
'SELECT p.subject, p.body, p.topic, p.parent, p.poster, ' . db_format_tsfield('p.ctime', 'ctime') . ', p.reported, p.reportedreason, m.user AS moderator, t.forum, p2.subject AS topicsubject, f.group, f.title AS forumtitle, g.name AS groupname
'SELECT p.subject, p.body, p.topic, p.parent, p.poster, ' . db_format_tsfield('p.ctime', 'ctime') . ',
m.user AS moderator, t.forum, p2.subject AS topicsubject, f.group, f.title AS forumtitle, g.name AS groupname
FROM {interaction_forum_post} p
INNER JOIN {interaction_forum_topic} t ON (p.topic = t.id AND t.deleted != 1)
INNER JOIN {interaction_forum_post} p2 ON (p2.topic = t.id AND p2.parent IS NULL)
......@@ -79,21 +80,19 @@ $form = pieform(array(
function reportpost_submit(Pieform $form, $values) {
global $SESSION, $USER, $postid, $post;
global $SESSION, $USER, $postid;
$ctime = time();
$reportedreason = array();
if ($post->reported) {
$reportedreason = unserialize($post->reportedreason);
}
array_push($reportedreason, array('reporter' => $USER->get('id'), 'ctime' => $ctime, 'message' => $values['message']));
update_record(
'interaction_forum_post',
array('reported' => 1, 'reportedreason' => serialize($reportedreason)),
array('id' => $postid, 'topic' => $values['topic'])
);
// Trigger activity
$data = new StdClass;
$objection = new stdClass();
$objection->objecttype = 'forum';
$objection->objectid = $postid;
$objection->reportedby = $USER->get('id');
$objection->report = $values['message'];
$objection->reportedtime = db_format_timestamp($ctime);
insert_record('objectionable', $objection);
// Trigger activity.
$data = new StdClass();
$data->postid = $postid;
$data->message = $values['message'];
$data->reporter = $USER->get('id');
......
......@@ -107,7 +107,7 @@ if (!empty($postid)) {
}
$posts = get_records_sql_array(
'SELECT p.id, p.parent, p.path, p.poster, p.subject, p.body, ' . db_format_tsfield('p.ctime', 'ctime') . ', p.deleted, p.reported, p.reportedreason
'SELECT p.id, p.parent, p.path, p.poster, p.subject, p.body, ' . db_format_tsfield('p.ctime', 'ctime') . ', p.deleted
FROM {interaction_forum_post} p
WHERE p.topic = ?
ORDER BY p.path, p.ctime, p.id',
......@@ -133,7 +133,9 @@ foreach ($posts as $postid => $post) {
// If this is the own post
$post->ownpost = ($USER->get('id') == $post->poster) ? true : false;
// Reported reason data
$post->reportedreason = unserialize($post->reportedreason);
$post->reports = get_records_select_array('objectionable',
'objecttype = ? AND objectid = ? AND resolvedby IS NULL AND resolvedtime IS NULL',
array('forum', $post->id));
// Consolidate deleted message posts by the same author into one "X posts by Spammer Joe were deleted"
......@@ -230,21 +232,22 @@ function buildpostlist($posts, $mode, $max_depth) {
* @param int $indent indent value
* @return string html output
*/
function renderpost($post, $indent) {
global $moderator, $topic, $groupadmins, $membership, $ineditwindow, $USER;
$reportedaction = ($moderator && (bool)$post->reported);
$reportedaction = ($moderator && !empty($post->reports));
$highlightreported = false;
if ($reportedaction) {
$highlightreported = true;
$reportedreason = array();
foreach ($post->reportedreason as $reporteddata) {
$reportedreason['msg_' . $reporteddata['ctime']] = array(
$objections = array();
foreach ($post->reports as $report) {
$reportedreason['msg_' . strtotime($report->reportedtime)] = array(
'type' => 'html',
'value' => get_string('reportedpostdetails', 'interaction.forum', display_default_name($reporteddata['reporter']),
strftime(get_string('strftimedaydatetime'), $reporteddata['ctime']), $reporteddata['message']),
'value' => get_string('reportedpostdetails', 'interaction.forum', display_default_name($report->reportedby),
strftime(get_string('strftimedaydatetime'), strtotime($report->reportedtime)), $report->report),
);
$objections[] = $report->id;
}
$post->postnotobjectionableform = pieform(array(
'name' => 'postnotobjectionable_' . $post->id,
......@@ -255,6 +258,10 @@ function renderpost($post, $indent) {
'pluginname' => 'forum',
'autofocus' => false,
'elements' => array(
'objection' => array(
'type' => 'hidden',
'value' => implode(',', $objections),
),
'text' => array(
'type' => 'html',
'class' => 'postnotobjectionable',
......@@ -279,9 +286,9 @@ function renderpost($post, $indent) {
)
));
}
else if (is_array($post->reportedreason)) {
foreach ($post->reportedreason as $reason) {
if ($reason['reporter'] == $USER->get('id')) {
else if (!empty($post->reports)) {
foreach ($post->reports as $report) {
if ($report->reportedby == $USER->get('id')) {
$highlightreported = true;
break;
}
......@@ -341,11 +348,19 @@ function postnotobjectionable_validate(Pieform $form, $values) {
function postnotobjectionable_submit(Pieform $form, $values) {
global $SESSION, $USER, $topicid;
update_record(
'interaction_forum_post',
array('reported' => 0, 'reportedreason' => ''),
array('id' => $values['postid'], 'topic' => $topicid)
);
db_begin();
$objections = explode(',', $values['objection']);
// Mark records as resolved.
foreach ($objections as $objection) {
$todb = new stdClass();
$todb->resolvedby = $USER->get('id');
$todb->resolvedtime = db_format_timestamp(time());
update_record('objectionable', $todb, array('id' => $objection));
}
// Trigger activity.
$data = new StdClass();
......@@ -356,6 +371,8 @@ function postnotobjectionable_submit(Pieform $form, $values) {
$data->event = MAKE_NOT_OBJECTIONABLE;
activity_occurred('reportpost', $data, 'interaction', 'forum');
db_commit();
$SESSION->add_ok_msg(get_string('postnotobjectionablesuccess', 'interaction.forum'));
$redirecturl = get_config('wwwroot') . 'interaction/forum/topic.php?id=' . $topicid . '&post=' . $values['postid'];
......
......@@ -9,5 +9,5 @@
*
*/
$config->version = 2014050800;
$config->version = 2014060500;
$config->release = '1.2.2';
......@@ -275,8 +275,9 @@ function setup_topics(&$topics) {
$topic->containsobjectionable = false;
if ($moderator) {
$topic->containsobjectionable = (bool) count_records_sql(
'SELECT count(fp.id) FROM {interaction_forum_post} fp
WHERE fp.deleted = 0 AND fp.reported = 1 AND fp.topic = ?', array($topic->id));
"SELECT count(fp.id) FROM {interaction_forum_post} fp
JOIN {objectionable} o ON (o.objecttype = 'forum' AND o.objectid = fp.id)
WHERE fp.deleted = 0 AND o.resolvedby IS NULL AND o.resolvedtime IS NULL AND fp.topic = ?", array($topic->id));
}
}
}
......
......@@ -862,7 +862,7 @@
<TABLE NAME="view_access">
<FIELDS>
<FIELD NAME="view" TYPE="int" LENGTH="10" NOTNULL="true" />
<FIELD NAME="accesstype" TYPE="char" LENGTH="16" NOTNULL="false" ENUM="true" ENUMVALUES="'public', 'loggedin', 'friends', 'objectionable'" />
<FIELD NAME="accesstype" TYPE="char" LENGTH="16" NOTNULL="false" ENUM="true" ENUMVALUES="'public', 'loggedin', 'friends'" />
<FIELD NAME="group" TYPE="int" LENGTH="10" NOTNULL="false" />
<FIELD NAME="role" TYPE="char" LENGTH="255" NOTNULL="false" />
<FIELD NAME="usr" TYPE="int" LENGTH="10" NOTNULL="false" />
......@@ -1211,5 +1211,25 @@
<KEY NAME="usrfk" TYPE="foreign" FIELDS="usr" REFTABLE="usr" REFFIELDS="id"/>
</KEYS>
</TABLE>
<TABLE NAME="objectionable">
<FIELDS>
<FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true" />
<FIELD NAME="objecttype" TYPE="char" LENGTH="20" NOTNULL="true" />
<FIELD NAME="objectid" TYPE="int" LENGTH="10" NOTNULL="true" />
<FIELD NAME="reportedby" TYPE="int" LENGTH="10" NOTNULL="true" />
<FIELD NAME="report" TYPE="text" NOTNULL="true" />
<FIELD NAME="reportedtime" TYPE="datetime" NOTNULL="true" />
<FIELD NAME="resolvedby" TYPE="int" LENGTH="10" NOTNULL="false" />
<FIELD NAME="resolvedtime" TYPE="datetime" NOTNULL="false" />
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id" />
<KEY NAME="reporterfk" TYPE="foreign" FIELDS="reportedby" REFTABLE="usr" REFFIELDS="id" />
<KEY NAME="resolverfk" TYPE="foreign" FIELDS="resolvedby" REFTABLE="usr" REFFIELDS="id" />
</KEYS>
<INDEXES>
<INDEX NAME="objectix" UNIQUE="false" FIELDS="objectid, objecttype"/>
</INDEXES>
</TABLE>
</TABLES>
</XMLDB>
......@@ -3336,5 +3336,68 @@ function xmldb_core_upgrade($oldversion=0) {
}
}
// Make objectionable independent of view_access page.
if ($oldversion < 2014060300) {
// Create 'objectionable' table.
$table = new XMLDBTable('objectionable');
$table->addFieldInfo('id', XMLDB_TYPE_INTEGER, 10, null, XMLDB_NOTNULL, XMLDB_SEQUENCE);
$table->addFieldInfo('objecttype', XMLDB_TYPE_CHAR, 20, null, XMLDB_NOTNULL);
$table->addFieldInfo('objectid', XMLDB_TYPE_INTEGER, 10, null, XMLDB_NOTNULL);
$table->addFieldInfo('reportedby', XMLDB_TYPE_INTEGER, 10, null, XMLDB_NOTNULL);
$table->addFieldInfo('report', XMLDB_TYPE_TEXT, 'small', null, XMLDB_NOTNULL);
$table->addFieldInfo('reportedtime', XMLDB_TYPE_DATETIME, null, null, XMLDB_NOTNULL);
$table->addFieldInfo('resolvedby', XMLDB_TYPE_INTEGER, 10, null, null);
$table->addFieldInfo('resolvedtime', XMLDB_TYPE_DATETIME, null, null);
$table->addKeyInfo('primary', XMLDB_KEY_PRIMARY, array('id'));
$table->addKeyInfo('reporterfk', XMLDB_KEY_FOREIGN, array('reportedby'), 'usr', array('id'));
$table->addKeyInfo('resolverfk', XMLDB_KEY_FOREIGN, array('resolvedby'), 'usr', array('id'));
$table->addIndexInfo('objectix', XMLDB_INDEX_NOTUNIQUE, array('objectid', 'objecttype'));
if (!table_exists($table)) {
create_table($table);
}
// Migrate data to a new format.
// Since we don't have report or name of the user, use root ID.
// Table 'notification_internal_activity' contains data that is
// not possible to extract in any reasonable way.
$objectionable = get_records_array('view_access', 'accesstype', 'objectionable');
db_begin();
if (!empty($objectionable)) {
foreach ($objectionable as $record) {
$todb = new stdClass();
$todb->objecttype = 'view';
$todb->objectid = $record->view;
$todb->reportedby = 0;
$todb->report = '';
$todb->reportedtime = $record->ctime;
if (!empty($record->stopdate)) {
// Since we can't get an ID of a user who resolved an issue, use root ID.
$todb->resolvedby = 0;
$todb->resolvedtime = $record->stopdate;
}
insert_record('objectionable', $todb);
}
}
// Delete data from 'view_access' table as we don't need it any more.
delete_records('view_access', 'accesstype', 'objectionable');
db_commit();
// Need to run this to avoid contraints problems on Postgres.
if (is_postgres()) {
execute_sql('ALTER TABLE {view_access} DROP CONSTRAINT {viewacce_acc_ck}');
}
// Update accesstype in 'view_access' not to use 'objectionable'.
$table = new XMLDBTable('view_access');
$field = new XMLDBField('accesstype');
$field->setAttributes(XMLDB_TYPE_CHAR, 16, null, null, null, XMLDB_ENUM, array('public', 'loggedin', 'friends'));
change_field_enum($table, $field);
}
return $status;
}
......@@ -2140,7 +2140,7 @@ function can_view_view($view, $user_id=null) {
continue;
}
}
else if ($a->accesstype == 'objectionable') {
else if ($view->is_objectionable()) {
if ($owner = $view->get('owner')) {
if ($user->is_admin_for_user($owner)) {
return true;
......
......@@ -54,20 +54,20 @@ function objection_form_submit(Pieform $form, $values) {
db_begin();
// The objectionable access record ensures the view is visible
// to admins, and also marks the view as objectionable.
$accessrecord = (object) array(
'view' => $view->get('id'),
'accesstype' => 'objectionable',
'allowcomments' => 1,
'approvecomments' => 0,
'visible' => 0,
'ctime' => db_format_timestamp(time()),
);
$objection = new stdClass();
if ($artefact) {
$objection->objecttype = 'artefact';
$objection->objectid = $artefact->get('id');
}
else {
$objection->objecttype = 'view';
$objection->objectid = $view->get('id');
}
$objection->reportedby = $USER->get('id');
$objection->report = $values['message'];
$objection->reportedtime = db_format_timestamp(time());
delete_records('view_access', 'view', $view->get('id'), 'accesstype', 'objectionable', 'visible', 0);
insert_record('view_access', $accessrecord);
insert_record('objectionable', $objection);
$data = new StdClass();
$data->view = $view->get('id');
......@@ -111,29 +111,20 @@ function objection_form_cancel_submit(Pieform $form) {
* @returns array Form elements.
*/
function notrude_form() {
global $USER, $view;
global $USER, $view, $artefact;
$owner = $view->get('owner');
if (!(($owner && ($USER->get('admin') || $USER->is_admin_for_user($owner)))
|| ($view->get('group') && $USER->get('admin')))) {
return;
}
$access = View::user_access_records($view->get('id'), $USER->get('id'));
if (empty($access)) {
return;
if ($artefact) {
$params = array('artefact', $artefact->get('id'));
}
$isrude = false;
foreach ($access as $a) {
// Nasty hack: If the objectionable access record has a stop date, it
// means that one of the admins has already dealt with it, so we don't
// mark the view as objectionable.
if ($a->accesstype == 'objectionable' && empty($a->stopdate)) {
$isrude = true;
break;
}
else {
$params = array('view', $view->get('id'));
}
$isrude = get_record_select('objectionable', 'objecttype = ? AND objectid = ? AND resolvedby IS NULL LIMIT 1', $params);
if (!$isrude) {
return;
......@@ -143,6 +134,10 @@ function notrude_form() {
'name' => 'notrude_form',
'method' => 'post',
'elements' => array(
'objection' => array(
'type' => 'hidden',
'value' => $isrude->id,
),
'text' => array(
'type' => 'html',
'value' => get_string('viewobjectionableunmark', 'view'),
......@@ -162,19 +157,19 @@ function notrude_form_submit(Pieform $form, $values) {
db_begin();
// Set exipiry date on view access record.
$accessrecord = (object) array(
'view' => $view->get('id'),
'accesstype' => 'objectionable',
'allowcomments' => 1,
'approvecomments' => 0,
'visible' => 0,
'stopdate' => db_format_timestamp(time() + 60*60*24*7),
'ctime' => db_format_timestamp(time()),
);
$objection = new stdClass();
if ($artefact) {
$objection->objecttype = 'artefact';
$objection->objectid = $artefact->get('id');
}
else {
$objection->objecttype = 'view';
$objection->objectid = $view->get('id');
}
$objection->resolvedby = $USER->get('id');
$objection->resolvedtime = db_format_timestamp(time());
delete_records('view_access', 'view', $view->get('id'), 'accesstype', 'objectionable', 'visible', 0);
insert_record('view_access', $accessrecord);
update_record('objectionable', $objection, array('id' => $values['objection']));
// Send notification to other admins.
$reportername = display_default_name($USER);
......
......@@ -15,7 +15,8 @@ $config = new stdClass();
// See https://wiki.mahara.org/index.php/Developer_Area/Version_Numbering_Policy
// For upgrades on stable branches, increment the version by one. On master, use the date.
$config->version = 2014050901;
$config->version = 2014060300;
$config->release = '1.10.0dev';
$config->minupgradefrom = 2009022600;
$config->minupgraderelease = '1.1.0 (release tag 1.1.0_RELEASE)';
......
......@@ -1203,6 +1203,16 @@ class View {
}
}
/**
* Returns true if the view is currently marked as objectionable
*
* @return boolean True if view is objectionable
*/
public function is_objectionable() {
$params = array('view', $this->id);
return record_exists_select('objectionable', 'objecttype = ? AND objectid = ? AND resolvedby IS NULL', $params);
}
public function is_public() {
$accessrecords = self::user_access_records($this->id, 0);
if (!$accessrecords) {
......@@ -4828,7 +4838,7 @@ class View {
WHERE va.view = ?
AND (va.startdate IS NULL OR va.startdate < current_timestamp)
AND (va.stopdate IS NULL OR va.stopdate > current_timestamp)
AND (va.accesstype IN ('public', 'loggedin', 'friends', 'objectionable')
AND (va.accesstype IN ('public', 'loggedin', 'friends')
OR va.usr = ? OR va.token IS NOT NULL OR gm.member IS NOT NULL OR va.institution IS NOT NULL)
ORDER BY va.token IS NULL DESC, va.accesstype != 'friends' DESC",
array($userid, $viewid, $userid)
......@@ -4909,8 +4919,9 @@ class View {
}
}
if ($a->allowcomments && (($a->accesstype == 'objectionable' && ($user->get('admin')
|| $user->is_institutional_admin()) || $a->accesstype != 'objectionable'))) {
$objectionable = $this->is_objectionable();
if ($a->allowcomments && (($objectionable && ($user->get('admin')
|| $user->is_institutional_admin()) || !$objectionable))) {
$allowcomments = $allowcomments || $a->allowcomments;
$approvecomments = $approvecomments && $a->approvecomments;
}
......
......@@ -358,7 +358,7 @@ class ElasticsearchType_artefact extends ElasticsearchType
WHERE vart.artefact = ?
AND (vac.startdate IS NULL OR vac.startdate < current_timestamp)
AND (vac.stopdate IS NULL OR vac.stopdate > current_timestamp)
AND (vac.accesstype != \'objectionable\' OR vac.accesstype IS NULL)
AND vac.accesstype IS NULL
UNION
SELECT vac.view AS view_id, vac.accesstype, vac.group, vac.role, vac.usr, vac.institution
FROM {artefact} art
......@@ -367,7 +367,7 @@ class ElasticsearchType_artefact extends ElasticsearchType
WHERE art.id = ?
AND (vac.startdate IS NULL OR vac.startdate < current_timestamp)
AND (vac.stopdate IS NULL OR vac.stopdate > current_timestamp)
AND (vac.accesstype != \'objectionable\' OR vac.accesstype IS NULL);
AND vac.accesstype IS NULL;
',
array($artefactid, $artefactid)
);
......
......@@ -206,7 +206,7 @@ class ElasticsearchType_collection extends ElasticsearchType
WHERE vcol.collection = ?
AND (vac.startdate IS NULL OR vac.startdate < current_timestamp)
AND (vac.stopdate IS NULL OR vac.stopdate > current_timestamp)
AND (vac.accesstype != \'objectionable\' OR vac.accesstype IS NULL)',
AND vac.accesstype IS NULL',
array($id)
);
......
......@@ -200,9 +200,9 @@ class ElasticsearchType_view extends ElasticsearchType
SELECT va.view AS view_id, va.accesstype, va.group, va.role, va.usr, va.institution
FROM {view_access} va
WHERE va.view = ?
AND (startdate IS NULL OR startdate < current_timestamp)
AND (stopdate IS NULL OR stopdate > current_timestamp)
AND (accesstype != \'objectionable\' OR accesstype IS NULL)',
AND (startdate IS NULL OR startdate < current_timestamp)
AND (stopdate IS NULL OR stopdate > current_timestamp)
AND accesstype IS NULL',
array($viewid)
);
......
{if $microheaders}{include file="viewmicroheader.tpl"}{else}{include file="header.tpl"}{/if}
{if $notrudeform}<div class="message delete">{$notrudeform|safe}</div>{/if}
{if $notrudeform}<div class="message deletemessage">{$notrudeform|safe}</div>{/if}
<h2>
{$view->display_title()|safe}{foreach from=$artefactpath item=a}:
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment