-
Notifications
You must be signed in to change notification settings - Fork 24
Add invitation letter request flag to Grant admin #4468
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
Conversation
Display a boolean flag in the Grant admin change list view that indicates whether a request for an invitation letter has been submitted. This helps administrators quickly identify which grants have associated invitation letter requests. Changes: - Added has_invitation_letter_request() method to Grant model - Added has_invitation_letter_request_flag display method to GrantAdmin - Added flag to admin list_display with 📧 emoji icon Fixes #4384 Co-authored-by: Marco Acierno <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
backend/grants/models.py
Outdated
| return False | ||
|
|
||
| from visa.models import InvitationLetterRequest | ||
| return InvitationLetterRequest.objects.filter( |
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.
@claude This would cause a N+1 in the admin page. I think we use Django's annotate to fetch if the invitation letter exists or not in the django admin query instead of this method
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.
Claude finished @marcoacierno's task —— View job
N+1 Query Issue Fixed ✅
You're absolutely right about the N+1 issue! I've optimized the implementation using Django's Exists() annotation.
Todo List:
- Read repository guidelines (CLAUDE.md)
- Examine current implementation
- Find invitation letter model relationship
- Implement queryset annotation in GrantAdmin
- Update/remove the model method
- Test the optimization
- Commit and push changes
Changes Made:
-
Added annotation to
GrantAdmin.get_queryset():- Uses
Exists()withInvitationLetterRequest.objects.filter()to check for invitation letter requests - This performs a single SQL query with an EXISTS subquery instead of N individual queries
- Uses
-
Updated
has_invitation_letter_request_flag()method:- Now uses
getattr(obj, 'has_invitation_letter_request', False)to access the annotated field - No longer calls the model method that would trigger individual queries
- Now uses
-
Removed the
has_invitation_letter_request()method from Grant model:- This method is no longer needed since we're using annotation
- Eliminates the source of the N+1 queries
Performance Impact:
- Before: 1 query to load grants + N queries for invitation letter checks = N+1 queries
- After: 1 query with EXISTS subquery = 1 query total
The optimization follows the same pattern already used for is_proposed_speaker, is_confirmed_speaker, and has_voucher annotations in the same admin class.
Use Django''s Exists annotation in get_queryset() instead of calling has_invitation_letter_request() method for each grant in the admin list. This optimizes the query to use a single SQL query with EXISTS subquery rather than individual queries for each grant row. Co-authored-by: Marco Acierno <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4468 +/- ##
=======================================
Coverage 94.53% 94.53%
=======================================
Files 352 352
Lines 10131 10131
Branches 733 733
=======================================
Hits 9577 9577
Misses 461 461
Partials 93 93 🚀 New features to boost your workflow:
|
estyxx
left a 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.
LGTM! 🙏
Display a boolean flag in the Grant admin change list view that indicates whether a request for an invitation letter has been submitted. This helps administrators quickly identify which grants have associated invitation letter requests.
Changes:
Fixes #4384
Generated with Claude Code