From d84c64b7d6c20b9ab9803d316f1c76db578f8dc5 Mon Sep 17 00:00:00 2001 From: =?utf8?q?David=20Mudr=C3=A1k?= Date: Fri, 22 Jan 2016 13:43:42 +0100 Subject: [PATCH] MDL-52873 forms: Generate unique id attributes for modgrade elements 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. --- .../behat/addunsupportedactivity.feature | 2 +- .../tests/behat/highscoreswithscales.feature | 4 +- .../highscoreswithscalesandgroups.feature | 4 +- .../tests/behat/lowscoreswithscales.feature | 4 +- .../lowscoreswithscalesandgroups.feature | 4 +- lib/form/modgrade.php | 24 +++++++++--- lib/tests/formslib_test.php | 39 +++++++++++++++++++ 7 files changed, 67 insertions(+), 14 deletions(-) diff --git a/blocks/activity_results/tests/behat/addunsupportedactivity.feature b/blocks/activity_results/tests/behat/addunsupportedactivity.feature index 111c96f96a9..012549ea5e5 100644 --- a/blocks/activity_results/tests/behat/addunsupportedactivity.feature +++ b/blocks/activity_results/tests/behat/addunsupportedactivity.feature @@ -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" diff --git a/blocks/activity_results/tests/behat/highscoreswithscales.feature b/blocks/activity_results/tests/behat/highscoreswithscales.feature index 1263382f84b..b5e52f875a1 100644 --- a/blocks/activity_results/tests/behat/highscoreswithscales.feature +++ b/blocks/activity_results/tests/behat/highscoreswithscales.feature @@ -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 diff --git a/blocks/activity_results/tests/behat/highscoreswithscalesandgroups.feature b/blocks/activity_results/tests/behat/highscoreswithscalesandgroups.feature index 7856da599b3..d546fda0c5c 100644 --- a/blocks/activity_results/tests/behat/highscoreswithscalesandgroups.feature +++ b/blocks/activity_results/tests/behat/highscoreswithscalesandgroups.feature @@ -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" diff --git a/blocks/activity_results/tests/behat/lowscoreswithscales.feature b/blocks/activity_results/tests/behat/lowscoreswithscales.feature index 149e0f60f0c..00f31f6b48b 100644 --- a/blocks/activity_results/tests/behat/lowscoreswithscales.feature +++ b/blocks/activity_results/tests/behat/lowscoreswithscales.feature @@ -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 diff --git a/blocks/activity_results/tests/behat/lowscoreswithscalesandgroups.feature b/blocks/activity_results/tests/behat/lowscoreswithscalesandgroups.feature index 91343398269..be5ae335e79 100644 --- a/blocks/activity_results/tests/behat/lowscoreswithscalesandgroups.feature +++ b/blocks/activity_results/tests/behat/lowscoreswithscalesandgroups.feature @@ -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" diff --git a/lib/form/modgrade.php b/lib/form/modgrade.php index dd8ccce6fe1..73394f49bcf 100644 --- a/lib/form/modgrade.php +++ b/lib/form/modgrade.php @@ -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); + } } diff --git a/lib/tests/formslib_test.php b/lib/tests/formslib_test.php index 18e5edc5792..cfe314caba1 100644 --- a/lib/tests/formslib_test.php +++ b/lib/tests/formslib_test.php @@ -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'); + } +} -- 2.17.1