Merge branch 'MDL-44071-master' of git://github.com/zbdd/moodle

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',
+                $warnings[] = array(
-                    '', format_string($course->fullname)." (id: $courseid)");
+                                '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()
+            )
+        );
     }
 
     /**

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);
     }
 
     /**

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.