Follow up for #2468045: When deleting a content type field, users do not realize the related View also is deleted.

When removing config display a message or add a 'disabled' fieldset that shows any disabled config.

Currently only a message is logged but no other feedback is given to the user that a config has been disabled. Currently this is grouped under the 'updated' config. More precise feedback would probably be better.

CommentFileSizeAuthor
#12 test-only-2832558-12.patch1.74 KBAnonymous (not verified)
#12 without-refactoring-2832558-12.patch4.8 KBAnonymous (not verified)
#12 2832558-12.patch8.65 KBAnonymous (not verified)
#11 interdiff-together-separately.txt868 bytesAnonymous (not verified)
#11 separately.png14.34 KBAnonymous (not verified)
#11 separately-2832558-11.patch7.17 KBAnonymous (not verified)
#11 together.png15.26 KBAnonymous (not verified)
#11 together-2832558-11.patch6.91 KBAnonymous (not verified)
#2 2832558-2.patch18.18 KBAnonymous (not verified)
#2 interdiff-2468045-86--2832558-2.txt5.1 KBAnonymous (not verified)
#2 2832558-2-refactoring.patch12.05 KBAnonymous (not verified)
#2 interdiff-2-refactoring.txt7.61 KBAnonymous (not verified)
#3 2832558-3-refactoring.patch22.08 KBAnonymous (not verified)
#8 interdiff-3-8-update_and_disable.txt912 bytesAnonymous (not verified)
#8 update_and_disable-2832558-8.patch21.93 KBAnonymous (not verified)
#8 interdiff-3-8-disable_save.txt721 bytesAnonymous (not verified)
#8 disable_save-2832558-8.patch22.78 KBAnonymous (not verified)

Issue fork drupal-2832558

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Lendude created an issue. See original summary.

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review
StatusFileSize
new 2832558-2.patch18.18 KB
new interdiff-2468045-86--2832558-2.txt5.1 KB
new 2832558-2-refactoring.patch12.05 KB
new interdiff-2-refactoring.txt7.61 KB

First of all, @Lendude thank you very much for luxurious quality work over #2468045: When deleting a content type field, users do not realize the related View also is deleted!

As a start for this issue I can offer the port part of my #77. For health reason it already contains the 2468045-86.patch.

Also for reduce c/p problem in addDependencyListsToForm(), we can bit refactoring this method. Because update part has only one different with delete part

      foreach ($entity_types as $entity_type_id => $label) {
        if (isset($form['entity_deletes'][$entity_type_id])) { # this check!

and using this check in all cases (update, disable, delete) is not problem.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 2832558-3-refactoring.patch22.08 KB

Wrong patch-refactoring. Sorry!

The last submitted patch, 2: 2832558-2.patch, failed testing.

The last submitted patch, 2: 2832558-2-refactoring.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 3: 2832558-3-refactoring.patch, failed testing.

Anonymous’s picture

Anonymous (not verified) commented

I forgot to save 'disable' group. Like

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
@@ -595,6 +595,11 @@ public static function preDelete(EntityStorageInterface $storage, array $entitie
      foreach ($config_entities['disable'] as $dependent_entity) {
        $dependent_entity->save();
      }

But may be better adding 'disable' entities to 'update' group too? This is more in harmony with $changed = TRUE and $disable = TRUE:

--- a/core/lib/Drupal/Core/Config/ConfigManager.php
+++ b/core/lib/Drupal/Core/Config/ConfigManager.php
@@ -342,14 +342,12 @@ public function getConfigEntitiesToChangeOnDependencyRemoval($type, array $names
           }
         }
         if ($fixed) {
+          $return['update'][] = $dependent;
+          $update_uuids[] = $dependent->uuid();
           if($dependent instanceof DependentWithRemovalPluginInterface && !$dependent->status()){
             $return['disable'][] = $dependent;
             $disable_uuids[] = $dependent->uuid();
           }
-          else{
-            $return['update'][] = $dependent;
-            $update_uuids[] = $dependent->uuid();
-          }
         }
       }
Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new disable_save-2832558-8.patch22.78 KB
new interdiff-3-8-disable_save.txt721 bytes
new update_and_disable-2832558-8.patch21.93 KB
new interdiff-3-8-update_and_disable.txt912 bytes

Patches for #7 variants

The last submitted patch, 8: disable_save-2832558-8.patch, failed testing.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new together-2832558-11.patch6.91 KB
new together.png15.26 KB
new separately-2832558-11.patch7.17 KB
new separately.png14.34 KB
new interdiff-together-separately.txt868 bytes

Patches are applied after #2468045-141: When deleting a content type field, users do not realize the related View also is deleted.

Together: View shows in both sections:
View in both sections

Separately: View shows in only disable section:
View in only disable section

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 2832558-12.patch8.65 KB
new without-refactoring-2832558-12.patch4.8 KB
new test-only-2832558-12.patch1.74 KB

Failure tests for variant where View shows in update and disable sections ('together'). And patch without refactoring (just c/p deletes section code) of addDependencyListsToForm, if refactoring can be out scope.

Patches still are applied only after #2468045-141: When deleting a content type field, users do not realize the related View also is deleted.

The last submitted patch, 12: 2832558-12.patch, failed testing.

The last submitted patch, 12: without-refactoring-2832558-12.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 12: test-only-2832558-12.patch, failed testing.

xjm’s picture

xjm
she/her
Primary language English
commented

I'm very concerned about introducing/generalizing this pattern of support for disabled config in an issue that is not focused on that. While there are a few types of config (like Views) that support a disabled behavior, in general, Drupal 8 has moved away from disabled things because it's a difficult usability pattern and causes all sorts of problems for data integrity and user expectations. So I'm really hesitant to generalize a pattern of disabled config like this at all.

For me, this issue might be wontfix, and we should instead focus on how to actually communicate to users what config is being updated or deleted and (for the specific usecase of Views disabling something) what the module's message about the updated config is.

As stated in #2468045: When deleting a content type field, users do not realize the related View also is deleted, I also think this should have been fixed in that issue. The issue was specifically intended to fix a usability problem, so splitting off the usability component of the fix breaks the UX gate.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Parent issue: » #2468045: When deleting a content type field, users do not realize the related View also is deleted
Related issues: +#2872149: All configuration entities can be disabled but many don't actually define what, if anything, that that means

I've created #2872149: All configuration entities can be disabled but many don't actually define what, if anything, that that means for the wider discussion on disabled configuration entities.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

vlad.dancer’s picture

vlad.dancer
Primary language Ukrainian
Location Kyiv
commented

For me, this issue might be wontfix, and we should instead focus on how to actually communicate to users what config is being updated or deleted

@xjm oh, really? Thank you, but no, we shouldn't!

I lost recently a day of local work while just tried to use views_natural_sort module in my views.
The same as here https://www.drupal.org/project/better_exposed_filters/issues/2992372#com...
And don't say me that I should backup such things!

I'm sorry but I feel myself very upset.
I wish nobody never get into this situation.

Why not just introduce "trash bin" concept without restoring, just to store any config deletions! Let's say "config_deleted" or "config_r__0ab2f8e96b" and so on?

pcrats33’s picture

pcrats33 commented

First timers are almost 100% not likely to understand the extent that they will lose their views. In my case, I lost the built in Content view that lets me search content from the content administration page. It was a bear to eyeball-copy/create it from deployed backup. My other view had a clone just like it. Still it's a recipe for disaster. Is there a better way to extract/import views? If the infrastructure doesn't play well with disabling views.. can you make a feature to export a single view to json and import it back into any site(with matching keys) from json? Perhaps export all deleted views to a .json file for backup? Or have a generic placeholder field you can replace the deleted field with to keep the view from (i'm assuming) crashing, and not delete the view?

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

guymandude’s picture

guymandude commented

I just came across this as a very recent user to D9/10. I deleted a field from a Content Type and it broke the Views that were using it. Very annoying to say the last, but more importantly it's a recipe for disaster. I also found I cannot simply edit the broken View and remove the same field from it to fix it quickly. The old fields are not visible on the View edit page any more!

The View was disabled which was helpful to understand what had happened, but can you not configure it such that a reference to the broken field is still visible allowing easy removable from the Views edit page?

ressa’s picture

ressa
he/him
commented
Related issues: +#3112005: View should not be deleted on deletion of content type or uninstall of contrib module, even with a confirmation form

I feel with the last three commenters, which confirms my thinking -- giving only an easily missed warning for the deletion of a View, instead of a hard blocker á la when uninstalling Forum module, is a recipe for disaster, and I just posted this in #3112005: View should not be deleted on deletion of content type or uninstall of contrib module, even with a confirmation form:

Whenever a new Drupal user encounters a silently deleted View, after spending many hours, or maybe even days painstakingly building it, it's a disaster. The majority of these new users probably don't use version control, and commit every so often, so the hard work is basically lost for good. Even if they did, restoring it could be difficult. I wouldn't be surprised if some new users get so fed up with Drupal, that they leave it behind, or at least are left with a bitter taste in their mouth.

Also, saw this in #2468045: When deleting a content type field, users do not realize the related View also is deleted:

Drupal Views

meeni_dhobale made their first commit to this issue’s fork.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.