Skip to content

Commit

Permalink
MDL-49642 mod_lesson: Fix issues found during integration review
Browse files Browse the repository at this point in the history
  • Loading branch information
Jean-Michel Vedrine committed Apr 5, 2015
1 parent 4ce4008 commit 247980b
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 119 deletions.
42 changes: 21 additions & 21 deletions mod/lesson/backup/moodle2/backup_lesson_stepslib.php
Expand Up @@ -20,27 +20,27 @@
*
* This is the "graphical" structure of the lesson module:
*
* lesson ------------>---------------|-------------->-----------|------------->------------|
* (CL,pk->id) | | |
* | | | |
* | lesson_grades lesson_high_scores lesson_timer
* | (UL, pk->id,fk->lessonid) (UL, pk->id,fk->lessonid) (UL, pk->id,fk->lessonid)
* | |
* | |
* | |
* | |
* lesson_pages----------->---------lesson_branch
* (CL,pk->id,fk->lessonid) (UL, pk->id,fk->pageid)
* |
* |
* |
* lesson_answers
* (CL,pk->id,fk->pageid)
* |
* |
* |
* lesson_attempts
* (UL,pk->id,fk->answerid)
* lesson ---------->-------------|------------>---------|----------->----------|----------->----------|
* (CL,pk->id) | | | |
* | | | | |
* | lesson_grades lesson_high_scores lesson_timer lesson_overrides
* | (UL, pk->id,fk->lessonid) (UL, pk->id,fk->lessonid) (UL, pk->id,fk->lessonid) (UL, pk->id,fk->lessonid)
* | |
* | |
* | |
* | |
* lesson_pages-------->-------lesson_branch
* (CL,pk->id,fk->lessonid) (UL, pk->id,fk->pageid)
* |
* |
* |
* lesson_answers
* (CL,pk->id,fk->pageid)
* |
* |
* |
* lesson_attempts
* (UL,pk->id,fk->answerid)
*
* Meaning: pk->primary key field of the table
* fk->foreign key to link with parent
Expand Down
2 changes: 1 addition & 1 deletion mod/lesson/classes/group_observers.php
Expand Up @@ -80,6 +80,6 @@ public static function group_deleted($event) {
// We will take care of that once the course reset ends.
return;
}
lesson_process_group_deleted_in_course($event->courseid);
lesson_process_group_deleted_in_course($event->courseid, $event->objectid);
}
}
5 changes: 3 additions & 2 deletions mod/lesson/lang/en/lesson.php
Expand Up @@ -234,6 +234,7 @@
$string['importcount'] = 'Importing {$a} questions';
$string['importquestions'] = 'Import questions';
$string['importquestions_help'] = 'This feature enables questions in a variety of formats to be imported via text file.';
$string['inactiveoverridehelp'] = '* Student does not have the correct group or role to view/attempt the lesson';
$string['insertedpage'] = 'Inserted page';
$string['invalidfile'] = 'Invalid file';
$string['invalidid'] = 'No course module ID or lesson ID were passed';
Expand Down Expand Up @@ -369,9 +370,9 @@
$string['ordered'] = 'Ordered';
$string['other'] = 'Other';
$string['outof'] = 'Out of {$a}';
$string['override'] = 'Override';
$string['overridedeletegroupsure'] = 'Are you sure you want to delete the override for group {$a}?';
$string['overridedeleteusersure'] = 'Are you sure you want to delete the override for user {$a}?';
$string['override'] = 'Override';
$string['overridegroup'] = 'Override group';
$string['overridegroupeventname'] = '{$a->lesson} - {$a->group}';
$string['overrides'] = 'Overrides';
Expand Down Expand Up @@ -495,9 +496,9 @@
$string['usemean'] = 'Use mean';
$string['usepassword'] = 'Password protected lesson';
$string['usepassword_help'] = 'If enabled, a password is required in order to access the lesson.';
$string['useroverrides'] = 'User overrides';
$string['useroverridesdeleted'] = 'User overrides deleted';
$string['usersnone'] = 'No students have access to this lesson';
$string['useroverrides'] = 'User overrides';
$string['viewgrades'] = 'View grades';
$string['viewhighscores'] = 'View high scores list';
$string['viewreports'] = 'View {$a->attempts} completed {$a->student} attempts';
Expand Down
30 changes: 20 additions & 10 deletions mod/lesson/locallib.php
Expand Up @@ -642,21 +642,31 @@ function lesson_get_media_html($lesson, $context) {
* Logic to happen when a/some group(s) has/have been deleted in a course.
*
* @param int $courseid The course ID.
* @param int $groupid The group id if it is known
* @return void
*/
function lesson_process_group_deleted_in_course($courseid) {
function lesson_process_group_deleted_in_course($courseid, $groupid = null) {
global $DB;

// It would be nice if we got the groupid that was deleted.
// Instead, we just update all lesson with orphaned group overrides.
$sql = "SELECT o.id, o.lesson
FROM {lesson_overrides} o
JOIN {lesson} lesson ON lesson.id = o.lessonid
LEFT JOIN {groups} grp ON grp.id = o.groupid
WHERE lesson.course = :courseid
AND o.groupid IS NOT NULL
AND grp.id IS NULL";
$params = array('courseid' => $courseid);
if ($groupid) {
$params['groupid'] = $groupid;
// We just update the group that was deleted.
$sql = "SELECT o.id, o.lessonid
FROM {lesson_overrides} o
JOIN {lesson} lesson ON lesson.id = o.lessonid
WHERE lesson.course = :courseid
AND o.groupid = :groupid";
} else {
// No groupid, we update all orphaned group overrides for all lessons in course.
$sql = "SELECT o.id, o.lessonid
FROM {lesson_overrides} o
JOIN {lesson} lesson ON lesson.id = o.lessonid
LEFT JOIN {groups} grp ON grp.id = o.groupid
WHERE lesson.course = :courseid
AND o.groupid IS NOT NULL
AND grp.id IS NULL";
}
$records = $DB->get_records_sql_menu($sql, $params);
if (!$records) {
return; // Nothing to do.
Expand Down
10 changes: 1 addition & 9 deletions mod/lesson/override_form.php
Expand Up @@ -132,16 +132,8 @@ protected function definition() {
$mform->freeze('userid');
} else {
// Prepare the list of users.
$users = array();
list($sort, $sortparams) = users_order_by_sql('u');
if (!empty($sortparams)) {
throw new coding_exception('users_order_by_sql returned some query parameters. ' .
'This is unexpected, and a problem because there is no way to pass these ' .
'parameters to get_users_by_capability. See MDL-34657.');
}
$users = get_enrolled_users($this->context, '', 0,
'u.id, u.email, ' . get_all_user_name_fields(true, 'u'),
$sort);
'u.id, u.email, ' . get_all_user_name_fields(true, 'u'));

// Filter users based on any fixed restrictions (groups, profile).
$info = new \core_availability\info_module($cm);
Expand Down
12 changes: 6 additions & 6 deletions mod/lesson/overrideedit.php
Expand Up @@ -89,17 +89,17 @@
}
}

// True if group-based override.
$groupmode = !empty($data->groupid) || ($action === 'addgroup' && empty($overrideid));

// If we are duplicating an override, then clear the user/group and override id
// since they will change.
if ($action === 'duplicate') {
$override->id = null;
$override->userid = null;
$override->groupid = null;
$override->id = $data->id = null;
$override->userid = $data->userid = null;
$override->groupid = $data->groupid = null;
}

// True if group-based override.
$groupmode = !empty($data->groupid) || ($action === 'addgroup' && empty($overrideid));

$overridelisturl = new moodle_url('/mod/lesson/overrides.php', array('cmid' => $cm->id));
if (!$groupmode) {
$overridelisturl->param('mode', 'user');
Expand Down
12 changes: 6 additions & 6 deletions mod/lesson/tests/behat/lesson_group_override.feature
Expand Up @@ -63,12 +63,12 @@ Feature: Lesson user override
| deadline[hour] | 08 |
| deadline[minute] | 00 |
And I press "Save"
And I should see "Wednesday, 1 January 2020, 8:00 am"
And I should see "Wednesday, 1 January 2020, 8:00"
Then I click on "Edit" "link"
And I set the following fields to these values:
| deadline[year] | 2030 |
And I press "Save"
And I should see "Tuesday, 1 January 2030, 8:00 am"
And I should see "Tuesday, 1 January 2030, 8:00"
And I click on "Delete" "link"
And I press "Continue"
And I should not see "Group 1"
Expand All @@ -86,13 +86,13 @@ Feature: Lesson user override
| deadline[hour] | 08 |
| deadline[minute] | 00 |
And I press "Save"
And I should see "Wednesday, 1 January 2020, 8:00 am"
And I should see "Wednesday, 1 January 2020, 8:00"
Then I click on "copy" "link"
And I set the following fields to these values:
| Override group | Group 2 |
| deadline[year] | 2030 |
And I press "Save"
And I should see "Tuesday, 1 January 2030, 8:00 am"
And I should see "Tuesday, 1 January 2030, 8:00"
And I should see "Group 2"

Scenario: Allow a single group to have re-take the lesson
Expand Down Expand Up @@ -310,7 +310,7 @@ Feature: Lesson user override
| available[hour] | 08 |
| available[minute] | 00 |
And I press "Save"
And I should see "Wednesday, 1 January 2020, 8:00 am"
And I should see "Wednesday, 1 January 2020, 8:00"
And I navigate to "User overrides" node in "Lesson administration"
And I press "Add user override"
And I set the following fields to these values:
Expand All @@ -322,7 +322,7 @@ Feature: Lesson user override
| available[hour] | 08 |
| available[minute] | 00 |
And I press "Save"
And I should see "Friday, 1 January 2021, 8:00 am"
And I should see "Friday, 1 January 2021, 8:00"
And I log out
Then I log in as "student1"
And I follow "Course 1"
Expand Down
8 changes: 4 additions & 4 deletions mod/lesson/tests/behat/lesson_user_override.feature
Expand Up @@ -56,12 +56,12 @@ Feature: Lesson user override
| deadline[hour] | 08 |
| deadline[minute] | 00 |
And I press "Save"
And I should see "Wednesday, 1 January 2020, 8:00 am"
And I should see "Wednesday, 1 January 2020, 8:00"
Then I click on "Edit" "link"
And I set the following fields to these values:
| deadline[year] | 2030 |
And I press "Save"
And I should see "Tuesday, 1 January 2030, 8:00 am"
And I should see "Tuesday, 1 January 2030, 8:00"
And I click on "Delete" "link"
And I press "Continue"
And I should not see "Sam1 Student1"
Expand All @@ -79,13 +79,13 @@ Feature: Lesson user override
| deadline[hour] | 08 |
| deadline[minute] | 00 |
And I press "Save"
And I should see "Wednesday, 1 January 2020, 8:00 am"
And I should see "Wednesday, 1 January 2020, 8:00"
Then I click on "copy" "link"
And I set the following fields to these values:
| Override user | Student2 |
| deadline[year] | 2030 |
And I press "Save"
And I should see "Tuesday, 1 January 2030, 8:00 am"
And I should see "Tuesday, 1 January 2030, 8:00"
And I should see "Sam2 Student2"

Scenario: Allow a single user to have re-take the lesson
Expand Down

0 comments on commit 247980b

Please sign in to comment.