MDL-52873 forms: Generate unique id attributes for modgrade elements
authorDavid Mudrák <david@moodle.com>
Fri, 22 Jan 2016 12:43:42 +0000 (13:43 +0100)
committerDavid Mudrák <david@moodle.com>
Fri, 22 Jan 2016 13:24:53 +0000 (14:24 +0100)
The previous method of generating the id attribute of the elements
within the modgrade group did not take the name of the modgrade field
into account. So if there were multiple fields of the modgrade type
added into a form (not a common case yet still valid), elements created
within the group were assigned same id attributes.

The patch introduces a new method for generating the id attribute of
modgrade elements. The new method takes the name of the modgrade group
into account and returns the id in the format similar to the default one
returned by HTML_QuickForm_element::_generateId().

The patch changes the generated id attribute. Apart from the
block_activity_results' behat feature files, not other places seem to
rely on the exact value.

blocks/activity_results/tests/behat/addunsupportedactivity.feature
blocks/activity_results/tests/behat/highscoreswithscales.feature
blocks/activity_results/tests/behat/highscoreswithscalesandgroups.feature
blocks/activity_results/tests/behat/lowscoreswithscales.feature
blocks/activity_results/tests/behat/lowscoreswithscalesandgroups.feature
lib/form/modgrade.php
lib/tests/formslib_test.php

index 111c96f..012549e 100644 (file)
@@ -35,6 +35,6 @@ Feature: The activity results block displays student scores
     When I follow "Test assignment"
     And I click on "Edit settings" "link" in the "Administration" "block"
     And I set the following fields to these values:
-      | id_modgrade_type | None |
+      | id_grade_modgrade_type | None |
     And I press "Save and return to course"
     Then I should see "Error: the activity selected uses a grading method that is not supported by this block." in the "Activity results" "block"
index 1263382..b5e52f8 100644 (file)
@@ -39,8 +39,8 @@ Feature: The activity results block displays student scores as scales
       | Assignment name | Test assignment |
       | Description | Offline text |
       | assignsubmission_file_enabled | 0 |
-      | id_modgrade_type | Scale |
-      | id_modgrade_scale | My Scale |
+      | id_grade_modgrade_type | Scale |
+      | id_grade_modgrade_scale | My Scale |
     And I follow "Course 1"
     And I navigate to "Grades" node in "Course administration"
     And I turn editing mode on
index 7856da5..d546fda 100644 (file)
@@ -56,8 +56,8 @@ Feature: The activity results block displays student scores as scales
       | Assignment name | Test assignment |
       | Description | Offline text |
       | assignsubmission_file_enabled | 0 |
-      | id_modgrade_type | Scale |
-      | id_modgrade_scale | My Scale |
+      | id_grade_modgrade_type | Scale |
+      | id_grade_modgrade_scale | My Scale |
       | Group mode | Separate groups |
     And I follow "Course 1"
     And I navigate to "Grades" node in "Course administration"
index 149e0f6..00f31f6 100644 (file)
@@ -39,8 +39,8 @@ Feature: The activity results block displays student scores as scales
       | Assignment name | Test assignment |
       | Description | Offline text |
       | assignsubmission_file_enabled | 0 |
-      | id_modgrade_type | Scale |
-      | id_modgrade_scale | My Scale |
+      | id_grade_modgrade_type | Scale |
+      | id_grade_modgrade_scale | My Scale |
     And I follow "Course 1"
     And I navigate to "Grades" node in "Course administration"
     And I turn editing mode on
index 9134339..be5ae33 100644 (file)
@@ -56,8 +56,8 @@ Feature: The activity results block displays student scores as scales
       | Assignment name | Test assignment |
       | Description | Offline text |
       | assignsubmission_file_enabled | 0 |
-      | id_modgrade_type | Scale |
-      | id_modgrade_scale | My Scale |
+      | id_grade_modgrade_type | Scale |
+      | id_grade_modgrade_scale | My Scale |
       | Group mode | Separate groups |
     And I follow "Course 1"
     And I navigate to "Grades" node in "Course administration"
index dd8ccce..73394f4 100644 (file)
@@ -91,15 +91,15 @@ class MoodleQuickForm_modgrade extends MoodleQuickForm_group{
         $langscale = get_string('modgradetypescale', 'grades');
         $scaleselect = @MoodleQuickForm::createElement('select', 'modgrade_scale', $langscale, $scales, $attributes);
         $scaleselect->setHiddenLabel = false;
-        $scaleselect->_generateId();
-        $scaleselectid = $scaleselect->getAttribute('id');
+        $scaleselectid = $this->generate_modgrade_subelement_id('modgrade_scale');
+        $scaleselect->updateAttributes(array('id' => $scaleselectid));
 
         // Maximum grade textbox.
         $langmaxgrade = get_string('modgrademaxgrade', 'grades');
         $maxgrade = @MoodleQuickForm::createElement('text', 'modgrade_point', $langmaxgrade, array());
         $maxgrade->setHiddenLabel = false;
-        $maxgrade->_generateId();
-        $maxgradeid = $maxgrade->getAttribute('id');
+        $maxgradeid = $this->generate_modgrade_subelement_id('modgrade_point');
+        $maxgrade->updateAttributes(array('id' => $maxgradeid));
 
         // Grade type select box.
         $gradetype = array(
@@ -110,7 +110,8 @@ class MoodleQuickForm_modgrade extends MoodleQuickForm_group{
         $langtype = get_string('modgradetype', 'grades');
         $typeselect = @MoodleQuickForm::createElement('select', 'modgrade_type', $langtype, $gradetype, $attributes, true);
         $typeselect->setHiddenLabel = false;
-        $typeselect->_generateId();
+        $typeselectid = $this->generate_modgrade_subelement_id('modgrade_type');
+        $typeselect->updateAttributes(array('id' => $typeselectid));
 
         // Add elements.
 
@@ -315,4 +316,17 @@ class MoodleQuickForm_modgrade extends MoodleQuickForm_group{
         return parent::onQuickFormEvent($event, $arg, $caller);
     }
 
+    /**
+     * Generates the id attribute for the subelement of the modgrade group.
+     *
+     * Uses algorithm similar to what {@link HTML_QuickForm_element::_generateId()}
+     * does but takes the name of the wrapping modgrade group into account.
+     *
+     * @param string $subname the name of the HTML_QuickForm_element in this modgrade group
+     * @return string
+     */
+    protected function generate_modgrade_subelement_id($subname) {
+        $gid = str_replace(array('[', ']'), array('_', ''), $this->getName());
+        return clean_param('id_'.$gid.'_'.$subname, PARAM_ALPHANUMEXT);
+    }
 }
index 18e5edc..cfe314c 100644 (file)
@@ -550,6 +550,33 @@ class core_formslib_testcase extends advanced_testcase {
         $data = $mform->get_data();
         $this->assertSame($expectedvalues, (array) $data);
     }
+
+    /**
+     * MDL-52873
+     */
+    public function test_multiple_modgrade_fields() {
+        $this->resetAfterTest(true);
+
+        $form = new formslib_multiple_modgrade_form();
+        ob_start();
+        $form->display();
+        $html = ob_get_clean();
+
+        $this->assertTag(array('id' => 'fgroup_id_grade1'), $html);
+        $this->assertTag(array('id' => 'id_grade1_modgrade_type'), $html);
+        $this->assertTag(array('id' => 'id_grade1_modgrade_point'), $html);
+        $this->assertTag(array('id' => 'id_grade1_modgrade_scale'), $html);
+
+        $this->assertTag(array('id' => 'fgroup_id_grade2'), $html);
+        $this->assertTag(array('id' => 'id_grade2_modgrade_type'), $html);
+        $this->assertTag(array('id' => 'id_grade2_modgrade_point'), $html);
+        $this->assertTag(array('id' => 'id_grade2_modgrade_scale'), $html);
+
+        $this->assertTag(array('id' => 'fgroup_id_grade_3'), $html);
+        $this->assertTag(array('id' => 'id_grade_3_modgrade_type'), $html);
+        $this->assertTag(array('id' => 'id_grade_3_modgrade_point'), $html);
+        $this->assertTag(array('id' => 'id_grade_3_modgrade_scale'), $html);
+    }
 }
 
 
@@ -822,3 +849,15 @@ class formslib_clean_value extends moodleform {
             'repeatnamedgroup[repeatnamedgroupel2]' => array('type' => PARAM_INT)), 'repeatablenamedgroup', 'add', 0);
     }
 }
+
+/**
+ * Used to test that modgrade fields get unique id attributes.
+ */
+class formslib_multiple_modgrade_form extends moodleform {
+    public function definition() {
+        $mform = $this->_form;
+        $mform->addElement('modgrade', 'grade1', 'Grade 1');
+        $mform->addElement('modgrade', 'grade2', 'Grade 2');
+        $mform->addElement('modgrade', 'grade[3]', 'Grade 3');
+    }
+}