Revert "Merge branch 'MDL-43127-master' of git://github.com/FMCorz/moodle"
authorDan Poltawski <dan@moodle.com>
Thu, 27 Nov 2014 09:33:52 +0000 (09:33 +0000)
committerDan Poltawski <dan@moodle.com>
Thu, 27 Nov 2014 09:33:52 +0000 (09:33 +0000)
This reverts commit 4e75dd78e6c93f7c341bca703d292606436610d5, reversing
changes made to f376c2ffd7d8304bafff851ead3ed10dee34c805.

admin/tool/uploadcourse/classes/course.php
admin/tool/uploadcourse/classes/helper.php
admin/tool/uploadcourse/classes/processor.php
admin/tool/uploadcourse/classes/tracker.php
admin/tool/uploadcourse/lang/en/tool_uploadcourse.php
admin/tool/uploadcourse/tests/course_test.php
admin/tool/uploadcourse/tests/helper_test.php

index f082eab..ff22833 100644 (file)
@@ -108,9 +108,6 @@ class tool_uploadcourse_course {
     static protected $importoptionsdefaults = array('canrename' => false, 'candelete' => false, 'canreset' => false,
         'reset' => false, 'restoredir' => null, 'shortnametemplate' => null);
 
-    /** @var array special fields used when processing the enrolment methods. */
-    static protected $enrolmentspecialfields = array('delete', 'disable', 'startdate', 'enddate', 'enrolperiod', 'role');
-
     /**
      * Constructor
      *
@@ -658,18 +655,9 @@ class tool_uploadcourse_course {
             return false;
         }
 
-        // Getting the enrolment data.
-        $errors = array();
-        $this->enrolmentdata = tool_uploadcourse_helper::get_enrolment_data($this->rawdata, $errors);
-        if (!empty($errors)) {
-            foreach ($errors as $key => $message) {
-                $this->error($key, $message);
-            }
-            return false;
-        }
-
         // Saving data.
         $this->data = $coursedata;
+        $this->enrolmentdata = tool_uploadcourse_helper::get_enrolment_data($this->rawdata);
 
         // Restore data.
         // TODO Speed up things by not really extracting the backup just yet, but checking that
@@ -781,20 +769,10 @@ class tool_uploadcourse_course {
             return;
         }
 
-        $cannotaddmethods = array();
         $enrolmentplugins = tool_uploadcourse_helper::get_enrolment_plugins();
         $instances = enrol_get_instances($course->id, false);
         foreach ($enrolmentdata as $enrolmethod => $method) {
 
-            $plugin = $enrolmentplugins[$enrolmethod];
-
-            // TODO MDL-48362 Abstract the logic to prevent it to be tied to the
-            // user interface. Ideally a plugin should have a method that returns
-            // whether or not a new instance can be added to the course rather than
-            // using enrol_plugin::get_newinstance_link() to figure that out.
-            $canadd = $plugin->get_newinstance_link($course->id);
-
-            // TODO MDL-43820 Handle multiple instances of the same type.
             $instance = null;
             foreach ($instances as $i) {
                 if ($i->enrol == $enrolmethod) {
@@ -828,57 +806,30 @@ class tool_uploadcourse_course {
                     }
                 }
             } else {
+                $plugin = null;
                 if (empty($instance)) {
-
-                    // Check if we can create a new instance of this enrolment method.
-                    if (!$canadd) {
-                        $cannotaddmethods[] = $enrolmethod;
-                        continue;
-                    }
-
-                    // Some plugins do not implement enrol_plugin::add_default_instance(),
-                    // but we will try anyway and call enrol_plugin::add_instance() if needed.
-                    $id = $plugin->add_default_instance($course);
-                    if (empty($id)) {
-                        $id = $plugin->add_instance($course);
-                    }
-
+                    $plugin = $enrolmentplugins[$enrolmethod];
                     $instance = new stdClass();
-                    $instance->id = $id;
+                    $instance->id = $plugin->add_default_instance($course);
                     $instance->roleid = $plugin->get_config('roleid');
                     $instance->status = ENROL_INSTANCE_ENABLED;
                 } else {
+                    $plugin = $enrolmentplugins[$instance->enrol];
                     $plugin->update_status($instance, ENROL_INSTANCE_ENABLED);
                 }
 
                 // Now update values.
                 foreach ($method as $k => $v) {
-                    if (in_array($k, self::$enrolmentspecialfields)) {
-                        // Skip the special import keys.
-                        continue;
-                    }
                     $instance->{$k} = $v;
                 }
 
                 // Sort out the start, end and date.
-                if (isset($method['startdate'])) {
-                    $instance->enrolstartdate = strtotime($method['startdate']);
-                } else if (!isset($instance->enrolstartdate)) {
-                    $instance->enrolstartdate = 0;
-                }
-                if (isset($method['enddate'])) {
-                    $instance->enrolenddate = strtotime($method['enddate']);
-                } else if (!isset($instance->enrolenddate)) {
-                    $instance->enrolenddate = 0;
-                }
+                $instance->enrolstartdate = (isset($method['startdate']) ? strtotime($method['startdate']) : 0);
+                $instance->enrolenddate = (isset($method['enddate']) ? strtotime($method['enddate']) : 0);
 
                 // Is the enrolment period set?
-                if (isset($method['enrolperiod'])) {
-                    if (empty($method['enrolperiod'])) {
-                        // Let's just make sure that it's 0.
-                        $method['enrolperiod'] = 0;
-                    } else if (preg_match('/^\d+$/', $method['enrolperiod'])) {
-                        // Cast integers to integers.
+                if (isset($method['enrolperiod']) && ! empty($method['enrolperiod'])) {
+                    if (preg_match('/^\d+$/', $method['enrolperiod'])) {
                         $method['enrolperiod'] = (int) $method['enrolperiod'];
                     } else {
                         // Try and convert period to seconds.
@@ -908,11 +859,6 @@ class tool_uploadcourse_course {
                 $DB->update_record('enrol', $instance);
             }
         }
-
-        if (!empty($cannotaddmethods)) {
-            $this->error('cannotaddenrolmentmethods', new lang_string('cannotaddenrolmentmethods', 'tool_uploadcourse',
-                implode(', ', $cannotaddmethods)));
-        }
     }
 
     /**
index 0a3e12e..2873f3b 100644 (file)
@@ -134,10 +134,9 @@ class tool_uploadcourse_helper {
      * )
      *
      * @param array $data data to extract the enrolment data from.
-     * @param array $errors will be populated with errors found.
      * @return array
      */
-    public static function get_enrolment_data($data, &$errors = array()) {
+    public static function get_enrolment_data($data) {
         $enrolmethods = array();
         $enroloptions = array();
         foreach ($data as $field => $value) {
@@ -159,26 +158,16 @@ class tool_uploadcourse_helper {
 
         // Combining enrolment methods and their options in a single array.
         $enrolmentdata = array();
-        $unknownmethods = array();
-        $methodsnotsupported = array();
         if (!empty($enrolmethods)) {
             $enrolmentplugins = self::get_enrolment_plugins();
             foreach ($enrolmethods as $key => $method) {
                 if (!array_key_exists($method, $enrolmentplugins)) {
-                    // Unknown enrolment method.
-                    $unknownmethods[] = $method;
+                    // Error!
                     continue;
                 }
                 $enrolmentdata[$enrolmethods[$key]] = $enroloptions[$key];
             }
         }
-
-        // Logging errors related to enrolment methods.
-        if (!empty($unknownmethods)) {
-            $errors['unknownenrolmentmethods'] = new lang_string('unknownenrolmentmethods', 'tool_uploadcourse',
-                implode(', ', $unknownmethods));
-        }
-
         return $enrolmentdata;
     }
 
index 2743846..8963f7e 100644 (file)
@@ -207,7 +207,6 @@ class tool_uploadcourse_processor {
             $course = $this->get_course($data);
             if ($course->prepare()) {
                 $course->proceed();
-                $outcome = 1;
 
                 $status = $course->get_statuses();
                 if (array_key_exists('coursecreated', $status)) {
@@ -218,19 +217,11 @@ class tool_uploadcourse_processor {
                     $deleted++;
                 }
 
-                // Errors can occur even though the course preparation returned true, often because
-                // some checks could not be done in course::prepare() because it requires the course to exist.
-                if ($course->has_errors()) {
-                    $status += $course->get_errors();
-                    $errors++;
-                    $outcome = 2;
-                }
-
                 $data = array_merge($data, $course->get_data(), array('id' => $course->get_id()));
-                $tracker->output($this->linenb, $outcome, $status, $data);
+                $tracker->output($this->linenb, true, $status, $data);
             } else {
                 $errors++;
-                $tracker->output($this->linenb, 0, $course->get_errors(), $data);
+                $tracker->output($this->linenb, false, $course->get_errors(), $data);
             }
         }
 
index 3ac78b4..0daa845 100644 (file)
@@ -136,8 +136,7 @@ class tool_uploadcourse_tracker {
      * Output one more line.
      *
      * @param int $line line number.
-     * @param int $outcome 0 for failure, 1 for success, 2 for success with errors. Use 2 when
-     *                     most of the process succeeded but there might have been outstanding errors.
+     * @param bool $outcome success or not?
      * @param array $status array of statuses.
      * @param array $data extra data to display.
      * @return void
@@ -149,16 +148,9 @@ class tool_uploadcourse_tracker {
         }
 
         if ($this->outputmode == self::OUTPUT_PLAIN) {
-            if ($outcome == 1) {
-                $ok = 'OK';
-            } else if (!$outcome) {
-                $ok = 'NOK';        // Not OK.
-            } else {
-                $ok = 'EOK';        // Errors, but OK.
-            }
             $message = array(
                 $line,
-                $ok,
+                $outcome ? 'OK' : 'NOK',
                 isset($data['id']) ? $data['id'] : '',
                 isset($data['shortname']) ? $data['shortname'] : '',
                 isset($data['fullname']) ? $data['fullname'] : '',
@@ -176,12 +168,10 @@ class tool_uploadcourse_tracker {
             if (is_array($status)) {
                 $status = implode(html_writer::empty_tag('br'), $status);
             }
-            if ($outcome == 1) {
+            if ($outcome) {
                 $outcome = $OUTPUT->pix_icon('i/valid', '');
-            } else if (!$outcome) {
-                $outcome = $OUTPUT->pix_icon('i/invalid', '');
             } else {
-                $outcome = $OUTPUT->pix_icon('i/caution', '');
+                $outcome = $OUTPUT->pix_icon('i/invalid', '');
             }
             echo html_writer::start_tag('tr', array('class' => 'r' . $this->rownb % 2));
             echo html_writer::tag('td', $line, array('class' => 'c' . $ci++));
index 64e96c5..96b847c 100644 (file)
@@ -29,7 +29,6 @@ $string['allowrenames_help'] = 'Whether the rename field is accepted or not.';
 $string['allowresets'] = 'Allow resets';
 $string['allowresets_help'] = 'Whether the reset field is accepted or not.';
 $string['cachedef_helper'] = 'Helper caching';
-$string['cannotaddenrolmentmethods'] = 'Cannot add the enrolment methods: {$a}';
 $string['cannotdeletecoursenotexist'] = 'Cannot delete a course that does not exist';
 $string['cannotgenerateshortnameupdatemode'] = 'Cannot generate a shortname when updates are allowed';
 $string['cannotreadbackupfile'] = 'Cannot read the backup file';
@@ -110,7 +109,6 @@ $string['shortnametemplate'] = 'Template to generate a shortname';
 $string['shortnametemplate_help'] = 'The short name of the course is displayed in the navigation. You may use template syntax here (%f = fullname, %i = idnumber), or enter an initial value that is incremented.';
 $string['templatefile'] = 'Restore from this file after upload';
 $string['templatefile_help'] = 'Select a file to use as a template for the creation of all courses.';
-$string['unknownenrolmentmethods'] = 'Unknown enrolment methods: {$a}';
 $string['unknownimportmode'] = 'Unknown import mode';
 $string['updatemissing'] = 'Fill in missing items from CSV data and defaults';
 $string['updatemode'] = 'Update mode';
index 9d12b80..a173e72 100644 (file)
@@ -900,11 +900,8 @@ class tool_uploadcourse_course_testcase extends advanced_testcase {
     }
 
     public function test_enrolment_data() {
-        global $DB;
         $this->resetAfterTest(true);
-        $this->setAdminUser();
 
-        // Adding an enrolment method to a new course.
         $mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
         $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
         $data = array('shortname' => 'c1', 'summary' => 'S', 'fullname' => 'FN', 'category' => '1');
@@ -924,135 +921,11 @@ class tool_uploadcourse_course_testcase extends advanced_testcase {
             $enroldata[$instance->enrol] = $instance;
         }
 
-        $this->assertFalse($co->has_errors());
         $this->assertNotEmpty($enroldata['manual']);
         $this->assertEquals(ENROL_INSTANCE_ENABLED, $enroldata['manual']->status);
         $this->assertEquals(strtotime($data['enrolment_1_startdate']), $enroldata['manual']->enrolstartdate);
         $this->assertEquals(strtotime('1970-01-01 GMT + ' . $data['enrolment_1_enrolperiod']), $enroldata['manual']->enrolperiod);
         $this->assertEquals(strtotime('12th July 2013'), $enroldata['manual']->enrolenddate);
-
-        // Updating a course's enrolment method.
-        $c2 = $this->getDataGenerator()->create_course(array('shortname' => 'c2'));
-        $enroldata = array();
-        $instances = enrol_get_instances($c2->id, false);
-        foreach ($instances as $instance) {
-            $enroldata[$instance->enrol] = $instance;
-        }
-        $manual = $enroldata['manual'];
-        $manual->enrolstartdate = strtotime('1st January 2000');
-        $manual->enrolenddate = strtotime('2nd January 2001');
-        $manual->status = ENROL_INSTANCE_DISABLED;
-        $DB->update_record('enrol', $manual);
-
-        $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY;
-        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
-        $data = array('shortname' => 'c2');
-        $data['enrolment_1'] = 'manual';
-        $data['enrolment_1_role'] = 'teacher';
-        $data['enrolment_1_enddate'] = '2nd August 2013';
-        $data['enrolment_1_enrolperiod'] = '10 days';
-        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
-        $this->assertTrue($co->prepare());
-        $co->proceed();
-
-        // Get the enrolment methods data.
-        $enroldata = array();
-        $instances = enrol_get_instances($co->get_id(), false);
-        foreach ($instances as $instance) {
-            $enroldata[$instance->enrol] = $instance;
-        }
-
-        $this->assertFalse($co->has_errors());
-        $this->assertNotEmpty($enroldata['manual']);
-        $this->assertEquals(ENROL_INSTANCE_ENABLED, $enroldata['manual']->status);
-        $this->assertEquals($manual->enrolstartdate, $enroldata['manual']->enrolstartdate);
-        $this->assertEquals(strtotime('1970-01-01 GMT + ' . $data['enrolment_1_enrolperiod']),
-            $enroldata['manual']->enrolperiod);
-        $this->assertEquals(strtotime('11th January 2000'), $enroldata['manual']->enrolenddate);
-
-        // Disabling an enrolment method.
-        $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY;
-        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
-        $data = array('shortname' => 'c2');
-        $data['enrolment_1'] = 'manual';
-        $data['enrolment_1_disable'] = '1';
-        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
-        $this->assertTrue($co->prepare());
-        $co->proceed();
-
-        $enroldata = array();
-        $instances = enrol_get_instances($co->get_id(), false);
-        foreach ($instances as $instance) {
-            $enroldata[$instance->enrol] = $instance;
-        }
-        $this->assertFalse($co->has_errors());
-        $this->assertNotEmpty($enroldata['manual']);
-        $this->assertEquals(ENROL_INSTANCE_DISABLED, $enroldata['manual']->status);
-
-        // Deleting an enrolment method.
-        $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY;
-        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
-        $data = array('shortname' => 'c2');
-        $data['enrolment_1'] = 'manual';
-        $data['enrolment_1_delete'] = '1';
-        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
-        $this->assertTrue($co->prepare());
-        $co->proceed();
-
-        $enroldata = array();
-        $instances = enrol_get_instances($co->get_id(), false);
-        foreach ($instances as $instance) {
-            $enroldata[$instance->enrol] = $instance;
-        }
-        $this->assertFalse($co->has_errors());
-        $this->assertArrayNotHasKey('manual', $enroldata);
-
-        // Trying to add a non-existing enrolment method.
-        $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY;
-        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
-        $data = array('shortname' => 'c2');
-        $data['enrolment_1'] = 'notexisting';
-        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
-        $this->assertFalse($co->prepare());
-        $this->assertArrayHasKey('unknownenrolmentmethods', $co->get_errors());
-
-        // Trying to add a non-compatible enrolment method.
-        $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY;
-        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
-        $data = array('shortname' => 'c2');
-        $data['enrolment_1'] = 'database';
-        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
-        $this->assertTrue($co->prepare());
-        $co->proceed();
-
-        $enroldata = array();
-        $instances = enrol_get_instances($co->get_id(), false);
-        foreach ($instances as $instance) {
-            $enroldata[$instance->enrol] = $instance;
-        }
-        $this->assertTrue($co->has_errors());
-        $this->assertArrayNotHasKey('database', $enroldata);
-        $this->assertArrayHasKey('cannotaddenrolmentmethods', $co->get_errors());
-
-        // Testing cohort enrolment method.
-        $cohort = $this->getDataGenerator()->create_cohort();
-        $mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY;
-        $updatemode = tool_uploadcourse_processor::UPDATE_ALL_WITH_DATA_ONLY;
-        $data = array('shortname' => 'c2');
-        $data['enrolment_1'] = 'cohort';
-        $data['enrolment_1_customint1'] = $cohort->id;
-        $co = new tool_uploadcourse_course($mode, $updatemode, $data);
-        $this->assertTrue($co->prepare());
-        $co->proceed();
-
-        $enroldata = array();
-        $instances = enrol_get_instances($co->get_id(), false);
-        foreach ($instances as $instance) {
-            $enroldata[$instance->enrol] = $instance;
-        }
-        $this->assertFalse($co->has_errors());
-        $this->assertArrayHasKey('cohort', $enroldata);
-        $this->assertEquals($cohort->id, $enroldata['cohort']->customint1);
     }
 
     public function test_idnumber_problems() {
index 22569e3..41a838c 100644 (file)
@@ -101,9 +101,7 @@ class tool_uploadcourse_helper_testcase extends advanced_testcase {
                 'test1' => 'test1',
             )
         );
-        $errors = array();
-        $this->assertSame(tool_uploadcourse_helper::get_enrolment_data($data, $errors), $expected);
-        $this->assertArrayHasKey('unknownenrolmentmethods', $errors);
+        $this->assertSame(tool_uploadcourse_helper::get_enrolment_data($data), $expected);
     }
 
     public function test_get_enrolment_plugins() {