Merge branch 'wip-MDL-33920-master' of git://github.com/marinaglancy/moodle
authorSam Hemelryk <sam@moodle.com>
Mon, 3 Feb 2014 21:47:00 +0000 (10:47 +1300)
committerSam Hemelryk <sam@moodle.com>
Mon, 3 Feb 2014 21:47:00 +0000 (10:47 +1300)
50 files changed:
admin/cli/backup.php [new file with mode: 0644]
admin/webservice/service_user_settings.php
auth/db/tests/db_test.php
auth/ldap/tests/plugin_test.php
badges/tests/badgeslib_test.php
blocks/comments/tests/events_test.php
enrol/database/tests/sync_test.php
files/tests/externallib_test.php
grade/grading/form/guide/backup/moodle2/backup_gradingform_guide_plugin.class.php
grade/grading/form/guide/backup/moodle2/restore_gradingform_guide_plugin.class.php
grade/tests/edittreelib_test.php
grade/tests/externallib_test.php
group/clientlib.js
lang/en/auth.php
lib/behat/form_field/behat_form_select.php
lib/classes/event/user_login_failed.php [new file with mode: 0644]
lib/dml/mssql_native_moodle_database.php
lib/dml/oci_native_moodle_database.php
lib/dml/pgsql_native_moodle_database.php
lib/dml/sqlite3_pdo_moodle_database.php
lib/dml/sqlsrv_native_moodle_database.php
lib/moodlelib.php
lib/pdflib.php
lib/tablelib.php
lib/tests/accesslib_test.php
lib/tests/authlib_test.php
lib/tests/completionlib_test.php
lib/tests/coursecatlib_test.php
lib/tests/messagelib_test.php
lib/tests/moodlelib_test.php
mod/assign/feedback/comments/locallib.php
mod/assign/feedback/editpdf/classes/document_services.php
mod/assign/feedback/editpdf/classes/pdf.php
mod/assign/gradingtable.php
mod/assign/locallib.php
mod/assign/submission/comments/tests/events_test.php
mod/assign/tests/behat/quickgrading.feature [new file with mode: 0644]
mod/assign/tests/externallib_test.php
mod/book/tests/events_test.php
mod/choice/tests/events_test.php
mod/data/tests/lib_test.php
mod/feedback/tests/events_test.php
mod/forum/lib.php
mod/forum/tests/externallib_test.php
mod/forum/tests/lib_test.php
mod/glossary/tests/events_test.php
mod/scorm/tests/event_test.php
mod/wiki/tests/events_test.php
question/upgrade.txt
repository/tests/repositorylib_test.php

diff --git a/admin/cli/backup.php b/admin/cli/backup.php
new file mode 100644 (file)
index 0000000..2770c03
--- /dev/null
@@ -0,0 +1,119 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * This script allows to do backup.
+ *
+ * @package    core
+ * @subpackage cli
+ * @copyright  2013 Lancaster University
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+define('CLI_SCRIPT', 1);
+
+require(dirname(dirname(dirname(__FILE__))).'/config.php');
+require_once($CFG->libdir.'/clilib.php');
+require_once($CFG->dirroot . '/backup/util/includes/backup_includes.php');
+
+// Now get cli options.
+list($options, $unrecognized) = cli_get_params(array(
+    'courseid' => false,
+    'courseshortname' => '',
+    'destination' => '',
+    'help' => false,
+    ), array('h' => 'help'));
+
+if ($unrecognized) {
+    $unrecognized = implode("\n  ", $unrecognized);
+    cli_error(get_string('cliunknowoption', 'admin', $unrecognized));
+}
+
+if ($options['help'] || !($options['courseid'] || $options['courseshortname'])) {
+    $help = <<<EOL
+Perform backup of the given course.
+
+Options:
+--courseid=INTEGER          Course ID for backup.
+--courseshortname=STRING    Course shortname for backup.
+--destination=STRING        Path where to store backup file. If not set the backup
+                            will be stored within the course backup file area.
+-h, --help                  Print out this help.
+
+Example:
+\$sudo -u www-data /usr/bin/php admin/cli/backup.php --courseid=2 --destination=/moodle/backup/\n
+EOL;
+
+    echo $help;
+    die;
+}
+
+$admin = get_admin();
+if (!$admin) {
+    mtrace("Error: No admin account was found");
+    die;
+}
+
+// Do we need to store backup somewhere else?
+$dir = rtrim($options['destination'], '/');
+if (!empty($dir)) {
+    if (!file_exists($dir) || !is_dir($dir) || !is_writable($dir)) {
+        mtrace("Destination directory does not exists or not writable.");
+        die;
+    }
+}
+
+// Check that the course exists.
+if ($options['courseid']) {
+    $course = $DB->get_record('course', array('id' => $options['courseid']), '*', MUST_EXIST);
+} else if ($options['courseshortname']) {
+    $course = $DB->get_record('course', array('shortname' => $options['courseshortname']), '*', MUST_EXIST);
+}
+
+cli_heading('Performing backup...');
+$bc = new backup_controller(backup::TYPE_1COURSE, $course->id, backup::FORMAT_MOODLE,
+                            backup::INTERACTIVE_YES, backup::MODE_GENERAL, $admin->id);
+// Set the default filename.
+$format = $bc->get_format();
+$type = $bc->get_type();
+$id = $bc->get_id();
+$users = $bc->get_plan()->get_setting('users')->get_value();
+$anonymised = $bc->get_plan()->get_setting('anonymize')->get_value();
+$filename = backup_plan_dbops::get_default_backup_filename($format, $type, $id, $users, $anonymised);
+$bc->get_plan()->get_setting('filename')->set_value($filename);
+
+// Execution.
+$bc->finish_ui();
+$bc->execute_plan();
+$results = $bc->get_results();
+$file = $results['backup_destination']; // May be empty if file already moved to target location.
+
+// Do we need to store backup somewhere else?
+if (!empty($dir)) {
+    if ($file) {
+        mtrace("Writing " . $dir.'/'.$filename);
+        if ($file->copy_content_to($dir.'/'.$filename)) {
+            $file->delete();
+            mtrace("Backup completed.");
+        } else {
+            mtrace("Destination directory does not exist or is not writable. Leaving the backup in the course backup file area.");
+        }
+    }
+} else {
+    mtrace("Backup completed, the new file is listed in the backup area of the given course");
+}
+$bc->destroy();
+exit(0);
\ No newline at end of file
index 18d0ef5..8a87fdb 100644 (file)
@@ -68,7 +68,7 @@ if ($usersettingsform->is_cancelled()) {
     //TODO: assign capability
 
     //display successful notification
-    $notification = $OUTPUT->notification(get_string('usersettingssaved', 'webservice'), 'success');
+    $notification = $OUTPUT->notification(get_string('usersettingssaved', 'webservice'), 'notifysuccess');
 }
 
 echo $OUTPUT->header();
index 97a1eca..9fccdd2 100644 (file)
@@ -28,11 +28,17 @@ defined('MOODLE_INTERNAL') || die();
 
 
 class auth_db_testcase extends advanced_testcase {
+    /** @var string Original error log */
+    protected $oldlog;
 
     protected function init_auth_database() {
         global $DB, $CFG;
         require_once("$CFG->dirroot/auth/db/auth.php");
 
+        // Discard error logs from AdoDB.
+        $this->oldlog = ini_get('error_log');
+        ini_set('error_log', "$CFG->dataroot/testlog.log");
+
         $dbman = $DB->get_manager();
 
         set_config('extencoding', 'utf-8', 'auth/db');
@@ -133,6 +139,8 @@ class auth_db_testcase extends advanced_testcase {
         $dbman = $DB->get_manager();
         $table = new xmldb_table('auth_db_users');
         $dbman->drop_table($table);
+
+        ini_set('error_log', $this->oldlog);
     }
 
     public function test_plugin() {
index 955b81e..13cdcc0 100644 (file)
@@ -268,10 +268,8 @@ class auth_ldap_plugin_testcase extends advanced_testcase {
         $sink->close();
 
         // Check that the event is valid.
-        $this->assertCount(2, $events);
-        $event = $events[0];
-        $this->assertInstanceOf('\core\event\user_updated', $event);
-        $event = $events[1];
+        $this->assertCount(1, $events);
+        $event = reset($events);
         $this->assertInstanceOf('\core\event\user_loggedin', $event);
         $this->assertEquals('user', $event->objecttable);
         $this->assertEquals('2', $event->objectid);
index 97b3271..aa074b4 100644 (file)
@@ -233,7 +233,7 @@ class core_badges_badgeslib_testcase extends advanced_testcase {
         $criteria_overall = award_criteria::build(array('criteriatype' => BADGE_CRITERIA_TYPE_OVERALL, 'badgeid' => $badge->id));
         $criteria_overall->save(array('agg' => BADGE_CRITERIA_AGGREGATION_ANY));
         $criteria_overall = award_criteria::build(array('criteriatype' => BADGE_CRITERIA_TYPE_ACTIVITY, 'badgeid' => $badge->id));
-        $criteria_overall->save(array('agg' => BADGE_CRITERIA_AGGREGATION_ANY, 'module_'.$this->module->id => $this->module->id));
+        $criteria_overall->save(array('agg' => BADGE_CRITERIA_AGGREGATION_ANY, 'module_'.$this->module->cmid => $this->module->cmid));
 
         // Set completion for forum activity.
         $c = new completion_info($this->course);
index cb4a1c5..1e7aae4 100644 (file)
@@ -88,7 +88,7 @@ class block_comments_events_testcase extends advanced_testcase {
         $this->assertEquals($url, $event->get_url());
 
         // Comments when block is on module (wiki) page.
-        $context = context_module::instance($this->wiki->id);
+        $context = context_module::instance($this->wiki->cmid);
         $args = new stdClass;
         $args->context   = $context;
         $args->course    = $this->course;
@@ -111,7 +111,7 @@ class block_comments_events_testcase extends advanced_testcase {
         // Checking that the event contains the expected values.
         $this->assertInstanceOf('\block_comments\event\comment_created', $event);
         $this->assertEquals($context, $event->get_context());
-        $url = new moodle_url('/mod/wiki/view.php', array('id' => $this->wiki->id));
+        $url = new moodle_url('/mod/wiki/view.php', array('id' => $this->wiki->cmid));
         $this->assertEquals($url, $event->get_url());
         $this->assertEventContextNotUsed($event);
     }
@@ -153,7 +153,7 @@ class block_comments_events_testcase extends advanced_testcase {
         $this->assertEquals($url, $event->get_url());
 
         // Comments when block is on module (wiki) page.
-        $context = context_module::instance($this->wiki->id);
+        $context = context_module::instance($this->wiki->cmid);
         $args = new stdClass;
         $args->context   = $context;
         $args->course    = $this->course;
@@ -177,7 +177,7 @@ class block_comments_events_testcase extends advanced_testcase {
         // Checking that the event contains the expected values.
         $this->assertInstanceOf('\block_comments\event\comment_deleted', $event);
         $this->assertEquals($context, $event->get_context());
-        $url = new moodle_url('/mod/wiki/view.php', array('id' => $this->wiki->id));
+        $url = new moodle_url('/mod/wiki/view.php', array('id' => $this->wiki->cmid));
         $this->assertEquals($url, $event->get_url());
         $this->assertEventContextNotUsed($event);
     }
index e46d1b1..9e73cc9 100644 (file)
@@ -31,9 +31,16 @@ class enrol_database_testcase extends advanced_testcase {
     protected static $users = array();
     protected static $roles = array();
 
+    /** @var string Original error log */
+    protected $oldlog;
+
     protected function init_enrol_database() {
         global $DB, $CFG;
 
+        // Discard error logs from AdoDB.
+        $this->oldlog = ini_get('error_log');
+        ini_set('error_log', "$CFG->dataroot/testlog.log");
+
         $dbman = $DB->get_manager();
 
         set_config('dbencoding', 'utf-8', 'enrol_database');
@@ -160,6 +167,8 @@ class enrol_database_testcase extends advanced_testcase {
         self::$courses = null;
         self::$users = null;
         self::$roles = null;
+
+        ini_set('error_log', $this->oldlog);
     }
 
     protected function reset_enrol_database() {
index 36383fb..bf34e14 100644 (file)
@@ -223,7 +223,7 @@ class core_files_externallib_testcase extends advanced_testcase {
         // Insert the information about the file.
         $contentid = $DB->insert_record('data_content', $datacontent);
         // Required information for uploading a file.
-        $context = context_module::instance($module->id);
+        $context = context_module::instance($module->cmid);
         $usercontext = context_user::instance($USER->id);
         $component = 'mod_data';
         $filearea = 'content';
@@ -301,7 +301,7 @@ class core_files_externallib_testcase extends advanced_testcase {
         $modified = 0;
         // Context level and instance ID are used to determine what the context is.
         $contextlevel = 'module';
-        $instanceid = $module->id;
+        $instanceid = $module->cmid;
         $testfilelisting = core_files_external::get_files($nocontext, $component, $filearea, $itemid, '/', $filename, $modified, $contextlevel, $instanceid);
         $this->assertEquals($testfilelisting, $testdata);
     }
index 75af83a..6ca09c9 100644 (file)
@@ -65,7 +65,7 @@ class backup_gradingform_guide_plugin extends backup_gradingform_plugin {
 
         $pluginwrapper->add_child($criteria);
         $criteria->add_child($criterion);
-        $criteria->add_child($comments);
+        $pluginwrapper->add_child($comments);
         $comments->add_child($comment);
 
         // Set sources to populate the data.
index dd99814..f833404 100644 (file)
@@ -48,6 +48,11 @@ class restore_gradingform_guide_plugin extends restore_gradingform_plugin {
         $paths[] = new restore_path_element('gradingform_guide_comment',
             $this->get_pathfor('/guidecomments/guidecomment'));
 
+        // MDL-37714: Correctly locate frequent used comments in both the
+        // current and incorrect old format.
+        $paths[] = new restore_path_element('gradingform_guide_comment_legacy',
+            $this->get_pathfor('/guidecriteria/guidecomments/guidecomment'));
+
         return $paths;
     }
 
@@ -99,6 +104,20 @@ class restore_gradingform_guide_plugin extends restore_gradingform_plugin {
         $DB->insert_record('gradingform_guide_comments', $data);
     }
 
+    /**
+     * Processes comments element data
+     *
+     * @param array|stdClass $data The data to insert as a comment
+     */
+    public function process_gradingform_guide_comment_legacy($data) {
+        global $DB;
+
+        $data = (object)$data;
+        $data->definitionid = $this->get_new_parentid('grading_definition');
+
+        $DB->insert_record('gradingform_guide_comments', $data);
+    }
+
     /**
      * Processes filling element data
      *
index f3d768c..051c155 100644 (file)
@@ -52,7 +52,7 @@ class core_grade_edittreelib_testcase extends advanced_testcase {
         $scale = $this->getDataGenerator()->create_scale();
         $course = $this->getDataGenerator()->create_course();
         $assign = $this->getDataGenerator()->create_module('assign', array('course' => $course->id));
-        $modulecontext = context_module::instance($assign->id);
+        $modulecontext = context_module::instance($assign->cmid);
         // The generator returns a dummy object, lets get the real assign object.
         $assign = new assign($modulecontext, false, false);
         $cm = $assign->get_course_module();
index 3230236..e341297 100644 (file)
@@ -66,7 +66,7 @@ class core_grading_externallib_testcase extends externallib_advanced_testcase {
         // Create a teacher and give them capabilities.
         $coursecontext = context_course::instance($course->id);
         $roleid = $this->assignUserCapability('moodle/course:viewparticipants', $coursecontext->id, 3);
-        $modulecontext = context_module::instance($cm->id);
+        $modulecontext = context_module::instance($cm->cmid);
         $this->assignUserCapability('mod/assign:grade', $modulecontext->id, $roleid);
 
         // Create the teacher's enrolment record.
@@ -146,7 +146,7 @@ class core_grading_externallib_testcase extends externallib_advanced_testcase {
         $DB->insert_record('gradingform_rubric_levels', $rubriclevel2);
 
         // Call the external function.
-        $cmids = array ($cm->id);
+        $cmids = array ($cm->cmid);
         $areaname = 'submissions';
         $result = core_grading_external::get_definitions($cmids, $areaname);
 
@@ -209,7 +209,7 @@ class core_grading_externallib_testcase extends externallib_advanced_testcase {
         // Create a teacher and give them capabilities.
         $coursecontext = context_course::instance($course->id);
         $roleid = $this->assignUserCapability('moodle/course:viewparticipants', $coursecontext->id, 3);
-        $modulecontext = context_module::instance($assign->id);
+        $modulecontext = context_module::instance($assign->cmid);
         $this->assignUserCapability('mod/assign:grade', $modulecontext->id, $roleid);
 
         // Create the teacher's enrolment record.
index fef76b1..ca22495 100644 (file)
@@ -64,22 +64,6 @@ function UpdatableGroupsCombo(wwwRoot, courseId) {
         }
 
     };
-
-    // Add onchange event to groups list box.
-    // Okay, this is not working in IE. The onchange is never fired...
-    // I'm hard coding the onchange in ../index.php. Not ideal, but it works
-    // then. vyshane AT moodle DOT com.
-    /*
-    groupsComboEl = document.getElementById("groups");
-    if (groupsComboEl) {
-        groupsComboEl.setAttribute("onchange", "membersCombo.refreshMembers(this.options[this.selectedIndex].value);");
-    }
-    */
-
-    // Hide the updategroups input since AJAX will take care of this.
-    YUI().use('yui2-dom', function (Y) {
-        Y.YUI2.util.Dom.setStyle("updategroups", "display", "none");
-    });
 }
 
 
@@ -131,9 +115,10 @@ function UpdatableMembersCombo(wwwRoot, courseId) {
     };
 
     // Hide the updatemembers input since AJAX will take care of this.
-    YUI().use('yui2-dom', function (Y) {
-        Y.YUI2.util.Dom.setStyle("updatemembers", "display", "none");
-    });
+    var updatemembers = Y.one('#updatemembers');
+    if (updatemembers) {
+        updatemembers.hide();
+    }
 }
 
 /**
index 6e8a439..1d73f7e 100644 (file)
@@ -83,6 +83,7 @@ $string['errorminpasswordupper'] = 'Passwords must have at least {$a} upper case
 $string['errorpasswordupdate'] = 'Error updating password, password not changed';
 $string['event_user_loggedin'] = 'User has logged in';
 $string['eventuserloggedinas'] = 'User logged in as another user';
+$string['eventuserloginfailed'] = 'User login failed';
 $string['forcechangepassword'] = 'Force change password';
 $string['forcechangepasswordfirst_help'] = 'Force users to change password on their first login to Moodle.';
 $string['forcechangepassword_help'] = 'Force users to change password on their next login to Moodle.';
index dce4028..208b95a 100644 (file)
@@ -94,8 +94,23 @@ class behat_form_select extends behat_form_field {
             return;
         }
 
+        // Wrapped in try & catch as the element may disappear if an AJAX request was submitted.
+        try {
+            $multiple = $this->field->hasAttribute('multiple');
+        } catch (Exception $e) {
+            // We do not specify any specific Exception type as there are
+            // different exceptions that can be thrown by the driver and
+            // we can not control them all, also depending on the selenium
+            // version the exception type can change.
+            return;
+        }
+
+        // Wait for all the possible AJAX requests that have been
+        // already triggered by selectOption() to be finished.
+        $this->session->wait(behat_base::TIMEOUT * 1000, behat_base::PAGE_READY_JS);
+
         // Single select sometimes needs an extra click in the option.
-        if (!$this->field->hasAttribute('multiple')) {
+        if (!$multiple) {
 
             // Using the driver direcly because Element methods are messy when dealing
             // with elements inside containers.
@@ -104,11 +119,6 @@ class behat_form_select extends behat_form_field {
                 // Wrapped in a try & catch as we can fall into race conditions
                 // and the element may not be there.
                 try {
-
-                    // Wait for all the possible AJAX requests that have been
-                    // already triggered by selectOption() to be finished.
-                    $this->session->wait(behat_base::TIMEOUT * 1000, behat_base::PAGE_READY_JS);
-
                     current($optionnodes)->click();
                 } catch (Exception $e) {
                     // We continue and return as this means that the element is not there or it is not the same.
@@ -118,10 +128,6 @@ class behat_form_select extends behat_form_field {
 
         } else {
 
-            // Wait for all the possible AJAX requests that have been
-            // already triggered by selectOption() to be finished.
-            $this->session->wait(behat_base::TIMEOUT * 1000, behat_base::PAGE_READY_JS);
-
             // Wrapped in a try & catch as we can fall into race conditions
             // and the element may not be there.
             try {
diff --git a/lib/classes/event/user_login_failed.php b/lib/classes/event/user_login_failed.php
new file mode 100644 (file)
index 0000000..6533a10
--- /dev/null
@@ -0,0 +1,109 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * User login failed event.
+ *
+ * @package    core
+ * @copyright  2014 Rajesh Taneja <rajesh@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace core\event;
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * User login failed event class.
+ *
+ * @property-read array $other {
+ *      Extra information about event.
+ *
+ *      @type string username name of user.
+ *      @type int reason failure reason.
+ * }
+ *
+ * @package    core
+ * @copyright  2014 Rajesh Taneja <rajesh@moodle.com>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class user_login_failed extends \core\event\base {
+    /**
+     * Init method.
+     *
+     * @return void
+     */
+    protected function init() {
+        $this->context = \context_system::instance();
+        $this->data['crud'] = 'r';
+        $this->data['edulevel'] = self::LEVEL_OTHER;
+    }
+
+    /**
+     * Return localised event name.
+     *
+     * @return string
+     */
+    public static function get_name() {
+        return get_string('eventuserloginfailed', 'auth');
+    }
+
+    /**
+     * Returns non-localised event description with id's for admin use only.
+     *
+     * @return string
+     */
+    public function get_description() {
+        return 'Login failed for user "' . $this->other['username'] . '" for reason id: ' . $this->other['reason'];
+    }
+
+    /**
+     * Get URL related to the action.
+     *
+     * @return \moodle_url
+     */
+    public function get_url() {
+        if (isset($this->data['userid'])) {
+            return new \moodle_url('/user/profile.php', array('id' => $this->data['userid']));
+        } else {
+            return null;
+        }
+    }
+
+    /**
+     * Return legacy data for add_to_log().
+     *
+     * @return array
+     */
+    protected function get_legacy_logdata() {
+        return array(SITEID, 'login', 'error', 'index.php', $this->other['username']);
+    }
+
+    /**
+     * Custom validation.
+     *
+     * @throws \coding_exception when validation does not pass.
+     * @return void
+     */
+    protected function validate_data() {
+        if (!isset($this->other['reason'])) {
+            throw new \coding_exception("other['reason'] has to be specified.");
+        } else if (!isset($this->other['username'])) {
+            throw new \coding_exception("other['username'] has to be specified.");
+        }
+    }
+
+}
index 9b8e98f..1360898 100644 (file)
@@ -327,7 +327,7 @@ class mssql_native_moodle_database extends moodle_database {
         if ($result) {
             while ($row = mssql_fetch_row($result)) {
                 $tablename = reset($row);
-                if ($this->prefix !== '') {
+                if ($this->prefix !== false && $this->prefix !== '') {
                     if (strpos($tablename, $this->prefix) !== 0) {
                         continue;
                     }
index 8e99da4..fed5c85 100644 (file)
@@ -406,7 +406,7 @@ class oci_native_moodle_database extends moodle_database {
         oci_free_statement($stmt);
         $records = array_map('strtolower', $records['TABLE_NAME']);
         foreach ($records as $tablename) {
-            if ($this->prefix !== '') {
+            if ($this->prefix !== false && $this->prefix !== '') {
                 if (strpos($tablename, $this->prefix) !== 0) {
                     continue;
                 }
index 14b2cc3..717e632 100644 (file)
@@ -321,7 +321,7 @@ class pgsql_native_moodle_database extends moodle_database {
         if ($result) {
             while ($row = pg_fetch_row($result)) {
                 $tablename = reset($row);
-                if ($this->prefix !== '') {
+                if ($this->prefix !== false && $this->prefix !== '') {
                     if (strpos($tablename, $this->prefix) !== 0) {
                         continue;
                     }
index 7828eb7..fcfe446 100644 (file)
@@ -150,7 +150,7 @@ class sqlite3_pdo_moodle_database extends pdo_moodle_database {
         foreach ($rstables as $table) {
             $table = $table['name'];
             $table = strtolower($table);
-            if ($this->prefix !== '') {
+            if ($this->prefix !== false && $this->prefix !== '') {
                 if (strpos($table, $this->prefix) !== 0) {
                     continue;
                 }
index 5110a32..eb4569f 100644 (file)
@@ -380,7 +380,7 @@ class sqlsrv_native_moodle_database extends moodle_database {
         if ($result) {
             while ($row = sqlsrv_fetch_array($result)) {
                 $tablename = reset($row);
-                if ($this->prefix !== '') {
+                if ($this->prefix !== false && $this->prefix !== '') {
                     if (strpos($tablename, $this->prefix) !== 0) {
                         continue;
                     }
index 05c7e16..39cfd10 100644 (file)
@@ -3397,9 +3397,7 @@ function get_user_key($script, $userid, $instance=null, $iprestriction=null, $va
  * @return bool Always returns true
  */
 function update_user_login_times() {
-    global $USER, $DB, $CFG;
-
-    require_once($CFG->dirroot.'/user/lib.php');
+    global $USER, $DB;
 
     if (isguestuser()) {
         // Do not update guest access times/ips for performance.
@@ -3425,7 +3423,9 @@ function update_user_login_times() {
     $USER->lastaccess = $user->lastaccess = $now;
     $USER->lastip = $user->lastip = getremoteaddr();
 
-    user_update_user($user, false);
+    // Note: do not call user_update_user() here because this is part of the login process,
+    //       the login event means that these fields were updated.
+    $DB->update_record('user', $user);
     return true;
 }
 
@@ -4338,16 +4338,24 @@ function authenticate_user_login($username, $password, $ignorelockout=false, &$f
         // Use manual if auth not set.
         $auth = empty($user->auth) ? 'manual' : $user->auth;
         if (!empty($user->suspended)) {
-            add_to_log(SITEID, 'login', 'error', 'index.php', $username);
-            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Suspended Login:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             $failurereason = AUTH_LOGIN_SUSPENDED;
+
+            // Trigger login failed event.
+            $event = \core\event\user_login_failed::create(array('userid' => $user->id,
+                    'other' => array('username' => $username, 'reason' => $failurereason)));
+            $event->trigger();
+            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Suspended Login:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             return false;
         }
         if ($auth=='nologin' or !is_enabled_auth($auth)) {
-            add_to_log(SITEID, 'login', 'error', 'index.php', $username);
-            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Disabled Login:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             // Legacy way to suspend user.
             $failurereason = AUTH_LOGIN_SUSPENDED;
+
+            // Trigger login failed event.
+            $event = \core\event\user_login_failed::create(array('userid' => $user->id,
+                    'other' => array('username' => $username, 'reason' => $failurereason)));
+            $event->trigger();
+            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Disabled Login:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             return false;
         }
         $auths = array($auth);
@@ -4355,16 +4363,27 @@ function authenticate_user_login($username, $password, $ignorelockout=false, &$f
     } else {
         // Check if there's a deleted record (cheaply), this should not happen because we mangle usernames in delete_user().
         if ($DB->get_field('user', 'id', array('username' => $username, 'mnethostid' => $CFG->mnet_localhost_id,  'deleted' => 1))) {
-            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Deleted Login:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             $failurereason = AUTH_LOGIN_NOUSER;
+
+            // Trigger login failed event.
+            $event = \core\event\user_login_failed::create(array('other' => array('username' => $username,
+                    'reason' => $failurereason)));
+            $event->trigger();
+            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Deleted Login:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             return false;
         }
 
         // Do not try to authenticate non-existent accounts when user creation is not disabled.
         if (!empty($CFG->authpreventaccountcreation)) {
-            add_to_log(SITEID, 'login', 'error', 'index.php', $username);
-            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Unknown user, can not create new accounts:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             $failurereason = AUTH_LOGIN_NOUSER;
+
+            // Trigger login failed event.
+            $event = \core\event\user_login_failed::create(array('other' => array('username' => $username,
+                    'reason' => $failurereason)));
+            $event->trigger();
+
+            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Unknown user, can not create new accounts:  $username  ".
+                    $_SERVER['HTTP_USER_AGENT']);
             return false;
         }
 
@@ -4380,9 +4399,14 @@ function authenticate_user_login($username, $password, $ignorelockout=false, &$f
     } else if ($user->id) {
         // Verify login lockout after other ways that may prevent user login.
         if (login_is_lockedout($user)) {
-            add_to_log(SITEID, 'login', 'error', 'index.php', $username);
-            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Login lockout:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             $failurereason = AUTH_LOGIN_LOCKOUT;
+
+            // Trigger login failed event.
+            $event = \core\event\user_login_failed::create(array('userid' => $user->id,
+                    'other' => array('username' => $username, 'reason' => $failurereason)));
+            $event->trigger();
+
+            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Login lockout:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             return false;
         }
     } else {
@@ -4428,14 +4452,21 @@ function authenticate_user_login($username, $password, $ignorelockout=false, &$f
 
         if (empty($user->id)) {
             $failurereason = AUTH_LOGIN_NOUSER;
+            // Trigger login failed event.
+            $event = \core\event\user_login_failed::create(array('other' => array('username' => $username,
+                    'reason' => $failurereason)));
+            $event->trigger();
             return false;
         }
 
         if (!empty($user->suspended)) {
             // Just in case some auth plugin suspended account.
-            add_to_log(SITEID, 'login', 'error', 'index.php', $username);
-            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Suspended Login:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             $failurereason = AUTH_LOGIN_SUSPENDED;
+            // Trigger login failed event.
+            $event = \core\event\user_login_failed::create(array('userid' => $user->id,
+                    'other' => array('username' => $username, 'reason' => $failurereason)));
+            $event->trigger();
+            error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Suspended Login:  $username  ".$_SERVER['HTTP_USER_AGENT']);
             return false;
         }
 
@@ -4445,7 +4476,6 @@ function authenticate_user_login($username, $password, $ignorelockout=false, &$f
     }
 
     // Failed if all the plugins have failed.
-    add_to_log(SITEID, 'login', 'error', 'index.php', $username);
     if (debugging('', DEBUG_ALL)) {
         error_log('[client '.getremoteaddr()."]  $CFG->wwwroot  Failed Login:  $username  ".$_SERVER['HTTP_USER_AGENT']);
     }
@@ -4453,8 +4483,16 @@ function authenticate_user_login($username, $password, $ignorelockout=false, &$f
     if ($user->id) {
         login_attempt_failed($user);
         $failurereason = AUTH_LOGIN_FAILED;
+        // Trigger login failed event.
+        $event = \core\event\user_login_failed::create(array('userid' => $user->id,
+                'other' => array('username' => $username, 'reason' => $failurereason)));
+        $event->trigger();
     } else {
         $failurereason = AUTH_LOGIN_NOUSER;
+        // Trigger login failed event.
+        $event = \core\event\user_login_failed::create(array('other' => array('username' => $username,
+                'reason' => $failurereason)));
+        $event->trigger();
     }
 
     return false;
index 3b72e6e..309698e 100644 (file)
@@ -81,6 +81,9 @@ define('K_CELL_HEIGHT_RATIO', 1.25);
 /** reduction factor for small font */
 define('K_SMALL_RATIO', 2/3);
 
+/** Throw exceptions from errors so they can be caught and recovered from. */
+define('K_TCPDF_THROW_EXCEPTION_ERROR', true);
+
 require_once(dirname(__FILE__).'/tcpdf/tcpdf.php');
 
 /**
index 657c72e..4e0058c 100644 (file)
@@ -797,7 +797,7 @@ class flexible_table {
             }
             return format_text($text, $format, $options);
         } else {
-            $eci =& $this->export_class_instance();
+            $eci = $this->export_class_instance();
             return $eci->format_text($text, $format, $options, $courseid);
         }
     }
@@ -1527,9 +1527,9 @@ class table_spreadsheet_export_format_parent extends table_default_export_format
         $filename = $filename.'.'.$this->fileextension;
         $this->define_workbook();
         // format types
-        $this->formatnormal =& $this->workbook->add_format();
+        $this->formatnormal = $this->workbook->add_format();
         $this->formatnormal->set_bold(0);
-        $this->formatheaders =& $this->workbook->add_format();
+        $this->formatheaders = $this->workbook->add_format();
         $this->formatheaders->set_bold(1);
         $this->formatheaders->set_align('center');
         // Sending HTTP headers
index 6a9f8d6..ac7cc06 100644 (file)
@@ -60,7 +60,7 @@ class core_accesslib_testcase extends advanced_testcase {
         $this->assertNotEmpty($ACCESSLIB_PRIVATE->rolepermissions);
         $this->assertNotEmpty($ACCESSLIB_PRIVATE->rolepermissions);
         $this->assertNotEmpty($ACCESSLIB_PRIVATE->accessdatabyuser);
-        accesslib_clear_all_caches(true);
+        accesslib_clear_all_caches_for_unit_testing();
         $this->assertEmpty($ACCESSLIB_PRIVATE->rolepermissions);
         $this->assertEmpty($ACCESSLIB_PRIVATE->rolepermissions);
         $this->assertEmpty($ACCESSLIB_PRIVATE->dirtycontexts);
@@ -2095,7 +2095,7 @@ class core_accesslib_testcase extends advanced_testcase {
         unassign_capability('moodle/site:accessallgroups', $allroles['teacher'], $frontpagecontext->id, true);
         unset($rc);
 
-        accesslib_clear_all_caches(false); // Must be done after assign_capability().
+        accesslib_clear_all_caches_for_unit_testing(); // Must be done after assign_capability().
 
 
         // Test role_assign(), role_unassign(), role_unassign_all() functions.
@@ -2112,7 +2112,7 @@ class core_accesslib_testcase extends advanced_testcase {
         $this->assertEquals(0, $DB->count_records('role_assignments', array('contextid'=>$context->id)));
         unset($context);
 
-        accesslib_clear_all_caches(false); // Just in case.
+        accesslib_clear_all_caches_for_unit_testing(); // Just in case.
 
 
         // Test has_capability(), get_users_by_capability(), role_switch(), reload_all_capabilities() and friends functions.
@@ -2173,7 +2173,7 @@ class core_accesslib_testcase extends advanced_testcase {
 
         assign_capability('mod/page:view', CAP_PREVENT, $allroles['guest'], $systemcontext, true);
 
-        accesslib_clear_all_caches(false); // Must be done after assign_capability().
+        accesslib_clear_all_caches_for_unit_testing(); /// Must be done after assign_capability().
 
         // Extra tests for guests and not-logged-in users because they can not be verified by cross checking
         // with get_users_by_capability() where they are ignored.
@@ -2296,7 +2296,7 @@ class core_accesslib_testcase extends advanced_testcase {
         unset($permissions);
         unset($roles);
 
-        accesslib_clear_all_caches(false); // Must be done after assign_capability().
+        accesslib_clear_all_caches_for_unit_testing(); // must be done after assign_capability().
 
         // Test time - let's set up some real user, just in case the logic for USER affects the others...
         $USER = $DB->get_record('user', array('id'=>$testusers[3]));
index be7666f..2a1a05e 100644 (file)
@@ -133,35 +133,104 @@ class core_authlib_testcase extends advanced_testcase {
         $user1 = $this->getDataGenerator()->create_user(array('username'=>'username1', 'password'=>'password1'));
         $user2 = $this->getDataGenerator()->create_user(array('username'=>'username2', 'password'=>'password2', 'suspended'=>1));
         $user3 = $this->getDataGenerator()->create_user(array('username'=>'username3', 'password'=>'password3', 'auth'=>'nologin'));
-
+        // Capture events.
+        $sink = $this->redirectEvents();
         $result = authenticate_user_login('username1', 'password1');
+        $events = $sink->get_events();
+        $sink->close();
+
+        // No event is triggred.
+        $this->assertEmpty($events);
         $this->assertInstanceOf('stdClass', $result);
         $this->assertEquals($user1->id, $result->id);
 
         $reason = null;
+        // Capture event.
+        $sink = $this->redirectEvents();
         $result = authenticate_user_login('username1', 'password1', false, $reason);
+        $events = $sink->get_events();
+        $sink->close();
+
+        // No event is triggred.
+        $this->assertEmpty($events);
         $this->assertInstanceOf('stdClass', $result);
         $this->assertEquals(AUTH_LOGIN_OK, $reason);
 
         $reason = null;
+        // Capture failed login event.
+        $sink = $this->redirectEvents();
         $result = authenticate_user_login('username1', 'nopass', false, $reason);
+        $events = $sink->get_events();
+        $sink->close();
+        $event = array_pop($events);
+
         $this->assertFalse($result);
         $this->assertEquals(AUTH_LOGIN_FAILED, $reason);
+        // Test Event.
+        $this->assertInstanceOf('\core\event\user_login_failed', $event);
+        $expectedlogdata = array(SITEID, 'login', 'error', 'index.php', 'username1');
+        $this->assertEventLegacyLogData($expectedlogdata, $event);
+        $eventdata = $event->get_data();
+        $this->assertSame($eventdata['other']['username'], 'username1');
+        $this->assertSame($eventdata['other']['reason'], AUTH_LOGIN_FAILED);
+        $this->assertEventContextNotUsed($event);
 
         $reason = null;
+        // Capture failed login event.
+        $sink = $this->redirectEvents();
         $result = authenticate_user_login('username2', 'password2', false, $reason);
+        $events = $sink->get_events();
+        $sink->close();
+        $event = array_pop($events);
+
         $this->assertFalse($result);
         $this->assertEquals(AUTH_LOGIN_SUSPENDED, $reason);
+        // Test Event.
+        $this->assertInstanceOf('\core\event\user_login_failed', $event);
+        $expectedlogdata = array(SITEID, 'login', 'error', 'index.php', 'username2');
+        $this->assertEventLegacyLogData($expectedlogdata, $event);
+        $eventdata = $event->get_data();
+        $this->assertSame($eventdata['other']['username'], 'username2');
+        $this->assertSame($eventdata['other']['reason'], AUTH_LOGIN_SUSPENDED);
+        $this->assertEventContextNotUsed($event);
 
         $reason = null;
+        // Capture failed login event.
+        $sink = $this->redirectEvents();
         $result = authenticate_user_login('username3', 'password3', false, $reason);
+        $events = $sink->get_events();
+        $sink->close();
+        $event = array_pop($events);
+
         $this->assertFalse($result);
         $this->assertEquals(AUTH_LOGIN_SUSPENDED, $reason);
+        // Test Event.
+        $this->assertInstanceOf('\core\event\user_login_failed', $event);
+        $expectedlogdata = array(SITEID, 'login', 'error', 'index.php', 'username3');
+        $this->assertEventLegacyLogData($expectedlogdata, $event);
+        $eventdata = $event->get_data();
+        $this->assertSame($eventdata['other']['username'], 'username3');
+        $this->assertSame($eventdata['other']['reason'], AUTH_LOGIN_SUSPENDED);
+        $this->assertEventContextNotUsed($event);
 
         $reason = null;
+        // Capture failed login event.
+        $sink = $this->redirectEvents();
         $result = authenticate_user_login('username4', 'password3', false, $reason);
+        $events = $sink->get_events();
+        $sink->close();
+        $event = array_pop($events);
+
         $this->assertFalse($result);
         $this->assertEquals(AUTH_LOGIN_NOUSER, $reason);
+        // Test Event.
+        $this->assertInstanceOf('\core\event\user_login_failed', $event);
+        $expectedlogdata = array(SITEID, 'login', 'error', 'index.php', 'username4');
+        $this->assertEventLegacyLogData($expectedlogdata, $event);
+        $eventdata = $event->get_data();
+        $this->assertSame($eventdata['other']['username'], 'username4');
+        $this->assertSame($eventdata['other']['reason'], AUTH_LOGIN_NOUSER);
+        $this->assertEventContextNotUsed($event);
 
         set_config('lockoutthreshold', 3);
 
index 63d67d6..23d7770 100644 (file)
@@ -812,7 +812,7 @@ class core_completionlib_testcase extends advanced_testcase {
         $this->assertInstanceOf('\core\event\course_module_completion_updated', $event);
         $this->assertEquals($forum->cmid, $event->get_record_snapshot('course_modules_completion', $event->objectid)->coursemoduleid);
         $this->assertEquals($current, $event->get_record_snapshot('course_modules_completion', $event->objectid));
-        $this->assertEquals(context_module::instance($forum->id), $event->get_context());
+        $this->assertEquals(context_module::instance($forum->cmid), $event->get_context());
         $this->assertEquals($USER->id, $event->userid);
         $this->assertEquals($this->user->id, $event->other['relateduserid']);
         $this->assertInstanceOf('moodle_url', $event->get_url());
index 61b3b65..b4b8180 100644 (file)
@@ -370,16 +370,18 @@ class core_coursecatlib_testcase extends advanced_testcase {
      * Test the countall function
      */
     public function test_count_all() {
-        // There should be just the default category.
-        $this->assertEquals(1, coursecat::count_all());
+        global $DB;
+        // Dont assume there is just one. An add-on might create a category as part of the install.
+        $numcategories = $DB->count_records('course_categories');
+        $this->assertEquals($numcategories, coursecat::count_all());
         $category1 = coursecat::create(array('name' => 'Cat1'));
         $category2 = coursecat::create(array('name' => 'Cat2', 'parent' => $category1->id));
         $category3 = coursecat::create(array('name' => 'Cat3', 'parent' => $category2->id, 'visible' => 0));
-        // Now we've got four.
-        $this->assertEquals(4, coursecat::count_all());
+        // Now we've got three more.
+        $this->assertEquals($numcategories + 3, coursecat::count_all());
         cache_helper::purge_by_event('changesincoursecat');
         // We should still have 4.
-        $this->assertEquals(4, coursecat::count_all());
+        $this->assertEquals($numcategories + 3, coursecat::count_all());
     }
 
     /**
index 46c710b..c279e28 100644 (file)
@@ -134,7 +134,7 @@ class core_messagelib_testcase extends advanced_testcase {
         // however mod_quiz doesn't have a data generator.
         // Instead we're going to use backup notifications and give and take away the capability at various levels.
         $assign = $this->getDataGenerator()->create_module('assign', array('course'=>$course->id));
-        $modulecontext = context_module::instance($assign->id);
+        $modulecontext = context_module::instance($assign->cmid);
 
         // Create and enrol a teacher.
         $teacherrole = $DB->get_record('role', array('shortname'=>'editingteacher'), '*', MUST_EXIST);
@@ -162,7 +162,7 @@ class core_messagelib_testcase extends advanced_testcase {
         // They should now be able to see the backup message.
         assign_capability('moodle/site:config', CAP_ALLOW, $teacherrole->id, $modulecontext->id, true);
         accesslib_clear_all_caches_for_unit_testing();
-        $modulecontext = context_module::instance($assign->id);
+        $modulecontext = context_module::instance($assign->cmid);
         $this->assertTrue(has_capability('moodle/site:config', $modulecontext));
 
         $providers = message_get_providers_for_user($teacher->id);
@@ -173,7 +173,7 @@ class core_messagelib_testcase extends advanced_testcase {
         // They should not be able to see the backup message.
         assign_capability('moodle/site:config', CAP_PROHIBIT, $teacherrole->id, $coursecontext->id, true);
         accesslib_clear_all_caches_for_unit_testing();
-        $modulecontext = context_module::instance($assign->id);
+        $modulecontext = context_module::instance($assign->cmid);
         $this->assertFalse(has_capability('moodle/site:config', $modulecontext));
 
         $providers = message_get_providers_for_user($teacher->id);
index dbb4b80..2e73ea2 100644 (file)
@@ -2452,10 +2452,8 @@ class core_moodlelib_testcase extends advanced_testcase {
         $events = $sink->get_events();
         $sink->close();
 
-        $this->assertCount(2, $events);
-        $event = $events[0];
-        $this->assertInstanceOf('\core\event\user_updated', $event);
-        $event = $events[1];
+        $this->assertCount(1, $events);
+        $event = reset($events);
         $this->assertInstanceOf('\core\event\user_loggedin', $event);
         $this->assertEquals('user', $event->objecttable);
         $this->assertEquals($user->id, $event->objectid);
@@ -2470,7 +2468,6 @@ class core_moodlelib_testcase extends advanced_testcase {
 
         $this->assertTimeCurrent($USER->firstaccess);
         $this->assertTimeCurrent($USER->lastaccess);
-        $this->assertTimeCurrent($USER->timemodified);
         $this->assertTimeCurrent($USER->currentlogin);
         $this->assertSame(sesskey(), $USER->sesskey);
         $this->assertTimeCurrent($USER->preference['_lastloaded']);
index 96dde23..e0ea32e 100644 (file)
@@ -94,7 +94,10 @@ class assign_feedback_comments extends assign_feedback_plugin {
                 $commenttext = $feedbackcomments->commenttext;
             }
         }
-        return optional_param('quickgrade_comments_' . $userid, '', PARAM_TEXT) != $commenttext;
+        // Note that this handles the difference between empty and not in the quickgrading
+        // form at all (hidden column).
+        $newvalue = optional_param('quickgrade_comments_' . $userid, false, PARAM_TEXT);
+        return ($newvalue !== false) && ($newvalue != $commenttext);
     }
 
 
@@ -173,6 +176,11 @@ class assign_feedback_comments extends assign_feedback_plugin {
     public function save_quickgrading_changes($userid, $grade) {
         global $DB;
         $feedbackcomment = $this->get_feedback_comments($grade->id);
+        $feedbackpresent = optional_param('quickgrade_comments_' . $userid, false, PARAM_TEXT) !== false;
+        if (!$feedbackpresent) {
+            // Nothing to save (e.g. hidden column).
+            return true;
+        }
         if ($feedbackcomment) {
             $feedbackcomment->commenttext = optional_param('quickgrade_comments_' . $userid, '', PARAM_TEXT);
             return $DB->update_record('assignfeedback_comments', $feedbackcomment);
index 3117df0..468b103 100644 (file)
@@ -207,9 +207,17 @@ class document_services {
             $tmpfile = $tmpdir . '/' . self::COMBINED_PDF_FILENAME;
 
             @unlink($tmpfile);
-            $pagecount = $pdf->combine_pdfs($compatiblepdfs, $tmpfile);
+            try {
+                $pagecount = $pdf->combine_pdfs($compatiblepdfs, $tmpfile);
+            } catch (\Exception $e) {
+                debugging('TCPDF could not process the pdf files:' . $e->getMessage(), DEBUG_DEVELOPER);
+                // TCPDF does not recover from errors so we need to re-initialise the class.
+                $pagecount = 0;
+            }
             if ($pagecount == 0) {
                 // We at least want a single blank page.
+                debugging('TCPDF did not produce a valid pdf:' . $tmpfile . '. Replacing with a blank pdf.', DEBUG_DEVELOPER);
+                $pdf = new pdf();
                 $pdf->AddPage();
                 @unlink($tmpfile);
                 $files = false;
@@ -229,14 +237,27 @@ class document_services {
 
         $fs->delete_area_files($record->contextid, $record->component, $record->filearea, $record->itemid);
 
+        // Detect corrupt generated pdfs and replace with a blank one.
+        if ($files) {
+            $pagecount = $pdf->load_pdf($tmpfile);
+            if ($pagecount <= 0) {
+                $files = false;
+            }
+        }
+
         if (!$files) {
             // This was a blank pdf.
+            unset($pdf);
+            $pdf = new pdf();
             $content = $pdf->Output(self::COMBINED_PDF_FILENAME, 'S');
             $file = $fs->create_file_from_string($record, $content);
         } else {
             // This was a combined pdf.
             $file = $fs->create_file_from_pathname($record, $tmpfile);
             @unlink($tmpfile);
+
+            // Test the generated file for correctness.
+            $compatiblepdf = pdf::ensure_pdf_compatible($file);
         }
 
         return $file;
index d11f077..c3c5f00 100644 (file)
@@ -78,6 +78,7 @@ class pdf extends \FPDI {
     public function combine_pdfs($pdflist, $outfilename) {
 
         raise_memory_limit(MEMORY_EXTRA);
+        $olddebug = error_reporting(0);
 
         $this->setPageUnit('pt');
         $this->setPrintHeader(false);
@@ -97,6 +98,7 @@ class pdf extends \FPDI {
         }
 
         $this->save_pdf($outfilename);
+        error_reporting($olddebug);
 
         return $totalpagecount;
     }
@@ -125,6 +127,7 @@ class pdf extends \FPDI {
      */
     public function load_pdf($filename) {
         raise_memory_limit(MEMORY_EXTRA);
+        $olddebug = error_reporting(0);
 
         $this->setPageUnit('pt');
         $this->scale = 72.0 / 100.0;
@@ -138,6 +141,7 @@ class pdf extends \FPDI {
         $this->pagecount = $this->setSourceFile($filename);
         $this->filename = $filename;
 
+        error_reporting($olddebug);
         return $this->pagecount;
     }
 
@@ -389,7 +393,9 @@ class pdf extends \FPDI {
      * @param string $filename the filename for the PDF (including the full path)
      */
     public function save_pdf($filename) {
+        $olddebug = error_reporting(0);
         $this->Output($filename, 'F');
+        error_reporting($olddebug);
     }
 
     /**
@@ -461,33 +467,26 @@ class pdf extends \FPDI {
      * @return string path to copy or converted pdf (false == fail)
      */
     public static function ensure_pdf_compatible(\stored_file $file) {
-        global $CFG;
-
-        $fp = $file->get_content_file_handle();
-        $ident = fread($fp, 10);
-        if (substr_compare('%PDF-', $ident, 0, 5) !== 0) {
-            return false; // This is not a PDF file at all.
-        }
-        $ident = substr($ident, 5); // Remove the '%PDF-' part.
-        $ident = explode('\x0A', $ident); // Truncate to first '0a' character.
-        list($major, $minor) = explode('.', $ident[0]); // Split the major / minor version.
-        $major = intval($major);
-        $minor = intval($minor);
-        if ($major == 0 || $minor == 0) {
-            return false; // Not a valid PDF version number.
-        }
         $temparea = \make_temp_directory('assignfeedback_editpdf');
         $hash = $file->get_contenthash(); // Use the contenthash to make sure the temp files have unique names.
         $tempsrc = $temparea . "/src-$hash.pdf";
         $tempdst = $temparea . "/dst-$hash.pdf";
+        $file->copy_content_to($tempsrc); // Copy the file.
 
-        if ($major = 1 && $minor<=4) {
-            // PDF is valid version - just create a copy we can use.
-            $file->copy_content_to($tempdst); // Copy the file.
-            return $tempdst;
+        $pdf = new pdf();
+        $pagecount = 0;
+        try {
+            $pagecount = $pdf->load_pdf($tempsrc);
+        } catch (\Exception $e) {
+            // PDF was not valid - try running it through ghostscript to clean it up.
+            $pagecount = 0;
+        }
+
+        if ($pagecount > 0) {
+            // Page is valid and can be read by tcpdf.
+            return $tempsrc;
         }
 
-        $file->copy_content_to($tempsrc); // Copy the file.
 
         $gsexec = \escapeshellarg(\get_config('assignfeedback_editpdf', 'gspath'));
         $tempdstarg = \escapeshellarg($tempdst);
@@ -500,6 +499,20 @@ class pdf extends \FPDI {
             return false;
         }
 
+        $pdf = new pdf();
+        $pagecount = 0;
+        try {
+            $pagecount = $pdf->load_pdf($tempdst);
+        } catch (\Exception $e) {
+            // PDF was not valid - try running it through ghostscript to clean it up.
+            $pagecount = 0;
+        }
+        if ($pagecount <= 0) {
+            @unlink($tempdst);
+            // Could not parse the converted pdf.
+            return false;
+        }
+
         return $tempdst;
     }
 
index fe5f1e6..c424251 100644 (file)
@@ -759,6 +759,9 @@ class assign_grading_table extends table_sql implements renderable {
                               id="selectuser_' . $row->userid . '"
                               name="selectedusers"
                               value="' . $row->userid . '"/>';
+        $selectcol .= '<input type="hidden"
+                              name="grademodified_' . $row->userid . '"
+                              value="' . $row->timemarked . '"/>';
         return $selectcol;
     }
 
index fc62b56..0ab485f 100644 (file)
@@ -1251,12 +1251,8 @@ class assign {
                               maxlength="10"
                               class="quickgrade"/>';
                 $o .= '&nbsp;/&nbsp;' . format_float($this->get_instance()->grade, 2);
-                $o .= '<input type="hidden"
-                              name="grademodified_' . $userid . '"
-                              value="' . $modified . '"/>';
                 return $o;
             } else {
-                $o .= '<input type="hidden" name="grademodified_' . $userid . '" value="' . $modified . '"/>';
                 if ($grade == -1 || $grade === null) {
                     $o .= '-';
                 } else {
@@ -1295,9 +1291,6 @@ class assign {
                     $o .= '<option value="' . $optionid . '" ' . $selected . '>' . $option . '</option>';
                 }
                 $o .= '</select>';
-                $o .= '<input type="hidden" ' .
-                             'name="grademodified_' . $userid . '" ' .
-                             'value="' . $modified . '"/>';
                 return $o;
             } else {
                 $scaleid = (int)$grade;
@@ -4945,8 +4938,8 @@ class assign {
             $record->userid = $userid;
             if ($modified >= 0) {
                 $record->grade = unformat_float(optional_param('quickgrade_' . $record->userid, -1, PARAM_TEXT));
-                $record->workflowstate = optional_param('quickgrade_' . $record->userid.'_workflowstate', '', PARAM_TEXT);
-                $record->allocatedmarker = optional_param('quickgrade_' . $record->userid.'_allocatedmarker', '', PARAM_INT);
+                $record->workflowstate = optional_param('quickgrade_' . $record->userid.'_workflowstate', false, PARAM_TEXT);
+                $record->allocatedmarker = optional_param('quickgrade_' . $record->userid.'_allocatedmarker', false, PARAM_INT);
             } else {
                 // This user was not in the grading table.
                 continue;
@@ -4984,6 +4977,8 @@ class assign {
         foreach ($currentgrades as $current) {
             $modified = $users[(int)$current->userid];
             $grade = $this->get_user_grade($modified->userid, false);
+            // Check to see if the grade column was even visible.
+            $gradecolpresent = optional_param('quickgrade_' . $modified->userid, false, PARAM_INT) !== false;
 
             // Check to see if the outcomes were modified.
             if ($CFG->enableoutcomes) {
@@ -4991,7 +4986,9 @@ class assign {
                     $oldoutcome = $outcome->grades[$modified->userid]->grade;
                     $paramname = 'outcome_' . $outcomeid . '_' . $modified->userid;
                     $newoutcome = optional_param($paramname, -1, PARAM_FLOAT);
-                    if ($oldoutcome != $newoutcome) {
+                    // Check to see if the outcome column was even visible.
+                    $outcomecolpresent = optional_param($paramname, false, PARAM_FLOAT) !== false;
+                    if ($outcomecolpresent && ($oldoutcome != $newoutcome)) {
                         // Can't check modified time for outcomes because it is not reported.
                         $modifiedusers[$modified->userid] = $modified;
                         continue;
@@ -5002,6 +4999,8 @@ class assign {
             // Let plugins participate.
             foreach ($this->feedbackplugins as $plugin) {
                 if ($plugin->is_visible() && $plugin->is_enabled() && $plugin->supports_quickgrading()) {
+                    // The plugins must handle is_quickgrading_modified correctly - ie
+                    // handle hidden columns.
                     if ($plugin->is_quickgrading_modified($modified->userid, $grade)) {
                         if ((int)$current->lastmodified > (int)$modified->lastmodified) {
                             return get_string('errorrecordmodified', 'assign');
@@ -5022,10 +5021,14 @@ class assign {
             if ($current->grade !== null) {
                 $current->grade = floatval($current->grade);
             }
-            if ($current->grade !== $modified->grade ||
-                 ($this->get_instance()->markingallocation && $current->allocatedmarker != $modified->allocatedmarker ) ||
-                 ($this->get_instance()->markingworkflow && $current->workflowstate !== $modified->workflowstate )) {
-
+            $gradechanged = $gradecolpresent && $current->grade !== $modified->grade;
+            $markingallocationchanged = $this->get_instance()->markingallocation &&
+                                            ($modified->allocatedmarker !== false) &&
+                                            ($current->allocatedmarker != $modified->allocatedmarker);
+            $workflowstatechanged = $this->get_instance()->markingworkflow &&
+                                            ($modified->workflowstate !== false) &&
+                                            ($current->workflowstate != $modified->workflowstate);
+            if ($gradechanged || $markingallocationchanged || $workflowstatechanged) {
                 // Grade changed.
                 if ($this->grading_disabled($modified->userid)) {
                     continue;
@@ -5050,6 +5053,7 @@ class assign {
             $flags = $this->get_user_flags($userid, true);
             $grade->grade= grade_floatval(unformat_float($modified->grade));
             $grade->grader= $USER->id;
+            $gradecolpresent = optional_param('quickgrade_' . $userid, false, PARAM_INT) !== false;
 
             // Save plugins data.
             foreach ($this->feedbackplugins as $plugin) {
@@ -5063,11 +5067,21 @@ class assign {
                 }
             }
 
-            if ($flags->workflowstate != $modified->workflowstate ||
-                $flags->allocatedmarker != $modified->allocatedmarker) {
+            // These will be set to false if they are not present in the quickgrading
+            // form (e.g. column hidden).
+            $workflowstatemodified = ($modified->workflowstate !== false) &&
+                                        ($flags->workflowstate != $modified->workflowstate);
+
+            $allocatedmarkermodified = ($modified->allocatedmarker !== false) &&
+                                        ($flags->allocatedmarker != $modified->allocatedmarker);
 
+            if ($workflowstatemodified) {
                 $flags->workflowstate = $modified->workflowstate;
+            }
+            if ($allocatedmarkermodified) {
                 $flags->allocatedmarker = $modified->allocatedmarker;
+            }
+            if ($workflowstatemodified || $allocatedmarkermodified) {
                 $this->update_user_flags($flags);
             }
             $this->update_grade($grade);
@@ -5082,8 +5096,10 @@ class assign {
                 foreach ($modified->gradinginfo->outcomes as $outcomeid => $outcome) {
                     $oldoutcome = $outcome->grades[$modified->userid]->grade;
                     $paramname = 'outcome_' . $outcomeid . '_' . $modified->userid;
-                    $newoutcome = optional_param($paramname, -1, PARAM_INT);
-                    if ($oldoutcome != $newoutcome) {
+                    // This will be false if the input was not in the quickgrading
+                    // form (e.g. column hidden).
+                    $newoutcome = optional_param($paramname, false, PARAM_INT);
+                    if ($newoutcome !== false && ($oldoutcome != $newoutcome)) {
                         $data[$outcomeid] = $newoutcome;
                     }
                 }
index 76b2fa3..eacc25c 100644 (file)
@@ -73,7 +73,7 @@ class assignsubmission_comments_events_testcase extends mod_assign_base_testcase
         // Checking that the event contains the expected values.
         $this->assertInstanceOf('\assignsubmission_comments\event\comment_created', $event);
         $this->assertEquals($context, $event->get_context());
-        $url = new moodle_url('/mod/assign/view.php', array('id' => $submission->id));
+        $url = new moodle_url('/mod/assign/view.php', array('id' => $assign->get_course_module()->id));
         $this->assertEquals($url, $event->get_url());
         $this->assertEventContextNotUsed($event);
     }
@@ -111,7 +111,7 @@ class assignsubmission_comments_events_testcase extends mod_assign_base_testcase
         // Checking that the event contains the expected values.
         $this->assertInstanceOf('\assignsubmission_comments\event\comment_deleted', $event);
         $this->assertEquals($context, $event->get_context());
-        $url = new moodle_url('/mod/assign/view.php', array('id' => $submission->id));
+        $url = new moodle_url('/mod/assign/view.php', array('id' => $assign->get_course_module()->id));
         $this->assertEquals($url, $event->get_url());
         $this->assertEventContextNotUsed($event);
     }
diff --git a/mod/assign/tests/behat/quickgrading.feature b/mod/assign/tests/behat/quickgrading.feature
new file mode 100644 (file)
index 0000000..75bf99c
--- /dev/null
@@ -0,0 +1,143 @@
+@mod @mod_assign
+Feature: In an assignment, teachers grade multiple students on one page
+  In order to quickly give students grades and feedback
+  As a teacher
+  I need to grade multiple students on one page
+
+  @javascript
+  Scenario: Grade multiple students on one page
+    Given the following "courses" exists:
+      | fullname | shortname | category | groupmode |
+      | Course 1 | C1 | 0 | 1 |
+    And the following "users" exists:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@asd.com |
+      | student1 | Student | 1 | student1@asd.com |
+      | student2 | Student | 2 | student2@asd.com |
+    And the following "course enrolments" exists:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | student1 | C1 | student |
+      | student2 | C1 | student |
+    When I log in as "admin"
+    And I set the following administration settings values:
+      | Enable outcomes | 1 |
+    And I log out
+    And I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Outcomes"
+    And I follow "Edit outcomes"
+    And I press "Add a new outcome"
+    And I press "Continue"
+    And I fill the moodle form with:
+      | Name | 1337dom scale |
+      | Scale | Noob, Nub, 1337, HaXor |
+    And I press "Save changes"
+    And I follow "Course 1"
+    And I follow "Outcomes"
+    And I follow "Edit outcomes"
+    And I press "Add a new outcome"
+    And I fill the moodle form with:
+      | Full name | M8d skillZ! |
+      | Short name | skillZ! |
+      | Scale | 1337dom scale |
+    And I press "Save changes"
+    And I follow "Course 1"
+    And I turn editing mode on
+    And I add a "Assignment" to section "1" and I fill the form with:
+      | Assignment name | Test assignment name |
+      | Description | Submit your online text |
+      | assignsubmission_onlinetext_enabled | 1 |
+      | assignsubmission_file_enabled | 0 |
+      | M8d skillZ! | 1 |
+    And I log out
+    And I log in as "student1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I press "Add submission"
+    And I fill the moodle form with:
+      | Online text | I'm the student1 submission |
+    And I press "Save changes"
+    And I log out
+    And I log in as "student2"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    When I press "Add submission"
+    And I fill the moodle form with:
+      | Online text | I'm the student2 submission |
+    And I press "Save changes"
+    And I log out
+    And I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I follow "View/grade all submissions"
+    And I click on "Grade Student 1" "link" in the "Student 1" "table_row"
+    And I fill the moodle form with:
+      | Grade out of 100 | 50.0 |
+      | M8d skillZ! | 1337 |
+      | Feedback comments | I'm the teacher first feedback |
+    And I press "Save changes"
+    And I press "Continue"
+    Then I click on "Quick grading" "checkbox"
+    And I fill in "User grade" with "60.0"
+    And I press "Save all quick grading changes"
+    And I should see "The grade changes were saved"
+    And I press "Continue"
+    And I log out
+    And I log in as "student1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I should see "I'm the teacher first feedback"
+    And I should see "60.0"
+    And I follow "Course 1"
+    And I follow "Grades"
+    And I should see "1337"
+    And I log out
+    And I log in as "student2"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I should not see "I'm the teacher first feedback"
+    And I should not see "60.0"
+    And I follow "Course 1"
+    And I follow "Grades"
+    And I should not see "1337"
+    And I log out
+    And I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I follow "View/grade all submissions"
+    And I click on "Hide User picture" "link"
+    And I click on "Hide Full name" "link"
+    And I click on "Hide Email address" "link"
+    And I click on "Hide Status" "link"
+    And I click on "Hide Grade" "link"
+    And I click on "Hide Edit" "link"
+    And I click on "Hide Last modified (submission)" "link"
+    And I click on "Hide Online text" "link"
+    And I click on "Hide Submission comments" "link"
+    And I click on "Hide Last modified (grade)" "link"
+    And I click on "Hide Feedback comments" "link"
+    And I click on "Hide Annotate PDF" "link"
+    And I click on "Hide Final grade" "link"
+    And I click on "Hide Outcomes" "link"
+    And I press "Save all quick grading changes"
+    And I should see "The grade changes were saved"
+    And I press "Continue"
+    And I log out
+    And I log in as "student1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I should see "I'm the teacher first feedback"
+    And I should see "60.0"
+    And I follow "Course 1"
+    And I follow "Grades"
+    And I should see "1337"
+    And I log out
+    And I log in as "student2"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I should not see "I'm the teacher first feedback"
+    And I should not see "60.0"
+    And I follow "Course 1"
+    And I follow "Grades"
+    And I should not see "1337"
index 5ab2faa..d1ad3a8 100644 (file)
@@ -66,7 +66,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
         // Create a teacher and give them capabilities.
         $context = context_course::instance($course->id);
         $roleid = $this->assignUserCapability('moodle/course:viewparticipants', $context->id, 3);
-        $context = context_module::instance($assign->id);
+        $context = context_module::instance($assign->cmid);
         $this->assignUserCapability('mod/assign:grade', $context->id, $roleid);
 
         // Create the teacher's enrolment record.
@@ -163,7 +163,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
         // Create the user and give them capabilities.
         $context = context_course::instance($course1->id);
         $roleid = $this->assignUserCapability('moodle/course:view', $context->id);
-        $context = context_module::instance($assign1->id);
+        $context = context_module::instance($assign1->cmid);
         $this->assignUserCapability('mod/assign:view', $context->id, $roleid);
 
         // Create the user enrolment record.
@@ -332,7 +332,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
         // Create a teacher and give them capabilities.
         $context = context_course::instance($course->id);
         $roleid = $this->assignUserCapability('moodle/course:viewparticipants', $context->id, 3);
-        $context = context_module::instance($assign->id);
+        $context = context_module::instance($assign->cmid);
         $this->assignUserCapability('mod/assign:grade', $context->id, $roleid);
 
         // Create the teacher's enrolment record.
@@ -403,7 +403,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
         // Create a teacher and give them capabilities.
         $context = context_course::instance($course->id);
         $roleid = $this->assignUserCapability('moodle/course:viewparticipants', $context->id, 3);
-        $context = context_module::instance($assign->id);
+        $context = context_module::instance($assign->cmid);
         $this->assignUserCapability('mod/assign:revealidentities', $context->id, $roleid);
 
         // Create the teacher's enrolment record.
@@ -1072,7 +1072,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
         // Create a teacher and give them capabilities.
         $context = context_course::instance($course->id);
         $roleid = $this->assignUserCapability('moodle/course:viewparticipants', $context->id, 3);
-        $context = context_module::instance($assign->id);
+        $context = context_module::instance($assign->cmid);
         $this->assignUserCapability('mod/assign:grade', $context->id, $roleid);
 
         // Create the teacher's enrolment record.
index 3f4dfc3..2992884 100644 (file)
@@ -65,7 +65,7 @@ class mod_book_events_testcase extends advanced_testcase {
 
         // Checking that the event contains the expected values.
         $this->assertInstanceOf('\mod_book\event\chapter_created', $event);
-        $this->assertEquals(context_module::instance($book->id), $event->get_context());
+        $this->assertEquals(context_module::instance($book->cmid), $event->get_context());
         $this->assertEquals($chapter->id, $event->objectid);
         $expected = array($course->id, 'book', 'add chapter', 'view.php?id='.$book->cmid.'&chapterid='.$chapter->id,
             $chapter->id, $book->cmid);
@@ -98,7 +98,7 @@ class mod_book_events_testcase extends advanced_testcase {
 
         // Checking that the event contains the expected values.
         $this->assertInstanceOf('\mod_book\event\chapter_updated', $event);
-        $this->assertEquals(context_module::instance($book->id), $event->get_context());
+        $this->assertEquals(context_module::instance($book->cmid), $event->get_context());
         $this->assertEquals($chapter->id, $event->objectid);
         $expected = array($course->id, 'book', 'update chapter', 'view.php?id='.$book->cmid.'&chapterid='.$chapter->id,
             $chapter->id, $book->cmid);
@@ -133,7 +133,7 @@ class mod_book_events_testcase extends advanced_testcase {
 
         // Checking that the event contains the expected values.
         $this->assertInstanceOf('\mod_book\event\chapter_deleted', $event);
-        $this->assertEquals(context_module::instance($book->id), $event->get_context());
+        $this->assertEquals(context_module::instance($book->cmid), $event->get_context());
         $this->assertEquals($chapter->id, $event->objectid);
         $this->assertEquals($chapter, $event->get_record_snapshot('book_chapters', $chapter->id));
         $this->assertEventLegacyLogData(array('1', 2, false), $event);
index 4b85263..dcc9030 100644 (file)
@@ -77,7 +77,7 @@ class mod_choice_events_testcase extends advanced_testcase {
         $this->assertCount(1, $events);
         $this->assertInstanceOf('\mod_choice\event\answer_submitted', $events[0]);
         $this->assertEquals($user->id, $events[0]->userid);
-        $this->assertEquals(context_module::instance($this->choice->id), $events[0]->get_context());
+        $this->assertEquals(context_module::instance($this->choice->cmid), $events[0]->get_context());
         $this->assertEquals(1, $events[0]->other['choiceid']);
         $this->assertEquals(3, $events[0]->other['optionid']);
         $expected = array($this->course->id, "choice", "choose", 'view.php?id=' . $this->cm->id, $this->choice->id, $this->cm->id);
@@ -128,7 +128,7 @@ class mod_choice_events_testcase extends advanced_testcase {
         $this->assertCount(1, $events);
         $this->assertInstanceOf('\mod_choice\event\answer_updated', $events[0]);
         $this->assertEquals($user->id, $events[0]->userid);
-        $this->assertEquals(context_module::instance($this->choice->id), $events[0]->get_context());
+        $this->assertEquals(context_module::instance($this->choice->cmid), $events[0]->get_context());
         $this->assertEquals(1, $events[0]->other['choiceid']);
         $this->assertEquals(3, $events[0]->other['optionid']);
         $expected = array($this->course->id, "choice", "choose again", 'view.php?id=' . $this->cm->id,
@@ -189,7 +189,7 @@ class mod_choice_events_testcase extends advanced_testcase {
         $this->assertCount(1, $event);
         $this->assertInstanceOf('\mod_choice\event\report_viewed', $event[0]);
         $this->assertEquals($USER->id, $event[0]->userid);
-        $this->assertEquals(context_module::instance($this->choice->id), $event[0]->get_context());
+        $this->assertEquals(context_module::instance($this->choice->cmid), $event[0]->get_context());
         $expected = array($this->course->id, "choice", "report", 'report.php?id=' . $this->context->instanceid,
                 $this->choice->id, $this->context->instanceid);
         $this->assertEventLegacyLogData($expected, $event[0]);
@@ -224,7 +224,7 @@ class mod_choice_events_testcase extends advanced_testcase {
         $this->assertCount(1, $event);
         $this->assertInstanceOf('\mod_choice\event\course_module_viewed', $event[0]);
         $this->assertEquals($USER->id, $event[0]->userid);
-        $this->assertEquals(context_module::instance($this->choice->id), $event[0]->get_context());
+        $this->assertEquals(context_module::instance($this->choice->cmid), $event[0]->get_context());
         $expected = array($this->course->id, "choice", "view", 'view.php?id=' . $this->context->instanceid,
                 $this->choice->id, $this->context->instanceid);
         $this->assertEventLegacyLogData($expected, $event[0]);
index 247ded7..1d757c2 100644 (file)
@@ -142,7 +142,7 @@ class data_lib_testcase extends advanced_testcase {
         $contentid = $DB->insert_record('data_content', $datacontent);
         $cm = get_coursemodule_from_instance('data', $module->id, $course->id);
 
-        $context = context_module::instance($module->id);
+        $context = context_module::instance($module->cmid);
         $cmt = new stdClass();
         $cmt->context = $context;
         $cmt->course = $course;
@@ -205,7 +205,7 @@ class data_lib_testcase extends advanced_testcase {
         $contentid = $DB->insert_record('data_content', $datacontent);
         $cm = get_coursemodule_from_instance('data', $module->id, $course->id);
 
-        $context = context_module::instance($module->id);
+        $context = context_module::instance($module->cmid);
         $cmt = new stdClass();
         $cmt->context = $context;
         $cmt->course = $course;
index 947541b..4d88711 100644 (file)
@@ -286,7 +286,7 @@ class mod_feedback_events_testcase extends advanced_testcase {
 
         // Test legacy data.
         $arr = array($this->eventcourse->id, 'feedback', 'submit', 'view.php?id=' . $this->eventcm->id, $this->eventfeedback->id,
-                     $this->eventfeedback->id, $USER->id);
+                     $this->eventcm->id, $USER->id);
         $this->assertEventLegacyLogData($arr, $event);
 
         // Test can_view().
index 7696f07..2bad212 100644 (file)
@@ -3561,7 +3561,7 @@ function forum_print_post($post, $discussion, $forum, &$cm, $course, $ownpost=fa
     $output .= html_writer::tag('div', implode(' | ', $commandhtml), array('class'=>'commands'));
 
     // Output link to post if required
-    if ($link) {
+    if ($link && forum_user_can_post($forum, $discussion, $USER, $cm, $course, $modcontext)) {
         if ($post->replies == 1) {
             $replystring = get_string('repliesone', 'forum', $post->replies);
         } else {
index 087e802..e78f863 100644 (file)
@@ -90,7 +90,7 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
         $enrol->enrol_user($instance2, $user->id);
 
         // Assign capabilities to view forums for forum 2.
-        $cm2 = get_coursemodule_from_id('forum', $forum2->id, 0, false, MUST_EXIST);
+        $cm2 = get_coursemodule_from_id('forum', $forum2->cmid, 0, false, MUST_EXIST);
         $context2 = context_module::instance($cm2->id);
         $newrole = create_role('Role 2', 'role2', 'Role 2 description');
         $roleid2 = $this->assignUserCapability('mod/forum:viewdiscussion', $context2->id, $newrole);
@@ -256,13 +256,13 @@ class mod_forum_external_testcase extends externallib_advanced_testcase {
         $enrol->enrol_user($instance2, $user1->id);
 
         // Assign capabilities to view discussions for forum 2.
-        $cm = get_coursemodule_from_id('forum', $forum2->id, 0, false, MUST_EXIST);
+        $cm = get_coursemodule_from_id('forum', $forum2->cmid, 0, false, MUST_EXIST);
         $context = context_module::instance($cm->id);
         $newrole = create_role('Role 2', 'role2', 'Role 2 description');
         $this->assignUserCapability('mod/forum:viewdiscussion', $context->id, $newrole);
 
         // Assign capabilities to view discussions for forum 3.
-        $cm = get_coursemodule_from_id('forum', $forum3->id, 0, false, MUST_EXIST);
+        $cm = get_coursemodule_from_id('forum', $forum3->cmid, 0, false, MUST_EXIST);
         $context = context_module::instance($cm->id);
         $this->assignUserCapability('mod/forum:viewdiscussion', $context->id, $newrole);
 
index a6cb6b0..d50a7a7 100644 (file)
@@ -36,7 +36,7 @@ class mod_forum_lib_testcase extends advanced_testcase {
 
         $this->setUser($user->id);
         $fakepost = (object) array('id' => 123, 'message' => 'Yay!', 'discussion' => 100);
-        $cm = get_coursemodule_from_instance('forum', $forum->cmid);
+        $cm = get_coursemodule_from_instance('forum', $forum->id);
 
         $fs = get_file_storage();
         $dummy = (object) array(
index 45fc60e..3305204 100644 (file)
@@ -52,7 +52,7 @@ class glossary_event_testcase extends advanced_testcase {
 
         $entry = $glossarygenerator->create_content($glossary);
 
-        $context = context_module::instance($glossary->id);
+        $context = context_module::instance($glossary->cmid);
         $cm = get_coursemodule_from_instance('data', $glossary->id, $course->id);
         $cmt = new stdClass();
         $cmt->component = 'mod_glossary';
@@ -96,7 +96,7 @@ class glossary_event_testcase extends advanced_testcase {
 
         $entry = $glossarygenerator->create_content($glossary);
 
-        $context = context_module::instance($glossary->id);
+        $context = context_module::instance($glossary->cmid);
         $cm = get_coursemodule_from_instance('data', $glossary->id, $course->id);
         $cmt = new stdClass();
         $cmt->component = 'mod_glossary';
index dbcbd04..7171bf2 100644 (file)
@@ -78,7 +78,7 @@ class mod_scorm_event_testcase extends advanced_testcase {
         $this->assertEquals(4, $event->other['attemptid']);
         $this->assertEquals(2, $event->relateduserid);
         $expected = array($this->eventcourse->id, 'scorm', 'delete attempts', 'report.php?id=' . $this->eventcm->id,
-                4, $this->eventscorm->id);
+                4, $this->eventcm->id);
         $this->assertEventLegacyLogData($expected, $events[0]);
         $this->assertEventContextNotUsed($event);
 
index 9b0d324..59e60b5 100644 (file)
@@ -237,7 +237,7 @@ class mod_wiki_events_testcase extends advanced_testcase {
         $this->assertInstanceOf('\mod_wiki\event\page_viewed', $event);
         $this->assertEquals($context, $event->get_context());
         $this->assertEquals($page->id, $event->objectid);
-        $expected = array($this->course->id, 'wiki', 'view', 'view.php?pageid=' . $page->id, $page->id, $this->wiki->id);
+        $expected = array($this->course->id, 'wiki', 'view', 'view.php?pageid=' . $page->id, $page->id, $this->wiki->cmid);
         $this->assertEventLegacyLogData($expected, $event);
         $this->assertEventContextNotUsed($event);
     }
@@ -272,7 +272,7 @@ class mod_wiki_events_testcase extends advanced_testcase {
         $this->assertInstanceOf('\mod_wiki\event\page_viewed', $event);
         $this->assertEquals($context, $event->get_context());
         $this->assertEquals($page->id, $event->objectid);
-        $expected = array($this->course->id, 'wiki', 'view', 'prettyview.php?pageid=' . $page->id, $page->id, $this->wiki->id);
+        $expected = array($this->course->id, 'wiki', 'view', 'prettyview.php?pageid=' . $page->id, $page->id, $this->wiki->cmid);
         $this->assertEventLegacyLogData($expected, $event);
         $this->assertEventContextNotUsed($event);
     }
@@ -345,7 +345,7 @@ class mod_wiki_events_testcase extends advanced_testcase {
         $this->assertInstanceOf('\mod_wiki\event\page_locks_deleted', $event);
         $this->assertEquals($context, $event->get_context());
         $this->assertEquals($page->id, $event->objectid);
-        $expected = array($this->course->id, 'wiki', 'overridelocks', 'view.php?pageid=' . $page->id, $page->id, $this->wiki->id);
+        $expected = array($this->course->id, 'wiki', 'overridelocks', 'view.php?pageid=' . $page->id, $page->id, $this->wiki->cmid);
         $this->assertEventLegacyLogData($expected, $event);
 
         // Delete all pages.
@@ -431,7 +431,7 @@ class mod_wiki_events_testcase extends advanced_testcase {
         $this->assertEquals($context, $event->get_context());
         $this->assertEquals($page->id, $event->objectid);
         $expected = array($this->course->id, 'wiki', 'diff', 'diff.php?pageid=' . $page->id . '&comparewith=' .
-            1 . '&compare=' .  2, $page->id, $this->wiki->id);
+            1 . '&compare=' .  2, $page->id, $this->wiki->cmid);
         $this->assertEventLegacyLogData($expected, $event);
         $this->assertEventContextNotUsed($event);
     }
@@ -465,7 +465,7 @@ class mod_wiki_events_testcase extends advanced_testcase {
         $this->assertInstanceOf('\mod_wiki\event\page_history_viewed', $event);
         $this->assertEquals($context, $event->get_context());
         $this->assertEquals($page->id, $event->objectid);
-        $expected = array($this->course->id, 'wiki', 'history', 'history.php?pageid=' . $page->id, $page->id, $this->wiki->id);
+        $expected = array($this->course->id, 'wiki', 'history', 'history.php?pageid=' . $page->id, $page->id, $this->wiki->cmid);
         $this->assertEventLegacyLogData($expected, $event);
         $this->assertEventContextNotUsed($event);
     }
@@ -503,7 +503,7 @@ class mod_wiki_events_testcase extends advanced_testcase {
         $this->assertEquals($context, $event->get_context());
         $this->assertEquals($page->id, $event->objectid);
         $this->assertEquals(0, $event->other['option']);
-        $expected = array($this->course->id, 'wiki', 'map', 'map.php?pageid=' . $page->id, $page->id, $this->wiki->id);
+        $expected = array($this->course->id, 'wiki', 'map', 'map.php?pageid=' . $page->id, $page->id, $this->wiki->cmid);
         $this->assertEventLegacyLogData($expected, $event);
         $this->assertEventContextNotUsed($event);
     }
@@ -542,7 +542,7 @@ class mod_wiki_events_testcase extends advanced_testcase {
         $this->assertEquals($page->id, $event->objectid);
         $this->assertEquals(1, $event->other['versionid']);
         $expected = array($this->course->id, 'wiki', 'history', 'viewversion.php?pageid=' . $page->id . '&versionid=1',
-            $page->id, $this->wiki->id);
+            $page->id, $this->wiki->cmid);
         $this->assertEventLegacyLogData($expected, $event);
         $this->assertEventContextNotUsed($event);
     }
@@ -569,7 +569,7 @@ class mod_wiki_events_testcase extends advanced_testcase {
         $this->assertEquals($context, $event->get_context());
         $this->assertEquals($version->id, $event->objectid);
         $this->assertEquals($page->id, $event->other['pageid']);
-        $expected = array($this->course->id, 'wiki', 'restore', 'view.php?pageid=' . $page->id, $page->id, $this->wiki->id);
+        $expected = array($this->course->id, 'wiki', 'restore', 'view.php?pageid=' . $page->id, $page->id, $this->wiki->cmid);
         $this->assertEventLegacyLogData($expected, $event);
         $this->assertEventContextNotUsed($event);
     }
index c06c3fb..ec61839 100644 (file)
@@ -48,3 +48,7 @@ To add filters, local plugins can now implement the function local_[pluginname]_
    To ensure you are no longer using or defining any deprecated functions,
    search for the regular expression:
    question_rewrite_questiontext_preview_urls|_questiontext_preview_pluginfile|question_send_questiontext_file
+
+3) The arugment list for core_question_renderer::mark_summary has changed.
+   Please update your calls. (The most likely secnario for this is if you have
+   overridden core_question_renderer::info in your own renderer.)
index fefb6be..ac161d1 100644 (file)
@@ -246,10 +246,10 @@ class core_repositorylib_testcase extends advanced_testcase {
         $forumdata = new stdClass();
         $forumdata->course = $course1->id;
         $forumc1 = $this->getDataGenerator()->create_module('forum', $forumdata);
-        $forumc1context = context_module::instance($forumc1->id);
+        $forumc1context = context_module::instance($forumc1->cmid);
         $forumdata->course = $course2->id;
         $forumc2 = $this->getDataGenerator()->create_module('forum', $forumdata);
-        $forumc2context = context_module::instance($forumc2->id);
+        $forumc2context = context_module::instance($forumc2->cmid);
 
         $blockdata = new stdClass();
         $blockdata->parentcontextid = $course1context->id;