alleyinteractive / wordpress-fieldmanager

Custom field types for WordPress
Other
533 stars 101 forks source link

Repeatable fields with nested FM Groups prevent proper saving #853

Open simonrcodrington opened 1 year ago

simonrcodrington commented 1 year ago

Summary

There's currently an issue with repeatable fields. When we have a repeater that contains nested FieldManager Groups, the item itself is saved incorrectly and when the page refreshes you're left with an empty item that can never be removed

This is on FieldManager 1.2.5

Example

In the below example we create a Fieldmanager Group to hold information about an array of items. Each item has a position and a nested grouping with a first name and last name

$group = new \Fieldmanager_Group('Data', [
      'name'                      => 'data',
      'description_after_element' => false,
      'collapsible'               => true,
      'collapsed'                 => false,
      'save_empty'                => false,
      'children'                  => [
          'items' => new \Fieldmanager_Group('Items', [
              'name'           => 'items',
              'add_more_label' => 'Add Item',
              'sortable'       => true,
              'collapsible'    => true,
              'save_empty'     => false,
              'extra_elements' => 0,
              'limit'          => 0,
              'children'       => [
                  'position'    => new \Fieldmanager_TextField('Position'),
                  'information' => new \Fieldmanager_Group('Information', [
                      'children' => [
                          'first_name' => new \Fieldmanager_TextField('First Name'),
                          'last_name'  => new \Fieldmanager_TextField('Last Name'),
                      ]
                  ])
              ]
          ])
      ]
  ]);

fieldmanager_example_1

When saved in the DB it has the following format.

fieldmanager_example_2

When we add records and save them, they get saved in the DB. However if we add a new item and leave it blank and save the data, the newly added item will be unable to be removed. If you hover over them and click the 'x' remove button it removes it from the DOM but still its incorrectly saved in the DB.

fieldmanager_example_3

It saves the extra field in the DB even though it shouldn't (its empty)

fieldmanager_example_4

When there is no nested groups i.e. only use top level fields like input, select, checbox etc, when the values are all left blank the new temporary item is not saved (as each field reports that it is empty)

Potential Workaround

As it stands now, you can add items but never successfully remove them once they've been saved incorrectly (as the data exists in the DB, so it will create the empty entry on reload)

To get this working I've had to manually go into the group and use the group_is_empty callback as below:

information' => new \Fieldmanager_Group('Information', [
      'group_is_empty' => function ($items) {
          $isEmpty = true;
          foreach ($items as $item) {
              if (!empty($item)) {
                  $isEmpty = false;
                  break;
              }
          }
          return $isEmpty;
      },
      'children' => [
          'first_name' => new \Fieldmanager_TextField('First Name'),
          'last_name'  => new \Fieldmanager_TextField('Last Name'),
      ]
  ])

With this, we manually check each item to see if it's empty, returning true if they are all empty. While this works for this simple example, if we have lots of fields (or nested groups) it will be a nightmare to determine what is empty

Is this an issue because we are running on older version of Fieldmanager? I've checked the tickets on here and can't see anyone else reporting this issue about repeaters

mboynes commented 1 year ago

Hi @simonrcodrington, thank you for taking the time to submit this issue and including such detail.

The current suggestion is to use group_is_empty as you discovered. The purpose of that property is to give the developer control over determining if a group is empty or not, which often times, is too complex to reliably calculate (specifically, and more importantly, reliably calculate that a group is empty and that emptiness is disposable).

That said, this isn't a particularly complex use case, and I share your expectation that Fieldmanager should be able to handle this on its own. I'll start to think through how much of a breaking change this would be, and I've asked a few others to think it through as well and chime in.