The main problem with the previous method was that it only worked
if the contract being validated inside the block was sharing the same
errors object as the contract calling it. The old implementation
was incapable of incorporating errors from a contract that had its
own errors object.
In my opinion it's a code smell that our contracts use the #errors
of the model they are validating as the place to store their validation
result, however I didn't dare touching this yet, because changing that
would certainly be a huge change. I can also imagine that some places rely
on it for historic reasons (because using "classic" validations also
puts the errors on the model). However, I think that no two contracts
should RELY on using the SAME errors object, just because the way we implemented
them happens to cause them to share the same object.
There were cases were the writable parameter of a parent contract would
take precedence of the child contract writable parameter when the parent
condition is a lambda: a child could say `writable: true` and it would
still be the parent `writable: -> { false }` that would "win" because
when reducing and evaluating `writable_conditions`, only lambda are
considered.
This commit fixes the issue by adding extensive tests and putting both
lambdas AND true/false values in the `writable_conditions` hash.
Because collect_ancestor_attributes_by goes from the subclass attributes
and then combining them with the superclass attributes, merge overrides
more specific definitions (from subclass) with the more generic ones.
Instead, `reverse_merge!` can be used, even merging in place as the
attributes are duped first.
For new models, per-field permissions are not considered, only the
default permission (e.g. add_work_package).
This is may not be a final solution, but at this moment I don't know how
to distinguish between per-field permissions for edit and create cases.
This fixes more tests, but copy WP scenario required also explicit
`status_id` in the merge block in CopyService#copied_attributes.
This was previously done for `parent_id` as well, so it's not exactly
a precedent.
* Add setting for whitelist
* Make attachments API BaseServices compatible
* Add prepare service and contract
* Correctly pass the filename to the UploadedFile
* Add presence check to filename
* Fix expected validation message
* We no longer raise a multipart error when metadata is empty
* Fix filesize validation on prepared uploads
* Add parser error if invalid metadata json
* When attachment is not saved, use filename property
* Return correct error message on JSON parser erroro
* Fix specs
* Use attachment upload representer
* Fix direct uploads mocks with new service layer
* Lint
* Fix export job using attachment service
* Fix IFC controller using attachment prepare service
* Fix export job
* RenameRename params_getter to params_source
* Fix mail handler using attachment service
* Fix usage of attachment create service in documents
* Reuse shared examples for document attachment spec
* Fix stubbed attachment service in export job spec
* Use admin user in backup spec
* Fix export job for bim
* Fix attachment integration spec
* Fix issues_controller spec
* Make budget resource spec reuse common examples
* Fix attachment parsing representer spec
* Replace prepare part of attachment spec into separate service spec
* Clear cache for login spec
* Convert document create/update into services
* Budget services
* Allow options to be passed to property twin
* Remove setting author on budget initialize
* Replace meetings update with services
* Replace ifc models attachment handling with services
* Don't check uploader if changed by system
* Fix uploader being changed by system
* Replace wiki page attach_files with attachable services
* Replace avatar saving
* Replace snapshot attach_files
* Skip double validation when container present
* Set snapshot through attachment service
* Remove attach_files
* Validate content type in contract
* Enforce writing the content type without accepting user input
* Expect changed content_type
* Fix content of viewpoint image to get correct content type
* Fix tsv spec
* Add create contract spec
* Bypass whitelist in internal services when conflicting with user
* Fix expects in specs after whitelist bypass
* Render contract errors for wiki
* Add before_hook to bodied to allow to pre-authorize permissions
* Budget errors from contract
* Document errors from contract
* Align ModelContract validate/valid? methods with ActiveModel::Validations
* Add BaseContract to allow contracts on non ActiveRecord resources
* Avoid using alias_method as that won't work with subclasses
Co-authored-by: Oliver Günther <mail@oliverguenther.de>
Ivan Kuchin