Extract grants and proposals logic into adapter pattern

[estyxx]

What

ToDo

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
pycon	Ready	Preview	Jan 19, 2026 1:34pm

[codecov]

Codecov Report

❌ Patch coverage is 60.45198% with 70 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.85%. Comparing base (58ffa06) to head (f9a044f).

Additional details and impacted files

@@                            Coverage Diff                             @@
##           grants/review-recap-add-internal-notes    #4534      +/-   ##
==========================================================================
- Coverage                                   94.43%   93.85%   -0.58%     
==========================================================================
  Files                                         352      353       +1     
  Lines                                       10162    10339     +177     
  Branches                                      738      761      +23     
==========================================================================
+ Hits                                         9596     9704     +108     
- Misses                                        468      532      +64     
- Partials                                       98      103       +5     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[estyxx]

@claude do the code review

[claude]

Claude finished @estyxx's task in 1m 23s —— View job

---

Code Review

This PR extracts the grants and proposals logic from admin.py into an adapter pattern in a new adapters.py file. The refactoring significantly reduces complexity in the admin module (from ~927 lines to ~389 lines) by moving domain-specific logic to dedicated adapter classes.

---

Overall Assessment: ✅ Approve with suggestions

The adapter pattern is well-implemented and improves code organization and maintainability. The separation of concerns between proposals and grants review logic is clean.

---

Strengths

1. Clean Protocol definition (adapters.py:53-155): The ReviewAdapter Protocol provides a clear interface that both adapters must implement, making the contract explicit.

2. Good separation of concerns: Each adapter encapsulates all logic specific to its review type (proposals vs grants), making the code easier to test and maintain.

3. Comprehensive docstrings: The module-level docstring and Protocol method docstrings clearly explain the purpose and usage.

4. Factory function (adapters.py:735-754): The get_review_adapter() function cleanly handles adapter selection with proper error handling for unknown session types.

5. Simplified admin.py: The admin module is now focused on Django admin mechanics rather than business logic.

---

Suggestions for Improvement

1. Missing test coverage for the adapters module

The codecov report shows 59.32% patch coverage with 72 lines missing coverage. Consider adding tests for:

• ProposalsReviewAdapter.get_recap_context()
• ProposalsReviewAdapter.process_recap_post()
• ProposalsReviewAdapter.get_review_context()
• get_review_adapter() with invalid session type

Add adapter tests →

2. Unused exclude parameter in GrantsReviewAdapter.get_next_to_review_item_id()

At adapters.py:702:

exclude = exclude or []

The exclude parameter is set but never used in the grants adapter (unlike the proposals adapter where it filters by tags at line 384-385). If grants don't support tag filtering, consider either:

• Removing the parameter from the grants adapter
• Adding a comment explaining why it's intentionally unused

Fix unused exclude →

3. Consider making adapters stateless singletons

Both adapter classes are instantiated fresh on every call to get_review_adapter() (adapters.py:749, adapters.py:752). Since they're stateless, you could use module-level singletons:

_PROPOSALS_ADAPTER = ProposalsReviewAdapter()
_GRANTS_ADAPTER = GrantsReviewAdapter()

def get_review_adapter(review_session: ReviewSession) -> ReviewAdapter:
    if review_session.is_proposals_review:
        return _PROPOSALS_ADAPTER
    if review_session.is_grants_review:
        return _GRANTS_ADAPTER
    raise ValueError(...)

This is a minor optimization and not required.

4. Type annotation mismatch in Protocol

The Protocol at adapters.py:71-82 shows get_recap_items_queryset returning QuerySet (generic), but the concrete implementations return QuerySet[Submission] and QuerySet[Grant]. This is fine in practice but you could consider using a generic type parameter for better static analysis.

5. Potential optimization in process_recap_post

In GrantsReviewAdapter.process_recap_post() (adapters.py:592-602), grants are saved individually in a loop. You already use bulk_update in ProposalsReviewAdapter (adapters.py:263-266). For consistency, consider if bulk operations could be used here too, though the audit logging complexity may make this impractical.

---

Minor Issues

1. adapters.py:293: languages variable is assigned but only used for the language check. The template context at line 318 fetches proposal.languages.all() again instead of reusing languages.

2. adapters.py:639-642: Internal notes update does a second query for grants. This could potentially be combined with the earlier grant loop, though the separation may be intentional for clarity.

---

Tests Review

The test changes look good. Tests now correctly use get_review_adapter() instead of directly testing admin methods. The test coverage for grants review scenarios is comprehensive.

---

The PR is well-executed and achieves its goal of separating concerns. The main actionable item is improving test coverage for the proposals adapter methods.