OptionsWidgetBase doesn't respect #required_error

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Add The Re-roll For The 10.1x Branch.

Cleaning out phpcs/phpstan issues.

@Prem Suthar: Please try to run commit-code-check sh script before creating any patch or re-roll.
Here are the steps to do that:

1. visit `core` directory on your local setup
2. run `yarn install`. This will install required js dependency
3. Run script using `sh ./core/scripts/dev/commit-code-check.sh`

Also, you can run test cases on local and resolve test case failures if any. Please refer documentation here https://www.drupal.org/docs/automated-testing/phpunit-in-drupal/running-... →

The issue summary should be updated to include proposed solution.

Updated the issue summary.
I think change record won't be required since it is a minor bug.

@mohit_aghera thanks!

Tests look good!

Verified the issue by adding a List (text) to the Article content type on a standard install.
Made it required
Added
$form['field_test']['widget']['#required_error'] = t('CUSTOM REQUIRED ERROR MESSAGE');
to claro_form_alter and verified errors message not used

Applied patch and not it is!

Adding issue credits

1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/OptionsWidgetBase.php
   @@ -90,7 +89,15 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
   +      elseif (isset($element['#title'])) {
   +        $form_state->setError($element, t('@name field is required.', ['@name' => $element['#title']]));
   +      }
   

   This isn't needed for a widget, the title is set by \Drupal\Core\Field\WidgetBase::form

   So we can keep this as (pseudo code)

   if (custom required) {
   use custom required
   }
   else {
   keep the original code
   }

2. +++ b/core/modules/options/tests/src/Functional/OptionsWidgetsTest.php
   @@ -371,7 +371,47 @@ public function testSelectListSingle() {
   +    // Disable form alter hook.
   +    \Drupal::state()->set('options_test.form_alter_enable', FALSE);
   

   This isn't needed, each test{Something} runs with a new database.

addressed #31, but there is test failure on OptionsWidgetsTest::testSelectListSingle because we removed elseif condition(#31.1)

So I changed the following code.
from

    // Submit form: select invalid 'none' option.
    $edit = ['card_1' => '_none'];
    $this->submitForm($edit, 'Save');
    $this->assertSession()->pageTextContains("{$field->getName()} field is required.");

to

    // Submit form: select invalid 'none' option.
    $edit = ['card_1' => '_none'];
    $this->submitForm($edit, 'Save');
    $this->assertSession()->pageTextContains("Error message");

because I can see only the following

sorry, ignore #32

Not sure we should just editing the test to just pass is the right path.

If making the changes in #31 breaks something we should discuss if that change is needed.

Just "Error message" doesn't seem to be useful.

@larowlan thoughts?

Hi @smustgrave, #32 was my understanding mistake , in #31.1 suggest that

if (custom required) {
use custom required
}
else {
keep the original code
}

so I have updated the code like that in #33, kindly ignore the #32 patch

+++ b/core/modules/options/tests/src/Functional/OptionsWidgetsTest.php
@@ -371,7 +371,44 @@ public function testSelectListSingle() {
-    $this->assertFieldValues($entity_init, 'card_1', []);

this looks like a regression

Putting the line back to testSelectListSingle () test case that was removed.
Both the test cases are passing on local.

I think it was accidentally removed by me in #14

Marking as #39 and #31 appears to be addressed.

Committed to 10.1.x

Waiting for tests before backport.

Thanks!

Resolved the merge conflict

Committed c6e6584 and pushed to 10.0.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.