It looks like core is going to provide a system for removing content entities: #2688945: Allow removing a module's content entities prior to module uninstallation.

This would mean we can remove the custom system we added in #2409673: impossible to uninstall Flag.

CommentFileSizeAuthor
#12 uninstall.png60.07 KBmartin107
#8 2712285-8.patch8.59 KBc.nish2k3
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

socketwench’s picture

socketwench CreditAttribution: socketwench at FFW commented

That'd be nice.

joachim’s picture

joachim CreditAttribution: joachim commented
Related issues: +#2688945: Allow removing a module's content entities prior to module uninstallation

Looks like #2688945: Allow removing a module's content entities prior to module uninstallation should make it into core fairly soon :)

joachim’s picture

joachim CreditAttribution: joachim commented
Title: Remove the uninstall system once Core takes care of it » Remove the uninstall system now that Core takes care of it
Status: Postponed » Active

It's in! hurrah!

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

In 8.2, so you might want to wait until 8.2.0 is out :)

joachim’s picture

joachim CreditAttribution: joachim commented

That's due in just over a week now -- October 5, 2016 according to this: https://www.drupal.org/core/release-cycle-overview

joachim’s picture

joachim CreditAttribution: joachim commented
Priority: Major » Normal
Issue tags: +Release blocker
c.nish2k3’s picture

c.nish2k3 CreditAttribution: c.nish2k3 as a volunteer commented
Status: Active » Needs review
FileSize
2712285-8.patch8.59 KB

Patch to remove the custom system as this is handled by core now.

joachim’s picture

joachim CreditAttribution: joachim commented 
+++ b/flag.links.task.yml
@@ -2,13 +2,3 @@ entity.flag.edit_form:
-entity.flag.collection:
-  title: 'List'
-  route_name: entity.flag.collection
-  base_route: entity.flag.collection

That looks like something we should be keeping... I'm surprised our tests pass without it!

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

No, not surprising. List and Clear all were the only local tasks on that page, if there is only one it is not shown anyway, you'd need at least two.

The only reason to keep it would be to make it easier to add some local tasks again later or letting other other modules do that.

joachim’s picture

joachim CreditAttribution: joachim commented

Ah right -- sorry, I misread those as being router items rather than tasks. Monday morning ... :/

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
uninstall.png60.07 KB

I think this is ready to go.

1) I agree with the idea behind this issue...
2) After a visual scan I can justify every change.

3) Manual testing:-
a) drush -y en flag_follower
b) create a article,
c) flag the node
d) go through flag_follower web page uninstall process see screen shot
e) complete everything has been deleted as expected.

+1 from me.

uninstall

  • joachim committed d9a6248 on 8.x-4.x authored by c.nish2k3 
    Issue #2712285 by c.nish2k3: Removed the uninstall system now that Core...
joachim’s picture

joachim CreditAttribution: joachim commented
Status: Reviewed & tested by the community » Fixed

Committed. Thanks everyone!

Status: Fixed » Closed (fixed)

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