Update strawberry_django decorators#3404
Conversation
Summary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR removes select_related and prefetch_related optimizations from multiple GraphQL query and node field decorators across GitHub and OWASP backend modules, while selectively adding select_related hints to five UserNode fields. It also introduces a GenericRelation to RepositoryBasedEntityModel and refactors entity_leaders logic to use this relation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
PR validation failed: No linked issue and no valid closing issue reference in PR description |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/tests/apps/owasp/api/internal/nodes/common_test.py`:
- Around line 16-24: The test incorrectly expects the resolver
GenericEntityNode.entity_leaders to call
mock_entity.entity_leaders.order_by("order"), but the resolver simply returns
root.entity_leaders (ordering is handled by the model property). Update the test
to stop asserting order_by was called: remove or replace
mock_entity.entity_leaders.order_by.assert_called_once_with("order") and instead
assert the resolver returned the mock_entity.entity_leaders value (e.g., keep
result == [mock_leader1, mock_leader2] or set mock_entity.entity_leaders to the
ordered list before calling GenericEntityNode.entity_leaders).
🧹 Nitpick comments (2)
backend/apps/owasp/api/internal/queries/member_snapshot.py (1)
64-67: Avoid the extra lookup query foruser_login.You can filter by the related login directly to avoid the
User.objects.getround‑trip and exception handling.♻️ Proposed refactor
- if user_login: - try: - snapshots = snapshots.filter(github_user=User.objects.get(login=user_login)) - except User.DoesNotExist: - return [] + if user_login: + snapshots = snapshots.filter(github_user__login=user_login)backend/apps/owasp/models/common.py (1)
101-104: Minor: Return type annotation mismatch.The property returns a
QuerySet, not alist[EntityMember]. While this works in practice since QuerySets are iterable, consider updating the annotation for accuracy:♻️ Suggested type annotation fix
+from django.db.models import QuerySet + `@cached_property` -def entity_leaders(self) -> list[EntityMember]: +def entity_leaders(self) -> QuerySet[EntityMember]: """Return entity's leaders.""" return self.entity_members.filter(role=EntityMember.Role.LEADER).order_by("order")
|
* Update strawberry_django decorators * Update entity_leaders * Update tests * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
* Update strawberry_django decorators * Update entity_leaders * Update tests * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>



Proposed change
Resolves #(put the issue number here)
Add the PR description here.
Checklist
make check-testlocally: all warnings addressed, tests passed