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

Allows versioning of the Plone Site type for the portal working copy to work #113

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

wesleybl
Copy link
Member

@wesleybl wesleybl commented Oct 2, 2024

This is part of: plone/volto#5284

For the working copy of plone.app.iterate to work on the Plone Site type, it is necessary that this type is versionable. See:

https://github.com/plone/plone.app.iterate/blob/1908aa1f4e9c95c37143fae88655469b72ea451a/plone/app/iterate/browser/control.py#L88-L90

This also allows the "Plone Site" type to be cloned.

Products.CMFEditions uses the pickle module to clone objects. See:

def _cloneByPickle(self, obj):
"""Returns a deep copy of a ZODB object, loading ghosts as needed."""
modifier = getToolByName(self, "portal_modifier")
callbacks = modifier.getOnCloneModifiers(obj)
if callbacks is not None:
pers_id, pers_load, inside_orefs, outside_orefs = callbacks[0:4]
else:
inside_orefs, outside_orefs = (), ()
stream = BytesIO()
p = Pickler(stream, 1)
if callbacks is not None:
p.persistent_id = pers_id
p.dump(aq_base(obj))
approxSize = stream.tell()
stream.seek(0)
u = Unpickler(stream)
if callbacks is not None:
u.persistent_load = pers_load
return approxSize, u.load(), inside_orefs, outside_orefs

These changes prevent the error:

TypeError: Can't pickle objects in acquisition wrappers

When trying to serialize an ImplicitAcquisitionWrapper object with the pickle dump. This error occurred when trying to check in a Plone Site type with plone.app.iterate.

These changes also allow the Plone Site type to be serialized with the pickle. When the persistent_id method of SkipParentPointers returns something other than None, the object is not serialized. So we need to make sure it returns None for the Plone Site type.

For the working copy of plone.app.iterate to work on the "Plone Site"
type, it is necessary that this type is versionable. See:

https://github.com/plone/plone.app.iterate/blob/1908aa1f4e9c95c37143fae88655469b72ea451a/plone/app/iterate/browser/control.py#L88-L90
Products.CMFEditions uses the pickle module to clone objects. See:

https://github.com/plone/Products.CMFEditions/blob/c80bc31af46bff45fee2908878b1f01190fda8d8/Products/CMFEditions/ArchivistTool.py#L210-L229

These changes prevent the error:

TypeError: Can't pickle objects in acquisition wrappers

When trying to serialize an ImplicitAcquisitionWrapper object with the
pickle dump. This error occurred when trying to check in a Plone Site
type with plone.app.iterate.

These changes also allow the Plone Site type to be serialized with the
pickle.
@mister-roboto
Copy link

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

@wesleybl
Copy link
Member Author

wesleybl commented Oct 2, 2024

@jenkins-plone-org please run jobs

@wesleybl
Copy link
Member Author

wesleybl commented Oct 3, 2024

I ran the tests for this PR along with plone/plone.app.iterate#128

See: Plone Jenkins CI - pull-request-6.0-3.11 and Plone Jenkins CI - pull-request-6.0-3.8

Comment on lines +29 to +32
<type name="Plone Site">
<policy name="at_edit_autoversion" />
<policy name="version_on_revert" />
</type>
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure whether I should do an upgrade step for this or just leave it for new portals.

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.

2 participants