MDL-37361 completion: Consolidate icon logic and clarify web service

- Icon generation logic moved to the course renderer.
- Web service should return the completion info only, not HTML.
- JS modified to handle icon rendering in relevant cases.

completion/classes/external.php

@@ -135,13 +135,11 @@ public static function override_activity_completion_status_parameters() {
      * @param  int $userid    User id
      * @param  int $cmid      Course module id
      * @param  int $newstate  Activity completion
-     * @return array          Result and possible warnings
+     * @return array          Array containing the current (updated) completion status.
      * @since Moodle 3.4
      * @throws moodle_exception
      */
     public static function override_activity_completion_status($userid, $cmid, $newstate) {
-        global $OUTPUT, $DB, $USER;
-
         // Validate and normalize parameters.
         $params = self::validate_parameters(self::override_activity_completion_status_parameters(),
             array('userid' => $userid, 'cmid' => $cmid, 'newstate' => $newstate));
@@ -160,52 +158,19 @@ public static function override_activity_completion_status($userid, $cmid, $news
             throw new moodle_exception('completionnotenabled', 'completion');
         }
 
-        // Update completion state.
+        // Update completion state and get the new state back.
         $completion->update_state($cm, $newstate, $userid, true);
-
-        // Get activity completion data.
         $completiondata = $completion->get_data($cm, false, $userid);
-        $state = $completiondata->completionstate;
-        $overrideby = $completiondata->overrideby;
-        $date = userdate($completiondata->timemodified);
-
-        // Work out how it corresponds to an icon.
-        switch($state) {
-            case COMPLETION_INCOMPLETE :
-                $completiontype = 'n'.($overrideby ? '-override' : '');
-                break;
-            case COMPLETION_COMPLETE :
-                $completiontype = 'y'.($overrideby ? '-override' : '');
-                break;
-            case COMPLETION_COMPLETE_PASS :
-                $completiontype = 'pass';
-                break;
-            case COMPLETION_COMPLETE_FAIL :
-                $completiontype = 'fail';
-                break;
-        }
-
-        $completionicon = 'completion-'.
-            ($cm->completion == COMPLETION_TRACKING_AUTOMATIC ? 'auto' : 'manual').
-            '-'.$completiontype;
-
-        $describe = get_string('completion-' . $completiontype, 'completion', fullname($USER));
-        $user = \core_user::get_user($userid, '*', MUST_EXIST);
-        $a = new StdClass;
-        $a->state = $describe;
-        $a->date = $date;
-        $a->user = fullname($user);
-        $a->activity = $cm->name;
-        $fulldescribe = get_string('progress-title', 'completion', $a);
-
-        $img = $OUTPUT->pix_icon('i/' . $completionicon, s($fulldescribe));
 
         // Return the current state of completion.
-        $result = [
-            'completionstate' => $state,
-            'img' => $img
+        return [
+            'cmid' => $completiondata->coursemoduleid,
+            'userid' => $completiondata->userid,
+            'state' => $completiondata->completionstate,
+            'timecompleted' => $completiondata->timemodified,
+            'overrideby' => $completiondata->overrideby,
+            'tracking' => $completion->is_enabled($cm)
         ];
-        return $result;
     }
 
     /**
@@ -218,8 +183,13 @@ public static function override_activity_completion_status_returns() {
 
         return new external_single_structure(
             array(
-                'completionstate'   => new external_value(PARAM_BOOL, 'The current completion state.'),
-                'img'           => new external_value(PARAM_RAW, 'Image element to replace existing one'),
+                'cmid' => new external_value(PARAM_INT, 'The course module id'),
+                'userid' => new external_value(PARAM_INT, 'The user id to which the completion info belongs'),
+                'state'   => new external_value(PARAM_INT, 'The current completion state.'),
+                'timecompleted' => new external_value(PARAM_INT, 'time of completion'),
+                'overrideby' => new external_value(PARAM_INT, 'The user id who has overriden the status, or null'),
+                'tracking'      => new external_value(PARAM_INT, 'type of tracking:
+                                                                    0 means none, 1 manual, 2 automatic'),
             )
         );
     }

completion/tests/externallib_test.php

@@ -220,36 +220,36 @@ public function test_override_activity_completion_status() {
         $this->setUser($teacher);
         $result = core_completion_external::override_activity_completion_status($student->id, $data->cmid, COMPLETION_INCOMPLETE);
         $result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
-        $this->assertEquals($result['completionstate'], COMPLETION_INCOMPLETE);
+        $this->assertEquals($result['state'], COMPLETION_INCOMPLETE);
         $completiondata = $completion->get_data($cmdata, false, $student->id);
         $this->assertEquals(COMPLETION_INCOMPLETE, $completiondata->completionstate);
 
         // Test overriding the status of the manual-completion-activity back to 'complete'.
         $result = core_completion_external::override_activity_completion_status($student->id, $data->cmid, COMPLETION_COMPLETE);
         $result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
-        $this->assertEquals($result['completionstate'], COMPLETION_COMPLETE);
+        $this->assertEquals($result['state'], COMPLETION_COMPLETE);
         $completiondata = $completion->get_data($cmdata, false, $student->id);
         $this->assertEquals(COMPLETION_COMPLETE, $completiondata->completionstate);
 
         // Test overriding the status of the auto-completion-activity to 'complete'.
         $result = core_completion_external::override_activity_completion_status($student->id, $forum->cmid, COMPLETION_COMPLETE);
         $result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
-        $this->assertEquals($result['completionstate'], COMPLETION_COMPLETE);
+        $this->assertEquals($result['state'], COMPLETION_COMPLETE);
         $completionforum = $completion->get_data($cmforum, false, $student->id);
         $this->assertEquals(COMPLETION_COMPLETE, $completionforum->completionstate);
 
         // Test overriding the status of the auto-completion-activity to 'incomplete'.
         $result = core_completion_external::override_activity_completion_status($student->id, $forum->cmid, COMPLETION_INCOMPLETE);
         $result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
-        $this->assertEquals($result['completionstate'], COMPLETION_INCOMPLETE);
+        $this->assertEquals($result['state'], COMPLETION_INCOMPLETE);
         $completionforum = $completion->get_data($cmforum, false, $student->id);
         $this->assertEquals(COMPLETION_INCOMPLETE, $completionforum->completionstate);
 
         // Test overriding the status of the auto-completion-activity to an invalid state. It should remain incomplete.
         $this->expectException('moodle_exception');
         $result = core_completion_external::override_activity_completion_status($student->id, $forum->cmid, 3);
         $result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
-        $this->assertEquals($result['completionstate'], COMPLETION_INCOMPLETE);
+        $this->assertEquals($result['state'], COMPLETION_INCOMPLETE);
         $completionforum = $completion->get_data($cmforum, false, $student->id);
         $this->assertEquals(COMPLETION_INCOMPLETE, $completionforum->completionstate);
 
@@ -258,7 +258,7 @@ public function test_override_activity_completion_status() {
         unassign_capability('moodle/course:overridecompletion', $teacherrole->id, $coursecontext);
         $result = core_completion_external::override_activity_completion_status($student->id, $forum->cmid, 1);
         $result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
-        $this->assertEquals($result['completionstate'], COMPLETION_INCOMPLETE);
+        $this->assertEquals($result['state'], COMPLETION_INCOMPLETE);
         $completionforum = $completion->get_data($cmforum, false, $student->id);
         $this->assertEquals(COMPLETION_INCOMPLETE, $completionforum->completionstate);
     }

report/progress/amd/src/completion_override.js

@@ -26,12 +26,30 @@ define(['jquery', 'core/ajax', 'core/str', 'core/modal_factory', 'core/modal_eve
         'core/custom_interaction_events', 'core/templates'],
     function($, Ajax, Str, ModalFactory, ModalEvents, Notification, CustomEvents, Templates) {
 
+        /**
+         * @type {String} the full name of the current user.
+         * @private
+         */
+        var userFullName;
+
         /**
          * @type {JQuery} JQuery object containing the element (completion link) that was most recently activated.
          * @private
          */
         var triggerElement;
 
+        /**
+         * Helper function to get the pix icon key based on the completion state.
+         * @method getIconDescriptorFromState
+         * @param {number} state The current completion state.
+         * @param {string} tracking The completion tracking type, either 'manual' or 'auto'.
+         * @return {string} the key for the respective icon.
+         * @private
+         */
+        var getIconKeyFromState = function(state, tracking) {
+            return state > 0 ? 'i/completion-' + tracking + '-y-override' : 'i/completion-' + tracking + '-n-override';
+        };
+
         /**
          * Handles the confirmation of an override change, calling the web service to update it.
          * @method setOverride
@@ -50,12 +68,30 @@ define(['jquery', 'core/ajax', 'core/str', 'core/modal_factory', 'core/modal_eve
                     args: override
                 }])[0];
             }).then(function(results) {
-                // Update the DOM accordingly.
-                var flipState = results.completionstate ? 0 : 1;
-                triggerElement.find('.loading-icon').remove();
-                triggerElement.data('changecompl', override.userid + '-' + override.cmid + '-' + flipState);
-                triggerElement.attr('data-changecompl', override.userid + '-' + override.cmid + '-' + flipState);
-                triggerElement.children("img").replaceWith(results.img);
+                var completionState = (results.state > 0) ? 1 : 0;
+
+                // Now, build the new title string, get the new icon, and update the DOM.
+                var tooltipKey = completionState ? 'completion-y-override' : 'completion-n-override';
+                Str.get_string(tooltipKey, 'completion', userFullName).then(function(stateString) {
+                    var params = {
+                        state: stateString,
+                        date: '',
+                        user: triggerElement.attr('data-userfullname'),
+                        activity: triggerElement.attr('data-activityname')
+                    };
+                    return Str.get_string('progress-title', 'completion', params);
+                }).then(function(titleString) {
+                    var completionTracking = triggerElement.attr('data-completiontracking');
+                    return Templates.renderPix(getIconKeyFromState(completionState, completionTracking), 'core', titleString);
+                }).then(function(html) {
+                    var oppositeState = completionState > 0 ? 0 : 1;
+                    triggerElement.find('.loading-icon').remove();
+                    triggerElement.data('changecompl', override.userid + '-' + override.cmid + '-' + oppositeState);
+                    triggerElement.attr('data-changecompl', override.userid + '-' + override.cmid + '-' + oppositeState);
+                    triggerElement.children("img").replaceWith(html);
+                    return;
+                }).catch(Notification.exception);
+
                 return;
             }).catch(Notification.exception);
         };
@@ -119,9 +155,12 @@ define(['jquery', 'core/ajax', 'core/str', 'core/modal_factory', 'core/modal_eve
         /**
          * Init this module which allows activity completion state to be changed via ajax.
          * @method init
+         * @param {string} fullName The current user's full name.
          * @private
          */
-        var init = function() {
+        var init = function(fullName) {
+            userFullName = fullName;
+
             // Register the click, space and enter events as activators for the trigger element.
             $('#completion-progress a.changecompl').each(function(index, element) {
                 CustomEvents.define(element, [CustomEvents.events.activate]);

report/progress/index.php

@@ -179,7 +179,7 @@ function csv_quote($value) {
     $PAGE->set_title($strcompletion);
     $PAGE->set_heading($course->fullname);
     echo $OUTPUT->header();
-    $PAGE->requires->js_call_amd('report_progress/completion_override', 'init');
+    $PAGE->requires->js_call_amd('report_progress/completion_override', 'init', [fullname($USER)]);
 
     // Handle groups (if enabled)
     groups_print_course_menu($course,$CFG->wwwroot.'/report/progress/?course='.$course->id);
@@ -393,10 +393,8 @@ function csv_quote($value) {
                 $completiontype = 'fail';
                 break;
         }
-
-        $completionicon='completion-'.
-            ($activity->completion==COMPLETION_TRACKING_AUTOMATIC ? 'auto' : 'manual').
-            '-'.$completiontype;
+        $completiontrackingstring = $activity->completion == COMPLETION_TRACKING_AUTOMATIC ? 'auto' : 'manual';
+        $completionicon = 'completion-' . $completiontrackingstring. '-' . $completiontype;
 
         if ($overrideby) {
             $overridebyuser = \core_user::get_user($overrideby, '*', MUST_EXIST);
@@ -421,6 +419,9 @@ function csv_quote($value) {
                 $changecompl = $user->id . '-' . $activity->id . '-' . $newstate;
                 $url = new moodle_url($PAGE->url, ['sesskey' => sesskey()]);
                 $celltext = html_writer::link($url, $celltext, array('class' => 'changecompl', 'data-changecompl' => $changecompl,
+                                                                     'data-activityname' => $a->activity,
+                                                                     'data-userfullname' => $a->user,
+                                                                     'data-completiontracking' => $completiontrackingstring,
                                                                      'aria-role' => 'button'));
             }
             print '<td class="completion-progresscell '.$formattedactivities[$activity->id]->datepassedclass.'">'.