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

Fix default DOI when adding content #142

Merged
merged 1 commit into from
Jan 23, 2024
Merged

Conversation

reinhardt
Copy link
Contributor

@reinhardt reinhardt commented Jan 22, 2024

This is probably not a final fix but it does fix an error when calling dexterity-types/*/@@fields on a type that uses the IBaseReview behavior.

syslabcom/scrum#1199

@reinhardt reinhardt requested a review from thet January 22, 2024 09:57
@reinhardt reinhardt force-pushed the scrum-1199-fix-default-doi branch from 0d62639 to ed5843d Compare January 23, 2024 13:31
Copy link
Member

@thet thet left a comment

Choose a reason for hiding this comment

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

I have a suggestion, which IMO should be implemented. If you do, just merge it then - you've got my approval.
If you decide against then there are reasons for that and then also go ahead and merge :)

@@ -26,6 +27,9 @@ def generateDoi(context):
Might not need to be called as a default factory, but just in the
edit form
"""
if IBaseReview not in iterSchemata(context):
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be nicer, maybe also more performant‌ (which probably isn't a problem here) if you would use a marker interface.

You create a marker interface, register it for the behavior like so:

  <plone:behavior
      name="recensio.base_review"
      title="Base Review Behavior"
      factory=".base_review.BaseReview"
      provides=".base_review.IBaseReview"
      marker=".base_review.IBaseReviewMarker"
      for="plone.dexterity.interfaces.IDexterityContent"
      />

and then:

Suggested change
if IBaseReview not in iterSchemata(context):
if not IBaseReviewMarker.providedBy(context):

@reinhardt
Copy link
Contributor Author

It's a good suggestion but a bad time to implement it because it's not yet clear whether the whole DOI generation/registration code is going to stay or be removed or reworked. So I'll merge as-is for a quick fix, and we'll likely revisit it later and can then decide about the marker interface. Cheers!

@reinhardt reinhardt merged commit 26b8309 into main Jan 23, 2024
3 checks passed
@reinhardt reinhardt deleted the scrum-1199-fix-default-doi branch January 23, 2024 14:27
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