Adding media library field on any entity and making it as required. Create the entity and submit the form without having value in media library field and it will show wrong validation message.

Steps to repeat issue:

1. Enable the Media Library and Media modules.

2. Create content type with Media Library field and make it a required field.

3. Create new content type using Media Library field and save the form to display the wrong error message, "This value should not be null"

The value should not be null

See screenshot for error message.

Issue fork drupal-3175146

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

deepaverma created an issue. See original summary.

deepaverma’s picture

Status: Active » Needs review
FileSize
4.49 KB

Created patch for Drupal 8.9.6

marcoscano’s picture

Just a straight re-roll for 9.2.x

volkswagenchick’s picture

Adding some tags for DrupalCon NA which is happening April 12-16 virtually.

Can steps to reproduce/test/validation be added to the summary?

I tagged this issue novice in hopes that a new contributor could work on this, thanks.

grgcrlsn321’s picture

Version: 8.9.x-dev » 9.2.x-dev
Issue summary: View changes
FileSize
37.48 KB
grgcrlsn321’s picture

Version: 9.2.x-dev » 8.9.x-dev
grgcrlsn321’s picture

Title: Media Library widget produces "This value should not be null" wrong error when field is required » Edit Media Library widget produces "This value should not be null" wrong error when field is required
Version: 8.9.x-dev » 9.2.x-dev
Issue summary: View changes

Updated summary.

Abhijith S’s picture

Applied patch #3 and it works fine.The error message is showing correctly with the field name after applying this patch.

Before patch:
before

After patch:
after

RTBC +1

grgcrlsn321’s picture

Status: Needs review » Reviewed & tested by the community

Tested patch and works as expected with correct error message, "Media content field is required."

catch’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/media_library/tests/src/Functional/FieldWidgetTest.php
@@ -0,0 +1,65 @@
+    $this->drupalGet("/node/add/$node_type");
+    $this->getSession()->getPage()->pressButton('Save');
+    $this->assertSession()->pageTextNotContains('This value should not be null');
+  }

We should add a positive assertion here for the text we actually expect to get, otherwise there's no guarantee we're getting a validation message at all.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

vakulrai’s picture

Status: Needs work » Needs review
FileSize
4.42 KB
584 bytes

@catch , I have rerolled and modified the patch. I have replaced pageTextNotContains with pageTextContains assertion.

Thanks.

chetanbharambe’s picture

Assigned: Unassigned » chetanbharambe
chetanbharambe’s picture

Assigned: chetanbharambe » Unassigned
Status: Needs review » Reviewed & tested by the community
FileSize
370.87 KB
369.49 KB

Verified and tested patch #12.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: Extend -> Enable the Media Library and Media modules.
# Create the content type with the Media Library field and make it a required field
# Create the new content type using the Media Library field and save it.
# User is able to see a wrong error message, "This value should not be null"

Expected Results:
# User should see the message like "Media field is required"

Actual Results:
# User is able to see a wrong error message, "This value should not be null"

Can be a move to RTBC.

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -Novice +Needs subsystem maintainer review
+++ b/core/modules/media_library/src/Plugin/Field/FieldWidget/MediaLibraryWidget.php
@@ -939,4 +942,34 @@ protected static function setFieldState(array $element, FormStateInterface $form
+
+    // Trigger error if the field is required and no media is present.
+    // Although the Form API's default validation would also catch this, the
+    // validation error message is too vague, so a more precise one is
+    // provided here.
+    $selection_count = !empty($element['selection']) ? count(Element::children($element['selection'])) : 0;
+    if (empty($media) && $selection_count === 0 && !empty($element['#required'])) {
+      $form_state->setError($element, new TranslatableMarkup('@name field is required.',
+        ['@name' => $element['#title']]));
+      return;
+    }
+  }

This feels like it shouldn't be necessary. Is there a reason we can't use the browser's own required form element feature? Going to tag for subsystem maintainer review.

Also feel like 'The value should not be null' isn't really good for any situation, so what about changing the default message itself?

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

sahilgidwani’s picture

Rerolled patch 12 for Drupal 9.5

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Ratan Priya’s picture

Status: Needs work » Needs review
FileSize
4.55 KB
2.68 KB

Tried to re-roll patch #12 for 11.x-dev.

smustgrave’s picture

Status: Needs review » Needs work

Did not test but was previously tagged for issue summary update which still looks like needs to be done.