Problem/Motivation

While working on #2442757: Move the field storage deletion if the last field has been removed to the UI layer, we noticed that we only have to fix the missing UI output to fix the critical part of this issue, so extracting this into a separate issue, will set that back to normal after this.

Proposed resolution

Expand the field delete form and related base class/trait to support more than one config being deleted, and some changes to make it easier to alter the list as it adds the field that is deleted to the output, which is confusing, so we want to remove that.

Remaining tasks

Not yet sure about the changes to ManageFieldsTest, might extract that method into a separate test to avoid affecting the existing test.

User interface changes

API changes

Data model changes

Files: 
CommentFileSizeAuthor
#5 2575605-field-storage-delete-2575605-5-interdiff.txt9.25 KBBerdir
#5 2575605-field-storage-delete-2575605-5.patch12.96 KBBerdir
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,401 pass(es). View
#2 tags_delete.png53.24 KBBerdir
#2 2575605-field-storage-delete-2575605-2.patch12.16 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,353 pass(es), 1 fail(s), and 0 exception(s). View
#2 2575605-field-storage-delete-2575605-2-tests-only.patch5.14 KBBerdir
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,085 pass(es), 4 fail(s), and 1 exception(s). View

Comments

Berdir created an issue. See original summary.

Berdir’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
5.14 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,085 pass(es), 4 fail(s), and 1 exception(s). View
12.16 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,353 pass(es), 1 fail(s), and 0 exception(s). View
53.24 KB

Here is the patch from the other issue with some additions like removing ourself and a test.

The test is very hacky right now but works. I think we should extract that into a separate test class, to avoid changing the field name and so on for the other test methods.

The last submitted patch, 2: 2575605-field-storage-delete-2575605-2-tests-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: 2575605-field-storage-delete-2575605-2.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
12.96 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,401 pass(es). View
9.25 KB

Forgot to uncomment the crucial line because I was testing if the test is failing as expected. It does ;)

Also moved the test to a separate class now.

This is ready for reviews.

The last submitted patch, 2: 2575605-field-storage-delete-2575605-2-tests-only.patch, failed testing.

The last submitted patch, 2: 2575605-field-storage-delete-2575605-2.patch, failed testing.

xjm’s picture

jhedstrom’s picture

I was concerned about the screenshot deleting the 'content' view when 'field_tags' are deleted, but I cannot produce that locally with, or without the patch--how does that arise? When I actually delete the tags field, the content view is still there.

The patch itself is looking good, assuming I'm not overlooking or missing something about the views config not being deleted.

Berdir’s picture

Sorry, I forgot to mention that I manually added the tags field to the content view to have an example dependency for testing :)

That does of course not happen with with the default content view only with views that actually depend on a field.

jhedstrom’s picture

Status: Needs review » Reviewed & tested by the community

I've tested this patch again with a view depending on a field, and it works as expected, and the code looks solid.

I am actually now more concerned that adding a field to a view, then deleting that field, deletes the view too--perhaps we need a major follow-up to instead swap in a broken handler into the view?

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 16b020e and pushed to 8.0.x. Thanks!

diff --git a/core/modules/field_ui/src/Form/FieldConfigDeleteForm.php b/core/modules/field_ui/src/Form/FieldConfigDeleteForm.php
index cb6e473..c38544d 100644
--- a/core/modules/field_ui/src/Form/FieldConfigDeleteForm.php
+++ b/core/modules/field_ui/src/Form/FieldConfigDeleteForm.php
@@ -53,9 +53,9 @@ public function buildForm(array $form, FormStateInterface $form_state) {
     $form = parent::buildForm($form, $form_state);
 
     // If we are adding the field storage as a dependency to delete, then that
-    // will list ourself as a dependency. That is confusing, so remove it.
-    // Recursively also remove the entity type and the whole entity deletions
-    // details element if nothing else is in there.
+    // will list the field as a dependency. That is confusing, so remove it.
+    // Also remove the entity type and the whole entity deletions details
+    // element if nothing else is in there.
     if (isset($form['entity_deletes']['field_config']['#items']) && isset($form['entity_deletes']['field_config']['#items'][$this->entity->id()])) {
       unset($form['entity_deletes']['field_config']['#items'][$this->entity->id()]);
       if (empty($form['entity_deletes']['field_config']['#items'])) {

Improved the documentation on commit.

  • alexpott committed a436fe4 on 8.0.x
    Issue #2575605 by Berdir, jhedstrom: Field delete form does not display...
yched’s picture

+1 for commit, @Berdir++

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.