Permalink
Browse files

MDL-41417 course: prevent duplicate idnumbers being used when updatin…

…g a course
  • Loading branch information...
1 parent b69ec28 commit 5536a5617c1c1a0073d8ebe53e08191c5a3b9aac Francis Devine committed with markn86 Sep 4, 2013
Showing with 77 additions and 16 deletions.
  1. +12 −8 course/edit_form.php
  2. +2 −8 course/externallib.php
  3. +15 −0 course/lib.php
  4. +48 −0 course/tests/courselib_test.php
View
@@ -332,16 +332,20 @@ function validation($data, $files) {
global $DB, $CFG;
$errors = parent::validation($data, $files);
- if ($foundcourses = $DB->get_records('course', array('shortname'=>$data['shortname']))) {
- if (!empty($data['id'])) {
- unset($foundcourses[$data['id']]);
+
+ // Add field validation check for duplicate shortname.
+ if ($course = $DB->get_record('course', array('shortname' => $data['shortname']), '*', IGNORE_MULTIPLE)) {
+ if (empty($data['id']) || $course->id != $data['id']) {
+ $errors['shortname'] = get_string('shortnametaken', '', $course->fullname);
}
- if (!empty($foundcourses)) {
- foreach ($foundcourses as $foundcourse) {
- $foundcoursenames[] = $foundcourse->fullname;
+ }
+
+ // Add field validation check for duplicate idnumber.
+ if (!empty($data['idnumber'])) {
+ if ($course = $DB->get_record('course', array('idnumber' => $data['idnumber']), '*', IGNORE_MULTIPLE)) {
+ if (empty($data['id']) || $course->id != $data['id']) {
+ $errors['idnumber']= get_string('courseidnumbertaken', 'error', $course->fullname);
}
- $foundcoursenamestring = implode(',', $foundcoursenames);
- $errors['shortname']= get_string('shortnametaken', '', $foundcoursenamestring);
}
}
@@ -717,20 +717,14 @@ public static function update_courses($courses) {
require_capability('moodle/course:changefullname', $context);
}
- // Check if the shortname already exist and user have capability.
+ // Check if the user can change shortname.
if (array_key_exists('shortname', $course) && ($oldcourse->shortname != $course['shortname'])) {
require_capability('moodle/course:changeshortname', $context);
- if ($DB->record_exists('course', array('shortname' => $course['shortname']))) {
- throw new moodle_exception('shortnametaken', '', '', $course['shortname']);
- }
}
- // Check if the id number already exist and user have capability.
+ // Check if the user can change the idnumber.
if (array_key_exists('idnumber', $course) && ($oldcourse->idnumber != $course['idnumber'])) {
require_capability('moodle/course:changeidnumber', $context);
- if ($DB->record_exists('course', array('idnumber' => $course['idnumber']))) {
- throw new moodle_exception('courseidnumbertaken', '', '', $course['idnumber']);
- }
}
// Check if user can change summary.
View
@@ -2368,6 +2368,21 @@ function update_course($data, $editoroptions = NULL) {
$data = file_postupdate_standard_filemanager($data, 'overviewfiles', $overviewfilesoptions, $context, 'course', 'overviewfiles', 0);
}
+ // Check we don't have a duplicate shortname.
+ if (!empty($data->shortname) && $oldcourse->shortname != $data->shortname) {
+ if ($DB->record_exists('course', array('shortname' => $data->shortname))) {
+ throw new moodle_exception('shortnametaken', '', '', $data->shortname);
+ }
+ }
+
+ // Check we don't have a duplicate idnumber.
+ if (!empty($data->idnumber) && $oldcourse->idnumber != $data->idnumber) {
+ if ($DB->record_exists('course', array('idnumber' => $data->idnumber))) {
+ throw new moodle_exception('courseidnumbertaken', '', '', $data->idnumber);
+ }
+ }
+
+
if (!isset($data->category) or empty($data->category)) {
// prevent nulls and 0 in category field
unset($data->category);
@@ -656,6 +656,54 @@ public function test_create_course_sections() {
$this->assertEquals(range(0, $course->numsections + 1), $sectionscreated);
}
+ public function test_update_course() {
+ global $DB;
+
+ $this->resetAfterTest();
+ $defaultcategory = $DB->get_field_select('course_categories', "MIN(id)", "parent=0");
+
+ $course = new stdClass();
+ $course->fullname = 'Apu loves Unit Təsts';
+ $course->shortname = 'test1';
+ $course->idnumber = '1';
+ $course->summary = 'Awesome!';
+ $course->summaryformat = FORMAT_PLAIN;
+ $course->format = 'topics';
+ $course->newsitems = 0;
+ $course->numsections = 5;
+ $course->category = $defaultcategory;
+ $original = (array) $course;
+
+ $created = create_course($course);
+ // Ensure the checks only work on idnumber/shortname that are not already ours.
+ $created = update_course($created);
+
+ $course->shortname = 'test2';
+ $course->idnumber = '2';
+
+ $created2 = create_course($course);
+
+ // Test duplicate idnumber.
+ $created2->idnumber = '1';
+ try {
+ update_course($created2);
+ $this->fail('Expected exception when trying to update a course with duplicate idnumber');
+ } catch (moodle_exception $e) {
+ $this->assertEquals(get_string('courseidnumbertaken', 'error', $created2->idnumber), $e->getMessage());
+ }
+
+ // Test duplicate shortname.
+ $created2->idnumber = '2';
+ $created2->shortname = 'test1';
+
+ try {
+ update_course($created2);
+ $this->fail('Expected exception when trying to update a course with a duplicate shortname');
+ } catch (moodle_exception $e) {
+ $this->assertEquals(get_string('shortnametaken', 'error', $created2->shortname), $e->getMessage());
+ }
+ }
+
public function test_course_add_cm_to_section() {
global $DB;
$this->resetAfterTest(true);

0 comments on commit 5536a56

Please sign in to comment.