From 32b8b88ce19c67e2fe13018fda3e1ea5b0f410cb Mon Sep 17 00:00:00 2001 From: d3vyce <44915747+d3vyce@users.noreply.github.com> Date: Sat, 14 Nov 2026 07:44:51 -0500 Subject: [PATCH 1/2] feat: notify in the app with a live inbox, on-screen messages and a switch per kind and channel Signed-off-by: d3vyce <44915747+d3vyce@users.noreply.github.com> --- ...c05afc970e324daa4712ca99c379c216eb76f.json | 53 ++++ ...1d0cad31c52cbd5925cf22ff167b3bac15e65.json | 15 ++ ...57e53a1030ae76443f94449339caf43b847cf.json | 32 +++ ...396a3d3d6da25a6029c9344307c30f9aa8a39.json | 156 ++++++++++++ ...10be25c9f0925ee95cccd791ff907fd260ac1.json | 52 ++++ ...8c33966d7ce05e3164e81734d5ec270b462f0.json | 14 ++ ...b05ce1bcdae79f9fe98cf97a46a8efd91a5ea.json | 66 +++++ ...c8f95a253c32f1f07c811b22d35b01c4c48e5.json | 23 ++ ...9c6712cfb751ea7f28c424b814f949b3d55aa.json | 29 +++ ...24c062e9e346a19854ec29ce9ffec1b9f056f.json | 24 ++ README.md | 2 +- .../20261114000000_in_app_notifications.sql | 59 +++++ crates/forgust-db/src/ci.rs | 36 +++ crates/forgust-db/src/merge_requests.rs | 10 +- crates/forgust-db/src/notifications.rs | 222 ++++++++++++++++ crates/forgust-server/src/api/mod.rs | 4 + crates/forgust-server/src/api/mr.rs | 15 +- .../forgust-server/src/api/notifications.rs | 97 ++++++- crates/forgust-server/src/api/openapi.rs | 2 +- crates/forgust-server/src/ci/pipeline.rs | 8 +- crates/forgust-server/src/events.rs | 26 +- crates/forgust-server/src/jobs.rs | 2 + crates/forgust-server/src/services/mr.rs | 6 +- crates/forgust-server/src/services/notify.rs | 237 ++++++++++++++++-- crates/forgust-server/src/services/people.rs | 14 +- crates/forgust-server/src/services/refs.rs | 19 +- .../forgust-server/src/services/security.rs | 12 +- crates/forgust-server/tests/m4_ci.rs | 60 +++++ crates/forgust-server/tests/m7_v1.rs | 101 ++++++++ web/e2e/everyday.spec.ts | 38 ++- web/openapi.json | 231 +++++++++++++++++ web/src/api/schema.d.ts | 157 ++++++++++++ web/src/components/Inbox.tsx | 137 ++++++++++ web/src/components/Notifications.tsx | 124 +++++++-- web/src/prefetch.ts | 4 + web/src/shell/Shell.tsx | 2 + web/src/shell/live.ts | 42 +++- web/src/shell/toast.tsx | 50 +++- web/src/styles.css | 137 +++++++++- 39 files changed, 2214 insertions(+), 104 deletions(-) create mode 100644 .sqlx/query-21fef664353bd76e856c4bd8bdcc05afc970e324daa4712ca99c379c216eb76f.json create mode 100644 .sqlx/query-2aa7a72e063321341ee059745321d0cad31c52cbd5925cf22ff167b3bac15e65.json create mode 100644 .sqlx/query-2f22de18355d6f3db2436ed07ed57e53a1030ae76443f94449339caf43b847cf.json create mode 100644 .sqlx/query-497b295c32fc31132353729aa85396a3d3d6da25a6029c9344307c30f9aa8a39.json create mode 100644 .sqlx/query-631d9a867ec44862af8eb107ba710be25c9f0925ee95cccd791ff907fd260ac1.json create mode 100644 .sqlx/query-86330ae0b29e236b96191cdfb8f8c33966d7ce05e3164e81734d5ec270b462f0.json create mode 100644 .sqlx/query-9a9498510076557458de141d928b05ce1bcdae79f9fe98cf97a46a8efd91a5ea.json create mode 100644 .sqlx/query-b4e8d6b0a03ab07c0d58503c999c8f95a253c32f1f07c811b22d35b01c4c48e5.json create mode 100644 .sqlx/query-d3e2f616dc1ed411161b517fbf49c6712cfb751ea7f28c424b814f949b3d55aa.json create mode 100644 .sqlx/query-e89e6471065c3ea18732b9116f324c062e9e346a19854ec29ce9ffec1b9f056f.json create mode 100644 crates/forgust-db/migrations/20261114000000_in_app_notifications.sql create mode 100644 web/src/components/Inbox.tsx diff --git a/.sqlx/query-21fef664353bd76e856c4bd8bdcc05afc970e324daa4712ca99c379c216eb76f.json b/.sqlx/query-21fef664353bd76e856c4bd8bdcc05afc970e324daa4712ca99c379c216eb76f.json new file mode 100644 index 0000000..8148da6 --- /dev/null +++ b/.sqlx/query-21fef664353bd76e856c4bd8bdcc05afc970e324daa4712ca99c379c216eb76f.json @@ -0,0 +1,53 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT user_id, channel, enabled FROM notification_channels WHERE user_id = ANY($1) AND kind = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "user_id", + "type_info": "Int8", + "origin": { + "Table": { + "table": "notification_channels", + "name": "user_id" + } + } + }, + { + "ordinal": 1, + "name": "channel", + "type_info": "Text", + "origin": { + "Table": { + "table": "notification_channels", + "name": "channel" + } + } + }, + { + "ordinal": 2, + "name": "enabled", + "type_info": "Bool", + "origin": { + "Table": { + "table": "notification_channels", + "name": "enabled" + } + } + } + ], + "parameters": { + "Left": [ + "Int8Array", + "Text" + ] + }, + "nullable": [ + false, + false, + false + ] + }, + "hash": "21fef664353bd76e856c4bd8bdcc05afc970e324daa4712ca99c379c216eb76f" +} diff --git a/.sqlx/query-2aa7a72e063321341ee059745321d0cad31c52cbd5925cf22ff167b3bac15e65.json b/.sqlx/query-2aa7a72e063321341ee059745321d0cad31c52cbd5925cf22ff167b3bac15e65.json new file mode 100644 index 0000000..29447d3 --- /dev/null +++ b/.sqlx/query-2aa7a72e063321341ee059745321d0cad31c52cbd5925cf22ff167b3bac15e65.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE notifications SET read_at = coalesce(read_at, now())\n WHERE user_id = $1 AND (($2::bigint IS NULL AND read_at IS NULL) OR id = $2)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + "Int8" + ] + }, + "nullable": [] + }, + "hash": "2aa7a72e063321341ee059745321d0cad31c52cbd5925cf22ff167b3bac15e65" +} diff --git a/.sqlx/query-2f22de18355d6f3db2436ed07ed57e53a1030ae76443f94449339caf43b847cf.json b/.sqlx/query-2f22de18355d6f3db2436ed07ed57e53a1030ae76443f94449339caf43b847cf.json new file mode 100644 index 0000000..8e99009 --- /dev/null +++ b/.sqlx/query-2f22de18355d6f3db2436ed07ed57e53a1030ae76443f94449339caf43b847cf.json @@ -0,0 +1,32 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT status FROM ci_pipelines\n WHERE project_id = $1 AND ref = $2 AND tag = $3 AND merge_request_id IS NOT DISTINCT FROM $4 AND id < $5\n AND status IN ('success', 'failed') AND source NOT IN ('parent_pipeline', 'renovate', 'security')\n ORDER BY id DESC LIMIT 1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "status", + "type_info": "Text", + "origin": { + "Table": { + "table": "ci_pipelines", + "name": "status" + } + } + } + ], + "parameters": { + "Left": [ + "Int8", + "Text", + "Bool", + "Int8", + "Int8" + ] + }, + "nullable": [ + false + ] + }, + "hash": "2f22de18355d6f3db2436ed07ed57e53a1030ae76443f94449339caf43b847cf" +} diff --git a/.sqlx/query-497b295c32fc31132353729aa85396a3d3d6da25a6029c9344307c30f9aa8a39.json b/.sqlx/query-497b295c32fc31132353729aa85396a3d3d6da25a6029c9344307c30f9aa8a39.json new file mode 100644 index 0000000..eb7fc55 --- /dev/null +++ b/.sqlx/query-497b295c32fc31132353729aa85396a3d3d6da25a6029c9344307c30f9aa8a39.json @@ -0,0 +1,156 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT n.id AS \"id!\", n.kind AS \"kind!: String\", n.project_id AS \"project_id!\", r.path AS project_path,\n n.target_type AS \"target_type!\", n.target_iid, n.title AS \"title!\", n.summary AS \"summary!\",\n u.username AS \"actor_username?\", u.name AS \"actor_name?\", n.read_at IS NOT NULL AS \"read!\", n.created_at AS \"created_at!\"\n FROM listed_notifications n\n JOIN routes r ON r.source_type = 'project' AND r.source_id = n.project_id\n LEFT JOIN users u ON u.id = n.actor_id\n WHERE n.user_id = $1\n ORDER BY n.created_at DESC, n.id DESC LIMIT $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id!", + "type_info": "Int8", + "origin": { + "Table": { + "table": "listed_notifications", + "name": "id" + } + } + }, + { + "ordinal": 1, + "name": "kind!: String", + "type_info": "Text", + "origin": { + "Table": { + "table": "listed_notifications", + "name": "kind" + } + } + }, + { + "ordinal": 2, + "name": "project_id!", + "type_info": "Int8", + "origin": { + "Table": { + "table": "listed_notifications", + "name": "project_id" + } + } + }, + { + "ordinal": 3, + "name": "project_path", + "type_info": "Text", + "origin": { + "Table": { + "table": "routes", + "name": "path" + } + } + }, + { + "ordinal": 4, + "name": "target_type!", + "type_info": "Text", + "origin": { + "Table": { + "table": "listed_notifications", + "name": "target_type" + } + } + }, + { + "ordinal": 5, + "name": "target_iid", + "type_info": "Int8", + "origin": { + "Table": { + "table": "listed_notifications", + "name": "target_iid" + } + } + }, + { + "ordinal": 6, + "name": "title!", + "type_info": "Text", + "origin": { + "Table": { + "table": "listed_notifications", + "name": "title" + } + } + }, + { + "ordinal": 7, + "name": "summary!", + "type_info": "Text", + "origin": { + "Table": { + "table": "listed_notifications", + "name": "summary" + } + } + }, + { + "ordinal": 8, + "name": "actor_username?", + "type_info": "Text", + "origin": { + "Table": { + "table": "users", + "name": "username" + } + } + }, + { + "ordinal": 9, + "name": "actor_name?", + "type_info": "Text", + "origin": { + "Table": { + "table": "users", + "name": "name" + } + } + }, + { + "ordinal": 10, + "name": "read!", + "type_info": "Bool", + "origin": "Expression" + }, + { + "ordinal": 11, + "name": "created_at!", + "type_info": "Timestamptz", + "origin": { + "Table": { + "table": "listed_notifications", + "name": "created_at" + } + } + } + ], + "parameters": { + "Left": [ + "Int8", + "Int8" + ] + }, + "nullable": [ + true, + true, + true, + false, + true, + true, + true, + true, + false, + false, + null, + true + ] + }, + "hash": "497b295c32fc31132353729aa85396a3d3d6da25a6029c9344307c30f9aa8a39" +} diff --git a/.sqlx/query-631d9a867ec44862af8eb107ba710be25c9f0925ee95cccd791ff907fd260ac1.json b/.sqlx/query-631d9a867ec44862af8eb107ba710be25c9f0925ee95cccd791ff907fd260ac1.json new file mode 100644 index 0000000..d451adb --- /dev/null +++ b/.sqlx/query-631d9a867ec44862af8eb107ba710be25c9f0925ee95cccd791ff907fd260ac1.json @@ -0,0 +1,52 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT kind AS \"kind!: String\", channel, enabled FROM notification_channels WHERE user_id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "kind!: String", + "type_info": "Text", + "origin": { + "Table": { + "table": "notification_channels", + "name": "kind" + } + } + }, + { + "ordinal": 1, + "name": "channel", + "type_info": "Text", + "origin": { + "Table": { + "table": "notification_channels", + "name": "channel" + } + } + }, + { + "ordinal": 2, + "name": "enabled", + "type_info": "Bool", + "origin": { + "Table": { + "table": "notification_channels", + "name": "enabled" + } + } + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + false, + false, + false + ] + }, + "hash": "631d9a867ec44862af8eb107ba710be25c9f0925ee95cccd791ff907fd260ac1" +} diff --git a/.sqlx/query-86330ae0b29e236b96191cdfb8f8c33966d7ce05e3164e81734d5ec270b462f0.json b/.sqlx/query-86330ae0b29e236b96191cdfb8f8c33966d7ce05e3164e81734d5ec270b462f0.json new file mode 100644 index 0000000..2f4d3b4 --- /dev/null +++ b/.sqlx/query-86330ae0b29e236b96191cdfb8f8c33966d7ce05e3164e81734d5ec270b462f0.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM notifications WHERE read_at < now() - make_interval(days => $1) OR created_at < now() - make_interval(days => 3 * $1)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int4" + ] + }, + "nullable": [] + }, + "hash": "86330ae0b29e236b96191cdfb8f8c33966d7ce05e3164e81734d5ec270b462f0" +} diff --git a/.sqlx/query-9a9498510076557458de141d928b05ce1bcdae79f9fe98cf97a46a8efd91a5ea.json b/.sqlx/query-9a9498510076557458de141d928b05ce1bcdae79f9fe98cf97a46a8efd91a5ea.json new file mode 100644 index 0000000..b07f0a5 --- /dev/null +++ b/.sqlx/query-9a9498510076557458de141d928b05ce1bcdae79f9fe98cf97a46a8efd91a5ea.json @@ -0,0 +1,66 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO notifications (user_id, project_id, kind, target_type, target_iid, actor_id, title, summary)\n SELECT u, $2, $3, $4, $5, $6, $7, $8 FROM unnest($1::bigint[]) u\n RETURNING user_id, id, created_at", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "user_id", + "type_info": "Int8", + "origin": { + "Table": { + "table": "notifications", + "name": "user_id" + } + } + }, + { + "ordinal": 1, + "name": "id", + "type_info": "Int8", + "origin": { + "Table": { + "table": "notifications", + "name": "id" + } + } + }, + { + "ordinal": 2, + "name": "created_at", + "type_info": "Timestamptz", + "origin": { + "Table": { + "table": "notifications", + "name": "created_at" + } + } + } + ], + "parameters": { + "Left": [ + "Int8Array", + "Int8", + { + "Custom": { + "name": "notification_kind", + "kind": { + "Domain": "Text" + } + } + }, + "Text", + "Int8", + "Int8", + "Text", + "Text" + ] + }, + "nullable": [ + false, + false, + false + ] + }, + "hash": "9a9498510076557458de141d928b05ce1bcdae79f9fe98cf97a46a8efd91a5ea" +} diff --git a/.sqlx/query-b4e8d6b0a03ab07c0d58503c999c8f95a253c32f1f07c811b22d35b01c4c48e5.json b/.sqlx/query-b4e8d6b0a03ab07c0d58503c999c8f95a253c32f1f07c811b22d35b01c4c48e5.json new file mode 100644 index 0000000..336620c --- /dev/null +++ b/.sqlx/query-b4e8d6b0a03ab07c0d58503c999c8f95a253c32f1f07c811b22d35b01c4c48e5.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT count(*) AS \"n!\" FROM (SELECT 1 FROM listed_notifications WHERE user_id = $1 AND read_at IS NULL LIMIT 100) unread", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "n!", + "type_info": "Int8", + "origin": "Expression" + } + ], + "parameters": { + "Left": [ + "Int8" + ] + }, + "nullable": [ + null + ] + }, + "hash": "b4e8d6b0a03ab07c0d58503c999c8f95a253c32f1f07c811b22d35b01c4c48e5" +} diff --git a/.sqlx/query-d3e2f616dc1ed411161b517fbf49c6712cfb751ea7f28c424b814f949b3d55aa.json b/.sqlx/query-d3e2f616dc1ed411161b517fbf49c6712cfb751ea7f28c424b814f949b3d55aa.json new file mode 100644 index 0000000..c164224 --- /dev/null +++ b/.sqlx/query-d3e2f616dc1ed411161b517fbf49c6712cfb751ea7f28c424b814f949b3d55aa.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE ci_pipelines p SET notified_status = $2\n FROM (SELECT id, notified_status FROM ci_pipelines WHERE id = $1 FOR UPDATE) before\n WHERE p.id = before.id\n RETURNING before.notified_status", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "notified_status", + "type_info": "Text", + "origin": { + "Table": { + "table": "ci_pipelines", + "name": "notified_status" + } + } + } + ], + "parameters": { + "Left": [ + "Int8", + "Text" + ] + }, + "nullable": [ + true + ] + }, + "hash": "d3e2f616dc1ed411161b517fbf49c6712cfb751ea7f28c424b814f949b3d55aa" +} diff --git a/.sqlx/query-e89e6471065c3ea18732b9116f324c062e9e346a19854ec29ce9ffec1b9f056f.json b/.sqlx/query-e89e6471065c3ea18732b9116f324c062e9e346a19854ec29ce9ffec1b9f056f.json new file mode 100644 index 0000000..f296615 --- /dev/null +++ b/.sqlx/query-e89e6471065c3ea18732b9116f324c062e9e346a19854ec29ce9ffec1b9f056f.json @@ -0,0 +1,24 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO notification_channels (user_id, kind, channel, enabled) VALUES ($1, $2, $3, $4)\n ON CONFLICT (user_id, kind, channel) DO UPDATE SET enabled = $4, updated_at = now()", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + { + "Custom": { + "name": "notification_kind", + "kind": { + "Domain": "Text" + } + } + }, + "Text", + "Bool" + ] + }, + "nullable": [] + }, + "hash": "e89e6471065c3ea18732b9116f324c062e9e346a19854ec29ce9ffec1b9f056f" +} diff --git a/README.md b/README.md index 9eac125..1844431 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ volumes: { forgust-data: {}, db-data: {} } ### 🗂️ Planning - Issues with labels, milestones, due dates, assignees and templates - Mentions, `#12` and `!45` references, to-dos and a "Your work" inbox -- Emoji reactions, watching projects, per-project notification levels +- Emoji reactions, watching projects, per-project notification levels, an inbox in the app (live), and a switch per kind of notification for the app and for email ### ⚙️ CI/CD - Pipelines with stages and `needs` graphs, live logs, artifacts, JUnit test reports diff --git a/crates/forgust-db/migrations/20261114000000_in_app_notifications.sql b/crates/forgust-db/migrations/20261114000000_in_app_notifications.sql new file mode 100644 index 0000000..37e4ebd --- /dev/null +++ b/crates/forgust-db/migrations/20261114000000_in_app_notifications.sql @@ -0,0 +1,59 @@ +-- In-app notifications: an inbox per person, and a switch per kind and channel. + +-- What a notification is about (`forgust_db::notifications::Kind`). +CREATE DOMAIN notification_kind AS text + CHECK (VALUE IN ('mention', 'assignment', 'activity', 'review', 'pipeline_failed', 'pipeline_fixed', 'security')); + +-- What reached someone in the app (the bell). The level (watch, participating, +-- mention, disabled) decides who hears about something; the channels below decide how. +CREATE TABLE notifications ( + id bigserial PRIMARY KEY, + user_id bigint NOT NULL REFERENCES users (id) ON DELETE CASCADE, + project_id bigint NOT NULL REFERENCES projects (id) ON DELETE CASCADE, + kind notification_kind NOT NULL, + -- the thing and its number in the project; `security`: the project's findings (no number) + target_type text NOT NULL CHECK (target_type IN ('merge_request', 'issue', 'pipeline', 'security')), + target_iid bigint CHECK ((target_iid IS NULL) = (target_type = 'security')), + actor_id bigint REFERENCES users (id) ON DELETE SET NULL, + -- what it is about ("Fix the login page", "Pipeline #12 on main") + title text NOT NULL, + -- what happened ("approved", "failed") + summary text NOT NULL, + read_at timestamptz, + created_at timestamptz NOT NULL DEFAULT now() +); +CREATE INDEX notifications_user_idx ON notifications (user_id, created_at DESC, id DESC); +CREATE INDEX notifications_unread_idx ON notifications (user_id) WHERE read_at IS NULL; +-- what was read a while ago, or never read for a long time, is removed (housekeeping) +CREATE INDEX notifications_read_idx ON notifications (read_at) WHERE read_at IS NOT NULL; +CREATE INDEX notifications_created_idx ON notifications (created_at); + +-- What an inbox may show: what its owner can still see. A member sees a +-- project's issues, and its merge requests, pipelines and findings (code) from +-- Reporter; anyone sees a public project's, and an administrator everything. +CREATE VIEW listed_notifications AS +SELECT n.* +FROM notifications n +JOIN projects p ON p.id = n.project_id +JOIN users me ON me.id = n.user_id +LEFT JOIN listed_project_authorizations a ON a.project_id = n.project_id AND a.user_id = n.user_id +WHERE p.visibility = 20 + OR me.is_admin + OR a.access_level >= 20 + OR (a.access_level IS NOT NULL AND n.target_type = 'issue'); + +-- A person's choice for a kind on a channel; no row: the default +-- (`forgust_db::notifications::default_on`). `web` is the bell, `screen` the +-- message that shows as it arrives while the app is open. +CREATE TABLE notification_channels ( + user_id bigint NOT NULL REFERENCES users (id) ON DELETE CASCADE, + kind notification_kind NOT NULL, + channel text NOT NULL CHECK (channel IN ('web', 'email', 'screen')), + enabled boolean NOT NULL, + updated_at timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY (user_id, kind, channel) +); + +-- How a pipeline ended the last time its people were told (`success` or `failed`): +-- a job retried or played makes it end again, which is said once per outcome. +ALTER TABLE ci_pipelines ADD COLUMN notified_status text CHECK (notified_status IN ('success', 'failed')); diff --git a/crates/forgust-db/src/ci.rs b/crates/forgust-db/src/ci.rs index 94cfb12..5a26dd8 100644 --- a/crates/forgust-db/src/ci.rs +++ b/crates/forgust-db/src/ci.rs @@ -440,6 +440,42 @@ pub async fn insert_pipeline(e: impl PgExecutor<'_>, p: NewPipeline<'_>) -> Resu .await } +/// How the pipeline before this one ended (`success` or `failed`; canceled, +/// skipped and child pipelines are not counted, nor the instance's own: +/// dependency updates and security scans): the one before on the same ref +/// and for the same merge request (a branch's pipelines and its merge +/// request's are two series). +pub async fn previous_status(e: impl PgExecutor<'_>, p: &Pipeline) -> Result> { + sqlx::query_scalar!( + "SELECT status FROM ci_pipelines + WHERE project_id = $1 AND ref = $2 AND tag = $3 AND merge_request_id IS NOT DISTINCT FROM $4 AND id < $5 + AND status IN ('success', 'failed') AND source NOT IN ('parent_pipeline', 'renovate', 'security') + ORDER BY id DESC LIMIT 1", + p.project_id, + p.git_ref, + p.tag, + p.merge_request_id, + p.id + ) + .fetch_optional(e) + .await +} + +/// Record how the pipeline ended (`success` or `failed`) for its notifications, +/// and return what was recorded before (none: it ends for the first time). +pub async fn set_notified_status(e: impl PgExecutor<'_>, pipeline_id: i64, status: &str) -> Result> { + sqlx::query_scalar!( + "UPDATE ci_pipelines p SET notified_status = $2 + FROM (SELECT id, notified_status FROM ci_pipelines WHERE id = $1 FOR UPDATE) before + WHERE p.id = before.id + RETURNING before.notified_status", + pipeline_id, + status + ) + .fetch_one(e) + .await +} + pub async fn pipeline(e: impl PgExecutor<'_>, id: i64) -> Result> { sqlx::query_as!( Pipeline, diff --git a/crates/forgust-db/src/merge_requests.rs b/crates/forgust-db/src/merge_requests.rs index f229169..517c568 100644 --- a/crates/forgust-db/src/merge_requests.rs +++ b/crates/forgust-db/src/merge_requests.rs @@ -597,11 +597,13 @@ pub async fn approvals(e: impl PgExecutor<'_>, mr_id: i64, project_id: i64) -> R .await } -pub async fn approve(e: impl PgExecutor<'_>, mr_id: i64, user_id: i64) -> Result<()> { - sqlx::query!("INSERT INTO approvals (merge_request_id, user_id) VALUES ($1, $2) ON CONFLICT DO NOTHING", mr_id, user_id) +/// Whether the approval is new (not one the person had already given). +pub async fn approve(e: impl PgExecutor<'_>, mr_id: i64, user_id: i64) -> Result { + Ok(sqlx::query!("INSERT INTO approvals (merge_request_id, user_id) VALUES ($1, $2) ON CONFLICT DO NOTHING", mr_id, user_id) .execute(e) - .await?; - Ok(()) + .await? + .rows_affected() + > 0) } pub async fn unapprove(e: impl PgExecutor<'_>, mr_id: i64, user_id: i64) -> Result { diff --git a/crates/forgust-db/src/notifications.rs b/crates/forgust-db/src/notifications.rs index fb1198c..a37ef7d 100644 --- a/crates/forgust-db/src/notifications.rs +++ b/crates/forgust-db/src/notifications.rs @@ -76,3 +76,225 @@ pub async fn watchers(e: impl PgExecutor<'_>, project_id: i64) -> Result &'static str { + match self { + Kind::Mention => "mention", + Kind::Assignment => "assignment", + Kind::Activity => "activity", + Kind::Review => "review", + Kind::PipelineFailed => "pipeline_failed", + Kind::PipelineFixed => "pipeline_fixed", + Kind::Security => "security", + } + } +} + +/// Where a notification goes. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize, utoipa::ToSchema)] +#[serde(rename_all = "snake_case")] +pub enum Channel { + /// The bell in the app. + Web, + Email, + /// A message on screen as it arrives, while the app is open (for what + /// reaches the bell). + Screen, +} + +impl Channel { + pub fn as_str(self) -> &'static str { + match self { + Channel::Web => "web", + Channel::Email => "email", + Channel::Screen => "screen", + } + } +} + +/// Whether a kind goes to a channel for someone who chose nothing: everything +/// but the email for a pipeline that passes again. +pub fn default_on(kind: Kind, channel: Channel) -> bool { + !(kind == Kind::PipelineFixed && channel == Channel::Email) +} + +/// Whether `kind` goes to `channel`, from a person's own choices for that kind +/// (`(channel, enabled)` rows) or the default. +pub fn enabled<'a>(own: impl IntoIterator, kind: Kind, channel: Channel) -> bool { + own.into_iter().find(|(c, _)| *c == channel.as_str()).map(|(_, on)| on).unwrap_or_else(|| default_on(kind, channel)) +} + +/// A person's own choices: (kind, channel, enabled). +pub async fn channels(e: impl PgExecutor<'_>, user_id: i64) -> Result> { + Ok(sqlx::query!(r#"SELECT kind AS "kind!: String", channel, enabled FROM notification_channels WHERE user_id = $1"#, user_id) + .fetch_all(e) + .await? + .into_iter() + .map(|r| (r.kind, r.channel, r.enabled)) + .collect()) +} + +pub async fn set_channel(e: impl PgExecutor<'_>, user_id: i64, kind: Kind, channel: Channel, enabled: bool) -> Result<()> { + sqlx::query!( + "INSERT INTO notification_channels (user_id, kind, channel, enabled) VALUES ($1, $2, $3, $4) + ON CONFLICT (user_id, kind, channel) DO UPDATE SET enabled = $4, updated_at = now()", + user_id, + kind.as_str(), + channel.as_str(), + enabled + ) + .execute(e) + .await?; + Ok(()) +} + +/// The choices these people made for one kind: (user, channel, enabled). +pub async fn channel_choices(e: impl PgExecutor<'_>, user_ids: &[i64], kind: Kind) -> Result> { + Ok(sqlx::query!( + "SELECT user_id, channel, enabled FROM notification_channels WHERE user_id = ANY($1) AND kind = $2", + user_ids, + kind.as_str() + ) + .fetch_all(e) + .await? + .into_iter() + .map(|r| (r.user_id, r.channel, r.enabled)) + .collect()) +} + +/// A notification in someone's inbox. +#[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize, utoipa::ToSchema)] +pub struct Notification { + pub id: i64, + /// mention, assignment, activity, review, pipeline_failed, pipeline_fixed or security + pub kind: String, + pub project_id: i64, + pub project_path: String, + /// merge_request, issue, pipeline or security (the project's findings) + pub target_type: String, + /// Its number in the project (none for security). + pub target_iid: Option, + pub title: String, + pub summary: String, + pub actor_username: Option, + pub actor_name: Option, + pub read: bool, + pub created_at: chrono::DateTime, +} + +pub struct NewNotification<'a> { + pub project_id: i64, + pub kind: Kind, + pub target_type: &'a str, + pub target_iid: Option, + pub actor_id: Option, + pub title: &'a str, + pub summary: &'a str, +} + +/// The same notification in the inbox of each of these people: (user, its id +/// there, when). +pub async fn add( + e: impl PgExecutor<'_>, + user_ids: &[i64], + n: NewNotification<'_>, +) -> Result)>> { + Ok(sqlx::query!( + "INSERT INTO notifications (user_id, project_id, kind, target_type, target_iid, actor_id, title, summary) + SELECT u, $2, $3, $4, $5, $6, $7, $8 FROM unnest($1::bigint[]) u + RETURNING user_id, id, created_at", + user_ids, + n.project_id, + n.kind.as_str(), + n.target_type, + n.target_iid, + n.actor_id, + n.title, + n.summary + ) + .fetch_all(e) + .await? + .into_iter() + .map(|r| (r.user_id, r.id, r.created_at)) + .collect()) +} + +/// Someone's inbox, newest first: what they can still see (`listed_notifications`). +pub async fn inbox(e: impl PgExecutor<'_>, user_id: i64, limit: i64) -> Result> { + sqlx::query_as!( + Notification, + r#"SELECT n.id AS "id!", n.kind AS "kind!: String", n.project_id AS "project_id!", r.path AS project_path, + n.target_type AS "target_type!", n.target_iid, n.title AS "title!", n.summary AS "summary!", + u.username AS "actor_username?", u.name AS "actor_name?", n.read_at IS NOT NULL AS "read!", n.created_at AS "created_at!" + FROM listed_notifications n + JOIN routes r ON r.source_type = 'project' AND r.source_id = n.project_id + LEFT JOIN users u ON u.id = n.actor_id + WHERE n.user_id = $1 + ORDER BY n.created_at DESC, n.id DESC LIMIT $2"#, + user_id, + limit + ) + .fetch_all(e) + .await +} + +/// How many are unread, up to 100 (the bell shows "99+" beyond). +pub async fn unread_count(e: impl PgExecutor<'_>, user_id: i64) -> Result { + sqlx::query_scalar!( + r#"SELECT count(*) AS "n!" FROM (SELECT 1 FROM listed_notifications WHERE user_id = $1 AND read_at IS NULL LIMIT 100) unread"#, + user_id + ) + .fetch_one(e) + .await +} + +/// Mark one (`id`, read already or not) or all of someone's notifications read; +/// returns how many there were. +pub async fn mark_read(e: impl PgExecutor<'_>, user_id: i64, id: Option) -> Result { + Ok(sqlx::query!( + "UPDATE notifications SET read_at = coalesce(read_at, now()) + WHERE user_id = $1 AND (($2::bigint IS NULL AND read_at IS NULL) OR id = $2)", + user_id, + id + ) + .execute(e) + .await? + .rows_affected()) +} + +/// Drop what was read more than `days` ago, and what nobody read for three +/// times as long (housekeeping). +pub async fn prune(e: impl PgExecutor<'_>, days: i32) -> Result { + Ok(sqlx::query!( + "DELETE FROM notifications WHERE read_at < now() - make_interval(days => $1) OR created_at < now() - make_interval(days => 3 * $1)", + days + ) + .execute(e) + .await? + .rows_affected()) +} diff --git a/crates/forgust-server/src/api/mod.rs b/crates/forgust-server/src/api/mod.rs index 1a03f6d..04df185 100644 --- a/crates/forgust-server/src/api/mod.rs +++ b/crates/forgust-server/src/api/mod.rs @@ -196,6 +196,10 @@ pub fn router() -> Router { .route("/projects/{id}/merge_requests/{iid}/reactions", get(awards::mr_reactions).post(awards::mr_react)) .route("/projects/{id}/issues/{iid}/reactions", get(awards::issue_reactions).post(awards::issue_react)) .route("/user/notification_settings", get(notifications::mine).put(notifications::set_mine)) + .route("/user/notification_channels", get(notifications::channels).put(notifications::set_channel)) + .route("/user/notifications", get(notifications::inbox)) + .route("/user/notifications/read", post(notifications::read_all)) + .route("/user/notifications/{id}/read", post(notifications::read_one)) .route("/unsubscribe/{user_id}/{project_id}/{sig}", get(notifications::unsubscribe_info).post(notifications::unsubscribe)) .route("/projects/{id}/notification_settings", get(notifications::project).put(notifications::set_project)) .route("/search", get(search::search)) diff --git a/crates/forgust-server/src/api/mr.rs b/crates/forgust-server/src/api/mr.rs index 8b63d4a..ffdf0cd 100644 --- a/crates/forgust-server/src/api/mr.rs +++ b/crates/forgust-server/src/api/mr.rs @@ -26,6 +26,7 @@ use super::{group_for, project_for}; use crate::auth::authz::{GroupAction, ProjectAction, can_project}; use crate::auth::{SignedIn, Viewer}; use crate::error::{ApiError, ApiResult}; +use crate::events::Audience; use crate::services::mr::{self, MappedPosition, MergeMethod, MergeStatus, NewPosition}; use crate::state::{AppState, ResponseCache}; @@ -768,7 +769,9 @@ pub async fn hook_deliveries( #[derive(Deserialize)] pub struct EventsQuery { - /// Projects this tab watches (comma-separated IDs). + /// Projects this tab watches (comma-separated IDs; none: only the + /// caller's own events, their inbox). + #[serde(default)] projects: String, } @@ -795,13 +798,19 @@ pub async fn events( } // access is looked at again (once a minute by default) when an event comes: a member // removed from a project stops hearing about it + let me = v.direct_user().map(|u| u.id); let checked = std::sync::Arc::new(tokio::sync::Mutex::new((allowed, std::time::Instant::now()))); let stream = futures::StreamExt::filter_map(BroadcastStream::new(state.events.subscribe()), move |ev| { let (state, v, checked) = (state.clone(), v.clone(), checked.clone()); async move { let ev = ev.ok()?; + // a person's own events (their inbox changed): whatever the projects watched + let project_id = match ev.audience() { + Audience::User(id) => return (Some(id) == me).then(|| Ok(SseEvent::default().json_data(&ev).unwrap_or_default())), + Audience::Project(id) => id, + }; let mut guard = checked.lock().await; - if !guard.0.contains(&ev.project_id()) { + if !guard.0.contains(&project_id) { return None; } if guard.1.elapsed() >= Duration::from_secs(state.cfg.limits.events_recheck_secs) { @@ -814,7 +823,7 @@ pub async fn events( } } *guard = (still, std::time::Instant::now()); - if !guard.0.contains(&ev.project_id()) { + if !guard.0.contains(&project_id) { return None; } } diff --git a/crates/forgust-server/src/api/notifications.rs b/crates/forgust-server/src/api/notifications.rs index 0033700..c6d9168 100644 --- a/crates/forgust-server/src/api/notifications.rs +++ b/crates/forgust-server/src/api/notifications.rs @@ -1,9 +1,10 @@ //! Notification levels (M7, ADR-0021): a person's default and their choice per -//! project (`global` = the default). Same shapes under /api/v4. +//! project (`global` = the default). Same shapes under /api/v4. Then where each +//! kind of notification goes (the app, email), and the inbox the app shows. use axum::Json; -use axum::extract::{Path, State}; -use forgust_db::notifications::{self as ndb, LEVELS}; +use axum::extract::{Path, Query, State}; +use forgust_db::notifications::{self as ndb, Channel, Kind, LEVELS}; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; @@ -122,3 +123,93 @@ pub async fn unsubscribe( ndb::set_project_level(&state.db, u.id, p.id, Some("disabled")).await?; Ok(Json(Unsubscription { project: p.full_path, level: "disabled".into() })) } + +/// Where one kind of notification goes. +#[derive(Serialize, ToSchema)] +pub struct ChannelDto { + pub kind: Kind, + /// The bell in the app. + pub web: bool, + pub email: bool, + /// A message on screen as it arrives, while the app is open (only what + /// reaches the bell). + pub screen: bool, +} + +#[derive(Deserialize, ToSchema)] +pub struct ChannelBody { + pub kind: Kind, + pub channel: Channel, + pub enabled: bool, +} + +async fn channels_of(state: &AppState, user_id: i64) -> ApiResult> { + let own = ndb::channels(&state.db, user_id).await?; + let on = |kind: Kind, channel| { + ndb::enabled(own.iter().filter(|(k, ..)| k == kind.as_str()).map(|(_, c, on)| (c.as_str(), *on)), kind, channel) + }; + Ok(Kind::ALL + .into_iter() + .map(|kind| ChannelDto { kind, web: on(kind, Channel::Web), email: on(kind, Channel::Email), screen: on(kind, Channel::Screen) }) + .collect()) +} + +/// Where each kind of notification goes for the caller. +#[utoipa::path(get, path = "/api/v1/user/notification_channels", responses((status = 200, body = [ChannelDto])), tag = "user")] +pub async fn channels(State(state): State, SignedIn(u, _): SignedIn) -> ApiResult>> { + Ok(Json(channels_of(&state, u.id).await?)) +} + +/// Turn one kind on or off for one channel. +#[utoipa::path(put, path = "/api/v1/user/notification_channels", request_body = ChannelBody, responses((status = 200, body = [ChannelDto])), tag = "user")] +pub async fn set_channel( + State(state): State, + SignedIn(u, v): SignedIn, + Json(b): Json, +) -> ApiResult>> { + if v.direct_user().is_none() { + return Err(ApiError::Unauthorized); + } + ndb::set_channel(&state.db, u.id, b.kind, b.channel, b.enabled).await?; + Ok(Json(channels_of(&state, u.id).await?)) +} + +#[derive(Serialize, ToSchema)] +pub struct Inbox { + /// How many are unread (of everything, not only `items`). + pub unread: i64, + /// Newest first. + pub items: Vec, +} + +#[derive(Deserialize)] +pub struct InboxQuery { + limit: Option, +} + +/// The caller's notifications in the app. +#[utoipa::path(get, path = "/api/v1/user/notifications", responses((status = 200, body = Inbox)), tag = "user")] +pub async fn inbox(State(state): State, SignedIn(u, _): SignedIn, Query(q): Query) -> ApiResult> { + let items = ndb::inbox(&state.db, u.id, q.limit.unwrap_or(30).clamp(1, 100)).await?; + Ok(Json(Inbox { unread: ndb::unread_count(&state.db, u.id).await?, items })) +} + +async fn read(state: &AppState, user_id: i64, id: Option) -> ApiResult> { + let n = ndb::mark_read(&state.db, user_id, id).await?; + if n == 0 && id.is_some() { + return Err(ApiError::NotFound); + } + // the person's other tabs follow + state.events.publish(crate::events::Event::Inbox { user_id }); + Ok(Json(Inbox { unread: ndb::unread_count(&state.db, user_id).await?, items: vec![] })) +} + +/// Mark one notification read. +pub async fn read_one(State(state): State, SignedIn(u, _): SignedIn, Path(id): Path) -> ApiResult> { + read(&state, u.id, Some(id)).await +} + +/// Mark every notification read. +pub async fn read_all(State(state): State, SignedIn(u, _): SignedIn) -> ApiResult> { + read(&state, u.id, None).await +} diff --git a/crates/forgust-server/src/api/openapi.rs b/crates/forgust-server/src/api/openapi.rs index f6ebfa8..eb26491 100644 --- a/crates/forgust-server/src/api/openapi.rs +++ b/crates/forgust-server/src/api/openapi.rs @@ -19,7 +19,7 @@ use super::{ repo::tree, repo::last_commits, repo::stats, repo::blob, repo::commits, repo::commit, repo::commit_diff, repo::branches, repo::blame, repo::templates, repo::create_branch, repo::create_commit, repo::create_tag, repo::delete_tag, mr::list, mr::count, mr::create, mr::show, mr::stack_on, mr::auto_merge, mr::merge_status, mr::merge, mr::diff, mr::notes, mr::set_labels, awards::mr_reactions, awards::mr_react, awards::issue_reactions, awards::issue_react, - notifications::mine, notifications::unsubscribe_info, notifications::unsubscribe, notifications::set_mine, notifications::project, notifications::set_project, + notifications::mine, notifications::channels, notifications::set_channel, notifications::inbox, notifications::unsubscribe_info, notifications::unsubscribe, notifications::set_mine, notifications::project, notifications::set_project, search::search, security::findings, security::finding, security::dismiss, security::reopen, security::merge_request, security::scanning, security::set_scanning, security::run_scan, security::group_scanning, security::set_group_scanning, security::admin_scanning, security::set_admin_scanning, tests::pipeline_tests, tests::pipeline_test_cases, tests::mr_tests, milestones::list, milestones::show, milestones::create, milestones::update, milestones::delete, diff --git a/crates/forgust-server/src/ci/pipeline.rs b/crates/forgust-server/src/ci/pipeline.rs index 5214706..e5be23a 100644 --- a/crates/forgust-server/src/ci/pipeline.rs +++ b/crates/forgust-server/src/ci/pipeline.rs @@ -411,7 +411,12 @@ pub async fn create(state: &AppState, t: Trigger<'_>) -> anyhow::Result pipeline_id, status: if errors.is_some() { "failed".into() } else { "created".into() }, }); - if errors.is_none() { + if errors.is_some() { + // a configuration that does not load: the pipeline is born failed + if let Some(pipeline) = forgust_db::ci::pipeline(&state.db, pipeline_id).await? { + crate::services::notify::pipeline_finished(state, &pipeline, "failed").await; + } + } else { process(state, pipeline_id).await?; if settings.auto_cancel_redundant && !t.tag && matches!(t.source, "push" | "merge_request_event") { auto_cancel_redundant(state, pipeline_id).await?; @@ -573,6 +578,7 @@ pub async fn process(state: &AppState, pipeline_id: i64) -> anyhow::Result<()> { // `refs/pipelines/` keeps the commit fetchable while jobs may run // (a retry or a played job makes the pipeline active again) if is_complete(status) { + crate::services::notify::pipeline_finished(state, &pipeline, status).await; // findings are resolved by the default branch's scans only: not a // tag's or a merge request's pipeline (no job queued for nothing) if !pipeline.tag && pipeline.merge_request_id.is_none() { diff --git a/crates/forgust-server/src/events.rs b/crates/forgust-server/src/events.rs index a31591a..721c6fb 100644 --- a/crates/forgust-server/src/events.rs +++ b/crates/forgust-server/src/events.rs @@ -48,6 +48,17 @@ pub enum Event { JobsQueued { project_id: i64, }, + /// A notification reached this person's inbox (only their own streams hear + /// it); `screen`: they want it shown as it arrives. + Notification { + user_id: i64, + screen: bool, + notification: forgust_db::notifications::Notification, + }, + /// This person's inbox changed otherwise (they read something elsewhere). + Inbox { + user_id: i64, + }, } impl Event { @@ -56,8 +67,10 @@ impl Event { Event::Job { project_id: b.project_id, pipeline_id: b.pipeline_id, job_id: b.id, status: status.into(), trace_size: b.trace_size } } - pub fn project_id(&self) -> i64 { + /// Who may hear it on a live stream. + pub fn audience(&self) -> Audience { match self { + Event::Notification { user_id, .. } | Event::Inbox { user_id } => Audience::User(*user_id), Event::Push { project_id, .. } | Event::ProjectDeleted { project_id } | Event::RepoReset { project_id } @@ -65,11 +78,18 @@ impl Event { | Event::Pipeline { project_id, .. } | Event::Job { project_id, .. } | Event::JobTrace { project_id, .. } - | Event::JobsQueued { project_id } => *project_id, + | Event::JobsQueued { project_id } => Audience::Project(*project_id), } } } +/// Who an event is for: those who see a project, or one person. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Audience { + Project(i64), + User(i64), +} + pub trait EventBus: Send + Sync + 'static { fn publish(&self, ev: Event); fn subscribe(&self) -> broadcast::Receiver; @@ -106,6 +126,6 @@ mod tests { let bus = InProcessBus::new(16); let mut rx = bus.subscribe(); bus.publish(Event::Push { project_id: 1, refs: vec!["refs/heads/main".into()] }); - assert_eq!(rx.recv().await.unwrap().project_id(), 1); + assert_eq!(rx.recv().await.unwrap().audience(), Audience::Project(1)); } } diff --git a/crates/forgust-server/src/jobs.rs b/crates/forgust-server/src/jobs.rs index 1c76019..1e6f666 100644 --- a/crates/forgust-server/src/jobs.rs +++ b/crates/forgust-server/src/jobs.rs @@ -415,6 +415,8 @@ impl TaskHandler for Cleanup { if !ended.is_empty() { tracing::info!(users = ended.len(), "expired memberships removed"); } + // notifications read a month ago leave the inbox + forgust_db::notifications::prune(&state.db, 30).await.map_err(|e| e.to_string())?; state.storage.reap_idle(); metrics::gauge!("forgust_catfile_processes").set(state.storage.catfile_processes() as f64); Ok::<(), String>(()) diff --git a/crates/forgust-server/src/services/mr.rs b/crates/forgust-server/src/services/mr.rs index 0bd7517..48793b1 100644 --- a/crates/forgust-server/src/services/mr.rs +++ b/crates/forgust-server/src/services/mr.rs @@ -787,9 +787,13 @@ pub async fn approve(state: &AppState, principal: &Principal, p: &Project, mr: & return Err(ApiError::conflict("only open merge requests can be approved")); } if approve { - mrdb::approve(&state.db, mr.id, user.id).await?; + let new = mrdb::approve(&state.db, mr.id, user.id).await?; super::people::reviewed(state, mr, user.id, super::people::ReviewAction::Approved).await; done(state, user.id, p, mr).await; + // (approving again says nothing new) + if new { + super::notify::approved(state, p, mr, user).await; + } } else { mrdb::unapprove(&state.db, mr.id, user.id).await?; super::people::reviewed(state, mr, user.id, super::people::ReviewAction::Unapproved).await; diff --git a/crates/forgust-server/src/services/notify.rs b/crates/forgust-server/src/services/notify.rs index 6c25b31..1449607 100644 --- a/crates/forgust-server/src/services/notify.rs +++ b/crates/forgust-server/src/services/notify.rs @@ -1,5 +1,6 @@ -//! Email notifications (SMTP), sent from a background job. Disabled when no SMTP -//! URL is configured. +//! Notifications: who hears about what (levels, ADR-0021), then where it goes, +//! a switch per kind and channel: the bell in the app, and email (SMTP, sent +//! from a background job; none when no SMTP URL is configured). use std::sync::Mutex; @@ -146,12 +147,74 @@ pub fn allows(level: &str, r: Reason) -> bool { } } +use forgust_db::notifications::Channel; +pub use forgust_db::notifications::Kind; + +/// What a notification points at, by its number in the project. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Target { + MergeRequest(i64), + Issue(i64), + Pipeline(i64), + /// The project's security findings. + Security, +} + +impl Target { + pub fn as_str(self) -> &'static str { + match self { + Target::MergeRequest(_) => "merge_request", + Target::Issue(_) => "issue", + Target::Pipeline(_) => "pipeline", + Target::Security => "security", + } + } + + pub fn iid(self) -> Option { + match self { + Target::MergeRequest(iid) | Target::Issue(iid) | Target::Pipeline(iid) => Some(iid), + Target::Security => None, + } + } + + /// What a recipient must still be allowed to do in the project to hear + /// about it: issues are for anyone who sees the project, the rest is code. + pub fn need(self) -> ProjectAction { + match self { + Target::Issue(_) => ProjectAction::Read, + _ => ProjectAction::ReadCode, + } + } +} + +/// What the bell shows: the thing, its title, and what happened in a few words +/// ("approved", "commented", "failed"). +pub struct About { + pub kind: Kind, + pub target: Target, + pub title: String, + pub summary: String, +} + +impl About { + /// What a scan of `git_ref` found: it leads to the project's findings. + pub fn security(git_ref: &str, summary: String) -> About { + About { kind: Kind::Security, target: Target::Security, title: format!("Security scan of {git_ref}"), summary } + } + + pub fn mr(kind: Kind, mr: &MergeRequest, summary: &str) -> About { + About { kind, target: Target::MergeRequest(mr.iid), title: mr.title.clone(), summary: summary.into() } + } + + pub fn issue(kind: Kind, issue: &forgust_db::issues::Issue, summary: &str) -> About { + About { kind, target: Target::Issue(issue.iid), title: issue.title.clone(), summary: summary.into() } + } +} + /// One notification about something in a project. pub struct Notice<'a> { pub project: &'a Project, - /// What a recipient must still be allowed to do in the project (Read for - /// issues, ReadCode for merge requests), checked when sending. - pub need: ProjectAction, + pub about: About, /// Who did it: never told about their own action. pub actor: Option, pub subject: String, @@ -198,9 +261,9 @@ pub fn unsubscribe_path(state: &AppState, user_id: i64, project_id: i64) -> Stri } /// The single way project notifications go out: each person once (with their -/// strongest reason), if their level allows it, if they are an active human with -/// a confirmed address, and if they can still see the thing. One Bcc message, so -/// the footer is the same for everyone (ADR-0021). +/// strongest reason), if their level allows it, if they are an active human and +/// can still see the thing. Then where they chose to get that kind: the bell, +/// and an email of their own when their address is confirmed (ADR-0021). pub async fn deliver(state: &AppState, n: Notice<'_>, people: Vec<(i64, Reason)>) { let mut strongest: std::collections::BTreeMap = std::collections::BTreeMap::new(); for (id, r) in people { @@ -220,14 +283,30 @@ pub async fn deliver(state: &AppState, n: Notice<'_>, people: Vec<(i64, Reason)> let thread_id = n.thread.as_ref().map(|t| format!("<{t}.{}@{host}>", n.project.id)); let allowed: Vec = levels.iter().filter(|(id, level)| allows(level, strongest[id])).map(|(id, _)| *id).collect(); let users = forgust_db::users::find_many(&state.db, &allowed).await.unwrap_or_default(); + let kind = n.about.kind; + // (not known: nobody is told, rather than people who turned it off) + let Ok(choices) = forgust_db::notifications::channel_choices(&state.db, &allowed, kind).await else { + tracing::warn!(kind = kind.as_str(), "notification choices not read: not sent"); + return; + }; + let on = |user: i64, channel: Channel| { + forgust_db::notifications::enabled(choices.iter().filter(|(id, ..)| *id == user).map(|(_, c, on)| (c.as_str(), *on)), kind, channel) + }; + let mut bell = Vec::new(); let mut emails = Vec::new(); for u in users { let reason = strongest[&u.id]; - if !u.is_active() || u.is_bot() || !u.email_verified() { + if !u.is_active() || u.is_bot() { continue; } let who = crate::auth::Principal::User { user: u.clone(), scopes: None }; - if !can_project(state, &who, n.need, n.project).await.unwrap_or(false) { + if !can_project(state, &who, n.about.target.need(), n.project).await.unwrap_or(false) { + continue; + } + if on(u.id, Channel::Web) { + bell.push(u.id); + } + if !on(u.id, Channel::Email) || !u.email_verified() { continue; } // one message each: their own reason, their own unsubscribe link (ADR-0021) @@ -258,6 +337,49 @@ pub async fn deliver(state: &AppState, n: Notice<'_>, people: Vec<(i64, Reason)> }; emails.push(email); } + if !bell.is_empty() { + let target = n.about.target; + let new = forgust_db::notifications::NewNotification { + project_id: n.project.id, + kind, + target_type: target.as_str(), + target_iid: target.iid(), + actor_id: n.actor, + title: &n.about.title, + summary: &n.about.summary, + }; + let actor = match n.actor { + Some(id) => forgust_db::users::find(&state.db, id).await.ok().flatten(), + None => None, + }; + match forgust_db::notifications::add(&state.db, &bell, new).await { + // each person's streams get theirs, to show at once + Ok(rows) => { + for (user_id, id, created_at) in rows { + let notification = forgust_db::notifications::Notification { + id, + kind: kind.as_str().into(), + project_id: n.project.id, + project_path: n.project.full_path.clone(), + target_type: target.as_str().into(), + target_iid: target.iid(), + title: n.about.title.clone(), + summary: n.about.summary.clone(), + actor_username: actor.as_ref().map(|u| u.username.clone()), + actor_name: actor.as_ref().map(|u| u.name.clone()), + read: false, + created_at, + }; + state.events.publish(crate::events::Event::Notification { + user_id, + screen: on(user_id, Channel::Screen), + notification, + }); + } + } + Err(e) => tracing::warn!(error = %e, "notifications not recorded"), + } + } queue_all(state, emails).await; } @@ -266,15 +388,15 @@ pub async fn deliver(state: &AppState, n: Notice<'_>, people: Vec<(i64, Reason)> pub async fn tell( state: &AppState, p: &Project, - need: ProjectAction, actor: i64, users: &[User], reason: Reason, thread: Option, + about: About, subject: &str, body: &str, ) { - let n = Notice { project: p, need, actor: Some(actor), subject: subject.into(), body: body.into(), thread, starts_thread: false }; + let n = Notice { project: p, about, actor: Some(actor), subject: subject.into(), body: body.into(), thread, starts_thread: false }; deliver(state, n, users.iter().map(|u| (u.id, reason)).collect()).await; } @@ -315,10 +437,10 @@ pub async fn merge_request_note(state: &AppState, p: &Project, mr: &MergeRequest }; let people = mr_audience(state, p, mr).await.into_iter().filter(|(id, _)| !skip.contains(id)).collect(); let n = Notice { + about: About::mr(Kind::Activity, mr, "commented"), thread: mr_thread(mr.id), starts_thread: false, project: p, - need: ProjectAction::ReadCode, actor: Some(author.id), subject: format!("Re: {} !{}: {}", p.full_path, mr.iid, mr.title), body: format!("{} commented{place}:\n\n{}\n\n{}\n", author.username, note.body, state.cfg.mr_url(&p.full_path, mr.iid)), @@ -328,17 +450,19 @@ pub async fn merge_request_note(state: &AppState, p: &Project, mr: &MergeRequest pub async fn merge_request_state(state: &AppState, p: &Project, mr: &MergeRequest, action: &str, actor: Option<&User>) { let who = actor.map(|u| u.username.as_str()).unwrap_or("someone"); - let verb = match action { - "merge" => format!("merged into {}", mr.target_branch), - "close" => "closed".into(), - "reopen" => "reopened".into(), - other => other.into(), + // what the bell says, and the email in full + let short = match action { + "merge" => "merged", + "close" => "closed", + "reopen" => "reopened", + other => other, }; + let verb = if action == "merge" { format!("merged into {}", mr.target_branch) } else { short.to_string() }; let n = Notice { + about: About::mr(Kind::Activity, mr, short), thread: mr_thread(mr.id), starts_thread: false, project: p, - need: ProjectAction::ReadCode, actor: actor.map(|u| u.id), subject: format!("{} !{}: {}", p.full_path, mr.iid, mr.title), body: format!("{who} {verb} merge request !{} \"{}\".\n\n{}\n", mr.iid, mr.title, state.cfg.mr_url(&p.full_path, mr.iid)), @@ -349,10 +473,10 @@ pub async fn merge_request_state(state: &AppState, p: &Project, mr: &MergeReques /// A new merge request: its watchers hear about it. pub async fn new_merge_request(state: &AppState, p: &Project, mr: &MergeRequest, author: &User) { let n = Notice { + about: About::mr(Kind::Activity, mr, "opened"), thread: mr_thread(mr.id), starts_thread: true, project: p, - need: ProjectAction::ReadCode, actor: Some(author.id), subject: format!("{} !{}: {}", p.full_path, mr.iid, mr.title), body: format!( @@ -372,10 +496,10 @@ pub async fn new_merge_request(state: &AppState, p: &Project, mr: &MergeRequest, /// A new issue: its watchers hear about it. pub async fn new_issue(state: &AppState, p: &Project, issue: &forgust_db::issues::Issue, author: &User) { let n = Notice { + about: About::issue(Kind::Activity, issue, "opened"), thread: issue_thread(issue.id), starts_thread: true, project: p, - need: ProjectAction::Read, actor: Some(author.id), subject: format!("{} #{}: {}", p.full_path, issue.iid, issue.title), body: format!( @@ -393,10 +517,10 @@ pub async fn new_issue(state: &AppState, p: &Project, issue: &forgust_db::issues pub async fn issue_note(state: &AppState, p: &Project, issue: &forgust_db::issues::Issue, author: &User, body: &str, skip: &[i64]) { let people = issue_audience(state, p, issue.id).await.into_iter().filter(|(id, _)| !skip.contains(id)).collect(); let n = Notice { + about: About::issue(Kind::Activity, issue, "commented"), thread: issue_thread(issue.id), starts_thread: false, project: p, - need: ProjectAction::Read, actor: Some(author.id), subject: format!("Re: {} #{}: {}", p.full_path, issue.iid, issue.title), body: format!("{} commented:\n\n{}\n\n{}\n", author.username, body, state.cfg.issue_url(&p.full_path, issue.iid)), @@ -406,10 +530,10 @@ pub async fn issue_note(state: &AppState, p: &Project, issue: &forgust_db::issue pub async fn issue_state(state: &AppState, p: &Project, issue: &forgust_db::issues::Issue, open: bool, actor: &User) { let n = Notice { + about: About::issue(Kind::Activity, issue, if open { "reopened" } else { "closed" }), thread: issue_thread(issue.id), starts_thread: false, project: p, - need: ProjectAction::Read, actor: Some(actor.id), subject: format!("{} #{}: {}", p.full_path, issue.iid, issue.title), body: format!( @@ -424,6 +548,73 @@ pub async fn issue_state(state: &AppState, p: &Project, issue: &forgust_db::issu deliver(state, n, issue_audience(state, p, issue.id).await).await; } +/// The author hears that someone approved their merge request (asking for +/// changes tells them from `people::request_changes`, with the comment). +pub async fn approved(state: &AppState, p: &Project, mr: &MergeRequest, by: &User) { + let Some(author) = mr.author_id else { return }; + let n = Notice { + about: About::mr(Kind::Review, mr, "approved"), + thread: mr_thread(mr.id), + starts_thread: false, + project: p, + actor: Some(by.id), + subject: format!("{} !{}: {}", p.full_path, mr.iid, mr.title), + body: format!( + "{} approved merge request !{} \"{}\".\n\n{}\n", + by.username, + mr.iid, + mr.title, + state.cfg.mr_url(&p.full_path, mr.iid) + ), + }; + deliver(state, n, vec![(author, Reason::Direct)]).await; +} + +/// A pipeline failed, or passed where the one before on its ref failed (or +/// where it failed itself before a retry): who started it hears about it, and +/// the authors of the merge requests it runs for. Not a child pipeline (its +/// parent says it), the instance's own (dependency updates, security scans), +/// nor one canceled or skipped. +pub async fn pipeline_finished(state: &AppState, pipeline: &forgust_db::ci::Pipeline, status: &str) { + if matches!(pipeline.source.as_str(), "parent_pipeline" | "renovate" | "security") || !matches!(status, "success" | "failed") { + return; + } + // a job retried or played makes the pipeline end again: each outcome is said once + let Ok(before) = forgust_db::ci::set_notified_status(&state.db, pipeline.id, status).await else { return }; + let (kind, outcome) = match (status, before.as_deref()) { + (_, Some(same)) if same == status => return, + ("failed", _) => (Kind::PipelineFailed, "failed"), + // it failed before and was retried + (_, Some(_)) => (Kind::PipelineFixed, "passed again"), + _ => match forgust_db::ci::previous_status(&state.db, pipeline).await { + Ok(Some(earlier)) if earlier == "failed" => (Kind::PipelineFixed, "passed again"), + _ => return, + }, + }; + let Ok(Some(p)) = forgust_db::projects::find(&state.db, pipeline.project_id).await else { return }; + let mrs = match pipeline.merge_request_id { + Some(id) => forgust_db::merge_requests::find_by_id(&state.db, id).await.ok().flatten().into_iter().collect(), + None if pipeline.tag => vec![], + None => forgust_db::merge_requests::open_with_source(&state.db, p.id, &pipeline.git_ref).await.unwrap_or_default(), + }; + let people: Vec<(i64, Reason)> = + pipeline.user_id.into_iter().chain(mrs.iter().filter_map(|m| m.author_id)).map(|id| (id, Reason::Direct)).collect(); + let title = format!("Pipeline #{} on {}", pipeline.iid, pipeline.git_ref); + let commit = pipeline.commit_title.as_deref().unwrap_or(""); + let url = state.cfg.project_page(&p.full_path, &format!("pipelines/{}", pipeline.iid)); + let n = Notice { + about: About { kind, target: Target::Pipeline(pipeline.iid), title: title.clone(), summary: outcome.into() }, + thread: None, + starts_thread: false, + project: &p, + // who started it is told too + actor: None, + subject: format!("{} pipeline #{} {outcome} on {}", p.full_path, pipeline.iid, pipeline.git_ref), + body: format!("{title} {outcome}.\n\n{} {commit}\n\n{url}\n", &pipeline.sha[..pipeline.sha.len().min(8)]), + }; + deliver(state, n, people).await; +} + /// One email to one address, whatever its state (password resets, address /// confirmations). pub async fn send_to(state: &AppState, to: &str, subject: &str, body: &str) { diff --git a/crates/forgust-server/src/services/people.rs b/crates/forgust-server/src/services/people.rs index b987a2b..edc8fbb 100644 --- a/crates/forgust-server/src/services/people.rs +++ b/crates/forgust-server/src/services/people.rs @@ -13,9 +13,10 @@ use forgust_db::projects::Project; use forgust_db::users::User; use crate::auth::Principal; -use crate::auth::authz::{ProjectAction, project_role}; +use crate::auth::authz::project_role; use crate::error::{ApiError, ApiResult}; use crate::events::Event; +use crate::services::notify::{About, Kind}; use crate::state::AppState; const MAX: usize = 20; @@ -128,16 +129,15 @@ pub async fn set_mr( let what = format!("merge request !{} \"{}\"", m.iid, m.title); let url = state.cfg.mr_url(&p.full_path, m.iid); let subject = format!("{} !{}: {}", p.full_path, m.iid, m.title); - let rc = ProjectAction::ReadCode; let direct = super::notify::Reason::Direct; super::notify::tell( state, p, - rc, actor.id, ¬ify(&assigned.added), direct, super::notify::mr_thread(m.id), + About::mr(Kind::Assignment, m, "assigned you"), &subject, &format!("{} assigned you to {what}.\n\n{url}\n", actor.username), ) @@ -145,11 +145,11 @@ pub async fn set_mr( super::notify::tell( state, p, - rc, actor.id, ¬ify(&requested.added), direct, super::notify::mr_thread(m.id), + About::mr(Kind::Assignment, m, "asked you to review"), &subject, &format!("{} asked you to review {what}.\n\n{url}\n", actor.username), ) @@ -178,11 +178,11 @@ pub async fn rerequest(state: &AppState, principal: &Principal, p: &Project, m: super::notify::tell( state, p, - ProjectAction::ReadCode, actor.id, &to, super::notify::Reason::Direct, super::notify::mr_thread(m.id), + About::mr(Kind::Assignment, m, "asked you to review again"), &format!("{} !{}: {}", p.full_path, m.iid, m.title), &format!("{} asked you to review merge request !{} \"{}\" again.\n\n{url}\n", actor.username, m.iid, m.title), ) @@ -270,11 +270,11 @@ pub async fn set_issue(state: &AppState, principal: &Principal, p: &Project, i: super::notify::tell( state, p, - ProjectAction::Read, actor.id, &to, super::notify::Reason::Direct, super::notify::issue_thread(i.id), + About::issue(Kind::Assignment, i, "assigned you"), &format!("{} #{}: {}", p.full_path, i.iid, i.title), &format!("{} assigned you to issue #{} \"{}\".\n\n{url}\n", actor.username, i.iid, i.title), ) @@ -329,11 +329,11 @@ pub async fn request_changes( super::notify::tell( state, p, - ProjectAction::ReadCode, user.id, &[author], super::notify::Reason::Direct, super::notify::mr_thread(m.id), + About::mr(Kind::Review, m, "requested changes"), &format!("{} !{}: {}", p.full_path, m.iid, m.title), &match comment { Some(text) => { diff --git a/crates/forgust-server/src/services/refs.rs b/crates/forgust-server/src/services/refs.rs index 4ad90e8..8d64c96 100644 --- a/crates/forgust-server/src/services/refs.rs +++ b/crates/forgust-server/src/services/refs.rs @@ -79,15 +79,6 @@ pub enum Target<'a> { } impl Target<'_> { - /// What seeing it takes: issues are for anyone who sees the project, merge - /// requests for those who see its code. - fn need(&self) -> crate::auth::authz::ProjectAction { - match self { - Target::MergeRequest { .. } => crate::auth::authz::ProjectAction::ReadCode, - Target::Issue { .. } => crate::auth::authz::ProjectAction::Read, - } - } - fn thread(&self) -> Option { match self { Target::MergeRequest { id, .. } => super::notify::mr_thread(*id), @@ -174,11 +165,19 @@ pub async fn on_write(state: &AppState, p: &Project, actor: &User, target: Targe super::notify::tell( state, p, - target.need(), actor.id, &to, super::notify::Reason::Mention, target.thread(), + super::notify::About { + kind: super::notify::Kind::Mention, + target: match target { + Target::MergeRequest { iid, .. } => super::notify::Target::MergeRequest(iid), + Target::Issue { iid, .. } => super::notify::Target::Issue(iid), + }, + title: title_of(&target).to_string(), + summary: "mentioned you".into(), + }, &format!("{} {}: {}", p.full_path, target_ref(&target), title_of(&target)), &format!("{} mentioned you in {}:\n\n{}\n\n{}\n", actor.username, target.label(), text.trim(), target.url(state, p)), ) diff --git a/crates/forgust-server/src/services/security.rs b/crates/forgust-server/src/services/security.rs index 13f4fdf..6c0e113 100644 --- a/crates/forgust-server/src/services/security.rs +++ b/crates/forgust-server/src/services/security.rs @@ -136,7 +136,7 @@ pub async fn ingest(state: &AppState, build_id: i64) -> anyhow::Result<()> { /// the Maintainers. A new secret: to whoever ran the pipeline (they pushed it), /// to rotate it. async fn alert(state: &AppState, p: &Project, pl: &Pipeline, fixable: &[Finding], secrets: &[Finding]) { - use super::notify::{Notice, Reason, deliver}; + use super::notify::{About, Notice, Reason, deliver}; let web = state.cfg.web_url(&p.full_path); if !fixable.is_empty() { let mut body = format!( @@ -170,7 +170,10 @@ async fn alert(state: &AppState, p: &Project, pl: &Pipeline, fixable: &[Finding] state, Notice { project: p, - need: crate::auth::authz::ProjectAction::ReadCode, + about: About::security( + &pl.git_ref, + format!("found {} vulnerable dependenc{} to update", fixable.len(), if fixable.len() == 1 { "y" } else { "ies" }), + ), actor: None, subject, body, @@ -204,7 +207,10 @@ async fn alert(state: &AppState, p: &Project, pl: &Pipeline, fixable: &[Finding] state, Notice { project: p, - need: crate::auth::authz::ProjectAction::ReadCode, + about: About::security( + &pl.git_ref, + format!("found {} secret{} in the code", secrets.len(), if secrets.len() == 1 { "" } else { "s" }), + ), actor: None, subject: format!("[{}] A secret was found in the code", p.full_path), body, diff --git a/crates/forgust-server/tests/m4_ci.rs b/crates/forgust-server/tests/m4_ci.rs index 562588d..816e754 100644 --- a/crates/forgust-server/tests/m4_ci.rs +++ b/crates/forgust-server/tests/m4_ci.rs @@ -866,3 +866,63 @@ async fn merge_request_pipelines_are_protected_only_between_protected_branches(p "main's own pipeline stays protected: {main}" ); } + +#[sqlx::test(migrator = "forgust_db::MIGRATOR")] +async fn a_failed_pipeline_and_the_one_that_fixes_it_reach_who_started_them(pool: sqlx::PgPool) { + let srv = TestServer::start_with(pool, |c| { + c.ci.long_poll_secs = 1; + }) + .await; + let owner = srv.user("olivia", false).await; + let (_org, _sub, project) = org_project(&owner).await; + let work = srv.work_dir("notify"); + must(git(&work, &["init", "-q", "-b", "main"]).await); + let f = Fixture { srv, owner, project, sha: String::new() }; + let token = new_runner(&f, json!({})).await; + // a push, then its only job ends as `state` + let run = async |n: usize, state: &str| { + std::fs::write(work.join(".gitlab-ci.yml"), format!("job:\n script: [make v{n}]\n")).unwrap(); + must(git(&work, &["add", "."]).await); + must(git(&work, &["commit", "-q", "-m", &format!("v{n}")]).await); + must(git(&work, &["push", "-q", &f.srv.http_url("olivia", "acme/platform/api"), "main"]).await); + wait_jobs(&f.srv.state).await; + let (s, j) = request_job(&f, &token).await; + assert_eq!(s, StatusCode::CREATED, "{j}"); + let mut body = json!({ "token": j["token"], "state": state }); + if state == "failed" { + body["failure_reason"] = json!("script_failure"); + } + assert_eq!(put(&f, j["id"].as_i64().unwrap(), body).await.status(), StatusCode::OK); + wait_jobs(&f.srv.state).await; + }; + let inbox = || async { + let got = f.owner.ok_get("/api/v1/user/notifications").await; + got["items"] + .as_array() + .unwrap() + .iter() + .map(|n| format!("{} {}", n["kind"].as_str().unwrap(), n["title"].as_str().unwrap())) + .collect::>() + }; + let mails = |text: &str| f.srv.state.mailer.sent().iter().filter(|m| m.subject.contains(text)).count(); + + // a pipeline that passes with nothing before it: nothing to say + run(1, "success").await; + assert!(inbox().await.is_empty(), "{:?}", inbox().await); + // a job fails: who pushed hears about it, in the app and by email + run(2, "failed").await; + assert_eq!(inbox().await, ["pipeline_failed Pipeline #2 on main"]); + assert_eq!(mails("pipeline #2 failed on main"), 1); + // the next one passes: fixed, in the app only (its email is off by default) + run(3, "success").await; + assert_eq!(inbox().await, ["pipeline_fixed Pipeline #3 on main", "pipeline_failed Pipeline #2 on main"]); + assert_eq!(mails("passed again"), 0); + // and the one after that is just a pipeline that passes + run(4, "success").await; + assert_eq!(inbox().await.len(), 2, "{:?}", inbox().await); + // with the email on, the next fix is mailed too + f.owner.ok_put("/api/v1/user/notification_channels", json!({ "kind": "pipeline_fixed", "channel": "email", "enabled": true })).await; + run(5, "failed").await; + run(6, "success").await; + assert_eq!(mails("pipeline #6 passed again on main"), 1); +} diff --git a/crates/forgust-server/tests/m7_v1.rs b/crates/forgust-server/tests/m7_v1.rs index 2536d89..1459fcc 100644 --- a/crates/forgust-server/tests/m7_v1.rs +++ b/crates/forgust-server/tests/m7_v1.rs @@ -604,6 +604,107 @@ async fn notification_levels_and_watching(pool: sqlx::PgPool) { assert_eq!(v4.put(&format!("/api/v4/projects/{proj}/notification_settings"), json!({ "level": "watch" })).await.1["level"], "watch"); } +#[sqlx::test(migrator = "forgust_db::MIGRATOR")] +async fn inbox_and_a_switch_per_kind_and_channel(pool: sqlx::PgPool) { + let srv = TestServer::start(pool).await; + let (owner, dev, rep, proj, work) = setup(&srv).await; + let inbox = "/api/v1/user/notifications"; + let channels = "/api/v1/user/notification_channels"; + let of = |v: &serde_json::Value, kind: &str| { + v["items"].as_array().unwrap().iter().filter(|n| n["kind"] == kind).cloned().collect::>() + }; + + // everything goes to the app and by email, but the email of a pipeline that passes again + let all = dev.ok_get(channels).await; + assert_eq!(all.as_array().unwrap().len(), 7, "{all}"); + assert!( + all.as_array() + .unwrap() + .iter() + .all(|c| c["web"] == true && c["screen"] == true && (c["email"] == true) == (c["kind"] != "pipeline_fixed")), + "{all}" + ); + assert_eq!(dev.ok_put(channels, json!({ "kind": "activity", "channel": "screen", "enabled": false })).await[2]["screen"], false); + assert!(matches!(dev.put(channels, json!({ "kind": "loud", "channel": "web", "enabled": false })).await.0, 400 | 422)); + assert!(matches!(dev.put(channels, json!({ "kind": "review", "channel": "pager", "enabled": false })).await.0, 400 | 422)); + // every kind the server knows is one the database takes + for c in all.as_array().unwrap() { + dev.ok_put(channels, json!({ "kind": c["kind"], "channel": "web", "enabled": true })).await; + } + + // an approval reaches the author: the bell and an email + push_branch(&srv, &work, "feature", "a.txt").await; + let mrs = format!("/api/v1/projects/{proj}/merge_requests"); + dev.ok_post(&mrs, json!({ "source_branch": "feature", "target_branch": "main", "title": "Add a" })).await; + rep.ok_post(&format!("{mrs}/1/approve"), json!({})).await; + wait_jobs(&srv.state).await; + let mine = dev.ok_get(inbox).await; + let reviews = of(&mine, "review"); + assert_eq!(reviews.len(), 1, "{mine}"); + assert_eq!(mine["unread"], 1, "{mine}"); + let n = &reviews[0]; + assert_eq!((n["summary"].as_str(), n["title"].as_str(), n["actor_username"].as_str()), (Some("approved"), Some("Add a"), Some("rita"))); + assert_eq!((n["target_type"].as_str(), n["target_iid"].as_i64(), n["read"].as_bool()), (Some("merge_request"), Some(1), Some(false))); + assert_eq!(mails_to(&srv, "dave", "rita approved merge request !1"), 1); + // approving again says nothing new + rep.ok_post(&format!("{mrs}/1/approve"), json!({})).await; + wait_jobs(&srv.state).await; + assert_eq!(of(&dev.ok_get(inbox).await, "review").len(), 1); + // not who approved, and nobody reads another's inbox + assert_eq!(rep.ok_get(inbox).await["unread"], 0); + let id = n["id"].as_i64().unwrap(); + assert_eq!(rep.post(&format!("{inbox}/{id}/read"), json!({})).await.0, 404); + + // read it; then all + assert_eq!(dev.ok_post(&format!("{inbox}/{id}/read"), json!({})).await["unread"], 0); + assert_eq!(dev.post(&format!("{inbox}/{id}/read"), json!({})).await.0, 200, "reading it again is not an error"); + assert_eq!(of(&dev.ok_get(inbox).await, "review")[0]["read"], true); + + // email off for reviews: the bell only; then the bell off too + let set = dev.ok_put(channels, json!({ "kind": "review", "channel": "email", "enabled": false })).await; + assert!(set.as_array().unwrap().iter().any(|c| c["kind"] == "review" && c["web"] == true && c["email"] == false), "{set}"); + rep.ok_post(&format!("{mrs}/1/request_changes"), json!({})).await; + wait_jobs(&srv.state).await; + assert_eq!(of(&dev.ok_get(inbox).await, "review").len(), 2); + assert_eq!(mails_to(&srv, "dave", "rita requested changes"), 0); + dev.ok_put(channels, json!({ "kind": "review", "channel": "web", "enabled": false })).await; + rep.ok_post(&format!("{mrs}/1/approve"), json!({})).await; + wait_jobs(&srv.state).await; + assert_eq!(of(&dev.ok_get(inbox).await, "review").len(), 2); + assert_eq!(mails_to(&srv, "dave", "rita approved merge request !1"), 1); + + // the other kinds are untouched: a comment is activity, a mention a mention + owner.ok_post(&format!("{mrs}/1/notes"), json!({ "body": "Looks fine @dave" })).await; + wait_jobs(&srv.state).await; + let mine = dev.ok_get(inbox).await; + assert_eq!(of(&mine, "mention")[0]["summary"], "mentioned you", "{mine}"); + assert_eq!(mails_to(&srv, "dave", "mentioned you"), 1); + assert_eq!(dev.ok_post(&format!("{inbox}/read"), json!({})).await["unread"], 0); + + // a pipeline that fails tells who started it and the author of its merge request + std::fs::write(work.join(".gitlab-ci.yml"), "job:\n script: x\n secrets:\n DB: {vault: db/pw}\n").unwrap(); + must(git(&work, &["add", "."]).await); + must(git(&work, &["commit", "-qm", "ci"]).await); + must(git(&work, &["push", "-q", &srv.http_url("olivia", REPO), "feature"]).await); + wait_jobs(&srv.state).await; + for who in [&owner, &dev] { + let got = who.ok_get(inbox).await; + let failed = of(&got, "pipeline_failed"); + // the branch's pipeline and the merge request's: a broken config fails both + assert_eq!(failed.len(), 2, "{got}"); + assert_eq!((failed[0]["target_type"].as_str(), failed[0]["summary"].as_str()), (Some("pipeline"), Some("failed")), "{got}"); + assert!(failed[0]["title"].as_str().unwrap().ends_with("on feature"), "{got}"); + } + assert_eq!(mails_to(&srv, "olivia", "failed on feature"), 2); + assert!(of(&rep.ok_get(inbox).await, "pipeline_failed").is_empty(), "rita has nothing to do with it"); + + // a level of disabled for the project: nothing, in the app either + dev.put(&format!("/api/v1/projects/{proj}/notification_settings"), json!({ "level": "disabled" })).await; + owner.ok_post(&format!("{mrs}/1/notes"), json!({ "body": "@dave again" })).await; + wait_jobs(&srv.state).await; + assert_eq!(of(&dev.ok_get(inbox).await, "mention").len(), 1); +} + #[sqlx::test(migrator = "forgust_db::MIGRATOR")] async fn instance_search_sees_only_what_it_may(pool: sqlx::PgPool) { let srv = TestServer::start(pool).await; diff --git a/web/e2e/everyday.spec.ts b/web/e2e/everyday.spec.ts index 3759d12..222932c 100644 --- a/web/e2e/everyday.spec.ts +++ b/web/e2e/everyday.spec.ts @@ -1,4 +1,4 @@ -import { test, expect, P, ADMIN, apiCall, csrfHeaders, freshProject, isMobile } from "./fixtures"; +import { test, expect, P, ADMIN, apiCall, at, csrfHeaders, freshProject, isMobile } from "./fixtures"; const MR = `${P}/-/merge_requests`; @@ -479,3 +479,39 @@ test("an unsubscribe link cannot point the page at another API route", async ({ await expect(page.getByRole("button", { name: /Stop emails/ })).toHaveCount(0); expect(posts).toEqual([]); }); + +test("the bell: a failed pipeline arrives live, and each kind has its switches", async ({ page }) => { + const csrf = await csrfHeaders(page); + await page.request.post("/api/v1/user/notifications/read", { headers: csrf }); + await page.goto("/"); + await expect(page.getByRole("button", { name: "Notifications", exact: true })).toBeVisible(); + // a configuration that does not load: the pipeline of the push is born failed + const fresh = await freshProject(page, "bell", { ".gitlab-ci.yml": "job:\n script: x\n secrets:\n DB: {vault: db/pw}\n" }); + try { + // no reload: the event stream brings it, and it shows on screen + await expect(page.locator(".toast.notice").last()).toContainText("failed: Pipeline #1 on main"); + await page.getByRole("button", { name: "Notifications: 1 unread" }).click(); + const item = page.locator(".inbox").getByRole("link", { name: /Pipeline #1 on main/ }); + await expect(item).toContainText("failed"); + await item.click(); + await at(page, `${fresh.web}/-/pipelines/1`); + // the click focused the link (its hover card is asked for) and the list closed + // under it: no card may show up in a corner afterwards + await page.waitForTimeout(700); + await expect(page.locator(".hover-card")).toHaveCount(0); + await expect(page.getByRole("button", { name: "Notifications", exact: true })).toBeVisible(); + + await page.goto("/profile/notifications"); + const email = page.getByRole("checkbox", { name: "Failed pipelines: email" }); + await expect(email).toBeChecked(); + await expect(page.getByRole("checkbox", { name: "Fixed pipelines: email" })).not.toBeChecked(); + await email.uncheck(); + await expect(email).not.toBeChecked(); + await page.reload(); + await expect(email).not.toBeChecked(); + await expect(page.getByRole("checkbox", { name: "Failed pipelines: in app" })).toBeChecked(); + } finally { + await page.request.put("/api/v1/user/notification_channels", { data: { kind: "pipeline_failed", channel: "email", enabled: true }, headers: csrf }); + await fresh.remove(); + } +}); diff --git a/web/openapi.json b/web/openapi.json index e990c43..11108f0 100644 --- a/web/openapi.json +++ b/web/openapi.json @@ -4067,6 +4067,60 @@ } } }, + "/api/v1/user/notification_channels": { + "get": { + "tags": [ + "user" + ], + "summary": "Where each kind of notification goes for the caller.", + "operationId": "channels", + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ChannelDto" + } + } + } + } + } + } + }, + "put": { + "tags": [ + "user" + ], + "summary": "Turn one kind on or off for one channel.", + "operationId": "set_channel", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ChannelBody" + } + } + }, + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ChannelDto" + } + } + } + } + } + } + } + }, "/api/v1/user/notification_settings": { "get": { "tags": [ @@ -4113,6 +4167,26 @@ } } }, + "/api/v1/user/notifications": { + "get": { + "tags": [ + "user" + ], + "summary": "The caller's notifications in the app.", + "operationId": "inbox", + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Inbox" + } + } + } + } + } + } + }, "/api/v1/user/projects": { "get": { "tags": [ @@ -5578,6 +5652,60 @@ } } }, + "Channel": { + "type": "string", + "description": "Where a notification goes.", + "enum": [ + "web", + "email", + "screen" + ] + }, + "ChannelBody": { + "type": "object", + "required": [ + "kind", + "channel", + "enabled" + ], + "properties": { + "channel": { + "$ref": "#/components/schemas/Channel" + }, + "enabled": { + "type": "boolean" + }, + "kind": { + "$ref": "#/components/schemas/Kind" + } + } + }, + "ChannelDto": { + "type": "object", + "description": "Where one kind of notification goes.", + "required": [ + "kind", + "web", + "email", + "screen" + ], + "properties": { + "email": { + "type": "boolean" + }, + "kind": { + "$ref": "#/components/schemas/Kind" + }, + "screen": { + "type": "boolean", + "description": "A message on screen as it arrives, while the app is open (only what\nreaches the bell)." + }, + "web": { + "type": "boolean", + "description": "The bell in the app." + } + } + }, "CiActivity": { "type": "object", "required": [ @@ -8186,6 +8314,27 @@ } } }, + "Inbox": { + "type": "object", + "required": [ + "unread", + "items" + ], + "properties": { + "items": { + "type": "array", + "items": { + "$ref": "#/components/schemas/Notification" + }, + "description": "Newest first." + }, + "unread": { + "type": "integer", + "format": "int64", + "description": "How many are unread (of everything, not only `items`)." + } + } + }, "InstanceCounts": { "type": "object", "required": [ @@ -8937,6 +9086,19 @@ } } }, + "Kind": { + "type": "string", + "description": "What a notification is about: each kind has its own switch per channel.\n(The `notification_kind` domain in the database lists the same names.)", + "enum": [ + "mention", + "assignment", + "activity", + "review", + "pipeline_failed", + "pipeline_fixed", + "security" + ] + }, "Label": { "type": "object", "required": [ @@ -10547,6 +10709,75 @@ } ] }, + "Notification": { + "type": "object", + "description": "A notification in someone's inbox.", + "required": [ + "id", + "kind", + "project_id", + "project_path", + "target_type", + "title", + "summary", + "read", + "created_at" + ], + "properties": { + "actor_name": { + "type": [ + "string", + "null" + ] + }, + "actor_username": { + "type": [ + "string", + "null" + ] + }, + "created_at": { + "type": "string", + "format": "date-time" + }, + "id": { + "type": "integer", + "format": "int64" + }, + "kind": { + "type": "string", + "description": "mention, assignment, activity, review, pipeline_failed, pipeline_fixed or security" + }, + "project_id": { + "type": "integer", + "format": "int64" + }, + "project_path": { + "type": "string" + }, + "read": { + "type": "boolean" + }, + "summary": { + "type": "string" + }, + "target_iid": { + "type": [ + "integer", + "null" + ], + "format": "int64", + "description": "Its number in the project (none for security)." + }, + "target_type": { + "type": "string", + "description": "merge_request, issue, pipeline or security (the project's findings)" + }, + "title": { + "type": "string" + } + } + }, "Options": { "type": "object", "description": "What an import brings besides the repository (all on by default).", diff --git a/web/src/api/schema.d.ts b/web/src/api/schema.d.ts index e5acd10..7729cb0 100644 --- a/web/src/api/schema.d.ts +++ b/web/src/api/schema.d.ts @@ -2240,6 +2240,24 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/user/notification_channels": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Where each kind of notification goes for the caller. */ + get: operations["channels"]; + /** Turn one kind on or off for one channel. */ + put: operations["set_channel"]; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/user/notification_settings": { parameters: { query?: never; @@ -2256,6 +2274,23 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/user/notifications": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** The caller's notifications in the app. */ + get: operations["inbox"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/user/projects": { parameters: { query?: never; @@ -2803,6 +2838,28 @@ export interface components { current?: string; new: string; }; + /** + * @description Where a notification goes. + * @enum {string} + */ + Channel: "web" | "email" | "screen"; + ChannelBody: { + channel: components["schemas"]["Channel"]; + enabled: boolean; + kind: components["schemas"]["Kind"]; + }; + /** @description Where one kind of notification goes. */ + ChannelDto: { + email: boolean; + kind: components["schemas"]["Kind"]; + /** + * @description A message on screen as it arrives, while the app is open (only what + * reaches the bell). + */ + screen: boolean; + /** @description The bell in the app. */ + web: boolean; + }; CiActivity: { /** @description Most pipelines in the last 30 days. */ busiest_projects: components["schemas"]["BusyProject"][]; @@ -3557,6 +3614,15 @@ export interface components { user_id?: number | null; username: string; }; + Inbox: { + /** @description Newest first. */ + items: components["schemas"]["Notification"][]; + /** + * Format: int64 + * @description How many are unread (of everything, not only `items`). + */ + unread: number; + }; InstanceCounts: { /** * Format: int64 @@ -3785,6 +3851,12 @@ export interface components { key: string; value: string; }; + /** + * @description What a notification is about: each kind has its own switch per channel. + * (The `notification_kind` domain in the database lists the same names.) + * @enum {string} + */ + Kind: "mention" | "assignment" | "activity" | "review" | "pipeline_failed" | "pipeline_fixed" | "security"; Label: { color: string; description: string; @@ -4255,6 +4327,30 @@ export interface components { */ version?: number | null; }; + /** @description A notification in someone's inbox. */ + Notification: { + actor_name?: string | null; + actor_username?: string | null; + /** Format: date-time */ + created_at: string; + /** Format: int64 */ + id: number; + /** @description mention, assignment, activity, review, pipeline_failed, pipeline_fixed or security */ + kind: string; + /** Format: int64 */ + project_id: number; + project_path: string; + read: boolean; + summary: string; + /** + * Format: int64 + * @description Its number in the project (none for security). + */ + target_iid?: number | null; + /** @description merge_request, issue, pipeline or security (the project's findings) */ + target_type: string; + title: string; + }; /** @description What an import brings besides the repository (all on by default). */ Options: { /** @default true */ @@ -9441,6 +9537,48 @@ export interface operations { }; }; }; + channels: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ChannelDto"][]; + }; + }; + }; + }; + set_channel: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["ChannelBody"]; + }; + }; + responses: { + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ChannelDto"][]; + }; + }; + }; + }; mine: { parameters: { query?: never; @@ -9483,6 +9621,25 @@ export interface operations { }; }; }; + inbox: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["Inbox"]; + }; + }; + }; + }; my_projects: { parameters: { query?: never; diff --git a/web/src/components/Inbox.tsx b/web/src/components/Inbox.tsx new file mode 100644 index 0000000..2a23d88 --- /dev/null +++ b/web/src/components/Inbox.tsx @@ -0,0 +1,137 @@ +import { useRef, useState } from "react"; +import { Link } from "react-router-dom"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { Bell, CircleAlert, CircleCheck, GitPullRequest, CircleDot, ShieldAlert } from "lucide-react"; +import { api, timeAgo, type S } from "../api/client"; +import { useUserEvents } from "../shell/live"; +import { useToast } from "../shell/toast"; +import { Popover } from "./controls"; + +type Item = S["Notification"]; + +function url(n: Item) { + const base = `/${n.project_path}/-`; + switch (n.target_type) { + case "issue": + return `${base}/issues/${n.target_iid}`; + case "merge_request": + return `${base}/merge_requests/${n.target_iid}`; + case "pipeline": + return `${base}/pipelines/${n.target_iid}`; + case "security": + return `${base}/security`; + default: + // a kind of target this build does not know yet: its project + return `/${n.project_path}`; + } +} + +function Icon({ n }: { n: Item }) { + if (n.kind === "pipeline_failed") return ; + if (n.kind === "pipeline_fixed") return ; + if (n.target_type === "security") return ; + if (n.target_type === "issue") return ; + return ; +} + +/** "!12 Fix the login page", "#3 Crash on start", "Pipeline #8 on main". */ +function title(n: Item) { + if (n.target_type === "merge_request") return `!${n.target_iid} ${n.title}`; + if (n.target_type === "issue") return `#${n.target_iid} ${n.title}`; + return n.title; +} + +/** "rita approved", or what happened on its own ("failed"). */ +function what(n: Item) { + const who = n.actor_name || n.actor_username; + return who ? `${who} ${n.summary}` : n.summary; +} + +/** The bell: what reached you in the app, live. */ +export function InboxBell() { + const qc = useQueryClient(); + const toast = useToast(); + const [open, setOpen] = useState(false); + // the event stream says when it changes; the interval covers a missed event + const q = useQuery({ queryKey: ["inbox"], queryFn: () => api.get("/user/notifications"), refetchInterval: 120_000 }); + // one action often tells the same person twice (assigned and mentioned): a + // burst of events, and a mark-read with its own event, refetch once + const refetch = useRef(undefined); + const refresh = () => { + clearTimeout(refetch.current); + refetch.current = window.setTimeout(() => qc.invalidateQueries({ queryKey: ["inbox"] }), 250); + }; + const unread = q.data?.unread ?? 0; + const items = q.data?.items ?? []; + const read = async (id?: number) => { + try { + await api.post(id ? `/user/notifications/${id}/read` : "/user/notifications/read"); + refresh(); + } catch (e) { + toast.error(e); + } + }; + // what arrives while the app is open also shows on screen, when the person + // kept that kind "on screen" (the server says so with the event) + useUserEvents((ev) => { + refresh(); + if (ev.type !== "notification" || !ev.screen) return; + const n = ev.notification; + toast.notice(`${what(n)}: ${title(n)}`, { to: url(n), onOpen: () => read(n.id) }); + }); + return ( + + + {unread > 0 && {unread > 99 ? "99+" : unread}} + + } + > +
+
+ Notifications + + {unread > 0 && ( + + )} + setOpen(false)}> + Settings + +
+ {items.length === 0 ? ( +

{q.isLoading ? "Loading…" : "Nothing yet. Reviews, mentions and pipelines of yours show up here."}

+ ) : ( +
    + {items.map((n) => ( +
  • + { + setOpen(false); + if (!n.read) read(n.id); + }} + > + + + {title(n)} + + {what(n)} · {n.project_path} · {timeAgo(n.created_at)} + + + {!n.read && } + +
    • + ))} +
+ )} +
+ + ); +} diff --git a/web/src/components/Notifications.tsx b/web/src/components/Notifications.tsx index b092474..947d1f3 100644 --- a/web/src/components/Notifications.tsx +++ b/web/src/components/Notifications.tsx @@ -3,13 +3,15 @@ import { useQuery, useQueryClient } from "@tanstack/react-query"; import { DropdownMenu } from "radix-ui"; import { Bell, BellOff, BellRing, Check } from "lucide-react"; import { api, type S } from "../api/client"; +import { useNotificationChannels } from "../prefetch"; import { useToast } from "../shell/toast"; +import { Section } from "./ui"; export const LEVELS: { value: string; label: string; hint: string }[] = [ { value: "watch", label: "Watch", hint: "Everything: new issues and merge requests, every comment and change" }, { value: "participating", label: "Participate", hint: "What you take part in, are assigned to or asked to review, and mentions" }, { value: "mention", label: "On mention", hint: "Only when someone @mentions you" }, - { value: "disabled", label: "Disabled", hint: "No email" }, + { value: "disabled", label: "Disabled", hint: "Nothing" }, ]; const label = (v: string) => LEVELS.find((l) => l.value === v)?.label ?? v; @@ -33,25 +35,109 @@ export function NotificationDefaults() { } }; return ( -
-
-

Email notifications

-

- Your default for every project. Choose otherwise for a project with the bell on its page. Watching by default covers the projects you are a - member of. -

-
-
- {LEVELS.map((l) => ( - - ))} +
+
+
+ {LEVELS.map((l) => ( + + ))} +
+
+ +
+ ); +} + +// (a kind the server adds must be named here: the type says so) +const KINDS: Record = { + mention: { label: "Mentions", hint: "Someone @mentions you" }, + assignment: { label: "Assignments and review requests", hint: "You are assigned, or asked to review" }, + activity: { label: "Activity", hint: "New issues and merge requests, comments, closing and merging" }, + review: { label: "Reviews of your merge requests", hint: "Approved, or changes requested" }, + pipeline_failed: { label: "Failed pipelines", hint: "Pipelines you started, and those of your merge requests" }, + pipeline_fixed: { label: "Fixed pipelines", hint: "A pipeline passes where the one before failed" }, + security: { label: "Security findings", hint: "What a scan found: vulnerable dependencies, secrets in the code" }, +}; + +type Channel = "web" | "email" | "screen"; + +/** Where each kind of notification goes: the bell in the app, a message on + * screen as it arrives, an email. */ +function NotificationChannels() { + const qc = useQueryClient(); + const toast = useToast(); + const key = ["notifications", "channels"]; + const q = useNotificationChannels(); + // shown at once; put back if the server refuses + const [picked, setPicked] = useState>({}); + const on = (c: S["ChannelDto"], channel: Channel) => picked[`${c.kind}:${channel}`] ?? c[channel]; + const set = async (kind: string, channel: Channel, enabled: boolean) => { + const id = `${kind}:${channel}`; + setPicked((p) => ({ ...p, [id]: enabled })); + try { + await api.put("/user/notification_channels", { kind, channel, enabled }); + } catch (e) { + toast.error(e); + } + // the list is read again (answers to quick changes may cross); then this pick + // goes, unless a newer one replaced it + await qc.invalidateQueries({ queryKey: key }); + setPicked(({ [id]: now, ...rest }) => (now === enabled ? rest : { ...rest, [id]: now })); + }; + return ( +
+
+ + + + + + + + + + {(q.data ?? []).map((c) => { + const k = KINDS[c.kind]; + const box = (channel: Channel, name: string, disabled = false) => ( + + ); + return ( + + + {box("web", "in app")} + {/* what does not reach the bell cannot show on screen */} + {box("screen", "on screen", !on(c, "web"))} + {box("email", "email")} + + ); + })} + +
+ In appOn screenEmail
+ set(c.kind, channel, e.target.checked)} + /> +
+ {k.label} + {k.hint} +
-
+
); } diff --git a/web/src/prefetch.ts b/web/src/prefetch.ts index 1e8ea8d..527f91d 100644 --- a/web/src/prefetch.ts +++ b/web/src/prefetch.ts @@ -11,6 +11,10 @@ export const useGroups = (enabled = true) => useQuery({ queryKey: ["groups"], qu /** "Your work": review requests, assignments, to-dos (the page and the sidebar badge). */ export const useWork = (enabled = true) => useQuery({ queryKey: ["work"], queryFn: () => api.get("/user/work"), enabled, staleTime: 15_000 }); +/** Where each kind of notification goes (the settings tab, and the bell for what shows on screen). */ +export const useNotificationChannels = () => + useQuery({ queryKey: ["notifications", "channels"], queryFn: () => api.get("/user/notification_channels"), staleTime: 60_000 }); + export const keys = { project: (id: number) => ["project", id] as const, pathResolve: (path: string) => ["resolve", path] as const, diff --git a/web/src/shell/Shell.tsx b/web/src/shell/Shell.tsx index dc35dae..95401ae 100644 --- a/web/src/shell/Shell.tsx +++ b/web/src/shell/Shell.tsx @@ -44,6 +44,7 @@ import { HoverCards } from "../components/HoverCards"; import { Avatar, Tile } from "../components/ui"; import { ResendLink } from "../components/ResendLink"; import { useWork } from "../prefetch"; +import { InboxBell } from "../components/Inbox"; /** A nav entry; `children` show under it while its section is active. */ type Item = { to: string; label: string; icon?: ReactNode; end?: boolean; count?: number; match?: RegExp; children?: Item[] }; @@ -459,6 +460,7 @@ export function Shell({ children }: { children: ReactNode }) { {IS_MAC ? "⌘K" : "Ctrl K"} + diff --git a/web/src/shell/live.ts b/web/src/shell/live.ts index a0c5fee..7a46940 100644 --- a/web/src/shell/live.ts +++ b/web/src/shell/live.ts @@ -1,10 +1,12 @@ import { useEffect, useRef } from "react"; import { useQueryClient } from "@tanstack/react-query"; +import type { S } from "../api/client"; /** A server event (see `events.rs`): `push`, `merge_request`, `pipeline`, `job`, `job_trace`, ... */ export type LiveEvent = { type: string; - project_id: number; + /** The project it happened in (none for the person's own events, see `UserEvent`). */ + project_id?: number; pipeline_id?: number; job_id?: number; iid?: number; @@ -12,11 +14,18 @@ export type LiveEvent = { trace_size?: number; }; +/** What only the signed-in person hears: a notification that reached their + * inbox (`screen`: they want it shown as it arrives), or their inbox changed + * otherwise (read in another tab). */ +export type UserEvent = { type: "notification"; screen: boolean; notification: S["Notification"] } | { type: "inbox" }; + type Listener = (ev: LiveEvent) => void; -// One SSE stream per tab, for every project the open views watch (browsers -// allow ~6 connections per site). It reconnects when the set changes. +// One SSE stream per tab, for every project the open views watch and the +// signed-in person's own events (browsers allow ~6 connections per site). It +// reconnects when the set changes. const listeners = new Map>(); +const userListeners = new Set<(ev: UserEvent) => void>(); let source: EventSource | null = null; let connectedFor = ""; let timer: number | null = null; @@ -24,17 +33,19 @@ let timer: number | null = null; function reconnect() { timer = null; const ids = [...listeners.entries()].filter(([, s]) => s.size > 0).map(([id]) => id).sort((a, b) => a - b); - const key = ids.join(","); + const projects = ids.join(","); + const key = `${userListeners.size ? "me" : ""}:${projects}`; if (key === connectedFor) return; source?.close(); source = null; connectedFor = key; - if (!ids.length || typeof EventSource === "undefined") return; - source = new EventSource(`/api/v1/events?projects=${key}`); + if ((!ids.length && !userListeners.size) || typeof EventSource === "undefined") return; + source = new EventSource(`/api/v1/events?projects=${projects}`); source.onmessage = (m) => { try { - const ev = JSON.parse(m.data) as LiveEvent; - listeners.get(ev.project_id)?.forEach((f) => f(ev)); + const ev = JSON.parse(m.data) as LiveEvent | UserEvent; + if (ev.type === "notification" || ev.type === "inbox") userListeners.forEach((f) => f(ev as UserEvent)); + else if ((ev as LiveEvent).project_id != null) listeners.get((ev as LiveEvent).project_id!)?.forEach((f) => f(ev as LiveEvent)); } catch { /* keep-alive or malformed */ } @@ -62,6 +73,21 @@ export function useProjectEvents(projectId: number | undefined, handler: Listene }, [projectId]); } +/** Call `handler` for what reaches the signed-in person (their inbox changed). */ +export function useUserEvents(handler: (ev: UserEvent) => void) { + const ref = useRef(handler); + ref.current = handler; + useEffect(() => { + const f = (ev: UserEvent) => ref.current(ev); + userListeners.add(f); + schedule(); + return () => { + userListeners.delete(f); + schedule(); + }; + }, []); +} + /** * Refetch this project's CI views when a pipeline or job changes state * (bursts are coalesced; log growth is `job_trace` and only log viewers read it). diff --git a/web/src/shell/toast.tsx b/web/src/shell/toast.tsx index 995ca06..96c760f 100644 --- a/web/src/shell/toast.tsx +++ b/web/src/shell/toast.tsx @@ -1,33 +1,59 @@ import { createContext, useCallback, useContext, useState, type ReactNode } from "react"; import { createPortal } from "react-dom"; -import { CheckCircle2, CircleAlert, X } from "lucide-react"; +import { Link } from "react-router-dom"; +import { Bell, CheckCircle2, CircleAlert, X } from "lucide-react"; import { ApiError } from "../api/client"; -type Toast = { id: number; kind: "ok" | "error"; text: string }; +/** Where a notice leads, and what opening it does (marks it read). */ +type Open = { to: string; onOpen?: () => void }; +type Toast = { id: number; kind: "ok" | "error" | "notice"; text: string; open?: Open }; -const Ctx = createContext<{ ok: (t: string) => void; error: (e: unknown) => void }>({ ok: () => {}, error: () => {} }); +const Ctx = createContext<{ ok: (t: string) => void; error: (e: unknown) => void; notice: (t: string, open?: Open) => void }>({ + ok: () => {}, + error: () => {}, + notice: () => {}, +}); let seq = 0; export function ToastProvider({ children }: { children: ReactNode }) { const [toasts, setToasts] = useState([]); - const push = useCallback((kind: Toast["kind"], text: string) => { - const id = ++seq; - setToasts((t) => [...t.slice(-3), { id, kind, text }]); - setTimeout(() => setToasts((t) => t.filter((x) => x.id !== id)), kind === "ok" ? 3500 : 7000); - }, []); + const dismiss = useCallback((id: number) => setToasts((t) => t.filter((x) => x.id !== id)), []); + const push = useCallback( + (kind: Toast["kind"], text: string, open?: Open) => { + const id = ++seq; + setToasts((t) => [...t.slice(-3), { id, kind, text, open }]); + setTimeout(() => dismiss(id), kind === "ok" ? 3500 : 7000); + }, + [dismiss], + ); + // something that reached the person (the inbox): it leads where it happened + const notice = useCallback((t: string, open?: Open) => push("notice", t, open), [push]); const ok = useCallback((t: string) => push("ok", t), [push]); const error = useCallback((e: unknown) => push("error", e instanceof ApiError ? e.message : String(e)), [push]); return ( - + {children} {createPortal(
{toasts.map((t) => (
- {t.kind === "ok" ? : } - {t.text} -
diff --git a/web/src/styles.css b/web/src/styles.css index 79f7029..8e58353 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -288,12 +288,12 @@ a:focus-visible { outline: 2px solid var(--accent); outline-offset: 1px; } -input[type="checkbox"] { - width: auto; - height: auto; -} input[type="checkbox"], input[type="radio"] { + flex: none; + width: auto; + height: auto; + padding: 0; accent-color: var(--accent); } @@ -498,6 +498,13 @@ label.check { align-items: center; gap: 8px; } +/* a choice whose text may wrap: the box stays on the first line */ +label.check.top { + align-items: flex-start; +} +label.check.top > input { + margin-top: 4px; +} form.stack { gap: 14px; } @@ -1559,6 +1566,13 @@ kbd { .toast.error { border-color: var(--danger); } +.toast.notice > svg:first-child { + flex: none; + color: var(--accent); +} +.toast-link { + color: var(--text); +} .toast.error > svg:first-child { color: var(--danger); } @@ -4935,6 +4949,121 @@ a.item.import-row:hover { display: block; white-space: normal; } +.notif-channels { + width: 100%; + border-collapse: collapse; +} +.notif-channels th, +.notif-channels td { + padding: 8px 0; + border-top: 1px solid var(--border); + text-align: left; + font-weight: 400; + vertical-align: top; +} +.notif-channels thead th { + border-top: 0; + padding-top: 0; + color: var(--text-muted); + font-size: 12.5px; +} +.notif-channels th + th, +.notif-channels td { + width: 72px; + text-align: center; +} +.notif-channels tbody tr:first-child th, +.notif-channels tbody tr:first-child td { + border-top-color: var(--border-strong); +} +.notif-channels td input { + margin-top: 4px; +} + +/* ---------------------------------------------------------------- inbox (the bell) */ +.inbox-btn { + position: relative; +} +.inbox-count { + position: absolute; + top: 1px; + right: 0; + min-width: 16px; + height: 16px; + padding: 0 4px; + border-radius: 8px; + background: var(--accent); + color: #fff; + font-size: 10.5px; + font-weight: 600; + line-height: 16px; + text-align: center; +} +.inbox { + width: min(380px, calc(100vw - 48px)); + margin: -12px; +} +.inbox-head { + display: flex; + align-items: center; + gap: 12px; + padding: 10px 12px; + border-bottom: 1px solid var(--border); +} +.inbox-empty { + margin: 0; + padding: 20px 12px; + text-align: center; +} +.inbox-list { + max-height: min(440px, 70vh); + margin: 0; + padding: 4px; + overflow-y: auto; + list-style: none; +} +.inbox-item { + display: flex; + align-items: flex-start; + gap: 10px; + padding: 8px; + border-radius: 6px; + color: var(--text); + text-decoration: none; +} +.inbox-item:hover { + background: var(--bg-hover); + text-decoration: none; +} +.inbox-icon { + flex: none; + margin-top: 2px; + color: var(--text-muted); +} +.inbox-icon.bad { + color: var(--danger); +} +.inbox-icon.ok { + color: var(--ok); +} +.inbox-text { + display: flex; + flex-direction: column; + flex: 1; + min-width: 0; + gap: 2px; +} +.inbox-item.unread .inbox-title { + font-weight: 600; +} +.inbox-dot { + flex: none; + width: 8px; + height: 8px; + margin-top: 6px; + border-radius: 50%; + background: var(--accent); +} /* ---------------------------------------------------------------- test reports (M7) */ .test-report { From b6469ab3fad042a7479b6ef986c5277e0e12ce6e Mon Sep 17 00:00:00 2001 From: d3vyce <44915747+d3vyce@users.noreply.github.com> Date: Sat, 14 Nov 2026 07:44:51 -0500 Subject: [PATCH 2/2] fix: tooltips and hover cards that stayed on screen after a click Signed-off-by: d3vyce <44915747+d3vyce@users.noreply.github.com> --- web/e2e/shell.spec.ts | 24 ++++++++++++++++- web/src/components/HoverCards.tsx | 44 ++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/web/e2e/shell.spec.ts b/web/e2e/shell.spec.ts index d04e4bb..acc49c1 100644 --- a/web/e2e/shell.spec.ts +++ b/web/e2e/shell.spec.ts @@ -1,4 +1,4 @@ -import { test, expect, P, at, isMobile } from "./fixtures"; +import { test, expect, P, ADMIN, at, isMobile } from "./fixtures"; test("dashboard → project through the sidebar and breadcrumbs", async ({ page }) => { await page.goto("/dashboard/projects"); @@ -149,3 +149,25 @@ test("search issues and merge requests across projects", async ({ page }) => { await page.getByRole("button", { name: "Merged" }).click(); await expect(page.getByText("Nothing matches")).toBeVisible(); }); + +test("tips go with a click, and do not come back when a popover closes", async ({ page }) => { + test.skip(isMobile(page), "hover needs a pointer"); + await page.goto("/"); + const tip = page.locator(".tip"); + // the avatar's tip: shown on hover, gone once the menu is open + await page.locator(".avatar-btn").hover(); + await expect(tip).toHaveText(ADMIN); + await page.locator(".avatar-btn").click(); + // (the click focuses the button: a tip asked for by that focus would show after its delay) + await page.waitForTimeout(600); + await expect(tip).toHaveCount(0); + await page.mouse.click(500, 500); + // the bell: closing its list hands the focus back to the button, the pointer + // being elsewhere: no tip may stay there + await page.locator(".inbox-btn").click(); + await expect(page.locator(".inbox")).toBeVisible(); + await page.mouse.click(500, 500); + await expect(page.locator(".inbox")).toHaveCount(0); + await page.waitForTimeout(600); + await expect(tip).toHaveCount(0); +}); diff --git a/web/src/components/HoverCards.tsx b/web/src/components/HoverCards.tsx index e70d3de..55ee74d 100644 --- a/web/src/components/HoverCards.tsx +++ b/web/src/components/HoverCards.tsx @@ -65,6 +65,25 @@ function Tip({ text, rect }: { text: string; rect: DOMRect }) { ); } +/** + * Focus asks for a tip or a card only when it came from the keyboard: a click + * focuses what it hits, and a menu or popover that closes hands the focus back + * to its button, with the pointer elsewhere and nothing to end the tip. + */ +function byKeyboard(e: Event, el: Element) { + return e.type !== "focusin" || el.matches(":focus-visible"); +} + +/** + * Whether the element still asks for its tip or card when the delay ends: on + * the page, and under the pointer or focused from the keyboard. Not one that + * went away meanwhile (a click that navigated, a menu that closed): its tip + * would show in a corner with nothing to end it. + */ +function wanted(el: Element) { + return el.isConnected && el.matches(":hover, :focus-visible"); +} + /** * Plain tooltips: an element's `title` shows in a styled tip instead of the * browser's, after a short delay. The title moves to `data-title` (and to @@ -79,6 +98,7 @@ function useTips() { const el = (e.target as Element | null)?.closest?.("[title], [data-title]") as HTMLElement | SVGElement | null; if (!el || el.closest(".hover-card")) return; if (el instanceof HTMLAnchorElement && targetOf(el)) return; + if (!byKeyboard(e, el)) return; const title = el.getAttribute("title"); if (title) { el.setAttribute("data-title", title); @@ -88,7 +108,7 @@ function useTips() { const text = el.getAttribute("data-title"); if (!text) return; clearTimeout(t.current); - t.current = window.setTimeout(() => setTip({ text, rect: el.getBoundingClientRect() }), TIP_MS); + t.current = window.setTimeout(() => wanted(el) && setTip({ text, rect: el.getBoundingClientRect() }), TIP_MS); }; const leave = (e: Event) => { const el = (e.target as Element | null)?.closest?.("[data-title]"); @@ -108,9 +128,13 @@ function useTips() { document.addEventListener("focusin", enter); document.addEventListener("focusout", leave); document.addEventListener("mousedown", close); + // a click focuses what it hits (a tip is asked for again) and may remove it + // (a link that navigates): no `mouseout` or `focusout` follows, so it ends here + document.addEventListener("click", close, true); window.addEventListener("scroll", close, true); window.addEventListener("popstate", close); return () => { + document.removeEventListener("click", close, true); document.removeEventListener("mouseover", enter); document.removeEventListener("mouseout", leave); document.removeEventListener("focusin", enter); @@ -131,7 +155,7 @@ function useTips() { */ export function HoverCards() { const tip = useTips(); - const [card, setCard] = useState<{ target: Target; rect: DOMRect; anchor: HTMLAnchorElement } | null>(null); + const [card, setCard] = useState<{ target: Target; rect: DOMRect } | null>(null); const showT = useRef(undefined); const hideT = useRef(undefined); const over = useRef(false); @@ -139,7 +163,7 @@ export function HoverCards() { useEffect(() => { const enter = (e: Event) => { const a = (e.target as Element | null)?.closest?.(SELECTOR) as HTMLAnchorElement | null; - if (!a) return; + if (!a || !byKeyboard(e, a)) return; const target = targetOf(a); if (!target) return; clearTimeout(hideT.current); @@ -149,7 +173,7 @@ export function HoverCards() { a.dataset.title = a.title; a.removeAttribute("title"); } - showT.current = window.setTimeout(() => setCard({ target, rect: a.getBoundingClientRect(), anchor: a }), SHOW_MS); + showT.current = window.setTimeout(() => wanted(a) && setCard({ target, rect: a.getBoundingClientRect() }), SHOW_MS); }; const leave = (e: Event) => { const a = (e.target as Element | null)?.closest?.(SELECTOR); @@ -171,9 +195,13 @@ export function HoverCards() { document.addEventListener("focusin", enter); document.addEventListener("focusout", leave); document.addEventListener("keydown", key); + // a click navigates (and focuses the link, which asks for its card again): + // the card goes, shown or about to be, wherever the link ends up + document.addEventListener("click", close, true); window.addEventListener("scroll", scrolled, true); window.addEventListener("popstate", close); return () => { + document.removeEventListener("click", close, true); document.removeEventListener("mouseover", enter); document.removeEventListener("mouseout", leave); document.removeEventListener("focusin", enter); @@ -184,14 +212,6 @@ export function HoverCards() { }; }, []); - // a click navigates: the card goes with it - useEffect(() => { - if (!card) return; - const click = () => setCard(null); - card.anchor.addEventListener("click", click); - return () => card.anchor.removeEventListener("click", click); - }, [card]); - if (!card) return tip && ; const width = 340; const left = Math.max(8, Math.min(card.rect.left, window.innerWidth - width - 8));