-
Notifications
You must be signed in to change notification settings - Fork 24
Track grant history #4529
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
Track grant history #4529
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4529 +/- ##
=======================================
Coverage 94.42% 94.43%
=======================================
Files 352 352
Lines 10157 10162 +5
Branches 738 738
=======================================
+ Hits 9591 9596 +5
Misses 468 468
Partials 98 98 🚀 New features to boost your workflow:
|
d06726b to
f972928
Compare
f972928 to
90b3468
Compare
90b3468 to
ae42d15
Compare
|
@claude do the code review |
|
Claude finished @estyxx's task in 1m 29s —— View job Code Review Complete
SummaryThis PR adds comprehensive audit logging for grant history tracking across the application. The implementation is well-structured and thorough, covering grant mutations, admin actions, and review session operations. Strengths
Suggestions1. Consider logging reimbursement amount changesIn # backend/reviews/admin.py:377-387
reimbursement, created = (
GrantReimbursement.objects.update_or_create(
grant=grant,
category_id=reimbursement_category_id,
defaults={
"granted_amount": reimbursement_categories[
reimbursement_category_id
].max_amount
},
)
)
if created:
create_addition_admin_log_entry(...)Consider adding a check for updates when 2. Minor: Transaction boundaries in mutationsIn # backend/api/grants/mutations.py:306-311
for attr, value in asdict(input).items():
setattr(instance, attr, value)
create_change_admin_log_entry(request.user, instance, "Grant updated")
instance.save()If
3. Consistency in log message prefixesSome messages use 4. Test cleanup: Print statement in test fileIn for e in LogEntry.objects.all():
print(e.change_message)Consider removing this before merging. Overall AssessmentThis is a solid implementation. The audit logging will provide valuable visibility into grant lifecycle changes. The code follows existing patterns in the codebase and is well-tested. The two minor suggestions above (transaction safety and removing the debug print) are worth addressing, but they don't block the PR. |
did i miss to log any actions for the grants? |
|
Claude finished @estyxx's task in 7m 21s —— View job Grant History Review Complete
SummaryChanges Made (commit d83b221)Improved audit log message consistency:
Grant Actions Coverage AnalysisAll grant-related actions are now properly logged:
Message Format ConventionMessages now follow a consistent pattern:
No missing grant actions were identified. The audit logging covers the full grant lifecycle. | | Branch: |
- Add periods at the end of all audit log messages for consistency - Simplify [Review Session] pending status message to match other patterns - Update all related tests to reflect the new message formats Co-authored-by: Ester Beltrami <estyxx@users.noreply.github.com>
What
ToDo
Attachments
Grant created by grantee
Grant created by admin
Grant updated by grantee
Grant changes in review session:
Confirm pending status (action)
Send email approved (action)
Grantee reply
accepts

or refuse the grant

admin changes