View should not be deleted on deletion of content type or uninstall of contrib module, even with a confirmation form

Is the affected site actually running Drupal 8.2.something? Its last release was 22 months ago.

This sounds like #2468045: When deleting a content type field, users do not realize the related View also is deleted.

I asked because you selected 8.2.x-dev for the version when creating the issue and I know this had been addressed in 8.4 or so. I've left a comment on #2468045: When deleting a content type field, users do not realize the related View also is deleted, which should draw attention to this issue.

Noticed something similar recently when deleting a product (variation?) type, which ended up trying to delete the view. I did get a warning though.

Yup, this is spot on, the delete dialog lists the primary dependencies, but not the secondary one it seems. And for some reason the 'remove the field and disable the View'-logic doesn't kick in either.

Added steps to reproduce this.

Bumping this to critical since this is a loss of data with no warning.

Unsure if this a config management thing or a views thing.

Here is a test only patch. It's not covering everything we want to cover here and might need to go in a different spot, it is mostly to illustrate the problem.

edit:

+++ b/core/modules/field_ui/tests/src/Functional/FieldUIDeleteTest.php
@@ -8,6 +8,7 @@
+use function mysql_xdevapi\getSession;

uhh phpstorm auto add my typo....

Hi there,

I tried to reproduce this issue ( D8 version 8.8.2).
Seems it works as expected. Please see the screenshot. I guess the issue could be closed.

@e.bogatyrev see the failing test in #8, this is not fixed

Hi @Lendude,

I didn't apply the patch, I verified this case from the clean Drupal 8.8.2

Updated the test only patch with the assertion that the view is shown in the deletion dialog, and that assertion fails too. Not sure how #12 was done, but my manual test confirm what the automated tests show: the view isn't shown and it gets deleted.

This just hit me in 9.1.x too.

Moving to 9.3.x, and it will need an updated test.

Re-roll 'test only patch' created for 9.3.x.

Fixing 'Custom command fail' issue, Removed 'Unused use statement' as per the warning mentioned below:

11 | WARNING | [x] Unused use statement

Please have a look and advise.

Also make a recommendation that a module cannot be uninstalled without potentially causing deletion upon uninstall.

I tested this on Drupal 10.1.x, standard install, and confirmed the problem still exists. I have updated the patch a bit. No fix though.

hmm. tricky thing, imho

If i delete the field, i got the View listed in the confirmation form, But to be updated, which makes sense.

If i delete the content type with the field, we do not have those dependencies "recursively", so we can get also the fields dependencies.
And the results, as issue says, it deletes the View - which i don't think it should happen, if we have another content types.

imho, It should the same as for field, the Dependent Views should be updated only.
Imagine there could be plenty of Content types and different Views, that actually wants to use this field, if exists.
It will delete all of the Views, it would be "nicer" to update them.

So this looks like a complex problem about removing Dependent Configs (or whatever).
Maybe unify the code used in the "background" and the one used for UI.
And i don't know if there is an existing issue, already.

Anyway, i think, this requires some discussions ... at least

So, i changed the status, maybe we'll have more questions to lead us to a plan, for start.

I can confirm this issue happens on Drupal 9.5.10. I recently deleted unused Vocabularys and suddenly the Admin Content view was deleted. I thought I was crazy because right before that I was deleting some unused content types and another set of views were deleted as well.

It seems that this issue is still happening in Drupal 10.1, as I've removed a content type and on drush cex I notice the entire view is deleted, which is not ideal because that view is still used in conjunction with other remaining content types.

I think the status here should be active to allow discussion of a solution.

I tested this on Drupal 10.1.6, standard install, and confirmed the problem still exists.

I'm still having this as well. Other config files successfully just remove the portion of the config that was mentioned, but specifically in Views, the entire view config file is deleted. This is a big bug.

I followed the steps to reproduce the issue but couldn't replicate it. The view appears in the list of all entities scheduled for deletion, which is the expected behavior.

If you have a view where the "Jobs" type is only one of several content types in a given view, the entire view is still deleted. Is that the expected behavior?

This is a major problem in D11.
If i develop a view in gui and then operate on content types and delete a content type used by the view, the view should NOT be deleted but maybe disabled. As features doesnt really work with D11 yet, it creates a disaster if many views uses the same content type and all of them must be recreated, manually.

Thats why its a major issue in D11

I dont know if this is a solution

use Drupal\views\Entity\View;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\EntityStorageException;

function mymodule_entity_predelete(EntityInterface $entity) {
    if ($entity->getEntityTypeId() === 'node_type') {
        $content_type_machine_name = $entity->id();

        // Load all views.
        $views = View::loadMultiple();
        foreach ($views as $view) {
            $changed = FALSE;

            // Loop through all displays in the view.
            foreach ($view->get('display') as $display_id => $display) {
                if (!isset($display['display_options']['filters'])) {
                    continue;
                }

                // Check if there is a filter for the content type.
                if (isset($display['display_options']['filters']['type']) &&
                    in_array($content_type_machine_name, $display['display_options']['filters']['type']['value'])) {

                    // Remove the specific content type filter (set to all content types).
                    $display['display_options']['filters']['type']['value'] = [];
                    $view->set('display', $display);
                    $changed = TRUE;
                }
            }

            // If changes were made, save the view.
            if ($changed) {
                $view->save();
                \Drupal::messenger()->addStatus(t('Updated view "@view" to remove dependency on content type "@type".', [
                    '@view' => $view->label(),
                    '@type' => $content_type_machine_name,
                ]));
            }
        }
    }
}

How about some log messages?

"view xy about to be deleted because content type x is being deleted" or "... by module xy uninstall."

"view xy's configuration export BEFORE changes: <export config>"

"view xy's configuration export Afterchanges: <export config>"

"view xy deleted by module xy uninstall. see previous log messages for configuration changes made."

I am getting this in Drupal 10 - it seems very strange behaviour that deleting a content type will cause Views to be deleted - I have a View which uses four content types and deleting one of these content types will cause this View with numerous displays to be deleted - why on earth would I want that? And in another case deleting content type X will cause View Y with about 30 displays to be deleted even though I don't think that content type X is used in View Y and there is no info given in the delete content type X warning to tell me where content type X is used in View Y so that I can remove content type X from the View.

It would be better not to delete Views when deleting a content type - or at least give us the option - do you want to delete Views A, B C ? Deleting a content type can effect how a View will display but this does not mean that the View should be deleted. Instead the Views UI could give a relevant warning - this View display is missing Field Z etc - so that the user has an option to correct / update / re-use the View. Or if I want to delete the View then I can do that in the Views UI.

I think I saw a View getting deleted, even after something as innocent as uninstalling an image formatter module (PhotoSwipe).

I agree with @maxilein, and would even take it a step further if possible, having robust stop guards. Ideally, a View should never be deleted automatically, and you should get the same message as when you try to uninstall the Forum module: You are required to delete all forum posts, containers and forums -- only then are you allowed to uninstall the Forum module.

From "Uninstall" page /admin/modules/uninstall

Forum
Provides discussion forums.
The following reasons prevent Forum from being uninstalled:

• To uninstall Forum, first delete all Forum content
• To uninstall Forum, first delete all Forums terms

Is something like below possible?

If you request to delete a content type, the user should be prompted:

The content type XYZ is used by View ZYX, please delete that View first, or remove the content type dependency in the View.

-- or --

The module Photoswipe is used as image formatter in View ZYX, please delete that View first, or remove the image formatter dependency in the View.

The warning given in #35 above is what I get in Drupal 10. But this warning is misleading because a user would naturally interpret this warning to mean that "View displays using only this content type will be deleted', not "View displays not using this content type will also be deleted"!!

@jennypanighetti

If you have a view where the "Jobs" type is only one of several content types in a given view, the entire view is still deleted. Is that the expected behavior?

This is what happens in Drupal 10 but surely it cannot be the intended behaviour. Is it a bug?

The obvious solution is that deleting a content type should not delete a View, or at least not without confirmation from the user - do you also want to delete View A, B, C - yes / no.

For now I am stuck with my unwanted content type because deleting the unwanted content type will also delete numerous Views that I don't want to delete.

Honestly, I think these warnings don't work as intended (sometimes it's an extremely long, cryptic list) and if the action have grave consequences, it should rather be a hard block, until the user either 1. Deletes the View -OR- 2. Removes the content type from the View.

Only then should the content type be allowed to be deleted, if no Views depends on it, thereby preventing it from getting deleted.

Please try and check out how the Forum module deals with this challenge (see comment #41), and see if that's not worth emulating here as well.

I just ran into this again. I installed the Views Filters Summary and used it in the Header of my View.

I decided to not use it, and uninstalled it, no warnings about the View:

$ drush un views_filters_summary
 [success] Successfully uninstalled: views_filters_summary

... and the View was gone as well.

Drush doesn't show config dependencies on module install, that's a drush issue that you should report there. I'm fairly certain that if you would use the drupal UI to uninstall that module, you *would* get a notice about that.

The critical part about this is "without notice" (within what core can control, which is the uninstall/config entity delete confirmation screen. Not Drush).

The same is true for the reports from @juc1. Per your screenshot, there *is* notice.

I do agree that there are issues and that views in general should by default just disable and not itself on any config dependency issue, mostly because views are complex and it can be a lot of work to recreate them, they tend to deal with broken and missing plugins fairly well and do have the mechanism to disable themself. That would solve the critical aspect of this issue.

But also again, this is mostly the config dependency system working as intended. If something depends on something else and doesn't report that it can autocorrect itself, it is getting deleted. That's by design. The alternative might be fatal errors or exceptions.

And for views, it is complex to autocorrect themself. Taking the example from @juc1, the job content type is deleted, and the view is named Jobs, so it almost certainly has a node type filter on jobs. If views would just remove that filter, it would result in showing all content, completely changing the intent of that view. So deleting the jobs view when the job node type is deleted seems perfectly valid.

Thanks for the thorough comment @berdir, and you are correct: You do get a warning, if you uninstall a contrib module PhotoSwipe used in a View via the GUI at /admin/modules/uninstall (see updated Issue Summary), as opposed to Drush. But it's not a blocker.

Interestingly, about the missing Views deletion notice (the original bug) I actually *do not* get a warning that a View will be deleted (see updated Issue Summary), so that's a pretty big problem. Maybe you can check if you get a warning or not?

Fundamentally, I think the correct solution would be to block unintended destructive consequences in the first place, by emulating the Forum module (see my comment #42, I added a reworded version in the Issue Summary) which has the ideal behaviour as I see it.

Disabling a View is not a great scenario, a hard block as Forum has would be better, and make everything much more transparent for the end user, since the user would proactively perform the required changes beforehand, and manually prepare the content type or contrib module for removal, by cleaning up the View beforehand, preparing it for the change.

As can be seen, the user do get warnings, so these potential changes are getting detected. So in that case, isn't it more or less a question of tweaking the actions for these events? I.e. instead of showing a warning ("About to delete XYZ view") we should instead demand deletion of the requirements in the View, before being able to proceed with the deletion of the content type, or uninstall of contrib module?

This would also make it automatically work in Drush, just like Forum does, where you get a hard block, and the same message as in the GUI.

Are you referring to Forum's uninstall validator?

That's the one, I recognize the messages, thanks @cilefen! I added a mention of it in the Issue Summary.

Fixing link.

Technically an uninstall validator in the views module could stop an uninstall if a view would be deleted, but it could do nothing for a content type deletion.

It sounds fantastic that it's possible to block a module uninstall, if it results in a View getting deleted.

Maybe you can describe the technical blockers, for not being able to do the same, when deleting a content type? What would it require, if we *needed* to make it work? Taking a helicopter perspective, we may have to put on our Gyro Gearloose thinking cap, and think up a global "Block this destructive action X, unless you first do Z"-function, that can be triggered by certain events, if that's required ...

My thinking is this:

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.

So I think we need to take this problem very serious, and find a solution, even if it is hard, and maybe not possible right now, with the current structure.

Is this related? https://www.drupal.org/project/drupal/issues/2575547

Assuming "patterns" refers to the patterns we encounter when modifying config, and defining a UI to support the decisions to be made.

@ressa This has been a critical bug for five years, which means it's being taken seriously.

Thanks @mortona2k, #2575547: Formalize patterns for delete/uninstall of configuration dependencies does look related, and I left a comment in that issue, and adding a few more related issues here, to connect them all.

And about users having a bad experience, getting a View deleted unexpectedly, please see the last few comments in #2832558: Add a 'disabled' section to config changes page when removing config. Like in my comment above (#52), the word "disaster" is used a few times ...

@cilefen: I don't mean to imply that this problem is not being taken seriously, that was not my intention at all.

I just think it's such a big problem that it needs as much focus as possible. I just now see in related issues, that work is being done, so that's very encouraging! Also new issues are getting created, that look promising.
EDIT: This is the recent one I found: #3520747: Don't delete view due to config dependencies being deleted

Still, I am very interested in hearing what you think about my questions and suggestions in my previous comment. Perhaps we can start out with doing the block which you mention is possible in a MR here, blocking an uninstall if a view would be deleted?

Then we can look at blocking a content type deletion, to save a View from getting deleted later ...

There are more scenarios we should take into consideration and it should always be the admin's responsibility how he wants to proceed.
Why? Because the approach of blocking the uninstall of a module is doable if you have a few views that you can adapt quickly. But if you have very many views and entity types on a large site the invalidation of them can be the approach with lesser pain and cost.
There are two problems:
Finding out which items are affected.
Fixing them.

Also in a dev environment you sometimes realize too late (once you have built lots of stuff with it) that a certain module is faulty or breaking some other much more important functionality so you have to be able to remove it easily.
Then invalidating items (views, entity fields,..) is the better approach, because it may break the site to some extend, but you can still access the configuration of the views and fix it. Invalidation just breaks parts of the site. While a faulty module that cannot be uninstalled can break everything.
If we offer an approach where the admin or dev can make the judgement regarding the path with the lesser pain - would be best in my opinion.

We need to be better in these things at least:
a) Show a warning: just listing the nodes in the gui before deletion is not good enough. If you have many views (100+) you cannot properly take notes for each of them. So also add one single(!) log message that lists all nodes including links to get there and if possible the particular information of configuration changes that will be made - if the admin procedes. This log message can be used as a task list for fixing things - even if the admin/dev does not procede and cancels deletion.
b) Create a log message the admin/dev can work with. see a) and create a very informative task list. Ideally something where I can derive a judgement of which effects the uninstall will have (e.g. is it just views, is it the display of entities, will widgets go missing, ...). We have all this information in the database, right?
c) let the admin/dev decide if he wants to delete OR invalidate affected entities - depending on his knowledge of the site.

Thanks for sharing your opinion @maxilein, but I fundamentally disagree.

The Drupal site builder/developer needs to attend to these Views, and fix them, regardless if it's 1 or 100. Also, probably only a minority of web site will have +100 Views, so spending time on such edge cases is not sound allocation of expensive developer resources, in my opinion, nor building complex logging solutions, or a complex delete OR invalidate feature.

Don't get me wrong, these could be fantastic tools to have, I just fear that building them would take a long time, and may drag on ... But they could of course be built in a follow up issue, for sure.

But the problem is here, today, and it has been this way for more than ten years, since Drupal 8 got launched in 2015. It's an extremely bad experience. Here are a few comments from #2832558: Add a 'disabled' section to config changes page when removing config, by users who got bit by this. I have seen similar heart breaking comments in other issues as well:

@vlad.dancer:

[...]
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.
[...]

@pcrats33:

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.
[...]

@guymandude:

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!
[...]

So I still believe that disabling View(s), or giving a warning is not the correct solution -- a hard block, like currently in Forum, would safe guard users from losing hours or days of work.

But this belief is based on an assumption, that a hard blocker solution is not extremely difficult to build, so it would be great to get an assessment of the difficulty, from a seasoned Drupal developer, with Views coding experience ...

@cilefen, @lendude or @berdir:

How hard would it be to add the uninstall blocker mentioned by @cilefen? If it's not extremely hard to build, a quick MR here could serve as a proof of concept, and we can take it from there?

Technically an uninstall validator in the views module could stop an uninstall if a view would be deleted [...]

I completely agree with you on this: The Drupal site builder/developer needs to attend to these Views, and fix them, regardless if it's 1 or 100.

But it should always be the developers decision on how to approach that.
If I want to get stuff out of a site immediately (for whatever reason) - it is my decision on how to deal with the fallout.
That's why transparency is key.

And since the current warning already lists impacted nodes it cannot be too difficult to provide some more meta information from inside that same routine.

I came to this issue thinking "how on earth did that possibly regress" but the title is wrong per #46 / #47. Updating the title to what the actual goal is.

I agree that deleting a whole view is potentially bad even with a warning, but we also don't want to make it impossible to uninstall modules. Some sites have hundreds of views and so manually editing every one is not feasible.

We did spend a long time before 8.0.0 deciding on exactly what we wanted this behavior to be for config dependencies in general, and the current behavior in HEAD is what we agreed on. We can't try to repair the view automatically because we don't know how the module's plugins or whatever are used and access bypasses etc. could be introduced.

BTW I debated bug vs. task (though it remains critical either way). I think enough people experience this as a very bad surprise that it should still be treated as a bug, even though there is technically a warning message and it is technically working as designed.

A better design in general might be to require a manual step beforehand to remove the config, similar to the content removal operation. That way the user has to think more about what they are doing. However, for Views, we have the option to disable the view. It's broken without its dependencies, but the dependencies could be restored from an export or from reinstalling a module etc.

Thanks for weighing in @maxilein and @xjm, I am glad to hear that we agree on many issues, and that you also consider this an important task, that needs to be solved.

I do think there's a slightly developer-oriented perspective in the Drupal core issue queue, where more empathy with non-developers could help find solutions which cater to these users, and their level of technical expertise.

Please understand, that I don't think this is done out of malignancy, it's a totally natural thing, to have a tendency to see things from your own perspective.

But very few new Drupal users use the configuration-system, or even source control. They click and configure, which is where Drupal REALLY shines. But when the View they spent a long time building is suddenly gone with a silent "poof", it's a critical disaster.

That's why I think it should be a hard blocker, and warning or disabling a View is not enough.

About the 100 Views scenario: I totally agree, let's allow them to go ahead, but it should be treated as the edge case it is, primarily relevant for advanced developers.

A better design in general might be to require a manual step beforehand to remove the config, similar to the content removal operation. That way the user has to think more about what they are doing.

That's a great idea! We could add something like this in settings.php, to allow overriding the "module uninstall/content type deletion" blocker?

/**
 * Allow Views deletion.
 *
 * If you delete a content type, or uninstall a module which a View relies on,
 * the View can also get deleted. Change the FALSE to TRUE to allow
 * deleting a View, as a side effect of configuration changes elsewhere.
 */
$settings['allow_views_deletion'] = FALSE;

We can't try to repair the view automatically because we don't know how the module's plugins or whatever are used and access bypasses etc. could be introduced.

I agree, that would probably require a lot of coding, and probably could never really function as expected anyway ... Blocking, and asking the user to get things in working order, would be a simple and sane solution.

About bug vs. task I also agree, the code is working as expected, but for the end user, it feels like a bug.

I agree, that would probably require a lot of coding, and probably could never really function as expected anyway ...

It's not that it would require a lot of coding. This is an official Security Team and release management statement that we already made an official decision, back in like 2014, that it was too dangerous from a security perspective to clean up the view the way we clean up other third-party config stuff. I am just documenting that past decision again for the record of this issue. 🙂

I don't necessarily agree that disabling the View is a non-option; we should discuss the pros and cons with design/usability and with the subsystem maintainers for Views and config.

If I delete my Article content type, I'd want to delete my Articles view.

If I have a Search view with multiple content types, and delete one, I'd like it to be removed from the filter.

If I uninstall a module that provided a view display... revert to safe default? Ask me to disable? Ask me what to change to?

I am dreaming of some kind of config quick fix UI where you get some kind of dialog for making necessary adjustments. I thought that's what #2575547 (formalize patterns) was about.

Disabling the view is ok, but sometimes they are recoverable, and sometimes they can't be edited without breaking, and take some dev hacks to fix.

Thanks for the explanation @xjm, that conclusion in 2014 does make a lot of sense, a sane decision!

And yes, let's discuss some more, though disabling the View method is still a too "round about" method as I see it, instead of a more direct method, of taking the bull by its horns, and dealing with the problem right away.

@mortona2k: About your vision of a quick fix UI, #2575547: Formalize patterns for delete/uninstall of configuration dependencies may be relevant, but the last comment was more than nine years ago, so it seems stalled ...

In each of your use cases, would it be an acceptable solution to let the user know there is a task to do, before being allowed to proceed? I would think it is easier to code, since it could be streamlined to behave the same way in each case ... Something like this, where we could link to the View in question:

Your View X is using the Y content type fields, please remove it from the View X before deleting the Y content type

Your View X is using the Y content type as Views filter, please remove it from the View X before deleting the Y content type

Your View X is using the Y Views Display formatter, please remove it from the View X before uninstalling the Y module

@ressa that would be fantastic, with the option to proceed and delete the view if I don't need it anymore.

Highlighting the part that will break is a huge step forward.

I was also considering disabling, but adding it as a note to the view config, ie disabled_reason: Content type article does not exist.