Skip to content

Commit

Permalink
MDL-39846 require context when creating events
Browse files Browse the repository at this point in the history 
No more guessing or falling back to system context.
  • Loading branch information
@skodak
skodak committed Jul 19, 2013
1 parent 62401e8 commit 27af3e6
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 29 deletions.
39 changes: 23 additions & 16 deletions lib/classes/event/base.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,32 +143,39 @@ public static final function create(array $data = null) {
$event->data['other'] = isset($data['other']) ? $data['other'] : null;
$event->data['relateduserid'] = isset($data['relateduserid']) ? $data['relateduserid'] : null;

$event->context = null;
if (isset($data['context'])) {
if (!empty($data['context'])) {
$event->context = $data['context'];
} else if (isset($data['contextid'])) {
$event->context = \context::instance_by_id($data['contextid']);
} else if ($event->data['courseid']) {
$event->context = \context_course::instance($event->data['courseid']);
} else if (isset($PAGE)) {
$event->context = $PAGE->context;
}
if (!$event->context) {
$event->context = \context_system::instance();
$event->data['contextid'] = $event->context->id;
$event->data['contextlevel'] = $event->context->contextlevel;
$event->data['contextinstanceid'] = $event->context->instanceid;

} else if (!empty($data['contextid'])) {
$event->context = null;
if (isset($data['contextlevel']) and isset($data['contextinstanceid'])) {
// Useful especially when deleting contexts because we can nto fetch it from DB anymore.
$event->data['contextid'] = $data['contextid'];
$event->data['contextlevel'] = $data['contextlevel'];
$event->data['contextinstanceid'] = $data['contextinstanceid'];
$event->context = \context::instance_by_id($data['contextid'], IGNORE_MISSING);
} else {
$event->context = \context::instance_by_id($data['contextid'], MUST_EXIST);
$event->data['contextid'] = $event->context->id;
$event->data['contextlevel'] = $event->context->contextlevel;
$event->data['contextinstanceid'] = $event->context->instanceid;
}
} else {
throw new \coding_exception('context (or contextid) is a required event property.');
}
$event->data['contextid'] = $event->context->id;
$event->data['contextlevel'] = $event->context->contextlevel;
$event->data['contextinstanceid'] = $event->context->instanceid;

if (!isset($event->data['courseid'])) {
if ($coursecontext = $event->context->get_course_context(false)) {
if ($event->context and $coursecontext = $event->context->get_course_context(false)) {
$event->data['courseid'] = $coursecontext->id;
} else {
$event->data['courseid'] = 0;
}
}

if (!array_key_exists('relateduserid', $data) and $event->context->contextlevel == CONTEXT_USER) {
if (!array_key_exists('relateduserid', $data) and $event->context and $event->context->contextlevel == CONTEXT_USER) {
$event->data['relateduserid'] = $event->context->instanceid;
}

Expand Down
39 changes: 26 additions & 13 deletions lib/tests/event_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ public function test_event_properties() {
} catch (\moodle_exception $e) {
$this->assertInstanceOf('coding_exception', $e);
}

$event2 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'contextid'=>$system->id, 'objectid'=>5, 'other'=>array('sample'=>null, 'xx'=>10)));
$this->assertSame($event->get_context(), $event2->get_context());

$event3 = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'contextid'=>999, 'contextlevel'=>CONTEXT_COURSE, 'contextinstanceid'=>4554645, 'objectid'=>5, 'other'=>array('sample'=>null, 'xx'=>10)));
$this->assertSame(false, $event3->get_context());
}

public function test_observers_parsing() {
Expand Down Expand Up @@ -539,39 +545,46 @@ public function test_trigger_problems() {
}

public function test_bad_events() {
$event = \core_tests\event\bad_event1::create();
try {
$event = \core_tests\event\unittest_executed::create(array('courseid'=>1, 'other'=>array('sample'=>5, 'xx'=>10)));
$this->fail('Exception expected when context and contextid missing');
} catch (Exception $e) {
$this->assertInstanceOf('coding_exception', $e);
}

$event = \core_tests\event\bad_event1::create(array('context'=>\context_system::instance()));
try {
$event->trigger();
$this->fail('Exception expected when $data not valid');
} catch (\moodle_exception $e) {
$this->assertInstanceOf('\coding_exception', $e);
}

$event = \core_tests\event\bad_event2::create();
$event = \core_tests\event\bad_event2::create(array('context'=>\context_system::instance()));
try {
$event->trigger();
$this->fail('Exception expected when $data not valid');
} catch (\moodle_exception $e) {
$this->assertInstanceOf('\coding_exception', $e);
}

$event = \core_tests\event\bad_event3::create();
$event = \core_tests\event\bad_event3::create(array('context'=>\context_system::instance()));
@$event->trigger();
$this->assertDebuggingCalled();

$event = \core_tests\event\bad_event4::create();
$event = \core_tests\event\bad_event4::create(array('context'=>\context_system::instance()));
@$event->trigger();
$this->assertDebuggingCalled();

$event = \core_tests\event\bad_event5::create();
$event = \core_tests\event\bad_event5::create(array('context'=>\context_system::instance()));
@$event->trigger();
$this->assertDebuggingCalled();

$event = \core_tests\event\bad_event6::create();
$event = \core_tests\event\bad_event6::create(array('context'=>\context_system::instance()));
$event->trigger();
$this->assertDebuggingCalled();

$event = \core_tests\event\bad_event7::create(array('objectid'=>1));
$event = \core_tests\event\bad_event7::create(array('objectid'=>1, 'context'=>\context_system::instance()));
try {
$event->trigger();
$this->fail('Exception expected when $data contains objectid by objecttable not specified');
Expand All @@ -582,30 +595,30 @@ public function test_bad_events() {

public function test_problematic_events() {
global $CFG;
$event1 = \core_tests\event\problematic_event1::create();
$event1 = \core_tests\event\problematic_event1::create(array('context'=>\context_system::instance()));
$this->assertDebuggingNotCalled();
$this->assertNull($event1->xxx);
$this->assertDebuggingCalled();

$event2 = \core_tests\event\problematic_event1::create(array('xxx'=>0));
$event2 = \core_tests\event\problematic_event1::create(array('xxx'=>0, 'context'=>\context_system::instance()));
$this->assertDebuggingCalled();

$CFG->debug = 0;
$event3 = \core_tests\event\problematic_event1::create(array('xxx'=>0));
$event3 = \core_tests\event\problematic_event1::create(array('xxx'=>0, 'context'=>\context_system::instance()));
$this->assertDebuggingNotCalled();
$CFG->debug = E_ALL | E_STRICT;

$event4 = \core_tests\event\problematic_event1::create(array('other'=>array('a'=>1)));
$event4 = \core_tests\event\problematic_event1::create(array('context'=>\context_system::instance(), 'other'=>array('a'=>1)));
$event4->trigger();
$this->assertDebuggingNotCalled();

$event5 = \core_tests\event\problematic_event1::create(array('other'=>(object)array('a'=>1)));
$event5 = \core_tests\event\problematic_event1::create(array('context'=>\context_system::instance(), 'other'=>(object)array('a'=>1)));
$this->assertDebuggingNotCalled();
$event5->trigger();
$this->assertDebuggingCalled();

$url = new moodle_url('/admin/');
$event6 = \core_tests\event\problematic_event1::create(array('other'=>array('a'=>$url)));
$event6 = \core_tests\event\problematic_event1::create(array('context'=>\context_system::instance(), 'other'=>array('a'=>$url)));
$this->assertDebuggingNotCalled();
$event6->trigger();
$this->assertDebuggingCalled();
Expand Down

0 comments on commit 27af3e6

Please sign in to comment.