From 128ae0ea85e6a210acfabde83eb54b466483d4a4 Mon Sep 17 00:00:00 2001 From: SimpleTest Date: Wed, 12 Aug 2026 16:04:11 +0300 Subject: [PATCH] Reduce transactional email noise --- .env.e2e.example | 1 + .env.example | 5 +- .env.load.example | 1 + README.md | 8 +- compose.yaml | 1 + config/config.exs | 2 +- config/runtime.exs | 3 +- .../who-need-help/templates/deployments.yaml | 2 + .../who-need-help/templates/migrate-job.yaml | 2 + deploy/helm/who-need-help/values.yaml | 1 + docs/architecture.md | 4 +- docs/operations.md | 18 ++- docs/support-and-content-removal.md | 8 +- docs/verification.md | 2 +- lib/who_need_help/accounts/auth_identity.ex | 6 +- lib/who_need_help/accounts/user.ex | 5 +- lib/who_need_help/accounts/user_notifier.ex | 13 +- lib/who_need_help/content_removal/notice.ex | 6 +- lib/who_need_help/content_removal/notifier.ex | 18 +-- lib/who_need_help/email_address.ex | 43 +++++++ lib/who_need_help/email_delivery.ex | 57 +++++++++ .../mail/support_operator_alert_worker.ex | 30 +++++ .../mail/support_update_worker.ex | 44 +++++++ lib/who_need_help/notifications.ex | 13 +- .../notifications/email_notifier.ex | 36 ------ .../notifications/nearby_subscription.ex | 13 -- .../push/notification_dispatch_worker.ex | 26 +--- .../push/notification_email_worker.ex | 44 ++----- lib/who_need_help/support.ex | 96 ++++++++------ lib/who_need_help/support/notifier.ex | 59 ++------- lib/who_need_help/support/support_request.ex | 6 +- .../controllers/support_html/show.html.heex | 2 +- .../live/notification_live.ex | 29 ++--- lib/who_need_help_web/telemetry.ex | 7 ++ ..._allow_inbox_only_nearby_subscriptions.exs | 18 +++ scripts/clean-deploy-verify.sh | 1 + scripts/ensure-local-e2e-env.sh | 1 + scripts/ensure-local-load-env.sh | 14 ++- scripts/oban-burst-run.sh | 1 + scripts/quality.sh | 1 + test/who_need_help/email_address_test.exs | 19 +++ test/who_need_help/notifications_test.exs | 23 +--- .../support_and_content_removal_test.exs | 118 ++++++++++++++---- .../controllers/metrics_controller_test.exs | 15 ++- .../live/mutual_aid_live_test.exs | 6 +- 45 files changed, 520 insertions(+), 308 deletions(-) create mode 100644 lib/who_need_help/email_address.ex create mode 100644 lib/who_need_help/email_delivery.ex create mode 100644 lib/who_need_help/mail/support_operator_alert_worker.ex create mode 100644 lib/who_need_help/mail/support_update_worker.ex delete mode 100644 lib/who_need_help/notifications/email_notifier.ex create mode 100644 priv/repo/migrations/20260812120611_allow_inbox_only_nearby_subscriptions.exs create mode 100644 test/who_need_help/email_address_test.exs diff --git a/.env.e2e.example b/.env.e2e.example index fcb3559..b010855 100644 --- a/.env.e2e.example +++ b/.env.e2e.example @@ -36,6 +36,7 @@ MIGRATE_POOL_SIZE=2 COMBINED_POOL_SIZE=4 OBAN_MAINTENANCE_CONCURRENCY=2 OBAN_PUSH_CONCURRENCY=1 +OBAN_MAIL_CONCURRENCY=1 WEB_REPLICAS=2 WORKER_REPLICAS=2 ERLANG_PORT_LIMIT=65536 diff --git a/.env.example b/.env.example index c34fef5..059dd23 100644 --- a/.env.example +++ b/.env.example @@ -199,6 +199,7 @@ MIGRATE_POOL_SIZE=2 COMBINED_POOL_SIZE=4 OBAN_MAINTENANCE_CONCURRENCY=2 OBAN_PUSH_CONCURRENCY=1 +OBAN_MAIL_CONCURRENCY=1 WEB_REPLICAS=2 WORKER_REPLICAS=2 # Maximum simultaneously existing Erlang ports (files, sockets and drivers). @@ -225,7 +226,9 @@ EMAIL_FROM_ADDRESS=contact@example.com SUPPORT_INBOX_ADDRESS= # Operator email alerts are disabled by default because the permission-scoped # staff workspace is the canonical queue. Set immediate only when a monitored -# mailbox should receive one metadata-only alert for each verified case/update. +# mailbox should receive one metadata-only alert for each newly verified case. +# Ongoing conversation stays in the staff workspace and in-app inbox. Both +# operator alerts and user-facing support updates use the separate Oban mail queue. SUPPORT_OPERATOR_EMAIL_MODE=disabled # Pilot policy: an anonymous support/removal email must be confirmed within one # day. Confirmed case links remain usable for one year. Both values are seconds diff --git a/.env.load.example b/.env.load.example index a5d03c0..cca5744 100644 --- a/.env.load.example +++ b/.env.load.example @@ -37,6 +37,7 @@ MIGRATE_POOL_SIZE=2 COMBINED_POOL_SIZE=4 OBAN_MAINTENANCE_CONCURRENCY=2 OBAN_PUSH_CONCURRENCY=1 +OBAN_MAIL_CONCURRENCY=1 WEB_REPLICAS=2 WORKER_REPLICAS=2 ERLANG_PORT_LIMIT=65536 diff --git a/README.md b/README.md index 7365c98..a2b798b 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,9 @@ local Codex CLI authenticated with their ChatGPT subscription. before raw totals. - A private notification inbox, category/radius/urgency/availability-based nearby-help subscriptions, quiet hours, per-channel preferences, browser Web - Push registrations, email alerts, and Android FCM device registrations. + Push registrations, and Android FCM device registrations. Nearby matches use + the inbox and optional real-time push; immediate nearby email is disabled and + a batched email digest is not implemented yet. Remote payloads contain navigation metadata and generic text, never chat bodies or exact coordinates; Oban retries transient delivery failures and disables rejected device registrations. @@ -73,7 +75,9 @@ local Codex CLI authenticated with their ChatGPT subscription. - Separate public support and content-removal intake, including moderation appeals, account deletion/data requests, a URL-only TAKE IT DOWN form, verified-contact status links, verification-gated support alerts, and audited - staff queues. + staff queues. Support sends one initial response email and later public-status + changes through the dedicated Oban `mail` queue; ordinary conversation + messages stay in the private inbox and optional push. Authenticated users can download an allow-listed JSON data export that omits password/session/push credentials and counterpart message bodies. A moderator-only deletion preflight reports active workflows without performing diff --git a/compose.yaml b/compose.yaml index 6bf9ace..08fee6f 100644 --- a/compose.yaml +++ b/compose.yaml @@ -66,6 +66,7 @@ x-app-environment: &app-environment ANDROID_APP_LINKS_SHA256_CERT_FINGERPRINTS: ${ANDROID_APP_LINKS_SHA256_CERT_FINGERPRINTS:-} OBAN_MAINTENANCE_CONCURRENCY: ${OBAN_MAINTENANCE_CONCURRENCY:-2} OBAN_PUSH_CONCURRENCY: ${OBAN_PUSH_CONCURRENCY:-1} + OBAN_MAIL_CONCURRENCY: ${OBAN_MAIL_CONCURRENCY:-1} services: docker-api-proxy: diff --git a/config/config.exs b/config/config.exs index 4c3c009..a5cbe75 100644 --- a/config/config.exs +++ b/config/config.exs @@ -73,7 +73,7 @@ config :phoenix, config :who_need_help, Oban, repo: WhoNeedHelp.Repo, - queues: [maintenance: 2, push: 1], + queues: [maintenance: 2, push: 1, mail: 1], plugins: [ {Oban.Plugins.Pruner, max_age: 86_400}, {Oban.Plugins.Cron, crontab: [{"* * * * *", WhoNeedHelp.Workers.ExpireRequests}]} diff --git a/config/runtime.exs b/config/runtime.exs index 239a4e0..8126ef1 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -108,7 +108,8 @@ if config_env() == :prod do config :who_need_help, Oban, queues: [ maintenance: positive_integer_with_default.("OBAN_MAINTENANCE_CONCURRENCY", 2), - push: positive_integer_with_default.("OBAN_PUSH_CONCURRENCY", 1) + push: positive_integer_with_default.("OBAN_PUSH_CONCURRENCY", 1), + mail: positive_integer_with_default.("OBAN_MAIL_CONCURRENCY", 1) ] end diff --git a/deploy/helm/who-need-help/templates/deployments.yaml b/deploy/helm/who-need-help/templates/deployments.yaml index 8148969..9a09a5d 100644 --- a/deploy/helm/who-need-help/templates/deployments.yaml +++ b/deploy/helm/who-need-help/templates/deployments.yaml @@ -95,6 +95,8 @@ spec: value: {{ $root.Values.worker.maintenanceConcurrency | quote }} - name: OBAN_PUSH_CONCURRENCY value: {{ $root.Values.worker.pushConcurrency | quote }} + - name: OBAN_MAIL_CONCURRENCY + value: {{ $root.Values.worker.mailConcurrency | quote }} - name: EMAIL_DELIVERY_PROVIDER value: {{ $root.Values.app.emailDeliveryProvider | quote }} - name: SMTP_RELAY diff --git a/deploy/helm/who-need-help/templates/migrate-job.yaml b/deploy/helm/who-need-help/templates/migrate-job.yaml index 347a132..2ea3a88 100644 --- a/deploy/helm/who-need-help/templates/migrate-job.yaml +++ b/deploy/helm/who-need-help/templates/migrate-job.yaml @@ -43,6 +43,8 @@ spec: value: {{ .Values.worker.maintenanceConcurrency | quote }} - name: OBAN_PUSH_CONCURRENCY value: {{ .Values.worker.pushConcurrency | quote }} + - name: OBAN_MAIL_CONCURRENCY + value: {{ .Values.worker.mailConcurrency | quote }} - name: EMAIL_DELIVERY_PROVIDER value: {{ .Values.app.emailDeliveryProvider | quote }} securityContext: diff --git a/deploy/helm/who-need-help/values.yaml b/deploy/helm/who-need-help/values.yaml index 51d88d3..17593c6 100644 --- a/deploy/helm/who-need-help/values.yaml +++ b/deploy/helm/who-need-help/values.yaml @@ -12,6 +12,7 @@ worker: poolSize: "2" maintenanceConcurrency: "2" pushConcurrency: "1" + mailConcurrency: "1" service: type: ClusterIP diff --git a/docs/architecture.md b/docs/architecture.md index f284178..ab291fd 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -75,7 +75,9 @@ and are not represented as complete. provider-neutral gateway delivery, direct Web Push and FCM device delivery, invalid-registration cleanup, and request/message/lifecycle/nearby events. - `Notifications`: the private inbox, device registry, user preferences, quiet - hours, durable email delivery, and PostGIS-backed nearby subscriptions. + hours, optional push, and PostGIS-backed nearby subscriptions. Immediate + per-match nearby email is retired; a future digest requires an explicit + batching cadence and delivery cursor. Subscription centers and push credentials are never part of public discovery results. - `ProductAnalytics`: daily aggregate counters from a fixed metric allow-list. diff --git a/docs/operations.md b/docs/operations.md index cd33a5f..fd017a5 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -442,8 +442,9 @@ Set optional `SUPPORT_INBOX_ADDRESS` to a monitored address for support/removal queue alerts and email `Reply-To`. Public support creates an alert only after email verification; authenticated support is already verified, while content-removal notification rules remain -separate. Leaving the value empty disables operator email alerts, not the -protected queues. If +separate. The alert is queued in `mail`; conversation messages remain in the +permission-scoped staff workspace rather than producing email per message. +Leaving the value empty disables operator email alerts, not the protected queues. If Google registration/sign-in is enabled, also set both Google Web client credentials and register `https://YOUR_PHX_HOST/auth/google/callback` as the exact authorized redirect @@ -1021,10 +1022,15 @@ requirements. ## Isolated Oban burst measurement -The worker role consumes only `maintenance` and `push`. Configure their -per-worker limits with `OBAN_MAINTENANCE_CONCURRENCY` and -`OBAN_PUSH_CONCURRENCY`; multiplying either value by the number of worker -replicas gives the configured cluster-wide concurrency for that queue. +The worker role consumes `maintenance`, `push`, and `mail`. Configure their +per-worker limits with `OBAN_MAINTENANCE_CONCURRENCY`, +`OBAN_PUSH_CONCURRENCY`, and `OBAN_MAIL_CONCURRENCY`; multiplying a value by +the number of worker replicas gives the configured cluster-wide concurrency +for that queue. Authentication and mandatory legal-intake confirmations remain +synchronous. Ordinary support-update email runs in `mail`, so SMTP latency does +not occupy a web request or the `push` queue. Optional new-case operator alerts +also run in `mail`; requester conversation messages do not generate operator +email. After starting the isolated load project, run an explicitly sized experiment: diff --git a/docs/support-and-content-removal.md b/docs/support-and-content-removal.md index 4a2cc7c..25fda22 100644 --- a/docs/support-and-content-removal.md +++ b/docs/support-and-content-removal.md @@ -45,9 +45,11 @@ status token in the redirect. They are stored as `pending_verification`, are not visible in the operator queue, and do not produce an operator alert. The private status link is sent to the contact email. Opening it atomically verifies the address, changes the request to `open`, makes it visible to authorised staff, -and sends one metadata-only operator alert. Opening the same link again does not -send another alert. An authenticated submission uses the confirmed account -email, is verified immediately, and enters the queue without this extra step. +and enqueues at most one metadata-only operator alert when that optional mode is +enabled. Opening the same link again does not send another alert. An authenticated +submission uses the confirmed account email, is verified immediately, and enters +the queue without this extra step. Ongoing requester messages update the staff +workspace through PubSub and do not generate one operator email per message. Exact repeats of the same unverified public support submission are represented by one pending row. Its temporary fingerprint is a SHA-256 digest of normalized diff --git a/docs/verification.md b/docs/verification.md index 8a8bb9a..ef70130 100644 --- a/docs/verification.md +++ b/docs/verification.md @@ -1068,7 +1068,7 @@ this audit. | Privacy settings | Implemented and browser-verified | The profile exposed hidden, approximate public, exact for active match, and explicit exact-public options. Blocking and current-position cleanup have automated tests. | Exact public location remains a user opt-in; legal privacy and retention text still requires jurisdiction-specific review before launch. | | Reputation and anti-abuse | Implemented at MVP level | Handover codes, two-party completion, double-blind reviews, unique-counterpart ranking, optional movement/proximity evidence, reports, blocks, abuse signals, and moderator audit paths have automated tests. | The system is not bot-proof and does not claim identity verification. No punitive numeric policy is enabled without measured and approved thresholds. | | Account registration and sign-in | Implemented and browser/physical-device verified | Email registration is a single passwordless flow: it records the display name and acceptance once, sends a confirmation link, and does not duplicate a user on subsequent sign-in. Confirmed users can keep using magic links or add a password in settings. Google OpenID Connect registration, sign-in, link, unlink, replay prevention, verified-email enforcement, and account-ownership rules are covered by the automated suite. Real headed Chrome exercised the development callback and identity-linking paths. The Play-delivered production build then completed production Google sign-in without a secondary ownership email or duplicate account. A production authentication email was also observed in the external Gmail mailbox with `whoneedhelp.com` DKIM signing. | Brevo rewrites the action href through its tracking domain; direct production-domain action URLs remain an open deliverability/privacy check. | -| Notifications and nearby alerts | Implemented and browser/physical-device verified | Users can configure push/email preferences, quiet hours, category/urgency/day/time filters, a private matching center, and 1/3/5/10/25 km radii. Durable inbox notifications and Oban delivery jobs are tested; public notification payloads omit chat text, exact coordinates, and the private saved-area label. Development Web Push and FCM delivery were exercised. The Play-delivered production build registered its FCM device, received one run-scoped production notification, and routed its tap to the in-app inbox; exact cleanup removed that notification and its jobs. A guarded production smoke then delivered one browser-only job to the newest real active Web Push subscription on its first attempt and removed the exact job and notification. | Provider acceptance and the still-active subscription are verified; visible operating-system presentation and click navigation were not programmatically observed. | +| Notifications and nearby alerts | Implemented and browser/physical-device verified | Users can configure push, quiet hours, category/urgency/day/time filters, a private matching center, and 1/3/5/10/25 km radii. Durable inbox notifications and Oban push jobs are tested; public notification payloads omit chat text, exact coordinates, and the private saved-area label. Immediate per-request nearby email is retired. Development Web Push and FCM delivery were exercised. The Play-delivered production build registered its FCM device, received one run-scoped production notification, and routed its tap to the in-app inbox; exact cleanup removed that notification and its jobs. A guarded production smoke then delivered one browser-only job to the newest real active Web Push subscription on its first attempt and removed the exact job and notification. | A batched nearby email digest is not implemented; it requires a defined cadence and delivery cursor. Provider acceptance and the still-active subscription are verified; visible operating-system presentation and click navigation were not programmatically observed. | | Social profiles | Manual links implemented; optional GitHub verification implemented and automated-tested | Manual links cannot set verification fields. The optional GitHub flow uses state, PKCE, a user-bound one-time session, unique provider ownership, and an audit record. The local protocol drill also performs real HTTP token/user exchanges without returning an access token to the application. | GitHub OAuth credentials are intentionally absent and are not required for registration or the help flow. The real external provider redirect/callback remains disabled and unverified; other providers remain manual/unverified. | | Support and content removal | Implemented and browser-verified | Public support, account deletion, general removal, and TAKE IT DOWN forms use separate audited workflows; public support remains pending and outside the staff queue until its private email link verifies the contact, while authenticated submissions use the account email immediately. Exact pending repeats are deduplicated, email/IP intake limits are independently configurable, and moderator-only operations can update verified cases. TAKE IT DOWN accepts URLs/text only and records a 48-hour review due time. Authenticated users can download an allowlisted JSON export, and moderators can run a read-only deletion relationship preflight. | The current product hosts no user media and does not claim TAKE IT DOWN coverage. Staffing, measured rate-limit thresholds, jurisdiction-specific legal classification, final retention rules, destructive account erasure/anonymisation, and identical-media-copy handling remain operational/legal work. | | Voluntary thanks | Implemented as an external optional link | A helper can expose an optional link after completion; the UI states that the platform does not process the payment. | The platform does not provide payments, escrow, refunds, tax reporting, or payment guarantees. | diff --git a/lib/who_need_help/accounts/auth_identity.ex b/lib/who_need_help/accounts/auth_identity.ex index db9bbb5..a5f3937 100644 --- a/lib/who_need_help/accounts/auth_identity.ex +++ b/lib/who_need_help/accounts/auth_identity.ex @@ -2,6 +2,8 @@ defmodule WhoNeedHelp.Accounts.AuthIdentity do use Ecto.Schema import Ecto.Changeset + alias WhoNeedHelp.EmailAddress + @primary_key {:id, :binary_id, autogenerate: true} @foreign_key_type :binary_id @@ -21,9 +23,7 @@ defmodule WhoNeedHelp.Accounts.AuthIdentity do |> validate_required([:provider, :provider_uid, :email, :user_id]) |> validate_inclusion(:provider, [:google]) |> validate_length(:provider_uid, min: 1, max: 255) - |> validate_format(:email, ~r/^[^@,;\s]+@[^@,;\s]+$/, - message: "must have the @ sign and no spaces" - ) + |> EmailAddress.validate(:email) |> validate_length(:email, max: 160) |> unique_constraint([:provider, :provider_uid]) |> unique_constraint([:user_id, :provider]) diff --git a/lib/who_need_help/accounts/user.ex b/lib/who_need_help/accounts/user.ex index 1d91719..bb64197 100644 --- a/lib/who_need_help/accounts/user.ex +++ b/lib/who_need_help/accounts/user.ex @@ -3,6 +3,7 @@ defmodule WhoNeedHelp.Accounts.User do import Ecto.Changeset alias WhoNeedHelp.Locales + alias WhoNeedHelp.EmailAddress @primary_key {:id, :binary_id, autogenerate: true} @foreign_key_type :binary_id @@ -133,9 +134,7 @@ defmodule WhoNeedHelp.Accounts.User do changeset = changeset |> validate_required([:email]) - |> validate_format(:email, ~r/^[^@,;\s]+@[^@,;\s]+$/, - message: "must have the @ sign and no spaces" - ) + |> EmailAddress.validate(:email) |> validate_length(:email, max: 160) if Keyword.get(opts, :validate_unique, true) do diff --git a/lib/who_need_help/accounts/user_notifier.ex b/lib/who_need_help/accounts/user_notifier.ex index 98f363f..eb9bc2e 100644 --- a/lib/who_need_help/accounts/user_notifier.ex +++ b/lib/who_need_help/accounts/user_notifier.ex @@ -3,11 +3,11 @@ defmodule WhoNeedHelp.Accounts.UserNotifier do import Swoosh.Email - alias WhoNeedHelp.Mailer alias WhoNeedHelp.Accounts.User + alias WhoNeedHelp.EmailDelivery # Delivers the email using the application mailer. - defp deliver(recipient, subject, text_body, html_body) do + defp deliver(recipient, subject, text_body, html_body, kind) do from = Application.fetch_env!(:who_need_help, :mailer_from) email = @@ -18,7 +18,7 @@ defmodule WhoNeedHelp.Accounts.UserNotifier do |> text_body(text_body) |> html_body(html_body) - with {:ok, _metadata} <- Mailer.deliver(email) do + with {:ok, _metadata} <- EmailDelivery.deliver(email, kind) do {:ok, email} end end @@ -29,6 +29,7 @@ defmodule WhoNeedHelp.Accounts.UserNotifier do def deliver_update_email_instructions(user, url) do with_user_locale(user, fn -> deliver_action_email(user, + kind: :auth_email_change, subject: gettext("Confirm your Who Need Help email change"), heading: gettext("Confirm your new email address"), introduction: @@ -58,6 +59,7 @@ defmodule WhoNeedHelp.Accounts.UserNotifier do def deliver_google_link_instructions(user, url) do with_user_locale(user, fn -> deliver_action_email(user, + kind: :auth_google_link, subject: gettext("Confirm Google sign-in for Who Need Help"), heading: gettext("Confirm Google sign-in"), introduction: @@ -74,6 +76,7 @@ defmodule WhoNeedHelp.Accounts.UserNotifier do defp deliver_magic_link_instructions(user, url) do with_user_locale(user, fn -> deliver_action_email(user, + kind: :auth_login, subject: gettext("Your Who Need Help sign-in link"), heading: gettext("Sign in to Who Need Help"), introduction: gettext("Use the secure link below to sign in to your account."), @@ -89,6 +92,7 @@ defmodule WhoNeedHelp.Accounts.UserNotifier do defp deliver_confirmation_instructions(user, url) do with_user_locale(user, fn -> deliver_action_email(user, + kind: :auth_registration, subject: gettext("Confirm your Who Need Help account"), heading: gettext("Confirm your account"), introduction: gettext("Use the secure link below to confirm your Who Need Help account."), @@ -102,6 +106,7 @@ defmodule WhoNeedHelp.Accounts.UserNotifier do end defp deliver_action_email(user, content) do + kind = Keyword.fetch!(content, :kind) subject = Keyword.fetch!(content, :subject) heading = Keyword.fetch!(content, :heading) introduction = Keyword.fetch!(content, :introduction) @@ -126,7 +131,7 @@ defmodule WhoNeedHelp.Accounts.UserNotifier do html = action_email_html(heading, introduction, action_label, url, expiry_note, security_note) - deliver(user.email, subject, text, html) + deliver(user.email, subject, text, html, kind) end defp action_email_html(heading, introduction, action_label, url, expiry_note, security_note) do diff --git a/lib/who_need_help/content_removal/notice.ex b/lib/who_need_help/content_removal/notice.ex index ee06d4b..f1676a7 100644 --- a/lib/who_need_help/content_removal/notice.ex +++ b/lib/who_need_help/content_removal/notice.ex @@ -2,6 +2,8 @@ defmodule WhoNeedHelp.ContentRemoval.Notice do use Ecto.Schema import Ecto.Changeset + alias WhoNeedHelp.EmailAddress + @primary_key {:id, :binary_id, autogenerate: true} @foreign_key_type :binary_id @@ -81,9 +83,7 @@ defmodule WhoNeedHelp.ContentRemoval.Notice do :regime ]) |> update_change(:contact_email, &normalize_email/1) - |> validate_format(:contact_email, ~r/^[^@,;\s]+@[^@,;\s]+$/, - message: "must have the @ sign and no spaces" - ) + |> EmailAddress.validate(:contact_email) |> validate_length(:submitter_name, min: 2, max: 160) |> validate_length(:contact_email, max: 160) |> validate_length(:content_locations, min: 5, max: 10_000) diff --git a/lib/who_need_help/content_removal/notifier.ex b/lib/who_need_help/content_removal/notifier.ex index 3f8ef05..6255a1a 100644 --- a/lib/who_need_help/content_removal/notifier.ex +++ b/lib/who_need_help/content_removal/notifier.ex @@ -2,7 +2,7 @@ defmodule WhoNeedHelp.ContentRemoval.Notifier do import Swoosh.Email alias WhoNeedHelp.ContentRemoval.Notice - alias WhoNeedHelp.Mailer + alias WhoNeedHelp.EmailDelivery def deliver_confirmation(%Notice{contact_email: email} = notice, status_url) when is_binary(email) and email != "" do @@ -16,7 +16,8 @@ defmodule WhoNeedHelp.ContentRemoval.Notifier do #{status_url} If you did not make this request, ignore this email. It will not reach the review queue. - """ + """, + :content_removal_confirmation ) end @@ -37,7 +38,8 @@ defmodule WhoNeedHelp.ContentRemoval.Notifier do Do not reply with or upload intimate visual material. If someone is in immediate danger, contact the emergency service for their location. - """ + """, + :content_removal_received ) end @@ -57,7 +59,8 @@ defmodule WhoNeedHelp.ContentRemoval.Notifier do View the current status: #{status_url} - """ + """, + :content_removal_update ) end @@ -80,7 +83,8 @@ defmodule WhoNeedHelp.ContentRemoval.Notifier do Operator queue: #{WhoNeedHelpWeb.Endpoint.url()}/support/operations Do not request the submitter to email or upload intimate visual material. - """ + """, + :content_removal_operator ) {:error, reason} -> @@ -88,7 +92,7 @@ defmodule WhoNeedHelp.ContentRemoval.Notifier do end end - defp deliver(recipient, subject, body) do + defp deliver(recipient, subject, body, kind) do from = Application.fetch_env!(:who_need_help, :mailer_from) new() @@ -97,7 +101,7 @@ defmodule WhoNeedHelp.ContentRemoval.Notifier do |> maybe_reply_to() |> subject(subject) |> text_body(body) - |> Mailer.deliver() + |> EmailDelivery.deliver(kind) end defp maybe_reply_to(email) do diff --git a/lib/who_need_help/email_address.ex b/lib/who_need_help/email_address.ex new file mode 100644 index 0000000..719caf7 --- /dev/null +++ b/lib/who_need_help/email_address.ex @@ -0,0 +1,43 @@ +defmodule WhoNeedHelp.EmailAddress do + @moduledoc false + + @spec valid?(term()) :: boolean() + def valid?(address) when is_binary(address) do + with [local, domain] <- String.split(address, "@"), + true <- valid_dot_atom?(local), + true <- valid_domain?(domain), + {:ok, [{_name, parsed_address}]} <- :smtp_util.parse_rfc5322_addresses(address) do + to_string(parsed_address) == address + else + _other -> false + end + end + + def valid?(_address), do: false + + def validate(changeset, field, message \\ "must have the @ sign and no spaces") do + Ecto.Changeset.validate_change(changeset, field, fn ^field, value -> + if valid?(value), do: [], else: [{field, message}] + end) + end + + defp valid_dot_atom?(local) do + local != "" and + Regex.match?(~r/^[A-Za-z0-9.!#$%&'*+\/=\?^_`{|}~-]+$/, local) and + not String.starts_with?(local, ".") and + not String.ends_with?(local, ".") and + not String.contains?(local, "..") + end + + defp valid_domain?(domain) do + labels = String.split(domain, ".") + + length(labels) >= 2 and + Enum.all?(labels, fn label -> + label != "" and + Regex.match?(~r/^[A-Za-z0-9-]+$/, label) and + not String.starts_with?(label, "-") and + not String.ends_with?(label, "-") + end) + end +end diff --git a/lib/who_need_help/email_delivery.ex b/lib/who_need_help/email_delivery.ex new file mode 100644 index 0000000..79d672f --- /dev/null +++ b/lib/who_need_help/email_delivery.ex @@ -0,0 +1,57 @@ +defmodule WhoNeedHelp.EmailDelivery do + @moduledoc """ + Delivers email while emitting low-cardinality, purpose-specific telemetry. + + Telemetry metadata contains only fixed type and outcome labels. Recipients, + subjects, bodies, references, and other personal data are never attached. + """ + + alias WhoNeedHelp.Mailer + + @kinds [ + :auth_email_change, + :auth_google_link, + :auth_login, + :auth_registration, + :content_removal_confirmation, + :content_removal_operator, + :content_removal_received, + :content_removal_update, + :nearby_alert, + :support_confirmation, + :support_operator, + :support_update + ] + + @spec deliver(Swoosh.Email.t(), atom()) :: {:ok, term()} | {:error, term()} + def deliver(%Swoosh.Email{} = email, kind) when kind in @kinds do + started_at = System.monotonic_time() + + try do + result = Mailer.deliver(email) + emit(started_at, kind, result_status(result)) + result + rescue + exception -> + emit(started_at, kind, :exception) + reraise exception, __STACKTRACE__ + catch + class, reason -> + emit(started_at, kind, :exception) + :erlang.raise(class, reason, __STACKTRACE__) + end + end + + def kinds, do: @kinds + + defp result_status({:ok, _metadata}), do: :ok + defp result_status({:error, _reason}), do: :error + + defp emit(started_at, kind, status) do + :telemetry.execute( + [:who_need_help, :email, :delivery], + %{duration: System.monotonic_time() - started_at}, + %{kind: to_string(kind), status: to_string(status)} + ) + end +end diff --git a/lib/who_need_help/mail/support_operator_alert_worker.ex b/lib/who_need_help/mail/support_operator_alert_worker.ex new file mode 100644 index 0000000..bf0acfd --- /dev/null +++ b/lib/who_need_help/mail/support_operator_alert_worker.ex @@ -0,0 +1,30 @@ +defmodule WhoNeedHelp.Mail.SupportOperatorAlertWorker do + @moduledoc false + + use Oban.Worker, + queue: :mail, + max_attempts: 8, + unique: [ + period: :infinity, + fields: [:args, :worker], + keys: [:request_id], + states: :incomplete + ] + + alias WhoNeedHelp.Repo + alias WhoNeedHelp.Support.{Notifier, SupportRequest} + + @impl Oban.Worker + def perform(%Oban.Job{args: %{"request_id" => request_id}}) do + case Repo.get(SupportRequest, request_id) do + nil -> + {:cancel, :support_request_missing} + + %SupportRequest{contact_verified_at: nil} -> + {:cancel, :contact_not_verified} + + %SupportRequest{} = request -> + Notifier.deliver_operator_alert(request) + end + end +end diff --git a/lib/who_need_help/mail/support_update_worker.ex b/lib/who_need_help/mail/support_update_worker.ex new file mode 100644 index 0000000..0a0cba9 --- /dev/null +++ b/lib/who_need_help/mail/support_update_worker.ex @@ -0,0 +1,44 @@ +defmodule WhoNeedHelp.Mail.SupportUpdateWorker do + @moduledoc false + + use Oban.Worker, + queue: :mail, + max_attempts: 8, + unique: [ + period: :infinity, + fields: [:args, :worker], + keys: [:request_id], + states: :incomplete + ] + + alias WhoNeedHelp.Repo + alias WhoNeedHelp.Support + alias WhoNeedHelp.Support.{Notifier, SupportRequest} + + @impl Oban.Worker + def perform(%Oban.Job{args: %{"request_id" => request_id}}) do + case Repo.get(SupportRequest, request_id) do + nil -> + {:cancel, :support_request_missing} + + %SupportRequest{contact_verified_at: nil} -> + {:cancel, :contact_not_verified} + + %SupportRequest{} = request -> + case Notifier.deliver_decision(request, Support.status_url(request)) do + {:ok, _metadata} -> mark_sent(request) + {:error, reason} -> {:error, reason} + end + end + end + + defp mark_sent(request) do + request + |> Ecto.Changeset.change(response_sent_at: DateTime.utc_now(:second)) + |> Repo.update() + |> case do + {:ok, _request} -> :ok + {:error, reason} -> {:error, reason} + end + end +end diff --git a/lib/who_need_help/notifications.ex b/lib/who_need_help/notifications.ex index af5ea21..3756c2e 100644 --- a/lib/who_need_help/notifications.ex +++ b/lib/who_need_help/notifications.ex @@ -359,8 +359,7 @@ defmodule WhoNeedHelp.Notifications do "request_id" => request.id, "category_id" => request.category_id, "urgency" => to_string(request.urgency), - "push_enabled" => subscription.push_enabled, - "email_enabled" => subscription.email_enabled + "push_enabled" => subscription.push_enabled }, idempotency_key: "nearby-request:#{request.id}:#{subscription.user_id}:#{safe_event_key(event_key)}" @@ -382,16 +381,6 @@ defmodule WhoNeedHelp.Notifications do end end - def email_allowed?( - %Preference{} = preference, - %Notification{kind: :nearby_request} = notification - ) do - preference.email_enabled and preference.nearby_email_enabled and - Map.get(notification.data, "email_enabled", false) - end - - def email_allowed?(%Preference{}, %Notification{}), do: false - def quiet_now?(%Preference{quiet_hours_enabled: false}, _now), do: false def quiet_now?( diff --git a/lib/who_need_help/notifications/email_notifier.ex b/lib/who_need_help/notifications/email_notifier.ex deleted file mode 100644 index d5e9ee8..0000000 --- a/lib/who_need_help/notifications/email_notifier.ex +++ /dev/null @@ -1,36 +0,0 @@ -defmodule WhoNeedHelp.Notifications.EmailNotifier do - @moduledoc false - - use Gettext, backend: WhoNeedHelpWeb.Gettext - - import Swoosh.Email - - alias WhoNeedHelp.Accounts.User - alias WhoNeedHelp.Mailer - alias WhoNeedHelp.Notifications.Notification - - def deliver(%User{} = user, %Notification{kind: :nearby_request} = notification) do - with_user_locale(user, fn -> - from = Application.fetch_env!(:who_need_help, :mailer_from) - url = WhoNeedHelpWeb.Endpoint.url() <> notification.path - - email = - new() - |> to(user.email) - |> from({from[:name], from[:address]}) - |> subject(gettext("New help request nearby")) - |> text_body( - gettext( - "A request matching one of your nearby-help alerts is available.\n\nOpen Who Need Help to review the public details:\n%{url}\n\nYou can change nearby alerts and email delivery in Notification settings.", - url: url - ) - ) - - Mailer.deliver(email) - end) - end - - defp with_user_locale(%User{locale: locale}, fun) do - Gettext.with_locale(WhoNeedHelpWeb.Gettext, WhoNeedHelp.Locales.normalize(locale), fun) - end -end diff --git a/lib/who_need_help/notifications/nearby_subscription.ex b/lib/who_need_help/notifications/nearby_subscription.ex index 1ce435a..e31dff8 100644 --- a/lib/who_need_help/notifications/nearby_subscription.ex +++ b/lib/who_need_help/notifications/nearby_subscription.ex @@ -60,11 +60,6 @@ defmodule WhoNeedHelp.Notifications.NearbySubscription do |> validate_subset(:available_days, 1..7) |> validate_length(:category_ids, max: 50) |> validate_schedule() - |> validate_delivery_channel() - |> check_constraint(:push_enabled, - name: :nearby_subscriptions_delivery_channel_required, - message: "select push, email, or both" - ) end def allowed_radii, do: @allowed_radii @@ -112,12 +107,4 @@ defmodule WhoNeedHelp.Notifications.NearbySubscription do add_error(changeset, :available_until, "set both availability times or leave both empty") end end - - defp validate_delivery_channel(changeset) do - if get_field(changeset, :push_enabled) or get_field(changeset, :email_enabled) do - changeset - else - add_error(changeset, :push_enabled, "select push, email, or both") - end - end end diff --git a/lib/who_need_help/push/notification_dispatch_worker.ex b/lib/who_need_help/push/notification_dispatch_worker.ex index 35e3ba4..9cb8ee6 100644 --- a/lib/who_need_help/push/notification_dispatch_worker.ex +++ b/lib/who_need_help/push/notification_dispatch_worker.ex @@ -8,7 +8,7 @@ defmodule WhoNeedHelp.Push.NotificationDispatchWorker do alias WhoNeedHelp.Notifications alias WhoNeedHelp.Notifications.{Notification, Preference, Text} alias WhoNeedHelp.Push - alias WhoNeedHelp.Push.{DeliveryWorker, DeviceDeliveryWorker, NotificationEmailWorker} + alias WhoNeedHelp.Push.{DeliveryWorker, DeviceDeliveryWorker} alias WhoNeedHelp.Repo @impl Oban.Worker @@ -24,10 +24,9 @@ defmodule WhoNeedHelp.Push.NotificationDispatchWorker do %Preference{user_id: notification.user_id} push_allowed? = Notifications.push_allowed?(preference, notification) - email_allowed? = Notifications.email_allowed?(preference, notification) cond do - not push_allowed? and not email_allowed? -> + not push_allowed? -> :ok Notifications.quiet_now?(preference, now) -> @@ -40,19 +39,14 @@ defmodule WhoNeedHelp.Push.NotificationDispatchWorker do {:snooze, seconds} true -> - case dispatch_push(localized_notification, push_allowed?) do - :ok -> dispatch_email(notification, email_allowed?) - {:error, _reason} = error -> error - end + dispatch_push(localized_notification) end else nil -> {:cancel, :notification_missing} end end - defp dispatch_push(_notification, false), do: :ok - - defp dispatch_push(notification, true) do + defp dispatch_push(notification) do devices = Notifications.active_devices(notification.user_id) cond do @@ -80,18 +74,6 @@ defmodule WhoNeedHelp.Push.NotificationDispatchWorker do end end - defp dispatch_email(_notification, false), do: :ok - - defp dispatch_email(notification, true) do - notification.id - |> then(&NotificationEmailWorker.new(%{notification_id: &1})) - |> Oban.insert() - |> case do - {:ok, _job} -> :ok - {:error, reason} -> {:error, reason} - end - end - defp gateway_notification(notification) do %{ idempotency_key: notification.idempotency_key, diff --git a/lib/who_need_help/push/notification_email_worker.ex b/lib/who_need_help/push/notification_email_worker.ex index 7ca6e96..ed84adc 100644 --- a/lib/who_need_help/push/notification_email_worker.ex +++ b/lib/who_need_help/push/notification_email_worker.ex @@ -1,40 +1,16 @@ defmodule WhoNeedHelp.Push.NotificationEmailWorker do - @moduledoc false + @moduledoc """ + Retires legacy immediate nearby-email jobs without delivering them. + + New notification dispatches no longer enqueue this worker. Keeping the module + lets Oban resolve and cancel jobs that were persisted before immediate nearby + email was retired. + """ use Oban.Worker, - queue: :push, - max_attempts: 8, - unique: [period: :infinity, fields: [:args, :worker], keys: [:notification_id]] - - alias WhoNeedHelp.Notifications - alias WhoNeedHelp.Notifications.{EmailNotifier, Notification, Preference, Text} - alias WhoNeedHelp.Repo + queue: :mail, + max_attempts: 1 @impl Oban.Worker - def perform(%Oban.Job{args: %{"notification_id" => notification_id}}) do - notification = Repo.get(Notification, notification_id) - - case notification do - nil -> - {:cancel, :notification_missing} - - %Notification{} = notification -> - notification = Repo.preload(notification, :user) - - preference = - Repo.get_by(Preference, user_id: notification.user_id) || - %Preference{user_id: notification.user_id} - - if Notifications.email_allowed?(preference, notification) do - localized_notification = Text.localize(notification, notification.user.locale) - - case EmailNotifier.deliver(notification.user, localized_notification) do - {:ok, _metadata} -> :ok - {:error, reason} -> {:error, reason} - end - else - {:cancel, :email_disabled} - end - end - end + def perform(_job), do: {:cancel, :immediate_nearby_email_retired} end diff --git a/lib/who_need_help/support.ex b/lib/who_need_help/support.ex index 78f64d3..336effc 100644 --- a/lib/who_need_help/support.ex +++ b/lib/who_need_help/support.ex @@ -6,6 +6,8 @@ defmodule WhoNeedHelp.Support do alias WhoNeedHelp.Accounts alias WhoNeedHelp.Accounts.DataLifecycle alias WhoNeedHelp.Accounts.{Scope, User} + alias WhoNeedHelp.Mail.{SupportOperatorAlertWorker, SupportUpdateWorker} + alias WhoNeedHelp.Notifications alias WhoNeedHelp.Pagination alias WhoNeedHelp.Repo alias WhoNeedHelp.Support.{ConversationMessage, Notifier, StatusEvent, SupportRequest} @@ -250,9 +252,9 @@ defmodule WhoNeedHelp.Support do moderator.id, :staff ), - {:ok, _message} <- + {:ok, message} <- maybe_record_message(request, moderator.id, :staff, response_to_record), - {:ok, _audit} <- + {:ok, audit} <- Trust.audit( moderator.id, "support_request.moderated", @@ -263,7 +265,13 @@ defmodule WhoNeedHelp.Support do "assigned_to_id" => request.assigned_to_id } ) do - {:ok, preload_conversation(request)} + {:ok, + {preload_conversation(request), + %{ + audit_id: audit.id, + message_added: match?(%ConversationMessage{}, message), + status_changed: previous_status != request.status + }}} end else {:error, :not_found} @@ -335,7 +343,6 @@ defmodule WhoNeedHelp.Support do {:error, :not_found} end end) - |> notify_operator_update() |> then(fn {:ok, {updated, _message}} -> {:ok, preload_conversation(updated)} error -> error @@ -380,7 +387,6 @@ defmodule WhoNeedHelp.Support do {:error, :invalid_status} end end) - |> notify_operator_update() |> then(fn {:ok, updated} -> {:ok, preload_conversation(updated)} error -> error @@ -413,47 +419,30 @@ defmodule WhoNeedHelp.Support do end defp notify_created({:ok, {request, :verified}}) do - _ = Notifier.deliver_received(request, status_url(request)) - _ = Notifier.deliver_operator_alert(request) + _ = enqueue_operator_alert(request) {:ok, request} end defp notify_created({:ok, {request, :duplicate}}), do: {:ok, request} defp notify_created(result), do: result - defp notify_decision({:ok, %SupportRequest{contact_verified_at: nil} = request}), + defp notify_decision({:ok, {%SupportRequest{contact_verified_at: nil} = request, _update}}), do: {:ok, request} - defp notify_decision({:ok, request}) do - case Notifier.deliver_decision(request, status_url(request)) do - {:ok, _metadata} -> - request - |> Ecto.Changeset.change(response_sent_at: DateTime.utc_now(:second)) - |> Repo.update() - |> then(fn - {:ok, updated} -> {:ok, preload_conversation(updated)} - error -> error - end) - - _error -> - {:ok, request} + defp notify_decision({:ok, {%SupportRequest{} = request, update}}) do + if update.status_changed or update.message_added do + notify_requester_in_app(request, update) end + + if support_email_needed?(request, update) do + _ = enqueue_support_email(request) + end + + {:ok, request} end defp notify_decision(result), do: result - defp notify_operator_update({:ok, {%SupportRequest{} = request, _message}} = result) do - _ = Notifier.deliver_requester_update(request) - result - end - - defp notify_operator_update({:ok, %SupportRequest{} = request} = result) do - _ = Notifier.deliver_requester_update(request) - result - end - - defp notify_operator_update(result), do: result - defp broadcast_request_update({:ok, %SupportRequest{} = request} = result) do event = {:support_request_updated, request.id} @@ -522,8 +511,7 @@ defmodule WhoNeedHelp.Support do defp verify_contact(%SupportRequest{} = request), do: {:ok, request} defp notify_verified_request({:ok, {request, :newly_verified}}) do - _ = Notifier.deliver_received(request, status_url(request)) - _ = Notifier.deliver_operator_alert(request) + _ = enqueue_operator_alert(request) event = {:support_request_updated, request.id} Phoenix.PubSub.broadcast(WhoNeedHelp.PubSub, request_topic(request.id), event) @@ -535,6 +523,44 @@ defmodule WhoNeedHelp.Support do defp notify_verified_request({:ok, {request, :already_verified}}), do: {:ok, request} defp notify_verified_request(result), do: result + defp notify_requester_in_app(%SupportRequest{requester_id: nil}, _update), do: :ok + + defp notify_requester_in_app(%SupportRequest{} = request, update) do + Notifications.notify_user(request.requester_id, %{ + kind: :support_update, + title: "Support request updated", + body: "Open Who Need Help to review the latest update.", + path: "/support/cases/#{request.id}", + idempotency_key: "support-request-update:#{update.audit_id}:#{request.requester_id}", + data: %{ + "kind" => "support_request_updated", + "request_id" => request.id, + "status" => to_string(request.status), + "response_added" => update.message_added + } + }) + end + + defp support_email_needed?(request, update) do + update.status_changed or (update.message_added and is_nil(request.response_sent_at)) + end + + defp enqueue_support_email(request) do + request.id + |> then(&SupportUpdateWorker.new(%{request_id: &1})) + |> Oban.insert() + end + + defp enqueue_operator_alert(request) do + if Notifier.operator_alerts_enabled?() do + request.id + |> then(&SupportOperatorAlertWorker.new(%{request_id: &1})) + |> Oban.insert() + else + {:ok, :disabled} + end + end + defp verify_legacy_confirmation(%SupportRequest{id: id} = request, token) do case Phoenix.Token.verify(WhoNeedHelpWeb.Endpoint, @access_salt, token, max_age: contact_verification_max_age_seconds() diff --git a/lib/who_need_help/support/notifier.ex b/lib/who_need_help/support/notifier.ex index 07d207b..994b5bb 100644 --- a/lib/who_need_help/support/notifier.ex +++ b/lib/who_need_help/support/notifier.ex @@ -1,7 +1,7 @@ defmodule WhoNeedHelp.Support.Notifier do import Swoosh.Email - alias WhoNeedHelp.Mailer + alias WhoNeedHelp.EmailDelivery alias WhoNeedHelp.Support.SupportRequest def deliver_confirmation(%SupportRequest{} = request, status_url) do @@ -15,25 +15,8 @@ defmodule WhoNeedHelp.Support.Notifier do #{status_url} If you did not make this request, ignore this email. It will not reach the support queue. - """ - ) - end - - def deliver_received(%SupportRequest{} = request, status_url) do - deliver( - request.contact_email, - "Who Need Help support request #{request.reference}", - """ - We received your support request #{request.reference}. - - Subject: #{request.subject} - Status: #{request.status} - - View the current status: - #{status_url} - - If there is immediate danger, contact the emergency service for your location. - """ + """, + :support_confirmation ) end @@ -45,12 +28,11 @@ defmodule WhoNeedHelp.Support.Notifier do Your support request #{request.reference} was updated. Status: #{request.status} - Response: - #{request.resolution_note || "No additional response was provided."} - View the current status: + Open the private status page to read the latest response and continue the conversation: #{status_url} - """ + """, + :support_update ) end @@ -67,7 +49,8 @@ defmodule WhoNeedHelp.Support.Notifier do Operator queue: #{WhoNeedHelpWeb.Endpoint.url()}/support/operations Sign in with an authorised moderator or administrator account to review it. - """ + """, + :support_operator ) {:error, reason} -> @@ -75,29 +58,9 @@ defmodule WhoNeedHelp.Support.Notifier do end end - def deliver_requester_update(%SupportRequest{} = request) do - case operator_alert_address() do - {:ok, address} -> - deliver( - address, - "Support request updated by requester #{request.reference}", - """ - The requester added a message or reopened a support request. + def operator_alerts_enabled?, do: match?({:ok, _address}, operator_alert_address()) - Reference: #{request.reference} - Status: #{request.status} - Operator queue: #{WhoNeedHelpWeb.Endpoint.url()}/support/operations - - Sign in with an authorised moderator or administrator account to review it. - """ - ) - - {:error, reason} -> - {:ok, reason} - end - end - - defp deliver(recipient, subject, body) do + defp deliver(recipient, subject, body, kind) do from = Application.fetch_env!(:who_need_help, :mailer_from) new() @@ -106,7 +69,7 @@ defmodule WhoNeedHelp.Support.Notifier do |> maybe_reply_to() |> subject(subject) |> text_body(body) - |> Mailer.deliver() + |> EmailDelivery.deliver(kind) end defp maybe_reply_to(email) do diff --git a/lib/who_need_help/support/support_request.ex b/lib/who_need_help/support/support_request.ex index 27dd718..f98fb8b 100644 --- a/lib/who_need_help/support/support_request.ex +++ b/lib/who_need_help/support/support_request.ex @@ -2,6 +2,8 @@ defmodule WhoNeedHelp.Support.SupportRequest do use Ecto.Schema import Ecto.Changeset + alias WhoNeedHelp.EmailAddress + @primary_key {:id, :binary_id, autogenerate: true} @foreign_key_type :binary_id @@ -57,9 +59,7 @@ defmodule WhoNeedHelp.Support.SupportRequest do |> cast(attrs, [:kind, :contact_email, :subject, :details]) |> validate_required([:kind, :contact_email, :subject, :details, :reference]) |> update_change(:contact_email, &normalize_email/1) - |> validate_format(:contact_email, ~r/^[^@,;\s]+@[^@,;\s]+$/, - message: "must have the @ sign and no spaces" - ) + |> EmailAddress.validate(:contact_email) |> validate_length(:contact_email, max: 160) |> validate_length(:subject, min: 3, max: 160) |> validate_length(:details, min: 10, max: 5_000) diff --git a/lib/who_need_help_web/controllers/support_html/show.html.heex b/lib/who_need_help_web/controllers/support_html/show.html.heex index 82e4124..0c4038d 100644 --- a/lib/who_need_help_web/controllers/support_html/show.html.heex +++ b/lib/who_need_help_web/controllers/support_html/show.html.heex @@ -156,7 +156,7 @@

{gettext( - "Keep this status link private. A copy is sent to the contact email when email delivery is configured." + "Keep this status link private. Important status changes may also be sent to the verified contact email." )}

diff --git a/lib/who_need_help_web/live/notification_live.ex b/lib/who_need_help_web/live/notification_live.ex index cec3978..dd90702 100644 --- a/lib/who_need_help_web/live/notification_live.ex +++ b/lib/who_need_help_web/live/notification_live.ex @@ -455,17 +455,11 @@ defmodule WhoNeedHelpWeb.NotificationLive do type="checkbox" label={gettext("Acceptance, arrival, completion, and cancellation")} /> -
{gettext("Email")}
- <.input - field={@preference_form[:email_enabled]} - type="checkbox" - label={gettext("Allow email notifications")} - /> - <.input - field={@preference_form[:nearby_email_enabled]} - type="checkbox" - label={gettext("Nearby requests by email")} - /> +
+ {gettext( + "Nearby updates use the in-app inbox and optional push. Email digests are not enabled yet." + )} +
<.input field={@preference_form[:quiet_hours_enabled]} type="checkbox" @@ -508,9 +502,6 @@ defmodule WhoNeedHelpWeb.NotificationLive do {gettext("Push")} - - {gettext("Email")} - {gettext("Delivery channels")}

- {gettext("Choose at least one way to receive this nearby alert.")} + {gettext("Nearby alerts always appear in your inbox. Push is optional.")}

-
+
<.input field={@subscription_form[:push_enabled]} type="checkbox" label={gettext("Push notification")} /> - <.input - field={@subscription_form[:email_enabled]} - type="checkbox" - label={gettext("Email notification")} - /> +
<.button class="btn btn-primary btn-lg w-full">{gettext("Create nearby alert")} diff --git a/lib/who_need_help_web/telemetry.ex b/lib/who_need_help_web/telemetry.ex index 529b549..605466a 100644 --- a/lib/who_need_help_web/telemetry.ex +++ b/lib/who_need_help_web/telemetry.ex @@ -171,6 +171,13 @@ defmodule WhoNeedHelpWeb.Telemetry do measurement: :duration, description: "Single-email delivery attempts that raised an exception" ), + counter("who_need_help.email.by_kind.deliveries.total", + event_name: [:who_need_help, :email, :delivery], + measurement: :duration, + tags: [:kind, :status], + description: + "Email delivery attempts by fixed purpose and outcome without personal labels" + ), sum("who_need_help.oban.job.duration.microseconds.total", event_name: [:oban, :job, :stop], measurement: fn measurements -> diff --git a/priv/repo/migrations/20260812120611_allow_inbox_only_nearby_subscriptions.exs b/priv/repo/migrations/20260812120611_allow_inbox_only_nearby_subscriptions.exs new file mode 100644 index 0000000..b943c69 --- /dev/null +++ b/priv/repo/migrations/20260812120611_allow_inbox_only_nearby_subscriptions.exs @@ -0,0 +1,18 @@ +defmodule WhoNeedHelp.Repo.Migrations.AllowInboxOnlyNearbySubscriptions do + use Ecto.Migration + + def up do + drop constraint( + :nearby_subscriptions, + :nearby_subscriptions_delivery_channel_required + ) + end + + def down do + create constraint( + :nearby_subscriptions, + :nearby_subscriptions_delivery_channel_required, + check: "push_enabled OR email_enabled" + ) + end +end diff --git a/scripts/clean-deploy-verify.sh b/scripts/clean-deploy-verify.sh index 6207ba4..968fc1b 100755 --- a/scripts/clean-deploy-verify.sh +++ b/scripts/clean-deploy-verify.sh @@ -206,6 +206,7 @@ MIGRATE_POOL_SIZE=2 COMBINED_POOL_SIZE=4 OBAN_MAINTENANCE_CONCURRENCY=2 OBAN_PUSH_CONCURRENCY=1 +OBAN_MAIL_CONCURRENCY=1 WEB_REPLICAS=2 WORKER_REPLICAS=2 ERLANG_PORT_LIMIT=65536 diff --git a/scripts/ensure-local-e2e-env.sh b/scripts/ensure-local-e2e-env.sh index 1375c55..d5e0e61 100755 --- a/scripts/ensure-local-e2e-env.sh +++ b/scripts/ensure-local-e2e-env.sh @@ -98,6 +98,7 @@ MIGRATE_POOL_SIZE=2 COMBINED_POOL_SIZE=4 OBAN_MAINTENANCE_CONCURRENCY=2 OBAN_PUSH_CONCURRENCY=1 +OBAN_MAIL_CONCURRENCY=1 WEB_REPLICAS=2 WORKER_REPLICAS=2 ERLANG_PORT_LIMIT=65536 diff --git a/scripts/ensure-local-load-env.sh b/scripts/ensure-local-load-env.sh index 66126d5..7bf9968 100755 --- a/scripts/ensure-local-load-env.sh +++ b/scripts/ensure-local-load-env.sh @@ -59,6 +59,7 @@ GOOGLE_OAUTH_HTTP_RECEIVE_TIMEOUT_MS needs_fixture_password=true needs_oban_maintenance_concurrency=true needs_oban_push_concurrency=true + needs_oban_mail_concurrency=true needs_resilience_timeout=true needs_resilience_interval=true needs_resilience_request_timeout=true @@ -88,6 +89,8 @@ GOOGLE_OAUTH_HTTP_RECEIVE_TIMEOUT_MS needs_oban_maintenance_concurrency=false grep -q '^OBAN_PUSH_CONCURRENCY=' "$ENV_FILE" && needs_oban_push_concurrency=false + grep -q '^OBAN_MAIL_CONCURRENCY=' "$ENV_FILE" && + needs_oban_mail_concurrency=false grep -q '^LOAD_RESILIENCE_RECOVERY_TIMEOUT_SECONDS=' "$ENV_FILE" && needs_resilience_timeout=false grep -q '^LOAD_RESILIENCE_PROBE_INTERVAL_SECONDS=' "$ENV_FILE" && @@ -137,6 +140,7 @@ GOOGLE_OAUTH_HTTP_RECEIVE_TIMEOUT_MS [ "$needs_fixture_password" = false ] && [ "$needs_oban_maintenance_concurrency" = false ] && [ "$needs_oban_push_concurrency" = false ] && + [ "$needs_oban_mail_concurrency" = false ] && [ "$needs_resilience_timeout" = false ] && [ "$needs_resilience_interval" = false ] && [ "$needs_resilience_request_timeout" = false ] && @@ -209,7 +213,8 @@ GOOGLE_OAUTH_HTTP_RECEIVE_TIMEOUT_MS fi if [ "$needs_oban_maintenance_concurrency" = true ] || - [ "$needs_oban_push_concurrency" = true ]; then + [ "$needs_oban_push_concurrency" = true ] || + [ "$needs_oban_mail_concurrency" = true ]; then printf '\n# Added by the worker-queue configuration upgrade.\n' fi @@ -221,6 +226,10 @@ GOOGLE_OAUTH_HTTP_RECEIVE_TIMEOUT_MS printf 'OBAN_PUSH_CONCURRENCY=1\n' fi + if [ "$needs_oban_mail_concurrency" = true ]; then + printf 'OBAN_MAIL_CONCURRENCY=1\n' + fi + if [ "$needs_resilience_timeout" = true ] || [ "$needs_resilience_interval" = true ]; then printf '\n# Added by the local resilience-profile upgrade.\n' @@ -347,7 +356,8 @@ GOOGLE_OAUTH_HTTP_RECEIVE_TIMEOUT_MS unset load_fixture_password observability_grafana_admin_password \ backup_minio_root_user backup_minio_root_password backup_restic_password \ needs_fixture_password needs_oban_maintenance_concurrency \ - needs_oban_push_concurrency needs_resilience_timeout \ + needs_oban_push_concurrency needs_oban_mail_concurrency \ + needs_resilience_timeout \ needs_resilience_interval needs_resilience_request_timeout \ needs_traefik_retry_attempts needs_traefik_api_insecure \ needs_observability_prometheus_port \ diff --git a/scripts/oban-burst-run.sh b/scripts/oban-burst-run.sh index 96b475f..ada384e 100755 --- a/scripts/oban-burst-run.sh +++ b/scripts/oban-burst-run.sh @@ -186,6 +186,7 @@ snapshot_domain_counts >"$output_dir/domain-counts-before.json" printf 'maintenance_concurrency_per_worker=%s\n' \ "${OBAN_MAINTENANCE_CONCURRENCY:-2}" printf 'push_concurrency_per_worker=%s\n' "${OBAN_PUSH_CONCURRENCY:-1}" + printf 'mail_concurrency_per_worker=%s\n' "${OBAN_MAIL_CONCURRENCY:-1}" } >"$output_dir/inputs.txt" for container in "${worker_containers[@]}"; do diff --git a/scripts/quality.sh b/scripts/quality.sh index e66fff6..f71a4cf 100755 --- a/scripts/quality.sh +++ b/scripts/quality.sh @@ -1356,6 +1356,7 @@ docker compose --project-name who_need_help_edge \ and $root.services.migrate.environment.POOL_SIZE == "2" and $root.services.worker.environment.OBAN_MAINTENANCE_CONCURRENCY == "2" and $root.services.worker.environment.OBAN_PUSH_CONCURRENCY == "1" + and $root.services.worker.environment.OBAN_MAIL_CONCURRENCY == "1" and $root.services.web.deploy.replicas == 2 and $root.services.worker.deploy.replicas == 2 and ($root.services.proxy.networks | keys | sort) == ["docker-api", "edge", "ingress"] diff --git a/test/who_need_help/email_address_test.exs b/test/who_need_help/email_address_test.exs new file mode 100644 index 0000000..157bd26 --- /dev/null +++ b/test/who_need_help/email_address_test.exs @@ -0,0 +1,19 @@ +defmodule WhoNeedHelp.EmailAddressTest do + use ExUnit.Case, async: true + + alias WhoNeedHelp.EmailAddress + + test "accepts ordinary mailbox addresses used by transactional email" do + assert EmailAddress.valid?("person@example.com") + assert EmailAddress.valid?("person+alerts@example.co.uk") + end + + test "rejects addresses that the SMTP adapter cannot safely parse" do + refute EmailAddress.valid?("two..dots@example.com") + refute EmailAddress.valid?("person@example") + refute EmailAddress.valid?("person@-example.com") + refute EmailAddress.valid?("person@example..com") + refute EmailAddress.valid?("person@exam ple.com") + refute EmailAddress.valid?("first@example.com,second@example.com") + end +end diff --git a/test/who_need_help/notifications_test.exs b/test/who_need_help/notifications_test.exs index fc5c4c3..6a47e9f 100644 --- a/test/who_need_help/notifications_test.exs +++ b/test/who_need_help/notifications_test.exs @@ -162,10 +162,10 @@ defmodule WhoNeedHelp.NotificationsTest do "email_enabled" => "false" }) - assert "select push, email, or both" in errors_on(no_channel_changeset).push_enabled + refute Map.has_key?(errors_on(no_channel_changeset), :push_enabled) end - test "nearby email is durable, generic, and enabled when any matching alert opts in", context do + test "nearby alerts use the inbox and push without immediate email", context do assert_email_sent() assert_email_sent() @@ -225,7 +225,6 @@ defmodule WhoNeedHelp.NotificationsTest do kind: :nearby_request ) - assert notification.data["email_enabled"] assert notification.data["push_enabled"] refute inspect(notification) =~ "Private home label" refute inspect(notification) =~ "Do not send this label" @@ -233,23 +232,11 @@ defmodule WhoNeedHelp.NotificationsTest do assert :ok = perform_job(NotificationDispatchWorker, %{"notification_id" => notification.id}) - assert_enqueued( - worker: NotificationEmailWorker, - queue: :push, - args: %{"notification_id" => notification.id} - ) + assert all_enqueued(worker: NotificationEmailWorker) == [] + refute_receive {:email, _nearby_email}, 50 - assert :ok = + assert {:cancel, :immediate_nearby_email_retired} = perform_job(NotificationEmailWorker, %{"notification_id" => notification.id}) - - assert_email_sent(fn email -> - assert email.to == [{"", context.helper.email}] - assert email.subject == "New help request nearby" - assert email.text_body =~ "/requests/#{request.id}" - refute email.text_body =~ "Private home label" - refute email.text_body =~ "Do not send this label" - true - end) end test "device registration is idempotent and follows an authenticated shared installation", diff --git a/test/who_need_help/support_and_content_removal_test.exs b/test/who_need_help/support_and_content_removal_test.exs index adc6485..684c281 100644 --- a/test/who_need_help/support_and_content_removal_test.exs +++ b/test/who_need_help/support_and_content_removal_test.exs @@ -1,11 +1,14 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do use WhoNeedHelp.DataCase, async: false + use Oban.Testing, repo: WhoNeedHelp.Repo import WhoNeedHelp.AccountsFixtures import Swoosh.TestAssertions alias WhoNeedHelp.{Catalog, ContentRemoval, Help, Repo, Support} alias WhoNeedHelp.ContentRemoval.Notice + alias WhoNeedHelp.Mail.{SupportOperatorAlertWorker, SupportUpdateWorker} + alias WhoNeedHelp.Notifications.Notification alias WhoNeedHelp.Support.{ConversationMessage, StatusEvent, SupportRequest} alias WhoNeedHelp.Trust.AuditEvent @@ -77,11 +80,7 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do event.target_id == ^request.id ) - assert_email_sent(fn received -> - received.to == [{"", request.contact_email}] and - received.subject == "Who Need Help support request #{request.reference}" and - received.text_body =~ "/support/cases/#{request.id}?token=" - end) + refute_receive {:email, _duplicate_received_email}, 50 end test "operator alert is sent only after public contact verification" do @@ -117,11 +116,14 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do assert verified.status == :open - assert_email_sent(fn email -> - email.to == [{"", "appeal@example.com"}] and - email.subject == "Who Need Help support request #{request.reference}" and - email.text_body =~ "/support/cases/#{request.id}?token=" - end) + assert_enqueued( + worker: SupportOperatorAlertWorker, + queue: :mail, + args: %{"request_id" => request.id} + ) + + assert {:ok, _metadata} = + perform_job(SupportOperatorAlertWorker, %{"request_id" => request.id}) assert_email_sent(fn email -> email.to == [{"", "support@example.com"}] and email.subject =~ request.reference and @@ -159,9 +161,7 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do assert request.contact_verified_at - assert_email_sent(fn email -> - email.to == [{"", user.email}] and email.subject =~ request.reference - end) + refute_enqueued(worker: SupportOperatorAlertWorker) refute_receive {:email, _operator_alert}, 50 @@ -237,7 +237,6 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do assert request.contact_email == user.email assert request.contact_verified_at - assert_email_sent() moderator = staff_user_fixture([:support, :legal]) @@ -256,7 +255,7 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do }) assert resolved.status == :resolved - assert resolved.response_sent_at + refute resolved.response_sent_at assert [%ConversationMessage{sender_role: :staff, body: response}] = resolved.conversation_messages @@ -264,9 +263,76 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do assert response == "The export is ready for the verified account owner." assert Enum.map(resolved.status_events, & &1.to_status) == [:open, :resolved] + assert_enqueued( + worker: SupportUpdateWorker, + queue: :mail, + args: %{"request_id" => request.id} + ) + + assert :ok = perform_job(SupportUpdateWorker, %{"request_id" => request.id}) + assert Repo.reload!(resolved).response_sent_at + assert_email_sent(fn email -> - email.subject =~ "support update" and email.text_body =~ "export is ready" + email.subject =~ "support update" and + email.text_body =~ "/support/cases/#{request.id}" and + not String.contains?(email.text_body, "export is ready") end) + + assert %Notification{kind: :support_update, user_id: user_id} = + Repo.get_by!(Notification, user_id: user.id, kind: :support_update) + + assert user_id == user.id + end + + test "support chat emails the first reply and later public status changes, not every message" do + requester = user_fixture() + assert_email_sent() + moderator = staff_user_fixture([:support]) + assert_email_sent() + + assert {:ok, request} = + Support.create_request(user_scope_fixture(requester), %{ + "kind" => "technical_issue", + "subject" => "Keep support email volume bounded", + "details" => "Use the private inbox for an ongoing support conversation." + }) + + assert {:ok, reviewing} = + Support.moderate(user_scope_fixture(moderator), request.id, %{ + "status" => "reviewing", + "response" => "We started reviewing this request." + }) + + assert reviewing.status == :reviewing + assert %{success: 1} = Oban.drain_queue(queue: :mail) + assert_email_sent() + + assert {:ok, still_reviewing} = + Support.moderate(user_scope_fixture(moderator), request.id, %{ + "status" => "reviewing", + "response" => "This is another message in the same active conversation." + }) + + assert still_reviewing.status == :reviewing + assert all_enqueued(worker: SupportUpdateWorker) == [] + refute_receive {:email, _chat_message_email}, 50 + + assert {:ok, waiting} = + Support.moderate(user_scope_fixture(moderator), request.id, %{ + "status" => "waiting_for_requester", + "response" => "Please add the missing diagnostic information." + }) + + assert waiting.status == :waiting_for_requester + + assert_enqueued( + worker: SupportUpdateWorker, + queue: :mail, + args: %{"request_id" => request.id} + ) + + assert %{success: 1} = Oban.drain_queue(queue: :mail) + assert_email_sent() end test "support and legal cases can be assigned only to active staff for the matching queue" do @@ -352,7 +418,15 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do "I need help understanding which nearby notifications are currently enabled." }) - assert_email_sent() + assert_enqueued( + worker: SupportOperatorAlertWorker, + queue: :mail, + args: %{"request_id" => request.id} + ) + + assert {:ok, _metadata} = + perform_job(SupportOperatorAlertWorker, %{"request_id" => request.id}) + assert_email_sent() assert {:ok, resolved} = @@ -362,8 +436,11 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do }) assert resolved.status == :resolved + assert :ok = perform_job(SupportUpdateWorker, %{"request_id" => request.id}) assert_email_sent() + operator_job_count = length(all_enqueued(worker: SupportOperatorAlertWorker)) + assert {:ok, reopened} = Support.add_requester_message(scope, request.id, nil, %{ "body" => "I tried that, but the switch still appears disabled." @@ -378,11 +455,8 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do assert Enum.map(reopened.status_events, & &1.to_status) == [:open, :resolved, :open] - assert_email_sent(fn email -> - email.to == [{"", "support@example.com"}] and - email.subject =~ "updated by requester" and - not String.contains?(email.text_body, "switch still appears disabled") - end) + assert length(all_enqueued(worker: SupportOperatorAlertWorker)) == operator_job_count + refute_receive {:email, _requester_message_email}, 50 end test "only the requester or a valid private token can reply or reopen a support case" do diff --git a/test/who_need_help_web/controllers/metrics_controller_test.exs b/test/who_need_help_web/controllers/metrics_controller_test.exs index 25639e6..a367507 100644 --- a/test/who_need_help_web/controllers/metrics_controller_test.exs +++ b/test/who_need_help_web/controllers/metrics_controller_test.exs @@ -51,7 +51,8 @@ defmodule WhoNeedHelpWeb.MetricsControllerTest do |> Enum.filter(fn metric -> metric.name in [ [:who_need_help, :email, :deliveries, :total], - [:who_need_help, :email, :delivery, :exceptions, :total] + [:who_need_help, :email, :delivery, :exceptions, :total], + [:who_need_help, :email, :by_kind, :deliveries, :total] ] end) @@ -77,14 +78,26 @@ defmodule WhoNeedHelpWeb.MetricsControllerTest do %{mailer: WhoNeedHelp.Mailer, kind: :error, reason: :timeout} ) + :telemetry.execute( + [:who_need_help, :email, :delivery], + %{duration: 100}, + %{kind: "support_update", status: "ok"} + ) + body = TelemetryMetricsPrometheus.Core.scrape(reporter_name) assert body =~ ~s(who_need_help_email_deliveries_total{status="ok"} 1) assert body =~ ~s(who_need_help_email_deliveries_total{status="error"} 1) assert body =~ "who_need_help_email_delivery_exceptions_total 1" + + assert body =~ + ~s(who_need_help_email_by_kind_deliveries_total{kind="support_update",status="ok"} 1) + refute body =~ "accepted" refute body =~ "rejected" refute body =~ "recipient" + refute body =~ "subject" + refute body =~ "email_address" end test "database execution metric tolerates events without query_time" do diff --git a/test/who_need_help_web/live/mutual_aid_live_test.exs b/test/who_need_help_web/live/mutual_aid_live_test.exs index ff75340..44dc80f 100644 --- a/test/who_need_help_web/live/mutual_aid_live_test.exs +++ b/test/who_need_help_web/live/mutual_aid_live_test.exs @@ -122,8 +122,6 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do "nearby_push_enabled" => "true", "message_push_enabled" => "true", "lifecycle_push_enabled" => "true", - "email_enabled" => "true", - "nearby_email_enabled" => "true", "quiet_hours_enabled" => "true", "quiet_start" => "22:00", "quiet_end" => "07:00", @@ -155,7 +153,7 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do "available_from" => "", "available_until" => "", "push_enabled" => "true", - "email_enabled" => "true" + "email_enabled" => "false" } }) |> render_submit() @@ -165,7 +163,7 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do assert html =~ "Medicine near home" assert html =~ "Private home area ยท 3 km" assert html =~ "Push" - assert html =~ "Email" + refute html =~ "Email" assert has_element?(view, "button[phx-click='toggle-subscription']", "Pause") end