fix: add first_payment to EmailType test expected values

forgejo_admin commented

2026-04-07 18:15:56 +00:00

Owner

Summary

PR #370 added first_payment EmailType enum but didn't update test_all_expected_values
This broke CI for all subsequent merges (#377, #378) — update-kustomize-tag skipped, no deploys
One-line fix: add "first_payment" to the expected set

Changes

tests/test_templated_email.py: add "first_payment" to expected set in test_all_expected_values

Test Plan

CI passes end-to-end including update-kustomize-tag
No regressions in email template tests

Review Checklist

No secrets committed
No unnecessary file changes
Commit messages are descriptive

Closes #381
Unblocks deploy of #372 (teams 422 fix) and #374 (photo placeholder fix)
westside-basketball — project affected by blocked deploys

## Summary - PR #370 added `first_payment` EmailType enum but didn't update `test_all_expected_values` - This broke CI for all subsequent merges (#377, #378) — `update-kustomize-tag` skipped, no deploys - One-line fix: add `"first_payment"` to the expected set ## Changes - `tests/test_templated_email.py`: add `"first_payment"` to `expected` set in `test_all_expected_values` ## Test Plan - [ ] CI passes end-to-end including `update-kustomize-tag` - [ ] No regressions in email template tests ## Review Checklist - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive ## Related Notes - Closes #381 - Unblocks deploy of #372 (teams 422 fix) and #374 (photo placeholder fix) - `westside-basketball` — project affected by blocked deploys

forgejo_admin added 1 commit

2026-04-07 18:15:56 +00:00

fix: add first_payment to EmailType test expected values 71d448417a

PR #370 added the first_payment EmailType enum value but didn't update
the test_all_expected_values test. This broke CI for all subsequent merges.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-04-07 18:17:25 +00:00

Author

Owner

PR #382 Review

DOMAIN REVIEW

Stack: Python / FastAPI / SQLAlchemy (basketball-api)

This is a one-line test fix. PR #370 added first_payment to the EmailType enum and introduced test_templated_email.py with a test_all_expected_values test that asserts all enum values are present in an expected set. The first_payment value was added to the enum but omitted from that expected set, causing CI to fail on every subsequent merge.

Correctness: The fix adds "first_payment" to the expected set at the correct location (after sponsor_outreach, before the closing brace). The test pattern expected == {e.value for e in EmailType} will catch any future drift in either direction -- this is a good guard test.

Completeness: Searched all test files referencing EmailType. No other tests enumerate the full set of values, so this is the only place that needed updating. Tests in test_tryouts.py, test_groupme.py, test_admin_email.py, and test_admin_registration_notification.py reference individual enum values, not the complete set.

Scope: Exactly one file changed, one line added. No unnecessary changes.

BLOCKERS

None.

NITS

Branch naming: Branch is fix-email-type-test rather than 381-fix-email-type-test. Convention is {issue-number}-{kebab-case-purpose}. Minor for a hotfix, but worth noting.

SOP COMPLIANCE

Branch named after issue (fix-email-type-test missing 381- prefix)
PR body follows template (Summary, Changes, Test Plan, Related)
Related references issue #381 and explains downstream impact (#372, #374 unblocked)
No secrets committed
No unnecessary file changes
Commit message is descriptive

PROCESS OBSERVATIONS

This is a CI-unblocking hotfix. The root cause is a process gap: PR #370 added an enum value but the reviewer (human or automated) did not catch the missing test update. This is exactly the class of bug that enum completeness tests are designed to catch -- the test was correct, it just wasn't updated alongside the enum change. Future PRs that add enum values should treat the completeness test as a mandatory co-change. No DORA metric impact beyond the deploy blockage this fix resolves.

VERDICT: APPROVED

## PR #382 Review ### DOMAIN REVIEW **Stack**: Python / FastAPI / SQLAlchemy (basketball-api) This is a one-line test fix. PR #370 added `first_payment` to the `EmailType` enum and introduced `test_templated_email.py` with a `test_all_expected_values` test that asserts all enum values are present in an expected set. The `first_payment` value was added to the enum but omitted from that expected set, causing CI to fail on every subsequent merge. **Correctness**: The fix adds `"first_payment"` to the expected set at the correct location (after `sponsor_outreach`, before the closing brace). The test pattern `expected == {e.value for e in EmailType}` will catch any future drift in either direction -- this is a good guard test. **Completeness**: Searched all test files referencing `EmailType`. No other tests enumerate the full set of values, so this is the only place that needed updating. Tests in `test_tryouts.py`, `test_groupme.py`, `test_admin_email.py`, and `test_admin_registration_notification.py` reference individual enum values, not the complete set. **Scope**: Exactly one file changed, one line added. No unnecessary changes. ### BLOCKERS None. ### NITS - **Branch naming**: Branch is `fix-email-type-test` rather than `381-fix-email-type-test`. Convention is `{issue-number}-{kebab-case-purpose}`. Minor for a hotfix, but worth noting. ### SOP COMPLIANCE - [ ] Branch named after issue (`fix-email-type-test` missing `381-` prefix) - [x] PR body follows template (Summary, Changes, Test Plan, Related) - [x] Related references issue #381 and explains downstream impact (#372, #374 unblocked) - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit message is descriptive ### PROCESS OBSERVATIONS This is a CI-unblocking hotfix. The root cause is a process gap: PR #370 added an enum value but the reviewer (human or automated) did not catch the missing test update. This is exactly the class of bug that enum completeness tests are designed to catch -- the test was correct, it just wasn't updated alongside the enum change. Future PRs that add enum values should treat the completeness test as a mandatory co-change. No DORA metric impact beyond the deploy blockage this fix resolves. ### VERDICT: APPROVED