POST /resources This seems to be impossible. It can be done later if we figure it out.

Why would it be impossible? We actually have that case where we want to create an object and get related resources in the response. Something like: POST /articles?include=author.articles (create an article and get all existing articles from the same author). Why would it be an issue? We have the author from relationship data of the POST request and we can check index? on the Article class easily.

Thanks for working on this!

Why would it be impossible?

The record itself isn't created by by the time the operation processor runs for create_action, so it gets quite difficult to try to get the related models underneath before the root object is built.

The policy scoping will be applied still for the has_many resources, but this PR doesn't yet support stopping the entire POST request if the user wouldn't be authorized to include the author.articles records.

We can always tackle that issue on a follow-up PR. This risks getting too large and difficult to review.

and we can check index? on the Article class easily.

Yes, but we'd first have to go through the author and use show? on it. Currently this functionality relies on the record being present by the time we go and fetch all the relationships etc. There might be a possibility to still get an index? authorization without having the record available yet, but show? for has_one associations is a no-go.

The way the include directives are parsed is by going up the hash tree available to operations processor via @request.include_directives. It relies on going through all the steps. And it might actually even crash for nils currently if a has_one association is null in a graph scenario 😅 author.comments will crash if author is nil.

I think what we discussed in #7 was to authorize all included resource from inside the records_for method and not at the operation level like your current implementation here. Handling POST request would not be an issue if all the logic is handled at the JSONAPI::Resource#records_for`. Don't you think?

I think what we discussed in #7 was to authorize all included resource from inside the records_for method

Yeah you're right. I wanted to see what kinds of requests trigger that records_for method. It seems that basic relationship operations also trigger it, and we can't really get enough context on the resource level #records_for on what kind of request is incoming to call only the correct authorizer function.

I think that my approach here, which relies on before hooks is a bit flawed. As inclusion of related resources happens after the operation is complete, maybe I should also just authorize the inclusion in after callbacks. This way I might even have the resource available somehow in the operation level instance variables, and it would work for POST requests, too.

I'd like to keep the authorizing logic inside the operations processor if at all possible, and if we do get proper insight to requests on resource level callbacks and methods, we might want to look into moving every authorization inside it.

Ok it makes sense to keep all the authorization logic inside the operations processor, in that case the after callback could be the way to go. 👍

Ok the functionality seems to be complete now, alongside with passing the relationship source record to the authorizer. Phew. The specs are a bit messy right now, but at least they should be complete.

I have tested this branch on merged with #11 and it works fine with that fix, all our 336 API specs are green 💚.

Great stuff, both of you! 👍

There is most definitely still work that could be done here, Currently authorize_include_item is such a beast that it's bound to come back and bite us. 😬 Still, it might be best to get this merged and keep coding on top of it, so as to lessen the risk of conflicts for other PRs.

Fixed the merge conflicts here after #11 was merged :)

Just tested that branch and everything still green for us. I'll use that one until it's merged.
Thanks a lot for your great work here. 🎉

We discussed this with @lime today. Sorry about the delay, I'll merge this now and release a new version soon after — I'll put up an issue about the public_send with relationships. I could need a little help with that, and it'll be much easier to do when the amount of code that needs to be changed is much smaller than this PR.

v0.6.0 pushed out :) feel free to use that one @thibaudgg, it should contain all the necessary bits :)

I'll put up an issue about the public_send with relationships. I could need a little help with that, and it'll be much easier to do when the amount of code that needs to be changed is much smaller than this PR.

Actually, I think that currently we are doing the right thing when fetching the relationships directly from the record instead of going through resource classes. If we hit a case for trying to sideload a record we're not allowed to see with include directive, we should authorize for them even if we'd get a 404 when we'd try to show that resource via a normal show operation.

👍 using 0.6 now! Thanks!