Skip to content

Conversation

@ArnabChatterjee20k
Copy link
Contributor

@ArnabChatterjee20k ArnabChatterjee20k commented Sep 25, 2025

  • Previously, Selecting attributes in a relationship didn't set decoded values in the top-level collection of the the final document in getDocument and find

  • Currently, its setting the filtered values to top-level collection if required

Summary by CodeRabbit

  • Bug Fixes

    • Improved selective field decoding with nested relationship selections for more consistent results.
    • Ensured non-relationship fields remain accurate when requesting specific related fields.
    • Improved behavior when multiple value filters are applied during selection.
  • Tests

    • Added end-to-end tests covering mixed selection types (including relationships and nested fields).
    • Added tests for decoding without relationships.
    • Added tests validating combined filters during field selection.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Adds conditional post-processing in Database::decode to handle nested relationship selections by caching transformed attribute values and restoring non-requested non-relationship fields when relationships are partially selected. Introduces comprehensive tests covering selective decoding with relationships, without relationships, and with multiple filters.

Changes

Cohort / File(s) Summary
Decode relationship post-processing
src/Database/Database.php
Adds filteredValue cache, detects presence of relationship selections, and post-processes decoded documents to restore non-requested non-relationship fields when relationships are partially selected.
E2E selective decode tests
tests/e2e/Adapter/Scopes/DocumentTests.php
Adds three public test methods validating decode behavior across selection types: with relationships, without relationships, and with multiple filters (upperCase, prefix).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant DB as Database::decode
  participant F as Filters
  participant R as Relationship Logic

  Client->>DB: decode(document, selection)
  DB->>F: apply attribute filters
  DB->>DB: cache transformed values (filteredValue)
  alt selection includes relationships
    DB->>R: process relationship selections
    DB->>DB: post-process non-relationship fields using filteredValue
  else
    DB->>DB: standard decode (no relationship post-processing)
  end
  DB-->>Client: decoded document
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • abnegate
  • fogelito

Poem

Thump-thump goes my code-hare heart,
I sift fields gently, pull threads apart—
Relations peek, then tuck back in,
Filters hum with a tidy grin.
Hop, decode, restore the sheen—
A carrot-bright result, crisp and clean. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title “fix relationship top level filter” touches on the subject of the change but is too terse and ambiguous to clearly convey that the pull request ensures filtered relationship fields are now propagated into the top-level document. It does not specify the action being fixed (propagation of filtered values) or the context of decoding and field selection, which may hinder quick understanding when scanning history. Consider rephrasing the title to clearly describe the change, for example: “Fix propagation of selected relationship fields to top-level documents,” so that it immediately communicates the core behavior update.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-relationship-top-level-filter

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 732ffef and 616b04e.

📒 Files selected for processing (2)
  • src/Database/Database.php (3 hunks)
  • tests/e2e/Adapter/Scopes/DocumentTests.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/Database/Database.php (1)
src/Database/Document.php (2)
  • getAttribute (224-231)
  • setAttribute (244-261)
tests/e2e/Adapter/Scopes/DocumentTests.php (5)
src/Database/Database.php (3)
  • addFilter (6659-6665)
  • createAttribute (1692-1747)
  • Database (37-7355)
src/Database/Adapter.php (3)
  • getSupportForRelationships (993-993)
  • getSupportForSpatialAttributes (1058-1058)
  • createAttribute (558-558)
src/Database/Adapter/SQL.php (3)
  • getSupportForRelationships (1470-1473)
  • getSupportForSpatialAttributes (1490-1493)
  • createAttribute (234-248)
src/Database/Adapter/Postgres.php (2)
  • getSupportForSpatialAttributes (1949-1952)
  • createAttribute (444-463)
src/Database/Query.php (2)
  • Query (8-1075)
  • select (576-579)
🪛 PHPMD (2.15.0)
tests/e2e/Adapter/Scopes/DocumentTests.php

6100-6100: Avoid unused local variables such as '$createdStore'. (undefined)

(UnusedLocalVariable)


6239-6239: Avoid unused local variables such as '$created'. (undefined)

(UnusedLocalVariable)


6322-6322: Avoid unused local variables such as '$created'. (undefined)

(UnusedLocalVariable)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Setup & Build Docker Image

@abnegate abnegate merged commit daf0276 into main Sep 25, 2025
15 checks passed
@abnegate abnegate deleted the fix-relationship-top-level-filter branch September 25, 2025 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants