MDL-66489 enrol: Send all form data to WS and do all busines logic there

It is discouraged to use JS Date functions. We should pass all raw data to PHP.

enrol/externallib.php

@@ -919,6 +919,7 @@ class core_enrol_external extends external_api {
     /**
      * Returns description of edit_user_enrolment() parameters
      *
+     * @deprecated since 3.8
      * @return external_function_parameters
      */
     public static function edit_user_enrolment_parameters() {
@@ -936,6 +937,7 @@ class core_enrol_external extends external_api {
     /**
      * External function that updates a given user enrolment.
      *
+     * @deprecated since 3.8
      * @param int $courseid The course ID.
      * @param int $ueid The user enrolment ID.
      * @param int $status The enrolment status.
@@ -1002,6 +1004,7 @@ class core_enrol_external extends external_api {
     /**
      * Returns description of edit_user_enrolment() result value
      *
+     * @deprecated since 3.8
      * @return external_description
      */
     public static function edit_user_enrolment_returns() {
@@ -1020,6 +1023,82 @@ class core_enrol_external extends external_api {
         );
     }
 
+    /**
+     * Mark the edit_user_enrolment web service as deprecated.
+     *
+     * @return  bool
+     */
+    public static function edit_user_enrolment_is_deprecated() {
+        return true;
+    }
+
+    /**
+     * Returns description of submit_user_enrolment_form parameters.
+     *
+     * @return external_function_parameters.
+     */
+    public static function submit_user_enrolment_form_parameters() {
+        return new external_function_parameters([
+            'formdata' => new external_value(PARAM_RAW, 'The data from the event form'),
+        ]);
+    }
+
+    /**
+     * External function that handles the user enrolment form submission.
+     *
+     * @param string $formdata The user enrolment form data in s URI encoded param string
+     * @return array An array consisting of the processing result and error flag, if available
+     */
+    public static function submit_user_enrolment_form($formdata) {
+        global $CFG, $DB, $PAGE;
+
+        // Parameter validation.
+        $params = self::validate_parameters(self::submit_user_enrolment_form_parameters(), ['formdata' => $formdata]);
+
+        $data = [];
+        parse_str($params['formdata'], $data);
+
+        $userenrolment = $DB->get_record('user_enrolments', ['id' => $data['ue']], '*', MUST_EXIST);
+        $instance = $DB->get_record('enrol', ['id' => $userenrolment->enrolid], '*', MUST_EXIST);
+        $plugin = enrol_get_plugin($instance->enrol);
+        $course = get_course($instance->courseid);
+        $context = context_course::instance($course->id);
+        self::validate_context($context);
+
+        require_once("$CFG->dirroot/enrol/editenrolment_form.php");
+        $customformdata = [
+            'ue' => $userenrolment,
+            'modal' => true,
+            'enrolinstancename' => $plugin->get_instance_name($instance)
+        ];
+        $mform = new enrol_user_enrolment_form(null, $customformdata, 'post', '', null, true, $data);
+
+        if ($validateddata = $mform->get_data()) {
+            if (!empty($validateddata->duration) && $validateddata->timeend == 0) {
+                $validateddata->timeend = $validateddata->timestart + $validateddata->duration;
+            }
+            require_once($CFG->dirroot . '/enrol/locallib.php');
+            $manager = new course_enrolment_manager($PAGE, $course);
+            $result = $manager->edit_enrolment($userenrolment, $validateddata);
+
+            return ['result' => $result];
+        } else {
+            return ['result' => false, 'validationerror' => true];
+        }
+    }
+
+    /**
+     * Returns description of submit_user_enrolment_form() result value
+     *
+     * @return external_description
+     */
+    public static function submit_user_enrolment_form_returns() {
+        return new external_single_structure([
+            'result' => new external_value(PARAM_BOOL, 'True if the user\'s enrolment was successfully updated'),
+            'validationerror' => new external_value(PARAM_BOOL, 'Indicates invalid form data', VALUE_DEFAULT, false),
+        ]);
+    }
+
     /**
      * Returns description of unenrol_user_enrolment() parameters
      *

enrol/tests/externallib_test.php

@@ -958,6 +958,155 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase {
         $this->assertEquals(ENROL_USER_SUSPENDED, $ue->status);
     }
 
+    /**
+     * dataProvider for test_submit_user_enrolment_form().
+     */
+    public function submit_user_enrolment_form_provider() {
+        $now = new DateTime();
+
+        $nextmonth = clone($now);
+        $nextmonth->add(new DateInterval('P1M'));
+
+        return [
+            'Invalid data' => [
+                'customdata' => [
+                    'status' => ENROL_USER_ACTIVE,
+                    'timestart' => [
+                        'day' => $now->format('j'),
+                        'month' => $now->format('n'),
+                        'year' => $now->format('Y'),
+                        'hour' => $now->format('G'),
+                        'minute' => 0,
+                        'enabled' => 1,
+                    ],
+                    'timeend' => [
+                        'day' => $now->format('j'),
+                        'month' => $now->format('n'),
+                        'year' => $now->format('Y'),
+                        'hour' => $now->format('G'),
+                        'minute' => 0,
+                        'enabled' => 1,
+                    ],
+                ],
+                'expectedresult' => false,
+                'validationerror' => true,
+            ],
+            'Valid data' => [
+                'customdata' => [
+                    'status' => ENROL_USER_ACTIVE,
+                    'timestart' => [
+                        'day' => $now->format('j'),
+                        'month' => $now->format('n'),
+                        'year' => $now->format('Y'),
+                        'hour' => $now->format('G'),
+                        'minute' => 0,
+                        'enabled' => 1,
+                    ],
+                    'timeend' => [
+                        'day' => $nextmonth->format('j'),
+                        'month' => $nextmonth->format('n'),
+                        'year' => $nextmonth->format('Y'),
+                        'hour' => $nextmonth->format('G'),
+                        'minute' => 0,
+                        'enabled' => 1,
+                    ],
+                ],
+                'expectedresult' => true,
+                'validationerror' => false
+            ],
+            'Suspend user' => [
+                'customdata' => [
+                    'status' => ENROL_USER_SUSPENDED,
+                ],
+                'expectedresult' => true,
+                'validationerror' => false
+            ],
+        ];
+    }
+
+    /**
+     * @param array $customdata The data we are providing to the webservice.
+     * @param bool $expectedresult The result we are expecting to receive from the webservice.
+     * @param bool $validationerror The validationerror we are expecting to receive from the webservice.
+     * @dataProvider submit_user_enrolment_form_provider
+     */
+    public function test_submit_user_enrolment_form($customdata, $expectedresult, $validationerror) {
+        global $CFG, $DB;
+
+        $this->resetAfterTest(true);
+        $datagen = $this->getDataGenerator();
+
+        /** @var enrol_manual_plugin $manualplugin */
+        $manualplugin = enrol_get_plugin('manual');
+
+        $studentroleid = $DB->get_field('role', 'id', ['shortname' => 'student'], MUST_EXIST);
+        $teacherroleid = $DB->get_field('role', 'id', ['shortname' => 'editingteacher'], MUST_EXIST);
+        $course = $datagen->create_course();
+        $user = $datagen->create_user();
+        $teacher = $datagen->create_user();
+
+        $instanceid = null;
+        $instances = enrol_get_instances($course->id, true);
+        foreach ($instances as $inst) {
+            if ($inst->enrol == 'manual') {
+                $instanceid = (int)$inst->id;
+                break;
+            }
+        }
+        if (empty($instanceid)) {
+            $instanceid = $manualplugin->add_default_instance($course);
+            if (empty($instanceid)) {
+                $instanceid = $manualplugin->add_instance($course);
+            }
+        }
+        $this->assertNotNull($instanceid);
+
+        $instance = $DB->get_record('enrol', ['id' => $instanceid], '*', MUST_EXIST);
+        $manualplugin->enrol_user($instance, $user->id, $studentroleid, 0, 0, ENROL_USER_ACTIVE);
+        $manualplugin->enrol_user($instance, $teacher->id, $teacherroleid, 0, 0, ENROL_USER_ACTIVE);
+        $ueid = (int) $DB->get_field(
+                'user_enrolments',
+                'id',
+                ['enrolid' => $instance->id, 'userid' => $user->id],
+                MUST_EXIST
+        );
+
+        // Login as teacher.
+        $teacher->ignoresesskey = true;
+        $this->setUser($teacher);
+
+        $formdata = [
+            'ue'        => $ueid,
+            'ifilter'   => 0,
+            'status'    => null,
+            'timestart' => null,
+            'duration'  => null,
+            'timeend'   => null,
+        ];
+
+        $formdata = array_merge($formdata, $customdata);
+
+        require_once("$CFG->dirroot/enrol/editenrolment_form.php");
+        $formdata = enrol_user_enrolment_form::mock_generate_submit_keys($formdata);
+
+        $querystring = http_build_query($formdata, '', '&');
+
+        $result = external_api::clean_returnvalue(
+                core_enrol_external::submit_user_enrolment_form_returns(),
+                core_enrol_external::submit_user_enrolment_form($querystring)
+        );
+
+        $this->assertEquals(
+                ['result' => $expectedresult, 'validationerror' => $validationerror],
+                $result,
+                '', 0.0, 10, true);
+
+        if ($result['result']) {
+            $ue = $DB->get_record('user_enrolments', ['id' => $ueid], '*', MUST_EXIST);
+            $this->assertEquals($formdata['status'], $ue->status);
+        }
+    }
+
     /**
      * Test for core_enrol_external::unenrol_user_enrolment().
      */

lib/db/services.php

@@ -647,7 +647,16 @@ $functions = array(
         'classname' => 'core_enrol_external',
         'methodname' => 'edit_user_enrolment',
         'classpath' => 'enrol/externallib.php',
-        'description' => 'External function that updates a given user enrolment',
+        'description' => '** DEPRECATED ** Please do not call this function any more.
+                          External function that updates a given user enrolment',
+        'type' => 'write',
+        'ajax' => true,
+    ),
+    'core_enrol_submit_user_enrolment_form' => array(
+        'classname' => 'core_enrol_external',
+        'methodname' => 'submit_user_enrolment_form',
+        'classpath' => 'enrol/externallib.php',
+        'description' => 'Submit form data for enrolment form',
         'type' => 'write',
         'ajax' => true,
     ),

lib/upgrade.txt

@@ -53,6 +53,7 @@ validation against and defaults to null (so, no user needed) if not provided.
 * Following behat steps have been removed from core:
     - I go to "<gradepath_string>" in the course gradebook
 * A new admin setting widget 'core_admin\local\settings\filesize' is added.
+* The core_enrol_edit_user_enrolment webservice has been deprecated. Please use core_enrol_submit_user_enrolment_form instead.
 
 === 3.7 ===
 

user/amd/build/status_field.min.js

@@ -1,2 +1,2 @@
-define ("core_user/status_field",["core/templates","jquery","core/str","core/config","core/notification","core/modal_factory","core/modal_events","core/fragment","core/ajax"],function(a,b,c,d,f,g,h,i,j){var k={EDIT_ENROLMENT:"[data-action=\"editenrolment\"]",SHOW_DETAILS:"[data-action=\"showdetails\"]",UNENROL:"[data-action=\"unenrol\"]"},l=function(a){this.contextid=a.contextid;this.courseid=a.courseid;this.bindEditEnrol();this.bindUnenrol();this.bindStatusDetails()};l.prototype.courseid=0;l.prototype.bindEditEnrol=function(){var a=this;b(k.EDIT_ENROLMENT).click(function(d){d.preventDefault();var e=b(this),i=e.parent(),j=i.data("fullname"),k=e.attr("rel");b.when(c.get_string("edituserenrolment","enrol",j)).then(function(a){return g.create({large:!0,title:a,type:g.types.SAVE_CANCEL})}).done(function(b){b.getRoot().on(h.save,function(c){c.preventDefault();a.submitEditFormAjax(b)});b.getRoot().on(h.hidden,function(){b.destroy()});b.setBody(a.getBody(k));b.show()}).fail(f.exception)})};l.prototype.bindUnenrol=function(){var a=this;b(k.UNENROL).click(function(d){d.preventDefault();var e=b(this),i=e.parent(),j=[{key:"unenrol",component:"enrol"},{key:"unenrolconfirm",component:"enrol",param:{user:i.data("fullname"),course:i.data("coursename"),enrolinstancename:i.data("enrolinstancename")}}],k=g.create({type:g.types.SAVE_CANCEL});b.when(c.get_strings(j),k).done(function(c,f){var g=c[0],i=c[1];f.setTitle(g);f.setBody(i);f.setSaveButtonText(g);f.getRoot().on(h.save,function(){var c={ueid:b(e).attr("rel")};d.preventDefault();a.submitUnenrolFormAjax(f,c)});f.getRoot().on(h.hidden,function(){f.destroy()});f.show()}).fail(f.exception)})};l.prototype.bindStatusDetails=function(){b(k.SHOW_DETAILS).click(function(d){d.preventDefault();var e=b(this),i=e.parent(),j={fullname:i.data("fullname"),coursename:i.data("coursename"),enrolinstancename:i.data("enrolinstancename"),status:i.data("status"),statusclass:i.find("span").attr("class"),timestart:i.data("timestart"),timeend:i.data("timeend"),timeenrolled:i.data("timeenrolled")},l=e.next(k.EDIT_ENROLMENT);if(l.length){j.editenrollink=b("<div>").append(l.clone()).html()}var m=c.get_strings([{key:"enroldetails",component:"enrol"}]),n=g.create({large:!0,type:g.types.CANCEL});b.when(m,n).done(function(c,d){var e=a.render("core_user/status_details",j);d.setTitle(c[0]);d.setBody(e);if(l.length){d.getRoot().on("click",k.EDIT_ENROLMENT,function(a){a.preventDefault();d.hide();b(l).trigger("click")})}d.show();d.getRoot().on(h.hidden,function(){d.destroy()})}).fail(f.exception)})};l.prototype.submitEditFormAjax=function(a){var c=this,d=a.getRoot().find("form"),e=b(d).find("[name=\"ue\"]").val(),g=b(d).find("[name=\"status\"]").val(),h={courseid:this.courseid,ueid:e,status:g},i=b(d).find("[name=\"timestart[enabled]\"]");if(i.is(":checked")){var k=b(d).find("[name=\"timestart[year]\"]").val(),l=b(d).find("[name=\"timestart[month]\"]").val()-1,m=b(d).find("[name=\"timestart[day]\"]").val(),n=b(d).find("[name=\"timestart[hour]\"]").val(),o=b(d).find("[name=\"timestart[minute]\"]").val(),p=new Date(k,l,m,n,o);h.timestart=p.getTime()/1e3}var q=b(d).find("[name=\"timeend[enabled]\"]");if(q.is(":checked")){var r=b(d).find("[name=\"timeend[year]\"]").val(),s=b(d).find("[name=\"timeend[month]\"]").val()-1,t=b(d).find("[name=\"timeend[day]\"]").val(),u=b(d).find("[name=\"timeend[hour]\"]").val(),v=b(d).find("[name=\"timeend[minute]\"]").val(),w=new Date(r,s,t,u,v);h.timeend=w.getTime()/1e3}var x=b(d).find("[name=\"duration\"]");if(x.is(":enabled")){h.timeend=h.timestart+parseInt(x.val())}j.call([{methodname:"core_enrol_edit_user_enrolment",args:h}])[0].done(function(b){if(b.result){a.hide();if("undefined"!=typeof window.M.core_formchangechecker){window.M.core_formchangechecker.reset_form_dirty_state()}window.location.reload()}else{var f=JSON.stringify(d.serialize());a.setBody(c.getBody(e,f))}}).fail(f.exception)};l.prototype.submitUnenrolFormAjax=function(a,b){j.call([{methodname:"core_enrol_unenrol_user_enrolment",args:b}])[0].done(function(b){if(b.result){a.hide();if("undefined"!=typeof window.M.core_formchangechecker){window.M.core_formchangechecker.reset_form_dirty_state()}window.location.reload()}else{f.alert(b.errors[0].key,b.errors[0].message)}}).fail(f.exception)};l.prototype.getBody=function(a,b){var c={ueid:a};if("undefined"!=typeof b){c.formdata=b}return i.loadFragment("enrol","user_enrolment_form",this.contextid,c).fail(f.exception)};return{init:function init(a){new l(a)}}});
+define ("core_user/status_field",["core/templates","jquery","core/str","core/config","core/notification","core/modal_factory","core/modal_events","core/fragment","core/ajax"],function(a,b,c,d,f,g,h,i,j){var k={EDIT_ENROLMENT:"[data-action=\"editenrolment\"]",SHOW_DETAILS:"[data-action=\"showdetails\"]",UNENROL:"[data-action=\"unenrol\"]"},l=function(a){this.contextid=a.contextid;this.courseid=a.courseid;this.bindEditEnrol();this.bindUnenrol();this.bindStatusDetails()};l.prototype.courseid=0;l.prototype.bindEditEnrol=function(){var a=this;b(k.EDIT_ENROLMENT).click(function(d){d.preventDefault();var e=b(this),i=e.parent(),j=i.data("fullname"),k=e.attr("rel");b.when(c.get_string("edituserenrolment","enrol",j)).then(function(a){return g.create({large:!0,title:a,type:g.types.SAVE_CANCEL})}).done(function(b){b.getRoot().on(h.save,function(c){c.preventDefault();a.submitEditFormAjax(b)});b.getRoot().on(h.hidden,function(){b.destroy()});b.setBody(a.getBody(k));b.show()}).fail(f.exception)})};l.prototype.bindUnenrol=function(){var a=this;b(k.UNENROL).click(function(d){d.preventDefault();var e=b(this),i=e.parent(),j=[{key:"unenrol",component:"enrol"},{key:"unenrolconfirm",component:"enrol",param:{user:i.data("fullname"),course:i.data("coursename"),enrolinstancename:i.data("enrolinstancename")}}],k=g.create({type:g.types.SAVE_CANCEL});b.when(c.get_strings(j),k).done(function(c,f){var g=c[0],i=c[1];f.setTitle(g);f.setBody(i);f.setSaveButtonText(g);f.getRoot().on(h.save,function(){var c={ueid:b(e).attr("rel")};d.preventDefault();a.submitUnenrolFormAjax(f,c)});f.getRoot().on(h.hidden,function(){f.destroy()});f.show()}).fail(f.exception)})};l.prototype.bindStatusDetails=function(){b(k.SHOW_DETAILS).click(function(d){d.preventDefault();var e=b(this),i=e.parent(),j={fullname:i.data("fullname"),coursename:i.data("coursename"),enrolinstancename:i.data("enrolinstancename"),status:i.data("status"),statusclass:i.find("span").attr("class"),timestart:i.data("timestart"),timeend:i.data("timeend"),timeenrolled:i.data("timeenrolled")},l=e.next(k.EDIT_ENROLMENT);if(l.length){j.editenrollink=b("<div>").append(l.clone()).html()}var m=c.get_strings([{key:"enroldetails",component:"enrol"}]),n=g.create({large:!0,type:g.types.CANCEL});b.when(m,n).done(function(c,d){var e=a.render("core_user/status_details",j);d.setTitle(c[0]);d.setBody(e);if(l.length){d.getRoot().on("click",k.EDIT_ENROLMENT,function(a){a.preventDefault();d.hide();b(l).trigger("click")})}d.show();d.getRoot().on(h.hidden,function(){d.destroy()})}).fail(f.exception)})};l.prototype.submitEditFormAjax=function(a){var c=this,d=a.getRoot().find("form"),e=b(d).find("[name=\"ue\"]").val(),g={methodname:"core_enrol_submit_user_enrolment_form",args:{formdata:d.serialize()}};j.call([g])[0].done(function(b){if(b.result){a.hide();if("undefined"!=typeof window.M.core_formchangechecker){window.M.core_formchangechecker.reset_form_dirty_state()}window.location.reload()}else{var f=JSON.stringify(d.serialize());a.setBody(c.getBody(e,f))}}).fail(f.exception)};l.prototype.submitUnenrolFormAjax=function(a,b){j.call([{methodname:"core_enrol_unenrol_user_enrolment",args:b}])[0].done(function(b){if(b.result){a.hide();if("undefined"!=typeof window.M.core_formchangechecker){window.M.core_formchangechecker.reset_form_dirty_state()}window.location.reload()}else{f.alert(b.errors[0].key,b.errors[0].message)}}).fail(f.exception)};l.prototype.getBody=function(a,b){var c={ueid:a};if("undefined"!=typeof b){c.formdata=b}return i.loadFragment("enrol","user_enrolment_form",this.contextid,c).fail(f.exception)};return{init:function init(a){new l(a)}}});
 //# sourceMappingURL=status_field.min.js.map

user/amd/src/status_field.js

@@ -261,48 +261,12 @@ define(['core/templates',
 
             // User enrolment ID.
             var ueid = $(form).find('[name="ue"]').val();
-            // Status.
-            var status = $(form).find('[name="status"]').val();
-
-            var params = {
-                'courseid': this.courseid,
-                'ueid': ueid,
-                'status': status
-            };
-
-            // Enrol time start.
-            var timeStartEnabled = $(form).find('[name="timestart[enabled]"]');
-            if (timeStartEnabled.is(':checked')) {
-                var timeStartYear = $(form).find('[name="timestart[year]"]').val();
-                var timeStartMonth = $(form).find('[name="timestart[month]"]').val() - 1;
-                var timeStartDay = $(form).find('[name="timestart[day]"]').val();
-                var timeStartHour = $(form).find('[name="timestart[hour]"]').val();
-                var timeStartMinute = $(form).find('[name="timestart[minute]"]').val();
-                var timeStart = new Date(timeStartYear, timeStartMonth, timeStartDay, timeStartHour, timeStartMinute);
-                params.timestart = timeStart.getTime() / 1000;
-            }
-
-            // Enrol time end.
-            var timeEndEnabled = $(form).find('[name="timeend[enabled]"]');
-            if (timeEndEnabled.is(':checked')) {
-                var timeEndYear = $(form).find('[name="timeend[year]"]').val();
-                var timeEndMonth = $(form).find('[name="timeend[month]"]').val() - 1;
-                var timeEndDay = $(form).find('[name="timeend[day]"]').val();
-                var timeEndHour = $(form).find('[name="timeend[hour]"]').val();
-                var timeEndMinute = $(form).find('[name="timeend[minute]"]').val();
-                var timeEnd = new Date(timeEndYear, timeEndMonth, timeEndDay, timeEndHour, timeEndMinute);
-                params.timeend = timeEnd.getTime() / 1000;
-            }
-
-            // Enrol duration.
-            var enrolDuration = $(form).find('[name="duration"]');
-            if (enrolDuration.is(':enabled')) {
-                params.timeend = params.timestart + parseInt(enrolDuration.val());
-            }
 
             var request = {
-                methodname: 'core_enrol_edit_user_enrolment',
+                methodname: 'core_enrol_submit_user_enrolment_form',
-                args: params
+                args: {
+                    formdata: form.serialize()
+                }
             };
 
             Ajax.call([request])[0].done(function(data) {

version.php

@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2019083000.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2019090400.00;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.