Enable multi-verse tafsirs across /verses APIs

[basit3407]

Summary
Add support for returning tafsir content when fetching multiple ayahs—via single ID, multiple IDs, or verse ranges—across the /verses APIs.

Why
Tafsirs previously worked reliably for single-verse lookups. This extends them to multi-verse queries while preventing duplicate rows when joins are combined.

What’s changed

• Whitelist both one-verse and n-verse tafsir resources in verse presenters.
• Ensure V4 and QDC verse finders de-dupe rows when combining multiple tafsir joins.
• Add request specs covering single ID, multiple IDs, multi-verse ranges, and duplicate-free results.
• Pin cld3 in Gemfile.lock to match the declared dependency.

API example

GET /api/v4/verses/by_range?from=2:254&to=2:257&tafsirs=169

Backward compatibility

• Single-verse behavior unchanged.
• Response structure unchanged; only broader query shapes now supported.

Testing

• bundle exec rspec spec/requests/api/v4/verses_tafsirs_spec.rb

Notes

• De-duplication logic verified in specs to avoid repeated tafsir entries across joins.

Summary by CodeRabbit

• Bug Fixes

  • Fixed duplicate tafsir records appearing in API responses.

• New Features

  • Extended support for multi-verse tafsirs alongside single-verse commentaries.
  • Enhanced filtering capabilities for verse-level content resources.

• Tests

  • Added comprehensive API test coverage for tafsir filtering and multi-verse range queries.

[coderabbitai]

Walkthrough

The changes extend tafsir filtering to support multi-verse content. Two new scopes are added to ResourceContent for cardinality-type filtering. Multiple finders and presenters are updated to use a broader scope encompassing both single and multi-verse tafsirs. Query deduplication via .distinct is added to prevent duplicate results from joins.

Changes

Cohort / File(s)	Summary
Query Deduplication app/finders/qdc/verse_finder.rb, app/finders/v4/verse_finder.rb	Added .distinct to load_tafsirs queries to eliminate duplicate tafsir records resulting from joins.
Resource Content Scopes app/models/resource_content.rb	Introduced two new scopes: n_verse filters by CardinalityType::NVerse, and verse_level filters by both CardinalityType::OneVerse and CardinalityType::NVerse.
Presenter Scope Updates app/presenters/qdc/verses_presenter.rb, app/presenters/verses_presenter.rb	Updated fetch_tafsirs to use .verse_level scope instead of .one_verse, broadening the subset of approved tafsirs before filtering.
Test Coverage spec/requests/api/v4/verses_tafsirs_spec.rb	New request spec exercising verse tafsir filtering across single and multi-verse resources, including helpers for test data creation and assertions validating deduplication and range handling.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Presenter
    participant Finder
    participant DB
    
    Client->>Presenter: fetch_tafsirs request
    Presenter->>Presenter: approved_tafsirs.verse_level<br/>(OneVerse + NVerse)
    Presenter->>Presenter: .allowed_to_share<br/>(scope chain)
    Presenter->>Finder: load_tafsirs(relation)
    Finder->>DB: .where().eager_load()<br/>.distinct
    DB-->>Finder: tafsir records (deduplicated)
    Finder-->>Presenter: tafsir set
    Presenter-->>Client: response with tafsirs
    
    Note over Presenter,Finder: verse_level scope now includes<br/>multi-verse (NVerse) content
    Note over Finder,DB: .distinct removes join-induced<br/>duplicate records

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review the new scopes in ResourceContent for correct cardinality-type filtering logic
• Verify the presenter scope chains transitioning from .one_verse to .verse_level and confirm this change is intentional across both presenter files
• Validate that .distinct placement in finders doesn't inadvertently affect eager-loading behavior or query performance
• Examine the comprehensive test spec to ensure helper methods and assertions cover edge cases for single and multi-verse tafsir scenarios, particularly deduplication validation

Poem

🐰 The tafsirs now bloom in multi-verse verse,
No longer one-song, we've broken the curse!
With scopes that expand and queries distinct,
Our tafsir ranges are perfectly synced.
From one verse to many, the finders now hop—
Duplicates fade where the .distinct blocks stop! 🌟

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 (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main objective of the PR: enabling multi-verse tafsirs across /verses APIs by adding support for n-verse tafsir resources, implementing de-duplication, and extending scope filtering.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78dfd2d and 5f6ce2d.

⛔ Files ignored due to path filters (1)

• Gemfile.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• app/finders/qdc/verse_finder.rb (1 hunks)
• app/finders/v4/verse_finder.rb (1 hunks)
• app/models/resource_content.rb (1 hunks)
• app/presenters/qdc/verses_presenter.rb (1 hunks)
• app/presenters/verses_presenter.rb (1 hunks)
• spec/requests/api/v4/verses_tafsirs_spec.rb (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

spec/requests/api/v4/verses_tafsirs_spec.rb (3)

spec/support/response_json.rb (1)

• response_json (4-8)

app/presenters/qdc/verses_presenter.rb (1)

• verses (90-100)

app/presenters/verses_presenter.rb (1)

• verses (174-184)

🔇 Additional comments (6)

app/presenters/verses_presenter.rb (1)

257-274: LGTM: Scope change enables multi-verse tafsir support.

The change from .one_verse to .verse_level correctly broadens the filter to include both single-verse and multi-verse (N-verse) tafsirs. This aligns with the PR objective while maintaining backward compatibility, as verse_level includes OneVerse cardinality.

app/finders/qdc/verse_finder.rb (1)

267-272: LGTM: Deduplication prevents duplicate verse records.

The .distinct call before eager loading is correctly placed to prevent duplicate verse records when a single verse has multiple matching tafsirs (e.g., from both OneVerse and NVerse resources). This is essential for correctness when supporting multi-verse tafsirs.

app/finders/v4/verse_finder.rb (1)

207-212: LGTM: Consistent deduplication across finders.

The .distinct addition mirrors the identical change in app/finders/qdc/verse_finder.rb, ensuring consistent behavior across both V4 and QDC APIs when handling multi-verse tafsirs.

app/presenters/qdc/verses_presenter.rb (1)

126-143: LGTM: Consistent scope change with improved formatting.

The change from .one_verse to .verse_level is consistent with app/presenters/verses_presenter.rb, ensuring uniform behavior across both presenters. Moving .allowed_to_share to its own line also improves readability.

app/models/resource_content.rb (1)

74-75: LGTM: Well-designed scopes for verse-level filtering.

The new n_verse and verse_level scopes follow existing naming conventions and correctly implement cardinality-based filtering. The verse_level scope appropriately combines both OneVerse and NVerse types, providing a clean abstraction for the presenters.

spec/requests/api/v4/verses_tafsirs_spec.rb (1)

1-200: Remove or significantly revise the review comment—it is inaccurate and misleading.

The test suite validates the by_chapter endpoint only. It does not test by_key, random, or by_range endpoints, making the claim of "comprehensive test coverage" incorrect. While the implementation itself appears to handle tafsir filtering consistently across endpoints (the presenter passes fetch_tafsirs uniformly and all finder methods accept the parameter), the test file does not provide evidence of this consistency.

The review contains conflicting guidance: it marks the code as approved with `` while simultaneously asking for verification via manual smoke tests on untested endpoints. This creates ambiguity about whether approval is conditional or unconditional.

Likely an incorrect or invalid review comment.

---

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.}