🐛 Fix deep selects

[Samoht70]

Closes #196

Summary by CodeRabbit

• Bug Fixes

  • Nested relation responses now use the correct include-specific context, fixing field selection, aggregates, and permission checks for nested data and preventing bleed-through from global request context.
  • Consistent handling for single related items and collections improves reliability of complex nested includes.

• Tests

  • Added and updated tests verifying deep field selection, nested includes behavior, and unauthorized relation validation.

• Improvements

  • Ensures stable default ordering for related collections in responses.

[coderabbitai]

Warning

Rate limit exceeded

@Samoht70 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 09866d0 and 723ab77.

📒 Files selected for processing (1)

• tests/Feature/Controllers/SearchSelectingOperationsTest.php (2 hunks)

Walkthrough

Replaced top-level requestArray with currentRequestArray when processing nested relations in src/Http/Response.php; updated related tests and added a BelongsTo relation plus default ordering in tests/Support/Rest/Resources/HasManyResource.php. Added a new deep-select test and adjusted an unauthorized-relations test path.

Changes

Cohort / File(s)	Summary
Nested relation context update src/Http/Response.php	In modelToResponse, replaced references to requestArray with currentRequestArray for nested relation handling: closure capture, recursive calls for single related models, and collection mapping now pass currentRequestArray.
Tests — deep selects & unauthorized include path tests/Feature/Controllers/SearchSelectingOperationsTest.php, tests/Feature/Controllers/SearchIncludingRelationshipsOperationsTest.php	Added test test_getting_a_list_of_resources_deep_selecting_fields (creates related data, sets policies, asserts nested selects) and updated unauthorized-relations test to use hasManyRelation.belongsToRelation for the second include path.
Test resource updates tests/Support/Rest/Resources/HasManyResource.php	Added a BelongsTo relation in relations(), imported BelongsTo, and introduced public function defaultOrderBy(RestRequest $request): array returning ['id' => 'asc'].

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Controller
  participant Response as Response::modelToResponse
  participant Model
  participant Relation as Related Model(s)

  Client->>Controller: POST /api/... (search with includes/selects)
  Controller->>Response: modelToResponse(Model, requestArray)
  activate Response

  Note over Response: Determine include context -> derive currentRequestArray

  alt Single related model
    Response->>Response: modelToResponse(Relation, currentRequestArray)
  else Collection of related models
    loop For each related model
      Response->>Response: modelToResponse(Relation[i], currentRequestArray)
    end
  end

  Response-->>Controller: Serialized payload (with deep selects applied)
  Controller-->>Client: HTTP response

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "🐛 Fix deep selects" directly describes the main change, which is fixing the deep/nested selects functionality that was not working properly in nested includes. The title clearly refers to the primary issue being addressed (issue #196), making it specific and understandable to teammates reviewing the code history. While the emoji is technically noise, the core message is clear and directly relevant to the changeset.
Linked Issues Check	✅ Passed	The PR directly addresses the requirement from issue #196 by fixing how field selections are applied to nested includes. The core fix in src/Http/Response.php replaces the global requestArray with currentRequestArray in nested relation processing, ensuring that nested relations respect their specific field selection context rather than inheriting the global one. Additionally, a new test case test_getting_a_list_of_resources_deep_selecting_fields is added to verify deep selecting works for nested includes, which addresses the reviewer's request for regression test coverage.
Out of Scope Changes Check	✅ Passed	All changes in the pull request are in scope relative to the linked issue. The core fix in src/Http/Response.php directly addresses the deep selects problem, while the test additions verify the fix works correctly. The changes to tests/Support/Rest/Resources/HasManyResource.php (adding a BelongsTo relation and defaultOrderBy method) serve as supporting infrastructure for the new test case that demonstrates the deep selects functionality working through nested includes.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[GautierDele]

As discussed privately, seems ok for me but could you add a test to cover this in the future please ?

[GautierDele]

@Samoht70 any news on this ?