Skip to content

Implement Recycle Bin#4166

Closed
rohnsha0 wants to merge 12 commits into
6.1.xfrom
rohnsha0-feat-recycle-bin
Closed

Implement Recycle Bin#4166
rohnsha0 wants to merge 12 commits into
6.1.xfrom
rohnsha0-feat-recycle-bin

Conversation

@rohnsha0

@rohnsha0 rohnsha0 commented Apr 27, 2025

Copy link
Copy Markdown
Member

ref #2966

image

@mister-roboto

Copy link
Copy Markdown

@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!

@rohnsha0

Copy link
Copy Markdown
Member Author

@jenkins-plone-org please run jobs

@rohnsha0 rohnsha0 marked this pull request as ready for review April 27, 2025 12:32
@rohnsha0

Copy link
Copy Markdown
Member Author

@jenkins-plone-org please run jobs

@stevepiercy stevepiercy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great addition! Minor tweaks to messages and news. Thank you!

Comment thread Products/CMFPlone/browser/recyclebin.py Outdated
Comment thread Products/CMFPlone/browser/recyclebin.py Outdated
Comment thread Products/CMFPlone/browser/recyclebin.py Outdated
Comment thread Products/CMFPlone/profiles/default/controlpanel.xml Outdated
Comment thread news/2966.feature Outdated
Co-authored-by: Steve Piercy <web@stevepiercy.com>

@stevepiercy stevepiercy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. Let's get a technical review from a senior Plonista.

@petschki petschki left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice, but I'd say this should go into master and 6.2 only as it is a new feature.

EDIT: maybe it could be refactored as a "core-addon" which could be used (optionally) in 6.1 too ?

@pbauer

pbauer commented Apr 29, 2025

Copy link
Copy Markdown
Member

This is an interesting solution to the problem that was not covered in https://community.plone.org/t/any-plans-for-an-official-trashcan-in-plone/5440
I have not tested it yet but would like know where you found inspiration or discussed the implementation. I also wonder what stashing the obj in an annotation means for relations and annotations. I would expect that relations are removed and not restored upon restore. Are annotations (e.g. comments) kept in an annotation on the obj stashed in an annotation?

@rohnsha0

rohnsha0 commented Apr 29, 2025

Copy link
Copy Markdown
Member Author

would like know where you found inspiration

I took the inspiration from reading about various other CMS :)

I would expect that relations are removed and not restored upon restore.

the relations are preserved throughout the restoration process... as they're essentially "frozen" in the state they were in at deletion time.

Are annotations (e.g. comments) kept in an annotation on the obj stashed in an annotation?

I was unfortunately unable to make it work on deleted comments. I was working on it since morning.. but somehow wasn't able to!

@rohnsha0

Copy link
Copy Markdown
Member Author

EDIT: maybe it could be refactored as a "core-addon" which could be used (optionally) in 6.1 too ?

+1

@ale-rt

ale-rt commented Apr 29, 2025

Copy link
Copy Markdown
Member

All the presentation logic for classic UI (in particular the templates), should go in plone.app.layout.
The code should be structured in such a way that it is convenient to build some Rest API endpoint on it.

I still did not have the time to deeply checked the code.

About the approach taken, from my previous experiences, I like the approach of applying a trashed interface because it is way faster.
This does matter a lot when you have big sites.

def __call__(self):
form = self.request.form

if form.get("form.submitted", False):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use a z3c.form form here

@rohnsha0 rohnsha0 mentioned this pull request Apr 29, 2025
@rohnsha0

Copy link
Copy Markdown
Member Author

closed in favour of #4168

@rohnsha0 rohnsha0 closed this Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants