Let creatorsDefault return None for anonymous user (fixes plone/plone.app.vocabularies/issues/59) - #311
Open
fulv wants to merge 1 commit into
Open
Let creatorsDefault return None for anonymous user (fixes plone/plone.app.vocabularies/issues/59)#311fulv wants to merge 1 commit into
fulv wants to merge 1 commit into
Conversation
This would fix plone/plone.app.vocabularies#59, but I'm only floating this change as an idea, hoping for some feedback. `z3c.form.widget.Widget.update()` passes the result of `creatorsDefault` (which is called by an `IValue` adapter's `get()` method) to `plone.app.z3cform.converters.AjaxSelectWidgetConverter.toWidgetValue()`. `getSecurityManager().getUser()` returns a regular `<PropertiedUser>` for most requests and thus the return value is a tuple, e.g. `('admin',)`. However, for ajax requests, e.g. `/++add++MyContentType/++widget++form.widgets.IMyBehavior.behavior_field`, the user is anonymous: ``` (Pdb) user <SpecialUser 'Anonymous User'> (Pdb) pp user.__dict__ {'__': '', 'domains': [], 'name': 'Anonymous User', 'roles': ('Anonymous',)} ``` Thus, the return value is the tuple `(None,)`. This breaks `AjaxSelectWidgets` when the content type includes the Dublin Core behavior or just the ownership field, even though the field itself is a `RelationList` with a `value_type=RelationChoice( source=CatalogSource(portal_type='MyContentType) )`. In other words, even though the broken field has no need for the item's creator, the creator gets calculated anyway when the request is handled. The problem with the tuple `(None,)` is that `toWidgetValue()` does not know how to deal with it and so it returns `None` instead of `field.missing_value` as it should. This could be fixed in at least four different places: 1. plone.app.dexterity.behaviors.metadata.creatorsDefault 2. plone.app.z3cform.converters.AjaxSelectWidgetConverter.toWidgetValue 3. z3c.form.widget.Widget.update 4. have getSecurityManager().getUser() return the logged in user even on Ajax requests. 1. is the origin of the unexpected value, however, I don't know if making it return `None` for anonymous users would make any other code mad. 2. is the place that does not expect a `(None,)` tuple instead of `None`, so this could be fixed by simply adding a check for `(None,)`. 3. is the "middleman" between 1 and 2, so it could convert `(None,)` to `None` before passing it on. 4. I have no idea what this would entail.
|
@fulv thanks for creating this Pull Request and help improve Plone! To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass. Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating: With this simple comment all the jobs will be started automatically. Happy hacking! |
Member
|
@jenkins-plone-org please run jobs |
Member
|
I think this one is fine. (except that the changelog/news entry is missing) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This would fix plone/plone.app.vocabularies#59, but I'm only floating this change as an idea, hoping for some feedback.
z3c.form.widget.Widget.update()passes the result ofcreatorsDefault(which is called by anIValueadapter'sget()method) toplone.app.z3cform.converters.AjaxSelectWidgetConverter.toWidgetValue().getSecurityManager().getUser()returns a regular<PropertiedUser>for most requests and thus the return value is a tuple, e.g.('admin',).However, for ajax requests, e.g.
/++add++MyContentType/++widget++form.widgets.IMyBehavior.behavior_field, the user is anonymous:Thus, the return value is the tuple
(None,). This breaksAjaxSelectWidgetswhen the content type includes the Dublin Core behavior or just the ownership field, even though the field itself is aRelationListwith avalue_type=RelationChoice( source=CatalogSource(portal_type='MyContentType) ). In other words, even though the broken field has no need for the item's creator, the creator gets calculated anyway when the request is handled.The problem with the tuple
(None,)is thattoWidgetValue()does not know how to deal with it and so it returnsNoneinstead offield.missing_valueas it should.This could be fixed in at least four different places:
Nonefor anonymous users would make any other code mad.(None,)tuple instead ofNone, so this could be fixed by simply adding a check for(None,).(None,)toNonebefore passing it on.I'll add tests and changelog, etc if this is the way to go.