MDL-59368 groups: Peer review fixes
authorDamyon Wiese <damyon@moodle.com>
Fri, 7 Jul 2017 02:08:18 +0000 (10:08 +0800)
committerDamyon Wiese <damyon@moodle.com>
Wed, 12 Jul 2017 02:09:09 +0000 (10:09 +0800)
* Fix coalesce on postgres.
* The edit icon's alt shows the HTML entities causing
* enrol/tests/behat/add_to_group.feature Contains a '@doit' tag which I assume is there from testing.
* group/classes/output/user_groups_editable.php
** Missing MOODLE_INTERNAL check.
** Unused variables context_system and moodle_exception.
** The PHPDocs for the constructors are wrong.
** export_for_template() returns an array, not stdClass (according to parent docs).
** Change moodle_exception to coding_exception at the beginning.
* group/lib.php
** Missing docs for core_group_inplace_editable().
* user/classes/participants_table.php
** The docs for the class variables $groups, $course and $context need a '\' beforehand as they are in a namespace.
** I would prefer $this->context = $context; and $this->groups = ... to be done at the end of the constructor with the other class variable assignments.
** You could get rid of the class variable courseid if we are setting course and use $this->course->id instead.
** The function col_groups has @param \stdclass $row but it should be $data
* lib/amd/src/form-autocomplete.js and lib/amd/src/inplace_editable.js
** Some issues here CiBot has pointed out.
* lib/classes/output/inplace_editable.php
** @see should be on a new line.

enrol/tests/behat/add_to_group.feature
group/classes/output/user_groups_editable.php
group/lib.php
lib/amd/build/form-autocomplete.min.js
lib/amd/build/inplace_editable.min.js
lib/amd/src/form-autocomplete.js
lib/amd/src/inplace_editable.js
lib/classes/output/inplace_editable.php
lib/templates/inplace_editable.mustache
user/classes/participants_table.php

index 0324747..c5fdca1 100644 (file)
@@ -1,4 +1,4 @@
-@core_enrol @core_group @doit
+@core_enrol @core_group
 Feature: Users can be added to multiple groups at once
   In order to manage group membership effectively
   As a user
index 12c8c57..f21be02 100644 (file)
 
 namespace core_group\output;
 
-use context_system;
 use context_course;
 use core_user;
 use core_external;
-use moodle_exception;
+use coding_exception;
+
+defined('MOODLE_INTERNAL') || die();
 
 require_once($CFG->dirroot . '/group/lib.php');
 
@@ -49,7 +50,11 @@ class user_groups_editable extends \core\output\inplace_editable {
     /**
      * Constructor.
      *
-     * @param \stdClass|core_tag_tag $tag
+     * @param \stdClass $course The current course
+     * @param \context $context The course context
+     * @param \stdClass $user The current user
+     * @param \stdClass[] $coursegroups The list of course groups from groups_get_all_groups with membership.
+     * @param string $value JSON Encoded list of group ids.
      */
     public function __construct($course, $context, $user, $coursegroups, $value) {
         // Check capabilities to get editable value.
@@ -83,7 +88,7 @@ class user_groups_editable extends \core\output\inplace_editable {
      * Export this data so it can be used as the context for a mustache template.
      *
      * @param \renderer_base $output
-     * @return \stdClass
+     * @return array
      */
     public function export_for_template(\renderer_base $output) {
         $listofgroups = [];
index 67d04dd..5df273b 100644 (file)
@@ -1097,6 +1097,14 @@ function groups_sync_with_enrolment($enrolname, $courseid = 0, $gidfield = 'cust
     return $affectedusers;
 }
 
+/**
+ * Callback for inplace editable API.
+ *
+ * @param string $itemtype - Only user_groups is supported.
+ * @param string $itemid - Userid and groupid separated by a :
+ * @param string $newvalue - json encoded list of groupids.
+ * @return \core\output\inplace_editable
+ */
 function core_group_inplace_editable($itemtype, $itemid, $newvalue) {
     if ($itemtype === 'user_groups') {
         return \core_group\output\user_groups_editable::update($itemid, $newvalue);
index 7e9a903..05b38d0 100644 (file)
Binary files a/lib/amd/build/form-autocomplete.min.js and b/lib/amd/build/form-autocomplete.min.js differ
index f77bf3d..73071d8 100644 (file)
Binary files a/lib/amd/build/inplace_editable.min.js and b/lib/amd/build/inplace_editable.min.js differ
index f199838..4dad374 100644 (file)
@@ -738,7 +738,7 @@ define(['jquery', 'core/log', 'core/str', 'core/templates', 'core/notification']
             var originalSelect = $(selector);
             if (!originalSelect) {
                 log.debug('Selector not found: ' + selector);
-                return;
+                return false;
             }
 
             // Hide the original select.
@@ -770,7 +770,7 @@ define(['jquery', 'core/log', 'core/str', 'core/templates', 'core/notification']
             var renderDatalist = templates.render('core/form_autocomplete_suggestions', context);
             var renderSelection = templates.render('core/form_autocomplete_selection', context);
 
-            return $.when(renderInput, renderDatalist, renderSelection).done(function(input, suggestions, selection) {
+            return $.when(renderInput, renderDatalist, renderSelection).then(function(input, suggestions, selection) {
                 // Add our new UI elements to the page.
                 originalSelect.after(suggestions);
                 originalSelect.after(input);
index 471795c..20ca0fb 100644 (file)
@@ -262,7 +262,9 @@ define(['jquery',
                 .then(function() {
                 // Focus on the enhanced combobox.
                 el.find('[role=combobox]').focus();
-            });
+                // Stop eslint nagging.
+                return;
+            }).fail(notification.exception);
 
             inputelement.on('keyup', function(e) {
                 if ((e.type === 'keyup' && e.keyCode === 27) || e.type === 'focusout') {
index 4bdc2ae..bcfa2f8 100644 (file)
@@ -197,7 +197,7 @@ class inplace_editable implements templatable, renderable {
      * Sets the element type to be an autocomplete field
      *
      * @param array $options associative array with dropdown options
-     * @param array $attributes associative array with attributes for autoselect field. @see AMD module core/form-autocomplete
+     * @param array $attributes associative array with attributes for autoselect field. See AMD module core/form-autocomplete.
      * @return self
      */
     public function set_type_autocomplete($options, $attributes) {
index 8091e85..dc78b41 100644 (file)
@@ -56,7 +56,7 @@
         {{^ linkeverything }}{{{displayvalue}}}{{/ linkeverything }}
         <a href="#" class="quickeditlink" data-inplaceeditablelink="1" title="{{edithint}}">
             {{# linkeverything }}{{{displayvalue}}}{{/ linkeverything }}
-            <span class="quickediticon visibleifjs">{{#pix}}t/editstring,core,{{edithint}}{{/pix}}</span>
+            <span class="quickediticon visibleifjs">{{#pix}}t/editstring,core,{{{edithint}}}{{/pix}}</span>
         </a>
 </span>
 {{#js}}
index 329467d..8730008 100644 (file)
@@ -76,7 +76,7 @@ class participants_table extends \table_sql {
     protected $countries;
 
     /**
-     * @var stdClass[] The list of groups with membership info for the course.
+     * @var \stdClass[] The list of groups with membership info for the course.
      */
     protected $groups;
 
@@ -86,12 +86,12 @@ class participants_table extends \table_sql {
     protected $extrafields;
 
     /**
-     * @var stdClass The course details.
+     * @var \stdClass The course details.
      */
     protected $course;
 
     /**
-     * @var context The course context.
+     * @var \context The course context.
      */
     protected $context;
 
@@ -115,7 +115,6 @@ class participants_table extends \table_sql {
         // Get the context.
         $this->course = get_course($courseid);
         $context = \context_course::instance($courseid, MUST_EXIST);
-        $this->context = $context;
 
         // Define the headers and columns.
         $headers = [];
@@ -136,7 +135,6 @@ class participants_table extends \table_sql {
         }
 
         // Load and cache the course groupinfo.
-        $this->groups = groups_get_all_groups($courseid, 0, 0, 'g.*', true);
         // Add column for groups.
         $headers[] = get_string('groups');
         $columns[] = 'groups';
@@ -173,7 +171,6 @@ class participants_table extends \table_sql {
         $this->set_attribute('id', 'participants');
 
         // Set the variables we need to use later.
-        $this->courseid = $courseid;
         $this->currentgroup = $currentgroup;
         $this->accesssince = $accesssince;
         $this->roleid = $roleid;
@@ -181,6 +178,8 @@ class participants_table extends \table_sql {
         $this->selectall = $selectall;
         $this->countries = get_string_manager()->get_list_of_countries();
         $this->extrafields = $extrafields;
+        $this->context = $context;
+        $this->groups = groups_get_all_groups($courseid, 0, 0, 'g.*', true);
     }
 
     /**
@@ -207,25 +206,25 @@ class participants_table extends \table_sql {
     public function col_fullname($data) {
         global $OUTPUT;
 
-        return $OUTPUT->user_picture($data, array('size' => 35, 'courseid' => $this->courseid)) . ' ' . fullname($data);
+        return $OUTPUT->user_picture($data, array('size' => 35, 'courseid' => $this->course->id)) . ' ' . fullname($user);
     }
 
     /**
      * Generate the groups column.
      *
-     * @param \stdClass $row
+     * @param \stdClass $data
      * @return string
      */
-    public function col_groups($user) {
+    public function col_groups($data) {
         global $OUTPUT;
 
         $usergroups = [];
         foreach ($this->groups as $coursegroup) {
-            if (isset($coursegroup->members[$user->id])) {
+            if (isset($coursegroup->members[$data->id])) {
                 $usergroups[] = $coursegroup->id;
             }
         }
-        $editable = new \core_group\output\user_groups_editable($this->course, $this->context, $user, $this->groups, $usergroups);
+        $editable = new \core_group\output\user_groups_editable($this->course, $this->context, $data, $this->groups, $usergroups);
         return $OUTPUT->render_from_template('core/inplace_editable', $editable->export_for_template($OUTPUT));
     }
 
@@ -295,7 +294,7 @@ class participants_table extends \table_sql {
     public function query_db($pagesize, $useinitialsbar = true) {
         list($twhere, $tparams) = $this->get_sql_where();
 
-        $total = user_get_total_participants($this->courseid, $this->currentgroup, $this->accesssince,
+        $total = user_get_total_participants($this->course->id, $this->currentgroup, $this->accesssince,
             $this->roleid, $this->search, $twhere, $tparams);
 
         $this->pagesize($pagesize, $total);
@@ -305,7 +304,7 @@ class participants_table extends \table_sql {
             $sort = 'ORDER BY ' . $sort;
         }
 
-        $this->rawdata = user_get_participants($this->courseid, $this->currentgroup, $this->accesssince,
+        $this->rawdata = user_get_participants($this->course->id, $this->currentgroup, $this->accesssince,
             $this->roleid, $this->search, $twhere, $tparams, $sort, $this->get_page_start(),
             $this->get_page_size());