Merge branch 'wip-MDL-46834-master' of git://github.com/marinaglancy/moodle
authorDan Poltawski <dan@moodle.com>
Tue, 9 Sep 2014 07:06:40 +0000 (08:06 +0100)
committerDan Poltawski <dan@moodle.com>
Tue, 9 Sep 2014 07:06:40 +0000 (08:06 +0100)
46 files changed:
admin/tool/behat/tests/behat/data_generators.feature
admin/user.php
backup/moodle2/backup_activity_task.class.php
backup/moodle2/backup_final_task.class.php
backup/moodle2/backup_root_task.class.php
backup/moodle2/backup_stepslib.php
backup/moodle2/restore_activity_task.class.php
backup/moodle2/restore_final_task.class.php
backup/moodle2/restore_root_task.class.php
backup/moodle2/restore_stepslib.php
blocks/badges/block_badges.php
blocks/course_overview/locallib.php
blocks/glossary_random/block_glossary_random.php
course/format/upgrade.txt
course/mod.php
course/modduplicate.php
course/tests/behat/behat_course.php
files/externallib.php
files/renderer.php
grade/edit/tree/index.php
grade/edit/tree/lib.php
grade/report/user/styles.css
lang/en/moodle.php
lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-debug.js
lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-min.js
lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor.js
lib/editor/atto/yui/src/editor/js/autosave.js
lib/editor/atto/yui/src/editor/js/notify.js
lib/tests/behat/behat_data_generators.php
message/output/airnotifier/message_output_airnotifier.php
mod/assign/feedback/editpdf/locallib.php
mod/assign/feedback/editpdf/tests/behat/annotate_pdf.feature
mod/forum/classes/observer.php
mod/forum/classes/subscriptions.php
mod/forum/discuss.php
mod/forum/index.php
mod/forum/lib.php
mod/forum/post.php
mod/forum/subscribe.php
mod/forum/subscribe_ajax.php
mod/forum/tests/subscriptions_test.php
question/engine/datalib.php
question/engine/questionattemptstep.php
question/engine/questionusage.php
question/upgrade.txt
theme/clean/lib.php

index 2b9dfdf..ea90637 100644 (file)
@@ -253,24 +253,40 @@ Feature: Set up contextual data for tests
     And I set the field "groups" to "Group 2 (1)"
     And the "members" select box should contain "Student 2"
 
-  Scenario: Add cohorts with data generator
+  Scenario: Add cohorts and cohort members with data generator
     Given the following "categories" exist:
       | name  | category | idnumber |
       | Cat 1 | 0        | CAT1     |
+    And the following "users" exist:
+      | username | firstname | lastname | email |
+      | student1 | Student | 1 | student1@asd.com |
+      | student2 | Student | 2 | student2@asd.com |
     And the following "cohorts" exist:
       | name            | idnumber |
-      | System cohort 1 | CH01     |
+      | System cohort A | CHSA     |
     And the following "cohorts" exist:
       | name                 | idnumber | contextlevel | reference |
-      | System cohort 2      | CH02     | System       |           |
-      | Cohort in category 1 | CH1      | Category     | CAT1      |
+      | System cohort B      | CHSB     | System       |           |
+      | Cohort in category   | CHC      | Category     | CAT1      |
+      | Empty cohort         | CHE      | Category     | CAT1      |
+    And the following "cohort members" exist:
+      | user     | cohort |
+      | student1 | CHSA   |
+      | student2 | CHSB   |
+      | student1 | CHSB   |
+      | student1 | CHC    |
     When I log in as "admin"
     And I navigate to "Cohorts" node in "Site administration > Users > Accounts"
-    Then I should see "System cohort 1"
-    And I should see "System cohort 2"
+    Then the following should exist in the "cohorts" table:
+      | Name            | Cohort size |
+      | System cohort A | 1           |
+      | System cohort B | 2           |
     And I should not see "Cohort in category"
     And I follow "Courses"
     And I follow "Cat 1"
     And I follow "Cohorts"
-    And I should see "Cohort in category 1"
     And I should not see "System cohort"
+    And the following should exist in the "cohorts" table:
+      | Name               | Cohort size |
+      | Cohort in category | 1           |
+      | Empty cohort       | 0           |
index a7e1e64..184ada6 100644 (file)
         $table->colclasses = array();
         $table->head[] = $fullnamedisplay;
         $table->attributes['class'] = 'admintable generaltable';
-        $table->colclasses[] = 'leftalign';
         foreach ($extracolumns as $field) {
             $table->head[] = ${$field};
-            $table->colclasses[] = 'leftalign';
         }
         $table->head[] = $city;
-        $table->colclasses[] = 'leftalign';
         $table->head[] = $country;
-        $table->colclasses[] = 'leftalign';
         $table->head[] = $lastaccess;
-        $table->colclasses[] = 'leftalign';
         $table->head[] = get_string('edit');
         $table->colclasses[] = 'centeralign';
         $table->head[] = "";
index 2dc9353..e39c858 100644 (file)
@@ -181,6 +181,9 @@ abstract class backup_activity_task extends backup_task {
         // Generate the grading file (conditionally)
         $this->add_step(new backup_activity_grading_structure_step('activity_grading', 'grading.xml'));
 
+        // Generate the grade history file. The setting 'grade_histories' is handled in the step.
+        $this->add_step(new backup_activity_grade_history_structure_step('activity_grade_history', 'grade_history.xml'));
+
         // Annotate the scales used in already annotated outcomes
         $this->add_step(new backup_annotate_scales_from_outcomes('annotate_scales'));
 
index 2cd3754..6f69c59 100644 (file)
@@ -89,6 +89,9 @@ class backup_final_task extends backup_task {
         // execute_condition() so only will be excuted if ALL module grade_items in course have been exported
         $this->add_step(new backup_gradebook_structure_step('course_gradebook','gradebook.xml'));
 
+        // Generate the grade history file, conditionally.
+        $this->add_step(new backup_grade_history_structure_step('course_grade_history','grade_history.xml'));
+
         // Generate the course completion
         $this->add_step(new backup_course_completion_structure_step('course_completion', 'completion.xml'));
 
index 3e97f68..01a9f62 100644 (file)
@@ -152,6 +152,9 @@ class backup_root_task extends backup_task {
         $gradehistories->set_ui(new backup_setting_ui_checkbox($gradehistories, get_string('rootsettinggradehistories', 'backup')));
         $this->add_setting($gradehistories);
         $users->add_dependency($gradehistories);
+        // The restore does not process the grade histories when some activities are ignored.
+        // So let's define a dependency to prevent false expectations from our users.
+        $activities->add_dependency($gradehistories);
 
         // Define question bank inclusion setting.
         $questionbank = new backup_generic_setting('questionbank', base_setting::IS_BOOLEAN, true);
index a6e3ac6..e79c667 100644 (file)
@@ -1022,6 +1022,65 @@ class backup_gradebook_structure_step extends backup_structure_step {
     }
 }
 
+/**
+ * Step in charge of constructing the grade_history.xml file containing the grade histories.
+ */
+class backup_grade_history_structure_step extends backup_structure_step {
+
+    /**
+     * Limit the execution.
+     *
+     * This applies the same logic than the one applied to {@link backup_gradebook_structure_step},
+     * because we do not want to save the history of items which are not backed up. At least for now.
+     */
+    protected function execute_condition() {
+        return backup_plan_dbops::require_gradebook_backup($this->get_courseid(), $this->get_backupid());
+    }
+
+    protected function define_structure() {
+
+        // Settings to use.
+        $userinfo = $this->get_setting_value('users');
+        $history = $this->get_setting_value('grade_histories');
+
+        // Create the nested elements.
+        $bookhistory = new backup_nested_element('grade_history');
+        $grades = new backup_nested_element('grade_grades');
+        $grade = new backup_nested_element('grade_grade', array('id'), array(
+            'action', 'oldid', 'source', 'loggeduser', 'itemid', 'userid',
+            'rawgrade', 'rawgrademax', 'rawgrademin', 'rawscaleid',
+            'usermodified', 'finalgrade', 'hidden', 'locked', 'locktime', 'exported', 'overridden',
+            'excluded', 'feedback', 'feedbackformat', 'information',
+            'informationformat', 'timemodified'));
+
+        // Build the tree.
+        $bookhistory->add_child($grades);
+        $grades->add_child($grade);
+
+        // This only happens if we are including user info and history.
+        if ($userinfo && $history) {
+            // Only keep the history of grades related to items which have been backed up, The query is
+            // similar (but not identical) to the one used in backup_gradebook_structure_step::define_structure().
+            $gradesql = "SELECT ggh.*
+                           FROM {grade_grades_history} ggh
+                           JOIN {grade_items} gi ON ggh.itemid = gi.id
+                          WHERE gi.courseid = :courseid
+                            AND (gi.itemtype = 'manual' OR gi.itemtype = 'course' OR gi.itemtype = 'category')";
+            $grade->set_source_sql($gradesql, array('courseid' => backup::VAR_COURSEID));
+        }
+
+        // Annotations. (Final annotations as this step is part of the final task).
+        $grade->annotate_ids('scalefinal', 'rawscaleid');
+        $grade->annotate_ids('userfinal', 'loggeduser');
+        $grade->annotate_ids('userfinal', 'userid');
+        $grade->annotate_ids('userfinal', 'usermodified');
+
+        // Return the root element.
+        return $bookhistory;
+    }
+
+}
+
 /**
  * structure step in charge if constructing the completion.xml file for all the users completion
  * information in a given activity
@@ -2162,6 +2221,56 @@ class backup_activity_grades_structure_step extends backup_structure_step {
     }
 }
 
+/**
+ * Structure step in charge of constructing the grade history of an activity.
+ *
+ * This step is added to the task regardless of the setting 'grade_histories'.
+ * The reason is to allow for a more flexible step in case the logic needs to be
+ * split accross different settings to control the history of items and/or grades.
+ */
+class backup_activity_grade_history_structure_step extends backup_structure_step {
+
+    protected function define_structure() {
+
+        // Settings to use.
+        $userinfo = $this->get_setting_value('userinfo');
+        $history = $this->get_setting_value('grade_histories');
+
+        // Create the nested elements.
+        $bookhistory = new backup_nested_element('grade_history');
+        $grades = new backup_nested_element('grade_grades');
+        $grade = new backup_nested_element('grade_grade', array('id'), array(
+            'action', 'oldid', 'source', 'loggeduser', 'itemid', 'userid',
+            'rawgrade', 'rawgrademax', 'rawgrademin', 'rawscaleid',
+            'usermodified', 'finalgrade', 'hidden', 'locked', 'locktime', 'exported', 'overridden',
+            'excluded', 'feedback', 'feedbackformat', 'information',
+            'informationformat', 'timemodified'));
+
+        // Build the tree.
+        $bookhistory->add_child($grades);
+        $grades->add_child($grade);
+
+        // This only happens if we are including user info and history.
+        if ($userinfo && $history) {
+            // Define sources. Only select the history related to existing activity items.
+            $grade->set_source_sql("SELECT ggh.*
+                                     FROM {grade_grades_history} ggh
+                                     JOIN {backup_ids_temp} bi ON ggh.itemid = bi.itemid
+                                    WHERE bi.backupid = ?
+                                      AND bi.itemname = 'grade_item'", array(backup::VAR_BACKUPID));
+        }
+
+        // Annotations.
+        $grade->annotate_ids('scalefinal', 'rawscaleid'); // Straight as scalefinal because it's > 0.
+        $grade->annotate_ids('user', 'loggeduser');
+        $grade->annotate_ids('user', 'userid');
+        $grade->annotate_ids('user', 'usermodified');
+
+        // Return the root element.
+        return $bookhistory;
+    }
+}
+
 /**
  * Backups up the course completion information for the course.
  */
index cac6824..24a0456 100644 (file)
@@ -163,6 +163,9 @@ abstract class restore_activity_task extends restore_task {
         // Advanced grading methods attached to the module
         $this->add_step(new restore_activity_grading_structure_step('activity_grading', 'grading.xml'));
 
+        // Grade history. The setting 'grade_history' is handled in the step.
+        $this->add_step(new restore_activity_grade_history_structure_step('activity_grade_history', 'grade_history.xml'));
+
         // Userscompletion (conditionally)
         if ($this->get_setting_value('userscompletion')) {
             $this->add_step(new restore_userscompletion_structure_step('activity_userscompletion', 'completion.xml'));
index 985cd72..e355871 100644 (file)
@@ -58,6 +58,7 @@ class restore_final_task extends restore_task {
         // Gradebook. Don't restore the gradebook unless activities are being restored.
         if ($this->get_setting_value('activities')) {
             $this->add_step(new restore_gradebook_structure_step('gradebook_step','gradebook.xml'));
+            $this->add_step(new restore_grade_history_structure_step('grade_history', 'grade_history.xml'));
         }
 
         // Course completion, executed conditionally if restoring to new course
index 0239d01..61e2c5c 100644 (file)
@@ -246,5 +246,9 @@ class restore_root_task extends restore_task {
         $gradehistories->get_ui()->set_changeable($changeable);
         $this->add_setting($gradehistories);
         $users->add_dependency($gradehistories);
+
+        // The restore does not process the grade histories when some activities are ignored.
+        // So let's define a dependency to prevent false expectations from our users.
+        $activities->add_dependency($gradehistories);
     }
 }
index 5d82e3a..e625115 100644 (file)
@@ -224,6 +224,7 @@ class restore_gradebook_structure_step extends restore_structure_step {
         global $DB;
 
         $data = (object)$data;
+        $oldid = $data->id;
         $olduserid = $data->userid;
 
         $data->itemid = $this->get_new_parentid('grade_item');
@@ -243,6 +244,7 @@ class restore_gradebook_structure_step extends restore_structure_step {
                 $this->log($message, backup::LOG_DEBUG);
             } else {
                 $newitemid = $DB->insert_record('grade_grades', $data);
+                $this->set_mapping('grade_grades', $oldid, $newitemid);
             }
         } else {
             $message = "Mapped user id not found for user id '{$olduserid}', grade item id '{$data->itemid}'";
@@ -445,6 +447,85 @@ class restore_gradebook_structure_step extends restore_structure_step {
     }
 }
 
+/**
+ * Step in charge of restoring the grade history of a course.
+ *
+ * The execution conditions are itendical to {@link restore_gradebook_structure_step} because
+ * we do not want to restore the history if the gradebook and its content has not been
+ * restored. At least for now.
+ */
+class restore_grade_history_structure_step extends restore_structure_step {
+
+     protected function execute_condition() {
+        global $CFG, $DB;
+
+        // No gradebook info found, don't execute.
+        $fullpath = $this->task->get_taskbasepath();
+        $fullpath = rtrim($fullpath, '/') . '/' . $this->filename;
+        if (!file_exists($fullpath)) {
+            return false;
+        }
+
+        // Some module present in backup file isn't available to restore in this site, don't execute.
+        if ($this->task->is_missing_modules()) {
+            return false;
+        }
+
+        // Some activity has been excluded to be restored, don't execute.
+        if ($this->task->is_excluding_activities()) {
+            return false;
+        }
+
+        // There should only be one grade category (the 1 associated with the course itself).
+        $category = new stdclass();
+        $category->courseid  = $this->get_courseid();
+        $catcount = $DB->count_records('grade_categories', (array)$category);
+        if ($catcount > 1) {
+            return false;
+        }
+
+        // Arrived here, execute the step.
+        return true;
+     }
+
+    protected function define_structure() {
+        $paths = array();
+
+        // Settings to use.
+        $userinfo = $this->get_setting_value('users');
+        $history = $this->get_setting_value('grade_histories');
+
+        if ($userinfo && $history) {
+            $paths[] = new restore_path_element('grade_grade',
+               '/grade_history/grade_grades/grade_grade');
+        }
+
+        return $paths;
+    }
+
+    protected function process_grade_grade($data) {
+        global $DB;
+
+        $data = (object)($data);
+        $olduserid = $data->userid;
+        unset($data->id);
+
+        $data->userid = $this->get_mappingid('user', $data->userid, null);
+        if (!empty($data->userid)) {
+            // Do not apply the date offsets as this is history.
+            $data->itemid = $this->get_mappingid('grade_item', $data->itemid);
+            $data->oldid = $this->get_mappingid('grade_grades', $data->oldid);
+            $data->usermodified = $this->get_mappingid('user', $data->usermodified, null);
+            $data->rawscaleid = $this->get_mappingid('scale', $data->rawscaleid);
+            $DB->insert_record('grade_grades_history', $data);
+        } else {
+            $message = "Mapped user id not found for user id '{$olduserid}', grade item id '{$data->itemid}'";
+            $this->log($message, backup::LOG_DEBUG);
+        }
+    }
+
+}
+
 /**
  * decode all the interlinks present in restored content
  * relying 100% in the restore_decode_processor that handles
@@ -2898,6 +2979,7 @@ class restore_activity_grades_structure_step extends restore_structure_step {
     protected function process_grade_grade($data) {
         $data = (object)($data);
         $olduserid = $data->userid;
+        $oldid = $data->id;
         unset($data->id);
 
         $data->itemid = $this->get_new_parentid('grade_item');
@@ -2911,7 +2993,7 @@ class restore_activity_grades_structure_step extends restore_structure_step {
 
             $grade = new grade_grade($data, false);
             $grade->insert('restore');
-            // no need to save any grade_grade mapping
+            $this->set_mapping('grade_grades', $oldid, $grade->id);
         } else {
             debugging("Mapped user id not found for user id '{$olduserid}', grade item id '{$data->itemid}'");
         }
@@ -2944,6 +3026,52 @@ class restore_activity_grades_structure_step extends restore_structure_step {
     }
 }
 
+/**
+ * Step in charge of restoring the grade history of an activity.
+ *
+ * This step is added to the task regardless of the setting 'grade_histories'.
+ * The reason is to allow for a more flexible step in case the logic needs to be
+ * split accross different settings to control the history of items and/or grades.
+ */
+class restore_activity_grade_history_structure_step extends restore_structure_step {
+
+    protected function define_structure() {
+        $paths = array();
+
+        // Settings to use.
+        $userinfo = $this->get_setting_value('userinfo');
+        $history = $this->get_setting_value('grade_histories');
+
+        if ($userinfo && $history) {
+            $paths[] = new restore_path_element('grade_grade',
+               '/grade_history/grade_grades/grade_grade');
+        }
+
+        return $paths;
+    }
+
+    protected function process_grade_grade($data) {
+        global $DB;
+
+        $data = (object) $data;
+        $olduserid = $data->userid;
+        unset($data->id);
+
+        $data->userid = $this->get_mappingid('user', $data->userid, null);
+        if (!empty($data->userid)) {
+            // Do not apply the date offsets as this is history.
+            $data->itemid = $this->get_mappingid('grade_item', $data->itemid);
+            $data->oldid = $this->get_mappingid('grade_grades', $data->oldid);
+            $data->usermodified = $this->get_mappingid('user', $data->usermodified, null);
+            $data->rawscaleid = $this->get_mappingid('scale', $data->rawscaleid);
+            $DB->insert_record('grade_grades_history', $data);
+        } else {
+            $message = "Mapped user id not found for user id '{$olduserid}', grade item id '{$data->itemid}'";
+            $this->log($message, backup::LOG_DEBUG);
+        }
+    }
+
+}
 
 /**
  * This structure steps restores one instance + positions of one block
index 2af1c5d..8fb51b6 100644 (file)
@@ -78,7 +78,7 @@ class block_badges extends block_base {
         }
 
         // Number of badges to display.
-        if (empty($this->config->numberofbadges)) {
+        if (!isset($this->config->numberofbadges)) {
             $this->config->numberofbadges = 10;
         }
 
index 5bd7f42..df94445 100644 (file)
@@ -59,10 +59,34 @@ function block_course_overview_update_mynumber($number) {
 /**
  * Sets user course sorting preference in course_overview block
  *
- * @param array $sortorder sort order of course
+ * @param array $sortorder list of course ids
  */
 function block_course_overview_update_myorder($sortorder) {
-    set_user_preference('course_overview_course_order', serialize($sortorder));
+    $value = implode(',', $sortorder);
+    if (core_text::strlen($value) > 1333) {
+        // The value won't fit into the user preference. Remove courses in the end of the list (mostly likely user won't even notice).
+        $value = preg_replace('/,[\d]*$/', '', core_text::substr($value, 0, 1334));
+    }
+    set_user_preference('course_overview_course_sortorder', $value);
+}
+
+/**
+ * Gets user course sorting preference in course_overview block
+ *
+ * @return array list of course ids
+ */
+function block_course_overview_get_myorder() {
+    if ($value = get_user_preferences('course_overview_course_sortorder')) {
+        return explode(',', $value);
+    }
+    // If preference was not found, look in the old location and convert if found.
+    $order = array();
+    if ($value = get_user_preferences('course_overview_course_order')) {
+        $order = unserialize($value);
+        block_course_overview_update_myorder($order);
+        unset_user_preference('course_overview_course_order');
+    }
+    return $order;
 }
 
 /**
@@ -167,10 +191,7 @@ function block_course_overview_get_sorted_courses($showallcourses = false) {
         $courses[$remoteid] = $val;
     }
 
-    $order = array();
-    if (!is_null($usersortorder = get_user_preferences('course_overview_course_order'))) {
-        $order = unserialize($usersortorder);
-    }
+    $order = block_course_overview_get_myorder();
 
     $sortedcourses = array();
     $counter = 0;
index 4dc10eb..a16d200 100644 (file)
@@ -245,12 +245,5 @@ class block_glossary_random extends block_base {
 
         return $this->content;
     }
-
-    function hide_header() {
-        if (empty($this->config->title)) {
-            return true;
-        }
-        return false;
-    }
 }
 
index 9d6bc3e..1eed444 100644 (file)
@@ -5,6 +5,8 @@ Overview of this plugin type at http://docs.moodle.org/dev/Course_formats
 === 2.8 ===
 * The activity chooser now uses M.course.format.get_sectionwrapperclass()
   to determine the section selector, rather than a hard-coded `li.section`.
+* Activity duplication in /course/modduplicate.php is deprecated and is now done in /course/mod.php.  Deprecated calls will be honored by
+  redirecting to /course/mod.php for 3rd party plugin support.
 
 === 2.7 ===
 * The ->testedbrowsers array no longer needs to be defined in supports_ajax().
index bd01eeb..2cca5c7 100644 (file)
@@ -79,41 +79,16 @@ if (!empty($add)) {
     redirect("$CFG->wwwroot/course/modedit.php?update=$update&return=$returntomod&sr=$sectionreturn");
 
 } else if (!empty($duplicate)) {
-    $cm     = get_coursemodule_from_id('', $duplicate, 0, true, MUST_EXIST);
-    $course = $DB->get_record('course', array('id' => $cm->course), '*', MUST_EXIST);
+     $cm     = get_coursemodule_from_id('', $duplicate, 0, true, MUST_EXIST);
+     $course = $DB->get_record('course', array('id' => $cm->course), '*', MUST_EXIST);
 
     require_login($course, false, $cm);
-    $coursecontext = context_course::instance($course->id);
     $modcontext = context_module::instance($cm->id);
-    require_capability('moodle/course:manageactivities', $coursecontext);
-
-    if (!$confirm or !confirm_sesskey()) {
-        $PAGE->set_title(get_string('duplicate'));
-        $PAGE->set_heading($course->fullname);
-        $PAGE->navbar->add(get_string('duplicatinga', 'core', format_string($cm->name)));
-        $PAGE->set_pagelayout('incourse');
-
-        $a = new stdClass();
-        $a->modtype = get_string('modulename', $cm->modname);
-        $a->modname = format_string($cm->name);
-        $a->modid   = $cm->id;
+    require_capability('moodle/course:manageactivities', $modcontext);
 
-        echo $OUTPUT->header();
-        echo $OUTPUT->confirm(
-            get_string('duplicateconfirm', 'core', $a),
-            new single_button(
-                new moodle_url('/course/modduplicate.php', array(
-                    'cmid' => $cm->id, 'course' => $course->id, 'sr' => $sectionreturn)),
-                get_string('continue'),
-                'post'),
-            new single_button(
-                course_get_url($course, $cm->sectionnum, array('sr' => $sectionreturn)),
-                get_string('cancel'),
-                'get')
-        );
-        echo $OUTPUT->footer();
-        die();
-    }
+     // Duplicate the module.
+     $newcm = duplicate_module($course, $cm);
+     redirect(course_get_url($course, $cm->sectionnum, array('sr' => $sectionreturn)));
 
 } else if (!empty($delete)) {
     $cm     = get_coursemodule_from_id('', $delete, 0, true, MUST_EXIST);
@@ -298,5 +273,3 @@ if ((!empty($movetosection) or !empty($moveto)) and confirm_sesskey()) {
 } else {
     print_error('unknowaction');
 }
-
-
index 94e9e00..ae75273 100644 (file)
@@ -23,6 +23,7 @@
  *
  * @package    core
  * @subpackage course
+ * @deprecated Moodle 2.8 MDL-46428 - Now redirects to mod.php.
  * @copyright  2011 David Mudrak <david@moodle.com>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 require_once(dirname(dirname(__FILE__)) . '/config.php');
 
 $cmid           = required_param('cmid', PARAM_INT);
-$courseid       = required_param('course', PARAM_INT);
+$courseid       = optional_param('course', PARAM_INT);
 $sectionreturn  = optional_param('sr', null, PARAM_INT);
 
-$course     = $DB->get_record('course', array('id' => $courseid), '*', MUST_EXIST);
-$cm         = get_coursemodule_from_id('', $cmid, $course->id, true, MUST_EXIST);
-$cmcontext  = context_module::instance($cm->id);
-$context    = context_course::instance($courseid);
-$section    = $DB->get_record('course_sections', array('id' => $cm->section, 'course' => $cm->course));
+debugging('Please use moodle_url(\'/course/mod.php\', array(\'duplicate\' => $cmid
+    , \'id\' => $courseid, \'sesskey\' => sesskey(), \'sr\' => $sectionreturn)))
+    instead of new moodle_url(\'/course/modduplicate.php\', array(\'cmid\' => $cmid
+    , \'course\' => $courseid, \'sr\' => $sectionreturn))', DEBUG_DEVELOPER);
 
-require_login($course);
-require_sesskey();
-require_capability('moodle/course:manageactivities', $context);
-// Require both target import caps to be able to duplicate, see course_get_cm_edit_actions()
-require_capability('moodle/backup:backuptargetimport', $context);
-require_capability('moodle/restore:restoretargetimport', $context);
-
-$PAGE->set_title(get_string('duplicate'));
-$PAGE->set_heading($course->fullname);
-$PAGE->set_url(new moodle_url('/course/modduplicate.php', array('cmid' => $cm->id, 'courseid' => $course->id)));
-$PAGE->set_pagelayout('incourse');
-
-$output = $PAGE->get_renderer('core', 'backup');
-
-// Duplicate the module.
-$newcm = duplicate_module($course, $cm);
-
-echo $output->header();
-
-$a          = new stdClass();
-$a->modtype = get_string('modulename', $cm->modname);
-$a->modname = format_string($cm->name);
-
-if (!empty($newcm)) {
-    echo $output->confirm(
-        get_string('duplicatesuccess', 'core', $a),
-        new single_button(
-            new moodle_url('/course/modedit.php', array('update' => $newcm->id, 'sr' => $sectionreturn)),
-            get_string('duplicatecontedit'),
-            'get'),
-        new single_button(
-            course_get_url($course, $cm->sectionnum, array('sr' => $sectionreturn)),
-            get_string('duplicatecontcourse'),
-            'get')
-    );
-
-} else {
-    echo $output->notification(get_string('duplicatesuccess', 'core', $a), 'notifysuccess');
-    echo $output->continue_button(course_get_url($course, $cm->sectionnum, array('sr' => $sectionreturn)));
-}
-
-echo $output->footer();
+redirect(new moodle_url('/course/mod.php', array('duplicate' => $cmid, 'id' => $courseid,
+                                                 'sesskey' => sesskey(), 'sr' => $sectionreturn)));
index 580003b..aaacef4 100644 (file)
@@ -730,10 +730,6 @@ class behat_course extends behat_base {
             $steps[] = new Given('I open "' . $activity . '" actions menu');
         }
         $steps[] = new Given('I click on "' . get_string('duplicate') . '" "link" in the "' . $activity . '" activity');
-        if (!$this->running_javascript()) {
-            $steps[] = new Given('I press "' . get_string('continue') .'"');
-            $steps[] = new Given('I press "' . get_string('duplicatecontcourse') .'"');
-        }
         return $steps;
     }
 
@@ -752,35 +748,35 @@ class behat_course extends behat_base {
         $activity = $this->escape($activityname);
         $activityliteral = $this->getSession()->getSelectorsHandler()->xpathLiteral($activityname);
 
-        if ($this->running_javascript()) {
-            $steps[] = new Given('I duplicate "' . $activity . '" activity');
+        $steps[] = new Given('I duplicate "' . $activity . '" activity');
 
+        // Determine the future new activity xpath from the former one.
+        $duplicatedxpath = "//li[contains(concat(' ', normalize-space(@class), ' '), ' activity ')]" .
+            "[contains(., $activityliteral)]/following-sibling::li";
+        $duplicatedactionsmenuxpath = $duplicatedxpath . "/descendant::a[@role='menuitem']";
+
+        if ($this->running_javascript()) {
             // We wait until the AJAX request finishes and the section is visible again.
-            $hiddenlightboxxpath = "//li[contains(concat(' ', normalize-space(@class), ' '), ' activity ')][contains(., $activityliteral)]" .
+            $hiddenlightboxxpath = "//li[contains(concat(' ', normalize-space(@class), ' '), ' activity ')]" .
+                "[contains(., $activityliteral)]" .
                 "/ancestor::li[contains(concat(' ', normalize-space(@class), ' '), ' section ')]" .
                 "/descendant::div[contains(concat(' ', @class, ' '), ' lightbox ')][contains(@style, 'display: none')]";
+
             $steps[] = new Given('I wait until the page is ready');
             $steps[] = new Given('I wait until "' . $this->escape($hiddenlightboxxpath) .'" "xpath_element" exists');
 
             // Close the original activity actions menu.
             $steps[] = new Given('I close "' . $activity . '" actions menu');
 
-            // Determine the future new activity xpath from the former one.
-            $duplicatedxpath = "//li[contains(concat(' ', normalize-space(@class), ' '), ' activity ')][contains(., $activityliteral)]" .
-                "/following-sibling::li";
-            $duplicatedactionsmenuxpath = $duplicatedxpath . "/descendant::a[@role='menuitem']";
-
             // The next sibling of the former activity will be the duplicated one, so we click on it from it's xpath as, at
             // this point, it don't even exists in the DOM (the steps are executed when we return them).
             $steps[] = new Given('I click on "' . $this->escape($duplicatedactionsmenuxpath) . '" "xpath_element"');
-
-            // We force the xpath as otherwise mink tries to interact with the former one.
-            $steps[] = new Given('I click on "' . get_string('editsettings') . '" "link" in the "' . $this->escape($duplicatedxpath) . '" "xpath_element"');
-        } else {
-            $steps[] = new Given('I click on "' . get_string('duplicate') . '" "link" in the "' . $activity . '" activity');
-            $steps[] = new Given('I press "' . get_string('continue') .'"');
-            $steps[] = new Given('I press "' . get_string('duplicatecontedit') . '"');
         }
+
+        // We force the xpath as otherwise mink tries to interact with the former one.
+        $steps[] = new Given('I click on "' . get_string('editsettings') . '" "link" in the "' .
+            $this->escape($duplicatedxpath) . '" "xpath_element"');
+
         $steps[] = new Given('I set the following fields to these values:', $data);
         $steps[] = new Given('I press "' . get_string('savechangesandreturntocourse') . '"');
         return $steps;
index e97478d..fdf63c5 100644 (file)
@@ -52,7 +52,7 @@ class core_files_external extends external_api {
                 'filearea'     => new external_value(PARAM_TEXT, 'file area'),
                 'itemid'       => new external_value(PARAM_INT, 'associated id'),
                 'filepath'     => new external_value(PARAM_PATH, 'file path'),
-                'filename'     => new external_value(PARAM_FILE, 'file name'),
+                'filename'     => new external_value(PARAM_TEXT, 'file name'),
                 'modified'     => new external_value(PARAM_INT, 'timestamp to return files changed after this time.', VALUE_DEFAULT, null),
                 'contextlevel' => new external_value(PARAM_ALPHA, 'The context level for the file location.', VALUE_DEFAULT, null),
                 'instanceid'   => new external_value(PARAM_INT, 'The instance id for where the file is located.', VALUE_DEFAULT, null)
@@ -112,9 +112,6 @@ class core_files_external extends external_api {
         if (empty($fileinfo['filearea'])) {
             $fileinfo['filearea'] = null;
         }
-        if (empty($fileinfo['itemid'])) {
-            $fileinfo['itemid'] = null;
-        }
         if (empty($fileinfo['filename'])) {
             $fileinfo['filename'] = null;
         }
@@ -209,7 +206,7 @@ class core_files_external extends external_api {
                             'filearea'  => new external_value(PARAM_AREA, ''),
                             'itemid'   => new external_value(PARAM_INT, ''),
                             'filepath' => new external_value(PARAM_TEXT, ''),
-                            'filename' => new external_value(PARAM_FILE, ''),
+                            'filename' => new external_value(PARAM_TEXT, ''),
                             'isdir'    => new external_value(PARAM_BOOL, ''),
                             'url'      => new external_value(PARAM_TEXT, ''),
                             'timemodified' => new external_value(PARAM_INT, ''),
index b497034..49e272e 100644 (file)
@@ -796,7 +796,7 @@ class core_files_renderer extends plugin_renderer_base {
             <div class="fp-author">'.get_string('author', 'repository').'<span class="fp-value"></span></div>
             <div class="fp-dimensions">'.get_string('dimensions', 'repository').'<span class="fp-value"></span></div>
         </div>
-    <div>
+    </div>
 </div>';
         return $rv;
     }
index e48643d..81dfe81 100644 (file)
@@ -368,6 +368,14 @@ if ($moving) {
 
 echo $OUTPUT->container_end();
 
+$PAGE->requires->yui_module('moodle-core-formchangechecker',
+    'M.core_formchangechecker.init',
+    array(array(
+        'formid' => 'gradetreeform'
+    ))
+);
+$PAGE->requires->string_for_js('changesmadereallygoaway', 'moodle');
+
 echo $OUTPUT->footer();
 
 // Restore original show/hide preference if moving
index c05eb92..ba65d9d 100644 (file)
@@ -671,6 +671,7 @@ class grade_edit_tree_column_aggregation extends grade_edit_tree_column_category
         } else {
             $attributes = array();
             $attributes['id'] = 'aggregation_'.$category->id;
+            $attributes['class'] = 'ignoredirty';
             $aggregation = html_writer::label(get_string('aggregation', 'grades'), 'aggregation_'.$category->id, false, array('class' => 'accesshide'));
             $aggregation .= html_writer::select($options, 'aggregation_'.$category->id, $category->aggregation, null, $attributes);
             $action = new component_action('change', 'update_category_aggregation', array('courseid' => $params['id'], 'category' => $category->id, 'sesskey' => sesskey()));
index c15f136..b481be4 100644 (file)
 .user-grade thead {border-bottom: 3px double black;}
 .user-grade thead th {padding: 0.25em 0.75em;}
 .user-grade tbody th {text-align:left;}
-.user-grade td.oddd1,
-.user-grade th.oddd1 {background-color: #f3dfd0;}
-.user-grade td.oddd2,
-.user-grade th.oddd2 {background-color: #d0dbf3;}
-.user-grade td.oddd3,
-.user-grade th.oddd3 {background-color: #d0f3d6;}
-.user-grade td.oddd4,
-.user-grade th.oddd4 {background-color: #f0f0aa;}
-.user-grade td.evend2,
-.user-grade th.evend2 {background-color: #b0bbd3;}
-.user-grade td.evend3,
-.user-grade th.evend3 {background-color: #b0dfb6;}
-.user-grade td.evend4,
-.user-grade th.evend4 {background-color: #cac8be;}
+/* These need to be this specific to override the css from .generaltable */
+.user-grade tbody > tr:nth-child(n) > td.oddd1,
+.user-grade tbody > tr:nth-child(n) > th.oddd1 {background-color: #f3dfd0;}
+.user-grade tbody > tr:nth-child(n) > td.oddd2,
+.user-grade tbody > tr:nth-child(n) > th.oddd2 {background-color: #d0dbf3;}
+.user-grade tbody > tr:nth-child(n) > td.oddd3,
+.user-grade tbody > tr:nth-child(n) > th.oddd3 {background-color: #d0f3d6;}
+.user-grade tbody > tr:nth-child(n) > td.oddd4,
+.user-grade tbody > tr:nth-child(n) > th.oddd4 {background-color: #f0f0aa;}
+.user-grade tbody > tr:nth-child(n) > td.evend2,
+.user-grade tbody > tr:nth-child(n) > th.evend2 {background-color: #b0bbd3;}
+.user-grade tbody > tr:nth-child(n) > td.evend3,
+.user-grade tbody > tr:nth-child(n) > th.evend3 {background-color: #b0dfb6;}
+.user-grade tbody > tr:nth-child(n) > td.evend4,
+.user-grade tbody > tr:nth-child(n) > th.evend4 {background-color: #cac8be;}
 .user-grade td.b1t,
 .user-grade td.b2t,
 .user-grade th.b1t,
index 61130db..be88703 100644 (file)
@@ -522,11 +522,6 @@ $string['downloadtext'] = 'Download in text format';
 $string['doyouagree'] = 'Have you read these conditions and understood them?';
 $string['droptoupload'] = 'Drop files here to upload';
 $string['duplicate'] = 'Duplicate';
-$string['duplicateconfirm'] = 'Are you sure you want to duplicate {$a->modtype} \'{$a->modname}\' ?';
-$string['duplicatecontcourse'] = 'Return to the course';
-$string['duplicatecontedit'] = 'Edit the new copy';
-$string['duplicatesuccess'] = '{$a->modtype} \'{$a->modname}\' has been duplicated successfully';
-$string['duplicatinga'] = 'Duplicating: {$a}';
 $string['edhelpaspellpath'] = 'To use spell-checking within the editor, you MUST have <strong>aspell 0.50</strong> or later installed on your server, and you must specify the correct path to access the aspell binary.  On Unix/Linux systems, this path is usually <strong>/usr/bin/aspell</strong>, but it might be something else.';
 $string['edhelpbgcolor'] = 'Define the edit area\'s background color.<br />Valid values are, for example: #FFFFFF or white';
 $string['edhelpcleanword'] = 'This setting enables or disables Word-specific format filtering.';
index 40cc593..6969164 100644 (file)
Binary files a/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-debug.js and b/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-debug.js differ
index 068aab8..5ce80c8 100644 (file)
Binary files a/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-min.js and b/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-min.js differ
index cb7326d..d743822 100644 (file)
Binary files a/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor.js and b/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor.js differ
index 33b0b88..07598bc 100644 (file)
@@ -215,7 +215,10 @@ EditorAutosave.prototype = {
      * @chainable
      */
     saveDraft: function() {
-        this.updateOriginal();
+        // Only copy the text from the div to the textarea if the textarea is not currently visible.
+        if (!this.editor.get('hidden')) {
+            this.updateOriginal();
+        }
         var newText = this.textarea.get('value');
 
         if (newText !== this.lastText) {
index 49ebc48..f9881c1 100644 (file)
@@ -84,7 +84,7 @@ EditorNotify.prototype = {
             this.messageOverlay = Y.Node.create('<div class="editor_atto_notification"></div>');
 
             this.messageOverlay.hide(true);
-            this._wrapper.append(this.messageOverlay);
+            this.textarea.get('parentNode').append(this.messageOverlay);
 
             this.messageOverlay.on('click', function() {
                 this.messageOverlay.hide(true);
index aba7b66..1522a35 100644 (file)
@@ -127,6 +127,11 @@ class behat_data_generators extends behat_base {
             'datagenerator' => 'cohort',
             'required' => array('idnumber')
         ),
+        'cohort members' => array(
+            'datagenerator' => 'cohort_member',
+            'required' => array('user', 'cohort'),
+            'switchids' => array('user' => 'userid', 'cohort' => 'cohortid')
+        ),
         'roles' => array(
             'datagenerator' => 'role',
             'required' => array('shortname')
@@ -412,6 +417,16 @@ class behat_data_generators extends behat_base {
         $this->datagenerator->create_role($data);
     }
 
+    /**
+     * Adds members to cohorts
+     *
+     * @param array $data
+     * @return void
+     */
+    protected function process_cohort_member($data) {
+        cohort_add_member($data['cohortid'], $data['userid']);
+    }
+
     /**
      * Gets the user id from it's username.
      * @throws Exception
@@ -509,6 +524,21 @@ class behat_data_generators extends behat_base {
         return $id;
     }
 
+    /**
+     * Gets the cohort id from it's idnumber.
+     * @throws Exception
+     * @param string $idnumber
+     * @return int
+     */
+    protected function get_cohort_id($idnumber) {
+        global $DB;
+
+        if (!$id = $DB->get_field('cohort', 'id', array('idnumber' => $idnumber))) {
+            throw new Exception('The specified cohort with idnumber "' . $idnumber . '" does not exist');
+        }
+        return $id;
+    }
+
     /**
      * Gets the internal context id from the context reference.
      *
index 7a98b89..7a2d78d 100644 (file)
@@ -44,6 +44,7 @@ class message_output_airnotifier extends message_output {
      */
     public function send_message($eventdata) {
         global $CFG;
+        require_once($CFG->libdir . '/filelib.php');
 
         if (!empty($CFG->noemailever)) {
             // Hidden setting for development sites, set in config.php if needed.
index be4a8a8..0bde754 100644 (file)
@@ -58,7 +58,7 @@ class assign_feedback_editpdf extends assign_feedback_plugin {
      */
     public function get_widget($userid, $grade, $readonly) {
         $attempt = -1;
-        if ($grade) {
+        if ($grade && $grade->attemptnumber) {
             $attempt = $grade->attemptnumber;
         } else {
             $grade = $this->assignment->get_user_grade($userid, true);
index 4f75acb..4a1ee07 100644 (file)
@@ -66,3 +66,75 @@ Feature: In an assignment, teacher can annotate PDF files during grading
     And I click on "Close" "button"
     And I press "Save changes"
     And I should see "The grade changes were saved"
+
+  @javascript
+  Scenario: Submit a PDF file as a student in a team and annotate the PDF as a teacher
+    Given ghostscript is installed
+    And the following "courses" exist:
+      | fullname | shortname | category | groupmode |
+      | Course 1 | C1 | 0 | 1 |
+    And the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@asd.com |
+      | student1 | Student | 1 | student1@asd.com |
+      | student2 | Student | 2 | student2@asd.com |
+      | student3 | Student | 3 | student3@asd.com |
+      | student4 | Student | 4 | student4@asd.com |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | student1 | C1 | student |
+      | student2 | C1 | student |
+      | student3 | C1 | student |
+      | student4 | C1 | student |
+    And the following "groups" exist:
+      | name | course | idnumber |
+      | G1 | C1 | G1 |
+      | G2 | C1 | G2 |
+    And the following "groupings" exist:
+      | name | course | idnumber |
+      | G1   | C1     | G1       |
+    And the following "group members" exist:
+      | user        | group |
+      | student1    | G1  |
+      | student2    | G1  |
+      | student3    | G2  |
+      | student4    | G2  |
+    And the following "grouping groups" exist:
+      | grouping | group |
+      | G1       | G1    |
+      | G1       | G2    |
+    And I log in as "teacher1"
+    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 PDF file |
+      | assignsubmission_file_enabled | 1 |
+      | Maximum number of uploaded files | 2 |
+      | Students submit in groups | Yes |
+      | Grouping for student groups | G1 |
+    And I log out
+    When I log in as "student1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I press "Add submission"
+    And I upload "mod/assign/feedback/editpdf/tests/fixtures/submission.pdf" file to "File submissions" filemanager
+    And I press "Save changes"
+    Then I should see "Submitted for grading"
+    And I should see "submission.pdf"
+    And I should see "Not graded"
+    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" "link" in the "Student 2" "table_row"
+    And I follow "Launch PDF editor..."
+    And I click on ".linebutton" "css_element"
+    And I drag ".drawingcanvas" "css_element" and I drop it in ".assignfeedback_editpdf_widget" "css_element"
+    And I click on "Close" "button"
+    And I press "Save changes"
+    And I should see "The grade changes were saved"
+    And I press "Continue"
+    And I should see "View annotated PDF..." in the "Student 1" "table_row"
index b630ab7..cc43934 100644 (file)
@@ -72,7 +72,7 @@ class mod_forum_observer {
         require_once($CFG->dirroot . '/mod/forum/lib.php');
 
         $userid = $event->relateduserid;
-        $sql = "SELECT f.id, cm.id AS cmid, f.forcesubscribe
+        $sql = "SELECT f.id, f.course as course, cm.id AS cmid, f.forcesubscribe
                   FROM {forum} f
                   JOIN {course_modules} cm ON (cm.instance = f.id)
                   JOIN {modules} m ON (m.id = cm.module)
index de4253b..79156a6 100644 (file)
@@ -104,14 +104,18 @@ class subscriptions {
      * @param int $userid The user ID
      * @param \stdClass $forum The record of the forum to test
      * @param int $discussionid The ID of the discussion to check
+     * @param \cm_info $cm The coursemodule record. If not supplied, this will be calculated using get_fast_modinfo instead.
      * @return boolean
      */
-    public static function is_subscribed($userid, $forum, $discussionid = null) {
+    public static function is_subscribed($userid, $forum, $discussionid = null, cm_info $cm = null) {
         // If forum is force subscribed and has allowforcesubscribe, then user is subscribed.
-        $cm = get_coursemodule_from_instance('forum', $forum->id);
-        if ($cm && self::is_forcesubscribed($forum) &&
-                has_capability('mod/forum:allowforcesubscribe', \context_module::instance($cm->id), $userid)) {
-            return true;
+        if (self::is_forcesubscribed($forum)) {
+            if (!$cm) {
+                $cm = get_fast_modinfo($forum->course)->instances['forum'][$forum->id];
+            }
+            if (has_capability('mod/forum:allowforcesubscribe', \context_module::instance($cm->id), $userid)) {
+                return true;
+            }
         }
 
         if ($discussionid === null) {
index dcded7a..eb05d32 100644 (file)
         $subscriptionchanges = array();
         foreach ($potentialsubscribers as $subuser) {
             $userid = $subuser->id;
-            $targetsubscription = \mod_forum\subscriptions::is_subscribed($userid, $forumto);
+            $targetsubscription = \mod_forum\subscriptions::is_subscribed($userid, $forumto, null, $cmto);
             if (\mod_forum\subscriptions::is_subscribed($userid, $forum, $discussion->id)) {
                 if (!$targetsubscription) {
                     $subscriptionchanges[$userid] = \mod_forum\subscriptions::FORUM_DISCUSSION_SUBSCRIBED;
index 1d4ac71..b06f87e 100644 (file)
@@ -192,7 +192,7 @@ if (!is_null($subscribe)) {
             $cansub = false;
         }
         if (!\mod_forum\subscriptions::is_forcesubscribed($forum)) {
-            $subscribed = \mod_forum\subscriptions::is_subscribed($USER->id, $forum);
+            $subscribed = \mod_forum\subscriptions::is_subscribed($USER->id, $forum, null, $cm);
             $canmanageactivities = has_capability('moodle/course:manageactivities', $coursecontext, $USER->id);
             if (($canmanageactivities || \mod_forum\subscriptions::is_subscribable($forum)) && $subscribe && !$subscribed && $cansub) {
                 \mod_forum\subscriptions::subscribe_user($USER->id, $forum, $modcontext, true);
index 7e578f3..b3a63b1 100644 (file)
@@ -615,7 +615,7 @@ function forum_cron() {
                     continue;
                 }
 
-                if (!\mod_forum\subscriptions::is_subscribed($userto->id, $forum, $post->discussion)) {
+                if (!\mod_forum\subscriptions::is_subscribed($userto->id, $forum, $post->discussion, $coursemodules[$forum->id])) {
                     // The user does not subscribe to this forum, or to this specific discussion.
                     continue;
                 }
index 516128b..b1bdd9f 100644 (file)
@@ -573,7 +573,8 @@ $mform_post = new mod_forum_post_form('post.php', array('course' => $course,
                                                         'modcontext' => $modcontext,
                                                         'forum' => $forum,
                                                         'post' => $post,
-                                                        'subscribe' => \mod_forum\subscriptions::is_subscribed($USER->id, $forum),
+                                                        'subscribe' => \mod_forum\subscriptions::is_subscribed($USER->id, $forum,
+                                                                null, $cm),
                                                         'thresholdwarning' => $thresholdwarning,
                                                         'edit' => $edit), 'post', '', array('id' => 'mformforum'));
 
@@ -616,7 +617,7 @@ $currenttext = file_prepare_draft_area($draftid_editor, $modcontext->id, 'mod_fo
 // which case use their existing preference.
 $discussionsubscribe = true;
 if (isset($discussion) && forum_user_has_posted($forum->id, $discussion->id, $USER->id)) {
-    $discussionsubscribe = \mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id);
+    $discussionsubscribe = \mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id, $cm);
 }
 $mform_post->set_data(array(        'attachments'=>$draftitemid,
                                     'general'=>$heading,
index 960b443..b55616a 100644 (file)
@@ -73,7 +73,7 @@ if (isset($cm->groupmode) && empty($course->groupmodeforce)) {
 } else {
     $groupmode = $course->groupmode;
 }
-if ($groupmode && !\mod_forum\subscriptions::is_subscribed($user->id, $forum) && !has_capability('moodle/site:accessallgroups', $context)) {
+if ($groupmode && !\mod_forum\subscriptions::is_subscribed($user->id, $forum, null, $cm) && !has_capability('moodle/site:accessallgroups', $context)) {
     if (!groups_get_all_groups($course->id, $USER->id)) {
         print_error('cannotsubscribe', 'forum');
     }
@@ -142,7 +142,7 @@ $info = new stdClass();
 $info->name  = fullname($user);
 $info->forum = format_string($forum->name);
 
-if (\mod_forum\subscriptions::is_subscribed($user->id, $forum, $discussionid)) {
+if (\mod_forum\subscriptions::is_subscribed($user->id, $forum, $discussionid, $cm)) {
     if (is_null($sesskey)) {    // we came here via link in email
         $PAGE->set_title($course->shortname);
         $PAGE->set_heading($course->fullname);
index ef2cbc7..d2a867a 100644 (file)
@@ -46,7 +46,7 @@ if (!\mod_forum\subscriptions::is_subscribable($forum)) {
     die;
 }
 
-if (\mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id)) {
+if (\mod_forum\subscriptions::is_subscribed($USER->id, $forum, $discussion->id, $cm)) {
     // The user is subscribed, unsubscribe them.
     \mod_forum\subscriptions::unsubscribe_user_from_discussion($USER->id, $discussion, $context);
 } else {
index 0d358fe..f161bdb 100644 (file)
@@ -1433,4 +1433,53 @@ class mod_forum_subscriptions_testcase extends advanced_testcase {
         )));
     }
 
+    /**
+     * Test that providing a context_module instance to is_subscribed does not result in additional lookups to retrieve
+     * the context_module.
+     */
+    public function test_is_subscribed_cm() {
+        global $DB;
+
+        $this->resetAfterTest(true);
+
+        // Create a course, with a forum.
+        $course = $this->getDataGenerator()->create_course();
+
+        $options = array('course' => $course->id, 'forcesubscribe' => FORUM_FORCESUBSCRIBE);
+        $forum = $this->getDataGenerator()->create_module('forum', $options);
+
+        // Create a user enrolled in the course as a student.
+        list($user) = $this->helper_create_users($course, 1);
+
+        // Retrieve the $cm now.
+        $cm = get_fast_modinfo($forum->course)->instances['forum'][$forum->id];
+
+        // Reset get_fast_modinfo.
+        get_fast_modinfo(0, 0, true);
+
+        // Call is_subscribed without passing the $cmid - this should result in a lookup and filling of some of the
+        // caches. This provides us with consistent data to start from.
+        $this->assertTrue(\mod_forum\subscriptions::is_subscribed($user->id, $forum));
+        $this->assertTrue(\mod_forum\subscriptions::is_subscribed($user->id, $forum));
+
+        // Make a note of the number of DB calls.
+        $basecount = $DB->perf_get_reads();
+
+        // Call is_subscribed - it should give return the correct result (False), and result in no additional queries.
+        $this->assertTrue(\mod_forum\subscriptions::is_subscribed($user->id, $forum, null, $cm));
+
+        // The capability check does require some queries, so we don't test it directly.
+        // We don't assert here because this is dependant upon linked code which could change at any time.
+        $suppliedcmcount = $DB->perf_get_reads() - $basecount;
+
+        // Call is_subscribed without passing the $cmid now - this should result in a lookup.
+        get_fast_modinfo(0, 0, true);
+        $basecount = $DB->perf_get_reads();
+        $this->assertTrue(\mod_forum\subscriptions::is_subscribed($user->id, $forum));
+        $calculatedcmcount = $DB->perf_get_reads() - $basecount;
+
+        // There should be more queries than when we performed the same check a moment ago.
+        $this->assertGreaterThan($suppliedcmcount, $calculatedcmcount);
+    }
+
 }
index daaa0f6..61910e8 100644 (file)
 /**
  * Code for loading and saving question attempts to and from the database.
  *
- * A note for future reference. This code is pretty efficient but there are two
+ * Note that many of the methods of this class should be considered private to
+ * the question engine. They should be accessed through the
+ * {@link question_engine} class. For example, you should call
+ * {@link question_engine::save_questions_usage_by_activity()} rather than
+ * {@link question_engine_data_mapper::insert_questions_usage_by_activity()}.
+ * The exception to this is some of the reporting methods, like
+ * {@link question_engine_data_mapper::load_attempts_at_question()}.
+ *
+ * (TODO, probably we should split this class up, so that it has no public
+ * methods. They should all be moved to a new public class.)
+ *
+ * A note for future reference. This code is pretty efficient but there are some
  * potential optimisations that could be contemplated, at the cost of making the
  * code more complex:
  *
- * 1. (This is the easier one, but probably not worth doing.) In the unit-of-work
- *    save method, we could get all the ids for steps due to be deleted or modified,
+ * 1. (This is probably not worth doing.) In the unit-of-work save method, we
+ *    could get all the ids for steps due to be deleted or modified,
  *    and delete all the question_attempt_step_data for all of those steps in one
  *    query. That would save one DB query for each ->stepsupdated. However that number
  *    is 0 except when re-grading, and when regrading, there are many more inserts
  *    into question_attempt_step_data than deletes, so it is really hardly worth it.
  *
- * 2. A more significant optimisation would be to write an efficient
- *    $DB->insert_records($arrayofrecords) method (for example using functions
- *    like pg_copy_from) and then whenever we save stuff (unit_of_work->save and
- *    insert_questions_usage_by_activity) collect together all the records that
- *    need to be inserted into question_attempt_step_data, and insert them with
- *    a single call to $DB->insert_records. This is likely to be the biggest win.
- *    We do a lot of separate inserts into question_attempt_step_data.
- *
- * @package    moodlecore
- * @subpackage questionengine
+ * @package    core_question
  * @copyright  2009 The Open University
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -75,6 +77,10 @@ class question_engine_data_mapper {
     /**
      * Store an entire {@link question_usage_by_activity} in the database,
      * including all the question_attempts that comprise it.
+     *
+     * You should not call this method directly. You should use
+     * @link question_engine::save_questions_usage_by_activity()}.
+     *
      * @param question_usage_by_activity $quba the usage to store.
      */
     public function insert_questions_usage_by_activity(question_usage_by_activity $quba) {
@@ -86,16 +92,30 @@ class question_engine_data_mapper {
         $newid = $this->db->insert_record('question_usages', $record);
         $quba->set_id_from_database($newid);
 
+        // Initially an array of array of question_attempt_step_objects.
+        // Built as a nested array for efficiency, then flattened.
+        $stepdata = array();
+
         foreach ($quba->get_attempt_iterator() as $qa) {
-            $this->insert_question_attempt($qa, $quba->get_owning_context());
+            $stepdata[] = $this->insert_question_attempt($qa, $quba->get_owning_context());
+        }
+
+        $stepdata = call_user_func_array('array_merge', $stepdata);
+        if ($stepdata) {
+            $this->insert_all_step_data($stepdata);
         }
     }
 
     /**
      * Store an entire {@link question_attempt} in the database,
      * including all the question_attempt_steps that comprise it.
+     *
+     * You should not call this method directly. You should use
+     * @link question_engine::save_questions_usage_by_activity()}.
+     *
      * @param question_attempt $qa the question attempt to store.
      * @param context $context the context of the owning question_usage_by_activity.
+     * @return array of question_attempt_step_data rows, that still need to be inserted.
      */
     public function insert_question_attempt(question_attempt $qa, $context) {
         $record = new stdClass();
@@ -120,9 +140,15 @@ class question_engine_data_mapper {
         $record->id = $this->db->insert_record('question_attempts', $record);
         $qa->set_database_id($record->id);
 
+        // Initially an array of array of question_attempt_step_objects.
+        // Built as a nested array for efficiency, then flattened.
+        $stepdata = array();
+
         foreach ($qa->get_step_iterator() as $seq => $step) {
-            $this->insert_question_attempt_step($step, $record->id, $seq, $context);
+            $stepdata[] = $this->insert_question_attempt_step($step, $record->id, $seq, $context);
         }
+
+        return call_user_func_array('array_merge', $stepdata);
     }
 
     /**
@@ -148,8 +174,10 @@ class question_engine_data_mapper {
      * @param question_attempt_step $step the step to store.
      * @param int $stepid the id of the step.
      * @param context $context the context of the owning question_usage_by_activity.
+     * @return array of question_attempt_step_data rows, that still need to be inserted.
      */
-    protected function insert_step_data(question_attempt_step $step, $stepid, $context) {
+    protected function prepare_step_data(question_attempt_step $step, $stepid, $context) {
+        $rows = array();
         foreach ($step->get_all_data() as $name => $value) {
             if ($value instanceof question_file_saver) {
                 $value->save_files($stepid, $context);
@@ -162,16 +190,35 @@ class question_engine_data_mapper {
             $data->attemptstepid = $stepid;
             $data->name = $name;
             $data->value = $value;
-            $this->db->insert_record('question_attempt_step_data', $data, false);
+            $rows[] = $data;
         }
+        return $rows;
+    }
+
+    /**
+     * Insert a lot of records into question_attempt_step_data in one go.
+     *
+     * Private method, only for use by other parts of the question engine.
+     *
+     * @param array $rows the rows to insert.
+     */
+    public function insert_all_step_data(array $rows) {
+        if (!$rows) {
+            return;
+        }
+        $this->db->insert_records('question_attempt_step_data', $rows);
     }
 
     /**
      * Store a {@link question_attempt_step} in the database.
+     *
+     * Private method, only for use by other parts of the question engine.
+     *
      * @param question_attempt_step $step the step to store.
      * @param int $questionattemptid the question attept id this step belongs to.
      * @param int $seq the sequence number of this stop.
      * @param context $context the context of the owning question_usage_by_activity.
+     * @return array of question_attempt_step_data rows, that still need to be inserted.
      */
     public function insert_question_attempt_step(question_attempt_step $step,
             $questionattemptid, $seq, $context) {
@@ -179,15 +226,19 @@ class question_engine_data_mapper {
         $record = $this->make_step_record($step, $questionattemptid, $seq);
         $record->id = $this->db->insert_record('question_attempt_steps', $record);
 
-        $this->insert_step_data($step, $record->id, $context);
+        return $this->prepare_step_data($step, $record->id, $context);
     }
 
     /**
      * Update a {@link question_attempt_step} in the database.
+     *
+     * Private method, only for use by other parts of the question engine.
+     *
      * @param question_attempt_step $qa the step to store.
      * @param int $questionattemptid the question attept id this step belongs to.
      * @param int $seq the sequence number of this stop.
      * @param context $context the context of the owning question_usage_by_activity.
+     * @return array of question_attempt_step_data rows, that still need to be inserted.
      */
     public function update_question_attempt_step(question_attempt_step $step,
             $questionattemptid, $seq, $context) {
@@ -198,11 +249,14 @@ class question_engine_data_mapper {
 
         $this->db->delete_records('question_attempt_step_data',
                 array('attemptstepid' => $record->id));
-        $this->insert_step_data($step, $record->id, $context);
+        return $this->prepare_step_data($step, $record->id, $context);
     }
 
     /**
      * Load a {@link question_attempt_step} from the database.
+     *
+     * Private method, only for use by other parts of the question engine.
+     *
      * @param int $stepid the id of the step to load.
      * @param question_attempt_step the step that was loaded.
      */
@@ -244,6 +298,11 @@ WHERE
     /**
      * Load a {@link question_attempt} from the database, including all its
      * steps.
+     *
+     * Normally, you should use {@link question_engine::load_questions_usage_by_activity()}
+     * but there may be rare occasions where for performance reasons, you only
+     * wish to load one qa, in which case you may call this method.
+     *
      * @param int $questionattemptid the id of the question attempt to load.
      * @param question_attempt the question attempt that was loaded.
      */
@@ -302,6 +361,10 @@ ORDER BY
     /**
      * Load a {@link question_usage_by_activity} from the database, including
      * all its {@link question_attempt}s and all their steps.
+     *
+     * You should call {@link question_engine::load_questions_usage_by_activity()}
+     * rather than calling this method directly.
+     *
      * @param int $qubaid the id of the usage to load.
      * @param question_usage_by_activity the usage that was loaded.
      */
@@ -361,6 +424,9 @@ ORDER BY
     /**
      * Load all {@link question_usage_by_activity} from the database for one qubaid_condition
      * Include all its {@link question_attempt}s and all their steps.
+     *
+     * This method may be called publicly.
+     *
      * @param qubaid_condition $qubaids the condition that tells us which usages to load.
      * @return question_usage_by_activity[] the usages that were loaded.
      */
@@ -426,6 +492,8 @@ ORDER BY
     /**
      * Load information about the latest state of each question from the database.
      *
+     * This method may be called publicly.
+     *
      * @param qubaid_condition $qubaids used to restrict which usages are included
      *                                  in the query. See {@link qubaid_condition}.
      * @param array            $slots   A list of slots for the questions you want to know about.
@@ -481,6 +549,8 @@ WHERE
      * attempts. This is used, for example, by the quiz manual grading report,
      * to show how many attempts at each question need to be graded.
      *
+     * This method may be called publicly.
+     *
      * @param qubaid_condition $qubaids used to restrict which usages are included
      * in the query. See {@link qubaid_condition}.
      * @param array $slots A list of slots for the questions you want to konw about.
@@ -561,6 +631,8 @@ ORDER BY
      * $limitnum. A special value 'random' can be passed as $orderby, in which case
      * $limitfrom is ignored.
      *
+     * This method may be called publicly.
+     *
      * @param qubaid_condition $qubaids used to restrict which usages are included
      * in the query. See {@link qubaid_condition}.
      * @param int $slot The slot for the questions you want to konw about.
@@ -640,12 +712,16 @@ $sqlorderby
     }
 
     /**
-     * Load a {@link question_usage_by_activity} from the database, including
-     * all its {@link question_attempt}s and all their steps.
+     * Load the average mark, and number of attempts, for each slot in a set of
+     * question usages..
+     *
+     * This method may be called publicly.
+     *
      * @param qubaid_condition $qubaids used to restrict which usages are included
      * in the query. See {@link qubaid_condition}.
      * @param array $slots if null, load info for all quesitions, otherwise only
      * load the averages for the specified questions.
+     * @return array of objects with fields ->slot, ->averagefraction and ->numaveraged.
      */
     public function load_average_marks(qubaid_condition $qubaids, $slots = null) {
         if (!empty($slots)) {
@@ -689,9 +765,11 @@ ORDER BY qa.slot
     }
 
     /**
-     * Load a {@link question_attempt} from the database, including all its
+     * Load all the attempts at a given queston from a set of question_usages.
      * steps.
      *
+     * This method may be called publicly.
+     *
      * @param int $questionid the question to load all the attempts fors.
      * @param qubaid_condition $qubaids used to restrict which usages are included
      * in the query. See {@link qubaid_condition}.
@@ -761,6 +839,10 @@ ORDER BY
     /**
      * Update a question_usages row to refect any changes in a usage (but not
      * any of its question_attempts.
+     *
+     * You should not call this method directly. You should use
+     * @link question_engine::save_questions_usage_by_activity()}.
+     *
      * @param question_usage_by_activity $quba the usage that has changed.
      */
     public function update_questions_usage_by_activity(question_usage_by_activity $quba) {
@@ -776,6 +858,10 @@ ORDER BY
     /**
      * Update a question_attempts row to refect any changes in a question_attempt
      * (but not any of its steps).
+     *
+     * You should not call this method directly. You should use
+     * @link question_engine::save_questions_usage_by_activity()}.
+     *
      * @param question_attempt $qa the question attempt that has changed.
      */
     public function update_question_attempt(question_attempt $qa) {
@@ -795,6 +881,10 @@ ORDER BY
 
     /**
      * Delete a question_usage_by_activity and all its associated
+     *
+     * You should not call this method directly. You should use
+     * @link question_engine::delete_questions_usage_by_activities()}.
+     *
      * {@link question_attempts} and {@link question_attempt_steps} from the
      * database.
      * @param qubaid_condition $qubaids identifies which question useages to delete.
@@ -867,6 +957,9 @@ ORDER BY
 
     /**
      * Delete all the steps for a question attempt.
+     *
+     * Private method, only for use by other parts of the question engine.
+     *
      * @param int $qaids question_attempt id.
      * @param context $context the context that the $quba belongs to.
      */
@@ -902,6 +995,9 @@ ORDER BY
 
     /**
      * Delete all the previews for a given question.
+     *
+     * Private method, only for use by other parts of the question engine.
+     *
      * @param int $questionid question id.
      */
     public function delete_previews($questionid) {
@@ -919,6 +1015,10 @@ ORDER BY
 
     /**
      * Update the flagged state of a question in the database.
+     *
+     * You should call {@link question_engine::update_flag()()}
+     * rather than calling this method directly.
+     *
      * @param int $qubaid the question usage id.
      * @param int $questionid the question id.
      * @param int $sessionid the question_attempt id.
@@ -950,6 +1050,9 @@ ORDER BY
     /**
      * Get the SQL needed to test that question_attempt_steps.state is in a
      * state corresponding to $summarystate.
+     *
+     * This method may be called publicly.
+     *
      * @param string $summarystate one of
      * inprogress, needsgrading, manuallygraded or autograded
      * @param bool $equal if false, do a NOT IN test. Default true.
@@ -964,6 +1067,10 @@ ORDER BY
     /**
      * Change the maxmark for the question_attempt with number in usage $slot
      * for all the specified question_attempts.
+     *
+     * You should call {@link question_engine::set_max_mark_in_attempts()}
+     * rather than calling this method directly.
+     *
      * @param qubaid_condition $qubaids Selects which usages are updated.
      * @param int $slot the number is usage to affect.
      * @param number $newmaxmark the new max mark to set.
@@ -982,6 +1089,8 @@ ORDER BY
      * See {@link quiz_update_all_attempt_sumgrades()} for an example of the usage of
      * this method.
      *
+     * This method may be called publicly.
+     *
      * @param string $qubaid SQL fragment that controls which usage is summed.
      * This will normally be the name of a column in the outer query. Not that this
      * SQL fragment must not contain any placeholders.
@@ -1011,6 +1120,9 @@ ORDER BY
      * Get a subquery that returns the latest step of every qa in some qubas.
      * Currently, this is only used by the quiz reports. See
      * {@link quiz_attempts_report_table::add_latest_state_join()}.
+     *
+     * This method may be called publicly.
+     *
      * @param string $alias alias to use for this inline-view.
      * @param qubaid_condition $qubaids restriction on which question_usages we
      *      are interested in. This is important for performance.
@@ -1055,9 +1167,14 @@ ORDER BY
     }
 
     /**
+     * Are any of these questions are currently in use?
+     *
+     * You should call {@link question_engine::questions_in_use()}
+     * rather than calling this method directly.
+     *
      * @param array $questionids of question ids.
      * @param qubaid_condition $qubaids ids of the usages to consider.
-     * @return boolean whether any of these questions are being used by any of
+     * @return bool whether any of these questions are being used by any of
      *      those usages.
      */
     public function questions_in_use(array $questionids, qubaid_condition $qubaids) {
@@ -1263,20 +1380,25 @@ class question_engine_unit_of_work implements question_usage_observer {
     public function save(question_engine_data_mapper $dm) {
         $dm->delete_steps(array_keys($this->stepsdeleted), $this->quba->get_owning_context());
 
+        // Initially an array of array of question_attempt_step_objects.
+        // Built as a nested array for efficiency, then flattened.
+        $stepdata = array();
+
         foreach ($this->stepsmodified as $stepinfo) {
             list($step, $questionattemptid, $seq) = $stepinfo;
-            $dm->update_question_attempt_step($step, $questionattemptid, $seq,
-                    $this->quba->get_owning_context());
+            $stepdata[] = $dm->update_question_attempt_step(
+                    $step, $questionattemptid, $seq, $this->quba->get_owning_context());
         }
 
         foreach ($this->stepsadded as $stepinfo) {
             list($step, $questionattemptid, $seq) = $stepinfo;
-            $dm->insert_question_attempt_step($step, $questionattemptid, $seq,
-                    $this->quba->get_owning_context());
+            $stepdata[] = $dm->insert_question_attempt_step(
+                    $step, $questionattemptid, $seq, $this->quba->get_owning_context());
         }
 
         foreach ($this->attemptsadded as $qa) {
-            $dm->insert_question_attempt($qa, $this->quba->get_owning_context());
+            $stepdata[] = $dm->insert_question_attempt(
+                    $qa, $this->quba->get_owning_context());
         }
 
         foreach ($this->attemptsmodified as $qa) {
@@ -1286,6 +1408,11 @@ class question_engine_unit_of_work implements question_usage_observer {
         if ($this->modified) {
             $dm->update_questions_usage_by_activity($this->quba);
         }
+
+        if (!$stepdata) {
+            return;
+        }
+        $dm->insert_all_step_data(call_user_func_array('array_merge', $stepdata));
     }
 }
 
index 7f7e238..e18f997 100644 (file)
@@ -362,7 +362,7 @@ class question_attempt_step {
      * Get all the data. behaviour variables have the - at the start of
      * their name. This is only intended for internal use, for example by
      * {@link question_engine_data_mapper::insert_question_attempt_step()},
-     * however, it can ocasionally be useful in test code. It should not be
+     * however, it can occasionally be useful in test code. It should not be
      * considered part of the public API of this class.
      * @param array name => value pairs.
      */
index 20d0a6a..ddadbfb 100644 (file)
@@ -310,10 +310,8 @@ class question_usage_by_activity {
     }
 
     /**
-     * Get the current mark awarded for the attempt at a question.
-     * @param int $slot the number used to identify this question within this usage.
-     * @return number|null The current mark for this question, or null if one has
-     * not been assigned yet.
+     * Get the total mark for all questions in this usage.
+     * @return number The sum of marks of all the question_attempts in this usage.
      */
     public function get_total_mark() {
         $mark = 0;
index 4abcef0..f235639 100644 (file)
@@ -1,5 +1,16 @@
 This files describes API changes for code that uses the question API.
 
+=== 2.8 ===
+
+1) This is jsut a warning that some methods of the question_engine_data_mapper
+   class have changed. All these methods are ones that you should not have been
+   calling directly from your code, so this should not cause any problems.
+   The changed methods are:
+    * insert_question_attempt
+    * insert_step_data
+    * update_question_attempt_step
+
+
 === 2.7 ===
 
 1)  Changes to class question_bank_view:
@@ -26,6 +37,7 @@ To add filters, local plugins can now implement the function local_[pluginname]_
 
 5) question_bank_column_base and it's derived classes have been namespaced to core_question\bank\column_base.
 
+
 === 2.6 ===
 
 1) Modules using the question bank MUST now declare their use of it with the xxx_supports()
index e31a51b..b190480 100644 (file)
@@ -143,7 +143,7 @@ function theme_clean_get_html_for_settings(renderer_base $output, moodle_page $p
 
     $return->footnote = '';
     if (!empty($page->theme->settings->footnote)) {
-        $return->footnote = '<div class="footnote text-center">'.$page->theme->settings->footnote.'</div>';
+        $return->footnote = '<div class="footnote text-center">'.format_text($page->theme->settings->footnote).'</div>';
     }
 
     return $return;