Skip to content
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

Patch "contentItems" to no longer return trashed content. #15

Merged
merged 1 commit into from Oct 1, 2019

Conversation

@jone
Copy link
Member

commented Oct 1, 2019

The contentItems is used for other methods such as listFolderContents and getFolderContents. We do not want those methods to include trashed content as they are used within Plone and within our addons. We are not patching the underlying objectItems though as it probably would make the trashed content disappear from ZMI and harder to restore etc.

@jone jone requested a review from 4teamwork/plone Oct 1, 2019
Copy link

left a comment

I'm not sure that method is used in Plone 5?

@djowett-ftw

This comment has been minimized.

Copy link

commented Oct 1, 2019

I'm not sure that method is used in Plone 5?

Ok - it is - my searchfu is off

@jone

This comment has been minimized.

Copy link
Member Author

commented Oct 1, 2019

I'm not sure that method is used in Plone 5?

I think one of them is older and the other should be used; but I don't remember which is which 🙈
But I'm not patching them directly anyways, so it shouldn't matter.

@jone jone requested a review from 4teamwork/plone Oct 1, 2019
Copy link

left a comment

Looks good then. But, I think you might want to:
a) have multiple children in the tests to prove that ordering is unchanged.
b) could docstrings for tests be distinct? (i.e. when it's a folder vs Plone Site)

I presume this doesn't stop an admin viewing the trashed content?

The ``contentItems`` is used for other methods such as
``listFolderContents`` and ``getFolderContents``.
We do not want those methods to include trashed content as they are
used within Plone and within our addons.
We are not patching the underlying ``objectItems`` though as it
probably would make the trashed content disappear from ZMI and harder
to restore etc.
@jone jone force-pushed the jone-patch-contentItems branch from 55d0df7 to 7918721 Oct 1, 2019
@jone

This comment has been minimized.

Copy link
Member Author

commented Oct 1, 2019

a) have multiple children in the tests to prove that ordering is unchanged.
b) could docstrings for tests be distinct? (i.e. when it's a folder vs Plone Site)

Updated.

I presume this doesn't stop an admin viewing the trashed content?

Yes, there are tests for the trash (viewing / restoring) and for the ZMI.

@djowett-ftw djowett-ftw self-requested a review Oct 1, 2019
Copy link

left a comment

Ok thanks - if the guvnor's happy so am I !

@jone jone merged commit 7e25cd7 into master Oct 1, 2019
2 checks passed
2 checks passed
CI Governor: test-plone-4.3.x.cfg Task #449313 succeeded
Details
CI Governor: test-plone-5.1.x.cfg Task #449314 succeeded
Details
@jone jone deleted the jone-patch-contentItems branch Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.