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

When creating an OiRa tool from a template, reset the UIDs to avoid c… #744

Merged
merged 1 commit into from
May 24, 2024

Conversation

ale-rt
Copy link
Member

@ale-rt ale-rt commented May 21, 2024

…omplaints from the catalog

@ale-rt ale-rt requested a review from mauritsvanrees May 21, 2024 14:49
@ale-rt
Copy link
Member Author

ale-rt commented May 21, 2024

The form URL is something like: https://example.com/sectors/eu/private-security-eu/++add++euphorie.surveygroup
To trigger the behavior you have to change the "How would you like to start" to the second or third option.

Without this patch you will see in the logs:

A different document with value '...' already exists in the index

@ale-rt ale-rt requested a review from reinhardt May 24, 2024 06:36
Comment on lines +136 to +141
# We use a fake event to reuse the subscriber from plone.uuid that
# resets the UIDs on copy
fake_copy_event = ObjectCopiedEvent(None, None)
addAttributeUUID(copy, fake_copy_event)
for id, item in copy.ZopeFind(copy, search_sub=1):
addAttributeUUID(item, fake_copy_event)
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried and it works. But we can simplify by notifying the actual event. This works for me:

Suggested change
# We use a fake event to reuse the subscriber from plone.uuid that
# resets the UIDs on copy
fake_copy_event = ObjectCopiedEvent(None, None)
addAttributeUUID(copy, fake_copy_event)
for id, item in copy.ZopeFind(copy, search_sub=1):
addAttributeUUID(item, fake_copy_event)
# This is handled by a subscriber in plone.uuid that resets the UIDs on copy.
notify(ObjectCopiedEvent(copy, template))

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 am a bit shy firing up the event because that can trigger unwanted subscribers

Copy link
Contributor

Choose a reason for hiding this comment

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

I can imagine. But I am a bit shy about not firing the event, because then we miss wanted subscribers. I dug a bit and found a few that could be useful:

In CMFCore:

    elif IObjectCopiedEvent.providedBy(event):
        if hasattr(aq_base(ob), 'workflow_history'):
            del ob.workflow_history

In plone.app.linkintegrity:

    <subscriber
        for="plone.app.relationfield.interfaces.IDexterityHasRelations
             zope.lifecycleevent.interfaces.IObjectCopiedEvent"
        handler=".handlers.modifiedContent" />

In plone.app.multilingual:

@adapter(ITranslatable, IObjectCreatedEvent)
def addAttributeTG(obj, event):

    if (
        not IObjectCopiedEvent.providedBy(event)
        and getattr(aq_base(obj), ATTRIBUTE_NAME, None)
    ):
        return  # defensive: keep existing TG on non-copy create

Probably no big deal, but still my suggestion stands.

Anyway, this PR is an improvement already because it creates new UIDs, so I approve.

@ale-rt ale-rt merged commit 2f7b8dd into main May 24, 2024
2 checks passed
@ale-rt ale-rt deleted the reset-uids branch May 24, 2024 10:15
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