From 38ac1adac70d6ab4038565816361a88dafcee34c Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 10 Sep 2026 14:27:05 +0100 Subject: [PATCH 01/10] Fixed Admin locale data type resolution ref https://github.com/TryGhost/Ghost/pull/30635 Admin lint and typecheck could not resolve the locale JSON export after the i18n migration, producing cascading unsafe-value errors. Resolve its types from the checked-in JSON and enable JSON modules in Admin so validation does not require generated i18n output. --- apps/admin/tsconfig.app.json | 1 + packages/i18n/package.json | 1 + 2 files changed, 2 insertions(+) diff --git a/apps/admin/tsconfig.app.json b/apps/admin/tsconfig.app.json index 976d1153363..1531adba243 100644 --- a/apps/admin/tsconfig.app.json +++ b/apps/admin/tsconfig.app.json @@ -10,6 +10,7 @@ /* Bundler mode */ "moduleResolution": "bundler", + "resolveJsonModule": true, "allowImportingTsExtensions": true, "verbatimModuleSyntax": true, "moduleDetection": "force", diff --git a/packages/i18n/package.json b/packages/i18n/package.json index 91acbe28795..00502f33a42 100644 --- a/packages/i18n/package.json +++ b/packages/i18n/package.json @@ -44,6 +44,7 @@ }, "./locale-data.json": { "source": "./src/locale-data.json", + "types": "./src/locale-data.json", "default": "./build/locale-data.json" }, "./package.json": "./package.json" From 918c250d2825ceaa5bb131f61ccf0f6596f05f7f Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Thu, 10 Sep 2026 08:53:48 -0500 Subject: [PATCH 02/10] Updated database for upcoming Tinybird automations sync (#30567) towards https://linear.app/ghost/issue/NY-1559 These migrations: - Add a `tinybird_syncs` table - Index `updated_at` on `automation_runs` and `automation_run_steps` Will be useful soon. --------- Co-authored-by: Troy Ciesco --- apps/ember-admin/package.json | 2 +- ghost/core/core/server/data/exporter/table-lists.js | 1 + ...13-56-18-add-automation-run-updated-at-indexes.js | 6 ++++++ .../2026-09-09-13-56-18-add-tinybird-syncs-table.js | 10 ++++++++++ ghost/core/core/server/data/schema/schema.js | 12 ++++++++++-- .../core/test/integration/exporter/exporter.test.js | 1 + .../test/unit/server/data/schema/integrity.test.js | 2 +- 7 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 ghost/core/core/server/data/migrations/versions/6.64/2026-09-09-13-56-18-add-automation-run-updated-at-indexes.js create mode 100644 ghost/core/core/server/data/migrations/versions/6.64/2026-09-09-13-56-18-add-tinybird-syncs-table.js diff --git a/apps/ember-admin/package.json b/apps/ember-admin/package.json index 2cd8133cfe0..f46aedd2437 100644 --- a/apps/ember-admin/package.json +++ b/apps/ember-admin/package.json @@ -1,6 +1,6 @@ { "name": "ghost-admin", - "version": "6.63.1-rc.0", + "version": "6.64.0-rc.0", "description": "Ember.js admin client for Ghost", "author": "Ghost Foundation", "homepage": "http://ghost.org", diff --git a/ghost/core/core/server/data/exporter/table-lists.js b/ghost/core/core/server/data/exporter/table-lists.js index 21466ed1514..e3aadf3b92a 100644 --- a/ghost/core/core/server/data/exporter/table-lists.js +++ b/ghost/core/core/server/data/exporter/table-lists.js @@ -76,6 +76,7 @@ const BACKUP_TABLES = [ 'automation_runs', 'welcome_email_automation_runs', 'welcome_email_automated_emails', + 'tinybird_syncs', ]; // NOTE: exposing only tables which are going to be included in a "default" export file diff --git a/ghost/core/core/server/data/migrations/versions/6.64/2026-09-09-13-56-18-add-automation-run-updated-at-indexes.js b/ghost/core/core/server/data/migrations/versions/6.64/2026-09-09-13-56-18-add-automation-run-updated-at-indexes.js new file mode 100644 index 00000000000..999e1b042ca --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/6.64/2026-09-09-13-56-18-add-automation-run-updated-at-indexes.js @@ -0,0 +1,6 @@ +const { combineTransactionalMigrations, createAddIndexMigration } = require('../../utils'); + +module.exports = combineTransactionalMigrations( + createAddIndexMigration('automation_runs', ['updated_at']), + createAddIndexMigration('automation_run_steps', ['updated_at']), +); diff --git a/ghost/core/core/server/data/migrations/versions/6.64/2026-09-09-13-56-18-add-tinybird-syncs-table.js b/ghost/core/core/server/data/migrations/versions/6.64/2026-09-09-13-56-18-add-tinybird-syncs-table.js new file mode 100644 index 00000000000..7595d62532d --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/6.64/2026-09-09-13-56-18-add-tinybird-syncs-table.js @@ -0,0 +1,10 @@ +const { addTable } = require('../../utils'); + +module.exports = addTable('tinybird_syncs', { + id: { type: 'string', maxlength: 24, nullable: false, primary: true }, + table_name: { type: 'string', maxlength: 191, nullable: false, unique: true }, + last_synced_updated_at: { type: 'dateTime', nullable: false }, + last_synced_id: { type: 'string', maxlength: 24, nullable: false }, + created_at: { type: 'dateTime', nullable: false }, + updated_at: { type: 'dateTime', nullable: true }, +}); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index cabfcc2e27c..4242135c08b 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -2270,7 +2270,7 @@ module.exports = { nullable: false, validations: { isEmail: true }, }, - '@@INDEXES@@': [['automation_id', 'created_at']], + '@@INDEXES@@': [['automation_id', 'created_at'], ['updated_at']], }, automation_run_steps: { id: { type: 'string', maxlength: 24, nullable: false, primary: true }, @@ -2314,7 +2314,7 @@ module.exports = { }, locked_by: { type: 'string', maxlength: 191, nullable: true }, locked_at: { type: 'dateTime', nullable: true }, - '@@INDEXES@@': [['status', 'ready_at', 'created_at', 'id']], + '@@INDEXES@@': [['status', 'ready_at', 'created_at', 'id'], ['updated_at']], }, welcome_email_automated_emails: { id: { type: 'string', maxlength: 24, nullable: false, primary: true }, @@ -2577,4 +2577,12 @@ module.exports = { { columns: ['email_provider_message_id'], length: 31 }, ], }, + tinybird_syncs: { + id: { type: 'string', maxlength: 24, nullable: false, primary: true }, + table_name: { type: 'string', maxlength: 191, nullable: false, unique: true }, + last_synced_updated_at: { type: 'dateTime', nullable: false }, + last_synced_id: { type: 'string', maxlength: 24, nullable: false }, + created_at: { type: 'dateTime', nullable: false }, + updated_at: { type: 'dateTime', nullable: true }, + }, }; diff --git a/ghost/core/test/integration/exporter/exporter.test.js b/ghost/core/test/integration/exporter/exporter.test.js index d5e187fd0fb..51570fa7706 100644 --- a/ghost/core/test/integration/exporter/exporter.test.js +++ b/ghost/core/test/integration/exporter/exporter.test.js @@ -114,6 +114,7 @@ describe('Exporter', function () { 'subscriptions', 'suppressions', 'tags', + 'tinybird_syncs', 'tokens', 'users', 'webhooks', diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index 61d26d2ec87..7530b75762c 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -37,7 +37,7 @@ const parseYaml = require('../../../../../core/server/services/route-settings/ya */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = '83816146af992ec6a8df98956c3e6bd9'; + const currentSchemaHash = 'f8167b5e21aac21f007f008e6e60a332'; const currentFixturesHash = '5718e0d4eb037f159c312369e949829a'; const currentSettingsHash = '6ea42a00cca61a1ba87f66eb6e25a78a'; const currentRoutesHash = 'd8c25fa01bf6d22a2bcb05ba0de70dc1'; From 72730ac2fa523d04d9296d229a6b7fbd81d8b454 Mon Sep 17 00:00:00 2001 From: Evan Hahn Date: Thu, 10 Sep 2026 08:53:48 -0500 Subject: [PATCH 03/10] Added Tinybird datasources, MVs, and pipes for automation analytics (#30623) closes https://linear.app/ghost/issue/NY-1557 Co-Authored-By: Chris Raible Co-Authored-By: Troy Ciesco Co-authored-by: Chris Raible Co-authored-by: Troy Ciesco --- .../_mv_automation_run_steps.datasource | 10 +++++ .../_mv_automation_runs.datasource | 10 +++++ .../_mv_pending_automation_runs.datasource | 10 +++++ .../automation_run_events.datasource | 12 ++++++ .../automation_run_step_events.datasource | 12 ++++++ .../api_automation_browse_stats.pipe | 43 +++++++++++++++++++ .../fixtures/automation_run_events.ndjson | 7 +++ .../automation_run_step_events.ndjson | 8 ++++ .../pipes/mv_automation_run_steps.pipe | 13 ++++++ .../tinybird/pipes/mv_automation_runs.pipe | 15 +++++++ .../pipes/mv_pending_automation_runs.pipe | 7 +++ .../tests/api_automation_browse_stats.yaml | 18 ++++++++ .../services/tinybird/tinybird-service.js | 1 + 13 files changed, 166 insertions(+) create mode 100644 ghost/core/core/server/data/tinybird/datasources/_mv_automation_run_steps.datasource create mode 100644 ghost/core/core/server/data/tinybird/datasources/_mv_automation_runs.datasource create mode 100644 ghost/core/core/server/data/tinybird/datasources/_mv_pending_automation_runs.datasource create mode 100644 ghost/core/core/server/data/tinybird/datasources/automation_run_events.datasource create mode 100644 ghost/core/core/server/data/tinybird/datasources/automation_run_step_events.datasource create mode 100644 ghost/core/core/server/data/tinybird/endpoints/api_automation_browse_stats.pipe create mode 100644 ghost/core/core/server/data/tinybird/fixtures/automation_run_events.ndjson create mode 100644 ghost/core/core/server/data/tinybird/fixtures/automation_run_step_events.ndjson create mode 100644 ghost/core/core/server/data/tinybird/pipes/mv_automation_run_steps.pipe create mode 100644 ghost/core/core/server/data/tinybird/pipes/mv_automation_runs.pipe create mode 100644 ghost/core/core/server/data/tinybird/pipes/mv_pending_automation_runs.pipe create mode 100644 ghost/core/core/server/data/tinybird/tests/api_automation_browse_stats.yaml diff --git a/ghost/core/core/server/data/tinybird/datasources/_mv_automation_run_steps.datasource b/ghost/core/core/server/data/tinybird/datasources/_mv_automation_run_steps.datasource new file mode 100644 index 00000000000..f393cc6140f --- /dev/null +++ b/ghost/core/core/server/data/tinybird/datasources/_mv_automation_run_steps.datasource @@ -0,0 +1,10 @@ +SCHEMA > + `site_uuid` UUID, + `id` String, + `automation_run_id` String, + `status` LowCardinality(String), + `updated_at` DateTime64(3) + +ENGINE ReplacingMergeTree +ENGINE_SORTING_KEY site_uuid, id +ENGINE_VER updated_at diff --git a/ghost/core/core/server/data/tinybird/datasources/_mv_automation_runs.datasource b/ghost/core/core/server/data/tinybird/datasources/_mv_automation_runs.datasource new file mode 100644 index 00000000000..265244e7aaa --- /dev/null +++ b/ghost/core/core/server/data/tinybird/datasources/_mv_automation_runs.datasource @@ -0,0 +1,10 @@ +SCHEMA > + `site_uuid` UUID, + `id` String, + `automation_id` String, + `created_at` DateTime64(3), + `updated_at` DateTime64(3) + +ENGINE ReplacingMergeTree +ENGINE_SORTING_KEY site_uuid, id +ENGINE_VER updated_at diff --git a/ghost/core/core/server/data/tinybird/datasources/_mv_pending_automation_runs.datasource b/ghost/core/core/server/data/tinybird/datasources/_mv_pending_automation_runs.datasource new file mode 100644 index 00000000000..09a6917d0df --- /dev/null +++ b/ghost/core/core/server/data/tinybird/datasources/_mv_pending_automation_runs.datasource @@ -0,0 +1,10 @@ +SCHEMA > + `site_uuid` UUID, + `id` String, + `automation_run_id` String, + `is_pending` UInt8, + `updated_at` DateTime64(3) + +ENGINE ReplacingMergeTree +ENGINE_SORTING_KEY site_uuid, id +ENGINE_VER updated_at diff --git a/ghost/core/core/server/data/tinybird/datasources/automation_run_events.datasource b/ghost/core/core/server/data/tinybird/datasources/automation_run_events.datasource new file mode 100644 index 00000000000..6bbe98e578d --- /dev/null +++ b/ghost/core/core/server/data/tinybird/datasources/automation_run_events.datasource @@ -0,0 +1,12 @@ +TOKEN "tracker" APPEND +TOKEN "analytics-service" APPEND + +SCHEMA > + `site_uuid` UUID `json:$.site_uuid`, + `id` String `json:$.id`, + `updated_at` DateTime64(3) `json:$.updated_at`, + `inserted_at` DateTime64(3) `json:$.inserted_at` DEFAULT now64(), + `payload` String `json:$.payload` + +ENGINE MergeTree +ENGINE_SORTING_KEY site_uuid, id diff --git a/ghost/core/core/server/data/tinybird/datasources/automation_run_step_events.datasource b/ghost/core/core/server/data/tinybird/datasources/automation_run_step_events.datasource new file mode 100644 index 00000000000..6bbe98e578d --- /dev/null +++ b/ghost/core/core/server/data/tinybird/datasources/automation_run_step_events.datasource @@ -0,0 +1,12 @@ +TOKEN "tracker" APPEND +TOKEN "analytics-service" APPEND + +SCHEMA > + `site_uuid` UUID `json:$.site_uuid`, + `id` String `json:$.id`, + `updated_at` DateTime64(3) `json:$.updated_at`, + `inserted_at` DateTime64(3) `json:$.inserted_at` DEFAULT now64(), + `payload` String `json:$.payload` + +ENGINE MergeTree +ENGINE_SORTING_KEY site_uuid, id diff --git a/ghost/core/core/server/data/tinybird/endpoints/api_automation_browse_stats.pipe b/ghost/core/core/server/data/tinybird/endpoints/api_automation_browse_stats.pipe new file mode 100644 index 00000000000..d2a0efe228f --- /dev/null +++ b/ghost/core/core/server/data/tinybird/endpoints/api_automation_browse_stats.pipe @@ -0,0 +1,43 @@ +NODE automation_browse_stats +SQL > + % + SELECT + runs.automation_id AS automation_id, + formatDateTimeInJodaSyntax( + max(runs.created_at), + 'yyyy-MM-dd''T''HH:mm:ss.SSS''Z''', + 'UTC' + ) AS last_run_created_at, + count() AS total_run_count, + countIf( + runs.id IN ( + SELECT automation_run_id + FROM _mv_pending_automation_runs FINAL + WHERE + site_uuid + = {{ + String( + site_uuid, + '00000000-0000-0000-0000-000000000000', + description="Site UUID", + required=True, + ) + }} + AND is_pending = 1 + ) + ) AS in_progress_run_count + FROM _mv_automation_runs AS runs FINAL + WHERE + runs.site_uuid + = {{ + String( + site_uuid, + '00000000-0000-0000-0000-000000000000', + description="Site UUID", + required=True, + ) + }} + GROUP BY runs.automation_id + ORDER BY runs.automation_id + +TYPE ENDPOINT diff --git a/ghost/core/core/server/data/tinybird/fixtures/automation_run_events.ndjson b/ghost/core/core/server/data/tinybird/fixtures/automation_run_events.ndjson new file mode 100644 index 00000000000..34ef4a08d74 --- /dev/null +++ b/ghost/core/core/server/data/tinybird/fixtures/automation_run_events.ndjson @@ -0,0 +1,7 @@ +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-0","updated_at":"2020-01-01T01:00:00.000Z","inserted_at":"2020-01-01T02:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-0","automation_id":"automation-1","created_at":"2020-01-01T01:00:00.000Z","updated_at":"2020-01-01T01:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-1","updated_at":"2020-01-01T01:00:00.000Z","inserted_at":"2020-01-01T02:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-1","automation_id":"automation-1","created_at":"2020-01-01T01:00:00.000Z","updated_at":"2020-01-01T01:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-2","updated_at":"2020-01-01T01:00:00.000Z","inserted_at":"2020-01-01T02:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-2","automation_id":"automation-1","created_at":"2020-01-01T01:00:00.000Z","updated_at":"2020-01-01T01:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-3","updated_at":"2020-02-01T01:00:00.000Z","inserted_at":"2020-02-01T02:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-3","automation_id":"automation-1","created_at":"2020-02-01T01:00:00.123Z","updated_at":"2020-02-01T01:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-3","updated_at":"2020-01-01T01:00:00.000Z","inserted_at":"2020-02-01T03:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-3","automation_id":"automation-1","created_at":"2029-12-31T01:00:00.000Z","updated_at":"2020-01-01T01:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-4","updated_at":"2020-01-01T01:00:00.000Z","inserted_at":"2020-01-01T02:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"run-4","automation_id":"automation-2","created_at":"2020-01-01T01:00:00.000Z","updated_at":"2020-01-01T01:00:00.000Z"}} +{"site_uuid":"6cce57c1-c0df-4771-bb86-53cbf494f90f","id":"run-1","updated_at":"2020-01-01T01:00:00.000Z","inserted_at":"2020-01-01T02:00:00.000Z","payload":{"site_uuid":"6cce57c1-c0df-4771-bb86-53cbf494f90f","id":"run-1","automation_id":"automation-1","created_at":"2020-01-01T01:00:00.000Z","updated_at":"2020-01-01T01:00:00.000Z"}} diff --git a/ghost/core/core/server/data/tinybird/fixtures/automation_run_step_events.ndjson b/ghost/core/core/server/data/tinybird/fixtures/automation_run_step_events.ndjson new file mode 100644 index 00000000000..318fe158fcd --- /dev/null +++ b/ghost/core/core/server/data/tinybird/fixtures/automation_run_step_events.ndjson @@ -0,0 +1,8 @@ +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-1","updated_at":"2020-01-01T01:00:00.000Z","inserted_at":"2020-01-01T02:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-1","automation_run_id":"run-1","status":"pending","updated_at":"2020-01-01T01:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-1","updated_at":"2020-01-01T03:00:00.000Z","inserted_at":"2020-01-01T04:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-1","automation_run_id":"run-1","status":"finished","updated_at":"2020-01-01T03:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-2","updated_at":"2020-01-01T05:00:00.000Z","inserted_at":"2020-01-01T06:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-2","automation_run_id":"run-1","status":"pending","updated_at":"2020-01-01T05:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-2","updated_at":"2020-01-01T07:00:00.000Z","inserted_at":"2020-01-01T08:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-2","automation_run_id":"run-1","status":"pending","updated_at":"2020-01-01T07:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-3","updated_at":"2020-01-01T01:00:00.000Z","inserted_at":"2020-01-01T02:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-3","automation_run_id":"run-2","status":"pending","updated_at":"2020-01-01T01:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-3","updated_at":"2020-01-01T03:00:00.000Z","inserted_at":"2020-01-01T04:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-3","automation_run_id":"run-2","status":"finished","updated_at":"2020-01-01T03:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-4","updated_at":"2020-01-01T05:00:00.000Z","inserted_at":"2020-01-01T06:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-4","automation_run_id":"run-2","status":"pending","updated_at":"2020-01-01T05:00:00.000Z"}} +{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-4","updated_at":"2020-01-01T07:00:00.000Z","inserted_at":"2020-01-01T08:00:00.000Z","payload":{"site_uuid":"bd05ceed-1df9-4af7-832a-d3b5faa7ca1d","id":"step-4","automation_run_id":"run-2","status":"pending","updated_at":"2020-01-01T07:00:00.000Z"}} diff --git a/ghost/core/core/server/data/tinybird/pipes/mv_automation_run_steps.pipe b/ghost/core/core/server/data/tinybird/pipes/mv_automation_run_steps.pipe new file mode 100644 index 00000000000..a3e45019842 --- /dev/null +++ b/ghost/core/core/server/data/tinybird/pipes/mv_automation_run_steps.pipe @@ -0,0 +1,13 @@ +NODE extract_automation_run_steps +SQL > + WITH JSONExtract(payload, 'Tuple(automation_run_id String, status String)') AS payload_json + SELECT + site_uuid, + id, + updated_at, + getSubcolumn(payload_json, 'automation_run_id') AS automation_run_id, + getSubcolumn(payload_json, 'status') AS status + FROM automation_run_step_events + +TYPE MATERIALIZED +DATASOURCE _mv_automation_run_steps diff --git a/ghost/core/core/server/data/tinybird/pipes/mv_automation_runs.pipe b/ghost/core/core/server/data/tinybird/pipes/mv_automation_runs.pipe new file mode 100644 index 00000000000..64a70555a11 --- /dev/null +++ b/ghost/core/core/server/data/tinybird/pipes/mv_automation_runs.pipe @@ -0,0 +1,15 @@ +NODE extract_automation_runs +SQL > + WITH JSONExtract(payload, 'Tuple(automation_id String, created_at String)') AS payload_json + SELECT + site_uuid, + id, + updated_at, + getSubcolumn(payload_json, 'automation_id') AS automation_id, + parseDateTime64BestEffort( + getSubcolumn(payload_json, 'created_at'), 3, 'UTC' + ) AS created_at + FROM automation_run_events + +TYPE MATERIALIZED +DATASOURCE _mv_automation_runs diff --git a/ghost/core/core/server/data/tinybird/pipes/mv_pending_automation_runs.pipe b/ghost/core/core/server/data/tinybird/pipes/mv_pending_automation_runs.pipe new file mode 100644 index 00000000000..a2d2bd574b4 --- /dev/null +++ b/ghost/core/core/server/data/tinybird/pipes/mv_pending_automation_runs.pipe @@ -0,0 +1,7 @@ +NODE extract_pending_automation_runs +SQL > + SELECT site_uuid, id, automation_run_id, status = 'pending' AS is_pending, updated_at + FROM _mv_automation_run_steps + +TYPE MATERIALIZED +DATASOURCE _mv_pending_automation_runs diff --git a/ghost/core/core/server/data/tinybird/tests/api_automation_browse_stats.yaml b/ghost/core/core/server/data/tinybird/tests/api_automation_browse_stats.yaml new file mode 100644 index 00000000000..78b2b920f31 --- /dev/null +++ b/ghost/core/core/server/data/tinybird/tests/api_automation_browse_stats.yaml @@ -0,0 +1,18 @@ +- name: automation_browse_stats + description: Uses latest run and step versions for automation browse statistics + parameters: site_uuid=bd05ceed-1df9-4af7-832a-d3b5faa7ca1d + expected_result: | + {"automation_id":"automation-1","last_run_created_at":"2020-02-01T01:00:00.123Z","total_run_count":4,"in_progress_run_count":2} + {"automation_id":"automation-2","last_run_created_at":"2020-01-01T01:00:00.000Z","total_run_count":1,"in_progress_run_count":0} + +- name: automation_browse_stats_other_site + description: Restricts statistics to one site + parameters: site_uuid=6cce57c1-c0df-4771-bb86-53cbf494f90f + expected_result: | + {"automation_id":"automation-1","last_run_created_at":"2020-01-01T01:00:00.000Z","total_run_count":1,"in_progress_run_count":0} + +- name: automation_browse_stats_missing_site_uuid + description: Requires site UUID + expected_http_status: 400 + parameters: '' + expected_result: '' diff --git a/ghost/core/core/server/services/tinybird/tinybird-service.js b/ghost/core/core/server/services/tinybird/tinybird-service.js index e8844636f79..34d6cfcbdcf 100644 --- a/ghost/core/core/server/services/tinybird/tinybird-service.js +++ b/ghost/core/core/server/services/tinybird/tinybird-service.js @@ -58,6 +58,7 @@ const TINYBIRD_PIPES = [ 'api_top_utm_terms', 'api_top_devices', 'api_gift_link_visits', + 'api_automation_browse_stats', // v2 pipes (materialized view optimization) 'api_kpis_v2', 'api_active_visitors_v2', From 9ddd710e08118248f5682ef42b74adef5e397d76 Mon Sep 17 00:00:00 2001 From: Weyland Swart <49831538+weylandswart@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:08:28 +0100 Subject: [PATCH 04/10] Improved email sending status in posts list ref https://linear.app/ghost/issue/BER-3923/show-sending-status-in-posts-list Shows preparing and sending progress in React post list rows. Shares polling, count formatting, and ETA behavior with post analytics. --- .../email-sending-status-banner.tsx | 46 +-- .../email-sending-status-provider.tsx | 38 +-- .../use-sending-eta.test.ts | 2 +- .../email-sending-status-copy.test.ts | 43 +++ .../email-sending-status-copy.ts | 27 ++ .../use-email-sending-status.ts | 73 ++++ .../email-sending-status/use-sending-eta.ts | 0 .../post-list-row-email-status-state.test.ts | 66 ++++ .../post-list-row-email-status-state.ts | 26 ++ .../components/post-list-row-email-status.tsx | 101 ++++++ .../posts/list/components/post-list-row.tsx | 76 ++++- .../list/components/post-metrics-cells.tsx | 6 +- .../list/posts-list-rows.acceptance.test.tsx | 317 ++++++++++++++++++ .../src/posts/list/posts-list-screen.tsx | 3 + 14 files changed, 750 insertions(+), 74 deletions(-) create mode 100644 apps/admin/src/posts/email-sending-status/email-sending-status-copy.test.ts create mode 100644 apps/admin/src/posts/email-sending-status/email-sending-status-copy.ts create mode 100644 apps/admin/src/posts/email-sending-status/use-email-sending-status.ts rename apps/admin/src/posts/{analytics => }/email-sending-status/use-sending-eta.ts (100%) create mode 100644 apps/admin/src/posts/list/components/post-list-row-email-status-state.test.ts create mode 100644 apps/admin/src/posts/list/components/post-list-row-email-status-state.ts create mode 100644 apps/admin/src/posts/list/components/post-list-row-email-status.tsx diff --git a/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-banner.tsx b/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-banner.tsx index bf995fbb0ad..2c0bdb5b2ca 100644 --- a/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-banner.tsx +++ b/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-banner.tsx @@ -1,11 +1,11 @@ import { Banner, Button } from '@tryghost/shade/components'; import { Inline, Text } from '@tryghost/shade/primitives'; import { LucideIcon, formatNumber } from '@tryghost/shade/utils'; -import { useSendingEta } from './use-sending-eta'; import { useEmailSendingStatusContext } from './email-sending-status-context'; import { usePostAnalytics } from '@/posts/analytics/providers/post-analytics-context'; +import { getEmailSendingProgressCopy } from '@/posts/email-sending-status/email-sending-status-copy'; +import { useSendingEta } from '@/posts/email-sending-status/use-sending-eta'; import type { EmailSendingState } from '@tryghost/admin-x-framework/api/emails'; -import type { ReactNode } from 'react'; const FILL_CLIP_ID = 'email-sending-fill-clip'; @@ -58,24 +58,6 @@ const StatusGlyph = ({ sending }: { sending: EmailSendingState }) => { ); }; -const activeDetail = ( - sending: Exclude, - estimate: string | null, -): ReactNode => { - const { completed, total } = sending.progress; - - if (total === 0) { - return estimate; - } - - return ( - <> - {`${formatNumber(completed)} of ${formatNumber(total)}`} - {estimate && ` · ${estimate}`} - - ); -}; - const failureDetail = ( sending: Extract, error?: string | null, @@ -109,18 +91,20 @@ const EmailSendingStatusBanner = () => { sending.failed_during === 'submitting' && sending.progress.completed > 0 : false; - const title = isFailed - ? hasSentEmails - ? 'Some emails failed to send' - : 'Emails failed to send' - : sending.status === 'preparing' - ? 'Preparing emails' - : 'Sending emails'; - const detail = isFailed - ? hasUnknownDeliveryOutcome + let title: string; + let detail: string | null; + + if (sending.status === 'failed') { + title = hasSentEmails ? 'Some emails failed to send' : 'Emails failed to send'; + detail = hasUnknownDeliveryOutcome ? post?.email?.error || 'Something went wrong while sending this email.' - : failureDetail(sending, post?.email?.error) - : activeDetail(sending, estimate); + : failureDetail(sending, post?.email?.error); + } else { + const progressCopy = getEmailSendingProgressCopy(sending, estimate); + title = progressCopy.title; + detail = progressCopy.detail; + } + const retryLabel = hasSentEmails ? 'Send remaining emails' : 'Retry sending email'; return ( diff --git a/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-provider.tsx b/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-provider.tsx index f0a3312fcdb..75fb55a1a47 100644 --- a/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-provider.tsx +++ b/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-provider.tsx @@ -1,5 +1,4 @@ import { EmailSendingStatusContext } from './email-sending-status-context'; -import { APIError } from '@tryghost/admin-x-framework/errors'; import { feedbackDataType } from '@tryghost/admin-x-framework/api/feedback'; import { hasBeenEmailed } from '@tryghost/admin-x-framework'; import { linksDataType } from '@tryghost/admin-x-framework/api/links'; @@ -8,17 +7,13 @@ import { newsletterBasicStatsDataType, newsletterClickStatsDataType, } from '@tryghost/admin-x-framework/api/stats'; -import { - useBrowseEmailBatches, - useEmailSendingStatus, - useRetryEmail, -} from '@tryghost/admin-x-framework/api/emails'; +import { useBrowseEmailBatches, useRetryEmail } from '@tryghost/admin-x-framework/api/emails'; import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from 'react'; import { useFeatureFlag, useHandleError } from '@tryghost/admin-x-framework/hooks'; import { usePostAnalytics } from '@/posts/analytics/providers/post-analytics-context'; +import { useEmailSendingStatusPolling } from '@/posts/email-sending-status/use-email-sending-status'; import { useQueryClient } from '@tanstack/react-query'; -const STATUS_POLL_INTERVAL = import.meta.env.MODE === 'test' ? 50 : 2000; const NEWSLETTER_DATA_TYPES = new Set([ postsDataType, linksDataType, @@ -37,36 +32,17 @@ const EmailSendingStatusProvider = ({ children }: { children: ReactNode }) => { Boolean(emailId) && (post?.status === 'published' || post?.status === 'sent'); const shouldQuery = enabled && hasPublishedEmail && Boolean(emailStatus); - const statusQuery = useEmailSendingStatus(emailId ?? '', { - enabled: (query) => { - const queriedStatus = query.state.data?.email_statuses[0]?.sending.status; - const missingBackend = - !query.state.data && - query.state.error instanceof APIError && - query.state.error.response?.status === 404; - const submittedBeforeStatusLoaded = !query.state.data && emailStatus === 'submitted'; - return ( - shouldQuery && - !missingBackend && - !submittedBeforeStatusLoaded && - queriedStatus !== 'submitted' - ); - }, - defaultErrorHandler: false, - refetchInterval: (query) => { - const status = query.state.data?.email_statuses[0]?.sending.status; - return status === 'preparing' || status === 'submitting' ? STATUS_POLL_INTERVAL : false; - }, - refetchIntervalInBackground: false, - refetchOnWindowFocus: true, - retry: false, + const statusQuery = useEmailSendingStatusPolling({ + emailId, + emailStatus, + enabled: shouldQuery, }); const { mutateAsync: retryEmail, isPending: isRetryMutationPending } = useRetryEmail(); const { refetch: refetchStatus } = statusQuery; const handleError = useHandleError(); const [isRetryRefreshPending, setIsRetryRefreshPending] = useState(false); - const status = statusQuery.data?.email_statuses[0]; + const status = statusQuery.status; const sendingStatus = status?.sending.status; const shouldQueryBatches = Boolean(enabled && emailId && sendingStatus === 'failed'); const batchesQuery = useBrowseEmailBatches(emailId ?? '', { diff --git a/apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.test.ts b/apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.test.ts index 7e5072244ca..7f7fc9f7fac 100644 --- a/apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.test.ts +++ b/apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.test.ts @@ -1,6 +1,6 @@ import { renderHook } from '@testing-library/react'; import { describe, expect, it } from 'vitest'; -import { useSendingEta } from './use-sending-eta'; +import { useSendingEta } from '@/posts/email-sending-status/use-sending-eta'; import type { EmailSendingStatus } from '@tryghost/admin-x-framework/api/emails'; function status( diff --git a/apps/admin/src/posts/email-sending-status/email-sending-status-copy.test.ts b/apps/admin/src/posts/email-sending-status/email-sending-status-copy.test.ts new file mode 100644 index 00000000000..dacf2cac2b0 --- /dev/null +++ b/apps/admin/src/posts/email-sending-status/email-sending-status-copy.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest'; +import { getEmailSendingProgressCopy } from './email-sending-status-copy'; + +describe('getEmailSendingProgressCopy', () => { + it('formats preparing progress and an estimate', () => { + expect( + getEmailSendingProgressCopy( + { + status: 'preparing', + progress: { completed: 1200, total: 5000, estimated_seconds_remaining: 60 }, + }, + 'About 1 minute left', + ), + ).toEqual({ + title: 'Preparing emails', + detail: '1,200 of 5,000 · About 1 minute left', + }); + }); + + it('formats sending progress without an estimate', () => { + expect( + getEmailSendingProgressCopy( + { + status: 'submitting', + progress: { completed: 250, total: 1000, estimated_seconds_remaining: null }, + }, + null, + ), + ).toEqual({ title: 'Sending emails', detail: '250 of 1,000' }); + }); + + it('shows only an estimate before a total is available', () => { + expect( + getEmailSendingProgressCopy( + { + status: 'preparing', + progress: { completed: 0, total: 0, estimated_seconds_remaining: 30 }, + }, + 'Less than 1 minute left', + ), + ).toEqual({ title: 'Preparing emails', detail: 'Less than 1 minute left' }); + }); +}); diff --git a/apps/admin/src/posts/email-sending-status/email-sending-status-copy.ts b/apps/admin/src/posts/email-sending-status/email-sending-status-copy.ts new file mode 100644 index 00000000000..32516f40b33 --- /dev/null +++ b/apps/admin/src/posts/email-sending-status/email-sending-status-copy.ts @@ -0,0 +1,27 @@ +import { formatNumber } from '@tryghost/shade/utils'; +import type { EmailSendingState } from '@tryghost/admin-x-framework/api/emails'; + +type NonFailedEmailSendingState = Exclude; + +export interface EmailSendingProgressCopy { + title: 'Preparing emails' | 'Sending emails'; + detail: string | null; +} + +/** + * The active send wording shared by post analytics and the posts list. + * `submitted` is accepted because analytics keeps the sending UI visible while + * its dependent post and newsletter data refreshes. + */ +export function getEmailSendingProgressCopy( + sending: NonFailedEmailSendingState, + estimate: string | null, +): EmailSendingProgressCopy { + const { completed, total } = sending.progress; + const progress = total === 0 ? null : `${formatNumber(completed)} of ${formatNumber(total)}`; + + return { + title: sending.status === 'preparing' ? 'Preparing emails' : 'Sending emails', + detail: [progress, estimate].filter(Boolean).join(' · ') || null, + }; +} diff --git a/apps/admin/src/posts/email-sending-status/use-email-sending-status.ts b/apps/admin/src/posts/email-sending-status/use-email-sending-status.ts new file mode 100644 index 00000000000..9349c3a3892 --- /dev/null +++ b/apps/admin/src/posts/email-sending-status/use-email-sending-status.ts @@ -0,0 +1,73 @@ +import { APIError } from '@tryghost/admin-x-framework/errors'; +import { useEmailSendingStatus } from '@tryghost/admin-x-framework/api/emails'; +import { useCallback } from 'react'; +import type { EmailSendingStatus } from '@tryghost/admin-x-framework/api/emails'; + +const STATUS_POLL_INTERVAL = import.meta.env.MODE === 'test' ? 50 : 2000; + +interface UseEmailSendingStatusPollingOptions { + emailId?: string | null; + emailStatus?: string | null; + enabled: boolean; +} + +interface EmailSendingStatusPollingResult { + status: EmailSendingStatus | undefined; + isLoading: boolean; + isUnsupported: boolean; + refetch: (options?: { throwOnError?: boolean }) => Promise; +} + +/** + * The polling and older-backend compatibility contract shared by every Admin + * surface that reports an email send in progress. + */ +export function useEmailSendingStatusPolling({ + emailId, + emailStatus, + enabled, +}: UseEmailSendingStatusPollingOptions): EmailSendingStatusPollingResult { + const shouldQuery = enabled && Boolean(emailId) && Boolean(emailStatus); + const query = useEmailSendingStatus(emailId ?? '', { + enabled: (currentQuery) => { + const queriedStatus = currentQuery.state.data?.email_statuses[0]?.sending.status; + const missingBackend = + !currentQuery.state.data && + currentQuery.state.error instanceof APIError && + currentQuery.state.error.response?.status === 404; + const submittedBeforeStatusLoaded = !currentQuery.state.data && emailStatus === 'submitted'; + + return ( + shouldQuery && + !missingBackend && + !submittedBeforeStatusLoaded && + queriedStatus !== 'submitted' + ); + }, + defaultErrorHandler: false, + refetchInterval: (currentQuery) => { + const status = currentQuery.state.data?.email_statuses[0]?.sending.status; + return status === 'preparing' || status === 'submitting' ? STATUS_POLL_INTERVAL : false; + }, + refetchIntervalInBackground: false, + refetchOnWindowFocus: true, + retry: false, + }); + const data = query.data; + const isUnsupported = + !data && query.error instanceof APIError && query.error.response?.status === 404; + const { refetch: refetchQuery } = query; + const refetch = useCallback( + async (options?: { throwOnError?: boolean }) => { + await refetchQuery(options); + }, + [refetchQuery], + ); + + return { + status: data?.email_statuses[0], + isLoading: query.isLoading, + isUnsupported, + refetch, + }; +} diff --git a/apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.ts b/apps/admin/src/posts/email-sending-status/use-sending-eta.ts similarity index 100% rename from apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.ts rename to apps/admin/src/posts/email-sending-status/use-sending-eta.ts diff --git a/apps/admin/src/posts/list/components/post-list-row-email-status-state.test.ts b/apps/admin/src/posts/list/components/post-list-row-email-status-state.test.ts new file mode 100644 index 00000000000..ea9c2df57ac --- /dev/null +++ b/apps/admin/src/posts/list/components/post-list-row-email-status-state.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest'; +import { hasInProgressEmail } from './post-list-row-email-status-state'; +import type { PostListItem } from '@/posts/list/hooks/use-posts-list'; + +const post = (overrides: Partial = {}): PostListItem => ({ + id: 'post-1', + uuid: 'post-uuid', + title: 'A post', + slug: 'a-post', + url: 'https://example.com/a-post/', + status: 'published', + email: { + id: 'email-1', + status: 'submitting', + email_count: 1000, + opened_count: 0, + }, + ...overrides, +}); + +describe('hasInProgressEmail', () => { + it.each(['pending', 'submitting'] as const)('includes %s emails', (status) => { + expect( + hasInProgressEmail( + post({ email: { id: 'email-1', status, email_count: 0, opened_count: 0 } }), + 'posts', + true, + ), + ).toBe(true); + }); + + it('includes email-only sent posts', () => { + expect(hasInProgressEmail(post({ status: 'sent', email_only: true }), 'posts', true)).toBe( + true, + ); + }); + + it.each([ + ['feature disabled', post(), 'posts', false], + ['page', post(), 'pages', true], + ['draft', post({ status: 'draft' }), 'posts', true], + ['scheduled', post({ status: 'scheduled' }), 'posts', true], + [ + 'missing email ID', + post({ email: { status: 'submitting', email_count: 0, opened_count: 0 } }), + 'posts', + true, + ], + [ + 'submitted email', + post({ + email: { id: 'email-1', status: 'submitted', email_count: 1000, opened_count: 0 }, + }), + 'posts', + true, + ], + [ + 'failed email', + post({ email: { id: 'email-1', status: 'failed', email_count: 0, opened_count: 0 } }), + 'posts', + true, + ], + ] as const)('excludes a %s', (_label, candidate, resource, enabled) => { + expect(hasInProgressEmail(candidate, resource, enabled)).toBe(false); + }); +}); diff --git a/apps/admin/src/posts/list/components/post-list-row-email-status-state.ts b/apps/admin/src/posts/list/components/post-list-row-email-status-state.ts new file mode 100644 index 00000000000..25d6c8d3c98 --- /dev/null +++ b/apps/admin/src/posts/list/components/post-list-row-email-status-state.ts @@ -0,0 +1,26 @@ +import type { PostListItem } from '@/posts/list/hooks/use-posts-list'; +import type { PostResource } from '@/posts/list/post-resource'; +import type { EmailSendingProgressCopy } from '@/posts/email-sending-status/email-sending-status-copy'; + +export type PostListRowEmailStatusState = + | { status: 'settled' } + | { status: 'sending'; copy: EmailSendingProgressCopy } + | { status: 'failed' }; + +export const SETTLED_POST_LIST_ROW_EMAIL_STATUS: PostListRowEmailStatusState = { + status: 'settled', +}; + +export function hasInProgressEmail( + post: PostListItem, + resource: PostResource, + improveSendingUI: boolean, +): boolean { + return Boolean( + improveSendingUI && + resource === 'posts' && + (post.status === 'published' || post.status === 'sent') && + post.email?.id && + (post.email.status === 'pending' || post.email.status === 'submitting'), + ); +} diff --git a/apps/admin/src/posts/list/components/post-list-row-email-status.tsx b/apps/admin/src/posts/list/components/post-list-row-email-status.tsx new file mode 100644 index 00000000000..7e6fbd42ab6 --- /dev/null +++ b/apps/admin/src/posts/list/components/post-list-row-email-status.tsx @@ -0,0 +1,101 @@ +import { postsDataType } from '@tryghost/admin-x-framework/api/posts'; +import { useQueryClient } from '@tanstack/react-query'; +import { useEffect, useState, type ReactNode } from 'react'; +import { getEmailSendingProgressCopy } from '@/posts/email-sending-status/email-sending-status-copy'; +import { useEmailSendingStatusPolling } from '@/posts/email-sending-status/use-email-sending-status'; +import { useSendingEta } from '@/posts/email-sending-status/use-sending-eta'; +import type { PostListItem } from '@/posts/list/hooks/use-posts-list'; +import { + SETTLED_POST_LIST_ROW_EMAIL_STATUS, + type PostListRowEmailStatusState, +} from '@/posts/list/components/post-list-row-email-status-state'; + +interface PostListRowEmailStatusProps { + post: PostListItem; + children: (state: PostListRowEmailStatusState) => ReactNode; +} + +/** + * Owns the live query for one in-progress row. Settled rows never mount this + * component, keeping their memo boundary independent of polling updates. + */ +export function PostListRowEmailStatus({ post, children }: PostListRowEmailStatusProps) { + const queryClient = useQueryClient(); + const emailId = post.email?.id; + const emailStatus = post.email?.status; + const query = useEmailSendingStatusPolling({ emailId, emailStatus, enabled: true }); + const status = query.status; + const sending = status?.sending; + const sendingStatus = sending?.status; + const estimate = useSendingEta(status); + const [refreshedSubmittedEmailId, setRefreshedSubmittedEmailId] = useState(null); + + useEffect(() => { + if (!emailId || !sendingStatus) { + return; + } + + if (sendingStatus !== 'submitted') { + setRefreshedSubmittedEmailId((currentEmailId) => + currentEmailId === emailId ? null : currentEmailId, + ); + } + + if (sendingStatus === 'failed') { + void queryClient.invalidateQueries({ queryKey: [postsDataType] }); + return; + } + + if (sendingStatus === 'submitted') { + let cancelled = false; + void queryClient.invalidateQueries({ queryKey: [postsDataType] }).then(() => { + if (!cancelled) { + setRefreshedSubmittedEmailId(emailId); + } + }); + + return () => { + cancelled = true; + }; + } + }, [emailId, queryClient, sendingStatus]); + + if (query.isUnsupported) { + return <>{children(SETTLED_POST_LIST_ROW_EMAIL_STATUS)}; + } + + if (sending?.status === 'failed') { + return <>{children({ status: 'failed' })}; + } + + const isRefreshingSubmittedData = Boolean( + emailId && sending?.status === 'submitted' && refreshedSubmittedEmailId !== emailId, + ); + + if (sending && (sending.status !== 'submitted' || isRefreshingSubmittedData)) { + return ( + <> + {children({ + status: 'sending', + copy: getEmailSendingProgressCopy(sending, estimate), + })} + + ); + } + + if (!sending && (emailStatus === 'pending' || emailStatus === 'submitting')) { + return ( + <> + {children({ + status: 'sending', + copy: { + title: emailStatus === 'pending' ? 'Preparing emails' : 'Sending emails', + detail: null, + }, + })} + + ); + } + + return <>{children(SETTLED_POST_LIST_ROW_EMAIL_STATUS)}; +} diff --git a/apps/admin/src/posts/list/components/post-list-row.tsx b/apps/admin/src/posts/list/components/post-list-row.tsx index 75e6c65e8de..9bb6ba04da2 100644 --- a/apps/admin/src/posts/list/components/post-list-row.tsx +++ b/apps/admin/src/posts/list/components/post-list-row.tsx @@ -13,6 +13,12 @@ import { } from '@/posts/list/post-row-copy'; import { hasPostAnalyticsPage, type PostMetricsSettings } from '@/posts/list/post-metrics'; import { PostMetricsCells } from '@/posts/list/components/post-metrics-cells'; +import { PostListRowEmailStatus } from '@/posts/list/components/post-list-row-email-status'; +import { + hasInProgressEmail, + SETTLED_POST_LIST_ROW_EMAIL_STATUS, + type PostListRowEmailStatusState, +} from '@/posts/list/components/post-list-row-email-status-state'; import { forwardRef, memo, useState } from 'react'; import type { ComponentPropsWithoutRef, MouseEvent as ReactMouseEvent } from 'react'; import type { PostListItem } from '@/posts/list/hooks/use-posts-list'; @@ -51,6 +57,11 @@ interface PostListRowProps extends Omit, 'onClick metricsSettings: PostMetricsSettings; visitorCounts?: Record; memberCounts?: Record; + improveSendingUI?: boolean; +} + +interface PostListRowComponentProps extends PostListRowProps { + emailSendingState: PostListRowEmailStatusState; } /** @@ -108,7 +119,7 @@ function FeatureImage({ post }: { post: PostListItem }) { return ; } -const PostListRowComponent = forwardRef( +const PostListRowComponent = forwardRef( function PostListRowComponent( { post, @@ -128,6 +139,8 @@ const PostListRowComponent = forwardRef( metricsSettings, visitorCounts, memberCounts, + improveSendingUI: _improveSendingUI, + emailSendingState, // Everything else lands on the
  • : the context menu wraps each row with // `asChild`, so Radix hands its trigger props and ref straight through. ...rest @@ -141,6 +154,15 @@ const PostListRowComponent = forwardRef( const statusLabel = getPostStatusLabel(post, resource); const statusDetail = getPostStatusDetail(post, { timezone, resource }); const isFailed = didPostEmailFail(post, resource); + const displayedPost = + emailSendingState.status === 'failed' + ? ({ ...post, email: { ...post.email, status: 'failed' } } as PostListItem) + : post; + const displayedIsFailed = emailSendingState.status === 'failed' || isFailed; + const displayedStatusLabel = + emailSendingState.status === 'failed' + ? getPostStatusLabel(displayedPost, resource) + : statusLabel; // Strictly `published`, matching Ember's `isPublished`. An email-only // `sent` post still opens in the editor for a contributor. @@ -243,18 +265,30 @@ const PostListRowComponent = forwardRef( )} - - {statusLabel} - {/* Mounted only while hovered, as Ember does. A CSS - opacity fade would keep it in the DOM, so a screen - reader would read every scheduled row's full - dispatch details aloud, always. */} - {isHovered && statusDetail && {statusDetail}} - + {emailSendingState.status === 'sending' ? ( + + {emailSendingState.copy.title} + {emailSendingState.copy.detail && ( + {` · ${emailSendingState.copy.detail}`} + )} + + ) : ( + + {displayedStatusLabel} + {/* Mounted only while hovered, as Ember does. A CSS + opacity fade would keep it in the DOM, so a screen + reader would read every scheduled row's full + dispatch details aloud, always. */} + {emailSendingState.status !== 'failed' && isHovered && statusDetail && ( + {statusDetail} + )} + + )} ( }, ); +const PostListRowWithEmailStatus = forwardRef( + function PostListRowWithEmailStatus(props, ref) { + if (hasInProgressEmail(props.post, props.resource, props.improveSendingUI ?? false)) { + return ( + + {(emailSendingState) => ( + + )} + + ); + } + + return ( + + ); + }, +); + /** * Memoised. Selection state and modifier "select mode" both live above the * list, so without this every cmd-click and every press of the Cmd key @@ -321,4 +377,4 @@ const PostListRowComponent = forwardRef( * Every prop is either a primitive or memoised upstream; `metricsSettings` in * particular is built with `useMemo` for this reason. */ -export const PostListRow = memo(PostListRowComponent); +export const PostListRow = memo(PostListRowWithEmailStatus); diff --git a/apps/admin/src/posts/list/components/post-metrics-cells.tsx b/apps/admin/src/posts/list/components/post-metrics-cells.tsx index 96d3462daa3..07595ba5f3a 100644 --- a/apps/admin/src/posts/list/components/post-metrics-cells.tsx +++ b/apps/admin/src/posts/list/components/post-metrics-cells.tsx @@ -28,6 +28,7 @@ interface PostMetricsCellsProps { memberCounts?: Record; paidMembersEnabled?: boolean; className?: string; + hideEmailMetrics?: boolean; } const EMAIL_KEYS: PostMetricKey[] = ['opens', 'clicks', 'sent']; @@ -74,8 +75,11 @@ export function PostMetricsCells({ memberCounts, paidMembersEnabled, className, + hideEmailMetrics, }: PostMetricsCellsProps) { - const columns = getPostMetricColumns(post, settings, resource); + const columns = getPostMetricColumns(post, settings, resource).filter( + (column) => !(hideEmailMetrics && EMAIL_KEYS.includes(column.key)), + ); const shown = new Set(columns.map((column) => column.key)); const members = memberCounts?.[post.id]; diff --git a/apps/admin/src/posts/list/posts-list-rows.acceptance.test.tsx b/apps/admin/src/posts/list/posts-list-rows.acceptance.test.tsx index 32e1af825c5..28d30026670 100644 --- a/apps/admin/src/posts/list/posts-list-rows.acceptance.test.tsx +++ b/apps/admin/src/posts/list/posts-list-rows.acceptance.test.tsx @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it } from 'vitest'; +import { focusManager } from '@tanstack/react-query'; import { currentRoute, @@ -16,6 +17,17 @@ import { import { postsListScreen } from './posts-list.screen'; const FLAG_ON = { labs: { postsListReact: true } }; +const SENDING_FLAG_ON = { labs: { postsListReact: true, improveSendingUI: true } }; +const EMAIL_ID = '64d623b64676110001e897ab'; + +const emailMetricsSettings = settingsResponse({ + settings: { + email_track_opens: true, + email_track_clicks: true, + web_analytics_enabled: true, + members_signup_access: 'all', + }, +}); /** * What a row says, and the two empty states — the parity-critical surface of @@ -175,6 +187,311 @@ describe('Posts list rows', () => { }); }); +describe('Posts list email sending status', () => { + beforeEach(() => { + fakePostsListScreen(); + }); + + it('shows preparing progress and hides only email metrics', async () => { + fakePosts([ + post({ + id: 'preparing-post', + title: 'Preparing post', + status: 'published', + email: { + id: EMAIL_ID, + status: 'pending', + email_count: 1000, + opened_count: 0, + track_opens: true, + track_clicks: true, + }, + count: { clicks: 0, positive_feedback: 0, negative_feedback: 0 }, + }), + ]); + fakeAdminEndpoint('GET', `/emails/${EMAIL_ID}/status/`, { + email_statuses: [ + { + id: EMAIL_ID, + sending: { + status: 'preparing', + progress: { completed: 250, total: 1000, estimated_seconds_remaining: 30 }, + }, + }, + ], + }); + + await renderAdminApp('/posts?type=published', { + ...SENDING_FLAG_ON, + boot: { browseSettings: { response: emailMetricsSettings } }, + }); + + const row = postsListScreen.listItems().first(); + await expect + .element(row) + .toHaveTextContent('Preparing emails · 250 of 1,000 · Less than 1 minute left'); + await expect.element(row).not.toHaveTextContent('Published and sent'); + await expect.element(row.getByLabelText(/Visitors/)).toBeVisible(); + await expect.element(row.getByLabelText(/Sent/)).not.toBeInTheDocument(); + await expect.element(row.getByLabelText(/Opens/)).not.toBeInTheDocument(); + await expect.element(row.getByLabelText(/Clicks/)).not.toBeInTheDocument(); + await expect.element(row.getByRole('status')).not.toBeInTheDocument(); + }); + + it('shows coarse sending copy through a transient error and recovers on focus', async () => { + fakePosts([ + post({ + id: 'sending-post', + title: 'Sending post', + status: 'published', + email: { id: EMAIL_ID, status: 'submitting', email_count: 1000, opened_count: 0 }, + }), + ]); + const failedStatusApi = fakeAdminEndpoint( + 'GET', + `/emails/${EMAIL_ID}/status/`, + { errors: [{ message: 'Bad gateway' }] }, + { status: 502 }, + ); + + await renderAdminApp('/posts?type=published', SENDING_FLAG_ON); + + const row = postsListScreen.listItems().first(); + await expect.element(row).toHaveTextContent('Sending emails'); + await expect.element(row).not.toHaveTextContent('Published and sent'); + await expect.poll(() => failedStatusApi.requests.length).toBe(1); + + const recoveredStatusApi = fakeAdminEndpoint('GET', `/emails/${EMAIL_ID}/status/`, { + email_statuses: [ + { + id: EMAIL_ID, + sending: { + status: 'submitting', + progress: { completed: 500, total: 1000, estimated_seconds_remaining: null }, + }, + }, + ], + }); + focusManager.setFocused(false); + focusManager.setFocused(true); + + await expect.poll(() => recoveredStatusApi.requests.length).toBeGreaterThan(0); + await expect.element(row).toHaveTextContent('Sending emails · 500 of 1,000'); + focusManager.setFocused(undefined); + }); + + it('refreshes the post and restores settled copy and metrics after sending', async () => { + const sendingPost = post({ + id: 'completed-post', + title: 'Completed post', + status: 'published', + email: { + id: EMAIL_ID, + status: 'submitting', + email_count: 0, + opened_count: 0, + track_opens: true, + track_clicks: true, + }, + count: { clicks: 0, positive_feedback: 0, negative_feedback: 0 }, + }); + const submittedPost = post({ + ...sendingPost, + email: { + ...sendingPost.email!, + status: 'submitted', + email_count: 1000, + opened_count: 400, + }, + count: { clicks: 60, positive_feedback: 0, negative_feedback: 0 }, + }); + let sendingComplete = false; + const postsApi = fakePosts(() => [sendingComplete ? submittedPost : sendingPost]); + let statusRequestCount = 0; + fakeAdminEndpoint('GET', `/emails/${EMAIL_ID}/status/`, () => { + statusRequestCount += 1; + return { + email_statuses: [ + { + id: EMAIL_ID, + sending: sendingComplete + ? { + status: 'submitted', + progress: { completed: 1000, total: 1000, estimated_seconds_remaining: 0 }, + } + : { + status: 'submitting', + progress: { completed: 500, total: 1000, estimated_seconds_remaining: null }, + }, + }, + ], + }; + }); + + await renderAdminApp('/posts?type=published', { + ...SENDING_FLAG_ON, + boot: { browseSettings: { response: emailMetricsSettings } }, + }); + + const row = postsListScreen.listItems().first(); + await expect.element(row).toHaveTextContent('Sending emails · 500 of 1,000'); + await expect.element(row.getByLabelText(/Sent/)).not.toBeInTheDocument(); + + sendingComplete = true; + const pendingStatusRequestCount = statusRequestCount; + await expect + .poll(() => statusRequestCount, { timeout: 3500 }) + .toBeGreaterThan(pendingStatusRequestCount); + await expect.poll(() => postsApi.requests.length).toBeGreaterThan(1); + await expect.element(row).toHaveTextContent('Published and sent'); + await expect.element(row).not.toHaveTextContent('Sending emails'); + await expect.element(row.getByLabelText(/Opens/)).toHaveTextContent('40%'); + await expect.element(row.getByLabelText(/Clicks/)).toHaveTextContent('6%'); + }); + + it('uses the existing failure state when polling reports a failure', async () => { + const sendingPost = post({ + id: 'failed-post', + title: 'Failed post', + status: 'published', + email: { id: EMAIL_ID, status: 'submitting', email_count: 0, opened_count: 0 }, + }); + let sendingFailed = false; + const postsApi = fakePosts(() => [ + sendingFailed + ? post({ + ...sendingPost, + email: { ...sendingPost.email!, status: 'failed', error: 'The send failed.' }, + }) + : sendingPost, + ]); + let statusRequestCount = 0; + fakeAdminEndpoint('GET', `/emails/${EMAIL_ID}/status/`, () => { + statusRequestCount += 1; + return { + email_statuses: [ + { + id: EMAIL_ID, + sending: sendingFailed + ? { + status: 'failed', + failed_during: 'submitting', + progress: { completed: 250, total: 1000, estimated_seconds_remaining: null }, + } + : { + status: 'submitting', + progress: { completed: 250, total: 1000, estimated_seconds_remaining: null }, + }, + }, + ], + }; + }); + + await renderAdminApp('/posts?type=published', SENDING_FLAG_ON); + const row = postsListScreen.listItems().first(); + await expect.element(row).toHaveTextContent('Sending emails'); + + sendingFailed = true; + const pendingStatusRequestCount = statusRequestCount; + await expect + .poll(() => statusRequestCount, { timeout: 3500 }) + .toBeGreaterThan(pendingStatusRequestCount); + await expect.element(row).toHaveTextContent('Published but failed to send newsletter'); + await expect.element(row).not.toHaveTextContent('Emails failed to send'); + await expect.poll(() => postsApi.requests.length).toBeGreaterThan(1); + }); + + it('falls back to the existing row when the status endpoint is unavailable', async () => { + fakePosts([ + post({ + id: 'unsupported-post', + title: 'Unsupported post', + status: 'published', + email: { id: EMAIL_ID, status: 'submitting', email_count: 1000, opened_count: 0 }, + }), + ]); + const statusApi = fakeAdminEndpoint( + 'GET', + `/emails/${EMAIL_ID}/status/`, + { errors: [{ message: 'Resource not found' }] }, + { status: 404 }, + ); + + const app = await renderAdminApp('/posts?type=published', SENDING_FLAG_ON); + + const row = postsListScreen.listItems().first(); + await expect.element(row).toHaveTextContent('Published and sent'); + await expect.element(row).not.toHaveTextContent('Sending emails'); + await expect.poll(() => statusApi.requests.length).toBe(1); + await app.unmount(); + }); + + it('polls every concurrently active email independently', async () => { + const firstEmailId = '64d623b64676110001e897a1'; + const secondEmailId = '64d623b64676110001e897a2'; + fakePosts([ + post({ + id: 'first-active-post', + title: 'First active post', + status: 'published', + email: { id: firstEmailId, status: 'submitting', email_count: 1000, opened_count: 0 }, + }), + post({ + id: 'second-active-post', + title: 'Second active post', + status: 'sent', + email_only: true, + email: { id: secondEmailId, status: 'pending', email_count: 2000, opened_count: 0 }, + }), + ]); + const firstStatusApi = fakeAdminEndpoint('GET', `/emails/${firstEmailId}/status/`, { + email_statuses: [ + { + id: firstEmailId, + sending: { + status: 'submitting', + progress: { completed: 100, total: 1000, estimated_seconds_remaining: null }, + }, + }, + ], + }); + const secondStatusApi = fakeAdminEndpoint('GET', `/emails/${secondEmailId}/status/`, { + email_statuses: [ + { + id: secondEmailId, + sending: { + status: 'preparing', + progress: { completed: 200, total: 2000, estimated_seconds_remaining: null }, + }, + }, + ], + }); + + await renderAdminApp('/posts?type=published', SENDING_FLAG_ON); + + await expect.element(postsListScreen.listItems().nth(0)).toHaveTextContent('100 of 1,000'); + await expect.element(postsListScreen.listItems().nth(1)).toHaveTextContent('200 of 2,000'); + await expect.poll(() => firstStatusApi.requests.length).toBeGreaterThan(0); + await expect.poll(() => secondStatusApi.requests.length).toBeGreaterThan(0); + }); + + it('does not request status when the sending UI flag is off', async () => { + fakePosts([ + post({ + title: 'Flagged off post', + status: 'published', + email: { id: EMAIL_ID, status: 'submitting', email_count: 1000, opened_count: 0 }, + }), + ]); + + await renderAdminApp('/posts?type=published', FLAG_ON); + + const row = postsListScreen.listItems().first(); + await expect.element(row).toHaveTextContent('Published and sent'); + await expect.element(row).not.toHaveTextContent('Sending emails'); + }); +}); + describe('Posts list empty states', () => { // The filter bar mounts with the screen and probes these to resolve any // author/tag slug in the URL into a name. diff --git a/apps/admin/src/posts/list/posts-list-screen.tsx b/apps/admin/src/posts/list/posts-list-screen.tsx index 17b62bf06ff..6117dc7858a 100644 --- a/apps/admin/src/posts/list/posts-list-screen.tsx +++ b/apps/admin/src/posts/list/posts-list-screen.tsx @@ -46,6 +46,7 @@ import { lazy, Suspense, useCallback, useEffect, useMemo, useRef, useState } fro import { useLocation } from '@tryghost/admin-x-framework'; import { usePostAnalyticsCounts } from './hooks/use-post-analytics-counts'; import { usePostsList } from './hooks/use-posts-list'; +import { useFeatureFlag } from '@tryghost/admin-x-framework/hooks'; /** * The React posts and pages list screens, served behind the `postsListReact` @@ -63,6 +64,7 @@ export function PostsListScreen({ resource }: { resource: PostResource }) { usePostsFilterState(); const { data: currentUser } = useCurrentUser(); const { data: settingsData } = useBrowseSettings(); + const improveSendingUI = useFeatureFlag('improveSendingUI'); // Report the current filters so the sidebar's Posts link can return here. const location = useLocation(); @@ -376,6 +378,7 @@ export function PostsListScreen({ resource }: { resource: PostResource }) { key={item.id} getMenuItems={getMenuItems} hasAdminAccess={isAdmin} + improveSendingUI={improveSendingUI} isContributor={isContributor} isSelected={selection.isSelected(item.id)} memberCounts={memberCounts} From 65d91015990e24855103dc45458599fe57f94e30 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 10 Sep 2026 13:49:59 +0100 Subject: [PATCH 05/10] Fixed stale email sending status on screen entry ref https://linear.app/ghost/issue/BER-3923/show-sending-status-in-posts-list Refresh email status on entry to the posts list and analytics so a cached failure cannot hide a retried send. Withhold cached status until the current screen receives a successful response. --- .../use-email-sending-status.test.ts | 117 ++++++++++++++++++ .../use-email-sending-status.ts | 25 ++-- 2 files changed, 125 insertions(+), 17 deletions(-) create mode 100644 apps/admin/src/posts/email-sending-status/use-email-sending-status.test.ts diff --git a/apps/admin/src/posts/email-sending-status/use-email-sending-status.test.ts b/apps/admin/src/posts/email-sending-status/use-email-sending-status.test.ts new file mode 100644 index 00000000000..4cb8990abc9 --- /dev/null +++ b/apps/admin/src/posts/email-sending-status/use-email-sending-status.test.ts @@ -0,0 +1,117 @@ +import { createElement, type ReactNode } from 'react'; +import { act, renderHook, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider, useQuery } from '@tanstack/react-query'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { useEmailSendingStatusPolling } from './use-email-sending-status'; +import type { EmailStatusesResponseType } from '@tryghost/admin-x-framework/api/emails'; + +const { fetchStatus } = vi.hoisted(() => ({ fetchStatus: vi.fn() })); + +// Keep the real query observer and production cache lifetime; only replace HTTP. +vi.mock('@tryghost/admin-x-framework/api/emails', () => ({ + useEmailSendingStatus: (id: string, options: object) => + useQuery({ + ...options, + queryKey: ['email-status', id], + queryFn: fetchStatus, + }), +})); + +function response( + status: 'preparing' | 'submitting' | 'submitted' | 'failed', +): EmailStatusesResponseType { + const progress = { completed: 10, total: 100, estimated_seconds_remaining: null }; + return { + email_statuses: [ + { + id: 'email-1', + sending: + status === 'failed' + ? { status, progress, failed_during: 'submitting' } + : { status, progress }, + }, + ], + }; +} + +const clients: QueryClient[] = []; + +afterEach(() => { + clients.forEach((client) => client.clear()); + clients.length = 0; + vi.resetAllMocks(); +}); + +function setup(cached: EmailStatusesResponseType, emailStatus = 'submitting') { + const client = new QueryClient({ + defaultOptions: { queries: { staleTime: 5 * 60 * 1000, retry: false } }, + }); + clients.push(client); + client.setQueryData(['email-status', 'email-1'], cached); + const wrapper = ({ children }: { children: ReactNode }) => + createElement(QueryClientProvider, { client }, children); + return renderHook( + () => + useEmailSendingStatusPolling({ + emailId: 'email-1', + emailStatus, + enabled: true, + }), + { wrapper }, + ); +} + +describe('email status on screen entry', () => { + it('fetches the current retry progress instead of reusing a cached send failure', async () => { + let resolve!: (value: EmailStatusesResponseType) => void; + fetchStatus.mockImplementation( + () => + new Promise((done) => { + resolve = done; + }), + ); + // The list previously observed a failure. After a retry elsewhere, the post + // says submitting, but the status query still has that failure in its cache. + const { result, unmount } = setup(response('failed'), 'submitting'); + + expect(fetchStatus).toHaveBeenCalledTimes(1); + expect(result.current.status).toBeUndefined(); + expect(result.current.isLoading).toBe(true); + + act(() => { + resolve(response('submitting')); + }); + await waitFor(() => expect(result.current.status?.sending.status).toBe('submitting')); + // Failed sends stop polling; the refreshed retry must start it again. + await waitFor(() => expect(fetchStatus.mock.calls.length).toBeGreaterThan(1)); + unmount(); + }); + + it('does not expose the cached failure if the entry refresh fails', async () => { + fetchStatus.mockRejectedValue(new Error('Bad gateway')); + const { result, unmount } = setup(response('failed')); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.status).toBeUndefined(); + unmount(); + }); + + it('does not revive cached sending UI for a post already submitted', () => { + const { result, unmount } = setup(response('failed'), 'submitted'); + expect(fetchStatus).not.toHaveBeenCalled(); + expect(result.current.status).toBeUndefined(); + unmount(); + }); + + it('propagates an explicit refresh failure so analytics can report a failed retry', async () => { + fetchStatus.mockResolvedValue(response('failed')); + const { result, unmount } = setup(response('failed'), 'failed'); + await waitFor(() => expect(result.current.status?.sending.status).toBe('failed')); + + const error = new Error('Bad gateway'); + fetchStatus.mockRejectedValue(error); + await act(async () => { + await expect(result.current.refetch({ throwOnError: true })).rejects.toBe(error); + }); + unmount(); + }); +}); diff --git a/apps/admin/src/posts/email-sending-status/use-email-sending-status.ts b/apps/admin/src/posts/email-sending-status/use-email-sending-status.ts index 9349c3a3892..905d2e49b17 100644 --- a/apps/admin/src/posts/email-sending-status/use-email-sending-status.ts +++ b/apps/admin/src/posts/email-sending-status/use-email-sending-status.ts @@ -29,21 +29,10 @@ export function useEmailSendingStatusPolling({ }: UseEmailSendingStatusPollingOptions): EmailSendingStatusPollingResult { const shouldQuery = enabled && Boolean(emailId) && Boolean(emailStatus); const query = useEmailSendingStatus(emailId ?? '', { - enabled: (currentQuery) => { - const queriedStatus = currentQuery.state.data?.email_statuses[0]?.sending.status; - const missingBackend = - !currentQuery.state.data && - currentQuery.state.error instanceof APIError && - currentQuery.state.error.response?.status === 404; - const submittedBeforeStatusLoaded = !currentQuery.state.data && emailStatus === 'submitted'; - - return ( - shouldQuery && - !missingBackend && - !submittedBeforeStatusLoaded && - queriedStatus !== 'submitted' - ); - }, + // The post's terminal state needs no status check. For every other state, + // refresh on entry, even if a previous screen cached a failure or completion. + enabled: shouldQuery && emailStatus !== 'submitted', + staleTime: 0, defaultErrorHandler: false, refetchInterval: (currentQuery) => { const status = currentQuery.state.data?.email_statuses[0]?.sending.status; @@ -53,7 +42,9 @@ export function useEmailSendingStatusPolling({ refetchOnWindowFocus: true, retry: false, }); - const data = query.data; + // Cached results must not drive banners or completion/failure effects before + // this screen's request succeeds (including when that request fails). + const data = query.isFetchedAfterMount && !query.isError ? query.data : undefined; const isUnsupported = !data && query.error instanceof APIError && query.error.response?.status === 404; const { refetch: refetchQuery } = query; @@ -66,7 +57,7 @@ export function useEmailSendingStatusPolling({ return { status: data?.email_statuses[0], - isLoading: query.isLoading, + isLoading: query.isLoading || (!data && query.isFetching), isUnsupported, refetch, }; From 24637dd379f0b9009a8dd2f1f77eae0459aa9e2e Mon Sep 17 00:00:00 2001 From: Luis Azevedo Date: Thu, 10 Sep 2026 15:41:51 +0100 Subject: [PATCH 06/10] Updated email sending progress banners(#30626) ref https://linear.app/ghost/issue/BER-3950/ ref https://linear.app/ghost/issue/BER-3955/ Show preparation as a percentage so recipient counts only climb during sending, and keep the banner readable on smaller screens. - Show preparation percentage alongside the total audience size - Hide inaccurate preparation time estimates in analytics and the posts list - Keep time estimates during sending and preserve failure and retry states - Place banner details below the title on smaller screens --- .../email-sending-status-banner.tsx | 65 +++++++++++++------ .../use-sending-eta.test.ts | 5 +- .../post-analytics.acceptance.test.tsx | 17 ++++- .../email-sending-status/use-sending-eta.ts | 5 +- .../list/posts-list-rows.acceptance.test.tsx | 5 +- 5 files changed, 69 insertions(+), 28 deletions(-) diff --git a/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-banner.tsx b/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-banner.tsx index 2c0bdb5b2ca..65f389692f6 100644 --- a/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-banner.tsx +++ b/apps/admin/src/posts/analytics/email-sending-status/email-sending-status-banner.tsx @@ -1,6 +1,6 @@ import { Banner, Button } from '@tryghost/shade/components'; -import { Inline, Text } from '@tryghost/shade/primitives'; -import { LucideIcon, formatNumber } from '@tryghost/shade/utils'; +import { Box, Grid, Text } from '@tryghost/shade/primitives'; +import { LucideIcon, cn, formatNumber } from '@tryghost/shade/utils'; import { useEmailSendingStatusContext } from './email-sending-status-context'; import { usePostAnalytics } from '@/posts/analytics/providers/post-analytics-context'; import { getEmailSendingProgressCopy } from '@/posts/email-sending-status/email-sending-status-copy'; @@ -58,6 +58,23 @@ const StatusGlyph = ({ sending }: { sending: EmailSendingState }) => { ); }; +const activeDetail = (sending: Exclude) => { + const { completed, total } = sending.progress; + + if (total === 0) { + return null; + } + + // Preparation reports a percentage so the recipient count only climbs + // once, during sending, while the audience size stays on screen throughout. + if (sending.status === 'preparing') { + const percent = Math.min(100, Math.floor((completed / total) * 100)); + return `${formatNumber(percent)}% complete · ${formatNumber(total)} total`; + } + + return getEmailSendingProgressCopy(sending, null).detail; +}; + const failureDetail = ( sending: Extract, error?: string | null, @@ -100,9 +117,9 @@ const EmailSendingStatusBanner = () => { ? post?.email?.error || 'Something went wrong while sending this email.' : failureDetail(sending, post?.email?.error); } else { - const progressCopy = getEmailSendingProgressCopy(sending, estimate); + const progressCopy = getEmailSendingProgressCopy(sending, null); title = progressCopy.title; - detail = progressCopy.detail; + detail = activeDetail(sending); } const retryLabel = hasSentEmails ? 'Send remaining emails' : 'Retry sending email'; @@ -114,24 +131,34 @@ const EmailSendingStatusBanner = () => { role={isFailed ? 'alert' : 'status'} size="lg" > - - + + - - - {title} - - {detail && ( - - {' · '} - {detail} - - )} + + + {title} + + + + {detail} - + {!isFailed && estimate && ( + + {detail && {' · '}} + {estimate} + + )} + {isFailed && !hasUnknownDeliveryOutcome && ( )} - + ); }; diff --git a/apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.test.ts b/apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.test.ts index 7f7fc9f7fac..65433eec072 100644 --- a/apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.test.ts +++ b/apps/admin/src/posts/analytics/email-sending-status/use-sending-eta.test.ts @@ -18,13 +18,15 @@ function status( } describe('useSendingEta', () => { - it('hides the time label until an active phase has an estimate', () => { + it('hides the time label until sending has an estimate', () => { const { result, rerender } = renderHook(useSendingEta, { initialProps: undefined as EmailSendingStatus | undefined, }); expect(result.current).toBeNull(); rerender(status(null, 'preparing')); expect(result.current).toBeNull(); + rerender(status(30, 'preparing')); + expect(result.current).toBeNull(); rerender(status(null, 'submitting')); expect(result.current).toBeNull(); rerender(status(30)); @@ -65,6 +67,7 @@ describe('useSendingEta', () => { const { result, rerender } = renderHook(useSendingEta, { initialProps: status(20, 'preparing'), }); + expect(result.current).toBeNull(); rerender(status(45)); expect(result.current).toBe('About 1 minute left'); rerender(status(35, 'submitting', 'email-2')); diff --git a/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx b/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx index e55ade25df5..f71cbbed27d 100644 --- a/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx +++ b/apps/admin/src/posts/analytics/post-analytics.acceptance.test.tsx @@ -291,11 +291,17 @@ describe('Post analytics overview', () => { await expect .element(postAnalyticsScreen.emailSendingStatusBanner()) - .toHaveTextContent('Sending emails · 250 of 1,000'); + .toHaveTextContent(/Sending emails\s*250 of 1,000/); + await expect + .element(postAnalyticsScreen.emailSendingStatusBanner()) + .not.toHaveTextContent('minute'); estimate = 30; await expect .element(postAnalyticsScreen.emailSendingStatusBanner()) - .toHaveTextContent('Sending emails · 250 of 1,000 · Less than 1 minute left'); + .toHaveTextContent(/Sending emails\s*250 of 1,000/); + await expect + .element(postAnalyticsScreen.emailSendingStatusBanner()) + .toHaveTextContent('Less than 1 minute left'); }); it('moves a failed send and its retry action into the banner', async () => { @@ -461,7 +467,12 @@ describe('Post analytics overview', () => { boot: webAnalyticsBootOverrides(), }); - await expect.element(page.getByText('Preparing emails')).toBeVisible(); + await expect + .element(postAnalyticsScreen.emailSendingStatusBanner()) + .toHaveTextContent(/Preparing emails\s*10% complete · 1,000 total/); + await expect + .element(postAnalyticsScreen.emailSendingStatusBanner()) + .not.toHaveTextContent('minute'); // Advance the fake server only after the initial state is visible: extra // mount-time requests must not race the assertion straight into failure. const initialStatusRequests = statusRequestCount; diff --git a/apps/admin/src/posts/email-sending-status/use-sending-eta.ts b/apps/admin/src/posts/email-sending-status/use-sending-eta.ts index ae345d753d9..415625ab551 100644 --- a/apps/admin/src/posts/email-sending-status/use-sending-eta.ts +++ b/apps/admin/src/posts/email-sending-status/use-sending-eta.ts @@ -18,8 +18,9 @@ function etaMinutes(seconds: number, previous: number | null): number { export function useSendingEta(status: EmailSendingStatus | undefined): string | null { const sending = status?.sending; const key = status ? `${status.id}:${status.sending.status}` : null; - const isActive = sending?.status === 'preparing' || sending?.status === 'submitting'; - const seconds = isActive ? sending.progress.estimated_seconds_remaining : null; + // Database timing makes preparation estimates unreliable. + const seconds = + sending?.status === 'submitting' ? sending.progress.estimated_seconds_remaining : null; const [previous, setPrevious] = useState<{ key: string | null; minutes: number | null }>({ key: null, minutes: null, diff --git a/apps/admin/src/posts/list/posts-list-rows.acceptance.test.tsx b/apps/admin/src/posts/list/posts-list-rows.acceptance.test.tsx index 28d30026670..2ad1426742a 100644 --- a/apps/admin/src/posts/list/posts-list-rows.acceptance.test.tsx +++ b/apps/admin/src/posts/list/posts-list-rows.acceptance.test.tsx @@ -227,9 +227,8 @@ describe('Posts list email sending status', () => { }); const row = postsListScreen.listItems().first(); - await expect - .element(row) - .toHaveTextContent('Preparing emails · 250 of 1,000 · Less than 1 minute left'); + await expect.element(row).toHaveTextContent('Preparing emails · 250 of 1,000'); + await expect.element(row).not.toHaveTextContent('minute'); await expect.element(row).not.toHaveTextContent('Published and sent'); await expect.element(row.getByLabelText(/Visitors/)).toBeVisible(); await expect.element(row.getByLabelText(/Sent/)).not.toBeInTheDocument(); From df470855ec274eeb7acbc82d73454e4096ed4bd1 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Thu, 10 Sep 2026 09:50:53 -0500 Subject: [PATCH 07/10] Fixed the React editor leaving the post when an upload finds no session (#30655) no ref The React editor opts every request it owns out of the transport's session-expiry redirect, because navigating to the sign-in screen mid-edit takes unsaved content with it. Uploads were the one gap: the feature image, the Facebook and X card images, and the files Koenig cards accept all ran through hooks that had no way to ask for that policy, so a 401 during an upload dropped the writer out of the post. --- apps/admin-x-framework/src/api/images.ts | 9 +- .../src/hooks/use-koenig-file-upload.ts | 13 ++- .../test/unit/api/images.test.ts | 23 ----- .../test/unit/api/images.test.tsx | 98 +++++++++++++++++++ .../unit/hooks/use-koenig-file-upload.test.ts | 45 +++++++++ .../editor-feature-image.acceptance.test.tsx | 32 +++++- apps/admin/src/editor/editor-screen.tsx | 4 +- apps/admin/src/editor/feature-image.tsx | 3 +- apps/admin/src/editor/koenig-file-uploader.ts | 15 +++ apps/admin/src/editor/koenig-post-editor.tsx | 9 +- apps/admin/src/editor/publish/README.md | 2 + .../editor/settings/facebook-card-section.tsx | 5 +- .../src/editor/settings/revision-preview.tsx | 9 +- .../src/editor/settings/x-card-section.tsx | 5 +- 14 files changed, 226 insertions(+), 46 deletions(-) delete mode 100644 apps/admin-x-framework/test/unit/api/images.test.ts create mode 100644 apps/admin-x-framework/test/unit/api/images.test.tsx create mode 100644 apps/admin/src/editor/koenig-file-uploader.ts diff --git a/apps/admin-x-framework/src/api/images.ts b/apps/admin-x-framework/src/api/images.ts index c0b016ffa7a..2c731657843 100644 --- a/apps/admin-x-framework/src/api/images.ts +++ b/apps/admin-x-framework/src/api/images.ts @@ -8,7 +8,13 @@ export interface ImagesResponseType { }[]; } -export const useUploadImage = createMutation({ +export interface UploadImagePayload { + file: File; + /** False when the caller handles an expired session itself instead of leaving the page. */ + sessionExpiryRedirect?: boolean; +} + +export const useUploadImage = createMutation({ method: 'POST', path: () => '/images/upload/', body: ({ file }) => { @@ -17,6 +23,7 @@ export const useUploadImage = createMutation formData.append('purpose', 'image'); return formData; }, + requestOptions: ({ sessionExpiryRedirect }) => ({ sessionExpiryRedirect }), }); const UploadedImageResponseSchema = z.object({ diff --git a/apps/admin-x-framework/src/hooks/use-koenig-file-upload.ts b/apps/admin-x-framework/src/hooks/use-koenig-file-upload.ts index 917e747af99..93e76ef2935 100644 --- a/apps/admin-x-framework/src/hooks/use-koenig-file-upload.ts +++ b/apps/admin-x-framework/src/hooks/use-koenig-file-upload.ts @@ -1,6 +1,6 @@ import { useRef, useState } from 'react'; import { getGhostPaths } from '../utils/helpers'; -import { useFetchApi } from '../utils/api/fetch-api'; +import { useFetchApi, type RequestOptions } from '../utils/api/fetch-api'; export const koenigFileUploadTypes = { image: { @@ -55,6 +55,10 @@ interface UploadOptions { formData?: Record; } +type UploadRequestOptions = Pick; + +const DEFAULT_REQUEST_OPTIONS: UploadRequestOptions = {}; + interface UploadError { fileName: string; message: string; @@ -86,7 +90,11 @@ const getStringAtPath = (maybeObj: unknown, path: Iterable): null | return typeof current === 'string' ? current : null; }; -export const useKoenigFileUpload = (type: KoenigFileUploadType = 'image'): FileUploadHook => { +/** The session-expiry policy applies to every upload this hook makes. */ +export const useKoenigFileUpload = ( + type: KoenigFileUploadType = 'image', + requestOptions: UploadRequestOptions = DEFAULT_REQUEST_OPTIONS, +): FileUploadHook => { const [progress, setProgress] = useState(0); const [isLoading, setLoading] = useState(false); const [errors, setErrors] = useState([]); @@ -164,6 +172,7 @@ export const useKoenigFileUpload = (type: KoenigFileUploadType = 'image'): FileU try { const uploadResponse = await fetchApi(url, { + ...requestOptions, method: koenigFileUploadTypes[type].requestMethod, body: fileFormData, onUploadProgress(uploadProgress) { diff --git a/apps/admin-x-framework/test/unit/api/images.test.ts b/apps/admin-x-framework/test/unit/api/images.test.ts deleted file mode 100644 index 35b5a921d65..00000000000 --- a/apps/admin-x-framework/test/unit/api/images.test.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { getImageUrl } from '../../../src/api/images'; - -describe('getImageUrl', () => { - it.each(['https://example.com/image.png', '/content/images/image.png'])( - 'accepts an uploaded image URL (%s)', - (url) => { - expect(getImageUrl({ images: [{ url, ref: null }] })).toBe(url); - }, - ); - - it.each([ - null, - {}, - { images: [] }, - { images: [{}] }, - { images: [{ url: 123 }] }, - { images: [{ url: true }] }, - { images: [{ url: '' }] }, - ])('rejects a malformed upload response (%j)', (response) => { - expect(() => getImageUrl(response)).toThrow(); - }); -}); diff --git a/apps/admin-x-framework/test/unit/api/images.test.tsx b/apps/admin-x-framework/test/unit/api/images.test.tsx new file mode 100644 index 00000000000..dd69cfd88ca --- /dev/null +++ b/apps/admin-x-framework/test/unit/api/images.test.tsx @@ -0,0 +1,98 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import React, { ReactNode } from 'react'; +import { describe, expect, it, vi } from 'vitest'; +import { withMockFetch } from '../../utils/mock-fetch'; +import { FrameworkProvider } from '../../../src/providers/framework-provider'; + +const fetchApiCalls = vi.hoisted(() => [] as { endpoint: unknown; options: RequestOptionsLike }[]); + +interface RequestOptionsLike { + sessionExpiryRedirect?: boolean; +} + +// Wraps the real transport so requests still run, and records what each call asked for +vi.mock('../../../src/utils/api/fetch-api', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useFetchApi: () => { + const fetchApi = actual.useFetchApi(); + return (endpoint: unknown, options: RequestOptionsLike = {}) => { + fetchApiCalls.push({ endpoint, options }); + return (fetchApi as (...args: unknown[]) => Promise)(endpoint, options); + }; + }, + }; +}); + +import { getImageUrl, useUploadImage } from '../../../src/api/images'; + +const wrapper: React.FC<{ children: ReactNode }> = ({ children }) => ( + {}} + ghostVersion="5.x" + sentryDSN="" + unsplashConfig={{ + Authorization: '', + 'Accept-Version': '', + 'Content-Type': '', + 'App-Pragma': '', + 'X-Unsplash-Cache': true, + }} + onDelete={() => {}} + onInvalidate={() => {}} + onUpdate={() => {}} + > + {children} + +); + +describe('getImageUrl', () => { + it.each(['https://example.com/image.png', '/content/images/image.png'])( + 'accepts an uploaded image URL (%s)', + (url) => { + expect(getImageUrl({ images: [{ url, ref: null }] })).toBe(url); + }, + ); + + it.each([ + null, + {}, + { images: [] }, + { images: [{}] }, + { images: [{ url: 123 }] }, + { images: [{ url: true }] }, + { images: [{ url: '' }] }, + ])('rejects a malformed upload response (%j)', (response) => { + expect(() => getImageUrl(response)).toThrow(); + }); +}); + +describe('useUploadImage', () => { + const uploaded = { images: [{ url: 'https://example.com/image.png', ref: null }] }; + + const upload = async (payload: { file: File; sessionExpiryRedirect?: boolean }) => { + fetchApiCalls.length = 0; + await withMockFetch({ json: uploaded }, async () => { + const { result } = renderHook(() => useUploadImage(), { wrapper }); + await waitFor(() => expect(result.current.mutateAsync).toBeTypeOf('function')); + await result.current.mutateAsync(payload); + }); + return fetchApiCalls[0]?.options; + }; + + it('keeps the session-expiry redirect when the caller asks for nothing', async () => { + const options = await upload({ file: new File(['image'], 'hills.png') }); + + expect(options?.sessionExpiryRedirect).toBeUndefined(); + }); + + it('passes the session-expiry opt-out to the transport', async () => { + const options = await upload({ + file: new File(['image'], 'hills.png'), + sessionExpiryRedirect: false, + }); + + expect(options?.sessionExpiryRedirect).toBe(false); + }); +}); diff --git a/apps/admin-x-framework/test/unit/hooks/use-koenig-file-upload.test.ts b/apps/admin-x-framework/test/unit/hooks/use-koenig-file-upload.test.ts index 1406a15bed3..c1cce05a27a 100644 --- a/apps/admin-x-framework/test/unit/hooks/use-koenig-file-upload.test.ts +++ b/apps/admin-x-framework/test/unit/hooks/use-koenig-file-upload.test.ts @@ -4,6 +4,28 @@ import { type AddressInfo } from 'node:net'; import http from 'node:http'; import { promisify } from 'node:util'; import * as helpers from '../../../src/utils/helpers'; + +interface RequestOptionsLike { + sessionExpiryRedirect?: boolean; +} + +const fetchApiCalls = vi.hoisted(() => [] as RequestOptionsLike[]); + +// Wraps the real transport so uploads still run, and records what each call asked for +vi.mock('../../../src/utils/api/fetch-api', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useFetchApi: () => { + const fetchApi = actual.useFetchApi(); + return (endpoint: unknown, options: RequestOptionsLike = {}) => { + fetchApiCalls.push(options); + return (fetchApi as (...args: unknown[]) => Promise)(endpoint, options); + }; + }, + }; +}); + import { useKoenigFileUpload } from '../../../src/hooks/use-koenig-file-upload'; function makeFile(name: string, type = 'image/jpeg'): File { @@ -59,6 +81,7 @@ describe('useKoenigFileUpload', () => { beforeEach(async () => { uploadResponse = successfulUploadResponse; requestLog = []; + fetchApiCalls.length = 0; server = http.createServer((req, res) => { requestLog.push({ method: req.method, url: req.url }); @@ -303,6 +326,28 @@ describe('useKoenigFileUpload', () => { expect(result.current.errors).toHaveLength(0); }); + it('keeps the session-expiry redirect when the caller asks for nothing', async () => { + const { result } = renderHook(() => useKoenigFileUpload('image')); + + await act(async () => { + await result.current.upload([makeFile('photo.jpg')]); + }); + + expect(fetchApiCalls[0]?.sessionExpiryRedirect).toBeUndefined(); + }); + + it('passes the session-expiry opt-out to the transport', async () => { + const { result } = renderHook(() => + useKoenigFileUpload('image', { sessionExpiryRedirect: false }), + ); + + await act(async () => { + await result.current.upload([makeFile('photo.jpg')]); + }); + + expect(fetchApiCalls[0]?.sessionExpiryRedirect).toBe(false); + }); + it('accepts all supported image extensions', async () => { const supportedExtensions = ['gif', 'jpg', 'jpeg', 'png', 'svg', 'svgz', 'webp']; diff --git a/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx b/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx index 0619761a48d..f7431a7871c 100644 --- a/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx @@ -1,7 +1,8 @@ import { describe, expect, it } from 'vitest'; -import { userEvent } from 'vitest/browser'; +import { page, userEvent } from 'vitest/browser'; import { + currentRoute, fakeAdminEndpoint, fakeEditorChrome, fakeEditorPost, @@ -222,4 +223,33 @@ describe('Post editor feature image', () => { }, SLOW, ); + + it( + 'stays in the editor when the upload finds no session', + async () => { + fakeSavablePost(); + const uploadApi = fakeAdminEndpoint( + 'POST', + '/images/upload/', + { errors: [{ type: 'UnauthorizedError', message: 'Authorization failed' }] }, + { status: 401 }, + ); + await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); + + await expect.element(editorScreen.featureImage()).toBeVisible(); + await editorScreen.titleInput().fill('Brand New Name'); + await userEvent.upload( + editorScreen.featureImageInput().element(), + new File(['image'], 'hills.png', { type: 'image/png' }), + ); + + // A 401 mid-upload must not navigate away from work that is still unsaved. + await expect.poll(() => uploadApi.requests.length, SAVE_POLL).toBe(1); + await expect.element(editorScreen.titleInput()).toHaveValue('Brand New Name'); + expect(currentRoute()).toBe(`/editor/post/${POST_ID}`); + await expect.element(editorScreen.featureImageInput()).toBeInTheDocument(); + await expect.element(page.getByText('Couldn’t upload the feature image.')).toBeVisible(); + }, + SLOW, + ); }); diff --git a/apps/admin/src/editor/editor-screen.tsx b/apps/admin/src/editor/editor-screen.tsx index 192834fdf48..0bccb336cde 100644 --- a/apps/admin/src/editor/editor-screen.tsx +++ b/apps/admin/src/editor/editor-screen.tsx @@ -304,8 +304,8 @@ function useLexicalConversion(postType: PostType) { try { const record: EditorRecord | undefined = postType === 'page' - ? (await editPage({ page: payload, options })).pages[0] - : (await editPost({ post: payload, options })).posts[0]; + ? (await editPage({ page: payload, options, ...EDITOR_REQUEST_OPTIONS })).pages[0] + : (await editPost({ post: payload, options, ...EDITOR_REQUEST_OPTIONS })).posts[0]; setState(record ? { id: source.id, record } : { id: source.id, error: true }); } catch (error) { setState({ id: source.id, error }); diff --git a/apps/admin/src/editor/feature-image.tsx b/apps/admin/src/editor/feature-image.tsx index c51e16d6c64..b1aa6e2e887 100644 --- a/apps/admin/src/editor/feature-image.tsx +++ b/apps/admin/src/editor/feature-image.tsx @@ -21,6 +21,7 @@ import { import type { KoenigInstance } from '@/settings/components/koenig-loader'; import type { PostCardConfig } from './card-config'; import { FeatureImageCaption } from './feature-image-caption'; +import { EDITOR_REQUEST_OPTIONS } from './request-options'; import { UnsplashPicker } from './unsplash-picker'; const ALT_MAX_LENGTH = 191; @@ -67,7 +68,7 @@ export function FeatureImage({ const handleUpload = useCallback( async (file: File) => { try { - onImageChange(getImageUrl(await uploadImage({ file }))); + onImageChange(getImageUrl(await uploadImage({ file, ...EDITOR_REQUEST_OPTIONS }))); } catch (error) { toast.error(uploadErrorMessage(error, IMAGE_SUBJECT)); } diff --git a/apps/admin/src/editor/koenig-file-uploader.ts b/apps/admin/src/editor/koenig-file-uploader.ts new file mode 100644 index 00000000000..3dbfa87a543 --- /dev/null +++ b/apps/admin/src/editor/koenig-file-uploader.ts @@ -0,0 +1,15 @@ +import { + koenigFileUploadTypes, + useKoenigFileUpload, + type KoenigFileUploadType, +} from '@tryghost/admin-x-framework/hooks'; +import { EDITOR_REQUEST_OPTIONS } from './request-options'; + +const useEditorFileUpload = (type: KoenigFileUploadType = 'image') => + useKoenigFileUpload(type, EDITOR_REQUEST_OPTIONS); + +/** The uploader Koenig cards use inside the editor, on the editor's request policy. */ +export const editorFileUploader = { + useFileUpload: useEditorFileUpload, + fileTypes: koenigFileUploadTypes, +}; diff --git a/apps/admin/src/editor/koenig-post-editor.tsx b/apps/admin/src/editor/koenig-post-editor.tsx index 52013a4b0d5..f2b790efcef 100644 --- a/apps/admin/src/editor/koenig-post-editor.tsx +++ b/apps/admin/src/editor/koenig-post-editor.tsx @@ -1,6 +1,5 @@ import { Suspense, useCallback, useMemo } from 'react'; import { LoadingIndicator } from '@tryghost/shade/components'; -import { koenigFileUploadTypes, useKoenigFileUpload } from '@tryghost/admin-x-framework/hooks'; import ErrorBoundary from '@/settings/components/error-boundary'; import { type EditorResource, @@ -9,11 +8,7 @@ import { } from '@/settings/components/koenig-loader'; import type { PostCardConfig } from './card-config'; import { reportKoenigError } from './koenig-error'; - -const fileUploader = { - useFileUpload: useKoenigFileUpload, - fileTypes: koenigFileUploadTypes, -}; +import { editorFileUploader } from './koenig-file-uploader'; const NOOP = () => {}; @@ -68,7 +63,7 @@ function KoenigInstanceMount({ { try { - session.editSettings({ og_image: getImageUrl(await uploadImage({ file })) }); + session.editSettings({ + og_image: getImageUrl(await uploadImage({ file, ...EDITOR_REQUEST_OPTIONS })), + }); } catch (error) { toast.error(uploadErrorMessage(error, IMAGE_SUBJECT)); } diff --git a/apps/admin/src/editor/settings/revision-preview.tsx b/apps/admin/src/editor/settings/revision-preview.tsx index bdc1d3f0076..9d7333b353b 100644 --- a/apps/admin/src/editor/settings/revision-preview.tsx +++ b/apps/admin/src/editor/settings/revision-preview.tsx @@ -2,7 +2,6 @@ import DOMPurify from 'dompurify'; import { Suspense, useCallback, useMemo } from 'react'; import { LoadingIndicator } from '@tryghost/shade/components'; import { Inline, Text } from '@tryghost/shade/primitives'; -import { koenigFileUploadTypes, useKoenigFileUpload } from '@tryghost/admin-x-framework/hooks'; import { postHistoryPreview, postHistoryPreviewBody, @@ -18,13 +17,9 @@ import { } from '@/settings/components/koenig-loader'; import type { PostCardConfig } from '@/editor/card-config'; import { reportKoenigError } from '@/editor/koenig-error'; +import { editorFileUploader } from '@/editor/koenig-file-uploader'; import type { RevisionEntry } from './post-history'; -const fileUploader = { - useFileUpload: useKoenigFileUpload, - fileTypes: koenigFileUploadTypes, -}; - /** The part of Lexical's editor the loader's minimal instance type leaves out. */ type LexicalEditable = { setEditable: (editable: boolean) => void }; @@ -62,7 +57,7 @@ function RevisionBody({ diff --git a/apps/admin/src/editor/settings/x-card-section.tsx b/apps/admin/src/editor/settings/x-card-section.tsx index 61b55fd40a6..ebbf6c2dbc2 100644 --- a/apps/admin/src/editor/settings/x-card-section.tsx +++ b/apps/admin/src/editor/settings/x-card-section.tsx @@ -29,6 +29,7 @@ import { uploadErrorMessage, } from '@/shared/images/image-upload'; import type { PostCardConfig } from '@/editor/card-config'; +import { EDITOR_REQUEST_OPTIONS } from '@/editor/request-options'; import { X_DESCRIPTION_MAX, X_DESCRIPTION_TOO_LONG, @@ -104,7 +105,9 @@ export function XCardSection({ session, siteUrl, featureImage, cardConfig }: XCa const handleUpload = useCallback( async (file: File) => { try { - session.editSettings({ twitter_image: getImageUrl(await uploadImage({ file })) }); + session.editSettings({ + twitter_image: getImageUrl(await uploadImage({ file, ...EDITOR_REQUEST_OPTIONS })), + }); } catch (error) { toast.error(uploadErrorMessage(error, IMAGE_SUBJECT)); } From 6ab8d6a9e8fd75897f553efea9701c4d1ba9360b Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Thu, 10 Sep 2026 10:30:24 -0500 Subject: [PATCH 08/10] Changed the React editor to use Shade's FieldError (#30656) no ref Unified to a Shade component. --- .../editor-settings-access.acceptance.test.tsx | 8 +++++++- ...itor-settings-meta-data.acceptance.test.tsx | 2 +- apps/admin/src/editor/editor-status.tsx | 2 +- apps/admin/src/editor/editor.screen.ts | 3 +-- .../src/editor/settings/access-section.tsx | 18 ++++++++++++++---- .../src/editor/settings/authors-section.tsx | 13 +++---------- .../editor/settings/facebook-card-section.tsx | 7 +++---- apps/admin/src/editor/settings/field-error.tsx | 10 ---------- .../src/editor/settings/meta-data-section.tsx | 11 ++++++----- .../editor/settings/publish-date-section.tsx | 12 +++--------- apps/admin/src/editor/settings/url-section.tsx | 12 +++--------- .../src/editor/settings/x-card-section.tsx | 7 +++---- 12 files changed, 45 insertions(+), 60 deletions(-) delete mode 100644 apps/admin/src/editor/settings/field-error.tsx diff --git a/apps/admin/src/editor/editor-settings-access.acceptance.test.tsx b/apps/admin/src/editor/editor-settings-access.acceptance.test.tsx index 80ced8dab2f..64705683256 100644 --- a/apps/admin/src/editor/editor-settings-access.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-settings-access.acceptance.test.tsx @@ -194,7 +194,13 @@ describe('Post settings access', () => { await chooseVisibility('Specific tier(s)'); // Nothing is selected yet, so the choice is held back rather than stripped. - await expect.element(editorScreen.settingsTiersError()).toBeVisible(); + await expect + .element(editorScreen.settingsTiersError()) + .toHaveTextContent('Please select at least one tier'); + await expect.element(editorScreen.settingsTiers()).toHaveAttribute('aria-invalid', 'true'); + await expect + .element(editorScreen.settingsTiers()) + .toHaveAttribute('aria-describedby', editorScreen.settingsTiersError().element().id); expect(saveApi.requests).toHaveLength(0); // Archived paid tiers are offered after the active ones; free tiers are not. await expect.element(editorScreen.settingsTier('Bronze')).toBeVisible(); diff --git a/apps/admin/src/editor/editor-settings-meta-data.acceptance.test.tsx b/apps/admin/src/editor/editor-settings-meta-data.acceptance.test.tsx index d0a3def24e2..3fc2e502835 100644 --- a/apps/admin/src/editor/editor-settings-meta-data.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-settings-meta-data.acceptance.test.tsx @@ -68,7 +68,7 @@ async function openMetaData() { /** Whether the countdown is showing the writer they are past the recommendation. */ function countdownIsOver(): boolean { - return !!editorScreen.settingsSubviewPane().element().querySelector('span.text-red'); + return !!editorScreen.settingsSubviewPane().element().querySelector('span.text-destructive'); } /** diff --git a/apps/admin/src/editor/editor-status.tsx b/apps/admin/src/editor/editor-status.tsx index 05e03db12e9..cdb5f5e9edb 100644 --- a/apps/admin/src/editor/editor-status.tsx +++ b/apps/admin/src/editor/editor-status.tsx @@ -66,7 +66,7 @@ function StatusBody({ }) { switch (view.kind) { case 'problem': - return {view.message}; + return {view.message}; case 'saving': return Saving…; case 'new': diff --git a/apps/admin/src/editor/editor.screen.ts b/apps/admin/src/editor/editor.screen.ts index 322956cc75f..1b9e25721ba 100644 --- a/apps/admin/src/editor/editor.screen.ts +++ b/apps/admin/src/editor/editor.screen.ts @@ -93,7 +93,6 @@ import { settingsTagsToken, settingsTemplateSelect, settingsTemplateSlugMatch, - settingsTiersError, settingsTiersPicker, settingsUrlPreview, settingsVisibilitySelect, @@ -178,7 +177,7 @@ export const editorScreen = { settingsTiers: () => page.getByTestId(settingsTiersPicker), settingsTier: (name: string) => page.getByTestId(settingsTiersPicker).getByRole('checkbox', { name }), - settingsTiersError: () => page.getByTestId(settingsTiersError), + settingsTiersError: () => page.getByTestId(settingsTiersPicker).getByRole('alert'), settingsTagsField: () => page.getByTestId(settingsTagsField), settingsTagsInput: () => page.getByTestId(settingsTagsInput), settingsTagsTokens: () => page.getByTestId(settingsTagsToken), diff --git a/apps/admin/src/editor/settings/access-section.tsx b/apps/admin/src/editor/settings/access-section.tsx index 45b9ed69303..d8f0bd8850c 100644 --- a/apps/admin/src/editor/settings/access-section.tsx +++ b/apps/admin/src/editor/settings/access-section.tsx @@ -1,6 +1,7 @@ import { useId } from 'react'; import { Checkbox, + FieldError, Label, Select, SelectContent, @@ -94,6 +95,7 @@ export interface AccessSectionProps { */ export function AccessSection({ session, postType }: AccessSectionProps) { const selectId = useId(); + const tiersErrorId = useId(); const { data: settingsData } = useBrowseSettings({ defaultErrorHandler: false, requestOptions: EDITOR_REQUEST_OPTIONS, @@ -105,6 +107,7 @@ export function AccessSection({ session, postType }: AccessSectionProps) { const visibility = selectedVisibility(session.settings.visibility, defaultContentVisibility); const selected = new Set(selectedTierIds(session.settings.tiers)); + const tiersMissing = tiersIncomplete(session.settings); const { data: tiersData } = useBrowseTiers({ defaultErrorHandler: false, @@ -146,7 +149,14 @@ export function AccessSection({ session, postType }: AccessSectionProps) { {visibility === 'tiers' ? ( - + !option.archived)} @@ -159,10 +169,10 @@ export function AccessSection({ session, postType }: AccessSectionProps) { selected={selected} onToggle={toggleTier} /> - {tiersIncomplete(session.settings) ? ( - + {tiersMissing ? ( + {TIERS_REQUIRED} - + ) : null} ) : null} diff --git a/apps/admin/src/editor/settings/authors-section.tsx b/apps/admin/src/editor/settings/authors-section.tsx index b8d57615e98..952657be8eb 100644 --- a/apps/admin/src/editor/settings/authors-section.tsx +++ b/apps/admin/src/editor/settings/authors-section.tsx @@ -1,6 +1,5 @@ import { useCallback, useId, useState } from 'react'; -import { Label } from '@tryghost/shade/components'; -import { Text } from '@tryghost/shade/primitives'; +import { FieldError, Label } from '@tryghost/shade/components'; import { useBrowseUsers, type User } from '@tryghost/admin-x-framework/api/users'; import type { PostAuthor } from '@tryghost/admin-x-framework/api/posts'; import { settingsAuthorsError } from '@tryghost/test-data/selectors/editor'; @@ -61,15 +60,9 @@ export function AuthorsSection({ session, currentUser }: AuthorsSectionProps) { onRetry={() => void refetch()} /> {invalid ? ( - + {AUTHORS_REQUIRED} - + ) : null} ); diff --git a/apps/admin/src/editor/settings/facebook-card-section.tsx b/apps/admin/src/editor/settings/facebook-card-section.tsx index d0c8b98e7bf..4427a983da4 100644 --- a/apps/admin/src/editor/settings/facebook-card-section.tsx +++ b/apps/admin/src/editor/settings/facebook-card-section.tsx @@ -1,6 +1,6 @@ import { useCallback, useId } from 'react'; import { toast } from 'sonner'; -import { Input, Label, LoadingIndicator, Textarea } from '@tryghost/shade/components'; +import { FieldError, Input, Label, LoadingIndicator, Textarea } from '@tryghost/shade/components'; import { ImageUpload, ImageUploadAction, @@ -36,7 +36,6 @@ import { } from '@/editor/session/settings-fields'; import type { EditorSessionHandle } from '@/editor/session/use-editor-session'; import { UnsplashPicker } from '@/editor/unsplash-picker'; -import { FieldError } from './field-error'; import { truncate } from './meta-data-fields'; import { SettingsSubview } from './settings-subview'; import { @@ -183,7 +182,7 @@ export function FacebookCardSection({ // A cleared field is stored as no value, the way the excerpt is. onChange={(event) => session.stageSettings({ og_title: event.target.value || null })} /> - {titleError ? : null} + {titleError ? {titleError} : null} @@ -202,7 +201,7 @@ export function FacebookCardSection({ } /> {descriptionError ? ( - + {descriptionError} ) : null} diff --git a/apps/admin/src/editor/settings/field-error.tsx b/apps/admin/src/editor/settings/field-error.tsx deleted file mode 100644 index d3a0f4bcf14..00000000000 --- a/apps/admin/src/editor/settings/field-error.tsx +++ /dev/null @@ -1,10 +0,0 @@ -import { Text } from '@tryghost/shade/primitives'; - -/** What a settings field says about a value it will not save. */ -export function FieldError({ id, message }: { id: string; message: string }) { - return ( - - {message} - - ); -} diff --git a/apps/admin/src/editor/settings/meta-data-section.tsx b/apps/admin/src/editor/settings/meta-data-section.tsx index da55d97ddcf..4d2a692a7b2 100644 --- a/apps/admin/src/editor/settings/meta-data-section.tsx +++ b/apps/admin/src/editor/settings/meta-data-section.tsx @@ -1,5 +1,5 @@ import { useId } from 'react'; -import { Input, Label, Textarea } from '@tryghost/shade/components'; +import { FieldError, Input, Label, Textarea } from '@tryghost/shade/components'; import { Stack, Text } from '@tryghost/shade/primitives'; import { LucideIcon, cn, formatNumber } from '@tryghost/shade/utils'; import { @@ -15,7 +15,6 @@ import { overLength, } from '@/editor/session/settings-fields'; import type { EditorSessionHandle } from '@/editor/session/use-editor-session'; -import { FieldError } from './field-error'; import { META_DESCRIPTION_RECOMMENDED, META_TITLE_RECOMMENDED, @@ -36,7 +35,9 @@ function Countdown({ id, value, recommended }: { id: string; value: string; reco return ( Recommended: {formatNumber(recommended)} characters. You've used{' '} - recommended ? 'text-red' : 'text-green')}> + recommended ? 'text-destructive' : 'text-state-success')} + > {formatNumber(used)} @@ -126,7 +127,7 @@ export function MetaDataSection({ session, siteUrl }: MetaDataSectionProps) { onChange={(event) => session.stageSettings({ meta_title: event.target.value || null })} /> - {titleError ? : null} + {titleError ? {titleError} : null} @@ -152,7 +153,7 @@ export function MetaDataSection({ session, siteUrl }: MetaDataSectionProps) { value={metaDescription} /> {descriptionError ? ( - + {descriptionError} ) : null} diff --git a/apps/admin/src/editor/settings/publish-date-section.tsx b/apps/admin/src/editor/settings/publish-date-section.tsx index bed5342c6d5..4425945b83a 100644 --- a/apps/admin/src/editor/settings/publish-date-section.tsx +++ b/apps/admin/src/editor/settings/publish-date-section.tsx @@ -1,5 +1,5 @@ import { useId } from 'react'; -import { Label } from '@tryghost/shade/components'; +import { FieldError, Label } from '@tryghost/shade/components'; import { Text } from '@tryghost/shade/primitives'; import { getSettingValue, useBrowseSettings } from '@tryghost/admin-x-framework/api/settings'; import { @@ -65,15 +65,9 @@ export function PublishDateSection({ session }: PublishDateSectionProps) { onChange={(date) => session.editPublishedAt(date.toISOString())} /> {invalid ? ( - + {PUBLISHED_AT_MUST_BE_PAST} - + ) : null} {isScheduled && !isPastScheduled ? ( diff --git a/apps/admin/src/editor/settings/url-section.tsx b/apps/admin/src/editor/settings/url-section.tsx index f58cd3726f3..7daf62cd2de 100644 --- a/apps/admin/src/editor/settings/url-section.tsx +++ b/apps/admin/src/editor/settings/url-section.tsx @@ -1,5 +1,5 @@ import { type KeyboardEvent, useCallback, useId, useState } from 'react'; -import { Input, Label } from '@tryghost/shade/components'; +import { FieldError, Input, Label } from '@tryghost/shade/components'; import { Text } from '@tryghost/shade/primitives'; import { settingsSlugError, @@ -74,15 +74,9 @@ export function UrlSection({ onKeyDown={onKeyDown} /> {failed ? ( - + {EDIT_FAILED} - + ) : null} {formatUrlPreview(siteUrl, value)} diff --git a/apps/admin/src/editor/settings/x-card-section.tsx b/apps/admin/src/editor/settings/x-card-section.tsx index ebbf6c2dbc2..b53143b7a64 100644 --- a/apps/admin/src/editor/settings/x-card-section.tsx +++ b/apps/admin/src/editor/settings/x-card-section.tsx @@ -1,6 +1,6 @@ import { useCallback, useId } from 'react'; import { toast } from 'sonner'; -import { Input, Label, LoadingIndicator, Textarea } from '@tryghost/shade/components'; +import { FieldError, Input, Label, LoadingIndicator, Textarea } from '@tryghost/shade/components'; import { ImageUpload, ImageUploadAction, @@ -39,7 +39,6 @@ import { } from '@/editor/session/settings-fields'; import type { EditorSessionHandle } from '@/editor/session/use-editor-session'; import { UnsplashPicker } from '@/editor/unsplash-picker'; -import { FieldError } from './field-error'; import { truncate } from './meta-data-fields'; import { SettingsSubview } from './settings-subview'; import { @@ -179,7 +178,7 @@ export function XCardSection({ session, siteUrl, featureImage, cardConfig }: XCa // A cleared field is stored as no value, the way the excerpt is. onChange={(event) => session.stageSettings({ twitter_title: event.target.value || null })} /> - {titleError ? : null} + {titleError ? {titleError} : null} @@ -198,7 +197,7 @@ export function XCardSection({ session, siteUrl, featureImage, cardConfig }: XCa } /> {descriptionError ? ( - + {descriptionError} ) : null} From d422e01d4d6fdac40a476627f6c1486584b8c9da Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Thu, 10 Sep 2026 10:30:59 -0500 Subject: [PATCH 09/10] Changed the editor's Unsplash search to answer only its own Escape (#30657) no ref Cleaned up the Escape handling for Unsplash's modal. --- apps/admin/package.json | 2 + .../editor-feature-image.acceptance.test.tsx | 23 +++++ ...settings-facebook-card.acceptance.test.tsx | 45 +--------- ...editor-settings-x-card.acceptance.test.tsx | 77 ++++++++--------- apps/admin/src/editor/editor.screen.ts | 5 +- apps/admin/src/editor/settings/README.md | 12 +++ .../settings/code-injection-section.tsx | 4 +- .../admin/src/editor/unsplash-picker.test.tsx | 86 +++++++++++++++++++ apps/admin/src/editor/unsplash-picker.tsx | 60 +++++++++---- apps/admin/src/shared/images/image-upload.ts | 2 +- apps/admin/test-utils/acceptance/editor.ts | 42 ++++++++- apps/admin/test-utils/acceptance/index.ts | 9 +- .../testing/test-data/src/selectors/editor.ts | 1 + pnpm-lock.yaml | 12 +++ pnpm-workspace.yaml | 2 + 15 files changed, 278 insertions(+), 104 deletions(-) create mode 100644 apps/admin/src/editor/unsplash-picker.test.tsx diff --git a/apps/admin/package.json b/apps/admin/package.json index 5a911904f1f..8a2614731de 100644 --- a/apps/admin/package.json +++ b/apps/admin/package.json @@ -25,6 +25,8 @@ "@codemirror/state": "catalog:", "@codemirror/theme-one-dark": "catalog:", "@dnd-kit/sortable": "catalog:", + "@radix-ui/react-focus-guards": "catalog:", + "@radix-ui/react-focus-scope": "catalog:", "@sentry/react": "catalog:", "@svg-maps/world": "2.0.0", "@tanstack/react-query": "catalog:", diff --git a/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx b/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx index f7431a7871c..4991a873b84 100644 --- a/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-feature-image.acceptance.test.tsx @@ -2,10 +2,12 @@ import { describe, expect, it } from 'vitest'; import { page, userEvent } from 'vitest/browser'; import { + UNSPLASH_PICKED, currentRoute, fakeAdminEndpoint, fakeEditorChrome, fakeEditorPost, + fakeUnsplashPhotos, post, renderAdminApp, submittedPost, @@ -201,6 +203,27 @@ describe('Post editor feature image', () => { SLOW, ); + it( + 'saves an image picked from Unsplash with the credit it carries', + async () => { + const saveApi = fakeSavablePost(); + fakeUnsplashPhotos(); + await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); + + await expect.element(editorScreen.featureImageUnsplashButton()).toBeVisible(); + await editorScreen.featureImageUnsplashButton().click(); + await editorScreen.unsplashInsertImage().click(); + + await expect.poll(() => saveApi.requests.length, SAVE_POLL).toBe(1); + const saved = submittedPost(saveApi); + expect(saved.feature_image).toBe(UNSPLASH_PICKED); + // The photographer credit the picker hands over, as the caption stores it. + expect(String(saved.feature_image_caption)).toContain('A Photographer'); + await expect.element(editorScreen.removeFeatureImage()).toBeVisible(); + }, + SLOW, + ); + it( 'clears the alt text and caption along with the image', async () => { diff --git a/apps/admin/src/editor/editor-settings-facebook-card.acceptance.test.tsx b/apps/admin/src/editor/editor-settings-facebook-card.acceptance.test.tsx index a5ee8c917e7..4c41319c301 100644 --- a/apps/admin/src/editor/editor-settings-facebook-card.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-settings-facebook-card.acceptance.test.tsx @@ -2,18 +2,19 @@ import { describe, expect, it } from 'vitest'; import { userEvent } from 'vitest/browser'; import { + UNSPLASH_PICKED, currentUserResponse, fakeAdminEndpoint, fakeEditorChrome, fakeEditorPost, - fakeEndpoint, fakeTiers, + fakeUnsplashPhotos, post, renderAdminApp, - settingsResponse, staffRole, submittedPost, unsavedChangesGuarded, + withoutUnsplash, type StaffRoleName, } from '@test-utils/acceptance'; import { editorScreen } from '@/editor/editor.screen'; @@ -67,44 +68,6 @@ function fakeSavablePost(overrides: Partial = {}) { }); } -const UNSPLASH_REGULAR = 'https://images.unsplash.com/photo-1?ixid=1&w=1080'; -// The picker asks Unsplash for a wider rendition of the image it inserts. -const UNSPLASH_PICKED = 'https://images.unsplash.com/photo-1?ixid=1&w=2000'; - -/** One Unsplash photo, in the shape the search modal lays out and inserts. */ -function fakeUnsplashPhotos() { - fakeEndpoint('GET', 'https://api.unsplash.com/photos', [ - { - id: 'photo-1', - color: '#123456', - alt_description: 'A hillside', - height: 800, - width: 1200, - likes: 12, - urls: { regular: UNSPLASH_REGULAR }, - links: { - html: 'https://unsplash.com/photos/photo-1', - download: 'https://unsplash.com/photos/photo-1/download', - download_location: 'https://api.unsplash.com/photos/photo-1/download', - }, - user: { - name: 'A Photographer', - links: { html: 'https://unsplash.com/@photographer' }, - profile_image: { medium: 'https://images.unsplash.com/profile-1' }, - }, - }, - ]); - fakeEndpoint('GET', 'https://api.unsplash.com/photos/photo-1/download', {}); -} - -/** The site fixture turns Unsplash on, so only the off case needs an override. */ -function withoutUnsplash() { - return { - ...FLAG_ON, - boot: { browseSettings: { response: settingsResponse({ settings: { unsplash: false } }) } }, - }; -} - async function openFacebookCard() { await editorScreen.settingsToggle().click(); await expect.element(editorScreen.settingsSidebar()).toBeVisible(); @@ -378,7 +341,7 @@ describe('Post settings Facebook card', () => { 'leaves Unsplash out while the site’s integration is off', async () => { fakeSavablePost(); - await renderAdminApp(`/editor/post/${POST_ID}`, withoutUnsplash()); + await renderAdminApp(`/editor/post/${POST_ID}`, { ...FLAG_ON, ...withoutUnsplash() }); await openFacebookCard(); await expect.element(editorScreen.settingsFacebookImageInput()).toBeInTheDocument(); diff --git a/apps/admin/src/editor/editor-settings-x-card.acceptance.test.tsx b/apps/admin/src/editor/editor-settings-x-card.acceptance.test.tsx index 5d39f8603fc..bc13e026135 100644 --- a/apps/admin/src/editor/editor-settings-x-card.acceptance.test.tsx +++ b/apps/admin/src/editor/editor-settings-x-card.acceptance.test.tsx @@ -2,18 +2,19 @@ import { describe, expect, it } from 'vitest'; import { page, userEvent } from 'vitest/browser'; import { + UNSPLASH_PICKED, currentUserResponse, fakeAdminEndpoint, fakeEditorChrome, fakeEditorPost, - fakeEndpoint, fakeTiers, + fakeUnsplashPhotos, post, renderAdminApp, - settingsResponse, staffRole, submittedPost, unsavedChangesGuarded, + withoutUnsplash, type StaffRoleName, } from '@test-utils/acceptance'; import { editorScreen } from '@/editor/editor.screen'; @@ -73,44 +74,6 @@ function fakeImageUpload() { }); } -const UNSPLASH_REGULAR = 'https://images.unsplash.com/photo-1?ixid=1&w=1080'; -// The picker asks Unsplash for a wider rendition of the image it inserts. -const UNSPLASH_PICKED = 'https://images.unsplash.com/photo-1?ixid=1&w=2000'; - -/** One Unsplash photo, in the shape the search modal lays out and inserts. */ -function fakeUnsplashPhotos() { - fakeEndpoint('GET', 'https://api.unsplash.com/photos', [ - { - id: 'photo-1', - color: '#123456', - alt_description: 'A hillside', - height: 800, - width: 1200, - likes: 12, - urls: { regular: UNSPLASH_REGULAR }, - links: { - html: 'https://unsplash.com/photos/photo-1', - download: 'https://unsplash.com/photos/photo-1/download', - download_location: 'https://api.unsplash.com/photos/photo-1/download', - }, - user: { - name: 'A Photographer', - links: { html: 'https://unsplash.com/@photographer' }, - profile_image: { medium: 'https://images.unsplash.com/profile-1' }, - }, - }, - ]); - fakeEndpoint('GET', 'https://api.unsplash.com/photos/photo-1/download', {}); -} - -/** The site fixture turns Unsplash on, so only the off case needs an override. */ -function withoutUnsplash() { - return { - ...FLAG_ON, - boot: { browseSettings: { response: settingsResponse({ settings: { unsplash: false } }) } }, - }; -} - async function openXCard() { await editorScreen.settingsToggle().click(); await expect.element(editorScreen.settingsSidebar()).toBeVisible(); @@ -517,7 +480,7 @@ describe('Post settings X card', () => { 'leaves Unsplash out while the site’s integration is off', async () => { fakeSavablePost(); - await renderAdminApp(`/editor/post/${POST_ID}`, withoutUnsplash()); + await renderAdminApp(`/editor/post/${POST_ID}`, { ...FLAG_ON, ...withoutUnsplash() }); await openXCard(); await expect.element(editorScreen.settingsXImageInput()).toBeInTheDocument(); @@ -546,6 +509,38 @@ describe('Post settings X card', () => { SLOW, ); + it( + 'keeps keyboard navigation inside Unsplash and restores focus after Escape', + async () => { + fakeSavablePost(); + fakeUnsplashPhotos(); + await renderAdminApp(`/editor/post/${POST_ID}`, FLAG_ON); + await openXCard(); + + await editorScreen.settingsXImageUnsplashButton().click(); + await expect.element(editorScreen.unsplashSearchInput()).toHaveFocus(); + await expect.element(editorScreen.unsplashInsertImage()).toBeVisible(); + + // Back past the close button: focus must wrap inside the search rather + // than reaching the picker button in the pane behind it. + await userEvent.keyboard('{Shift>}{Tab}{Tab}{/Shift}'); + expect(editorScreen.unsplashSearch().element().contains(document.activeElement)).toBe(true); + + // Traverse past the close button, search field and one photo's links. + // Every stop stays inside, including the forward wrap. + for (let i = 0; i < 6; i++) { + await userEvent.keyboard('{Tab}'); + expect(editorScreen.unsplashSearch().element().contains(document.activeElement)).toBe(true); + } + await userEvent.keyboard('{Escape}'); + + await expect(editorScreen.unsplashModal()).toHaveCount(0); + await expect.element(editorScreen.settingsSubviewPane()).toBeVisible(); + await expect.element(editorScreen.settingsXImageUnsplashButton()).toHaveFocus(); + }, + SLOW, + ); + it( 'keeps the pane open when Escape dismisses the Unsplash search', async () => { diff --git a/apps/admin/src/editor/editor.screen.ts b/apps/admin/src/editor/editor.screen.ts index 1b9e25721ba..2680a3679be 100644 --- a/apps/admin/src/editor/editor.screen.ts +++ b/apps/admin/src/editor/editor.screen.ts @@ -100,6 +100,7 @@ import { stayInEditorButton, tkIndicator, toggleFeatureImageAltButton, + unsplashSearchModal, } from '@tryghost/test-data/selectors/editor'; /** Editor screen locators and gestures for acceptance specs; no assertions. */ @@ -284,7 +285,9 @@ export const editorScreen = { featureImageUnsplashButton: () => page.getByRole('button', { name: featureImageUnsplashButton }), /** The Unsplash search modal, wherever the picker that opened it sits. */ unsplashModal: () => page.getByRole('heading', { name: 'Unsplash' }), - unsplashInsertImage: () => page.getByText('Insert image'), + unsplashSearch: () => page.getByTestId(unsplashSearchModal), + unsplashSearchInput: () => page.getByPlaceholder('Search free high-resolution photos'), + unsplashInsertImage: () => page.getByTestId(unsplashSearchModal).getByText('Insert image'), removeFeatureImage: () => page.getByRole('button', { name: removeFeatureImageButton }), featureImageAltToggle: () => page.getByRole('button', { name: toggleFeatureImageAltButton }), featureImageAltInput: () => page.getByLabelText(featureImageAltLabel), diff --git a/apps/admin/src/editor/settings/README.md b/apps/admin/src/editor/settings/README.md index 13986cad949..9622a362846 100644 --- a/apps/admin/src/editor/settings/README.md +++ b/apps/admin/src/editor/settings/README.md @@ -196,6 +196,18 @@ that cannot write it, falls back to the section list rather than an empty panel. The panel owns which pane is open, so closing the panel or leaving the editor drops it and the panel is next opened on the section list. +## Escape + +Escape closes one layer, the innermost the writer is in. In a tag or author list +it closes the list and keeps the term that was typed. In a code injection editor +it frees the editor's Tab and closes nothing. In a dialog, a select or an +uploader it closes that control. In the Unsplash search it closes the search and +leaves the field it was opened from. With none of those open it closes the pane, +and with no pane open the sidebar answers Escape with nothing. + +The Unsplash search traps focus and loops Tab navigation in both directions. +Closing it returns focus to the picker button when that field is still present. + ## Access Access is two coupled fields, `visibility` and `tiers`, and only an Owner, diff --git a/apps/admin/src/editor/settings/code-injection-section.tsx b/apps/admin/src/editor/settings/code-injection-section.tsx index aca5f0ccbe1..42455a5a1c3 100644 --- a/apps/admin/src/editor/settings/code-injection-section.tsx +++ b/apps/admin/src/editor/settings/code-injection-section.tsx @@ -4,8 +4,8 @@ import type { PostType } from '@/editor/card-config'; import type { EditorSessionHandle } from '@/editor/session/use-editor-session'; import { SettingsSubview } from './settings-subview'; -// An Escape no other binding answers leaves the event unprevented, closing the -// pane. Arm tab-focus mode for the 2s @codemirror/view does, so Tab leaves. +// A binding that returns true prevents the event's default, which the pane +// reads as answered. Arm tab-focus mode for the 2s @codemirror/view does. const TAB_FOCUS_ESCAPE = () => import('@uiw/react-codemirror').then(({ Prec, keymap }) => Prec.lowest( diff --git a/apps/admin/src/editor/unsplash-picker.test.tsx b/apps/admin/src/editor/unsplash-picker.test.tsx new file mode 100644 index 00000000000..f5c3e010170 --- /dev/null +++ b/apps/admin/src/editor/unsplash-picker.test.tsx @@ -0,0 +1,86 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { UnsplashPicker } from './unsplash-picker'; + +vi.mock('@tryghost/kg-unsplash-selector', () => ({ + UnsplashSearchModal: ({ onClose }: { onClose: () => void }) => ( +
    + Unsplash search + +
    + ), +})); + +vi.mock('@tryghost/admin-x-framework', () => ({ + useFramework: () => ({ unsplashConfig: null }), +})); + +const LABEL = 'Select an image from Unsplash'; + +/** Whether an Escape raised from `from` is marked as already answered. */ +function escapeMarked(from: EventTarget): boolean { + const event = new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }); + from.dispatchEvent(event); + return event.defaultPrevented; +} + +function picker(enabled: boolean) { + return ; +} + +function openSearch() { + fireEvent.click(screen.getByRole('button', { name: LABEL })); + return screen.getByText('Unsplash search'); +} + +/** + * The Unsplash affordance on an image field. Only the search itself answers + * Escape, so the pane it opened over keeps its own. + */ +describe('UnsplashPicker', () => { + it('returns focus to the picker when the search closes', async () => { + render(picker(true)); + const trigger = screen.getByRole('button', { name: LABEL }); + trigger.focus(); + openSearch(); + + const close = screen.getByRole('button', { name: 'Close search' }); + expect(close).toHaveFocus(); + fireEvent.click(close); + + await waitFor(() => expect(trigger).toHaveFocus()); + }); + + it('marks only the Escape raised inside the open search', () => { + render(picker(true)); + + const search = openSearch(); + + expect(escapeMarked(search)).toBe(true); + // Anything the search does not contain still reaches the pane behind it. + expect(escapeMarked(window)).toBe(false); + expect(escapeMarked(document.body)).toBe(false); + }); + + it('closes the search and gives Escape back when the integration goes off', () => { + const { rerender } = render(picker(true)); + + const search = openSearch(); + + expect(search).toBeInTheDocument(); + expect(escapeMarked(search)).toBe(true); + + rerender(picker(false)); + + expect(screen.queryByText('Unsplash search')).not.toBeInTheDocument(); + expect(escapeMarked(search)).toBe(false); + + rerender(picker(true)); + + // The affordance comes back closed rather than reopening what was dismissed. + expect(screen.queryByText('Unsplash search')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: LABEL })).toBeVisible(); + }); +}); diff --git a/apps/admin/src/editor/unsplash-picker.tsx b/apps/admin/src/editor/unsplash-picker.tsx index 475c567d5af..33bc700c329 100644 --- a/apps/admin/src/editor/unsplash-picker.tsx +++ b/apps/admin/src/editor/unsplash-picker.tsx @@ -1,10 +1,13 @@ -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; +import { FocusGuards } from '@radix-ui/react-focus-guards'; +import { FocusScope } from '@radix-ui/react-focus-scope'; import { UnsplashSearchModal } from '@tryghost/kg-unsplash-selector'; import { Button } from '@tryghost/shade/components'; import { ImageUploadActions } from '@tryghost/shade/patterns'; import { cn } from '@tryghost/shade/utils'; import { useFramework } from '@tryghost/admin-x-framework'; +import { unsplashSearchModal } from '@tryghost/test-data/selectors/editor'; import BrandIcon from '@/shared/brand-icon/brand-icon'; export interface UnsplashSelection { @@ -36,12 +39,22 @@ export function UnsplashPicker({ onSelect, }: UnsplashPickerProps) { const { unsplashConfig } = useFramework(); + const triggerRef = useRef(null); const [isOpen, setIsOpen] = useState(false); + const [modalRoot, setModalRoot] = useState(null); + + // A gate that goes off while the search is open closes it, so nothing is left + // listening for a field the writer can no longer see. + useEffect(() => { + if (!enabled) { + setIsOpen(false); + } + }, [enabled]); // The modal answers Escape itself but does not mark it, so a settings pane // behind it would read the same Escape as its own and close too. useEffect(() => { - if (!isOpen) { + if (!modalRoot) { return; } @@ -51,9 +64,9 @@ export function UnsplashPicker({ } }; - window.addEventListener('keydown', markHandled, true); - return () => window.removeEventListener('keydown', markHandled, true); - }, [isOpen]); + modalRoot.addEventListener('keydown', markHandled, true); + return () => modalRoot.removeEventListener('keydown', markHandled, true); + }, [modalRoot]); if (!enabled) { return null; @@ -63,6 +76,7 @@ export function UnsplashPicker({ <>