Skip to content

fix(facet-count): count library facets by document#4169

Open
anouillz wants to merge 1 commit into
rero:stagingfrom
anouillz:goa-correct-facet-counts
Open

fix(facet-count): count library facets by document#4169
anouillz wants to merge 1 commit into
rero:stagingfrom
anouillz:goa-correct-facet-counts

Conversation

@anouillz

Copy link
Copy Markdown
Contributor

Note: this change adds new indexed fields for document facet counts. Existing records need to be reindexed for the corrected organisation/library/location counts to be available.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Document indexing now stores document-level organisation, library, and location keyword lists derived from holdings. Facet configuration uses these fields instead of nested holding aggregations. DocumentJSONSerializer reconstructs nested facet buckets, applies view-specific filtering, enriches names, and removes empty buckets. A unit test verifies the resulting hierarchy and counts.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the main change: fixing facet counts by indexing document-based paths.
Description check ✅ Passed The description matches the changes: indexed facet paths, document counts, and facet-tree rebuild.
Linked Issues check ✅ Passed The PR addresses #4003 by fixing library facet counts to reflect documents instead of holdings.
Out of Scope Changes check ✅ Passed The code changes all support the facet-count fix and add no obvious unrelated functionality.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rero_ils/modules/documents/serializers/json.py`:
- Around line 201-202: Update the organisation aggregation handling in the
serializer to materialize the doc_count filter as a list and assign that list
back to aggregations["organisation"]["buckets"]. Keep the existing nonzero-count
predicate and subsequent iteration unchanged so global views serialize only
populated organisation buckets.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d598bd13-417d-475a-9d9d-ebcc6be1c1d0

📥 Commits

Reviewing files that changed from the base of the PR and between 2effdad and d26a837.

📒 Files selected for processing (5)
  • rero_ils/config.py
  • rero_ils/modules/documents/dumpers/indexer.py
  • rero_ils/modules/documents/mappings/v7/documents/document-v0.0.1.json
  • rero_ils/modules/documents/serializers/json.py
  • tests/unit/documents/test_document_organisation_facets.py

Comment thread rero_ils/modules/documents/serializers/json.py Outdated
@anouillz
anouillz force-pushed the goa-correct-facet-counts branch from d26a837 to 80d6f1b Compare July 16, 2026 15:47
@anouillz
anouillz force-pushed the goa-correct-facet-counts branch 3 times, most recently from 43c09d0 to f5786ca Compare July 20, 2026 09:50
@anouillz
anouillz marked this pull request as draft July 20, 2026 11:20
* Replaces nested facet aggregations with indexed organisation library
  location paths.
* Stores unique pid combinations in organisation_library_location.
* Uses those paths to count documents instead of holdings.
* Rebuilds the same facet tree for the UI.
* Replaces rero/rero-ils-ui#1508 and
  rero/ng-core#831.
* Closes rero#4003

Co-Authored-by: Ana Gomes <anagoncalves7@icloud.com>
@anouillz
anouillz force-pushed the goa-correct-facet-counts branch from f5786ca to d8effb5 Compare July 20, 2026 11:47
@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 91.29% (-0.01%) from 91.303% — anouillz:goa-correct-facet-counts into rero:staging

@anouillz
anouillz marked this pull request as ready for review July 20, 2026 14:53
@anouillz
anouillz requested review from Garfield-fr, PascalRepond, jma and rerowep and removed request for Garfield-fr, PascalRepond, jma and rerowep July 21, 2026 06:52
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.

The organisation/library facet counts are incorrect

2 participants