Test coverage: Inclusion of intermediate resources when include is a multi-part relationship path

I think this is first a bug, then a feature request.

Since we don't currently support Example 2, we should be throwing a 400 Bad Request along with a helpful error. That error should at least suggest the request in example #1.

However, it seems reasonable that both #1 and #2 should work as @pixelwhip said he expected. Adding that would solve two issues with one patch :)

Wow, superb bug report, @pixelwhip! Thank you so much!

Once #2953321: Comprehensive JSON API integration test coverage phase 5: nested includes and sparse field sets lands, test coverage will be simpler to add. Postponing.

#2953321: Comprehensive JSON API integration test coverage phase 5: nested includes and sparse field sets landed, adding explicit test coverage for this became a lot simpler.

Also, I think #2976108-5: Spec Compliance: Impossible to include related resources when relationship field is not in a request's sparse fieldset solves at least part of this problem, if not all of it.

Also, I think #2976108 solves at least part of this problem, if not all of it.

I think it does. Test coverage patch for when that lands, combined patch to double check.

Giving credit to @pixelwhip.

I just pushed it :P It's quite likely that your combined patch will not apply because of that…

Still NR, as @Wim Leers's prediction was correct.

Failed asserting that Array &0 (
    0 => 'data'
    1 => 'jsonapi'
    2 => 'links'
) is identical to Array &0 (
    0 => 'data'
    1 => 'jsonapi'
    2 => 'links'
    3 => 'meta'
).

This is almost certainly an access related failure in the expectation, I'll debug/fix.

#12: I think I fixed the same failure in #2956084: Impossible to raise an error when an `include` is requested for an inaccessible relationship field., or at least a related one that had the same failure output :) I suspect that this will become trivial after #2956084: Impossible to raise an error when an `include` is requested for an inaccessible relationship field..

I suspect that this will become trivial after #2956084

Indeed, I stopped tinkering with this because #2956084: Impossible to raise an error when an `include` is requested for an inaccessible relationship field. was just too much of a pain to work around. Let's postpone this one one that.

This works as of #2976108: Spec Compliance: Impossible to include related resources when relationship field is not in a request's sparse fieldset, it just needs tests to prove it.

AFAICT this does not work?

/jsonapi/node/article?include=uid.roles or /jsonapi/node/article?include=field_tags.vid or /jsonapi/node/article?include=field_tags.parent should include roles, vocabularies or terms, respectively, but noen of them work?

This is blocked on #2956084: Impossible to raise an error when an `include` is requested for an inaccessible relationship field.. But that is a normal task. Either this issue needs to be downgraded to a normal task too, or we need to upgrade #2956084 to major.

#2997600: Resolve included resources prior to normalization landed just now. In #2997600-24: Resolve included resources prior to normalization, @gabesullice said:

Manual testing should show that this patch does not require the intermediate field.

I tried to apply the test coverage from #2946537 and found out that it's incomplete. The actual results are correct, but the expectations are wrong.

I'm pretty sure that means this is still something that requires code changes and not just test coverage.

#2956084: Impossible to raise an error when an `include` is requested for an inaccessible relationship field. landed! This is unblocked now :)

The #23 failures are all because the test coverage is asserting the previous, incorrect behavior. The actual behavior is correct though.

Confirming #25.

This interdiff makes at least N pass. For such a trivial change, it took far longer to find/solve than I'd like to admit 😅

Green! Yay :)

While debugging to spot the root cause (fixed in #26), I did spot one other bug, but it's one that didn't actually manifest itself in any way. Yet. The bug: getExpectedIncludedResourceResponse() always uses the tested resource type to look up internal field names. But it should actually be using the resource type of the current entity when evaluating a nested include path/expression.

Nice work!

Feel free to commit it — it doesn't need to be me who does that :)

Done.