Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRecomputes the Algolia cache key to include a serialized Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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.
1 issue found across 1 file
Confidence score: 3/5
- There is a concrete crash risk in
backend/apps/core/api/internal/algolia.pywhenfacetFiltersincludes nested arrays (valid Algolia OR syntax), which would raise aTypeErrorat runtime. - Because this affects valid query shapes, there is some user-impacting regression risk despite being localized.
- Pay close attention to
backend/apps/core/api/internal/algolia.py- handling of nestedfacetFiltersarrays.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/apps/core/api/internal/algolia.py">
<violation number="1" location="backend/apps/core/api/internal/algolia.py:54">
P1: This will crash with `TypeError` if `facetFilters` contains nested arrays (valid Algolia OR syntax like `[["genre:comedy", "genre:drama"]]`). Consider using `json.dumps(facet_filters, sort_keys=True)` or flattening nested arrays before joining to handle all valid Algolia filter formats.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/apps/core/api/internal/algolia.py`:
- Around line 54-58: The cache-key generation using
"_".join(sorted(facet_filters)) is collision-prone and will break on nested
lists; replace it by serializing facet_filters into a deterministic JSON string
and use that as filters_key (e.g., json.dumps(facet_filters, separators=(",",
":"), sort_keys=True)) so filters_key is collision-free and handles nested
structures; update where filters_key is referenced (the cache_key construction
that combines CACHE_PREFIX, index_name, query, page, limit, and filters_key) to
use this JSON-serialized value.
arkid15r
left a comment
There was a problem hiding this comment.
Please add/update tests.
|
Hi ! @arkid15r do you want me to use |
The suggestion sounds reasonable. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/tests/apps/core/api/internal/algolia_test.py`:
- Around line 252-253: Fix the inline comment and file ending: update the
comment before the assertion to include a space after the `#` and spaces around
the `=` (e.g., change `#backend only called once = caching worked` to `# backend
only called once = caching worked` or better `# backend only called once =
caching worked`), and ensure the test file ends with a trailing newline
character so Ruff W292 is satisfied; the assertion call to locate the spot is
`mock_get_search_results.assert_called_once()`.
🧹 Nitpick comments (1)
backend/tests/apps/core/api/internal/algolia_test.py (1)
31-45: Consider using_build_requestin existing tests for consistency.The helper is well-structured, but the existing
test_algolia_search_valid_request(lines 70–81) andtest_algolia_search_invalid_request(lines 163–173) still manually construct mock requests with the same structure. Consider refactoring them to use_build_requestto reduce duplication.
|
Added 2 tests to verify that different facet filters produce different results while identical requests return cached results. |
|
ready for review @arkid15r |
|
arkid15r
left a comment
There was a problem hiding this comment.
Please start running make check locally.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3825 +/- ##
==========================================
+ Coverage 95.37% 95.38% +0.01%
==========================================
Files 463 463
Lines 14541 14543 +2
Branches 2060 2017 -43
==========================================
+ Hits 13868 13872 +4
+ Misses 329 328 -1
+ Partials 344 343 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
Noted , Thanks! |
* Run make update * Clean up snapshot generated videos * Update backend/data/nest.dump * feat(ui): revamp corporate supporters carousel (Infinite Marquee + Dark Mode fix) (#3837) * feat(ui): revamp corporate supporters carousel (Infinite Marquee + Dark Mode fix) * fix: resolve failing test case * fix: add fallback text for unnamed sponsors * docs: add docstrings to satisfy coverage requirements * Run make check and fix tests. --------- Co-authored-by: Kate <kate@kgthreads.com> * Fix/redundant typescript assertion (#3834) * Fix Sonar S4325 by narrowing session user fields instead of casting * Fix unused ExtendedSession in mentorship page * fix: redundant-typescript-assertion * Fix stale latest date displayed in Project Health Dashboard metrics (#3842) * Fixed latest date in proejct health dashboard * updated order * Update code * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * feat: improve backend test coverage to 96% (#3840) * feat: improve backend test coverage to 96% * fix comments * fix issues * fix issue * fix cubic-dev-ai comments * Update code * Fix tests --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Fix: merge consecutive RUN instructions in frontend Dockerfile (#3644) * Fix: merge consecutive RUN instructions in frontend Dockerfile * fix: comment Dockerfile note to prevent syntax error * Update code * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Fix 'is_merged' not being available on the Issue (#3843) * Fix 'is_merged' not being available on the Issue * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * CI: Add ansible-lint workflow for Ansible playbooks (#3796) * ci: add ansible-lint workflow Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * Update .github/workflows/lint-ansible.yaml Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * ci: add ansible-lint make target and workflow Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * ci: add ansible-lint pre-commit hook Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * fix: whitespace & version Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * Update Makefile Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * ci: enable ansible-lint scanning and add requirements.yml Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * chore(ansible):align linting and module usage Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * ci(ansible): install collections before deploy playbooks Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * Update code * Update code * Update .github/workflows/run-ci-cd.yaml --------- Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * Fix ElevenLabs API error (#3861) * use default liam voice * bump speed by 0.10 --------- Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Add Ime Iyonsi to MENTORS.md (#3866) * Add mentor profile for Ime Iyonsi Added Ime Iyonsi's mentor profile. * Fix GitHub link for Ime Iyonsi Corrected GitHub link for Ime Iyonsi. * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * Update MENTORS.md * Enabled Strict Mode (#3776) * Enabled Strict Mode * fixed ai review * fix * fixed review * fix * update test * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Resolve case-sensitivity in QueryParser to support Chapters/Members search (#3844) * resolve query parser blocker * use case_sensitive flag in QueryParser * feat: add case_sensitive option to QueryParser and update tests * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Update dependencies (#3874) * Update dependencies * Bump django-ninja version * fix(proxy): pin nginx and certbot images (#3848) * fix(proxy): pin nginx and certbot images Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * fix stable verssions Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> --------- Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Update docker-compose/proxy/compose.yaml * Update backend/pyproject.toml * Update ansible lint configuration (#3880) * Update .github/ansible/.ansible-lint.yaml * Improve frontend test coverage above 80% and add missing test files (#3864) * Imrove test coverage to 80% and added test * Fixed coderabbit review * update code * fixed coderabbit ai * fixed soanrqube warning * fixed review * update * fixed aloglia cache_key (#3825) * fixed aloglia cache_key * change separator val to be semicolon (;) * Update code * add tests + use json filters * add trailing newline * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * fix: remove unused className prop from AnchorTitle component (#3822) * fix: remove unused className prop from AnchorTitle component Fixes #3805 The className prop was defined in AnchorTitleProps but never used in the component implementation. Removing it resolves Sonar rule typescript:S6767 and improves code maintainability. * fix: use className prop instead of removing it - Added className back to AnchorTitleProps interface - Accept className parameter in component - Apply className to root div element - Resolves reviewer feedback on PR #3822 * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> --------- Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Yashraj Pahuja <yashrajpahuja9999@gmail.com> Co-authored-by: Kate <kate@kgthreads.com> Co-authored-by: CodeAritraDhank <aritradhank21@gmail.com> Co-authored-by: Anurag Yadav <143180737+anurag2787@users.noreply.github.com> Co-authored-by: Harshit Verma <harshit1092004@gmail.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Co-authored-by: Shuban Mutagi <shubanmutagi55@gmail.com> Co-authored-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: emaybu <152900874+emaybu@users.noreply.github.com> Co-authored-by: sai chethana <saichethanavesireddy@gmail.com> Co-authored-by: Rahul Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Co-authored-by: Lavanya <lavanyayadawad30@gmail.com>





Resolves #3815
Summary
facetFilterswas extracted and used in queries but missing from the cache key.Proposed change
Include facetFilters in the Algolia cache key. Filters are sorted and joined with _ to match existing key style and consistency.
Checklist
make check-testlocally: all warnings addressed, tests passed