Implement Recycle Bin by rohnsha0 · Pull Request #4168 · plone/Products.CMFPlone

rohnsha0 · 2025-04-29T14:02:27Z

ref #2966

to be merged together:-

plone.base - #81
plone.app.discussion - #264

replaces #4166

…tions Control Panel

Co-authored-by: Steve Piercy <web@stevepiercy.com>

mister-roboto · 2025-04-29T14:02:39Z

@rohnsha0 thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

stevepiercy

Refactor recyclerbin to recyclebin. I missed this on my previous pass.

Products/CMFPlone/controlpanel/browser/configure.zcml

Products/CMFPlone/controlpanel/browser/recyclerbin.py

Products/CMFPlone/profiles/dependencies/registry.xml

Products/CMFPlone/recyclebin.py

Products/CMFPlone/tests/test_recyclebin.py

stevepiercy · 2025-04-29T19:46:17Z

@rohnsha0 can you check the failing CI checks from meta? Let us know if you have any questions about it.

stevepiercy · 2025-04-29T19:57:35Z

@rohnsha0 here's one problem.

https://github.com/plone/Products.CMFPlone/actions/runs/14733219302/job/41352593835?pr=4168#step:7:70

error: cannot format Products/CMFPlone/browser/recyclebin.py: Cannot parse for target version Python 3.8: 77:67:         message = f"{restored_count} item{'s' if deleted_count != 1} restored successfully."

I think we need to change that, as it appears to have support only since Python 3.12, according to PEP 701. Bummer.

Confirmed by https://github.com/plone/Products.CMFPlone/actions/runs/14733219302/job/41352596237?pr=4168#step:7:773.

jensens · 2025-05-09T09:32:23Z

@ale-rt i noticed only core presentations are placed in p.a.layout. i did not find any controlpanel thing in there... i am not super sure what to do!

Controlpanel goes into https://github.com/plone/Products.CMFPlone/tree/master/Products/CMFPlone/controlpanel/browser

…onal child processing

davisagli · 2025-05-11T01:43:53Z

Hi @rohnsha0 -- thanks for your diligent work so far.

Tomorrow I plan to test some edge cases. I will also record a video to show to a UX expert our company works with, to see if he has any suggestions.

…ring restore

davisagli · 2025-05-11T23:37:44Z

@rohnsha0 Here are the issues I found while testing:

rohnsha0 · 2025-05-17T04:28:59Z

@davisagli I have opened all PRs that fixes the given issues:-

Recycle bin: can't delete an item again after it was restored #4170 ➡️ Fix deletion of items restored from recycle bin in Actions menu plone.app.content#305
Recycle bin: Can't restore item if it's original container no longer exists #4171 ➡️ [RECYCLE BIN] Fixes broken redirection after restoration and handle value-error gracefully if parent is deleted #4175
Recycle bin: can't delete item with a preview image #4172 ➡️ Fix deleting for image set as preview image in page plone.volto#195
Recycle bin: allow non-admins to access the recycle bin? #4173 ➡️ [RECYCLE BIN] Implement permission for accessing recyclebin #4176

davisagli · 2025-05-29T05:47:34Z

Products/CMFPlone/recyclebin.py

+        try:
+            settings = self._get_settings()
+            return settings.recycling_enabled
+        except (KeyError, AttributeError):


This can raise zope.interface.interfaces.ComponentLookupError when this is called while the Plone site is being deleted, so we should also catch that here.

Traceback (innermost last): Module ZPublisher.WSGIPublisher, line 181, in transaction_pubevents Module ZPublisher.WSGIPublisher, line 390, in publish_module Module ZPublisher.WSGIPublisher, line 284, in publish Module ZPublisher.mapply, line 98, in mapply Module ZPublisher.WSGIPublisher, line 68, in call_object Module OFS.ObjectManager, line 560, in manage_delObjects Module OFS.ObjectManager, line 417, in _delObject Module zope.event, line 33, in notify Module zope.component.event, line 27, in dispatch Module zope.component._api, line 146, in subscribers Module zope.interface.registry, line 458, in subscribers Module zope.interface.adapter, line 918, in subscribers Module zope.component.event, line 37, in objectEventNotify Module zope.component._api, line 146, in subscribers Module zope.interface.registry, line 458, in subscribers Module zope.interface.adapter, line 918, in subscribers Module Products.CMFPlone.events, line 60, in handle_content_removal Module Products.CMFPlone.recyclebin, line 164, in is_enabled Module Products.CMFPlone.recyclebin, line 156, in _get_settings Module zope.component._api, line 180, in getUtility zope.interface.interfaces.ComponentLookupError: (<InterfaceClass plone.registry.interfaces.IRegistry>, '')

done in fa502d9

… deleting Plone Site

stevepiercy

minor tweaks and a general question about translations and localization of various forms of plurality

stevepiercy · 2025-06-01T11:30:10Z

Products/CMFPlone/browser/recyclebin.py

+
+        message = translate(
+            _(
+                "${count} item(s) restored successfully.",


I know there's a way to use plurals intelligently in translations, but I don't know what that would be. Here's my clueless English attempt.

There might also be something else to handle plurals in translations, but my memory fails me. Here is some reference material.

https://learn.microsoft.com/en-us/globalization/internationalization/pluralization

https://phrase.com/blog/posts/pluralization/

Perhaps @erral has a better idea of how to support localized singular and plural forms?

Suggested change

"${count} item(s) restored successfully.",

"${count} item{count != 1 ? "s"} restored successfully.",

This pluralisation does not work. I tried it, and I just literally get this in the browser:

1 item${count != 1 ? 's'} restored successfully.

When I extract messages with i18ndude, the po files contain this:

msgid "${count} item${count != 1 ? 's'} restored successfully."

You can't really translate that.

zope.i18n has support for plural messages, but this has not been hooked up in Plone as far as I know. Also, it would need changes in i18ndude.

So: it would be nice to have pluralisation at some point, but currently it does not work.

stevepiercy · 2025-06-01T11:43:30Z

Products/CMFPlone/browser/recyclebin.py

+
+        message = translate(
+            _(
+                "${count} item(s) permanently deleted.",


Flagging for pluralization.

stevepiercy · 2025-06-01T11:44:01Z

Products/CMFPlone/browser/recyclebin.py

+
+        message = translate(
+            _(
+                "Recycle bin emptied. ${count} item(s) permanently deleted.",


Flagging for pluralization.

stevepiercy · 2025-06-01T11:56:39Z

Products/CMFPlone/browser/templates/recyclebin.pt

+                    <div class="d-flex justify-content-between align-items-center flex-wrap gap-2">
+                        <div class="btn-group" role="group">
+                            <button type="submit" name="form.buttons.restore" class="btn btn-primary" i18n:translate="">
+                                <span class="ms-1">Restore Selected</span>


Suggested change

Restore Selected

Restore selected

stevepiercy · 2025-06-01T11:56:49Z

Products/CMFPlone/browser/templates/recyclebin.pt

+                                <span class="ms-1">Restore Selected</span>
+                            </button>
+                            <button type="submit" name="form.buttons.delete" class="btn btn-danger" i18n:translate="">
+                                <span class="ms-1">Delete Selected</span>


Suggested change

Delete Selected

Delete selected

stevepiercy · 2025-06-01T11:57:17Z

Products/CMFPlone/browser/templates/recyclebin.pt

+                                    class="btn btn-outline-danger"
+                                    onclick="return confirm(recyclebinEmptyMessage);" 
+                                    i18n:translate="">
+                                <span class="ms-1">Empty Recycle Bin</span>


Suggested change

Empty Recycle Bin

Empty recycle bin

stevepiercy · 2025-06-01T12:02:04Z

Products/CMFPlone/recyclebin.py

+                return f'Comment thread: "{comment_preview}" ({comment_count} comments)'
+            else:
+                return f"Comment thread ({comment_count} comments)"


We have two possible plurals to handle here.

… bin. This avoids having a strange acquisition chain after the item is restored, which then causes a 'has already been deleted' message if you try to delete it again.

davisagli · 2025-06-17T05:54:34Z

@rohnsha0 Here's another thing that didn't quite work like I expected: I created a folder with a page inside, and deleted the folder. Then in the recycle bin I selected the folder and clicked the "Delete Selected" button to delete it permanently. I expected that this would include all children, but afterward the recycle bin still contained the page.

rohnsha0 · 2025-06-17T08:20:40Z

@rohnsha0 Here's another thing that didn't quite work like I expected: I created a folder with a page inside, and deleted the folder. Then in the recycle bin I selected the folder and clicked the "Delete Selected" button to delete it permanently. I expected that this would include all children, but afterward the recycle bin still contained the page.

@davisagli I fixed it, tested it locally and it now works

edit: also included a testcase covering the same!

rohnsha0 · 2025-07-10T13:22:39Z

@jenkins-plone-org please run jobs

davisagli · 2025-08-24T22:07:38Z

@rohnsha0 I discussed your implementation with our UX expert, Dominique Steppeler. He had the following suggestions:

When deleting an item, if the recycle bin is enabled, the delete confirmation dialog should have a message saying that the content can be restored by admins, but will be deleted permanently after X days.
The UI would be a bit easier to understand if we didn't always show the option to restore to a different path. Instead we could do one of the following options:
- a. Only prompt the user for a different path after they try to restore to the original location but it is not available.
- b. Always restore content to a “restored content” folder instead of to where it was deleted
In some cases, it might be best if content is restored in its initial workflow state (usually "draft") rather than the workflow state it was in when it was deleted. This could be a setting in the recycle bin control panel.
To avoid confusion, we should change the icon for the Undo control panel so that it is more of a normal undo icon, not a trash bin.
Additional information that could be useful to show for each item:
- preview image for content that has one
- deleted by (username who deleted the item)
- sub-items (number of contained items)
- workflow state (i.e. published, private)
Additional filters that could be useful for finding the right items to restore:
- date range for when it was deleted
- “deleted by” (user who deleted it)
- checkbox for whether it has sub-items
- language
- workflow state

What do you think?

…alue-error gracefully if parent is deleted (#4175) * feat(recyclebin): enhance restoration process with error handling for missing target containers * feat(recyclebin): enhance redirect logic after restore with URL validation * fix failing tests * refactor(recyclebin): lint * changelog * fix(recyclebin): improve parent existence check for comments and discussion items * refactor(recyclebin): remove obsolete bugfix for ValueError handling and redirection after parent deletion * refactor(recyclebin): simplify redirection logic after item restoration by using context state * Store object without acquisition wrapper; reindex it when restored --------- Co-authored-by: David Glick <david@glicksoftware.com>

rohnsha0 · 2025-09-21T10:21:19Z

replaced by #4223

rohnsha0 · 2025-09-21T10:21:51Z

@davisagli I have implemented your suggested changes, please verify those in the new PR

rohnsha0 and others added 12 commits April 29, 2025 19:27

Add Recycle Bin control panel and settings configuration

647faa7

Implement Recycle Bin functionality with management views and settings

5f7c305

Add Recycle Bin action to actions.xml

ad85ec8

lint

3389289

Remove obsolete recycle bin event handler code

2b6b7e4

changelog

ad77b73

add tests

f4de4c0

Fix action index in test cases for adding and changing category in Ac…

7a9094c

…tions Control Panel

Refactor button classes in recycle bin template for consistency

52c9e1f

remove dedundant format_date logic

9534b79

ui cleanup

b8cebee

Apply suggestions from code review

5c65330

Co-authored-by: Steve Piercy <web@stevepiercy.com>

rohnsha0 requested review from ale-rt, davisagli, gforcada, jensens, mauritsvanrees, pbauer, petschki, sneridagh, tisto and tkimnguyen April 29, 2025 14:02

rohnsha0 mentioned this pull request Apr 29, 2025

Implement Recycle Bin #4166

Closed

implement z3c.form

5687b42

stevepiercy requested changes Apr 29, 2025

View reviewed changes

rohnsha0 added 2 commits April 30, 2025 09:25

fix(recyclebin): added support for discussions

28aab1a

feat(recyclebin): enhance comment tree handling with meaningful titles

07fc9b4

rohnsha0 added 2 commits May 9, 2025 18:45

feat(recyclebin): add process_children parameter to add_item for opti…

da965d4

…onal child processing

lint

8c8fb47

rohnsha0 mentioned this pull request May 11, 2025

Unstable behavior from mr.roboto plone/mr.roboto#168

Open

fix(recyclebin): improve error message for missing target location du…

7079b08

…ring restore

rohnsha0 mentioned this pull request May 12, 2025

Fixes TypeError while deleting image, added as preview image in other Page #4174

Closed

davisagli reviewed May 29, 2025

View reviewed changes

fix(recyclebin): dont break when checking is_enabled-recyclebin while…

fa502d9

… deleting Plone Site

rohnsha0 requested a review from davisagli May 30, 2025 02:02

stevepiercy requested changes Jun 1, 2025

View reviewed changes

Remove Acquisition wrapper from object when storing it in the recycle…

be104f9

… bin. This avoids having a strange acquisition chain after the item is restored, which then causes a 'has already been deleted' message if you try to delete it again.

fix(recyclebin): fix childrens not being purged for folders

0d8815c

Merge branch 'master' into rohnsha0-plip-recyclebin

05f5f7d

This was referenced Sep 16, 2025

PLIP: implement deletion to a recycle bin that allows users to restore items #2966

Open

[REWORK] Implement Recycle Bin #4223

Open

rohnsha0 closed this Sep 21, 2025

	"${count} item(s) restored successfully.",
	"${count} item{count != 1 ? "s"} restored successfully.",

	<span class="ms-1">Restore Selected</span>
	<span class="ms-1">Restore selected</span>

	<span class="ms-1">Delete Selected</span>
	<span class="ms-1">Delete selected</span>

	<span class="ms-1">Empty Recycle Bin</span>
	<span class="ms-1">Empty recycle bin</span>

Uh oh!

Conversation

rohnsha0 commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

to be merged together:-

Uh oh!

mister-roboto commented Apr 29, 2025

Uh oh!

stevepiercy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevepiercy commented Apr 29, 2025

Uh oh!

stevepiercy commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jensens commented May 9, 2025

Uh oh!

davisagli commented May 11, 2025

Uh oh!

davisagli commented May 11, 2025

Uh oh!

rohnsha0 commented May 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevepiercy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davisagli commented Jun 17, 2025

Uh oh!

rohnsha0 commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rohnsha0 commented Jul 10, 2025

Uh oh!

davisagli commented Aug 24, 2025

Uh oh!

rohnsha0 commented Sep 21, 2025

Uh oh!

rohnsha0 commented Sep 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

rohnsha0 commented Apr 29, 2025 •

edited

Loading

stevepiercy commented Apr 29, 2025 •

edited

Loading

rohnsha0 commented Jun 17, 2025 •

edited

Loading