Merge branch 'MDL-61444-master' of git://github.com/lameze/moodle
authorAndrew Nicols <andrew@nicols.co.uk>
Thu, 22 Feb 2018 01:59:15 +0000 (09:59 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Thu, 22 Feb 2018 01:59:15 +0000 (09:59 +0800)
39 files changed:
admin/settings/development.php
admin/tool/cohortroles/classes/api.php
cohort/lib.php
cohort/tests/externallib_test.php
enrol/lti/classes/task/sync_grades.php
lang/en/admin.php
lang/en/search.php
lib/classes/scss.php
lib/outputlib.php
lib/tests/scss_test.php
mod/choice/lib.php
mod/choice/report.php
mod/choice/tests/behat/modify_choice.feature
mod/data/classes/search/entry.php
mod/data/tests/search_test.php
mod/forum/classes/search/post.php
mod/forum/tests/search_test.php
mod/wiki/classes/search/collaborative_page.php
mod/wiki/tests/search_test.php
question/behaviour/rendererbase.php
question/editlib.php
search/classes/base_mod.php
search/classes/document.php
search/classes/engine.php
search/classes/manager.php
search/engine/solr/classes/engine.php
search/engine/solr/classes/schema.php
search/engine/solr/tests/engine_test.php
search/tests/engine_test.php
search/tests/fixtures/mock_search_engine.php
search/tests/manager_test.php
search/upgrade.txt
theme/boost/scss/moodle/forms.scss
theme/boost/tests/scss_test.php [new file with mode: 0644]
user/edit.php
user/edit_form.php
user/editadvanced.php
user/editadvanced_form.php
user/tests/behat/name_fields.feature

index 80a8047..41d779f 100644 (file)
@@ -14,6 +14,8 @@ if ($hassiteconfig) { // speedup for non-admins, add all caps used on this page
 
     $temp->add(new admin_setting_configcheckbox('dndallowtextandlinks', new lang_string('dndallowtextandlinks', 'admin'), new lang_string('configdndallowtextandlinks', 'admin'), 0));
 
+    $temp->add(new admin_setting_configexecutable('pathtosassc', new lang_string('pathtosassc', 'admin'), new lang_string('pathtosassc_help', 'admin'), ''));
+
     $ADMIN->add('experimental', $temp);
 
     // "debugging" settingpage
index 8357e70..b3f0e54 100644 (file)
@@ -167,7 +167,7 @@ class api {
                 $params['roleid'] = $roleid;
                 $params['userid'] = $userid;
 
-                $sql = 'SELECT u.id AS userid, ra.id, ctx.id AS contextid
+                $sql = 'SELECT DISTINCT u.id AS userid, ra.id, ctx.id AS contextid
                           FROM {user} u
                           JOIN {cohort_members} cm ON u.id = cm.userid
                           JOIN {context} ctx ON u.id = ctx.instanceid AND ctx.contextlevel = :usercontext
index 5f257cd..f3c3a16 100644 (file)
@@ -500,6 +500,9 @@ function cohort_get_invisible_contexts() {
     $excludedcontexts = array();
     foreach ($records as $ctx) {
         context_helper::preload_from_record($ctx);
+        if (context::instance_by_id($ctx->id) == context_system::instance()) {
+            continue; // System context cohorts should be available and permissions already checked.
+        }
         if (!has_any_capability(array('moodle/cohort:manage', 'moodle/cohort:view'), context::instance_by_id($ctx->id))) {
             $excludedcontexts[] = $ctx->id;
         }
index 690d31a..85b2f02 100644 (file)
@@ -465,10 +465,13 @@ class core_cohort_externallib_testcase extends externallib_advanced_testcase {
         $user = $this->getDataGenerator()->create_user();
         $catuser = $this->getDataGenerator()->create_user();
         $catcreator = $this->getDataGenerator()->create_user();
+        $courseuser = $this->getDataGenerator()->create_user();
         $category = $this->getDataGenerator()->create_category();
         $othercategory = $this->getDataGenerator()->create_category();
+        $course = $this->getDataGenerator()->create_course();
         $syscontext = context_system::instance();
         $catcontext = context_coursecat::instance($category->id);
+        $coursecontext = context_course::instance($course->id);
 
         // Fetching default authenticated user role.
         $userroles = get_archetype_roles('user');
@@ -481,8 +484,10 @@ class core_cohort_externallib_testcase extends externallib_advanced_testcase {
         // Creating specific roles.
         $creatorrole = create_role('Creator role', 'creatorrole', 'creator role description');
         $userrole = create_role('User role', 'userrole', 'user role description');
+        $courserole = create_role('Course user role', 'courserole', 'course user role description');
 
         assign_capability('moodle/cohort:manage', CAP_ALLOW, $creatorrole, $syscontext->id);
+        assign_capability('moodle/cohort:view', CAP_ALLOW, $courserole, $syscontext->id);
 
         // Check for parameter $includes = 'parents'.
         role_assign($creatorrole, $creator->id, $syscontext->id);
@@ -490,9 +495,13 @@ class core_cohort_externallib_testcase extends externallib_advanced_testcase {
         role_assign($userrole, $user->id, $syscontext->id);
         role_assign($userrole, $catuser->id, $catcontext->id);
 
+        // Enrol user in the course.
+        $this->getDataGenerator()->enrol_user($courseuser->id, $course->id, 'courserole');
+
         $syscontext = array('contextid' => context_system::instance()->id);
         $catcontext = array('contextid' => context_coursecat::instance($category->id)->id);
         $othercatcontext = array('contextid' => context_coursecat::instance($othercategory->id)->id);
+        $coursecontext = array('contextid' => context_course::instance($course->id)->id);
 
         $cohort1 = $this->getDataGenerator()->create_cohort(array_merge($syscontext, array('name' => 'Cohortsearch 1')));
         $cohort2 = $this->getDataGenerator()->create_cohort(array_merge($catcontext, array('name' => 'Cohortsearch 2')));
@@ -543,6 +552,12 @@ class core_cohort_externallib_testcase extends externallib_advanced_testcase {
         $result = core_cohort_external::search_cohorts("Cohortsearch", $syscontext, 'all');
         $this->assertEquals(3, count($result['cohorts']));
 
+        // A user in the course context with the system cohort:view capability. Check that all the system cohorts are returned.
+        $this->setUser($courseuser);
+        $result = core_cohort_external::search_cohorts("Cohortsearch", $coursecontext, 'all');
+        $this->assertEquals(1, count($result['cohorts']));
+        $this->assertEquals('Cohortsearch 1', $result['cohorts'][$cohort1->id]->name);
+
         // Detect invalid parameter $includes.
         $this->setUser($creator);
         try {
index 944de37..f20802e 100644 (file)
@@ -154,8 +154,8 @@ class sync_grades extends \core\task\scheduled_task {
                             continue;
                         }
 
-                        // This can happen if the sync process has an unexpected error.
-                        if ($grade == $ltiuser->lastgrade) {
+                        // Check to see if the grade has changed.
+                        if (!grade_floats_different($grade, $ltiuser->lastgrade)) {
                             mtrace("Not sent - The grade $mtracecontent was not sent as the grades are the same.");
                             continue;
                         }
@@ -174,7 +174,7 @@ class sync_grades extends \core\task\scheduled_task {
                         }
 
                         if (strpos(strtolower($response), 'success') !== false) {
-                            $DB->set_field('enrol_lti_users', 'lastgrade', intval($grade), array('id' => $ltiuser->id));
+                            $DB->set_field('enrol_lti_users', 'lastgrade', grade_floatval($grade), array('id' => $ltiuser->id));
                             mtrace("Success - The grade '$floatgrade' $mtracecontent was sent.");
                             $sendcount = $sendcount + 1;
                         } else {
index 4a49dd7..eb6030d 100644 (file)
@@ -837,6 +837,8 @@ $string['pathtopsqldesc'] = 'This is only necessary to enter if you have more th
 $string['pathtopsqlinvalid'] = 'Invalid path to psql - either wrong path or not executable';
 $string['pathtopython'] = 'Path to Python';
 $string['pathtopythondesc'] = 'Path to your executable Python binary (both Python 2 and Python 3 are acceptable).';
+$string['pathtosassc'] = 'Path to SassC';
+$string['pathtosassc_help'] = 'Specifying the location of the SassC binary will switch the SASS compiler from Moodle\'s PHP implementation to SassC. See https://github.com/sass/sassc for more information.';
 $string['pcreunicodewarning'] = 'It is strongly recommended to use PCRE PHP extension that is compatible with Unicode characters.';
 $string['perfdebug'] = 'Performance info';
 $string['performance'] = 'Performance';
index 8241c69..4d515bb 100644 (file)
@@ -90,6 +90,8 @@ $string['queueheading'] = 'Additional indexing queue ({$a} items)';
 $string['resultsreturnedfor'] = 'results returned for';
 $string['runindexer'] = 'Run indexer (real)';
 $string['runindexertest'] = 'Run indexer test';
+$string['schemanotupdated'] = 'The search schema is out of date.';
+$string['schemaversionunknown'] = 'Search engine does not know about the current schema version.';
 $string['score'] = 'Score';
 $string['search'] = 'Search';
 $string['search:message_received'] = 'Messages - received';
index a8aa4a9..2281080 100644 (file)
@@ -98,6 +98,54 @@ class core_scss extends \Leafo\ScssPhp\Compiler {
         return $this->compile($content);
     }
 
+    /**
+     * Compile scss.
+     *
+     * Overrides ScssPHP's implementation, using the SassC compiler if it is available.
+     *
+     * @param string $code SCSS to compile.
+     * @param string $path Path to SCSS to compile.
+     *
+     * @return string The compiled CSS.
+     */
+    public function compile($code, $path = null) {
+        global $CFG;
+
+        $pathtosassc = trim($CFG->pathtosassc ?? '');
+
+        if (!empty($pathtosassc) && is_executable($pathtosassc) && !is_dir($pathtosassc)) {
+            $process = proc_open(
+                $pathtosassc . ' -I' . implode(':', $this->importPaths) . ' -s',
+                [
+                    ['pipe', 'r'], // Set the process stdin pipe to read mode.
+                    ['pipe', 'w'], // Set the process stdout pipe to write mode.
+                    ['pipe', 'w'] // Set the process stderr pipe to write mode.
+                ],
+                $pipes // Pipes become available in $pipes (pass by reference).
+            );
+            if (is_resource($process)) {
+                fwrite($pipes[0], $code); // Write the raw scss to the sassc process stdin.
+                fclose($pipes[0]);
+
+                $stdout = stream_get_contents($pipes[1]);
+                $stderr = stream_get_contents($pipes[2]);
+
+                fclose($pipes[1]);
+                fclose($pipes[2]);
+
+                // The proc_close function returns the process exit status. Anything other than 0 is bad.
+                if (proc_close($process) !== 0) {
+                    throw new coding_exception($stderr);
+                }
+
+                // Compiled CSS code will be available from stdout.
+                return $stdout;
+            }
+        }
+
+        return parent::compile($code, $path);
+    }
+
     /**
      * Compile child; returns a value to halt execution
      *
index e5a8514..00e9fff 100644 (file)
@@ -1475,7 +1475,7 @@ class theme_config {
             // Compile!
             $compiled = $compiler->to_css();
 
-        } catch (\Leafo\ScssPhp\Exception $e) {
+        } catch (\Exception $e) {
             $compiled = false;
             debugging('Error while compiling SCSS: ' . $e->getMessage(), DEBUG_DEVELOPER);
         }
index 80f3cb5..55e7cd9 100644 (file)
@@ -70,6 +70,61 @@ class core_scss_testcase extends advanced_testcase {
         ];
     }
 
+    /**
+     * Test cases for SassC compilation.
+     */
+    public function scss_compilation_provider() {
+        return [
+            'simple' => [
+                'scss' => '$font-stack: Helvetica, sans-serif;
+                           $primary-color: #333;
+
+                           body {
+                             font: 100% $font-stack;
+                             color: $primary-color;
+                           }',
+                'expected' => <<<CSS
+body {
+  font: 100% Helvetica, sans-serif;
+  color: #333; }
+
+CSS
+            ],
+            'nested' => [
+                'scss' => 'nav {
+                             ul {
+                               margin: 0;
+                               padding: 0;
+                               list-style: none;
+                             }
+
+                           li { display: inline-block; }
+
+                           a {
+                             display: block;
+                             padding: 6px 12px;
+                             text-decoration: none;
+                           }
+                         }',
+                'expected' => <<<CSS
+nav ul {
+  margin: 0;
+  padding: 0;
+  list-style: none; }
+
+nav li {
+  display: inline-block; }
+
+nav a {
+  display: block;
+  padding: 6px 12px;
+  text-decoration: none; }
+
+CSS
+            ]
+        ];
+    }
+
     /**
      * @dataProvider is_valid_file_provider
      */
@@ -78,4 +133,22 @@ class core_scss_testcase extends advanced_testcase {
         $pathvalid = phpunit_util::call_internal_method($scss, 'is_valid_file', [$path], \core_scss::class);
         $this->assertSame($valid, $pathvalid);
     }
-}
\ No newline at end of file
+
+    /**
+     * Test that we can use the SassC compiler if it's provided.
+     *
+     * @dataProvider scss_compilation_provider
+     * @param string $scss The raw scss to compile.
+     * @param string $expectedcss The expected CSS output.
+     */
+    public function test_scss_compilation_with_sassc($scss, $expectedcss) {
+        if (!defined('PHPUNIT_PATH_TO_SASSC')) {
+            $this->markTestSkipped('Path to SassC not provided');
+        }
+
+        $this->resetAfterTest();
+        set_config('pathtosassc', PHPUNIT_PATH_TO_SASSC);
+        $compiler = new core_scss();
+        $this->assertSame($compiler->compile($scss), $expectedcss);
+    }
+}
index 4e6eef0..bf548a0 100644 (file)
@@ -791,8 +791,9 @@ function choice_get_response_data($choice, $cm, $groupmode, $onlyactive) {
 
 /// First get all the users who have access here
 /// To start with we assume they are all "unanswered" then move them later
+    $extrafields = get_extra_user_fields($context);
     $allresponses[0] = get_enrolled_users($context, 'mod/choice:choose', $currentgroup,
-            user_picture::fields('u', array('idnumber')), null, 0, 0, $onlyactive);
+            user_picture::fields('u', $extrafields), null, 0, 0, $onlyactive);
 
 /// Get all the recorded responses for this choice
     $rawresponses = $DB->get_records('choice_answers', array('choiceid' => $choice->id));
index 8c0293b..9e187d5 100644 (file)
         }
         exit;
     }
-    // Always show those who haven't answered the question.
-    $choice->showunanswered = 1;
     $results = prepare_choice_show_results($choice, $course, $cm, $users);
     $renderer = $PAGE->get_renderer('mod_choice');
     echo $renderer->display_result($results, true);
index 6730174..da8754c 100644 (file)
@@ -26,7 +26,15 @@ Feature: Teacher can modify choices of the students
 
   @javascript
   Scenario: Delete students choice response as a teacher
-    When I log in as "student1"
+    When I log in as "teacher1"
+    And I am on "Course 1" course homepage
+    And I follow "Choice name"
+    And I navigate to "Edit settings" in current page administration
+    And I expand all fieldsets
+    And I set the field "Show column for unanswered" to "Yes"
+    And I press "Save and return to course"
+    And I log out
+    And I log in as "student1"
     And I am on "Course 1" course homepage
     And I choose "Option 1" from "Choice name" choice activity
     Then I should see "Your selection: Option 1"
@@ -44,7 +52,15 @@ Feature: Teacher can modify choices of the students
 
   @javascript
   Scenario: Teacher set answers of students who did not respond or change existing answers
-    When I log in as "student1"
+    When I log in as "teacher1"
+    And I am on "Course 1" course homepage
+    And I follow "Choice name"
+    And I navigate to "Edit settings" in current page administration
+    And I expand all fieldsets
+    And I set the field "Show column for unanswered" to "Yes"
+    And I press "Save and return to course"
+    And I log out
+    And I log in as "student1"
     And I am on "Course 1" course homepage
     And I choose "Option 1" from "Choice name" choice activity
     Then I should see "Your selection: Option 1"
index e2a2580..b1a6fd9 100644 (file)
@@ -76,8 +76,6 @@ class entry extends \core_search\base_mod {
      * @return \core_search\document
      */
     public function get_document($entry, $options = array()) {
-        global $DB;
-
         try {
             $cm = $this->get_cm('data', $entry->dataid, $entry->course);
             $context = \context_module::instance($cm->id);
@@ -97,6 +95,9 @@ class entry extends \core_search\base_mod {
         $doc->set('contextid', $context->id);
         $doc->set('courseid', $entry->course);
         $doc->set('userid', $entry->userid);
+        if ($entry->groupid > 0) {
+            $doc->set('groupid', $entry->groupid);
+        }
         $doc->set('owneruserid', \core_search\manager::NO_OWNER_ID);
         $doc->set('modified', $entry->timemodified);
 
@@ -353,4 +354,13 @@ class entry extends \core_search\base_mod {
         require_once($CFG->dirroot . '/mod/data/field/' . $fieldtype . '/field.class.php');
         return 'data_field_' . $fieldtype;
     }
+
+    /**
+     * Confirms that data entries support group restrictions.
+     *
+     * @return bool True
+     */
+    public function supports_group_restriction() {
+        return true;
+    }
 }
index 78d1c6c..8988ea6 100644 (file)
@@ -575,6 +575,77 @@ class mod_data_search_test extends advanced_testcase {
 
     }
 
+    /**
+     * Group support for data entries.
+     */
+    public function test_data_entries_group_support() {
+        global $DB;
+
+        // Get the search area and test generators.
+        $searcharea = \core_search\manager::get_search_area($this->databaseentryareaid);
+        $generator = $this->getDataGenerator();
+        $datagenerator = $generator->get_plugin_generator('mod_data');
+
+        // Create a course, a user, and two groups.
+        $course = $generator->create_course();
+        $user = $generator->create_user();
+        $generator->enrol_user($user->id, $course->id, 'teacher');
+        $group1 = $generator->create_group(['courseid' => $course->id]);
+        $group2 = $generator->create_group(['courseid' => $course->id]);
+
+        // Separate groups database.
+        $data = self::getDataGenerator()->create_module('data', ['course' => $course->id,
+                'groupmode' => SEPARATEGROUPS]);
+        $fieldtypes = ['text', 'textarea'];
+        $this->create_default_data_fields($fieldtypes, $data);
+        $fields = $DB->get_records('data_fields', array('dataid' => $data->id));
+        foreach ($fields as $field) {
+            switch ($field->type) {
+                case 'text' :
+                    $textid = $field->id;
+                    break;
+                case 'textarea' :
+                    $textareaid = $field->id;
+                    break;
+            }
+        }
+
+        // As admin, create entries with each group and all groups.
+        $this->setAdminUser();
+        $fieldvalues = [$textid => 'Title', $textareaid => 'Content'];
+        $e1 = $datagenerator->create_entry($data, $fieldvalues, $group1->id);
+        $e2 = $datagenerator->create_entry($data, $fieldvalues, $group2->id);
+        $e3 = $datagenerator->create_entry($data, $fieldvalues);
+
+        // Do the indexing of all 3 entries.
+        $rs = $searcharea->get_recordset_by_timestamp(0);
+        $results = [];
+        foreach ($rs as $rec) {
+            $results[$rec->id] = $rec;
+        }
+        $rs->close();
+        $this->assertCount(3, $results);
+
+        // Check each has the correct groupid.
+        $doc = $searcharea->get_document($results[$e1]);
+        $this->assertTrue($doc->is_set('groupid'));
+        $this->assertEquals($group1->id, $doc->get('groupid'));
+        $doc = $searcharea->get_document($results[$e2]);
+        $this->assertTrue($doc->is_set('groupid'));
+        $this->assertEquals($group2->id, $doc->get('groupid'));
+        $doc = $searcharea->get_document($results[$e3]);
+        $this->assertFalse($doc->is_set('groupid'));
+
+        // While we're here, also test that the search area requests restriction by group.
+        $modinfo = get_fast_modinfo($course);
+        $this->assertTrue($searcharea->restrict_cm_access_by_group($modinfo->get_cm($data->cmid)));
+
+        // In visible groups mode, it won't request restriction by group.
+        set_coursemodule_groupmode($data->cmid, VISIBLEGROUPS);
+        $modinfo = get_fast_modinfo($course);
+        $this->assertFalse($searcharea->restrict_cm_access_by_group($modinfo->get_cm($data->cmid)));
+    }
+
     /**
      * Document accesses.
      *
index 9f5d68e..e83ddb9 100644 (file)
@@ -68,7 +68,7 @@ class post extends \core_search\base_mod {
             return null;
         }
 
-        $sql = "SELECT fp.*, f.id AS forumid, f.course AS courseid
+        $sql = "SELECT fp.*, f.id AS forumid, f.course AS courseid, fd.groupid AS groupid
                   FROM {forum_posts} fp
                   JOIN {forum_discussions} fd ON fd.id = fp.discussion
                   JOIN {forum} f ON f.id = fd.forum
@@ -110,6 +110,11 @@ class post extends \core_search\base_mod {
         $doc->set('owneruserid', \core_search\manager::NO_OWNER_ID);
         $doc->set('modified', $record->modified);
 
+        // Store group id if there is one. (0 and -1 both mean not restricted to group.)
+        if ($record->groupid > 0) {
+            $doc->set('groupid', $record->groupid);
+        }
+
         // Check if this document should be considered new.
         if (isset($options['lastindexedtime']) && ($options['lastindexedtime'] < $record->created)) {
             // If the document was created after the last index time, it must be new.
@@ -305,4 +310,13 @@ class post extends \core_search\base_mod {
             'MAX(fd.timemodified) DESC'
         ];
     }
+
+    /**
+     * Confirms that data entries support group restrictions.
+     *
+     * @return bool True
+     */
+    public function supports_group_restriction() {
+        return true;
+    }
 }
index d2cd423..93095aa 100644 (file)
@@ -196,6 +196,7 @@ class mod_forum_search_testcase extends advanced_testcase {
         $record->userid = $user->id;
         $record->forum = $forum1->id;
         $record->message = 'discussion';
+        $record->groupid = 0;
         $discussion1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_discussion($record);
 
         // Create post1 in discussion1.
@@ -205,11 +206,13 @@ class mod_forum_search_testcase extends advanced_testcase {
         $record->userid = $user->id;
         $record->subject = 'subject1';
         $record->message = 'post1';
+        $record->groupid = -1;
         $discussion1reply1 = self::getDataGenerator()->get_plugin_generator('mod_forum')->create_post($record);
 
         $post1 = $DB->get_record('forum_posts', array('id' => $discussion1reply1->id));
         $post1->forumid = $forum1->id;
         $post1->courseid = $forum1->course;
+        $post1->groupid = -1;
 
         $doc = $searcharea->get_document($post1);
         $this->assertInstanceOf('\core_search\document', $doc);
@@ -221,6 +224,72 @@ class mod_forum_search_testcase extends advanced_testcase {
         $this->assertEquals($discussion1reply1->message, $doc->get('content'));
     }
 
+    /**
+     * Group support for forum posts.
+     */
+    public function test_posts_group_support() {
+        // Get the search area and test generators.
+        $searcharea = \core_search\manager::get_search_area($this->forumpostareaid);
+        $generator = $this->getDataGenerator();
+        $forumgenerator = $generator->get_plugin_generator('mod_forum');
+
+        // Create a course, a user, and two groups.
+        $course = $generator->create_course();
+        $user = $generator->create_user();
+        $generator->enrol_user($user->id, $course->id, 'teacher');
+        $group1 = $generator->create_group(['courseid' => $course->id]);
+        $group2 = $generator->create_group(['courseid' => $course->id]);
+
+        // Separate groups forum.
+        $forum = self::getDataGenerator()->create_module('forum', ['course' => $course->id,
+                'groupmode' => SEPARATEGROUPS]);
+
+        // Create discussion with each group and one for all groups. One has a post in.
+        $discussion1 = $forumgenerator->create_discussion(['course' => $course->id,
+                'userid' => $user->id, 'forum' => $forum->id, 'message' => 'd1',
+                'groupid' => $group1->id]);
+        $forumgenerator->create_discussion(['course' => $course->id,
+                'userid' => $user->id, 'forum' => $forum->id, 'message' => 'd2',
+                'groupid' => $group2->id]);
+        $forumgenerator->create_discussion(['course' => $course->id,
+                'userid' => $user->id, 'forum' => $forum->id, 'message' => 'd3']);
+
+        // Create a reply in discussion1.
+        $forumgenerator->create_post(['discussion' => $discussion1->id, 'parent' => $discussion1->firstpost,
+                'userid' => $user->id, 'message' => 'p1']);
+
+        // Do the indexing of all 4 posts.
+        $rs = $searcharea->get_recordset_by_timestamp(0);
+        $results = [];
+        foreach ($rs as $rec) {
+            $results[$rec->message] = $rec;
+        }
+        $rs->close();
+        $this->assertCount(4, $results);
+
+        // Check each document has the correct groupid.
+        $doc = $searcharea->get_document($results['d1']);
+        $this->assertTrue($doc->is_set('groupid'));
+        $this->assertEquals($group1->id, $doc->get('groupid'));
+        $doc = $searcharea->get_document($results['d2']);
+        $this->assertTrue($doc->is_set('groupid'));
+        $this->assertEquals($group2->id, $doc->get('groupid'));
+        $doc = $searcharea->get_document($results['d3']);
+        $this->assertFalse($doc->is_set('groupid'));
+        $doc = $searcharea->get_document($results['p1']);
+        $this->assertTrue($doc->is_set('groupid'));
+        $this->assertEquals($group1->id, $doc->get('groupid'));
+
+        // While we're here, also test that the search area requests restriction by group.
+        $modinfo = get_fast_modinfo($course);
+        $this->assertTrue($searcharea->restrict_cm_access_by_group($modinfo->get_cm($forum->cmid)));
+
+        // In visible groups mode, it won't request restriction by group.
+        set_coursemodule_groupmode($forum->cmid, VISIBLEGROUPS);
+        $modinfo = get_fast_modinfo($course);
+        $this->assertFalse($searcharea->restrict_cm_access_by_group($modinfo->get_cm($forum->cmid)));
+    }
+
     /**
      * Document accesses.
      *
index 11220d2..81ca229 100644 (file)
@@ -57,7 +57,7 @@ class collaborative_page extends \core_search\base_mod {
             return null;
         }
 
-        $sql = "SELECT p.*, w.id AS wikiid, w.course AS courseid
+        $sql = "SELECT p.*, w.id AS wikiid, w.course AS courseid, s.groupid AS groupid
                   FROM {wiki_pages} p
                   JOIN {wiki_subwikis} s ON s.id = p.subwikiid
                   JOIN {wiki} w ON w.id = s.wikiid
@@ -111,6 +111,9 @@ class collaborative_page extends \core_search\base_mod {
         $doc->set('content', $content);
         $doc->set('contextid', $context->id);
         $doc->set('courseid', $record->courseid);
+        if ($record->groupid > 0) {
+            $doc->set('groupid', $record->groupid);
+        }
         $doc->set('owneruserid', \core_search\manager::NO_OWNER_ID);
         $doc->set('modified', $record->timemodified);
 
@@ -205,4 +208,13 @@ class collaborative_page extends \core_search\base_mod {
 
         return $fileareas;
     }
+
+    /**
+     * Confirms that data entries support group restrictions.
+     *
+     * @return bool True
+     */
+    public function supports_group_restriction() {
+        return true;
+    }
 }
index 3daa804..9c3f907 100644 (file)
@@ -143,6 +143,60 @@ class mod_wiki_search_testcase extends advanced_testcase {
         $rs->close();
     }
 
+    /**
+     * Group support for wiki entries.
+     */
+    public function test_collaborative_page_group_support() {
+        // Get the search area and test generators.
+        $searcharea = \core_search\manager::get_search_area($this->wikicollabpageareaid);
+        $generator = $this->getDataGenerator();
+        $wikigenerator = $generator->get_plugin_generator('mod_wiki');
+
+        // Create a course, a user, and two groups.
+        $course = $generator->create_course();
+        $user = $generator->create_user();
+        $generator->enrol_user($user->id, $course->id, 'teacher');
+        $group1 = $generator->create_group(['courseid' => $course->id]);
+        $group2 = $generator->create_group(['courseid' => $course->id]);
+
+        // Separate groups wiki.
+        $wiki = self::getDataGenerator()->create_module('wiki', ['course' => $course->id,
+                'groupmode' => SEPARATEGROUPS]);
+
+        // Create page with each group and one for all groups.
+        $wikigenerator->create_page($wiki, ['title' => 'G1', 'group' => $group1->id]);
+        $wikigenerator->create_page($wiki, ['title' => 'G2', 'group' => $group2->id]);
+        $wikigenerator->create_page($wiki, ['title' => 'ALLGROUPS']);
+
+        // Do the indexing of all 3 pages.
+        $rs = $searcharea->get_recordset_by_timestamp(0);
+        $results = [];
+        foreach ($rs as $rec) {
+            $results[$rec->title] = $rec;
+        }
+        $rs->close();
+        $this->assertCount(3, $results);
+
+        // Check each document has the correct groupid.
+        $doc = $searcharea->get_document($results['G1']);
+        $this->assertTrue($doc->is_set('groupid'));
+        $this->assertEquals($group1->id, $doc->get('groupid'));
+        $doc = $searcharea->get_document($results['G2']);
+        $this->assertTrue($doc->is_set('groupid'));
+        $this->assertEquals($group2->id, $doc->get('groupid'));
+        $doc = $searcharea->get_document($results['ALLGROUPS']);
+        $this->assertFalse($doc->is_set('groupid'));
+
+        // While we're here, also test that the search area requests restriction by group.
+        $modinfo = get_fast_modinfo($course);
+        $this->assertTrue($searcharea->restrict_cm_access_by_group($modinfo->get_cm($wiki->cmid)));
+
+        // In visible groups mode, it won't request restriction by group.
+        set_coursemodule_groupmode($wiki->cmid, VISIBLEGROUPS);
+        $modinfo = get_fast_modinfo($course);
+        $this->assertFalse($searcharea->restrict_cm_access_by_group($modinfo->get_cm($wiki->cmid)));
+    }
+
     /**
      * Check collaborative_page check access.
      *
index c791866..416826a 100644 (file)
@@ -234,7 +234,7 @@ abstract class qbehaviour_renderer extends plugin_renderer_base {
             'id' => $qa->get_behaviour_field_name('submit'),
             'name' => $qa->get_behaviour_field_name('submit'),
             'value' => get_string('check', 'question'),
-            'class' => 'submit btn',
+            'class' => 'submit btn btn-default',
         );
         if ($options->readonly) {
             $attributes['disabled'] = 'disabled';
index 093dc10..da255f5 100644 (file)
@@ -481,7 +481,7 @@ function question_build_edit_resources($edittab, $baseurl, $params) {
         $pagevars['qpage'] = 0;
     }
 
-    $pagevars['qperpage'] = question_build_display_preference(
+    $pagevars['qperpage'] = question_set_or_get_user_preference(
             'qperpage', $qperpage, DEFAULT_QUESTIONS_PER_PAGE, $thispageurl);
 
     $defaultcategory = question_make_default_categories($contexts->all());
@@ -503,9 +503,9 @@ function question_build_edit_resources($edittab, $baseurl, $params) {
     }
 
     // Display options.
-    $pagevars['recurse']    = question_build_display_preference('recurse', $recurse, 1, $thispageurl);
-    $pagevars['showhidden'] = question_build_display_preference('showhidden', $showhidden, 0, $thispageurl);
-    $pagevars['qbshowtext'] = question_build_display_preference('qbshowtext', $qbshowtext, 0, $thispageurl);
+    $pagevars['recurse']    = question_set_or_get_user_preference('recurse', $recurse, 1, $thispageurl);
+    $pagevars['showhidden'] = question_set_or_get_user_preference('showhidden', $showhidden, 0, $thispageurl);
+    $pagevars['qbshowtext'] = question_set_or_get_user_preference('qbshowtext', $qbshowtext, 0, $thispageurl);
 
     // Category list page.
     $pagevars['cpage'] = $cpage;
@@ -548,7 +548,7 @@ function question_get_category_id_from_pagevars(array $pagevars) {
  */
 function question_get_display_preference($param, $default, $type, $thispageurl) {
     $submittedvalue = optional_param($param, null, $type);
-    return question_build_display_preference($param, $submittedvalue, $default, $thispageurl);
+    return question_set_or_get_user_preference($param, $submittedvalue, $default, $thispageurl);
 }
 
 /**
@@ -570,7 +570,7 @@ function question_get_display_preference($param, $default, $type, $thispageurl)
  *      it to this URL.
  * @return mixed the parameter value to use.
  */
-function question_build_display_preference($name, $value, $default, $thispageurl) {
+function question_set_or_get_user_preference($name, $value, $default, $thispageurl) {
     if (is_null($value)) {
         return get_user_preferences('question_bank_' . $name, $default);
     }
index 1ad7a77..b45ae69 100644 (file)
@@ -245,4 +245,45 @@ abstract class base_mod extends base {
             return \context::instance_by_id($id);
         });
     }
+
+    /**
+     * Indicates whether this search area may restrict access by group.
+     *
+     * This should return true if the search area (sometimes) sets the 'groupid' schema field, and
+     * false if it never sets that field.
+     *
+     * (If this function returns false, but the field is set, then results may be restricted
+     * unintentionally.)
+     *
+     * If this returns true, the search engine will automatically apply group restrictions in some
+     * cases (by default, where a module is configured to use separate groups). See function
+     * restrict_cm_access_by_group().
+     *
+     * @return bool
+     */
+    public function supports_group_restriction() {
+        return false;
+    }
+
+    /**
+     * Checks whether the content of this search area should be restricted by group for a
+     * specific module. Called at query time.
+     *
+     * The default behaviour simply checks if the effective group mode is SEPARATEGROUPS, which
+     * is probably correct for most cases.
+     *
+     * If restricted by group, the search query will (where supported by the engine) filter out
+     * results for groups the user does not belong to, unless the user has 'access all groups'
+     * for the activity. This affects only documents which set the 'groupid' field; results with no
+     * groupid will not be restricted.
+     *
+     * Even if you return true to this function, you may still need to do group access checks in
+     * check_access, because the search engine may not support group restrictions.
+     *
+     * @param \cm_info $cm
+     * @return bool True to restrict by group
+     */
+    public function restrict_cm_access_by_group(\cm_info $cm) {
+        return $cm->effectivegroupmode == SEPARATEGROUPS;
+    }
 }
index 64831b1..1fea314 100644 (file)
@@ -78,6 +78,14 @@ class document implements \renderable, \templatable {
      */
     protected $files = array();
 
+    /**
+     * Change list (for engine implementers):
+     * 2017091700 - add optional field groupid
+     *
+     * @var int Schema version number (update if any change)
+     */
+    const SCHEMA_VERSION = 2017091700;
+
     /**
      * All required fields any doc should contain.
      *
@@ -159,6 +167,11 @@ class document implements \renderable, \templatable {
             'stored' => true,
             'indexed' => true
         ),
+        'groupid' => array(
+            'type' => 'int',
+            'stored' => true,
+            'indexed' => true
+        ),
         'description1' => array(
             'type' => 'text',
             'stored' => true,
index 52a7ab8..ef3d05b 100644 (file)
@@ -42,7 +42,7 @@ abstract class engine {
     /**
      * The search engine configuration.
      *
-     * @var stdClass
+     * @var \stdClass
      */
     protected $config = null;
 
@@ -65,7 +65,7 @@ abstract class engine {
     /**
      * User data required to show their fullnames. Indexed by userid.
      *
-     * @var stdClass[]
+     * @var \stdClass[]
      */
     protected static $cachedusers = array();
 
@@ -439,12 +439,36 @@ abstract class engine {
      *
      * Engines should reasonably attempt to fill up to limit with valid results if they are available.
      *
-     * @param  stdClass $filters Query and filters to apply.
-     * @param  array    $usercontexts Contexts where the user has access. True if the user can access all contexts.
+     * The $filters object may include the following fields (optional except q):
+     * - q: value of main search field; results should include this text
+     * - title: if included, title must match this search
+     * - areaids: array of search area id strings (only these areas will be searched)
+     * - courseids: array of course ids (only these courses will be searched)
+     * - groupids: array of group ids (only results specifically from these groupids will be
+     *   searched) - this option will be ignored if the search engine doesn't support groups
+     *
+     * The $accessinfo parameter has two different values (for historical compatibility). If the
+     * engine returns false to supports_group_filtering then it is an array of user contexts, or
+     * true if the user can access all contexts. (This parameter used to be called $usercontexts.)
+     * If the engine returns true to supports_group_filtering then it will be an object containing
+     * these fields:
+     * - everything (true if admin is searching with no restrictions)
+     * - usercontexts (same as above)
+     * - separategroupscontexts (array of context ids where separate groups are used)
+     * - visiblegroupscontextsareas (array of subset of those where some areas use visible groups)
+     * - usergroups (array of relevant group ids that user belongs to)
+     *
+     * The engine should apply group restrictions to those contexts listed in the
+     * 'separategroupscontexts' array. In these contexts, it shouled only include results if the
+     * groupid is not set, or if the groupid matches one of the values in USER_GROUPS array, or
+     * if the search area is one of those listed in 'visiblegroupscontextsareas' for that context.
+     *
+     * @param \stdClass $filters Query and filters to apply.
+     * @param \stdClass $accessinfo Information about the contexts the user can access
      * @param  int      $limit The maximum number of results to return. If empty, limit to manager::MAX_RESULTS.
      * @return \core_search\document[] Results or false if no results
      */
-    abstract function execute_query($filters, $usercontexts, $limit = 0);
+    public abstract function execute_query($filters, $accessinfo, $limit = 0);
 
     /**
      * Delete all documents.
@@ -453,4 +477,70 @@ abstract class engine {
      * @return void
      */
     abstract function delete($areaid = null);
+
+    /**
+     * Checks that the schema is the latest version. If the version stored in config does not match
+     * the current, this function will attempt to upgrade the schema.
+     *
+     * @return bool|string True if schema is OK, a string if user needs to take action
+     */
+    public function check_latest_schema() {
+        if (empty($this->config->schemaversion)) {
+            $currentversion = 0;
+        } else {
+            $currentversion = $this->config->schemaversion;
+        }
+        if ($currentversion < document::SCHEMA_VERSION) {
+            return $this->update_schema((int)$currentversion, (int)document::SCHEMA_VERSION);
+        } else {
+            return true;
+        }
+    }
+
+    /**
+     * Usually called by the engine; marks that the schema has been updated.
+     *
+     * @param int $version Records the schema version now applied
+     */
+    public function record_applied_schema_version($version) {
+        set_config('schemaversion', $version, $this->pluginname);
+    }
+
+    /**
+     * Requests the search engine to upgrade the schema. The engine should update the schema if
+     * possible/necessary, and should ensure that record_applied_schema_version is called as a
+     * result.
+     *
+     * If it is not possible to upgrade the schema at the moment, it can do nothing and return; the
+     * function will be called again next time search is initialised.
+     *
+     * The default implementation just returns, with a DEBUG_DEVELOPER warning.
+     *
+     * @param int $oldversion Old schema version
+     * @param int $newversion New schema version
+     * @return bool|string True if schema is updated successfully, a string if it needs updating manually
+     */
+    protected function update_schema($oldversion, $newversion) {
+        debugging('Unable to update search engine schema: ' . $this->pluginname, DEBUG_DEVELOPER);
+        return get_string('schemanotupdated', 'search');
+    }
+
+    /**
+     * Checks if this search engine supports groups.
+     *
+     * Note that returning true to this function causes the parameters to execute_query to be
+     * passed differently!
+     *
+     * In order to implement groups and return true to this function, the search engine should:
+     *
+     * 1. Handle the fields ->separategroupscontexts and ->usergroups in the $accessinfo parameter
+     *    to execute_query (ideally, using these to automatically restrict search results).
+     * 2. Support the optional groupids parameter in the $filter parameter for execute_query to
+     *    restrict results to only those where the stored groupid matches the given value.
+     *
+     * @return bool True if this engine supports searching by group id field
+     */
+    public function supports_group_filtering() {
+        return false;
+    }
 }
index e04cf98..34a93d3 100644 (file)
@@ -364,11 +364,15 @@ class manager {
     }
 
     /**
-     * Returns the contexts the user can access.
+     * Returns information about the areas which the user can access.
      *
-     * The returned value is a multidimensional array because some search engines can group
-     * information and there will be a performance benefit on passing only some contexts
-     * instead of the whole context array set.
+     * The returned value is a stdClass object with the following fields:
+     * - everything (bool, true for admin only)
+     * - usercontexts (indexed by area identifier then context
+     * - separategroupscontexts (contexts within which group restrictions apply)
+     * - visiblegroupscontextsareas (overrides to the above when the same contexts also have
+     *   'visible groups' for certain search area ids - hopefully rare)
+     * - usergroups (groups which the current user belongs to)
      *
      * The areas can be limited by course id and context id. If specifying context ids, results
      * are limited to the exact context ids specified and not their children (for example, giving
@@ -378,7 +382,7 @@ class manager {
      *
      * @param array|false $limitcourseids An array of course ids to limit the search to. False for no limiting.
      * @param array|false $limitcontextids An array of context ids to limit the search to. False for no limiting.
-     * @return bool|array Indexed by area identifier (component + area name). Returns true if the user can see everything.
+     * @return \stdClass Object as described above
      */
     protected function get_areas_user_accesses($limitcourseids = false, $limitcontextids = false) {
         global $DB, $USER;
@@ -386,7 +390,7 @@ class manager {
         // All results for admins (unless they have chosen to limit results). Eventually we could
         // add a new capability for managers.
         if (is_siteadmin() && !$limitcourseids && !$limitcontextids) {
-            return true;
+            return (object)array('everything' => true);
         }
 
         $areasbylevel = array();
@@ -404,6 +408,11 @@ class manager {
         // This will store area - allowed contexts relations.
         $areascontexts = array();
 
+        // Initialise two special-case arrays for storing other information related to the contexts.
+        $separategroupscontexts = array();
+        $visiblegroupscontextsareas = array();
+        $usergroups = array();
+
         if (empty($limitcourseids) && !empty($areasbylevel[CONTEXT_SYSTEM])) {
             // We add system context to all search areas working at this level. Here each area is fully responsible of
             // the access control as we can not automate much, we can not even check guest access as some areas might
@@ -453,6 +462,7 @@ class manager {
 
         // Keep a list of included course context ids (needed for the block calculation below).
         $coursecontextids = [];
+        $modulecms = [];
 
         foreach ($courses as $course) {
             if (!empty($limitcourseids) && !in_array($course->id, $limitcourseids)) {
@@ -462,6 +472,7 @@ class manager {
 
             $coursecontext = \context_course::instance($course->id);
             $coursecontextids[] = $coursecontext->id;
+            $hasgrouprestrictions = false;
 
             // Info about the course modules.
             $modinfo = get_fast_modinfo($course);
@@ -491,11 +502,47 @@ class manager {
                             continue;
                         }
                         if ($modinstance->uservisible) {
-                            $areascontexts[$areaid][$modinstance->context->id] = $modinstance->context->id;
+                            $contextid = $modinstance->context->id;
+                            $areascontexts[$areaid][$contextid] = $contextid;
+                            $modulecms[$modinstance->id] = $modinstance;
+
+                            if (!has_capability('moodle/site:accessallgroups', $modinstance->context) &&
+                                    ($searchclass instanceof base_mod) &&
+                                    $searchclass->supports_group_restriction()) {
+                                if ($searchclass->restrict_cm_access_by_group($modinstance)) {
+                                    $separategroupscontexts[$contextid] = $contextid;
+                                    $hasgrouprestrictions = true;
+                                } else {
+                                    // Track a list of anything that has a group id (so might get
+                                    // filtered) and doesn't want to be, in this context.
+                                    if (!array_key_exists($contextid, $visiblegroupscontextsareas)) {
+                                        $visiblegroupscontextsareas[$contextid] = array();
+                                    }
+                                    $visiblegroupscontextsareas[$contextid][$areaid] = $areaid;
+                                }
+                            }
                         }
                     }
                 }
             }
+
+            // Insert group information for course (unless there aren't any modules restricted by
+            // group for this user in this course, in which case don't bother).
+            if ($hasgrouprestrictions) {
+                $groups = groups_get_all_groups($course->id, $USER->id, 0, 'g.id');
+                foreach ($groups as $group) {
+                    $usergroups[$group->id] = $group->id;
+                }
+            }
+        }
+
+        // Chuck away all the 'visible groups contexts' data unless there is actually something
+        // that does use separate groups in the same context (this data is only used as an
+        // 'override' in cases where the search is restricting to separate groups).
+        foreach ($visiblegroupscontextsareas as $contextid => $areas) {
+            if (!array_key_exists($contextid, $separategroupscontexts)) {
+                unset($visiblegroupscontextsareas[$contextid]);
+            }
         }
 
         // Add all supported block contexts, in a single query for performance.
@@ -564,7 +611,10 @@ class manager {
             }
         }
 
-        return $areascontexts;
+        // Return all the data.
+        return (object)array('everything' => false, 'usercontexts' => $areascontexts,
+                'separategroupscontexts' => $separategroupscontexts, 'usergroups' => $usergroups,
+                'visiblegroupscontextsareas' => $visiblegroupscontextsareas);
     }
 
     /**
@@ -687,12 +737,19 @@ class manager {
         // Clears previous query errors.
         $this->engine->clear_query_error();
 
-        $areascontexts = $this->get_areas_user_accesses($limitcourseids, $limitcontextids);
-        if (!$areascontexts) {
+        $contextinfo = $this->get_areas_user_accesses($limitcourseids, $limitcontextids);
+        if (!$contextinfo->everything && !$contextinfo->usercontexts) {
             // User can not access any context.
             $docs = array();
         } else {
-            $docs = $this->engine->execute_query($formdata, $areascontexts, $limit);
+            // If engine does not support groups, remove group information from the context info -
+            // use the old format instead (true = admin, array = user contexts).
+            if (!$this->engine->supports_group_filtering()) {
+                $contextinfo = $contextinfo->everything ? true : $contextinfo->usercontexts;
+            }
+
+            // Execute the actual query.
+            $docs = $this->engine->execute_query($formdata, $contextinfo, $limit);
         }
 
         return $docs;
index 875f3d1..9921713 100644 (file)
@@ -126,12 +126,12 @@ class engine extends \core_search\engine {
      * Prepares a Solr query, applies filters and executes it returning its results.
      *
      * @throws \core_search\engine_exception
-     * @param  stdClass  $filters Containing query and filters.
-     * @param  array     $usercontexts Contexts where the user has access. True if the user can access all contexts.
+     * @param  \stdClass $filters Containing query and filters.
+     * @param  \stdClass $accessinfo Information about areas user can access.
      * @param  int       $limit The maximum number of results to return.
      * @return \core_search\document[] Results or false if no results
      */
-    public function execute_query($filters, $usercontexts, $limit = 0) {
+    public function execute_query($filters, $accessinfo, $limit = 0) {
         global $USER;
 
         if (empty($limit)) {
@@ -142,7 +142,7 @@ class engine extends \core_search\engine {
         $client = $this->get_search_client();
 
         // Create the query object.
-        $query = $this->create_user_query($filters, $usercontexts);
+        $query = $this->create_user_query($filters, $accessinfo);
 
         // If the query cannot have results, return none.
         if (!$query) {
@@ -254,11 +254,11 @@ class engine extends \core_search\engine {
     /**
      * Prepares a new query object with needed limits, filters, etc.
      *
-     * @param stdClass  $filters Containing query and filters.
-     * @param array     $usercontexts Contexts where the user has access. True if the user can access all contexts.
+     * @param \stdClass $filters Containing query and filters.
+     * @param \stdClass $accessinfo Information about contexts the user can access
      * @return \SolrDisMaxQuery|null Query object or null if they can't get any results
      */
-    protected function create_user_query($filters, $usercontexts) {
+    protected function create_user_query($filters, $accessinfo) {
         global $USER;
 
         // Let's keep these changes internal.
@@ -281,6 +281,9 @@ class engine extends \core_search\engine {
         if (!empty($data->courseids)) {
             $query->addFilterQuery('{!cache=false}courseid:(' . implode(' OR ', $data->courseids) . ')');
         }
+        if (!empty($data->groupids)) {
+            $query->addFilterQuery('{!cache=false}groupid:(' . implode(' OR ', $data->groupids) . ')');
+        }
 
         if (!empty($data->timestart) or !empty($data->timeend)) {
             if (empty($data->timestart)) {
@@ -304,10 +307,10 @@ class engine extends \core_search\engine {
         // And finally restrict it to the context where the user can access, we want this one cached.
         // If the user can access all contexts $usercontexts value is just true, we don't need to filter
         // in that case.
-        if ($usercontexts && is_array($usercontexts)) {
+        if (!$accessinfo->everything && is_array($accessinfo->usercontexts)) {
             // Join all area contexts into a single array and implode.
             $allcontexts = array();
-            foreach ($usercontexts as $areaid => $areacontexts) {
+            foreach ($accessinfo->usercontexts as $areaid => $areacontexts) {
                 if (!empty($data->areaids) && !in_array($areaid, $data->areaids)) {
                     // Skip unused areas.
                     continue;
@@ -324,6 +327,38 @@ class engine extends \core_search\engine {
             $query->addFilterQuery('contextid:(' . implode(' OR ', $allcontexts) . ')');
         }
 
+        if (!$accessinfo->everything && $accessinfo->separategroupscontexts) {
+            // Add another restriction to handle group ids. If there are any contexts using separate
+            // groups, then results in that context will not show unless you belong to the group.
+            // (Note: Access all groups is taken care of earlier, when computing these arrays.)
+
+            // This special exceptions list allows for particularly pig-headed developers to create
+            // multiple search areas within the same module, where one of them uses separate
+            // groups and the other uses visible groups. It is a little inefficient, but this should
+            // be rare.
+            $exceptions = '';
+            if ($accessinfo->visiblegroupscontextsareas) {
+                foreach ($accessinfo->visiblegroupscontextsareas as $contextid => $areaids) {
+                    $exceptions .= ' OR (contextid:' . $contextid . ' AND areaid:(' .
+                            implode(' OR ', $areaids) . '))';
+                }
+            }
+
+            if ($accessinfo->usergroups) {
+                // Either the document has no groupid, or the groupid is one that the user
+                // belongs to, or the context is not one of the separate groups contexts.
+                $query->addFilterQuery('(*:* -groupid:[* TO *]) OR ' .
+                        'groupid:(' . implode(' OR ', $accessinfo->usergroups) . ') OR ' .
+                        '(*:* -contextid:(' . implode(' OR ', $accessinfo->separategroupscontexts) . '))' .
+                        $exceptions);
+            } else {
+                // Either the document has no groupid, or the context is not a restricted one.
+                $query->addFilterQuery('(*:* -groupid:[* TO *]) OR ' .
+                        '(*:* -contextid:(' . implode(' OR ', $accessinfo->separategroupscontexts) . '))' .
+                        $exceptions);
+            }
+        }
+
         if ($this->file_indexing_enabled()) {
             // Now group records by solr_filegroupingid. Limit to 3 results per group.
             $query->setGroup(true);
@@ -1086,6 +1121,12 @@ class engine extends \core_search\engine {
             return $configured;
         }
 
+        // Update schema if required/possible.
+        $schemalatest = $this->check_latest_schema();
+        if ($schemalatest !== true) {
+            return $schemalatest;
+        }
+
         // Check that the schema is already set up.
         try {
             $schema = new \search_solr\schema();
@@ -1280,4 +1321,40 @@ class engine extends \core_search\engine {
 
         return new \moodle_url($url);
     }
+
+    /**
+     * Solr includes group support in the execute_query function.
+     *
+     * @return bool True
+     */
+    public function supports_group_filtering() {
+        return true;
+    }
+
+    protected function update_schema($oldversion, $newversion) {
+        // Construct schema.
+        $schema = new schema();
+        $cansetup = $schema->can_setup_server();
+        if ($cansetup !== true) {
+            return $cansetup;
+        }
+
+        switch ($newversion) {
+            // This version just requires a setup call to add new fields.
+            case 2017091700:
+                $setup = true;
+                break;
+
+            // If we don't know about the schema version we might not have implemented the
+            // change correctly, so return.
+            default:
+                return get_string('schemaversionunknown', 'search');
+        }
+
+        if ($setup) {
+            $schema->setup();
+        }
+
+        return true;
+    }
 }
index 86bd777..a62d4b9 100644 (file)
@@ -116,7 +116,12 @@ class schema {
 
         $this->check_index();
 
-        return $this->add_fields($fields, $checkexisting);
+        $return = $this->add_fields($fields, $checkexisting);
+
+        // Tell the engine we are now using the latest schema version.
+        $this->engine->record_applied_schema_version(document::SCHEMA_VERSION);
+
+        return $return;
     }
 
     /**
index 6d3d117..7a4bcde 100644 (file)
@@ -54,7 +54,7 @@ require_once($CFG->dirroot . '/search/engine/solr/tests/fixtures/testable_engine
 class search_solr_engine_testcase extends advanced_testcase {
 
     /**
-     * @var \core_search::manager
+     * @var \core_search\manager
      */
     protected $search = null;
 
@@ -707,18 +707,18 @@ class search_solr_engine_testcase extends advanced_testcase {
         $querydata->q = 'Something1 Something2 Something3 Something4';
 
         // In this first set, it should have determined the first 10 of 40 are bad, so there could be up to 30 left.
-        $results = $this->engine->execute_query($querydata, true, 5);
+        $results = $this->engine->execute_query($querydata, (object)['everything' => true], 5);
         $this->assertEquals(30, $this->engine->get_query_total_count());
         $this->assertCount(5, $results);
 
         // To get to 15, it has to process the first 10 that are bad, 10 that are good, 10 that are bad, then 5 that are good.
         // So we now know 20 are bad out of 40.
-        $results = $this->engine->execute_query($querydata, true, 15);
+        $results = $this->engine->execute_query($querydata, (object)['everything' => true], 15);
         $this->assertEquals(20, $this->engine->get_query_total_count());
         $this->assertCount(15, $results);
 
         // Try to get more then all, make sure we still see 20 count and 20 returned.
-        $results = $this->engine->execute_query($querydata, true, 30);
+        $results = $this->engine->execute_query($querydata, (object)['everything' => true], 30);
         $this->assertEquals(20, $this->engine->get_query_total_count());
         $this->assertCount(20, $results);
     }
@@ -830,6 +830,111 @@ class search_solr_engine_testcase extends advanced_testcase {
         $this->assert_result_titles([], $results);
     }
 
+    /**
+     * Tests searching for results in groups, either by specified group ids or based on user
+     * access permissions.
+     */
+    public function test_groups() {
+        global $USER;
+
+        // Use real search areas.
+        $this->search->clear_static();
+        $this->search->add_core_search_areas();
+
+        // Create 2 courses and a selection of forums with different group mode.
+        $generator = $this->getDataGenerator();
+        $course1 = $generator->create_course(['fullname' => 'Course 1']);
+        $forum1nogroups = $generator->create_module('forum', ['course' => $course1, 'groupmode' => NOGROUPS]);
+        $forum1separategroups = $generator->create_module('forum', ['course' => $course1, 'groupmode' => SEPARATEGROUPS]);
+        $forum1visiblegroups = $generator->create_module('forum', ['course' => $course1, 'groupmode' => VISIBLEGROUPS]);
+        $course2 = $generator->create_course(['fullname' => 'Course 2']);
+        $forum2separategroups = $generator->create_module('forum', ['course' => $course2, 'groupmode' => SEPARATEGROUPS]);
+
+        // Create two groups on each course.
+        $group1a = $generator->create_group(['courseid' => $course1->id]);
+        $group1b = $generator->create_group(['courseid' => $course1->id]);
+        $group2a = $generator->create_group(['courseid' => $course2->id]);
+        $group2b = $generator->create_group(['courseid' => $course2->id]);
+
+        // Create search records in each activity and (where relevant) in each group.
+        $forumgenerator = $generator->get_plugin_generator('mod_forum');
+        $forumgenerator->create_discussion(['course' => $course1->id, 'userid' => $USER->id,
+                'forum' => $forum1nogroups->id, 'name' => 'F1NG', 'message' => 'xyzzy']);
+        $forumgenerator->create_discussion(['course' => $course1->id, 'userid' => $USER->id,
+                'forum' => $forum1separategroups->id, 'name' => 'F1SG-A',  'message' => 'xyzzy',
+                'groupid' => $group1a->id]);
+        $forumgenerator->create_discussion(['course' => $course1->id, 'userid' => $USER->id,
+                'forum' => $forum1separategroups->id, 'name' => 'F1SG-B', 'message' => 'xyzzy',
+                'groupid' => $group1b->id]);
+        $forumgenerator->create_discussion(['course' => $course1->id, 'userid' => $USER->id,
+                'forum' => $forum1visiblegroups->id, 'name' => 'F1VG-A', 'message' => 'xyzzy',
+                'groupid' => $group1a->id]);
+        $forumgenerator->create_discussion(['course' => $course1->id, 'userid' => $USER->id,
+                'forum' => $forum1visiblegroups->id, 'name' => 'F1VG-B', 'message' => 'xyzzy',
+                'groupid' => $group1b->id]);
+        $forumgenerator->create_discussion(['course' => $course2->id, 'userid' => $USER->id,
+                'forum' => $forum2separategroups->id, 'name' => 'F2SG-A', 'message' => 'xyzzy',
+                'groupid' => $group2a->id]);
+        $forumgenerator->create_discussion(['course' => $course2->id, 'userid' => $USER->id,
+                'forum' => $forum2separategroups->id, 'name' => 'F2SG-B', 'message' => 'xyzzy',
+                'groupid' => $group2b->id]);
+
+        $this->search->index();
+
+        // Search as admin user should find everything.
+        $querydata = new stdClass();
+        $querydata->q = 'xyzzy';
+        $results = $this->search->search($querydata);
+        $this->assert_result_titles(
+                ['F1NG', 'F1SG-A', 'F1SG-B', 'F1VG-A', 'F1VG-B', 'F2SG-A', 'F2SG-B'], $results);
+
+        // Admin user manually restricts results by groups.
+        $querydata->groupids = [$group1b->id, $group2a->id];
+        $results = $this->search->search($querydata);
+        $this->assert_result_titles(['F1SG-B', 'F1VG-B', 'F2SG-A'], $results);
+
+        // Student enrolled in both courses but no groups.
+        $student1 = $generator->create_user();
+        $generator->enrol_user($student1->id, $course1->id, 'student');
+        $generator->enrol_user($student1->id, $course2->id, 'student');
+        $this->setUser($student1);
+
+        unset($querydata->groupids);
+        $results = $this->search->search($querydata);
+        $this->assert_result_titles(['F1NG', 'F1VG-A', 'F1VG-B'], $results);
+
+        // Student enrolled in both courses and group A in both cases.
+        $student2 = $generator->create_user();
+        $generator->enrol_user($student2->id, $course1->id, 'student');
+        $generator->enrol_user($student2->id, $course2->id, 'student');
+        groups_add_member($group1a, $student2);
+        groups_add_member($group2a, $student2);
+        $this->setUser($student2);
+
+        $results = $this->search->search($querydata);
+        $this->assert_result_titles(['F1NG', 'F1SG-A', 'F1VG-A', 'F1VG-B', 'F2SG-A'], $results);
+
+        // Manually restrict results to group B in course 1.
+        $querydata->groupids = [$group1b->id];
+        $results = $this->search->search($querydata);
+        $this->assert_result_titles(['F1VG-B'], $results);
+
+        // Manually restrict results to group A in course 1.
+        $querydata->groupids = [$group1a->id];
+        $results = $this->search->search($querydata);
+        $this->assert_result_titles(['F1SG-A', 'F1VG-A'], $results);
+
+        // Manager enrolled in both courses (has access all groups).
+        $manager = $generator->create_user();
+        $generator->enrol_user($manager->id, $course1->id, 'manager');
+        $generator->enrol_user($manager->id, $course2->id, 'manager');
+        $this->setUser($manager);
+        unset($querydata->groupids);
+        $results = $this->search->search($querydata);
+        $this->assert_result_titles(
+                ['F1NG', 'F1SG-A', 'F1SG-B', 'F1VG-A', 'F1VG-B', 'F2SG-A', 'F2SG-B'], $results);
+    }
+
     /**
      * Asserts that the returned documents have the expected titles (regardless of order).
      *
index 380ca7f..d1f566d 100644 (file)
@@ -88,4 +88,35 @@ class search_engine_testcase extends advanced_testcase {
         $this->assertEquals($dbreads, $DB->perf_get_reads());
 
     }
+
+    /**
+     * Tests the core functions related to schema updates.
+     */
+    public function test_engine_schema_modification() {
+        // Apply a schema update starting from no version.
+        $engine = new \mock_search\engine();
+        $engine->check_latest_schema();
+        $updates = $engine->get_and_clear_schema_updates();
+        $this->assertCount(1, $updates);
+        $this->assertEquals(0, $updates[0][0]);
+        $this->assertEquals(\core_search\document::SCHEMA_VERSION, $updates[0][1]);
+
+        // Store older version and check that.
+        $engine->record_applied_schema_version(1066101400);
+
+        $engine = new \mock_search\engine();
+        $engine->check_latest_schema();
+        $updates = $engine->get_and_clear_schema_updates();
+        $this->assertCount(1, $updates);
+        $this->assertEquals(1066101400, $updates[0][0]);
+        $this->assertEquals(\core_search\document::SCHEMA_VERSION, $updates[0][1]);
+
+        // Store current version and check no updates.
+        $engine->record_applied_schema_version(\core_search\document::SCHEMA_VERSION);
+
+        $engine = new \mock_search\engine();
+        $engine->check_latest_schema();
+        $updates = $engine->get_and_clear_schema_updates();
+        $this->assertCount(0, $updates);
+    }
 }
index 235082f..c263e62 100644 (file)
@@ -37,6 +37,9 @@ class engine extends \core_search\engine {
     /** @var \core_search\document[] Documents added */
     protected $added = [];
 
+    /** @var array Schema updates applied */
+    protected $schemaupdates = [];
+
     public function is_installed() {
         return true;
     }
@@ -97,4 +100,20 @@ class engine extends \core_search\engine {
         $this->added = [];
         return $added;
     }
+
+    public function update_schema($oldversion, $newversion) {
+        $this->schemaupdates[] = [$oldversion, $newversion];
+    }
+
+    /**
+     * Gets all schema updates applied, as an array. Each entry has an array with two values,
+     * old and new version.
+     *
+     * @return array List of schema updates for comparison
+     */
+    public function get_and_clear_schema_updates() {
+        $result = $this->schemaupdates;
+        $this->schemaupdates = [];
+        return $result;
+    }
 }
index b64cb15..675bc6f 100644 (file)
@@ -555,20 +555,20 @@ class search_manager_testcase extends advanced_testcase {
         $search->add_search_area($mockareaid, new core_mocksearch\search\mock_search_area());
 
         $this->setAdminUser();
-        $this->assertTrue($search->get_areas_user_accesses());
+        $this->assertEquals((object)['everything' => true], $search->get_areas_user_accesses());
 
         $sitectx = \context_course::instance(SITEID);
 
         // Can access the frontpage ones.
         $this->setUser($noaccess);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertEquals(array($frontpageforumcontext->id => $frontpageforumcontext->id), $contexts[$this->forumpostareaid]);
         $this->assertEquals(array($sitectx->id => $sitectx->id), $contexts[$this->mycoursesareaid]);
         $mockctxs = array($noaccessctx->id => $noaccessctx->id, $frontpagectx->id => $frontpagectx->id);
         $this->assertEquals($mockctxs, $contexts[$mockareaid]);
 
         $this->setUser($teacher);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $frontpageandcourse1 = array($frontpageforumcontext->id => $frontpageforumcontext->id, $context1->id => $context1->id,
             $context2->id => $context2->id);
         $this->assertEquals($frontpageandcourse1, $contexts[$this->forumpostareaid]);
@@ -579,7 +579,7 @@ class search_manager_testcase extends advanced_testcase {
         $this->assertEquals($mockctxs, $contexts[$mockareaid]);
 
         $this->setUser($student);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertEquals($frontpageandcourse1, $contexts[$this->forumpostareaid]);
         $this->assertEquals(array($sitectx->id => $sitectx->id, $course1ctx->id => $course1ctx->id),
             $contexts[$this->mycoursesareaid]);
@@ -589,39 +589,39 @@ class search_manager_testcase extends advanced_testcase {
 
         // Hide the activity.
         set_coursemodule_visible($forum2->cmid, 0);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertEquals(array($frontpageforumcontext->id => $frontpageforumcontext->id, $context1->id => $context1->id),
             $contexts[$this->forumpostareaid]);
 
         // Now test course limited searches.
         set_coursemodule_visible($forum2->cmid, 1);
         $this->getDataGenerator()->enrol_user($student->id, $course2->id, 'student');
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $allcontexts = array($frontpageforumcontext->id => $frontpageforumcontext->id, $context1->id => $context1->id,
             $context2->id => $context2->id, $context3->id => $context3->id);
         $this->assertEquals($allcontexts, $contexts[$this->forumpostareaid]);
         $this->assertEquals(array($sitectx->id => $sitectx->id, $course1ctx->id => $course1ctx->id,
             $course2ctx->id => $course2ctx->id), $contexts[$this->mycoursesareaid]);
 
-        $contexts = $search->get_areas_user_accesses(array($course1->id, $course2->id));
+        $contexts = $search->get_areas_user_accesses(array($course1->id, $course2->id))->usercontexts;
         $allcontexts = array($context1->id => $context1->id, $context2->id => $context2->id, $context3->id => $context3->id);
         $this->assertEquals($allcontexts, $contexts[$this->forumpostareaid]);
         $this->assertEquals(array($course1ctx->id => $course1ctx->id,
             $course2ctx->id => $course2ctx->id), $contexts[$this->mycoursesareaid]);
 
-        $contexts = $search->get_areas_user_accesses(array($course2->id));
+        $contexts = $search->get_areas_user_accesses(array($course2->id))->usercontexts;
         $allcontexts = array($context3->id => $context3->id);
         $this->assertEquals($allcontexts, $contexts[$this->forumpostareaid]);
         $this->assertEquals(array($course2ctx->id => $course2ctx->id), $contexts[$this->mycoursesareaid]);
 
-        $contexts = $search->get_areas_user_accesses(array($course1->id));
+        $contexts = $search->get_areas_user_accesses(array($course1->id))->usercontexts;
         $allcontexts = array($context1->id => $context1->id, $context2->id => $context2->id);
         $this->assertEquals($allcontexts, $contexts[$this->forumpostareaid]);
         $this->assertEquals(array($course1ctx->id => $course1ctx->id), $contexts[$this->mycoursesareaid]);
 
         // Test context limited search with no course limit.
         $contexts = $search->get_areas_user_accesses(false,
-                [$frontpageforumcontext->id, $course2ctx->id]);
+                [$frontpageforumcontext->id, $course2ctx->id])->usercontexts;
         $this->assertEquals([$frontpageforumcontext->id => $frontpageforumcontext->id],
                 $contexts[$this->forumpostareaid]);
         $this->assertEquals([$course2ctx->id => $course2ctx->id],
@@ -629,19 +629,19 @@ class search_manager_testcase extends advanced_testcase {
 
         // Test context limited search with course limit.
         $contexts = $search->get_areas_user_accesses([$course1->id, $course2->id],
-                [$frontpageforumcontext->id, $course2ctx->id]);
+                [$frontpageforumcontext->id, $course2ctx->id])->usercontexts;
         $this->assertArrayNotHasKey($this->forumpostareaid, $contexts);
         $this->assertEquals([$course2ctx->id => $course2ctx->id],
                 $contexts[$this->mycoursesareaid]);
 
         // Single context and course.
-        $contexts = $search->get_areas_user_accesses([$course1->id], [$context1->id]);
+        $contexts = $search->get_areas_user_accesses([$course1->id], [$context1->id])->usercontexts;
         $this->assertEquals([$context1->id => $context1->id], $contexts[$this->forumpostareaid]);
         $this->assertArrayNotHasKey($this->mycoursesareaid, $contexts);
 
         // For admins, this is still limited only if we specify the things, so it should be same.
         $this->setAdminUser();
-        $contexts = $search->get_areas_user_accesses([$course1->id], [$context1->id]);
+        $contexts = $search->get_areas_user_accesses([$course1->id], [$context1->id])->usercontexts;
         $this->assertEquals([$context1->id => $context1->id], $contexts[$this->forumpostareaid]);
         $this->assertArrayNotHasKey($this->mycoursesareaid, $contexts);
     }
@@ -722,28 +722,28 @@ class search_manager_testcase extends advanced_testcase {
 
         // Admin gets 'true' result to function regardless of blocks.
         $this->setAdminUser();
-        $this->assertTrue($search->get_areas_user_accesses());
+        $this->assertEquals((object)['everything' => true], $search->get_areas_user_accesses());
 
         // Student 1 gets all 3 block contexts.
         $this->setUser($student1);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertArrayHasKey('block_html-content', $contexts);
         $this->assertCount(3, $contexts['block_html-content']);
 
         // Student 2 does not get any blocks.
         $this->setUser($student2);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertArrayNotHasKey('block_html-content', $contexts);
 
         // Student 3 gets only two of them.
         $this->setUser($student3);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertArrayHasKey('block_html-content', $contexts);
         $this->assertCount(2, $contexts['block_html-content']);
 
         // A course limited search for student 1 is the same as the student 3 search.
         $this->setUser($student1);
-        $limitedcontexts = $search->get_areas_user_accesses([$course3->id]);
+        $limitedcontexts = $search->get_areas_user_accesses([$course3->id])->usercontexts;
         $this->assertEquals($contexts['block_html-content'], $limitedcontexts['block_html-content']);
 
         // Get block context ids for the blocks that appear.
@@ -758,12 +758,12 @@ class search_manager_testcase extends advanced_testcase {
 
         // Context limited search (no course).
         $contexts = $search->get_areas_user_accesses(false,
-                [$blockcontextids[0], $blockcontextids[2]]);
+                [$blockcontextids[0], $blockcontextids[2]])->usercontexts;
         $this->assertCount(2, $contexts['block_html-content']);
 
         // Context limited search (with course 3).
         $contexts = $search->get_areas_user_accesses([$course2->id, $course3->id],
-                [$blockcontextids[0], $blockcontextids[2]]);
+                [$blockcontextids[0], $blockcontextids[2]])->usercontexts;
         $this->assertCount(1, $contexts['block_html-content']);
     }
 
@@ -817,16 +817,16 @@ class search_manager_testcase extends advanced_testcase {
 
         // Admin user can access everything.
         $this->setAdminUser();
-        $this->assertTrue($search->get_areas_user_accesses());
+        $this->assertEquals((object)['everything' => true], $search->get_areas_user_accesses());
 
         // No-access user can access only the front page forum.
         $this->setUser($noaccess);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertEquals([$forumfrontctx->id], array_keys($contexts[$this->forumpostareaid]));
 
         // Student can access the front page forum plus the enrolled one.
         $this->setUser($student);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertEquals([$forum1ctx->id, $forumfrontctx->id],
                 array_keys($contexts[$this->forumpostareaid]));
 
@@ -835,21 +835,119 @@ class search_manager_testcase extends advanced_testcase {
 
         // Admin user can access everything.
         $this->setAdminUser();
-        $this->assertTrue($search->get_areas_user_accesses());
+        $this->assertEquals((object)['everything' => true], $search->get_areas_user_accesses());
 
         // No-access user can access the front page forum and course 2, 3.
         $this->setUser($noaccess);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertEquals([$forum2ctx->id, $forum3ctx->id, $forumfrontctx->id],
                 array_keys($contexts[$this->forumpostareaid]));
 
         // Student can access the front page forum plus the enrolled one plus courses 2, 3.
         $this->setUser($student);
-        $contexts = $search->get_areas_user_accesses();
+        $contexts = $search->get_areas_user_accesses()->usercontexts;
         $this->assertEquals([$forum1ctx->id, $forum2ctx->id, $forum3ctx->id, $forumfrontctx->id],
                 array_keys($contexts[$this->forumpostareaid]));
     }
 
+    /**
+     * Tests group-related aspects of the get_areas_user_accesses function.
+     */
+    public function test_search_user_accesses_groups() {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        // Create 2 courses each with 2 groups and 2 forums (separate/visible groups).
+        $generator = $this->getDataGenerator();
+        $course1 = $generator->create_course();
+        $course2 = $generator->create_course();
+        $group1 = $generator->create_group(['courseid' => $course1->id]);
+        $group2 = $generator->create_group(['courseid' => $course1->id]);
+        $group3 = $generator->create_group(['courseid' => $course2->id]);
+        $group4 = $generator->create_group(['courseid' => $course2->id]);
+        $forum1s = $generator->create_module('forum', ['course' => $course1->id, 'groupmode' => SEPARATEGROUPS]);
+        $id1s = context_module::instance($forum1s->cmid)->id;
+        $forum1v = $generator->create_module('forum', ['course' => $course1->id, 'groupmode' => VISIBLEGROUPS]);
+        $id1v = context_module::instance($forum1v->cmid)->id;
+        $forum2s = $generator->create_module('forum', ['course' => $course2->id, 'groupmode' => SEPARATEGROUPS]);
+        $id2s = context_module::instance($forum2s->cmid)->id;
+        $forum2n = $generator->create_module('forum', ['course' => $course2->id, 'groupmode' => NOGROUPS]);
+        $id2n = context_module::instance($forum2n->cmid)->id;
+
+        // Get search instance.
+        $search = testable_core_search::instance();
+        $search->add_core_search_areas();
+
+        // User 1 is a manager in one course and a student in the other one. They belong to
+        // all of the groups 1, 2, 3, and 4.
+        $user1 = $generator->create_user();
+        $generator->enrol_user($user1->id, $course1->id, 'manager');
+        $generator->enrol_user($user1->id, $course2->id, 'student');
+        groups_add_member($group1, $user1);
+        groups_add_member($group2, $user1);
+        groups_add_member($group3, $user1);
+        groups_add_member($group4, $user1);
+
+        $this->setUser($user1);
+        $accessinfo = $search->get_areas_user_accesses();
+        $contexts = $accessinfo->usercontexts;
+
+        // Double-check all the forum contexts.
+        $postcontexts = $contexts['mod_forum-post'];
+        sort($postcontexts);
+        $this->assertEquals([$id1s, $id1v, $id2s, $id2n], $postcontexts);
+
+        // Only the context in the second course (no accessallgroups) is restricted.
+        $restrictedcontexts = $accessinfo->separategroupscontexts;
+        sort($restrictedcontexts);
+        $this->assertEquals([$id2s], $restrictedcontexts);
+
+        // Only the groups from the second course (no accessallgroups) are included.
+        $groupids = $accessinfo->usergroups;
+        sort($groupids);
+        $this->assertEquals([$group3->id, $group4->id], $groupids);
+
+        // User 2 is a student in each course and belongs to groups 2 and 4.
+        $user2 = $generator->create_user();
+        $generator->enrol_user($user2->id, $course1->id, 'student');
+        $generator->enrol_user($user2->id, $course2->id, 'student');
+        groups_add_member($group2, $user2);
+        groups_add_member($group4, $user2);
+
+        $this->setUser($user2);
+        $accessinfo = $search->get_areas_user_accesses();
+        $contexts = $accessinfo->usercontexts;
+
+        // Double-check all the forum contexts.
+        $postcontexts = $contexts['mod_forum-post'];
+        sort($postcontexts);
+        $this->assertEquals([$id1s, $id1v, $id2s, $id2n], $postcontexts);
+
+        // Both separate groups forums are restricted.
+        $restrictedcontexts = $accessinfo->separategroupscontexts;
+        sort($restrictedcontexts);
+        $this->assertEquals([$id1s, $id2s], $restrictedcontexts);
+
+        // Groups from both courses are included.
+        $groupids = $accessinfo->usergroups;
+        sort($groupids);
+        $this->assertEquals([$group2->id, $group4->id], $groupids);
+
+        // User 3 is a manager at system level.
+        $user3 = $generator->create_user();
+        role_assign($DB->get_field('role', 'id', ['shortname' => 'manager'], MUST_EXIST), $user3->id,
+                \context_system::instance());
+
+        $this->setUser($user3);
+        $accessinfo = $search->get_areas_user_accesses();
+
+        // Nothing is restricted and no groups are relevant.
+        $this->assertEquals([], $accessinfo->separategroupscontexts);
+        $this->assertEquals([], $accessinfo->usergroups);
+    }
+
     /**
      * test_is_search_area
      *
index 0371a65..ddc5efc 100644 (file)
@@ -10,6 +10,26 @@ information provided here is intended especially for developers.
   the newest items first; for other types of search area it will just index the whole system
   context, oldest data first.
 
+* Module search areas that wish to support group filtering should set the new optional search
+  document field groupid (note: to remain compatible with earlier versions, do this inside an if
+  statement so that it only happens on 3.4+) and return true to the supports_group_restriction
+  function. See documentation in \core_search\base_mod class and example in \mod_forum\search\post.
+
+* When a search engine supports group filtering, the \core_search\manager::search function now
+  accepts the optional 'groupids' parameter in its $data input. This parameter is an array of one
+  or more group IDs. If supplied, only results from those groups will be returned.
+
+* Search engine plugins will need to be be modified if they wish to support group filtering.
+  (Search engines should continue to work unmodified, but will not then support group filtering.)
+  The modification steps are:
+  - Implement the new update_schema function to make the schema change (add groupid field).
+  - Ensure that the groupid field is stored correctly when provided in a document while indexing.
+  - Return true to new supports_group_filtering() function.
+  - execute_query should support the new $data->groupids parameter (to allow users to restrict
+    search results to specific groups) and the modified meaning of the second parameter,
+    $accessinfo (to automatically restrict search results users cannot access due to groups).
+    See implementation in Solr search engine.
+
 === 3.4 ===
 
 * Search indexing now supports time limits to make the scheduled task run more neatly. In order for
index 7d1c68f..2084c78 100644 (file)
@@ -153,12 +153,12 @@ div.backup-section + form,
 // I think this could be avoided (or at least tidied up) ifr
 // we used HTML5 input types like url, phone, email, number etc.
 /* rtl:ignore */
-.mform .fitem .felement input[name=email],
-.mform .fitem .felement input[name=email2],
-.mform .fitem .felement input[name=url],
-.mform .fitem .felement input[name=idnumber],
-.mform .fitem .felement input[name=phone1],
-.mform .fitem .felement input[name=phone2] {
+.mform .fitem .felement input[name="email"],
+.mform .fitem .felement input[name="email2"],
+.mform .fitem .felement input[name="url"],
+.mform .fitem .felement input[name="idnumber"],
+.mform .fitem .felement input[name="phone1"],
+.mform .fitem .felement input[name="phone2"] {
     text-align: left;
     direction: ltr;
 }
@@ -326,7 +326,7 @@ select[size],
 select[multiple] {
     overflow: auto;
 }
-select[size=1] {
+select[size="1"] {
     overflow: visible;
 }
 
diff --git a/theme/boost/tests/scss_test.php b/theme/boost/tests/scss_test.php
new file mode 100644 (file)
index 0000000..5b1fc8a
--- /dev/null
@@ -0,0 +1,50 @@
+<?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 file contains the unittests for boost's scss compilation.
+ *
+ * @package   theme_boost
+ * @copyright 2018 Cameron Ball <cameron@cameron1729.xyz>
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Unit tests for scss compilation.
+ *
+ * @package   theme_boost
+ * @copyright 2016 onwards Ankit Agarwal
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class theme_boost_scss_testcase extends advanced_testcase {
+    /**
+     * Test that boost can be compiled using SassC (the defacto implemention).
+     */
+    public function test_scss_compilation_with_sassc() {
+        if (!defined('PHPUNIT_PATH_TO_SASSC')) {
+            $this->markTestSkipped('Path to SassC not provided');
+        }
+
+        $this->resetAfterTest();
+        set_config('pathtosassc', PHPUNIT_PATH_TO_SASSC);
+
+        $this->assertNotEmpty(
+            theme_config::load('boost')->get_css_content_debug('scss', null, null)
+        );
+    }
+}
index f837016..7fc19ff 100644 (file)
@@ -176,18 +176,20 @@ $userform = new user_edit_form(new moodle_url($PAGE->url, array('returnto' => $r
 
 $emailchanged = false;
 
-if ($usernew = $userform->get_data()) {
-
-    // Deciding where to send the user back in most cases.
-    if ($returnto === 'profile') {
-        if ($course->id != SITEID) {
-            $returnurl = new moodle_url('/user/view.php', array('id' => $user->id, 'course' => $course->id));
-        } else {
-            $returnurl = new moodle_url('/user/profile.php', array('id' => $user->id));
-        }
+// Deciding where to send the user back in most cases.
+if ($returnto === 'profile') {
+    if ($course->id != SITEID) {
+        $returnurl = new moodle_url('/user/view.php', array('id' => $user->id, 'course' => $course->id));
     } else {
-        $returnurl = new moodle_url('/user/preferences.php', array('userid' => $user->id));
+        $returnurl = new moodle_url('/user/profile.php', array('id' => $user->id));
     }
+} else {
+    $returnurl = new moodle_url('/user/preferences.php', array('userid' => $user->id));
+}
+
+if ($userform->is_cancelled()) {
+    redirect($returnurl);
+} else if ($usernew = $userform->get_data()) {
 
     $emailchangedhtml = '';
 
index bb614ac..4f2214f 100644 (file)
@@ -104,7 +104,7 @@ class user_edit_form extends moodleform {
         // Next the customisable profile fields.
         profile_definition($mform, $userid);
 
-        $this->add_action_buttons(false, get_string('updatemyprofile'));
+        $this->add_action_buttons(true, get_string('updatemyprofile'));
 
         $this->set_data($user);
     }
index 6f3d07e..07fc5e0 100644 (file)
@@ -154,7 +154,21 @@ $userform = new user_editadvanced_form(new moodle_url($PAGE->url, array('returnt
     'filemanageroptions' => $filemanageroptions,
     'user' => $user));
 
-if ($usernew = $userform->get_data()) {
+
+// Deciding where to send the user back in most cases.
+if ($returnto === 'profile') {
+    if ($course->id != SITEID) {
+        $returnurl = new moodle_url('/user/view.php', array('id' => $user->id, 'course' => $course->id));
+    } else {
+        $returnurl = new moodle_url('/user/profile.php', array('id' => $user->id));
+    }
+} else {
+    $returnurl = new moodle_url('/user/preferences.php', array('userid' => $user->id));
+}
+
+if ($userform->is_cancelled()) {
+    redirect($returnurl);
+} else if ($usernew = $userform->get_data()) {
     $usercreated = false;
 
     if (empty($usernew->auth)) {
@@ -292,15 +306,6 @@ if ($usernew = $userform->get_data()) {
             // Somebody double clicked when editing admin user during install.
             redirect("$CFG->wwwroot/$CFG->admin/");
         } else {
-            if ($returnto === 'profile') {
-                if ($course->id != SITEID) {
-                    $returnurl = new moodle_url('/user/view.php', array('id' => $user->id, 'course' => $course->id));
-                } else {
-                    $returnurl = new moodle_url('/user/profile.php', array('id' => $user->id));
-                }
-            } else {
-                $returnurl = new moodle_url('/user/preferences.php', array('userid' => $user->id));
-            }
             redirect($returnurl);
         }
     } else {
index d2bd16a..fe2fd31 100644 (file)
@@ -154,7 +154,7 @@ class user_editadvanced_form extends moodleform {
             $btnstring = get_string('updatemyprofile');
         }
 
-        $this->add_action_buttons(false, $btnstring);
+        $this->add_action_buttons(true, $btnstring);
 
         $this->set_data($user);
     }
index d309ffd..4f26b27 100644 (file)
@@ -35,6 +35,13 @@ Feature: Both first name and surname are always available for every user
     # End UI test covering "I open my profile in edit mode"
     When I set the field "First name" to " "
     And I set the field "Surname" to " "
+    And I click on "Cancel" "button"
+    And I follow "Profile" in the user menu
+    And I click on "Edit profile" "link" in the "region-main" "region"
+    Then I should see "Foo"
+    And I should see "Bar"
+    When I set the field "First name" to " "
+    And I set the field "Surname" to " "
     And I click on "Update profile" "button"
     Then I should see "Missing given name"
     And I should see "Missing surname"
@@ -49,6 +56,13 @@ Feature: Both first name and surname are always available for every user
     And I click on "Edit profile" "link" in the "region-main" "region"
     When I set the field "First name" to " "
     And I set the field "Surname" to " "
+    And I click on "Cancel" "button"
+    And I follow "Foo Bar"
+    And I click on "Edit profile" "link" in the "region-main" "region"
+    Then I should see "Foo"
+    And I should see "Bar"
+    When I set the field "First name" to " "
+    And I set the field "Surname" to " "
     And I click on "Update profile" "button"
     Then I should see "Missing given name"
     And I should see "Missing surname"