-
-
Notifications
You must be signed in to change notification settings - Fork 314
removed indexing from repositories and projects that do not belong to any organization #2758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe changes add organization-based filtering to repository and project indexing logic. Repositories and projects without associated organizations are now excluded from the indexed queryset. Updated tests validate the new filtering behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/tests/apps/owasp/index/registry/project_test.py (1)
1-1: Consider adding__init__.pyto the test directory.The static analysis tool flags that this file is part of an implicit namespace package. While this doesn't break functionality in Python 3.3+, adding an
__init__.pyfile to the test directory improves compatibility and makes the package structure explicit.Create an empty
__init__.pyfile inbackend/tests/apps/owasp/index/registry/directory:# backend/tests/apps/owasp/index/registry/__init__.py
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/apps/github/index/registry/repository.py(1 hunks)backend/apps/owasp/index/registry/project.py(1 hunks)backend/tests/apps/github/index/registry/repository_test.py(1 hunks)backend/tests/apps/owasp/index/registry/project_test.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-23T11:37:26.253Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2606
File: backend/apps/api/rest/v0/project.py:43-48
Timestamp: 2025-11-23T11:37:26.253Z
Learning: In the OWASP Nest backend, `entity_leaders` is a `property` method defined in `RepositoryBasedEntityModel` (backend/apps/owasp/models/common.py) that returns a dynamically constructed QuerySet. It cannot be prefetched using standard `prefetch_related()` because Django's prefetch mechanism only works on model fields and relations, not property methods.
Applied to files:
backend/apps/owasp/index/registry/project.py
🧬 Code graph analysis (1)
backend/tests/apps/owasp/index/registry/project_test.py (1)
backend/apps/owasp/index/registry/project.py (4)
ProjectIndex(8-115)configure_replicas(82-99)update_synonyms(102-104)get_entities(106-115)
🪛 Ruff (0.14.6)
backend/tests/apps/owasp/index/registry/project_test.py
1-1: File backend/tests/apps/owasp/index/registry/project_test.py is part of an implicit namespace package. Add an __init__.py.
(INP001)
🔇 Additional comments (4)
backend/apps/github/index/registry/repository.py (1)
73-78: LGTM!The addition of
organization__isnull=Falsecorrectly filters repositories to include only those associated with an organization. The filter is applied beforeprefetch_related, which is the optimal order for query performance.backend/tests/apps/github/index/registry/repository_test.py (1)
32-49: LGTM!The test correctly validates the updated queryset construction with the new
organization__isnull=Falsefilter. The mock setup and assertions appropriately verify both the filter arguments and the prefetch_related call.backend/tests/apps/owasp/index/registry/project_test.py (1)
1-58: Excellent test coverage!The test suite comprehensively validates ProjectIndex behavior including class attributes, delegation to parent methods, and the queryset construction in
get_entities. The mock setup correctly reflects the implementation's chaining ofprefetch_related(),filter(), anddistinct().backend/apps/owasp/index/registry/project.py (1)
108-115: The ManyToMany filtering approach is correct.The code already includes
.distinct()which properly handles the duplication issue that occurs when filtering ManyToMany relationships withorganizations__isnull=False. Django's__isnull=Falseon ManyToMany fields performs an INNER JOIN that produces duplicate parent rows for each matching related object, but the.distinct()call removes these duplicates as intended. The current implementation is appropriate and no changes are necessary.
|



… an organization
Proposed change
Resolves #2700
This PR updates indexing logic so that only repositories and projects associated with an Organization are indexed
Changes:
Repositories: Updated
RepositoryIndex.get_entitiesinbackend/apps/github/index/registry/repository.pyto filter fororganization__isnull=FalseProjects: Updated
ProjectIndex.get_entitiesinbackend/apps/owasp/index/registry/project.pyto filter fororganizations__isnull=FalseAdded.distinct()to the project queryset to prevent duplicate entries from arisingChecklist
make check-testlocally; all checks and tests passed.