-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
Add get_parents and get_closed_parent functions #560
base: main
Are you sure you want to change the base?
Conversation
@rohnsha0 thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment:
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! |
@rohnsha0 this PR will need tests and docs, per https://6.docs.plone.org/plone.api/contribute.html |
@stevepiercy Ik, I'll do it today 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are not actually getting the "parents" but the ancestors.
Okay, so how would I get that!? I'm unsure of the same! @ale-rt |
@rohnsha0 the tests are failing, you have already been suggested to work on your editing tool. Before pushing, please:
Please read: https://6.docs.plone.org/plone.api/contribute.html |
not sure, why the tests are now failing! |
Co-authored-by: David Glick <[email protected]>
@rohnsha0 I think @ale-rt was suggesting to replace the string |
Thanks @stevepiercy 🥇 |
@jenkins-plone-org please run jobs |
src/plone/api/content.py
Outdated
filtered_chain = [] | ||
for parent in chain: | ||
try: | ||
if predicate(parent): | ||
filtered_chain.append(parent) | ||
except WorkflowException: | ||
# Skip objects that raise WorkflowException | ||
continue | ||
chain = filtered_chain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this change does not make any sense to... it was already good as it was before...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If not handled here, it raises WorkflowException
. The change ig is needeed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If not handled here, it raises
WorkflowException
. The change ig is needeed
Not really, the code of this function should be agnostic of the predicate internals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uhm, then documenting that predicates should handle their own exceptions, would be workaround? @ale-rt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you mark this conversation as resolved why it is not. This does not help getting your work merged.
I would not even document that, IMO it is obvious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you mark this conversation as resolved why it is not.
I pushed the changes. I thought my question about that doesn't matter now since I just documented it. Still sorry @ale-rt. Wasn't intended.
This does not help getting your work merged.
Ik, thanks! Marking a convo resolved doesn't relate to getting work merged! If the code is useful then it'll be merged else closed. Simple!
But was just done coz I thought I've made the changes so it's not relevant. Which ig was wrong. Bt please don't take it elsewhere.
Please @rohnsha0, before continuing check how the conversation ere develops: #531 (comment) |
closes #531
📚 Documentation preview 📚: https://ploneapi--560.org.readthedocs.build/