From fde3369b6cf2ee3849013c9f5d69e759acdffed1 Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole Date: Wed, 7 Jun 2017 15:25:41 +0100 Subject: [PATCH] Issue #2486019 by mpdonadio, pfrenssen, webflo, michielnugter, maris.abols, ivanjaros, hauruck, MickeA, Jo Fitzgerald, mian3010, iMiksu: Wrong validation messages in Datelist::validateDatelist() --- .../Core/Datetime/Element/DateElementBase.php | 33 +++++++++++++++++++ .../Drupal/Core/Datetime/Element/Datelist.php | 7 ++-- .../tests/src/Functional/DateTestBase.php | 4 ++- .../src/Functional/DateTimeFieldTest.php | 21 +++++++++--- .../src/Functional/DateRangeFieldTest.php | 17 ++++++---- 5 files changed, 69 insertions(+), 13 deletions(-) diff --git a/core/lib/Drupal/Core/Datetime/Element/DateElementBase.php b/core/lib/Drupal/Core/Datetime/Element/DateElementBase.php index 63b9abb871..433cdf8b18 100644 --- a/core/lib/Drupal/Core/Datetime/Element/DateElementBase.php +++ b/core/lib/Drupal/Core/Datetime/Element/DateElementBase.php @@ -2,6 +2,7 @@ namespace Drupal\Core\Datetime\Element; +use Drupal\Component\Utility\NestedArray; use Drupal\Core\Datetime\DrupalDateTime; use Drupal\Core\Render\Element\FormElement; @@ -69,4 +70,36 @@ protected static function datetimeRangeYears($string, $date = NULL) { return [$min_year, $max_year]; } + /** + * Returns the most relevant title of a datetime element. + * + * Since datetime form elements often consist of combined date and time fields + * the element title might not be located on the element itself but on the + * parent container element. + * + * @param array $element + * The element being processed. + * @param array $complete_form + * The complete form structure. + * + * @return string + * The title. + */ + protected static function getElementTitle($element, $complete_form) { + $title = ''; + if (!empty($element['#title'])) { + $title = $element['#title']; + } + else { + $parents = $element['#array_parents']; + array_pop($parents); + $parent_element = NestedArray::getValue($complete_form, $parents); + if (!empty($parent_element['#title'])) { + $title = $parent_element['#title']; + } + } + + return $title; + } + } diff --git a/core/lib/Drupal/Core/Datetime/Element/Datelist.php b/core/lib/Drupal/Core/Datetime/Element/Datelist.php index 59177e77e9..6f110d395f 100644 --- a/core/lib/Drupal/Core/Datetime/Element/Datelist.php +++ b/core/lib/Drupal/Core/Datetime/Element/Datelist.php @@ -302,6 +302,8 @@ public static function processDatelist(&$element, FormStateInterface $form_state public static function validateDatelist(&$element, FormStateInterface $form_state, &$complete_form) { $input_exists = FALSE; $input = NestedArray::getValue($form_state->getValues(), $element['#parents'], $input_exists); + $title = static::getElementTitle($element, $complete_form); + if ($input_exists) { $all_empty = static::checkEmptyInputs($input, $element['#date_part_order']); @@ -311,10 +313,11 @@ public static function validateDatelist(&$element, FormStateInterface $form_stat } // If there's empty input and the field is required, set an error. elseif (empty($input['year']) && empty($input['month']) && empty($input['day']) && $element['#required']) { - $form_state->setError($element, t('The %field date is required.')); + $form_state->setError($element, t('The %field date is required.', ['%field' => $title])); } elseif (!empty($all_empty)) { foreach ($all_empty as $value) { + $form_state->setError($element, t('The %field date is incomplete.', ['%field' => $title])); $form_state->setError($element[$value], t('A value must be selected for %part.', ['%part' => $value])); } } @@ -326,7 +329,7 @@ public static function validateDatelist(&$element, FormStateInterface $form_stat } // If the input is invalid and an error doesn't exist, set one. elseif ($form_state->getError($element) === NULL) { - $form_state->setError($element, t('The %field date is invalid.', ['%field' => !empty($element['#title']) ? $element['#title'] : ''])); + $form_state->setError($element, t('The %field date is invalid.', ['%field' => $title])); } } } diff --git a/core/modules/datetime/tests/src/Functional/DateTestBase.php b/core/modules/datetime/tests/src/Functional/DateTestBase.php index 369e793343..9127eb25d4 100644 --- a/core/modules/datetime/tests/src/Functional/DateTestBase.php +++ b/core/modules/datetime/tests/src/Functional/DateTestBase.php @@ -107,6 +107,7 @@ protected function setUp() { */ protected function createField() { $field_name = Unicode::strtolower($this->randomMachineName()); + $field_label = Unicode::ucfirst(Unicode::strtolower($this->randomMachineName())); $type = $this->getTestFieldType(); $widget_type = $formatter_type = $type . '_default'; @@ -119,8 +120,9 @@ protected function createField() { $this->fieldStorage->save(); $this->field = FieldConfig::create([ 'field_storage' => $this->fieldStorage, + 'label' => $field_label, 'bundle' => 'entity_test', - 'description' => 'Description for ' . $field_name, + 'description' => 'Description for ' . $field_label, 'required' => TRUE, ]); $this->field->save(); diff --git a/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php b/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php index ab03ff5772..3df2582e3e 100644 --- a/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php +++ b/core/modules/datetime/tests/src/Functional/DateTimeFieldTest.php @@ -208,6 +208,7 @@ public function testDateField() { */ public function testDatetimeField() { $field_name = $this->fieldStorage->getName(); + $field_label = $this->field->label(); // Change the field to a datetime field. $this->fieldStorage->setSetting('datetime_type', 'datetime'); $this->fieldStorage->save(); @@ -216,7 +217,7 @@ public function testDatetimeField() { $this->drupalGet('entity_test/add'); $this->assertFieldByName("{$field_name}[0][value][date]", '', 'Date element found.'); $this->assertFieldByName("{$field_name}[0][value][time]", '', 'Time element found.'); - $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_name, 'Fieldset and label found'); + $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_label, 'Fieldset and label found'); $this->assertFieldByXPath('//fieldset[@aria-describedby="edit-' . $field_name . '-0--description"]', NULL, 'ARIA described-by found'); $this->assertFieldByXPath('//div[@id="edit-' . $field_name . '-0--description"]', NULL, 'ARIA description found'); @@ -352,6 +353,7 @@ public function testDatetimeField() { */ public function testDatelistWidget() { $field_name = $this->fieldStorage->getName(); + $field_label = $this->field->label(); // Ensure field is set to a date only field. $this->fieldStorage->setSetting('datetime_type', 'date'); @@ -370,7 +372,7 @@ public function testDatelistWidget() { // Display creation form. $this->drupalGet('entity_test/add'); - $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_name, 'Fieldset and label found'); + $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_label, 'Fieldset and label found'); $this->assertFieldByXPath('//fieldset[@aria-describedby="edit-' . $field_name . '-0--description"]', NULL, 'ARIA described-by found'); $this->assertFieldByXPath('//div[@id="edit-' . $field_name . '-0--description"]', NULL, 'ARIA description found'); @@ -511,7 +513,7 @@ public function testDatelistWidget() { \Drupal::entityManager()->clearCachedFieldDefinitions(); // Test the widget for validation notifications. - foreach ($this->datelistDataProvider() as $data) { + foreach ($this->datelistDataProvider($field_label) as $data) { list($date_value, $expected) = $data; // Display creation form. @@ -562,13 +564,21 @@ public function testDatelistWidget() { /** * The data provider for testing the validation of the datelist widget. * + * @param string $field_label + * The label of the field being tested. + * * @return array * An array of datelist input permutations to test. */ - protected function datelistDataProvider() { + protected function datelistDataProvider($field_label) { return [ + // Nothing selected. + [['year' => '', 'month' => '', 'day' => '', 'hour' => '', 'minute' => ''], [ + "The $field_label date is required.", + ]], // Year only selected, validation error on Month, Day, Hour, Minute. [['year' => 2012, 'month' => '', 'day' => '', 'hour' => '', 'minute' => ''], [ + "The $field_label date is incomplete.", 'A value must be selected for month.', 'A value must be selected for day.', 'A value must be selected for hour.', @@ -576,17 +586,20 @@ protected function datelistDataProvider() { ]], // Year and Month selected, validation error on Day, Hour, Minute. [['year' => 2012, 'month' => '12', 'day' => '', 'hour' => '', 'minute' => ''], [ + "The $field_label date is incomplete.", 'A value must be selected for day.', 'A value must be selected for hour.', 'A value must be selected for minute.', ]], // Year, Month and Day selected, validation error on Hour, Minute. [['year' => 2012, 'month' => '12', 'day' => '31', 'hour' => '', 'minute' => ''], [ + "The $field_label date is incomplete.", 'A value must be selected for hour.', 'A value must be selected for minute.', ]], // Year, Month, Day and Hour selected, validation error on Minute only. [['year' => 2012, 'month' => '12', 'day' => '31', 'hour' => '0', 'minute' => ''], [ + "The $field_label date is incomplete.", 'A value must be selected for minute.', ]], ]; diff --git a/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php b/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php index 23ba8fab03..e7f0fbbd47 100644 --- a/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php +++ b/core/modules/datetime_range/tests/src/Functional/DateRangeFieldTest.php @@ -46,6 +46,7 @@ protected function getTestFieldType() { */ public function testDateRangeField() { $field_name = $this->fieldStorage->getName(); + $field_label = $this->field->label(); // Loop through defined timezones to test that date-only fields work at the // extremes. @@ -64,7 +65,7 @@ public function testDateRangeField() { $this->assertFieldByXPath('//*[@id="edit-' . $field_name . '-wrapper"]//label[contains(@class, "js-form-required")]', TRUE, 'Required markup found'); $this->assertNoFieldByName("{$field_name}[0][value][time]", '', 'Start time element not found.'); $this->assertNoFieldByName("{$field_name}[0][end_value][time]", '', 'End time element not found.'); - $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_name, 'Fieldset and label found'); + $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_label, 'Fieldset and label found'); $this->assertFieldByXPath('//fieldset[@aria-describedby="edit-' . $field_name . '-0--description"]', NULL, 'ARIA described-by found'); $this->assertFieldByXPath('//div[@id="edit-' . $field_name . '-0--description"]', NULL, 'ARIA description found'); @@ -256,6 +257,7 @@ public function testDateRangeField() { */ public function testDatetimeRangeField() { $field_name = $this->fieldStorage->getName(); + $field_label = $this->field->label(); // Ensure the field to a datetime field. $this->fieldStorage->setSetting('datetime_type', DateRangeItem::DATETIME_TYPE_DATETIME); @@ -267,7 +269,7 @@ public function testDatetimeRangeField() { $this->assertFieldByName("{$field_name}[0][value][time]", '', 'Start time element found.'); $this->assertFieldByName("{$field_name}[0][end_value][date]", '', 'End date element found.'); $this->assertFieldByName("{$field_name}[0][end_value][time]", '', 'End time element found.'); - $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_name, 'Fieldset and label found'); + $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_label, 'Fieldset and label found'); $this->assertFieldByXPath('//fieldset[@aria-describedby="edit-' . $field_name . '-0--description"]', NULL, 'ARIA described-by found'); $this->assertFieldByXPath('//div[@id="edit-' . $field_name . '-0--description"]', NULL, 'ARIA description found'); @@ -428,6 +430,7 @@ public function testDatetimeRangeField() { */ public function testAlldayRangeField() { $field_name = $this->fieldStorage->getName(); + $field_label = $this->field->label(); // Ensure field is set to a all-day field. $this->fieldStorage->setSetting('datetime_type', DateRangeItem::DATETIME_TYPE_ALLDAY); @@ -440,7 +443,7 @@ public function testAlldayRangeField() { $this->assertFieldByXPath('//*[@id="edit-' . $field_name . '-wrapper"]//label[contains(@class, "js-form-required")]', TRUE, 'Required markup found'); $this->assertNoFieldByName("{$field_name}[0][value][time]", '', 'Start time element not found.'); $this->assertNoFieldByName("{$field_name}[0][end_value][time]", '', 'End time element not found.'); - $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_name, 'Fieldset and label found'); + $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_label, 'Fieldset and label found'); $this->assertFieldByXPath('//fieldset[@aria-describedby="edit-' . $field_name . '-0--description"]', NULL, 'ARIA described-by found'); $this->assertFieldByXPath('//div[@id="edit-' . $field_name . '-0--description"]', NULL, 'ARIA description found'); @@ -598,6 +601,7 @@ public function testAlldayRangeField() { */ public function testDatelistWidget() { $field_name = $this->fieldStorage->getName(); + $field_label = $this->field->label(); // Ensure field is set to a date only field. $this->fieldStorage->setSetting('datetime_type', DateRangeItem::DATETIME_TYPE_DATE); @@ -616,7 +620,7 @@ public function testDatelistWidget() { // Display creation form. $this->drupalGet('entity_test/add'); - $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_name, 'Fieldset and label found'); + $this->assertFieldByXPath('//fieldset[@id="edit-' . $field_name . '-0"]/legend', $field_label, 'Fieldset and label found'); $this->assertFieldByXPath('//fieldset[@aria-describedby="edit-' . $field_name . '-0--description"]', NULL, 'ARIA described-by found'); $this->assertFieldByXPath('//div[@id="edit-' . $field_name . '-0--description"]', NULL, 'ARIA description found'); @@ -1121,6 +1125,7 @@ public function testInvalidField() { $this->fieldStorage->setSetting('datetime_type', DateRangeItem::DATETIME_TYPE_DATETIME); $this->fieldStorage->save(); $field_name = $this->fieldStorage->getName(); + $field_label = $this->field->label(); $this->drupalGet('entity_test/add'); $this->assertFieldByName("{$field_name}[0][value][date]", '', 'Start date element found.'); @@ -1299,7 +1304,7 @@ public function testInvalidField() { "{$field_name}[0][end_value][time]" => '12:00:00', ]; $this->drupalPostForm(NULL, $edit, t('Save')); - $this->assertText(new FormattableMarkup('The @title end date cannot be before the start date', ['@title' => $field_name]), 'End date before start date has been caught.'); + $this->assertText(new FormattableMarkup('The @title end date cannot be before the start date', ['@title' => $field_label]), 'End date before start date has been caught.'); $edit = [ "{$field_name}[0][value][date]" => '2012-12-01', @@ -1308,7 +1313,7 @@ public function testInvalidField() { "{$field_name}[0][end_value][time]" => '11:00:00', ]; $this->drupalPostForm(NULL, $edit, t('Save')); - $this->assertText(new FormattableMarkup('The @title end date cannot be before the start date', ['@title' => $field_name]), 'End time before start time has been caught.'); + $this->assertText(new FormattableMarkup('The @title end date cannot be before the start date', ['@title' => $field_label]), 'End time before start time has been caught.'); } /** -- 2.18.1