Do not throw not found error when resource relationship is null

[jpalumickas]

This PR extends #54 with added tests.

[valscion]

Ok, now I see what's happening. Thanks for the PR!

Do you think it would be better if it were somehow possible to tell the authorizer that the relationship is about to get emptied? What if the expected authorizer call would look like this instead?

allow_operation(
  'replace_fields',
  article,
  [{
    relation_name: :author,
    relation_type: :to_one,
    records: nil
  }]
)

These kind of relationship specs currently reside in a slightly badly named file spec/requests/tricky_operations_spec.rb. Do you think you could move the added spec there, and maybe structure it like so?

describe 'PATCH /articles/:id (nullifying to-one relationship)' do
  # spec code that is similar to the
  # 'PATCH /articles/:id (mass-modifying relationships)' spec code
  # ...
end

Would you also be open to change the behavior to match that?

Thanks for your contribution! If you want me to open up something more, feel free to ask me anything ☺️

[jpalumickas]

@valscion
Moved tests to tricky operations.

Do you think it would be better if it were somehow possible to tell the authorizer that the relationship is about to get emptied? What if the expected authorizer call would look like this instead?

For this one I had error, so I left as it is for now:

#<InstanceDouble(JSONAPI::Authorization::DefaultPunditAuthorizer) (anonymous)> received :replace_fields with unexpected arguments
         expected: (#<Article id: 1012824740, external_id: "3", author_id: 676971205, blank_value: nil>, [{:relation_name=>:author, :relation_type=>:to_one, :records=>nil}])
              got: (#<Article id: 1012824740, external_id: "3", author_id: 676971205, blank_value: nil>, [])

[valscion]

For this one I had error, so I left as it is for now:

Ah, but actually that error highlights the behavior that we would actually want to have. We would want to allow the authorizer to have a way of handling this situation.

However, in the DefaultPunditAuthorizer, we might want to actually call remove_to_one_relationship when the records is nil so that the policy method that would end up being called would be "remove_#{relationship_type}?".

Do you want to go ahead and do these changes? That would mean these changes:

• Change the next in AuthorizingProcessor#related_models_with_context to nil so that the authorizer class gets nil in the records part of the hash
  • This would mean that the test error you saw and didn't push would pass, so the spec could be updated
  • You could then also removed the .compact in the end of the #related_models_with_context method as it is no longer necessary
• Update the part inDefaultPunditAuthorizer#authorize_related_resources where it handles :to_one relation type to check whether records is nil and if so, call remove_to_one_relationship
  • Add a third describe block inside the DefaultPunditAuthorizer#replace_fields specs that is almost 1-to-1 the same as the with "relation_type: :to_one" block but just with the description of with "relation_type: :to_one" and records is nil. Note that the policy methods being asserted against would also change: Instead of callingreplace_<type>?, we'd want to call remove_<type>?.

Does this sound reasonable to you?

[valscion]

I don't think we need to change the def related_models method at all. Your bug fix should only be necessary to touch the related_models_with_context method.

[valscion]

This spec would actually pass even if we didn't handle UserPolicy::Scope at all, so I guess these user_policy_scope things could be removed.

[jpalumickas]

@valscion Fixed 😉

[valscion]

Wow, looking great! Thanks for the fast turn-around 💞

Are you OK with me adding you to the list of contributors in the README?

[valscion]

Thanks!

[jpalumickas]

Sure, cheers ! 😉

[valscion]

I'll release alpha4 version after I've had lunch. Thanks for your contributions!

[jpalumickas]

Thanks!

[valscion]

Released v1.0.0.alpha4 with this change :)

[jpalumickas]

Perfect! thanks for the release 🚀