Add e2e tests for boolean checkbox in page data form

Created on 9 April 2025, about 1 month ago

Overview

In 📌 Discover cases where is no 1:1 map between field, prop and widget values Active we're adding e2e tests for core widgets.

Proposed resolution

Add explicit e2e coverage to entity-form-field-types-test.cy.js for boolean checkbox widget

User interface changes

📌 Task
Status

Active

Version

0.0

Component

Redux-integrated field widgets

Created by

🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

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

Merge Requests

Comments & Activities

  • Issue created by @larowlan
  • First commit to issue fork.
  • 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 682s
    #471437
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1702s
    #471447
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Pushed the required fix here.

    The nuance is this in FormBuilder::handleInputElement

    // Get the input for the current element. NULL values in the input need
            // to be explicitly distinguished from missing input. (see below)
            $input_exists = NULL;
            $input = NestedArray::getValue($form_state->getUserInput(), $element['#parents'], $input_exists);
            // For browser-submitted forms, the submitted values do not contain
            // values for certain elements (empty multiple select, unchecked
            // checkbox). During initial form processing, we add explicit NULL
            // values for such elements in FormState::$input. When rebuilding the
            // form, we can distinguish elements having NULL input from elements
            // that were not part of the initially submitted form and can therefore
            // use default values for the latter, if required. Programmatically
            // submitted forms can submit explicit NULL values when calling
            // self::submitForm() so we do not modify FormState::$input for them.
            if (!$input_exists && !$form_state->isRebuilding() && !$form_state->isProgrammed()) { // 👈️👈️👈️👈️👈️👈️👈️
              // Add the necessary parent keys to FormState::$input and sets the
              // element's input value to NULL.
              NestedArray::setValue($form_state->getUserInput(), $element['#parents'], NULL);
              $input_exists = TRUE;
            }
    
    

    With \Drupal\experience_builder\ClientDataToEntityConverter::setEntityFields we're doing a $form_state->setProgrammed() so that set value never happens. Therefore the old code that was unsetting (or rather filtering out) unticked checkboxes actually needs to be setting them to NULL rather than removing them.

    With that change in place, the test passes 🙌

  • First commit to issue fork.
  • Pipeline finished with Canceled
    28 days ago
    Total: 67s
    #473379
  • Pipeline finished with Success
    28 days ago
    Total: 1509s
    #473380
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    The MR looks good, but could this be expanded to include a checkbox with a default value unchecked, in addition to the existing one with ['value' => 1],?

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #9: great call! This one is tantalizingly close!

  • Pipeline finished with Canceled
    13 days ago
    Total: 64s
    #484390
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Rebased, addressed review comments and #9

  • Pipeline finished with Canceled
    13 days ago
    Total: 436s
    #484391
  • Pipeline finished with Failed
    13 days ago
    Total: 636s
    #484393
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    @bnjmnm's feedback in #9 has been addressed 👍

  • Pipeline finished with Skipped
    13 days ago
    #484810
  • Pipeline finished with Skipped
    13 days ago
    #484811
  • Pipeline finished with Skipped
    13 days ago
    #484812
  • Pipeline finished with Skipped
    13 days ago
    #484813
  • Pipeline finished with Skipped
    13 days ago
    #484814
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Looks like this somehow caused consistent failures in entity-form-field-types-test.cy.js despite multiple CI runs on this MR being green— see https://git.drupalcode.org/project/experience_builder/-/commit/bc8ce91c5....

  • Merge request !963#3518313 update to fix fails → (Merged) created by bnjmnm
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    OMG I AM AN IDIOT 😭🙈

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Ben's my hero of the day! 🦸

  • Pipeline finished with Skipped
    13 days ago
    #484942
  • Pipeline finished with Skipped
    13 days ago
    #484943
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Thanks so much!

Production build 0.71.5 2024