From 9586d21ca21de24c4416d6f8b4f3ac26e4028686 Mon Sep 17 00:00:00 2001 From: NguyenND Date: Thu, 6 Aug 2026 12:35:09 -0400 Subject: [PATCH] Aug 6 - Update enrollment page, notify admin user --- .claude/settings.json | 4 +- CLAUDE.md | 16 ++- app/enrollment/mailer.py | 128 ++++++++++++++++-- app/enrollment/routes.py | 7 +- .../enrollment/email_admin_notice.html | 89 ++++++++++++ 5 files changed, 221 insertions(+), 23 deletions(-) create mode 100644 app/enrollment/templates/enrollment/email_admin_notice.html diff --git a/.claude/settings.json b/.claude/settings.json index ffad427..d85f32e 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -18,7 +18,9 @@ "Bash(python -c ' *)", "Bash(git diff *)", "Bash(python -c \"import ast;ast.parse\\(open\\('app/enrollment/routes.py',encoding='utf-8'\\).read\\(\\)\\);print\\('parses OK'\\)\")", - "Bash(python -c \"import ast;[ast.parse\\(open\\(f,encoding='utf-8'\\).read\\(\\)\\) for f in ['app/enrollment/mailer.py','app/enrollment/schema.py']];print\\('parses OK'\\)\")" + "Bash(python -c \"import ast;[ast.parse\\(open\\(f,encoding='utf-8'\\).read\\(\\)\\) for f in ['app/enrollment/mailer.py','app/enrollment/schema.py']];print\\('parses OK'\\)\")", + "Bash(python -c \"import ast;ast.parse\\(open\\('app/enrollment/mailer.py',encoding='utf-8'\\).read\\(\\)\\);print\\('parses OK'\\)\")", + "Bash(grep -n \"\\\\\\\\\\\\\\\\n\" app/enrollment/mailer.py)" ] } } diff --git a/CLAUDE.md b/CLAUDE.md index 46b3fe1..bc2a10a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -164,6 +164,7 @@ part of the tree — see §7. Device registration on the API side lives in | `REDIS_URL` | Optional. When set, Flask-Limiter uses Redis for shared rate-limit counters across Gunicorn workers. | | `GROQ_API_KEY` | Optional. When set, enables the AI chatbot at `/support/chat`. Absent → chat input disabled; customers see a "Submit to Support" fallback only. | | `GROQ_MODEL` | Optional. Groq model ID. Defaults to `llama-3.3-70b-versatile`. | +| `ENROLLMENT_NOTIFY_EMAILS` | Optional. Comma-separated extra addresses alerted on a new enrollment, **in addition to** every active `admin` account. For people who should be told but hold no JQC login. | | `ENROLLMENT_DIR` | Optional. Directory for enrollment-form JSON submissions. Defaults to `/enrollments` (git-ignored). Created at boot. | | `PHOTO_STAMP_ENABLED` | Optional, default `true`. Burns a capture-time + geo overlay into photos uploaded via `POST /api/v1/photos/upload`. Set `false` to store raw uploads. | @@ -1488,7 +1489,7 @@ timeout = 30 | 84 | **"Instructions" is a LABEL over `notes` — never rename the field, attribute, column or API key** | `ScheduledInspectionForm.notes` renders as "Instructions" and both the web execute page and the iPad say "Instructions". The wire key stays `notes` (`api/scheduled.py::_scheduled_payload`), which is what `APIScheduledInspection.notes` decodes into `LocalScheduledInspection.notes`; the iPad exposes it through a computed `instructions` accessor that also trims blank text. Renaming any of the storage identifiers would silently break the iPad decode — the field is `try?`-decoded, so it would fail to nil rather than throwing. | | 85 | **`next_due_date` is mutable state, `end_date` is a fixed boundary — never conflate them** | `fulfill()` rewrites `next_due_date` after every completed inspection; `end_date` is set by the manager and never touched by the app. The old single label "Start / Due Date" said both at once, which is what users reported as confusing. The label now follows context — `form.next_due_date.label.text` is set to "Start Date" in `create()` and "Next Due Date" in `edit()`. Do not rename the `next_due_date` column to match a label: it is indexed, it is the API payload key the iPad decodes, and the reminder cron filters on it. | | 87 | **Never write `role == 'inspector'` — use `user.is_inspector` (`User.INSPECTOR_ROLES`)** | phase49 added `external_inspector`, which must behave as an inspector everywhere. An equality check silently drops it into the *privileged* branch of every `if inspector: scope … else: org-wide` block — i.e. a third-party inspector would see **every contract in the system**. This is a fail-OPEN mistake: nothing errors, the data just leaks. The sweep converted ~44 Python sites and 7 template sites; the only surviving `== 'inspector'` literals are the matrix docstring, the `MATRIX_DEFAULTS` mirror comprehension, and the default-checked box in `admin/broadcast.html`. Query-level checks use `User.role.in_(User.INSPECTOR_ROLES)` (never `filter_by(role='inspector')`). A **new** `app/api/*` blueprint's `_ALLOWED_ROLES` must include `external_inspector`, same as rule 79 requires for `auditor`. | -| 88 | **`app/enrollment/` imports no model and writes no DB row — keep the vertical slice sealed** | The enrollment form describes accounts that do NOT exist yet (no contract, facility or user to key a row against), so it stores flat JSON in `ENROLLMENT_DIR` and owns its own templates. Adding a model/migration for it, or letting the public POST create Users, would couple an unauthenticated endpoint to the account system — the exact thing the separation buys. If enrollment must ever provision accounts, do it as a separate admin-triggered action that reads a stored submission. Submission ids are filesystem paths: validate against `_ID_RE` before every open (path traversal). See §24. | +| 88 | **`app/enrollment/` writes no DB row and has exactly ONE read — keep the vertical slice sealed** | The enrollment form describes accounts that do NOT exist yet (no contract, facility or user to key a row against), so it stores flat JSON in `ENROLLMENT_DIR` and owns its own templates. The single permitted model access is `mailer._admin_recipients()` reading active `admin` users to address the new-enrollment alert — function-local, read-only, and guarded so a DB failure cannot break a submission. Adding a model/migration for enrollment, or letting the public POST **create** Users, would couple an unauthenticated endpoint to the account system — the exact thing the separation buys. If enrollment must ever provision accounts, do it as a separate admin-triggered action that reads a stored submission. Submission ids are filesystem paths: validate against `_ID_RE` before every open (path traversal). See §24. | | 81 | **Photo timestamp/geo overlay is burned at UPLOAD, never on `PATCH /issues//photos`** | That PATCH receives only path strings — the bytes are already in storage and the payload carries no capture metadata. Burning there would need a read-modify-write per key plus an overwrite-in-place primitive (`storage.save()` mints a NEW uuid key, and §22 requires key == DB path), and would risk a **double burn** since the endpoint is deliberately idempotent/retry-safe (rule 45). Stamp in `POST /photos/upload`, where the raw bytes + EXIF are in hand and each call writes exactly one already-stamped object. Stamping failures must always fall back to storing the ORIGINAL bytes — never lose a photo to a stamping bug. See §23. | --- @@ -1677,11 +1678,18 @@ A **"Recommendation selection"** button applies `schema.recommendation_map()` pe **Legacy submissions.** Files stored in the original fixed-seat format are never rewritten; `schema.people_of()` / `cell()` / `wants_mobile()` normalise on read, so the admin list, detail view and CSV render both shapes identically — verified against a hand-written legacy file. -### Confirmation email +### Emails on submission -`mailer.send_confirmation(record)` emails the requester a copy of what they submitted (reference number, project, and the full people table with roles and mobile-app flags), HTML + plain-text, rendered from `templates/enrollment/email_confirmation.html`. Sent on a background thread (rule 14) with `branded_sender()` as the From (rules 64/76). +Two messages, both fired *after* `storage.save()`, both on a background thread via `_dispatch()` (rule 14), both From `branded_sender()` (rules 64/76): -**It cannot cost a customer their enrollment.** It fires *after* `storage.save()` and every failure path is caught and logged: no `MAIL_SERVER`, `mail.send` raising, or the template itself blowing up all still return the normal thank-you page with the record safely on disk — verified for all three. `mailer.py` is the only part of the package touching shared mail infrastructure; it still imports no models and writes no DB row. +| To | Function | Template | Contents | +|---|---|---|---| +| The **requester** | `send_confirmation()` | `email_confirmation.html` | Reference, project, the people table. Corrections are directed to `schema.CORRECTIONS_EMAIL`, **not** a reply — the From is an unmonitored no-reply. | +| **JQC admins** | `send_admin_notification()` | `email_admin_notice.html` | Project, requester, reference, people table, customer notes, and an **"Open in JQC"** deep link to `/enrollment/admin/` built from the submitting host (so a multi-domain deployment links to the host actually in use). | + +**Recipients** come from `_admin_recipients()`: active `admin` accounts, plus any addresses in the optional **`ENROLLMENT_NOTIFY_EMAILS`** config (comma-separated) for people who should be told but hold no JQC login. Deduplicated case-insensitively. Directors, inspectors and *inactive* admins are excluded — verified. + +**Neither email can cost a customer their enrollment.** Every failure path is caught and logged: no `MAIL_SERVER`, `mail.send` raising, the template blowing up, or the admin lookup failing because the DB is unreachable — all still return the normal thank-you page with the record safely on disk. Verified for all four. A missing admin list does not suppress the requester's confirmation. ### Storage diff --git a/app/enrollment/mailer.py b/app/enrollment/mailer.py index d4a3073..39bda17 100644 --- a/app/enrollment/mailer.py +++ b/app/enrollment/mailer.py @@ -13,7 +13,11 @@ response), and the From identity comes from branded_sender() so it stays an SMTP-authorized address that actually delivers (rules 64 / 76). This is the only part of app/enrollment that touches shared mail -infrastructure; it still imports no models and writes no DB row. +infrastructure. It performs exactly ONE database read — resolving the active +admin accounts to notify — and no write. That read is a deliberate, narrowed +exception to the package's no-models rule (rule 88): the alternative, a +hand-maintained recipient list in config, drifts out of step with reality the +first time someone joins or leaves. Everything else here stays model-free. """ import logging @@ -53,6 +57,113 @@ def _text_body(record, people, corrections_email): return '\n'.join(lines) +def _dispatch(msg, label, record): + """Send one message on a background thread. Never raises. + + Rule 14 — the HTTP response must not wait on SMTP. The submission is + already on disk by the time anything here runs, so a mail failure is + logged and dropped rather than surfaced to the customer. + """ + app = current_app._get_current_object() + + def _send(): + with app.app_context(): + try: + from app import mail + mail.send(msg) + logger.info('ENROLLMENT %s SENT | to=%s | id=%s', + label, msg.recipients, record.get('id')) + except Exception as exc: + logger.error('ENROLLMENT %s FAILED | to=%s | id=%s | error=%s', + label, msg.recipients, record.get('id'), exc) + + threading.Thread(target=_send, daemon=True).start() + + +def _admin_recipients(): + """Addresses to alert when a new enrollment arrives. + + Active `admin` accounts, plus any extra addresses in the optional + ENROLLMENT_NOTIFY_EMAILS config (comma-separated) for people who should be + told but do not hold a JQC login. Deduplicated case-insensitively. + + The User import is function-local and read-only — see the module docstring. + """ + emails = [] + try: + from app.models.user import User + rows = User.query.filter(User.role == 'admin', + User.active == True).all() # noqa: E712 + emails += [u.email for u in rows if u.email] + except Exception: + # A DB problem must not stop the confirmation going out, nor the + # submission from succeeding. + logger.exception('ENROLLMENT | could not resolve admin recipients') + + extra = current_app.config.get('ENROLLMENT_NOTIFY_EMAILS') or '' + emails += [e.strip() for e in extra.split(',') if e.strip()] + + seen, out = set(), [] + for e in emails: + low = e.lower() + if low not in seen: + seen.add(low) + out.append(e) + return out + + +def send_admin_notification(record, base_url=None): + """Alert JQC admins that a new enrollment form has arrived. Never raises.""" + if not current_app.config.get('MAIL_SERVER'): + logger.warning('ENROLLMENT ADMIN EMAIL SKIPPED | no MAIL_SERVER | id=%s', + record.get('id')) + return + + try: + from flask_mail import Message + from app.utils.mail_utils import branded_sender + from . import schema + + recipients = _admin_recipients() + if not recipients: + logger.warning('ENROLLMENT | no admin recipients for id=%s', + record.get('id')) + return + + effective_base = (base_url + or current_app.config.get('APP_BASE_URL', '')).rstrip('/') + people = schema.people_of(record) + link = f'{effective_base}/enrollment/admin/{record.get("id")}' + + lines = [ + 'A new JQC enrollment form has been submitted.', + '', + f'Project: {record.get("project_name")}', + f'Requester: {record.get("request_by")} <{record.get("requester_email")}>', + f'Reference: {record.get("id")}', + f'People: {len(people)}', + '', + f'Open it here: {link}', + ] + if record.get('notes'): + lines += ['', f'Customer notes: {record["notes"]}'] + + msg = Message( + subject = f'[JQC] New enrollment — {record.get("project_name")}', + sender = branded_sender(effective_base), + recipients = recipients, + body = '\n'.join(lines), + html = render_template('enrollment/email_admin_notice.html', + record=record, people=people, + schema=schema, link=link), + ) + _dispatch(msg, 'ADMIN EMAIL', record) + + except Exception: + logger.exception('ENROLLMENT ADMIN EMAIL BUILD FAILED | id=%s', + record.get('id')) + + def send_confirmation(record, base_url=None): """Email the requester a copy of what they submitted. Never raises.""" email = (record.get('requester_email') or '').strip() @@ -66,7 +177,6 @@ def send_confirmation(record, base_url=None): try: from flask_mail import Message - from app import mail from app.utils.mail_utils import branded_sender from . import schema @@ -84,19 +194,7 @@ def send_confirmation(record, base_url=None): schema=schema), ) - app = current_app._get_current_object() - - def _send(): - with app.app_context(): - try: - mail.send(msg) - logger.info('ENROLLMENT EMAIL SENT | to=%s | id=%s', - email, record.get('id')) - except Exception as exc: - logger.error('ENROLLMENT EMAIL FAILED | to=%s | id=%s | error=%s', - email, record.get('id'), exc) - - threading.Thread(target=_send, daemon=True).start() + _dispatch(msg, 'EMAIL', record) except Exception: # Building the message failed (bad template, mail misconfigured, …). diff --git a/app/enrollment/routes.py b/app/enrollment/routes.py index 9bd2cf3..0f478dc 100644 --- a/app/enrollment/routes.py +++ b/app/enrollment/routes.py @@ -243,9 +243,10 @@ def submit(): logger.info('ENROLLMENT | submitted | id=%s project=%r people=%d ip=%s', record['id'], project_name, len(named), request.remote_addr) - # Confirmation to the requester. Fired AFTER the save and fully guarded — - # a mail problem must never cost the customer their submission. - mailer.send_confirmation(record, base_url=request.host_url) + # Both emails fire AFTER the save and are fully guarded — a mail problem + # must never cost the customer their submission. + mailer.send_confirmation(record, base_url=request.host_url) # requester + mailer.send_admin_notification(record, base_url=request.host_url) # JQC admins return render_template('enrollment/submitted.html', reference=record['id'], email=requester_email) diff --git a/app/enrollment/templates/enrollment/email_admin_notice.html b/app/enrollment/templates/enrollment/email_admin_notice.html new file mode 100644 index 0000000..8d91da8 --- /dev/null +++ b/app/enrollment/templates/enrollment/email_admin_notice.html @@ -0,0 +1,89 @@ +{# Internal alert to JQC admins when a new enrollment form arrives. Inline + styles only and no external assets — mail clients strip