Problem/Motivation

When a module saves third-party settings on a view, uninstalling that module will delete the view (instead of just removing the third-party settings).

This exact problem is normally taken care of by \Drupal\Core\Config\Entity\ConfigEntityBase::onDependencyRemoval(). However, the Views entity class overrides the method but fails to call the parent (or fix the problem itself).

Steps to reproduce

  1. Write a small module that adds third-party settings to a view.
  2. Go to the “Uninstall” page and select your module.
  3. On the confirm form, you will see that the view will get deleted.

Proposed resolution

Unless there is a good reason not to do that, I would suggest just calling the parent method.

Remaining tasks

  • Agree on the solution.
  • Implement it.

User interface changes

None.

Introduced terminology

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

Issue fork drupal-3496867

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:

Comments

drunken monkey created an issue. See original summary.

drunken monkey’s picture

Status: Active » Needs review

My trivial suggested solution is implemented in this MR.
Let’s see whether the pipelines succeed, otherwise we might already know the reason why this is currently not done.

smustgrave’s picture

Status: Needs review » Needs work
Related issues: +#2992372: When module uninstall, view is deleted

This has been seen in better exposed filters, Probably getting test coverage can help show the problem and if the approach works

nicxvan’s picture

Issue tags: +Needs tests

If after all this time this was all that was needed...

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

dcam’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

I added a test.

smustgrave’s picture

Bummer doesn't actually solve the problem for better_exposed_filters but how does third_party_settings get added anyway?

dcam’s picture

Bummer doesn't actually solve the problem for better_exposed_filters but how does third_party_settings get added anyway?

Yeah, I noticed that, but forgot to mention it last night. Since BEF doesn't use third party settings to modify views, this fix won't apply to that problem.

Third party settings are applied to config entities via the setThirdPartySetting() function. I think most of the time you do that in a Hook or EventSubscriber.

drunken monkey’s picture

Thanks a lot for writing the test, looks great!
I just made a few minor changes, now I’d say this is RTBC.

smustgrave’s picture

Nit but should change the wording of the last commit. Comes off like someone owns this so not sure read right

nicxvan’s picture

I added a suggestion for that comment.

dcam’s picture

I chose:

Tests removing third-party settings when a provider module is uninstalled.

nicxvan’s picture

That is better!

smustgrave’s picture

Works for me. Probably good to self RTBC after

dcam’s picture

Status: Needs review » Reviewed & tested by the community

Right on.

berdir’s picture

Status: Reviewed & tested by the community » Needs review

Don't want to hold this up, but I think a follow-up to change views to _never_ delete due to config dependencies but just disable the view. There are lots of similar issues open where views get deleted due to a role or node type dependency in a condition that will not be fixed by this, this specifically only fixes third party settings.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

  • catch committed 73977554 on 11.2.x
    Issue #3496867 by dcam, drunken monkey, smustgrave, nicxvan:...

  • catch committed db6ddf92 on 11.x
    Issue #3496867 by dcam, drunken monkey, smustgrave, nicxvan:...

  • catch committed ee24f8d2 on 11.1.x
    Issue #3496867 by dcam, drunken monkey, smustgrave, nicxvan:...
catch’s picture

Version: 11.x-dev » 11.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, cherry-picked to 11.2.x and 11.1.x, thanks!

Status: Fixed » Closed (fixed)

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

xjm’s picture

Status: Closed (fixed) » Needs work
Issue tags: +Needs followup

Nice fix! I feel like this has some old duplicates or related issues out there having to do with config dependencies. These issues are close to my heart. We might want to run issues like this past config maintainers before commit.

There was also an important-sounding followup @berdir suggested in #17 that never got filed?

xjm’s picture

Title: Uninstalling a module deletes all views that have third-party settings by that module » [already committed; needs followup] Uninstalling a module deletes all views that have third-party settings by that module
catch’s picture

@xjm I think that's the one smustgrave opened and linked to in #18?

dcam’s picture

Title: [already committed; needs followup] Uninstalling a module deletes all views that have third-party settings by that module » Uninstalling a module deletes all views that have third-party settings by that module
Status: Needs work » Fixed
Issue tags: -Needs followup

Yes, that's the follow-up. @smustgrave quoted @berdir from #17 in #3520747: Don't delete view due to config dependencies being deleted. I'm setting this back to Fixed.

Status: Fixed » Closed (fixed)

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