-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
AttributeError occurs in creator_name when the user is missing #1867
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
@jenkins-plone-org please run jobs |
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.
@mamico From what I can see, Zope locks store their creator using the userid, not the username (login name) -- see https://github.com/zopefoundation/Zope/blob/master/src/OFS/LockItem.py#L88 and https://github.com/zopefoundation/AccessControl/blob/master/src/AccessControl/owner.py#L263
So I think we should pass userid to api.user.get
here, not username. The existing implementation would only work for users for whom the user id and login name are the same.
You are right. If it is ok for you, I'll add also this change in the PR. Regarding the workingcopy endpoint, I was wrong, there is a different implementation of creator_name and it seems that userid was used correctly instead of username. https://github.com/plone/plone.restapi/blob/main/src/plone/restapi/serializer/working_copy.py#L94 |
@mamico Yes please! |
Hi @davisagli, I did. But, I started writing with the test, and I observed that it didn’t fail. Upon closer inspection, I realized that for usernames, Just wanted to record this observation for reference. |
If the user is not found
creator_name
fails withAttributeError: 'NoneType' object has no attribute 'getProperty'
.The issue can occur, for example, if the user is no longer present or due to an LDAP service disruption. IMO falling back to the username is still a conservative strategy.
The test are for the
@locking
endpoint, but thecreator_name
function is used also in other contexts (ie workingopy)📚 Documentation preview 📚: https://plonerestapi--1867.org.readthedocs.build/