Skip to content

Conversation

@estyxx
Copy link
Member

@estyxx estyxx commented Jan 28, 2026

Summary

  • Add a dedicated grants_waiting_list_update deadline type instead of using a "custom" deadline with name matching
  • This deadline is used to inform grant applicants on the waiting list when they should expect an update about their application
  • Includes a data migration to convert existing custom deadlines with name "Update Grants in Waiting List" to the new type
  • Updates CLAUDE.md to specify that all Python/Django commands must run inside Docker

Test plan

  • Tests pass: docker exec pycon-backend-1 uv run pytest grants/tests/test_tasks.py -k "waiting_list"

@vercel
Copy link

vercel bot commented Jan 28, 2026

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

Project Deployment Review Updated (UTC)
pycon Ready Ready Preview Jan 28, 2026 6:50pm

@claude

This comment has been minimized.

@estyxx estyxx force-pushed the add-grants-waiting-list-update-deadline-type branch from 5409943 to b65cb2f Compare January 28, 2026 18:25
@claude

This comment has been minimized.

@codecov
Copy link

codecov bot commented Jan 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.15%. Comparing base (efadf5a) to head (c9a9b88).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4544   +/-   ##
=======================================
  Coverage   94.15%   94.15%           
=======================================
  Files         353      353           
  Lines       10339    10342    +3     
  Branches      760      761    +1     
=======================================
+ Hits         9735     9738    +3     
  Misses        502      502           
  Partials      102      102           
🚀 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 estyxx force-pushed the add-grants-waiting-list-update-deadline-type branch from b65cb2f to 7af5e2f Compare January 28, 2026 18:32
@claude

This comment has been minimized.

Replace the awkward filtering by custom type with name matching for
grants waiting list update deadlines with a proper dedicated deadline type.

- Add `grants_waiting_list_update` to Deadline.TYPES enum
- Update `_send_grant_waiting_list_email` to filter by new type
- Add migration to convert existing custom deadlines with name
  "Update Grants in Waiting List" to the new type
- Update CLAUDE.md to specify Docker-based development commands
@estyxx estyxx force-pushed the add-grants-waiting-list-update-deadline-type branch from 7af5e2f to c9a9b88 Compare January 28, 2026 18:49
@estyxx estyxx merged commit f52e441 into main Jan 28, 2026
6 of 7 checks passed
@estyxx estyxx deleted the add-grants-waiting-list-update-deadline-type branch January 28, 2026 18:49
@claude
Copy link
Contributor

claude bot commented Jan 28, 2026

Summary

This PR refactors the grants waiting list deadline handling by introducing a dedicated grants_waiting_list_update deadline type instead of relying on string matching against custom deadlines. The change includes a data migration to convert existing custom deadlines and updates the related task logic to use the new type with proper error handling.

Overall, this is a solid improvement that makes the codebase more maintainable and type-safe. The changes are well-tested and the migration is properly reversible.


Architecture & Design ✅

Strengths:

  • Moving from string-based "magic" matching (name__contains={"en": "Update Grants in Waiting List"}) to a proper enum value is excellent for type safety and maintainability
  • The new deadline type follows the established pattern (similar to grants, cfp, voting, etc.)
  • Proper separation: the deadline type lives in the model, the migration handles data conversion, and the task logic is updated accordingly

Minor consideration:

  • The admin validation at backend/conferences/admin/conference.py:52-61 allows multiple custom deadlines but will now also allow multiple grants_waiting_list_update deadlines since it skips validation for custom types. Consider whether you want to enforce uniqueness for this new type. If yes, update the admin validation logic to exclude it from the type == Deadline.TYPES.custom check.

Testing & Coverage ✅

Strengths:

  • Excellent addition of the test case for missing deadline scenario (test_send_grant_waiting_list_email_missing_deadline)
  • All existing tests updated to use the new deadline type
  • Both waiting list email scenarios are covered (initial and update)

Minor improvement opportunity:
The migration's data conversion logic could benefit from a test, though this is optional. Consider adding a migration test that:

  1. Creates a custom deadline with the old name pattern
  2. Runs the migration
  3. Verifies the type is converted correctly
  4. Tests the reverse migration

Error Handling ✅

Strengths:

  • Proper error handling added when deadline is missing (lines 143-149 in tasks.py)
  • Clear, actionable error messages that include the conference code
  • Appropriate use of ValueError for configuration issues

Performance ✅

No performance concerns. The .filter(type="grants_waiting_list_update").first() query is simple and will benefit from existing database indexes on the deadline type field.


Migration Considerations

Potential issue with JSONField query:
The migration uses name__contains={"en": "Update Grants in Waiting List"} (line 15). This works for PostgreSQL JSONField queries, but be aware:

  • This will only match deadlines where the English translation contains the exact string
  • If any production deadlines have slightly different wording, capitalization, or spacing, they won't be migrated
  • Consider checking production data first to ensure all relevant deadlines will be caught

Suggestion: Before deploying, verify in production:

Deadline.objects.filter(type="custom", name__contains={"en": "Update Grants"}).values('id', 'name', 'conference__code')

CLAUDE.md Changes ✅

The Docker command documentation updates are helpful and clear. Good addition for developer experience.


Recommendations

High priority:
None - the PR is ready to merge from a code quality perspective

Nice to have:

  1. Verify production data matches the migration query before deploying
  2. Consider adding admin validation to prevent multiple grants_waiting_list_update deadlines per conference (if desired)
  3. Optional: Add a migration test

Great work on this refactoring! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants