Add support for multi-value fields like Name and Address fields

Created on 12 June 2024, about 1 year ago

Problem/Motivation

Address and Name fields in its entirety are detected by Require-On-Publish (ROP) module but not the subfields. The subfields are not considered as actual fields and it doesn't add indicators to them. So, ROP validates the entire multi-value field and only throws an error if the entire field (all sub-fields) is empty.

However, if only a part of the name field or address is empty, e.g."state" or "city" or "address line 1" field is empty, ROP does not consider them and still allows you to publish, without validating them, and no required-on-publish validation error is shown on the field.

Please note that: these fields e.g. name or address have their own required/optional setting for the subfields. Those can be set as either optional or required. ROP doesn't consider this either.

Steps to reproduce

Install address, https://www.drupal.org/project/address or name field module: https://www.drupal.org/project/name
Create a field of address field.
Set the subfields components to 'Optional'. Setting to 'Required' will not allow the form to be submitted even in draft/unpublished.

Ensure ROP module is enabled on the address field, fill only one of the subfields, leave others empty, submit Published. ROP doesn't kick in and you don't get any required messages on the address field. Same with Name field.

Remaining tasks

Add support for multivalue fields.

User interface changes

Add indicators (blue carat) to subfields.

Feature request
Status

Active

Version

1.0

Component

Code

Created by

🇺🇸United States jumoke

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @jumoke
  • 🇺🇸United States jcandan

    Updated the description to take into account how the https://www.drupal.org/project/address and https://www.drupal.org/project/name modules let site-builders manage which sub-fields are required.

  • 🇺🇸United States jcandan

    Updated title to clarify that this isn't about cardinality, but fields types that include multiple fields--compound fields.

  • 🇺🇸United States jcandan

    Original report

    The original report proposed that all subfields should be marked optional when reproducing this issue. It then suggested that the field could be subsequently submitted as Published without the desired required fields. This makes sense because there was no field marked as required. ROP validation kicks in if the field is left completely empty.

    For the Address field, this means that to not have selected a Country, it would not pass Require on Publish validation. And it correctly does not.

    For the Name field, it does not even allow you to set all subfields as optional, at least Given or Family must be required:

    If I were to fill in the Given and Family fields, and Select a Country, this is able to be saved as Published.

  • 🇺🇸United States jcandan

    Gave this a shot.

    /**
     * Implements hook_field_widget_complete_form_alter().
     */
    function require_on_publish_field_widget_complete_form_alter(&$field_widget_complete_form, FormStateInterface $form_state, $context) {
      $field_config = $context['items']->getFieldDefinition();
      $field_name = $field_widget_complete_form['widget']['#field_name'];
      /** @var \Drupal\Core\Entity\EntityFormInterface $form_object */
      $form_object = $form_state->getFormObject();
      if ($form_object instanceof EntityFormInterface) {
        /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
        $entity = $form_object->getEntity();
        if (_require_on_publish_entity_is_publishable(get_class($entity))) {
          // $field = $entity->getFields()[$field_name];
          // $field_config = $field->getFieldDefinition();
          if (($field_config instanceof FieldConfigInterface)) {
            if ($field_config->getThirdPartySetting('require_on_publish', 'require_on_publish', FALSE)) {
              $place_breakpoint_here = 'Analyze the available Widget';
              // Loop the widget sub-fields and determine if they're required. Then,
              // treat each required subfield as if it were required on publish.
            }
          }
        }
      }
    }
    

    Tried similar attempts from preprocess hooks, like require_on_publish_preprocess_form_element(). The problem is that I cannot get a combination of render array and the entity context necessary to alter the render array to:

    1. Determine the subfield's parent is required on publish.
    2. Remove the required attribute.
    3. Somehow implement the require_on_publish logic.

    I am open to suggestions and willing to accept DMs in #slack.

  • 🇺🇸United States jcandan

    Just had an idea, noting it here so I don't forget.

    What if I tackle this from a new Constraint?

  • 🇺🇸United States jcandan

    I was able to Disable HTML5 validation to get around the Address module's required attribute. So, similar to Name, I am getting the required subfield validation.

  • 🇺🇸United States jcandan

    Using the existing constraint, if I could just get it to run the AddressFormatConstraintValidator before RequireOnPublishValidator, I think I would then be able to alter the $this->context->getViolations.

    Tried increasing the module weight with module_set_weight() to no avail.

  • Status changed to Needs work about 1 year ago
  • 🇺🇸United States jcandan

    A patch supporting the Name field.

  • 🇺🇸United States jcandan

    Needs work: Incorrectly replacing other field validation error messages with @field_label also requires the following parts when publishing: <em>@components</em>.

  • 🇺🇸United States jcandan

    When a simple field is required on publish, and the compound (e.g. name) field error validation is ordered before the simple field's messages, the messages are all replaced by the @field_label also requires the following parts when publishing: <em>@components</em>.

  • 🇺🇸United States jcandan

    Fixed issue where other field validation messages were being overridden.

  • 🇺🇸United States jcandan

    As per #13, updating the scope of this ticket to target the name field only. Address field already has a ticket in the queue: 📌 Add support for Address field Active .

  • Merge request !26Issue #3454164: Add support for Name fields → (Open) created by jcandan
  • Pipeline finished with Failed
    about 1 month ago
    Total: 142s
    #532597
  • 🇺🇸United States jcandan

    Pipelines fail. Needs fixes. Would like to shoot for a 2.1.0 release.

  • 🇮🇳India anirudhsingh19

    Working on it! Will fix the pipeline failure issues.

  • Pipeline finished with Failed
    5 days ago
    Total: 350s
    #561296
  • Pipeline finished with Failed
    5 days ago
    Total: 298s
    #561300
  • Pipeline finished with Success
    5 days ago
    Total: 224s
    #561307
  • 🇮🇳India anirudhsingh19

    Fixed the failing pipeline. Please review!!

  • 🇺🇸United States jcandan

    @anirudhsingh19, thank you for contributing! We're close. We need test coverage. I got us part of the way there by adding Name as a part of the CI pipeline.

    However, I recognized a problem while writing tests for this. We're currently blocked by 📌 Ensure require_on_publish setting is applied when FieldConfig is created programmatically Active .

  • 🇺🇸United States jcandan

    Never mind. #3539138-2: Ensure require_on_publish setting is applied when FieldConfig is created programmatically explains why.

    But, there is another issue. 📌 Fix EditPageTest not actually ever hitting assertions Active needs to be resolved so we can effectively test the Name Field support.

  • 🇺🇸United States jcandan

    Added test coverage.

Production build 0.71.5 2024