Skip to content

[RECYCLE BIN] Fixes broken redirection after restoration and handle value-error gracefully if parent is deleted #4175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: rohnsha0-plip-recyclebin
Choose a base branch
from

Conversation

This method implements a fallback strategy:
1. Try restored object's view URL first
2. If that fails, try target container URL
3. If that fails, use the recycle bin URL
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the strategy. What is the situation where an item was restored but we can't redirect to its view URL?

Copy link
Member Author

Choose a reason for hiding this comment

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

Umm, the implementation was thought inna way:-

  • try to get the {absolute_url()}/view but if failing (maybe it is not available for the object)
  • then get the target container url but if the worst case scenario - object is broken
  • then target recyclebin url...

Copy link
Member

Choose a reason for hiding this comment

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

but if failing (maybe it is not available for the object)

This is the part I don't understand. Getting the object's absolute_url should always work. You can use restored_obj.restrictedTraverse("@@plone_context_state").view_url() to get it with the /view suffix for the content types that require that. (This comes from https://github.com/plone/plone.app.layout/blob/master/plone/app/layout/globals/context.py#L64)

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe, I overcomplicated it too much... However, I have implemented a simplified based on ur suggestions in 56428b1

@rohnsha0 rohnsha0 requested a review from davisagli May 23, 2025 03:59
Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

I'm still not seeing the restored object after I restore it to a different path on the item detail page. The restored object needs to be reindexed.

This method implements a fallback strategy:
1. Try restored object's view URL first
2. If that fails, try target container URL
3. If that fails, use the recycle bin URL
Copy link
Member

Choose a reason for hiding this comment

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

but if failing (maybe it is not available for the object)

This is the part I don't understand. Getting the object's absolute_url should always work. You can use restored_obj.restrictedTraverse("@@plone_context_state").view_url() to get it with the /view suffix for the content types that require that. (This comes from https://github.com/plone/plone.app.layout/blob/master/plone/app/layout/globals/context.py#L64)

Copy link
Member Author

@rohnsha0 rohnsha0 left a comment

Choose a reason for hiding this comment

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

  1. create a folder
  2. create a news page inside the same folder
  3. delete it
  4. navigate to recycleItemView and input url of other sister folder
  5. click restore
  6. get 404 error that page does not exists
  7. go to the sister folder
  8. expected: content is present but actually it is lost

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recycle bin: Can't restore item if it's original container no longer exists
2 participants