Public Access
docs: Phase 6 SendGrid implementation plan
This commit is contained in:
@@ -0,0 +1,877 @@
|
|||||||
|
# Phase 6 — SendGrid Email Integration Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Replace the `ConsoleEmailBackend` stub with a live SendGrid backend, add a pre-deadline reminder step, and fix two hygiene issues from Phase 5.
|
||||||
|
|
||||||
|
**Architecture:** `SendGridEmailBackend.send()` is wired using the `sendgrid` Python library already present in `requirements.txt`. Sender identity (`from_email`, `reply_to`) comes from two new Settings fields. A new `step_reminder` orchestrator step (idempotent via `reminded_at` column, added by migration 0009) fires at Fri 16:00 PT and emails only members who have un-voted PENDING items. The `STEPS` tuple in `runner.py` grows from 5 to 6; admin and scheduler get matching updates.
|
||||||
|
|
||||||
|
**Tech Stack:** Python 3.11, FastAPI 0.109, SQLAlchemy 2.0, Alembic 1.13, sendgrid 6.12, APScheduler 3.10, pytest 7.4. All tests in `backend/tests/`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## File map
|
||||||
|
|
||||||
|
| File | Action |
|
||||||
|
|---|---|
|
||||||
|
| `backend/alembic/versions/0009_phase6_reminded_at.py` | **Create** — add `reminded_at TIMESTAMPTZ NULL` to `weekly_run` |
|
||||||
|
| `backend/app/models/__init__.py` | **Modify** — add `reminded_at` column to `WeeklyRun` |
|
||||||
|
| `backend/app/services/email.py` | **Modify** — wire `SendGridEmailBackend`; keep `ConsoleEmailBackend` unchanged |
|
||||||
|
| `backend/app/config.py` | **Modify** — add `SENDGRID_FROM_EMAIL` and `SENDGRID_REPLY_TO` |
|
||||||
|
| `backend/tests/test_email_backend.py` | **Create** — 2 unit tests for `SendGridEmailBackend` |
|
||||||
|
| `backend/app/services/orchestrator/steps.py` | **Modify** — add `step_reminder`; add `html` + `MealPlanVote` imports; html-escape (#P5-a); fix getattr (#P5-b) |
|
||||||
|
| `backend/app/services/orchestrator/runner.py` | **Modify** — add `"reminder"` to `STEPS` tuple and `step_fns` dict |
|
||||||
|
| `backend/app/api/admin.py` | **Modify** — add `"reminder"` to `_VALID_STEPS` |
|
||||||
|
| `backend/app/scheduler/__main__.py` | **Modify** — add 6th job Fri 16:00 PT; update docstring |
|
||||||
|
| `backend/tests/test_orchestrator.py` | **Modify** — add `weekly_run_reminded` fixture + 4 `step_reminder` tests |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 1: Migration 0009 — add `reminded_at` to `weekly_run`
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create: `backend/alembic/versions/0009_phase6_reminded_at.py`
|
||||||
|
- Modify: `backend/app/models/__init__.py`
|
||||||
|
|
||||||
|
### Context
|
||||||
|
|
||||||
|
`WeeklyRun` is defined at the bottom of `backend/app/models/__init__.py`. It currently has `scraped_at`, `generated_at`, `emailed_at`, `deadline_passed_at`, `finalized_at`. The last migration is `0008`. Alembic is run inside the Docker container via `docker compose exec backend alembic upgrade head`.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Create the migration file**
|
||||||
|
|
||||||
|
Create `backend/alembic/versions/0009_phase6_reminded_at.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
"""Phase 6: add reminded_at to weekly_run
|
||||||
|
|
||||||
|
Revision ID: 0009
|
||||||
|
Revises: 0008
|
||||||
|
Create Date: 2026-05-07
|
||||||
|
"""
|
||||||
|
from alembic import op
|
||||||
|
import sqlalchemy as sa
|
||||||
|
|
||||||
|
revision = "0009"
|
||||||
|
down_revision = "0008"
|
||||||
|
branch_labels = None
|
||||||
|
depends_on = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
op.add_column(
|
||||||
|
"weekly_run",
|
||||||
|
sa.Column("reminded_at", sa.DateTime(timezone=True), nullable=True),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
op.drop_column("weekly_run", "reminded_at")
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Add `reminded_at` to the `WeeklyRun` model**
|
||||||
|
|
||||||
|
Open `backend/app/models/__init__.py`. Find the `WeeklyRun` class (near the bottom of the file). After the `emailed_at` column line, add the new column. The existing block looks like:
|
||||||
|
|
||||||
|
```python
|
||||||
|
scraped_at = Column(DateTime(timezone=True))
|
||||||
|
generated_at = Column(DateTime(timezone=True))
|
||||||
|
emailed_at = Column(DateTime(timezone=True))
|
||||||
|
deadline_passed_at = Column(DateTime(timezone=True))
|
||||||
|
finalized_at = Column(DateTime(timezone=True))
|
||||||
|
```
|
||||||
|
|
||||||
|
Change it to:
|
||||||
|
|
||||||
|
```python
|
||||||
|
scraped_at = Column(DateTime(timezone=True))
|
||||||
|
generated_at = Column(DateTime(timezone=True))
|
||||||
|
emailed_at = Column(DateTime(timezone=True))
|
||||||
|
reminded_at = Column(DateTime(timezone=True))
|
||||||
|
deadline_passed_at = Column(DateTime(timezone=True))
|
||||||
|
finalized_at = Column(DateTime(timezone=True))
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 3: Apply migration inside Docker and verify round-trip**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test up -d db backend
|
||||||
|
docker compose --env-file .env.test exec backend alembic upgrade head
|
||||||
|
# Expected: INFO [alembic.runtime.migration] Running upgrade 0008 -> 0009 ...
|
||||||
|
|
||||||
|
docker compose --env-file .env.test exec backend alembic downgrade -1
|
||||||
|
# Expected: INFO [alembic.runtime.migration] Running downgrade 0009 -> 0008 ...
|
||||||
|
|
||||||
|
docker compose --env-file .env.test exec backend alembic upgrade head
|
||||||
|
# Expected: Running upgrade 0008 -> 0009 again, clean
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Verify existing tests still pass**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `115 passed`
|
||||||
|
|
||||||
|
- [ ] **Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/alembic/versions/0009_phase6_reminded_at.py \
|
||||||
|
backend/app/models/__init__.py
|
||||||
|
git commit -m "feat: migration 0009 — add reminded_at to weekly_run"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 2: Wire `SendGridEmailBackend`
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/app/services/email.py`
|
||||||
|
- Modify: `backend/app/config.py`
|
||||||
|
- Create: `backend/tests/test_email_backend.py`
|
||||||
|
|
||||||
|
### Context
|
||||||
|
|
||||||
|
`backend/app/services/email.py` already has an `EmailBackend` Protocol, `ConsoleEmailBackend`, and a `SendGridEmailBackend` stub that raises `NotImplementedError`. The `sendgrid==6.12.0` package is already in `requirements.txt`. The `settings` object in `email.py` comes from `from app.config import settings` at module level — tests monkeypatch it at `app.services.email.SendGridAPIClient`.
|
||||||
|
|
||||||
|
`backend/app/config.py` has `SENDGRID_API_KEY: Optional[str] = None` already. Two new fields are needed.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing tests**
|
||||||
|
|
||||||
|
Create `backend/tests/test_email_backend.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
"""Unit tests for SendGridEmailBackend — no Postgres needed."""
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
|
||||||
|
def test_sendgrid_send_calls_api(monkeypatch):
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
class FakeResponse:
|
||||||
|
status_code = 202
|
||||||
|
body = b""
|
||||||
|
|
||||||
|
class FakeClient:
|
||||||
|
def send(self, msg):
|
||||||
|
calls.append(msg)
|
||||||
|
return FakeResponse()
|
||||||
|
|
||||||
|
monkeypatch.setattr("app.services.email.SendGridAPIClient", lambda _key: FakeClient())
|
||||||
|
|
||||||
|
from app.services.email import SendGridEmailBackend
|
||||||
|
backend = SendGridEmailBackend()
|
||||||
|
backend.send(to="to@example.com", subject="Hello", html="<p>Hi</p>")
|
||||||
|
|
||||||
|
assert len(calls) == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_sendgrid_raises_on_4xx(monkeypatch):
|
||||||
|
class FakeResponse:
|
||||||
|
status_code = 400
|
||||||
|
body = b"bad request"
|
||||||
|
|
||||||
|
class FakeClient:
|
||||||
|
def send(self, msg):
|
||||||
|
return FakeResponse()
|
||||||
|
|
||||||
|
monkeypatch.setattr("app.services.email.SendGridAPIClient", lambda _key: FakeClient())
|
||||||
|
|
||||||
|
from app.services.email import SendGridEmailBackend
|
||||||
|
backend = SendGridEmailBackend()
|
||||||
|
with pytest.raises(RuntimeError, match="400"):
|
||||||
|
backend.send(to="to@example.com", subject="Hello", html="<p>Hi</p>")
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run tests to confirm they fail**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec backend pytest -q tests/test_email_backend.py -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `FAILED` — `NotImplementedError: Wire SendGrid in R3-C`
|
||||||
|
|
||||||
|
- [ ] **Step 3: Add `SENDGRID_FROM_EMAIL` and `SENDGRID_REPLY_TO` to Settings**
|
||||||
|
|
||||||
|
Open `backend/app/config.py`. After `SENDGRID_API_KEY`, add:
|
||||||
|
|
||||||
|
```python
|
||||||
|
SENDGRID_FROM_EMAIL: str = "peter@research.bike"
|
||||||
|
SENDGRID_REPLY_TO: str = "peter@research.bike"
|
||||||
|
```
|
||||||
|
|
||||||
|
The full relevant block becomes:
|
||||||
|
|
||||||
|
```python
|
||||||
|
SENDGRID_API_KEY: Optional[str] = None
|
||||||
|
SENDGRID_FROM_EMAIL: str = "peter@research.bike"
|
||||||
|
SENDGRID_REPLY_TO: str = "peter@research.bike"
|
||||||
|
EMAIL_BACKEND: str = "console"
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Wire `SendGridEmailBackend.send()`**
|
||||||
|
|
||||||
|
Open `backend/app/services/email.py`. Make two changes:
|
||||||
|
|
||||||
|
**4a — Add module-level sendgrid imports** after the existing stdlib imports and before `from app.config import settings`. The full import block at the top of `email.py` should look like:
|
||||||
|
|
||||||
|
```python
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
|
import sys
|
||||||
|
from datetime import datetime, timezone
|
||||||
|
from pathlib import Path
|
||||||
|
from typing import Optional, Protocol
|
||||||
|
|
||||||
|
from sendgrid import SendGridAPIClient
|
||||||
|
from sendgrid.helpers.mail import Mail, ReplyTo
|
||||||
|
|
||||||
|
from sendgrid import SendGridAPIClient
|
||||||
|
from sendgrid.helpers.mail import Mail, ReplyTo
|
||||||
|
|
||||||
|
from app.config import settings
|
||||||
|
```
|
||||||
|
|
||||||
|
Remove the `import os` line if present (unused). Keep everything else the same.
|
||||||
|
|
||||||
|
**4b — Replace the `SendGridEmailBackend` class** (find the stub class that raises `NotImplementedError` and replace the entire class body):
|
||||||
|
|
||||||
|
```python
|
||||||
|
class SendGridEmailBackend:
|
||||||
|
def __init__(self) -> None:
|
||||||
|
self._client = SendGridAPIClient(settings.SENDGRID_API_KEY)
|
||||||
|
self._from_email = settings.SENDGRID_FROM_EMAIL
|
||||||
|
self._reply_to = settings.SENDGRID_REPLY_TO
|
||||||
|
|
||||||
|
def send(
|
||||||
|
self,
|
||||||
|
to: str,
|
||||||
|
subject: str,
|
||||||
|
html: str,
|
||||||
|
text: Optional[str] = None,
|
||||||
|
) -> None:
|
||||||
|
message = Mail(
|
||||||
|
from_email=self._from_email,
|
||||||
|
to_emails=to,
|
||||||
|
subject=subject,
|
||||||
|
html_content=html,
|
||||||
|
plain_text_content=text,
|
||||||
|
)
|
||||||
|
message.reply_to = ReplyTo(self._reply_to)
|
||||||
|
response = self._client.send(message)
|
||||||
|
if response.status_code >= 400:
|
||||||
|
raise RuntimeError(
|
||||||
|
f"SendGrid error {response.status_code}: {response.body}"
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
`SendGridAPIClient` and `Mail`/`ReplyTo` are module-level names, so the test's `monkeypatch.setattr("app.services.email.SendGridAPIClient", ...)` patches them correctly.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Run tests to confirm they pass**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec backend pytest -q tests/test_email_backend.py -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `2 passed`
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run full suite to confirm no regressions**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `117 passed` (115 existing + 2 new)
|
||||||
|
|
||||||
|
- [ ] **Step 7: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/app/services/email.py \
|
||||||
|
backend/app/config.py \
|
||||||
|
backend/tests/test_email_backend.py
|
||||||
|
git commit -m "feat: wire SendGridEmailBackend with from_email/reply_to settings"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 3: `step_reminder` + tests
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/app/services/orchestrator/steps.py`
|
||||||
|
- Modify: `backend/tests/test_orchestrator.py`
|
||||||
|
|
||||||
|
### Context
|
||||||
|
|
||||||
|
`steps.py` has module-level imports from `app.models` and other services. Tests monkeypatch via `app.services.orchestrator.steps.<name>`. The `MealPlanVote` model is in `app.models` and has columns `meal_plan_item_id` (FK), `family_member_id` (FK), `vote` (Boolean). `WeeklyRun` now has `reminded_at` (added in Task 1).
|
||||||
|
|
||||||
|
The `weekly_run_emailed` fixture (in `test_orchestrator.py`) has `scraped_at`, `generated_at`, and `emailed_at` all set — it's the right base for reminder tests.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing tests**
|
||||||
|
|
||||||
|
In `backend/tests/test_orchestrator.py`, add the following. First, add a new fixture after the existing `weekly_run_emailed` fixture:
|
||||||
|
|
||||||
|
```python
|
||||||
|
@pytest.fixture()
|
||||||
|
def weekly_run_reminded(db, family):
|
||||||
|
from app.models import WeeklyRun
|
||||||
|
r = WeeklyRun(
|
||||||
|
family_id=family.id,
|
||||||
|
week_start_date=WEEK,
|
||||||
|
status="running",
|
||||||
|
scraped_at=datetime.now(timezone.utc),
|
||||||
|
generated_at=datetime.now(timezone.utc),
|
||||||
|
emailed_at=datetime.now(timezone.utc),
|
||||||
|
reminded_at=datetime.now(timezone.utc),
|
||||||
|
)
|
||||||
|
db.add(r)
|
||||||
|
db.flush()
|
||||||
|
return r
|
||||||
|
```
|
||||||
|
|
||||||
|
Then add the four tests (these can go after the existing `step_email` tests):
|
||||||
|
|
||||||
|
```python
|
||||||
|
# ── step_reminder ──────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def test_step_reminder_idempotent(db, weekly_run_reminded):
|
||||||
|
original_ts = weekly_run_reminded.reminded_at
|
||||||
|
from app.services.orchestrator.steps import step_reminder
|
||||||
|
step_reminder(weekly_run_reminded, db)
|
||||||
|
assert weekly_run_reminded.reminded_at == original_ts
|
||||||
|
|
||||||
|
|
||||||
|
def test_step_reminder_skips_when_not_emailed(db, weekly_run_generated, monkeypatch):
|
||||||
|
sent = []
|
||||||
|
|
||||||
|
class FakeBackend:
|
||||||
|
def send(self, **kwargs):
|
||||||
|
sent.append(kwargs)
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.services.orchestrator.steps.get_email_backend",
|
||||||
|
lambda: FakeBackend(),
|
||||||
|
)
|
||||||
|
from app.services.orchestrator.steps import step_reminder
|
||||||
|
step_reminder(weekly_run_generated, db)
|
||||||
|
# emailed_at is None on weekly_run_generated — should be a no-op
|
||||||
|
assert len(sent) == 0
|
||||||
|
assert weekly_run_generated.reminded_at is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_step_reminder_sends_to_non_voter(
|
||||||
|
db, weekly_run_emailed, meal_plan, pending_item, member, monkeypatch
|
||||||
|
):
|
||||||
|
sent = []
|
||||||
|
|
||||||
|
class FakeBackend:
|
||||||
|
def send(self, **kwargs):
|
||||||
|
sent.append(kwargs)
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.services.orchestrator.steps.get_email_backend",
|
||||||
|
lambda: FakeBackend(),
|
||||||
|
)
|
||||||
|
from app.services.orchestrator.steps import step_reminder
|
||||||
|
step_reminder(weekly_run_emailed, db)
|
||||||
|
assert weekly_run_emailed.reminded_at is not None
|
||||||
|
assert len(sent) == 1
|
||||||
|
assert sent[0]["to"] == "alice@example.com"
|
||||||
|
assert "1 hour" in sent[0]["subject"].lower() or "closes" in sent[0]["subject"].lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_step_reminder_skips_voter(
|
||||||
|
db, weekly_run_emailed, meal_plan, pending_item, member, monkeypatch
|
||||||
|
):
|
||||||
|
from app.models import MealPlanVote
|
||||||
|
vote = MealPlanVote(
|
||||||
|
meal_plan_item_id=pending_item.id,
|
||||||
|
family_member_id=member.id,
|
||||||
|
vote=True,
|
||||||
|
)
|
||||||
|
db.add(vote)
|
||||||
|
db.flush()
|
||||||
|
|
||||||
|
sent = []
|
||||||
|
|
||||||
|
class FakeBackend:
|
||||||
|
def send(self, **kwargs):
|
||||||
|
sent.append(kwargs)
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.services.orchestrator.steps.get_email_backend",
|
||||||
|
lambda: FakeBackend(),
|
||||||
|
)
|
||||||
|
from app.services.orchestrator.steps import step_reminder
|
||||||
|
step_reminder(weekly_run_emailed, db)
|
||||||
|
# member already voted — no reminder
|
||||||
|
assert len(sent) == 0
|
||||||
|
assert weekly_run_emailed.reminded_at is not None
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run tests to confirm they fail**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/test_orchestrator.py::test_step_reminder_idempotent \
|
||||||
|
tests/test_orchestrator.py::test_step_reminder_skips_when_not_emailed \
|
||||||
|
tests/test_orchestrator.py::test_step_reminder_sends_to_non_voter \
|
||||||
|
tests/test_orchestrator.py::test_step_reminder_skips_voter -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `4 errors` — `ImportError: cannot import name 'step_reminder'`
|
||||||
|
|
||||||
|
- [ ] **Step 3: Add module-level imports to `steps.py`**
|
||||||
|
|
||||||
|
Open `backend/app/services/orchestrator/steps.py`. At the top of the file, after the other standard library imports, add `import html`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import html
|
||||||
|
import logging
|
||||||
|
from datetime import datetime, timezone
|
||||||
|
from typing import TYPE_CHECKING
|
||||||
|
```
|
||||||
|
|
||||||
|
In the `from app.models import ...` line, add `MealPlanVote`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
from app.models import FamilyMember, FamilyProfile, MealPlan, MealPlanItemStatus, MealPlanVote
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Add `step_reminder` to `steps.py`**
|
||||||
|
|
||||||
|
Add the following function at the end of `steps.py`, after `step_finalize`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def step_reminder(run: "WeeklyRun", db: "Session") -> None:
|
||||||
|
if run.reminded_at is not None:
|
||||||
|
logger.info("step_reminder: already done for %s", run.week_start_date)
|
||||||
|
return
|
||||||
|
|
||||||
|
if run.emailed_at is None:
|
||||||
|
logger.info("step_reminder: proposal not sent yet for %s, skipping", run.week_start_date)
|
||||||
|
return
|
||||||
|
|
||||||
|
plan = (
|
||||||
|
db.query(MealPlan)
|
||||||
|
.filter(
|
||||||
|
MealPlan.family_profile_id == run.family_id,
|
||||||
|
MealPlan.week_start_date == run.week_start_date,
|
||||||
|
)
|
||||||
|
.first()
|
||||||
|
)
|
||||||
|
if plan is None:
|
||||||
|
run.reminded_at = datetime.now(timezone.utc)
|
||||||
|
db.commit()
|
||||||
|
return
|
||||||
|
|
||||||
|
pending_item_ids = [
|
||||||
|
item.id
|
||||||
|
for item in plan.items
|
||||||
|
if item.approval_status == MealPlanItemStatus.PENDING
|
||||||
|
]
|
||||||
|
if not pending_item_ids:
|
||||||
|
run.reminded_at = datetime.now(timezone.utc)
|
||||||
|
db.commit()
|
||||||
|
return
|
||||||
|
|
||||||
|
members = (
|
||||||
|
db.query(FamilyMember)
|
||||||
|
.filter(
|
||||||
|
FamilyMember.family_profile_id == run.family_id,
|
||||||
|
FamilyMember.email.isnot(None),
|
||||||
|
)
|
||||||
|
.all()
|
||||||
|
)
|
||||||
|
|
||||||
|
backend = get_email_backend()
|
||||||
|
for member in members:
|
||||||
|
voted_ids = {
|
||||||
|
v.meal_plan_item_id
|
||||||
|
for v in db.query(MealPlanVote)
|
||||||
|
.filter(
|
||||||
|
MealPlanVote.meal_plan_item_id.in_(pending_item_ids),
|
||||||
|
MealPlanVote.family_member_id == member.id,
|
||||||
|
)
|
||||||
|
.all()
|
||||||
|
}
|
||||||
|
unvoted = [
|
||||||
|
item
|
||||||
|
for item in plan.items
|
||||||
|
if item.approval_status == MealPlanItemStatus.PENDING
|
||||||
|
and item.id not in voted_ids
|
||||||
|
]
|
||||||
|
if not unvoted:
|
||||||
|
continue
|
||||||
|
|
||||||
|
item_html_parts = []
|
||||||
|
for item in unvoted:
|
||||||
|
token = issue_token(item.id, member.id)
|
||||||
|
vote_url = (
|
||||||
|
f"{settings.APP_BASE_URL}/api/meals/vote/{item.id}?token={token}"
|
||||||
|
)
|
||||||
|
recipe_name = html.escape(
|
||||||
|
item.recipe.name if item.recipe else str(item.recipe_id)
|
||||||
|
)
|
||||||
|
item_html_parts.append(
|
||||||
|
f'<li>{recipe_name} — <a href="{vote_url}">Vote</a></li>'
|
||||||
|
)
|
||||||
|
|
||||||
|
email_html = (
|
||||||
|
f"<h2>Vote closes in 1 hour!</h2>"
|
||||||
|
f"<p>Hi {html.escape(member.name)}, the meal plan vote closes at Fri 17:00 PT. "
|
||||||
|
f"You haven't voted on:</p>"
|
||||||
|
f"<ul>{''.join(item_html_parts)}</ul>"
|
||||||
|
)
|
||||||
|
backend.send(
|
||||||
|
to=member.email,
|
||||||
|
subject=f"Meal plan vote closes in 1 hour — week of {run.week_start_date}",
|
||||||
|
html=email_html,
|
||||||
|
)
|
||||||
|
logger.info("step_reminder: sent to %s", member.email)
|
||||||
|
|
||||||
|
run.reminded_at = datetime.now(timezone.utc)
|
||||||
|
db.commit()
|
||||||
|
logger.info("step_reminder: done for %s", run.week_start_date)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Run tests to confirm they pass**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/test_orchestrator.py::test_step_reminder_idempotent \
|
||||||
|
tests/test_orchestrator.py::test_step_reminder_skips_when_not_emailed \
|
||||||
|
tests/test_orchestrator.py::test_step_reminder_sends_to_non_voter \
|
||||||
|
tests/test_orchestrator.py::test_step_reminder_skips_voter -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `4 passed`
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run full suite**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `121 passed` (117 from Task 2 + 4 new)
|
||||||
|
|
||||||
|
- [ ] **Step 7: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/app/services/orchestrator/steps.py \
|
||||||
|
backend/tests/test_orchestrator.py
|
||||||
|
git commit -m "feat: step_reminder — 1-hour pre-deadline nudge for non-voters"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 4: Wire reminder into runner, admin, and scheduler
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/app/services/orchestrator/runner.py`
|
||||||
|
- Modify: `backend/app/api/admin.py`
|
||||||
|
- Modify: `backend/app/scheduler/__main__.py`
|
||||||
|
|
||||||
|
### Context
|
||||||
|
|
||||||
|
`runner.py` has `STEPS = ("scrape", "generate", "email", "deadline", "finalize")` and a `step_fns` dict inside `run_step`. `admin.py` has `_VALID_STEPS = {"scrape", "generate", "email", "deadline", "finalize"}`. The scheduler has 5 jobs; the docstring lists 5 lines. All three files need `"reminder"` added.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Update `runner.py`**
|
||||||
|
|
||||||
|
Open `backend/app/services/orchestrator/runner.py`. Change the `STEPS` tuple:
|
||||||
|
|
||||||
|
```python
|
||||||
|
STEPS = ("scrape", "generate", "email", "reminder", "deadline", "finalize")
|
||||||
|
```
|
||||||
|
|
||||||
|
Inside `run_step`, find the `step_fns` dict and add the `"reminder"` entry:
|
||||||
|
|
||||||
|
```python
|
||||||
|
step_fns = {
|
||||||
|
"scrape": s.step_scrape,
|
||||||
|
"generate": s.step_generate,
|
||||||
|
"email": s.step_email,
|
||||||
|
"reminder": s.step_reminder,
|
||||||
|
"deadline": s.step_deadline,
|
||||||
|
"finalize": s.step_finalize,
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Update `admin.py`**
|
||||||
|
|
||||||
|
Open `backend/app/api/admin.py`. Change `_VALID_STEPS`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
_VALID_STEPS = {"scrape", "generate", "email", "reminder", "deadline", "finalize"}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 3: Update scheduler `__main__.py`**
|
||||||
|
|
||||||
|
Open `backend/app/scheduler/__main__.py`. After the `weekly_email` job block and before the `weekly_deadline` job block, add:
|
||||||
|
|
||||||
|
```python
|
||||||
|
scheduler.add_job(
|
||||||
|
lambda: run_step("reminder"),
|
||||||
|
CronTrigger(day_of_week="fri", hour=16, minute=0, timezone=TZ),
|
||||||
|
id="weekly_reminder",
|
||||||
|
name="Weekly reminder (Fri 16:00 PT)",
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
Also update the module docstring to include the new step:
|
||||||
|
|
||||||
|
```
|
||||||
|
Weekly cadence (America/Los_Angeles):
|
||||||
|
Fri 02:00 — scrape (Lucky weekend prices live by ~midnight)
|
||||||
|
Fri 05:00 — generate (fresh grocery data)
|
||||||
|
Fri 06:00 — email (per-voter approval links)
|
||||||
|
Fri 16:00 — reminder (nudge non-voters, 1h before deadline)
|
||||||
|
Fri 17:00 — deadline (pending items resolved per family policy)
|
||||||
|
Fri 18:00 — finalize (shopping-list email)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Run full test suite**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `121 passed` (unchanged count — no new tests in this task)
|
||||||
|
|
||||||
|
- [ ] **Step 5: Verify scheduler registers 6 jobs**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test up -d scheduler
|
||||||
|
docker compose --env-file .env.test logs scheduler | grep "Registered"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: 6 lines — scrape / generate / email / reminder / deadline / finalize
|
||||||
|
|
||||||
|
- [ ] **Step 6: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/app/services/orchestrator/runner.py \
|
||||||
|
backend/app/api/admin.py \
|
||||||
|
backend/app/scheduler/__main__.py
|
||||||
|
git commit -m "feat: wire step_reminder into runner, admin, and scheduler (Fri 16:00 PT)"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 5: HTML-escape (#P5-a) and `getattr` cleanup (#P5-b)
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/app/services/orchestrator/steps.py`
|
||||||
|
|
||||||
|
### Context
|
||||||
|
|
||||||
|
`import html` was already added in Task 3. This task applies `html.escape()` to the two existing functions that inline user-derived strings into HTML, and removes a dead `getattr` default in `step_deadline`.
|
||||||
|
|
||||||
|
**`step_email`** currently builds recipe name like:
|
||||||
|
```python
|
||||||
|
recipe_name = item.recipe.name if item.recipe else str(item.recipe_id)
|
||||||
|
item_html_parts.append(f'<li>{recipe_name} — <a href="{vote_url}">Vote</a></li>')
|
||||||
|
```
|
||||||
|
|
||||||
|
**`step_finalize`** currently builds rows like:
|
||||||
|
```python
|
||||||
|
f"<tr><td>{item.recipe.name}</td>"
|
||||||
|
f"<td>{ing.get('name', '')}</td>"
|
||||||
|
f"<td>{ing.get('qty', '')} {ing.get('unit', '')}</td></tr>"
|
||||||
|
```
|
||||||
|
|
||||||
|
**`step_deadline`** currently has:
|
||||||
|
```python
|
||||||
|
policy = getattr(family, "pending_approval_policy", "approve") if family else "approve"
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write a test to verify HTML-escaping in `step_email`**
|
||||||
|
|
||||||
|
Add to `backend/tests/test_orchestrator.py` (after the existing `step_email` tests):
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_step_email_escapes_recipe_name(
|
||||||
|
db, weekly_run_generated, meal_plan, pending_item, member, monkeypatch
|
||||||
|
):
|
||||||
|
# Inject a recipe name with HTML-special chars
|
||||||
|
pending_item.recipe.name = "<script>alert('xss')</script>"
|
||||||
|
db.flush()
|
||||||
|
|
||||||
|
sent = []
|
||||||
|
|
||||||
|
class FakeBackend:
|
||||||
|
def send(self, **kwargs):
|
||||||
|
sent.append(kwargs)
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.services.orchestrator.steps.get_email_backend",
|
||||||
|
lambda: FakeBackend(),
|
||||||
|
)
|
||||||
|
from app.services.orchestrator.steps import step_email
|
||||||
|
step_email(weekly_run_generated, db)
|
||||||
|
assert len(sent) == 1
|
||||||
|
assert "<script>" not in sent[0]["html"]
|
||||||
|
assert "<script>" in sent[0]["html"]
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run the test to confirm it fails**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/test_orchestrator.py::test_step_email_escapes_recipe_name -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `FAILED` — `<script>` is present unescaped
|
||||||
|
|
||||||
|
- [ ] **Step 3: Apply `html.escape()` in `step_email`**
|
||||||
|
|
||||||
|
In `step_email`, find the recipe name line:
|
||||||
|
|
||||||
|
```python
|
||||||
|
recipe_name = item.recipe.name if item.recipe else str(item.recipe_id)
|
||||||
|
```
|
||||||
|
|
||||||
|
Replace with:
|
||||||
|
|
||||||
|
```python
|
||||||
|
recipe_name = html.escape(
|
||||||
|
item.recipe.name if item.recipe else str(item.recipe_id)
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Apply `html.escape()` in `step_finalize`**
|
||||||
|
|
||||||
|
In `step_finalize`, find the rows_html generation:
|
||||||
|
|
||||||
|
```python
|
||||||
|
rows_html = "".join(
|
||||||
|
f"<tr><td>{item.recipe.name}</td>"
|
||||||
|
f"<td>{ing.get('name', '')}</td>"
|
||||||
|
f"<td>{ing.get('qty', '')} {ing.get('unit', '')}</td></tr>"
|
||||||
|
for item in approved_items
|
||||||
|
if item.recipe
|
||||||
|
for ing in (item.recipe.ingredients or [])
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
Replace with:
|
||||||
|
|
||||||
|
```python
|
||||||
|
rows_html = "".join(
|
||||||
|
f"<tr><td>{html.escape(item.recipe.name)}</td>"
|
||||||
|
f"<td>{html.escape(str(ing.get('name', '')))}</td>"
|
||||||
|
f"<td>{html.escape(str(ing.get('qty', '')))} {html.escape(str(ing.get('unit', '')))}</td></tr>"
|
||||||
|
for item in approved_items
|
||||||
|
if item.recipe
|
||||||
|
for ing in (item.recipe.ingredients or [])
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Remove dead `getattr` in `step_deadline` (#P5-b)**
|
||||||
|
|
||||||
|
In `step_deadline`, find:
|
||||||
|
|
||||||
|
```python
|
||||||
|
policy = getattr(family, "pending_approval_policy", "approve") if family else "approve"
|
||||||
|
```
|
||||||
|
|
||||||
|
Replace with:
|
||||||
|
|
||||||
|
```python
|
||||||
|
policy = family.pending_approval_policy if family else "approve"
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run the escape test to confirm it passes**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/test_orchestrator.py::test_step_email_escapes_recipe_name -v
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `1 passed`
|
||||||
|
|
||||||
|
- [ ] **Step 7: Run full suite**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `122 passed` (121 + 1 new)
|
||||||
|
|
||||||
|
- [ ] **Step 8: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/app/services/orchestrator/steps.py \
|
||||||
|
backend/tests/test_orchestrator.py
|
||||||
|
git commit -m "fix: html-escape recipe/ingredient names in email templates (#P5-a, #P5-b)"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Final verification
|
||||||
|
|
||||||
|
After all 5 tasks are complete:
|
||||||
|
|
||||||
|
- [ ] **Full test suite green**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec \
|
||||||
|
-e TEST_DATABASE_URL=postgresql://mealplanner:${POSTGRES_PASSWORD}@db:5432/mealplanner \
|
||||||
|
backend pytest -q tests/
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `122 passed`
|
||||||
|
|
||||||
|
- [ ] **Alembic round-trip clean**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test exec backend alembic downgrade base
|
||||||
|
docker compose --env-file .env.test exec backend alembic upgrade head
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: no errors
|
||||||
|
|
||||||
|
- [ ] **Scheduler shows 6 jobs**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker compose --env-file .env.test logs scheduler | grep "Registered"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: 6 lines
|
||||||
|
|
||||||
|
- [ ] **Update HANDOFF.md and ORIENTATION.md**
|
||||||
|
|
||||||
|
In `docs/HANDOFF.md`:
|
||||||
|
- Phase 6 status: "Stub only" → "**Complete** — SendGrid wired; `step_reminder` (Fri 16:00 PT) nudges non-voters; html-escape applied to all email templates."
|
||||||
|
- Test count: 115 → 122
|
||||||
|
- Remove #P5-a and #P5-b from open tasks
|
||||||
|
- Update verification gate
|
||||||
|
|
||||||
|
In `docs/ORIENTATION.md`:
|
||||||
|
- Phase 6 row → **Complete**
|
||||||
|
- Verification gate: 115/115 → 122/122
|
||||||
|
- Scheduler block: add `Fri 16:00 reminder` line
|
||||||
|
- Env vars: add `SENDGRID_FROM_EMAIL`, `SENDGRID_REPLY_TO`
|
||||||
|
|
||||||
|
Commit:
|
||||||
|
```bash
|
||||||
|
git add docs/HANDOFF.md docs/ORIENTATION.md
|
||||||
|
git commit -m "docs: refresh HANDOFF + ORIENTATION for Phase 6 completion"
|
||||||
|
```
|
||||||
Reference in New Issue
Block a user