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

api.content.create lacks schema validation #415

Open
jaroel opened this issue Oct 5, 2018 · 5 comments
Open

api.content.create lacks schema validation #415

jaroel opened this issue Oct 5, 2018 · 5 comments

Comments

@jaroel
Copy link
Member

jaroel commented Oct 5, 2018

As seen in https://community.plone.org/t/creating-custom-dexterity-objects-from-plone-api/7318/ we can currently set an invalid value/type for fields.
In this case the user is setting a str instead of a RichTextValue, which leads to tracebacks like

AttributeError: 'str' object has no attribute 'output_relative_to'

 - Expression: "python:context.text.output_relative_to(view.context)"
 - Filename:   ... egg/plone/app/contenttypes/browser/templates/document.pt
 - Location:   (line 15: col 29)
 - Source:     ... ucture python:context.text.output_relative_to(view.context)"
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Is this something we need to handle here, or is this something DX should do?

@jaroel jaroel changed the title api.content.create lack schema validation api.content.create lacks schema validation Oct 5, 2018
@tomgross
Copy link
Member

tomgross commented Oct 6, 2018

I regularly use api.content.create in test to create mock-like objects with just the attributes I need for testing. Validating against the full schema would break a LOT of tests for me. An additional optional parameter to do the check could be a way to go.

@jaroel
Copy link
Member Author

jaroel commented Oct 6, 2018

I was thinking about the attributes you pass in, not the whole schema.
We don't see that many questions about it, though.

@davisagli
Copy link
Member

Dexterity also doesn't validate attributes when you set them directly on an existing object. Validation happens via forms and the REST API. Maybe we should consider it when working with objects directly, but it would be a pretty big change, and would require some thought about how to handle things like invariants involving multiple fields.

@gbastien
Copy link
Member

Hi @jaroel

funny, I was just about to add the same issue ;-)

I think a parameter set to True by default should validate fields, we end up creating our own "validate_fields" method called after every api.content.create (see https://github.com/IMIO/imio.helpers/blob/master/src/imio/helpers/content.py#L239) and we even needed to write migrations sometimes to fix things, and so on...

@jaroel if you do not propose a PR, we will ;-) Or we can work on this at the PloneConference sprint if you are there :-p

Gauthier

@jaroel
Copy link
Member Author

jaroel commented Oct 15, 2018 via email

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

No branches or pull requests

4 participants