From bc29320285412daf642c1f75c79d18130d31a07a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0koda?= Date: Thu, 20 Mar 2014 15:45:37 +0800 Subject: [PATCH 1/2] MDL-44661 implement event->anonymous property --- admin/tool/log/store/standard/db/install.xml | 3 +- admin/tool/log/store/standard/db/upgrade.php | 46 +++++++++++++++++++ admin/tool/log/store/standard/version.php | 2 +- lib/classes/event/base.php | 11 ++++- lib/tests/event_test.php | 8 +++- .../classes/event/course_module_viewed.php | 4 +- .../classes/event/response_deleted.php | 5 +- .../classes/event/response_submitted.php | 7 ++- mod/feedback/lib.php | 9 +++- mod/feedback/tests/events_test.php | 5 +- mod/feedback/view.php | 3 +- 11 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 admin/tool/log/store/standard/db/upgrade.php diff --git a/admin/tool/log/store/standard/db/install.xml b/admin/tool/log/store/standard/db/install.xml index 6232e6eeafb..053f8cdcb80 100644 --- a/admin/tool/log/store/standard/db/install.xml +++ b/admin/tool/log/store/standard/db/install.xml @@ -1,5 +1,5 @@ - @@ -21,6 +21,7 @@ + diff --git a/admin/tool/log/store/standard/db/upgrade.php b/admin/tool/log/store/standard/db/upgrade.php new file mode 100644 index 00000000000..db7a122262e --- /dev/null +++ b/admin/tool/log/store/standard/db/upgrade.php @@ -0,0 +1,46 @@ +. + +/** + * Standard log store upgrade. + * + * @package logstore_standard + * @copyright 2014 Petr Skoda + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +function xmldb_logstore_standard_upgrade($oldversion) { + global $CFG, $DB; + + $dbman = $DB->get_manager(); + + if ($oldversion < 2014032000) { + + // Define field anonymous to be added to logstore_standard_log. + $table = new xmldb_table('logstore_standard_log'); + $field = new xmldb_field('anonymous', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'relateduserid'); + + // Conditionally launch add field anonymous. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Standard savepoint reached. + upgrade_plugin_savepoint(true, 2014032000, 'logstore', 'standard'); + } + + return true; +} diff --git a/admin/tool/log/store/standard/version.php b/admin/tool/log/store/standard/version.php index 60a98ef2232..94f88e238da 100644 --- a/admin/tool/log/store/standard/version.php +++ b/admin/tool/log/store/standard/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2014031300; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2014032000; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2014031200; // Requires this Moodle version. $plugin->component = 'logstore_standard'; // Full name of the plugin (used for diagnostics). diff --git a/lib/classes/event/base.php b/lib/classes/event/base.php index 0050d963642..c2c621ed352 100644 --- a/lib/classes/event/base.php +++ b/lib/classes/event/base.php @@ -47,6 +47,8 @@ defined('MOODLE_INTERNAL') || die(); * @property-read int $userid who did this? * @property-read int $courseid * @property-read int $relateduserid + * @property-read int $anonymous 1 means event should not be visible in reports, 0 means normal event, + * create() argument may be also true/false. * @property-read mixed $other array or scalar, can not contain objects * @property-read int $timecreated */ @@ -102,7 +104,7 @@ abstract class base implements \IteratorAggregate { /** @var array list of event properties */ private static $fields = array( 'eventname', 'component', 'action', 'target', 'objecttable', 'objectid', 'crud', 'edulevel', 'contextid', - 'contextlevel', 'contextinstanceid', 'userid', 'courseid', 'relateduserid', 'other', + 'contextlevel', 'contextinstanceid', 'userid', 'courseid', 'relateduserid', 'anonymous', 'other', 'timecreated'); /** @var array simple record cache */ @@ -158,6 +160,9 @@ abstract class base implements \IteratorAggregate { $event->restored = false; $event->dispatched = false; + // By default all events are visible in logs. + $event->data['anonymous'] = 0; + // Set static event data specific for child class. $event->init(); @@ -178,6 +183,10 @@ abstract class base implements \IteratorAggregate { $event->data['userid'] = isset($data['userid']) ? $data['userid'] : $USER->id; $event->data['other'] = isset($data['other']) ? $data['other'] : null; $event->data['relateduserid'] = isset($data['relateduserid']) ? $data['relateduserid'] : null; + if (isset($data['anonymous'])) { + $event->data['anonymous'] = $data['anonymous']; + } + $event->data['anonymous'] = (int)(bool)$event->data['anonymous']; if (isset($event->context)) { if (isset($data['context'])) { diff --git a/lib/tests/event_test.php b/lib/tests/event_test.php index 28f761bce68..fe79d577c13 100644 --- a/lib/tests/event_test.php +++ b/lib/tests/event_test.php @@ -54,6 +54,8 @@ class core_event_testcase extends advanced_testcase { $this->assertNull($event->relateduserid); $this->assertFalse(isset($event->relateduserid)); + $this->assertSame(0, $event->anonymous); + $this->assertSame(array('sample'=>null, 'xx'=>10), $event->other); $this->assertTrue(isset($event->other['xx'])); $this->assertFalse(isset($event->other['sample'])); @@ -74,8 +76,12 @@ class core_event_testcase extends advanced_testcase { $this->assertInstanceOf('coding_exception', $e); } - $event2 = \core_tests\event\unittest_executed::create(array('contextid'=>$system->id, 'objectid'=>5, 'other'=>array('sample'=>null, 'xx'=>10))); + $event2 = \core_tests\event\unittest_executed::create(array('contextid'=>$system->id, 'objectid'=>5, 'anonymous'=>1, 'other'=>array('sample'=>null, 'xx'=>10))); $this->assertEquals($event->get_context(), $event2->get_context()); + $this->assertSame(1, $event2->anonymous); + + $event3 = \core_tests\event\unittest_executed::create(array('contextid'=>$system->id, 'objectid'=>5, 'anonymous'=>true, 'other'=>array('sample'=>null, 'xx'=>10))); + $this->assertSame(1, $event3->anonymous); } public function test_event_properties_guessing() { diff --git a/mod/feedback/classes/event/course_module_viewed.php b/mod/feedback/classes/event/course_module_viewed.php index b822bca68c2..2f3e1df4501 100644 --- a/mod/feedback/classes/event/course_module_viewed.php +++ b/mod/feedback/classes/event/course_module_viewed.php @@ -62,7 +62,7 @@ class course_module_viewed extends \core\event\course_module_viewed { if (empty($userorid)) { $userorid = $USER; } - if ($this->other['anonymous'] == FEEDBACK_ANONYMOUS_YES) { + if ($this->anonymous) { return is_siteadmin($userorid); } else { return has_capability('mod/feedback:viewreports', $this->context, $userorid); @@ -76,7 +76,7 @@ class course_module_viewed extends \core\event\course_module_viewed { * @return array of parameters to be passed to legacy add_to_log() function. */ protected function get_legacy_logdata() { - if ($this->other['anonymous'] == FEEDBACK_ANONYMOUS_YES) { + if ($this->anonymous) { return null; } else { return parent::get_legacy_logdata(); diff --git a/mod/feedback/classes/event/response_deleted.php b/mod/feedback/classes/event/response_deleted.php index 14ab5ee59d4..e200f1abf07 100644 --- a/mod/feedback/classes/event/response_deleted.php +++ b/mod/feedback/classes/event/response_deleted.php @@ -93,7 +93,7 @@ class response_deleted extends \core\event\base { if (empty($userorid)) { $userorid = $USER; } - if ($this->other['anonymous'] == FEEDBACK_ANONYMOUS_YES) { + if ($this->anonymous) { return is_siteadmin($userorid); } else { return has_capability('mod/feedback:viewreports', $this->context, $userorid); @@ -115,6 +115,9 @@ class response_deleted extends \core\event\base { if (!isset($this->other['instanceid'])) { throw new \coding_exception("Field other['instanceid'] cannot be empty"); } + + // Call parent validations. + parent::validate_data(); } } diff --git a/mod/feedback/classes/event/response_submitted.php b/mod/feedback/classes/event/response_submitted.php index 239232ff4a8..83ff1da1956 100644 --- a/mod/feedback/classes/event/response_submitted.php +++ b/mod/feedback/classes/event/response_submitted.php @@ -95,7 +95,7 @@ class response_submitted extends \core\event\base { * @return array of parameters to be passed to legacy add_to_log() function. */ protected function get_legacy_logdata() { - if ($this->other['anonymous'] == FEEDBACK_ANONYMOUS_YES) { + if ($this->anonymous) { return null; } else { return array($this->courseid, 'feedback', 'submit', 'view.php?id=' . $this->other['cmid'], @@ -115,7 +115,7 @@ class response_submitted extends \core\event\base { if (empty($userorid)) { $userorid = $USER; } - if ($this->other['anonymous'] == FEEDBACK_ANONYMOUS_YES) { + if ($this->anonymous) { return is_siteadmin($userorid); } else { return has_capability('mod/feedback:viewreports', $this->context, $userorid); @@ -137,6 +137,9 @@ class response_submitted extends \core\event\base { if (!isset($this->other['instanceid'])) { throw new \coding_exception("Field other['instanceid'] cannot be empty"); } + + // Call parent validations. + parent::validate_data(); } } diff --git a/mod/feedback/lib.php b/mod/feedback/lib.php index 760e3c6f029..18d87a088df 100644 --- a/mod/feedback/lib.php +++ b/mod/feedback/lib.php @@ -1915,10 +1915,11 @@ function feedback_save_tmp_values($feedbackcompletedtmp, $feedbackcompleted, $us 'relateduserid' => $userid, 'objectid' => $feedbackcompleted->id, 'context' => context_module::instance($cm->id), + 'anonymous' => ($feedbackcompleted->anonymous_response == FEEDBACK_ANONYMOUS_YES), 'other' => array( 'cmid' => $cm->id, 'instanceid' => $feedbackcompleted->feedback, - 'anonymous' => $feedbackcompleted->anonymous_response + 'anonymous' => $feedbackcompleted->anonymous_response // Deprecated. ) )); @@ -2695,7 +2696,11 @@ function feedback_delete_completed($completedid) { 'objectid' => $completedid, 'courseid' => $course->id, 'context' => context_module::instance($cm->id), - 'other' => array('cmid' => $cm->id, 'instanceid' => $feedback->id, 'anonymous' => $completed->anonymous_response) + 'anonymous' => ($completed->anonymous_response == FEEDBACK_ANONYMOUS_YES), + 'other' => array( + 'cmid' => $cm->id, + 'instanceid' => $feedback->id, + 'anonymous' => $completed->anonymous_response) // Deprecated. )); $event->add_record_snapshot('feedback_completed', $completed); diff --git a/mod/feedback/tests/events_test.php b/mod/feedback/tests/events_test.php index 6da1f61197d..bdd422e4a5c 100644 --- a/mod/feedback/tests/events_test.php +++ b/mod/feedback/tests/events_test.php @@ -224,6 +224,7 @@ class mod_feedback_events_testcase extends advanced_testcase { $this->assertEquals($USER->id, $event->userid); $this->assertEquals($USER->id, $event->relateduserid); $this->assertEquals('feedback_completed', $event->objecttable); + $this->assertEquals(1, $event->anonymous); $this->assertEquals(FEEDBACK_ANONYMOUS_YES, $event->other['anonymous']); $this->setUser($this->eventuser); $this->assertFalse($event->can_view()); @@ -267,7 +268,7 @@ class mod_feedback_events_testcase extends advanced_testcase { } /** - * Tests for event validations related to feedback response submittion. + * Tests for event validations related to feedback response submission. */ public function test_response_submitted_event_exceptions() { @@ -280,6 +281,7 @@ class mod_feedback_events_testcase extends advanced_testcase { \mod_feedback\event\response_submitted::create(array( 'context' => $context, 'objectid' => $this->eventfeedbackcompleted->id, + 'anonymous' => 0, 'other' => array('cmid' => $this->eventcm->id, 'anonymous' => 2) )); $this->fail("Event validation should not allow \\mod_feedback\\event\\response_deleted to be triggered without @@ -293,6 +295,7 @@ class mod_feedback_events_testcase extends advanced_testcase { \mod_feedback\event\response_submitted::create(array( 'context' => $context, 'objectid' => $this->eventfeedbackcompleted->id, + 'anonymous' => 0, 'other' => array('instanceid' => $this->eventfeedback->id, 'anonymous' => 2) )); $this->fail("Event validation should not allow \\mod_feedback\\event\\response_deleted to be triggered without diff --git a/mod/feedback/view.php b/mod/feedback/view.php index 75f86b7035e..83dec2720b3 100644 --- a/mod/feedback/view.php +++ b/mod/feedback/view.php @@ -100,8 +100,9 @@ if ($courseid AND $courseid != SITEID) { $event = \mod_feedback\event\course_module_viewed::create(array( 'objectid' => $feedback->id, 'context' => $context, + 'anonymous' => ($feedback->anonymous == FEEDBACK_ANONYMOUS_YES), 'other' => array( - 'anonymous' => $feedback->anonymous + 'anonymous' => $feedback->anonymous // Deprecated. ) )); $event->add_record_snapshot('course_modules', $cm); From 454c7b5d3980584de0ecebcf7cbc5067589dcdd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0koda?= Date: Thu, 20 Mar 2014 17:06:28 +0800 Subject: [PATCH 2/2] MDL-44661 fix legacy log reader to ignore new anonymous field --- admin/tool/log/store/legacy/classes/log/store.php | 3 ++- admin/tool/log/store/legacy/tests/store_test.php | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/admin/tool/log/store/legacy/classes/log/store.php b/admin/tool/log/store/legacy/classes/log/store.php index c68c767c9a5..a8e981c627d 100644 --- a/admin/tool/log/store/legacy/classes/log/store.php +++ b/admin/tool/log/store/legacy/classes/log/store.php @@ -39,7 +39,8 @@ class store implements \tool_log\log\store, \core\log\sql_select_reader { 'timecreated' => 'time', 'courseid' => 'course', 'contextinstanceid' => 'cmid', - 'origin' => 'ip' + 'origin' => 'ip', + 'anonymous' => 0, ); /** @var string Regex to replace the crud params */ diff --git a/admin/tool/log/store/legacy/tests/store_test.php b/admin/tool/log/store/legacy/tests/store_test.php index c99f017ed61..89479dcf6b8 100644 --- a/admin/tool/log/store/legacy/tests/store_test.php +++ b/admin/tool/log/store/legacy/tests/store_test.php @@ -213,6 +213,11 @@ class logstore_legacy_store_testcase extends advanced_testcase { list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); $this->assertEquals("edulevel = 0 and action LIKE '%update%' OR action NOT LIKE '%view%' AND action NOT LIKE '%report%' or action NOT LIKE '%delete%'", $updatewhere); + // The anonymous select returns all data. + $selectwhere = "anonymous = 0"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertSame("0 = 0", $updatewhere); + // Test legacy field names are mapped. $selectwhere = "timecreated = :timecreated and courseid = :courseid and contextinstanceid = :contextinstanceid and origin = :origin"; $params = array('timecreated' => 2, 'courseid' => 3, 'contextinstanceid' => 4, 'origin' => 5 );