fix(aggregations): update content of the intitial closed aggs.#841
Conversation
WalkthroughThe aggregation enrichment logic now explicitly sets Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
There was a problem hiding this comment.
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
`@projects/rero/ng-core/src/lib/record/component/search/store/features/with-aggregations.feature.ts`:
- Line 174: Update the enriched aggregation handling around the included field
so it does not unconditionally set included to true. Preserve the existing
included state, or derive it from the request that triggered enrichment,
ensuring closed aggregations remain false and facetsParameter excludes them from
subsequent searches.
🪄 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: 4549ddf0-3f27-4c5d-b23e-11d8930e1435
📒 Files selected for processing (1)
projects/rero/ng-core/src/lib/record/component/search/store/features/with-aggregations.feature.ts
| type: esAggregation.type || aggregation.type || 'terms', | ||
| config: esAggregation.config || aggregation.config, | ||
| name: aggregation.name || esAggregation.name, | ||
| included: true, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not unconditionally mark enriched aggregations as included.
This overwrites included: false for closed aggregations returned by Elasticsearch. Since facetsParameter uses included to decide which aggregations to request, this can cause closed facets to be included in subsequent searches; the supplied spec explicitly expects enriched closed aggregations to remain false. Preserve the existing state or derive inclusion from the specific request that triggered enrichment.
🤖 Prompt for 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.
In
`@projects/rero/ng-core/src/lib/record/component/search/store/features/with-aggregations.feature.ts`
at line 174, Update the enriched aggregation handling around the included field
so it does not unconditionally set included to true. Preserve the existing
included state, or derive it from the request that triggered enrichment,
ensuring closed aggregations remain false and facetsParameter excludes them from
subsequent searches.
Set `included` property to true when an aggregation is proccessed in order to be added for the next fetchRecords call. * Closes rero/rero-ils#4177. Co-Authored-By: Johnny Mariéthoz <johnny.mariethoz@rero.ch> Co-Authored-By: Bertrand Zuchuat <bertrand.zuchuat@rero.ch>
Set
includedproperty to true when an aggregation is proccessed in order to be added for the next fetchRecords call.Co-Authored-By: Johnny Mariéthoz johnny.mariethoz@rero.ch
Co-Authored-By: Bertrand Zuchuat bertrand.zuchuat@rero.ch