[NodeBundle] Undo deleting pages

[JZuidema]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Fixed tickets

Some of our clients have run into the issue a couple times now, where they accidentally remove a page. Either because they thought they were removing a different page, or because they pressed the wrong button when canceling their delete action.
When it happens, we have to manually update the database to set the page to not being deleted anymore, since it doesn't seem like you can do it in the CMS.

So, I've added a bit of code for it:

On the page overview screen, the screen you see when you've clicked on 'Pages' in the left top, there is an extra button now: "View deleted pages"

When you click on it, it'll show a list of pages you've deleted at one point. There is a button at the right top to navigate back to the pages view. The list doesn't show the deleted at date. It could be useful I guess, but the deleted at date is not being tracked right now, so it would have to be added.
The edit button will just send you to the edit view of the page, where as the undo button next to it will let you undo the deletion of the specific page.

If you choose to press edit on the page, you can also undo the deletion there

[ProfessorKuma]

Hi @, your PR needs some changes

• This PR seems to need a milestone of a minor release.

[ProfessorKuma]

Hi @, your PR passed all our requirements.

Thank you for contributing!

[Numkil]

Hey @JZuidema. Thank you for your contribution. I have read your PR and tested in a local environment and i had a few things that might need looking at.

The adminlist of deleted nodes does not seem to be limited to the current domain when having the multidomainbundle activated. I think this is quite important because otherwise it will become very confusing for which domains you are restoring pages, especially if they have identical titles.

De button for restoring a page after it has been deleted should probably set that page in status Unpublished so that it doesn't immediately go live if it was published when it was deleted. Maybe a modal that shows a warning to the user on pressing the button might be helpful too.

There was a small problem with buttons overlapping on my small laptop screen

Thanks for taking a look at these things.

[JZuidema]

@Numkil, thanks for replying!

The adminlist of deleted nodes does not seem to be limited to the current domain when having the multidomainbundle activated. I think this is quite important because otherwise it will become very confusing for which domains you are restoring pages, especially if they have identical titles.
I'm confused, this PR allows you to undo deleting the node, which affects all pages in all languages (just like when you delete a page, it deletes it for all languages).

De button for restoring a page after it has been deleted should probably set that page in status Unpublished so that it doesn't immediately go live if it was published when it was deleted.
Good point, I've added this into the PR

Maybe a modal that shows a warning to the user on pressing the button might be helpful too.
Agreed. The modal shows when trying the undo deleting the page in the overview as well as on the page itself.

There was a small problem with buttons overlapping on my small laptop screen
I do not seem to be able to reproduce this, how can you get it to overlap? Also, I'm not even sure how that button gets there, it should be a button in the dropdown list. Like this:

[Numkil]

@JZuidema I think i had made a mistake in the styling of my demo project. It is displayed correctly now.

With the multidomain i meant this feature to manage multiple domains/websites with 1 cms.
https://kunstmaanbundlescms.readthedocs.io/en/latest/cookbook/using-the-multi-domain-bundle/

Because the pages are managed seperatly depending on which domain you are currently managing it would be confusing if the list for deleted nodes does not follow the same logic.
for example you might have a Page 'disclaimer' on both domain1.kunstmaan.be and domain2.kunstmaan.be. Both of these could be managed by a single cms using the multidomain bundle. So if you delete the disclaimer page on both these domains using the ui to change which domain you are managing. Then you would see 2 times a disclaimer page in the list of deleted nodes and you would not know which page belonged to domain1 and which to domain2 unless you restored them.

[JZuidema]

Hi @Numkil

I have never used the multi domain bundle, so I've added it into one of our projects. I'm not entirely sure how to fix the problem you're describing. From what i can see, nodes do not relate to a domain, even when this bundle is active, correct?
Which means if you delete a page (and therefore the node) on one of the domains, you'll delete the page for all the domains. Therefore it would only show up once in the list of deleted pages, right? Or is there something I'm not seeing?

[Numkil]

@JZuidema

When enabling the multidomain bundle you have to make a new Homepage for every domain.
The seperation of nodes in the tree works by only showing the children of the configured Homepage in the multidomain configuration. Because of that no node will ever appear multiple times over the multidomains.
Therefore when going to the list of deleted nodes when multidomain is enabled it should only show nodes that have the configured Homepage as their parent directly or indirectly.

This can be achieved with the following code.
https://github.com/Kunstmaan/KunstmaanBundlesCMS/blob/master/src/Kunstmaan/NodeBundle/AdminList/NodeAdminListConfigurator.php#L254

[JZuidema]

@Numkil thanks for clarifying, it's clear now :). I'll change the code, hopefully somewhere next week.

[JZuidema]

@Numkil actually I just realised it's quite an easy fix. Instead of overwriting the where's, I've parameterized the deleted value.
So all of your feedback should be handled now

[Numkil]

@JZuidema i made a single suggestion that is necessary for the multi domain to work. Otherwise this feature works well and looks good.

@acrobat i think this pull request looks ready for merging as soon as my suggestion is commited in the code.

[JZuidema]

@acrobat I've approved @Numkil's suggestion

[acrobat]

Maybe the spacing between the buttons should be tweaked a bit.

Also I'm thinking if it would make sense to have a config option to enable/disable restoring deleted pages?

Also the multidomain check to split the deleted pages still needs to be added

[acrobat]

I would add a early return here if the node is not deleted. A possible case could be that you delete a parent with nested children and already restore a child before.

[JZuidema]

I've applied this

[JZuidema]

@acrobat I've applied your feedback about being able to restoring deleted pages, and I've applied the early return if the node is not deleted.

I'm not sure if your point about Maybe the spacing between the buttons should be tweaked a bit. applies, since the button only appears after adding a SimpleItemAction to the configurator; it's just normal functionality right?

You mention Also the multidomain check to split the deleted pages still needs to be added, but I've approved the the suggestion right? I'm not sure what you mean here.

[acrobat]

This still needs some work as the overview adminlist of deleted pages should only show pages of the current rootNode. For single domain websites is does indeed show all deleted pages, but in a muiltidomain setup with separate rootNodes you should only see the deleted pages of the current rootNode/Domain.

I'm removing the milestone so we finish this somewhere in 6.x

[svdv22]

Does anybody know what the current status/timeline for this PR is?