diff --git a/modules/custom_elements_ui/src/Form/EntityCustomElementsDisplayEditForm.php b/modules/custom_elements_ui/src/Form/EntityCustomElementsDisplayEditForm.php index 2de68b47234dd0e41f34450d1fdf337e2e0c58a9..780c1cd4f1e50c588e4c913b6ea0259d2e0ec972 100644 --- a/modules/custom_elements_ui/src/Form/EntityCustomElementsDisplayEditForm.php +++ b/modules/custom_elements_ui/src/Form/EntityCustomElementsDisplayEditForm.php @@ -97,6 +97,40 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { ); } + /** + * {@inheritdoc} + */ + public function setEntity(EntityInterface $entity) { + // Convert the entity's components to the 'fixed row names' which the form + // can use. Add component property 'name' for the editable names. + $convert_entity_for_form = TRUE; + $converted_components = []; + foreach ($entity->getComponents() as $component_name => $component) { + if (isset($component['name'])) { + // The entity is not the format as loaded from disk (which has no + // 'name'), so apparently it already was converted. This never happens + // in normal operation. + $convert_entity_for_form = FALSE; + break; + } + $component['name'] = $component_name; + $converted_components[$component['field_name']] = $component; + } + + if ($convert_entity_for_form) { + $remove_components = array_fill_keys(array_keys($entity->getComponents()), TRUE); + foreach ($converted_components as $row_name => $component) { + $entity->setComponent($row_name, $component); + unset($remove_components[$row_name]); + } + foreach (array_keys($remove_components) as $key) { + $entity->removeComponent($key); + } + } + + return parent::setEntity($entity); + } + /** * {@inheritdoc} */ @@ -159,7 +193,7 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { if (!$layout_builder_enabled) { $form['help'] = [ '#type' => 'markup', - '#markup' => $this->t("<p><strong>Warning: this user interface is subject to change and contains some bugs. As a result:</strong><ul><li><strong>If you want to rename a 'Key': immediately afterwards, press TAB and then press Save for each individual renamed key.</strong></li><li><strong>When editing anything else, do not rename keys at the same time.</strong></li></ul></p>"), + '#markup' => $this->t("<p><strong>Warning: this user interface is subject to change and contains some bugs. As a result:</strong><ul><li><strong>If you want to rename a 'Key': immediately afterwards, press TAB and then press Save for each individual renamed key.</strong></li></ul></p>"), ]; } $form = parent::form($form, $form_state); @@ -178,13 +212,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { } } - // Remove EntityForm::afterBuild(), which calls copyFormValuesToEntity(); - // we cannot update $this->entity before form values are properly validated. - // Any other validation functions must take this into account. - if (!empty($form['#after_build'])) { - $form['#after_build'] = array_diff($form['#after_build'], ['::afterBuild']); - } - return $form; } @@ -277,7 +304,7 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { '#type' => 'textfield', '#title' => $this->t('Attribute / Slot name'), '#title_display' => 'invisible', - '#default_value' => $component_name ?: $default_name, + '#default_value' => $display_options['name'] ?? $default_name, '#size' => 20, '#required' => empty($display_options) || $display_options['region'] != 'hidden', ]; @@ -323,10 +350,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { '#default_value' => $field_name, '#attributes' => ['class' => ['field-name']], ], - 'existing_name' => [ - '#type' => 'hidden', - '#default_value' => $component_name, - ], ], 'region' => [ '#type' => 'select', @@ -462,65 +485,15 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { /** * Gets component name from field name. * - * This is temporary code as long as the UI still has one row per field. It - * doesn't check whether the CE display has multiple components with the - * same field name - which isn't supported by the UI yet. There is also no - * check on whether a CE display has 'non-field' components. - * - * WARNING: Re-saving such CE displays through the UI will mess up data! + * This is temporary code as long as the UI still has one row per field. * * @return string * The component name. - */ - private function getComponentNameFromFieldName(string $field_name, $entity = NULL, $log = TRUE): string { - if (!isset($entity)) { - $entity = $this->entity; - } - $component_name = ''; - $components = $entity->getComponents(); - foreach ($components as $name => $component) { - if (isset($component['field_name']) && $component['field_name'] == $field_name) { - $component_name = $name; - } - } - - if (!$component_name && $log) { - $this->logger('custom_elements')->warning('No component name found for field @field_name. Alpha UI code is unstable.', ['@field_name' => $field_name]); - } - - return $component_name; - } - - /** - * Form validation callback for the full form. * - * @param array $form - * A nested array of form elements comprising the form. - * @param \Drupal\Core\Form\FormStateInterface $form_state - * The current state of the form. + * @todo remove in #3446485. */ - public function validateForm(array &$form, FormStateInterface $form_state) { - parent::validateForm($form, $form_state); - $form_values = $form_state->getValues(); - - // Validate that no duplicate component name is used. - // @todo there still is a bug with this, as long as the 'name' element - // still has AJAX behavior / triggers requests while renaming individual - // duplicate elements. Fix (remove AJAX behavior) in #3446485. - $component_names_used = []; - foreach ($form['#fields'] as $field_name) { - $name = $form_values['fields'][$field_name]['name']; - if (isset($component_names_used[$name])) { - $form_state->setError($form['fields'][$field_name]['name'], $this->t('Duplicate name %name', ['%name' => $name])); - } - $component_names_used[$name] = TRUE; - } - - // If the form isn't being submitted yet: need to set all components in - // the form's entity, so all plugin forms are rebuilt correctly. - if (!$form_state->getErrors() && !empty($form_state->getTriggeringElement()['#ajax'])) { - $this->doCopyFormValuesToEntity($this->entity, $form, $form_state); - } + private function getComponentNameFromFieldName(string $field_name, $entity = NULL, $log = TRUE): string { + return $field_name; } /** @@ -539,7 +512,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { $validate_entity = clone $this->entity; $field_name = $form_state->getTriggeringElement()['#field_name']; $component_name = $this->getComponentNameFromFieldName($field_name, $validate_entity); - $this->doCopyFormValuesToEntity($validate_entity, $form, $form_state, $component_name); // Allow the 'triggering' formatter to validate its configuration. $plugin = $validate_entity->getRenderer($component_name); @@ -558,29 +530,6 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { * {@inheritdoc} */ protected function copyFormValuesToEntity(EntityInterface $entity, array $form, FormStateInterface $form_state) { - // See form(): this is only called on submit, with form values validated. - $this->doCopyFormValuesToEntity($entity, $form, $form_state); - } - - /** - * Copies form values, or a subset, to entity properties. - * - * This should not change existing entity properties that are not being edited - * by this form. - * - * @param \Drupal\Core\Entity\EntityInterface $entity - * The entity the current form should operate upon. - * @param array $form - * A nested array of form elements comprising the form. - * @param \Drupal\Core\Form\FormStateInterface $form_state - * The current state of the form. - * @param string $validate_component - * (Optional) Only make sure the basic properties of the specified component - * are updated, in order to be able to validate the full component's config. - * Do not copy the component's full (unvalidated) configuration yet; leave - * other component's values in an undefined state. - */ - protected function doCopyFormValuesToEntity(EntityInterface $entity, array $form, FormStateInterface $form_state, string $validate_component = '') { assert($entity instanceof EntityCeDisplayInterface); $form_values = $form_state->getValues(); if ($this->entity instanceof EntityWithPluginCollectionInterface) { @@ -588,18 +537,12 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { $form_values = array_diff_key($form_values, $this->entity->getPluginCollections()); } - // 'Rename' components, in a way that components can switch names. - $this->renameEntityComponents($entity, $form, $form_state); - // Collect data for 'regular' fields. // @todo Change this structure in #3446485: $form keys will be component // names, not field names. Rename '#fields' (here and elsewhere)? foreach ($form['#fields'] as $field_name) { $values = $form_values['fields'][$field_name]; - $component_name = $values['name']; - if ($validate_component && $validate_component !== $component_name) { - continue; - } + $component_name = $field_name; if ($values['region'] == 'hidden') { $entity->removeComponent($component_name); @@ -608,13 +551,13 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { $options = $entity->getComponent($component_name); $options['formatter'] = $values['formatter']; $options['field_name'] = $field_name; + $options['name'] = $values['name']; $options['is_slot'] = (bool) $values['is_slot']; $options['weight'] = $values['weight']; $options['region'] = $values['region']; - // Update field settings only if the submit handler told us to. Ignore - // if we're being called to validate these settings. - if ($form_state->get('plugin_settings_update') === $field_name && !$validate_component) { + // Update field settings only if the submit handler told us to. + if ($form_state->get('plugin_settings_update') === $field_name) { $component_name = $this->getComponentNameFromFieldName($field_name); // getRenderer() needs basic properties to be updated. $entity->setComponent($component_name, $options); @@ -640,60 +583,37 @@ class EntityCustomElementsDisplayEditForm extends EntityDisplayFormBase { } /** - * Renames CE display components according to form state info. - * - * This method must be called exactly once on $this->entity, each time a full - * form is submitted (and rebuilt on AJAX submit, or submitted/saved). - * Calling this more than once - * - may result in data corruption if a component is renamed to another - * existing component (if that component is also renamed at the same time; - * otherwise a validation error occurs and this is supposedly not called). - * - will result in warnings (but no data corruption), otherwise. (We can - * therefore assume that data corruption never occurs if no warnings are - * ever logged.) - * - * @param \Drupal\custom_elements\Entity\EntityCeDisplayInterface $entity - * The CE display whose components to rename. The component names must be - * equal to the 'existing_name' form state values, and will get new names - * if applicable. - * @param array $form - * A nested array of form elements comprising the form. - * @param \Drupal\Core\Form\FormStateInterface $form_state - * The current state of the form. - * - * @todo This setup fails when doing any editing after a rename. It should - * probably be replaced by a setup that keeps the original name in - * $this->entity (with all other changed config) and only does the renaming - * on 'real' submit. + * {@inheritdoc} */ - protected function renameEntityComponents(EntityCeDisplayInterface $entity, array $form, FormStateInterface $form_state) { - // 'Rename' components, in a way that components can switch names. - $new_components = []; - $form_values = $form_state->getValues(); - // @todo Change this structure in #3446485: $form keys will be component - // names, not field names. Rename '#fields' (here and elsewhere)? - foreach ($form['#fields'] as $field_name) { - $values = $form_values['fields'][$field_name]; - $component_name = $values['name']; - if ($values['region'] != 'hidden') { - if (isset($values['parent_wrapper']['existing_name'])) { - $existing_name = $values['parent_wrapper']['existing_name']; - if ($existing_name && $component_name !== $existing_name) { - // Old component can be empty (and populated later) if it used to - // be hidden. - $new_components[$component_name] = $entity->getComponent($existing_name) ?? []; - $entity->removeComponent($existing_name); - } - } - else { - // This should never happen. Are we being called twice? - $this->logger('custom_elements')->warning('Field row for @field_name somehow has no existing-name. (This suggests it is being renamed more than once.)', ['@field_name' => $field_name]); - } + public function save(array $form, FormStateInterface $form_state) { + $components = $this->entity->getComponents(); + // Components are keyed by the form's 'fixed row value'. Key them by + // component name as defined the schema. + $remove_components = array_fill_keys(array_keys($components), TRUE); + $set_components = []; + foreach ($components as $form_row_name => $component) { + $component_name = $component['name']; + if (isset($set_components[$component_name])) { + // Duplicate name; the first one is more likely to be the one we want. + // Do not cancel; just don't save. + // @todo figure out duplicate name detection after #3446485. + $this->messenger()->addWarning( + $this->t('Several components have the key %name. The component for row %row_name was not saved.', + ['%name' => $component_name, '%row_name' => $form_row_name] + )); + continue; } + + unset($component['name']); + $this->entity->setComponent($component_name, $component); + unset($remove_components[$component_name]); + $set_components[$component_name] = TRUE; } - foreach ($new_components as $component_name => $component) { - $entity->setComponent($component_name, $component); + foreach (array_keys($remove_components) as $key) { + $this->entity->removeComponent($key); } + + return parent::save($form, $form_state); } /**