forgejo_admin/basketball-api
1
0
Fork 1
Code Issues 88 Pull requests 9 Projects Releases Packages Wiki Activity Actions

feat: add generic send_templated_email() + contract EmailType values #298

Merged
forgejo_admin merged 1 commit from 294-generic-send-templated-email into main 2026-04-03 19:30:36 +00:00
Conversation 1 Commits 1 Files changed 4 +410
forgejo_admin commented 2026-04-03 19:26:26 +00:00
Owner
Copy link

Summary

Adds a single reusable send_templated_email() function that accepts a template layout name and data dict, so new email types no longer require new Python functions. Also adds contract_offer and contract_reminder to the EmailType enum with a corresponding Alembic migration.

Changes

  • src/basketball_api/models.py -- added contract_offer and contract_reminder to EmailType enum
  • src/basketball_api/services/email.py -- added send_templated_email() (~50 lines), _html_to_plain_text() helper, and import re
  • alembic/versions/031_add_contract_email_type_enum_values.py -- migration to add enum values to PostgreSQL
  • tests/test_templated_email.py -- 16 tests covering success/failure/missing template/plain text/enum values

Test Plan

  • pytest tests/ -k "test_templated_email or test_models" -v -- 16 targeted tests pass
  • Full suite: 701 tests pass (0 failures)
  • ruff format and ruff check clean

Review Checklist

  • EmailType enum contains contract_offer and contract_reminder
  • Alembic migration 031 supports upgrade and downgrade
  • send_templated_email() creates EmailLog with correct fields
  • Plain text fallback generated from HTML with link URL preservation
  • Existing send functions remain unchanged
  • All 701 tests pass
  • ruff format and lint clean

Related Notes

  • Soft dependency: #293 (MJML templates)
  • Downstream consumer: #295 (blast endpoint)

Related

Closes #294

## Summary Adds a single reusable `send_templated_email()` function that accepts a template layout name and data dict, so new email types no longer require new Python functions. Also adds `contract_offer` and `contract_reminder` to the `EmailType` enum with a corresponding Alembic migration. ## Changes - `src/basketball_api/models.py` -- added `contract_offer` and `contract_reminder` to `EmailType` enum - `src/basketball_api/services/email.py` -- added `send_templated_email()` (~50 lines), `_html_to_plain_text()` helper, and `import re` - `alembic/versions/031_add_contract_email_type_enum_values.py` -- migration to add enum values to PostgreSQL - `tests/test_templated_email.py` -- 16 tests covering success/failure/missing template/plain text/enum values ## Test Plan - `pytest tests/ -k "test_templated_email or test_models" -v` -- 16 targeted tests pass - Full suite: 701 tests pass (0 failures) - `ruff format` and `ruff check` clean ## Review Checklist - [x] EmailType enum contains `contract_offer` and `contract_reminder` - [x] Alembic migration 031 supports upgrade and downgrade - [x] `send_templated_email()` creates EmailLog with correct fields - [x] Plain text fallback generated from HTML with link URL preservation - [x] Existing send functions remain unchanged - [x] All 701 tests pass - [x] ruff format and lint clean ## Related Notes - Soft dependency: #293 (MJML templates) - Downstream consumer: #295 (blast endpoint) ## Related Closes #294
feat: add generic send_templated_email() and contract EmailType values
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
b5960f0843 
Add a single reusable send function that takes a template layout name
and data dict, eliminating the need for new Python functions per email
type. Adds contract_offer and contract_reminder to the EmailType enum
with an Alembic migration, and includes plain text fallback generation
from HTML with link URL preservation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
forgejo_admin commented 2026-04-03 19:27:02 +00:00
Author
Owner
Copy link

QA Review -- PR #298

Scope Verification

  • send_templated_email() signature matches issue spec (tenant, layout, data, to, subject, email_type, db, parent_id, player_id)
  • EmailLog entry created with correct fields (tenant_id, parent_id, player_id, email_type, recipient_email, gmail_message_id)
  • Plain text fallback generated from HTML with link URL preservation
  • EmailType enum contains contract_offer and contract_reminder
  • Alembic migration 031 follows pattern of 027, supports upgrade/downgrade
  • Existing send functions untouched (0 deletions in email.py)

Code Quality

  • send_templated_email() uses keyword-only args after tenant -- good API design preventing positional arg mistakes
  • db is required (not optional) -- correct for a function that always writes EmailLog
  • FileNotFoundError re-raised after logging -- lets caller decide error handling (blast endpoint can skip and continue)
  • Gmail send failures return None without raising -- matches existing pattern in send_jersey_reminder_email, send_interest_notification
  • _html_to_plain_text() handles links, entities, blank lines -- solid plain text fallback
  • Migration uses IF NOT EXISTS for idempotent enum value addition

Test Coverage

  • Success path with parent_id/player_id -- verifies EmailLog fields
  • Success path without parent_id/player_id -- verifies nullable FKs
  • Gmail failure returns None, no EmailLog written
  • Missing template raises FileNotFoundError, no EmailLog written
  • Plain text helper: tag stripping, link preservation, entity decoding, blank line collapsing
  • EmailType enum value assertions including exhaustive set check
  • Full suite: 701 tests pass

Findings

No issues found. Clean, well-scoped implementation that follows existing patterns exactly.

VERDICT: APPROVED

## QA Review -- PR #298 ### Scope Verification - [x] `send_templated_email()` signature matches issue spec (tenant, layout, data, to, subject, email_type, db, parent_id, player_id) - [x] EmailLog entry created with correct fields (tenant_id, parent_id, player_id, email_type, recipient_email, gmail_message_id) - [x] Plain text fallback generated from HTML with link URL preservation - [x] EmailType enum contains `contract_offer` and `contract_reminder` - [x] Alembic migration 031 follows pattern of 027, supports upgrade/downgrade - [x] Existing send functions untouched (0 deletions in email.py) ### Code Quality - [x] `send_templated_email()` uses keyword-only args after `tenant` -- good API design preventing positional arg mistakes - [x] `db` is required (not optional) -- correct for a function that always writes EmailLog - [x] FileNotFoundError re-raised after logging -- lets caller decide error handling (blast endpoint can skip and continue) - [x] Gmail send failures return None without raising -- matches existing pattern in `send_jersey_reminder_email`, `send_interest_notification` - [x] `_html_to_plain_text()` handles links, entities, blank lines -- solid plain text fallback - [x] Migration uses `IF NOT EXISTS` for idempotent enum value addition ### Test Coverage - [x] Success path with parent_id/player_id -- verifies EmailLog fields - [x] Success path without parent_id/player_id -- verifies nullable FKs - [x] Gmail failure returns None, no EmailLog written - [x] Missing template raises FileNotFoundError, no EmailLog written - [x] Plain text helper: tag stripping, link preservation, entity decoding, blank line collapsing - [x] EmailType enum value assertions including exhaustive set check - [x] Full suite: 701 tests pass ### Findings No issues found. Clean, well-scoped implementation that follows existing patterns exactly. **VERDICT: APPROVED**
forgejo_admin deleted branch 294-generic-send-templated-email 2026-04-03 19:30:36 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend

   
status:approved

QA passed, awaiting merge approval

  
status:in-progress

Dev agent is actively working

  
status:needs-fix

QA found issues, back to dev

  
status:qa

PR submitted, awaiting QA review

  
type:bug

Bug fix

  
type:devops

Infrastructure/CI/config work

  
type:feature

New functionality

No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/basketball-api!298
No description provided.