Skip to content

Commit

Permalink
MDL-44725 Availability: Replace groupmembersonly - workshop (12)
Browse files Browse the repository at this point in the history
The availability restrictions that apply to user lists (group, grouping)
now apply in workshop:

* In user lists.
* When randomly allocating users (also now works as expected if you use
  group mode and a grouping with the activity).
  • Loading branch information
sammarshallou committed Sep 2, 2014
1 parent 1a7049a commit 45ab2d9
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 44 deletions.
5 changes: 1 addition & 4 deletions mod/workshop/allocation/random/tests/allocator_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,8 @@ class workshopallocation_random_testcase extends basic_testcase {
protected function setUp() {
parent::setUp();

$cm = new stdclass();
$course = new stdclass();
$context = new stdclass();
$workshop = (object)array('id' => 42);
$this->workshop = new workshop($workshop, $cm, $course, $context);
$this->workshop = new workshop($workshop, null, null);
$this->allocator = new testable_workshop_random_allocator($this->workshop);
}

Expand Down
5 changes: 1 addition & 4 deletions mod/workshop/eval/best/tests/lib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,8 @@ class workshopeval_best_evaluation_testcase extends basic_testcase {
protected function setUp() {
parent::setUp();

$cm = new stdclass();
$course = new stdclass();
$context = new stdclass();
$workshop = (object)array('id' => 42, 'evaluation' => 'best');
$this->workshop = new workshop($workshop, $cm, $course, $context);
$this->workshop = new workshop($workshop, null, null);
$this->evaluator = new testable_workshop_best_evaluation($this->workshop);
}

Expand Down
5 changes: 1 addition & 4 deletions mod/workshop/form/accumulative/tests/lib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,8 @@ class workshop_accumulative_strategy_testcase extends advanced_testcase {
protected function setUp() {
parent::setUp();

$cm = new stdclass();
$course = new stdclass();
$context = new stdclass();
$workshop = (object)array('id' => 42, 'strategy' => 'accumulative');
$this->workshop = new workshop($workshop, $cm, $course, $context);
$this->workshop = new workshop($workshop, null, null);
$this->strategy = new testable_workshop_accumulative_strategy($this->workshop);
}

Expand Down
5 changes: 1 addition & 4 deletions mod/workshop/form/numerrors/tests/lib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,8 @@ class workshopform_numerrors_strategy_testcase extends advanced_testcase {
protected function setUp() {
parent::setUp();

$cm = new stdclass();
$course = new stdclass();
$context = new stdclass();
$workshop = (object)array('id' => 42, 'strategy' => 'numerrors');
$this->workshop = new workshop($workshop, $cm, $course, $context);
$this->workshop = new workshop($workshop, null, null);
$this->strategy = new testable_workshop_numerrors_strategy($this->workshop);
}

Expand Down
5 changes: 1 addition & 4 deletions mod/workshop/form/rubric/tests/lib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,8 @@ class workshopform_rubric_strategy_test extends advanced_testcase {
*/
protected function setUp() {
parent::setUp();
$cm = new stdclass();
$course = new stdclass();
$context = new stdclass();
$workshop = (object)array('id' => 42, 'strategy' => 'rubric');
$this->workshop = new workshop($workshop, $cm, $course, $context);
$this->workshop = new workshop($workshop, null, null, null);
$this->strategy = new testable_workshop_rubric_strategy($this->workshop);

// prepare dimensions definition
Expand Down
73 changes: 50 additions & 23 deletions mod/workshop/locallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class workshop {
const EXAMPLES_BEFORE_SUBMISSION = 1;
const EXAMPLES_BEFORE_ASSESSMENT = 2;

/** @var stdclass course module record */
/** @var cm_info course module record */
public $cm;

/** @var stdclass course record */
Expand Down Expand Up @@ -178,26 +178,40 @@ class workshop {
/**
* Initializes the workshop API instance using the data from DB
*
* Makes deep copy of all passed records properties. Replaces integer $course attribute
* with a full database record (course should not be stored in instances table anyway).
* Makes deep copy of all passed records properties.
*
* For unit testing only, $cm and $course may be set to null. This is so that
* you can test without having any real database objects if you like. Not all
* functions will work in this situation.
*
* @param stdClass $dbrecord Workshop instance data from {workshop} table
* @param stdClass $cm Course module record as returned by {@link get_coursemodule_from_id()}
* @param stdClass $course Course record from {course} table
* @param stdClass $context The context of the workshop instance
* @param stdClass|cm_info $cm Course module record
* @param stdClass $course Course record from {course} table
* @param stdClass $context The context of the workshop instance
*/
public function __construct(stdclass $dbrecord, stdclass $cm, stdclass $course, stdclass $context=null) {
public function __construct(stdclass $dbrecord, $cm, $course, stdclass $context=null) {
foreach ($dbrecord as $field => $value) {
if (property_exists('workshop', $field)) {
$this->{$field} = $value;
}
}
$this->cm = $cm;
$this->course = $course;
if (is_null($context)) {
$this->context = context_module::instance($this->cm->id);
if (is_null($cm) || is_null($course)) {
if (!PHPUNIT_TEST) {
throw new coding_exception('Must specify $cm and $course');
}
} else {
$this->context = $context;
$this->course = $course;
if ($cm instanceof cm_info) {
$this->cm = $cm;
} else {
$modinfo = get_fast_modinfo($course);
$this->cm = $modinfo->get_cm($cm->id);
}
if (is_null($context)) {
$this->context = context_module::instance($this->cm->id);
} else {
$this->context = $context;
}
}
}

Expand Down Expand Up @@ -595,9 +609,9 @@ public function is_participant($userid=null) {
/**
* Groups the given users by the group membership
*
* This takes the module grouping settings into account. If "Available for group members only"
* is set, returns only groups withing the course module grouping. Always returns group [0] with
* all the given users.
* This takes the module grouping settings into account. If a grouping is
* set, returns only groups withing the course module grouping. Always
* returns group [0] with all the given users.
*
* @param array $users array[userid] => stdclass{->id ->lastname ->firstname}
* @return array array[groupid][userid] => stdclass{->id ->lastname ->firstname}
Expand All @@ -610,10 +624,10 @@ public function get_grouped($users) {
if (empty($users)) {
return $grouped;
}
if (!empty($CFG->enablegroupmembersonly) and $this->cm->groupmembersonly) {
// Available for group members only - the workshop is available only
// to users assigned to groups within the selected grouping, or to
// any group if no grouping is selected.
if ($this->cm->groupingid) {
// Group workshop set to specified grouping - only consider groups
// within this grouping, and leave out users who aren't members of
// this grouping.
$groupingid = $this->cm->groupingid;
// All users that are members of at least one group will be
// added into a virtual group id 0
Expand Down Expand Up @@ -2505,7 +2519,10 @@ protected function aggregate_grading_grades_process(array $assessments, $timegra
* Returns SQL to fetch all enrolled users with the given capability in the current workshop
*
* The returned array consists of string $sql and the $params array. Note that the $sql can be
* empty if groupmembersonly is enabled and the associated grouping is empty.
* empty if a grouping is selected and it has no groups.
*
* The list is automatically restricted according to any availability restrictions
* that apply to user lists (e.g. group, grouping restrictions).
*
* @param string $capability the name of the capability
* @param bool $musthavesubmission ff true, return only users who have already submitted
Expand All @@ -2518,9 +2535,10 @@ protected function get_users_with_capability_sql($capability, $musthavesubmissio
static $inc = 0;
$inc++;

// if the caller requests all groups and we are in groupmembersonly mode, use the
// recursive call of itself to get users from all groups in the grouping
if (empty($groupid) and !empty($CFG->enablegroupmembersonly) and $this->cm->groupmembersonly) {
// If the caller requests all groups and we are using a selected grouping,
// recursively call this function for each group in the grouping (this is
// needed because get_enrolled_sql only supports a single group).
if (empty($groupid) and $this->cm->groupingid) {
$groupingid = $this->cm->groupingid;
$groupinggroupids = array_keys(groups_get_all_groups($this->cm->course, 0, $this->cm->groupingid, 'g.id'));
$sql = array();
Expand Down Expand Up @@ -2549,6 +2567,15 @@ protected function get_users_with_capability_sql($capability, $musthavesubmissio
$params['workshopid'.$inc] = $this->id;
}

// If the activity is restricted so that only certain users should appear
// in user lists, integrate this into the same SQL.
$info = new \core_availability\info_module($this->cm);
list ($listsql, $listparams) = $info->get_user_list_sql(false);
if ($listsql) {
$sql .= " JOIN ($listsql) restricted ON restricted.id = u.id ";
$params = array_merge($params, $listparams);
}

return array($sql, $params);
}

Expand Down
2 changes: 1 addition & 1 deletion mod/workshop/mod_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public function definition() {
plagiarism_get_form_elements_module($mform, $coursecontext, 'mod_workshop');

// Common module settings, Restrict availability, Activity completion etc. ----
$features = array('groups'=>true, 'groupings'=>true, 'groupmembersonly'=>true,
$features = array('groups' => true, 'groupings' => true,
'outcomes'=>true, 'gradecat'=>false, 'idnumber'=>false);

$this->standard_coursemodule_elements();
Expand Down
113 changes: 113 additions & 0 deletions mod/workshop/tests/locallib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -416,4 +416,117 @@ public function test_prepare_example_reference_assessment() {
// excersise SUT
$a = $this->workshop->prepare_example_reference_assessment($fakerawrecord);
}

/**
* Tests user restrictions, as they affect lists of users returned by
* core API functions.
*
* This includes the groupingid option (when group mode is in use), and
* standard activity restrictions using the availability API.
*/
public function test_user_restrictions() {
global $DB, $CFG;

$this->resetAfterTest();

// Use existing sample course from setUp.
$courseid = $this->workshop->course->id;

// Make a test grouping and two groups.
$generator = $this->getDataGenerator();
$grouping = $generator->create_grouping(array('courseid' => $courseid));
$group1 = $generator->create_group(array('courseid' => $courseid));
groups_assign_grouping($grouping->id, $group1->id);
$group2 = $generator->create_group(array('courseid' => $courseid));
groups_assign_grouping($grouping->id, $group2->id);

// Group 3 is not in the grouping.
$group3 = $generator->create_group(array('courseid' => $courseid));

// Enrol some students.
$roleids = $DB->get_records_menu('role', null, '', 'shortname, id');
$student1 = $generator->create_user();
$student2 = $generator->create_user();
$student3 = $generator->create_user();
$generator->enrol_user($student1->id, $courseid, $roleids['student']);
$generator->enrol_user($student2->id, $courseid, $roleids['student']);
$generator->enrol_user($student3->id, $courseid, $roleids['student']);

// Place students in groups (except student 3).
groups_add_member($group1, $student1);
groups_add_member($group2, $student2);
groups_add_member($group3, $student3);

// The existing workshop doesn't have any restrictions, so user lists
// should include all three users.
$allusers = get_enrolled_users(context_course::instance($courseid));
$result = $this->workshop->get_grouped($allusers);
$this->assertCount(4, $result);
$users = array_keys($result[0]);
sort($users);
$this->assertEquals(array($student1->id, $student2->id, $student3->id), $users);
$this->assertEquals(array($student1->id), array_keys($result[$group1->id]));
$this->assertEquals(array($student2->id), array_keys($result[$group2->id]));
$this->assertEquals(array($student3->id), array_keys($result[$group3->id]));

// Test get_users_with_capability_sql (via get_potential_authors).
$users = $this->workshop->get_potential_authors(false);
$this->assertCount(3, $users);
$users = $this->workshop->get_potential_authors(false, $group2->id);
$this->assertEquals(array($student2->id), array_keys($users));

// Create another test workshop with grouping set.
$workshopitem = $this->getDataGenerator()->create_module('workshop',
array('course' => $courseid, 'groupmode' => SEPARATEGROUPS,
'groupingid' => $grouping->id));
$cm = get_coursemodule_from_instance('workshop', $workshopitem->id,
$courseid, false, MUST_EXIST);
$workshopgrouping = new testable_workshop($workshopitem, $cm, $this->workshop->course);

// This time the result should only include users and groups in the
// selected grouping.
$result = $workshopgrouping->get_grouped($allusers);
$this->assertCount(3, $result);
$users = array_keys($result[0]);
sort($users);
$this->assertEquals(array($student1->id, $student2->id), $users);
$this->assertEquals(array($student1->id), array_keys($result[$group1->id]));
$this->assertEquals(array($student2->id), array_keys($result[$group2->id]));

// Test get_users_with_capability_sql (via get_potential_authors).
$users = $workshopgrouping->get_potential_authors(false);
$userids = array_keys($users);
sort($userids);
$this->assertEquals(array($student1->id, $student2->id), $userids);
$users = $workshopgrouping->get_potential_authors(false, $group2->id);
$this->assertEquals(array($student2->id), array_keys($users));

// Enable the availability system and create another test workshop with
// availability restriction on grouping.
$CFG->enableavailability = true;
$workshopitem = $this->getDataGenerator()->create_module('workshop',
array('course' => $courseid, 'availability' => json_encode(
\core_availability\tree::get_root_json(array(
\availability_grouping\condition::get_json($grouping->id)),
\core_availability\tree::OP_AND, false))));
$cm = get_coursemodule_from_instance('workshop', $workshopitem->id,
$courseid, false, MUST_EXIST);
$workshoprestricted = new testable_workshop($workshopitem, $cm, $this->workshop->course);

// The get_grouped function isn't intended to apply this restriction,
// so it should be the same as the base workshop. (Note: in reality,
// get_grouped is always run with the parameter being the result of
// one of the get_potential_xxx functions, so it works.)
$result = $workshoprestricted->get_grouped($allusers);
$this->assertCount(4, $result);
$this->assertCount(3, $result[0]);

// The get_users_with_capability_sql-based functions should apply it.
$users = $workshoprestricted->get_potential_authors(false);
$userids = array_keys($users);
sort($userids);
$this->assertEquals(array($student1->id, $student2->id), $userids);
$users = $workshoprestricted->get_potential_authors(false, $group2->id);
$this->assertEquals(array($student2->id), array_keys($users));
}
}

0 comments on commit 45ab2d9

Please sign in to comment.