Possible Bug: sideposting via PATCH does not pick up errors on has_many children when only posting new ones

[mattgibson]

The scenario: I have a model with a has_many relationship, with an instance and some of its children already persisted. I want to add a new one. I send a PATCH request with only the to-be-created child object, skipping the existing ones that I want to leave alone. All works fine unless the new child is invalid. In that case, the error is ignored and a 200 response is returned with no errors.

The problem: The reason is the following code in Graphiti::Util::ValidationResponse:

        related_objects.each_with_index do |r, index|
          method = payload[index].try(:[], :meta).try(:[], :method)
          next if [nil, :destroy, :disassociate].include?(method)

Because the index is from the existing association on the model, it includes all the DB models as well as the one I am adding, so when adding a fourth to three existing ones, the index for the new one is 3. However, the payload only had the one I am adding, so to find the params, we would need to use payload[0].

I am unsure if this is actually a bug or if I am misusing the sideposting mechanism. The spec and docs don't seem to mention whether a PATCH can be done this way.

[richmolj]

Hey @mattgibson ! 👋

The spec and docs don't seem to mention whether a PATCH can be done this way.

You should definitely be able to do this. I think the big thing is

Because the index is from the existing association on the model, it includes all the DB models as well as the one I am adding

We actually run some wonky code to make sure model.relationships only returns the records sent in the request. So we might have a bug, unless you're on an older version of Graphiti or running some custom code in your app? It would be a big help to see this reproduced in either the sample app or a test within Graphiti.

[mattgibson]

Ah, OK. Sounds like a bug, then. I stepped throught that code with the debugger and association already contains the existing 3 records at that point, so it adds the to-be-created one.

Maybe a solution is to loop over the objects in the relationship payload matching by id or temp-id, but I don't know if temp-id is still available at that point.

I'll have a go at making a test for it

[richmolj]

It should be available as a magic variable on the model instance, e.g. @post.instance_variable_get(:'@_jsonapi_temp_id') (unavoidable 😝 )

already contains the existing 3 records at that point

And these are not sent in the payload? If that's the case I consider that the bug, and the validation just a downstream symptom of a larger disease.

[richmolj]

@mattgibson I just tried to replicate in the sample app and couldn't. I had an employee with a pre-existing position, then tried to add an invalid position. I get back the correct error message and behavior. Am I missing something? Here is the branch https://github.com/graphiti-api/employee_directory/tree/issue198