diff --git a/course/externallib.php b/course/externallib.php index 71c24a0d595..75b7a60c3d1 100644 --- a/course/externallib.php +++ b/course/externallib.php @@ -838,27 +838,50 @@ class core_course_external extends external_api { // Parameter validation. $params = self::validate_parameters(self::delete_courses_parameters(), array('courseids'=>$courseids)); - $transaction = $DB->start_delegated_transaction(); + $warnings = array(); foreach ($params['courseids'] as $courseid) { - $course = $DB->get_record('course', array('id'=>$courseid), '*', MUST_EXIST); + $course = $DB->get_record('course', array('id' => $courseid)); + + if ($course === false) { + $warnings[] = array( + 'item' => 'course', + 'itemid' => $courseid, + 'warningcode' => 'unknowncourseidnumber', + 'message' => 'Unknown course ID ' . $courseid + ); + continue; + } // Check if the context is valid. $coursecontext = context_course::instance($course->id); self::validate_context($coursecontext); - // Check if the current user has enought permissions. + // Check if the current user has permission. if (!can_delete_course($courseid)) { - throw new moodle_exception('cannotdeletecategorycourse', 'error', - '', format_string($course->fullname)." (id: $courseid)"); + $warnings[] = array( + 'item' => 'course', + 'itemid' => $courseid, + 'warningcode' => 'cannotdeletecourse', + 'message' => 'You do not have the permission to delete this course' . $courseid + ); + continue; } - delete_course($course, false); + if (delete_course($course, false) === false) { + $warnings[] = array( + 'item' => 'course', + 'itemid' => $courseid, + 'warningcode' => 'cannotdeletecategorycourse', + 'message' => 'Course ' . $courseid . ' failed to be deleted' + ); + continue; + } } - $transaction->allow_commit(); + fix_course_sortorder(); - return null; + return array('warnings' => $warnings); } /** @@ -868,7 +891,11 @@ class core_course_external extends external_api { * @since Moodle 2.2 */ public static function delete_courses_returns() { - return null; + return new external_single_structure( + array( + 'warnings' => new external_warnings() + ) + ); } /** diff --git a/course/tests/externallib_test.php b/course/tests/externallib_test.php index a4e423450ba..2cbfcd11933 100644 --- a/course/tests/externallib_test.php +++ b/course/tests/externallib_test.php @@ -473,17 +473,46 @@ class core_course_externallib_testcase extends externallib_advanced_testcase { $course3 = self::getDataGenerator()->create_course(); // Delete courses. - core_course_external::delete_courses(array($course1->id, $course2->id)); + $result = core_course_external::delete_courses(array($course1->id, $course2->id)); + $result = external_api::clean_returnvalue(core_course_external::delete_courses_returns(), $result); + // Check for 0 warnings. + $this->assertEquals(0, count($result['warnings'])); // Check $course 1 and 2 are deleted. $notdeletedcount = $DB->count_records_select('course', 'id IN ( ' . $course1->id . ',' . $course2->id . ')'); $this->assertEquals(0, $notdeletedcount); + // Try to delete non-existent course. + $result = core_course_external::delete_courses(array($course1->id)); + $result = external_api::clean_returnvalue(core_course_external::delete_courses_returns(), $result); + // Check for 1 warnings. + $this->assertEquals(1, count($result['warnings'])); + + // Try to delete Frontpage course. + $result = core_course_external::delete_courses(array(0)); + $result = external_api::clean_returnvalue(core_course_external::delete_courses_returns(), $result); + // Check for 1 warnings. + $this->assertEquals(1, count($result['warnings'])); + + // Fail when the user has access to course (enrolled) but does not have permission or is not admin. + $student1 = self::getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $this->getDataGenerator()->enrol_user($student1->id, + $course3->id, + $studentrole->id); + $this->setUser($student1); + $result = core_course_external::delete_courses(array($course3->id)); + $result = external_api::clean_returnvalue(core_course_external::delete_courses_returns(), $result); + // Check for 1 warnings. + $this->assertEquals(1, count($result['warnings'])); + // Fail when the user is not allow to access the course (enrolled) or is not admin. $this->setGuestUser(); $this->setExpectedException('require_login_exception'); - $createdsubcats = core_course_external::delete_courses(array($course3->id)); + + $result = core_course_external::delete_courses(array($course3->id)); + $result = external_api::clean_returnvalue(core_course_external::delete_courses_returns(), $result); } /** diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 6971492668f..395e1f08393 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -3,6 +3,7 @@ information provided here is intended especially for developers. === 2.9 === +* Course deletions now return warning messages on any failures and do not try to rollback the entire deletion. * \core\event\course_viewed 'other' argument renamed from coursesectionid to coursesectionnumber as it contains the section number. * New API core_filetypes::add_type (etc.) allows custom filetypes to be added and modified. * PHPUnit: PHPMailer Sink is now started for all tests and is setup within the phpunit wrapper for advanced tests.