From 7ab4e09f1232be79d1412ce9f1cb22ab98b2894d Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Tue, 28 Jul 2026 14:49:15 +1000 Subject: [PATCH 1/9] feat(delete): optionally remove resources associated with a zone or domain (PPT-1203) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a zone has never removed the systems inside it. `Zone#destroy` cascades child zones, trigger instances, metadata, settings and group links, but `sys.zones` is a text array with no foreign key, so the zone id was simply stripped and any system left with no zones became an orphan (PROJ-845 — the dev install currently carries six such systems). The confirmation copy already claimed otherwise: "Deleting this zone will immediately remove systems without another zone". The delete confirmation now offers "Also delete associated resources", **off by default** — deleting an item without touching what hangs off it stays the default behaviour. Switching it on resolves a plan first and shows exactly what would go before anything is confirmed. Zones remove the systems whose every zone falls inside the subtree, so a system shared with a zone outside it is kept, as it is today. The backend takes their modules, triggers, metadata and settings. Domains remove their OAuth applications (`oauth_applications.owner_id` has no foreign key, so these orphan today) and their staff API tenant, matched on domain name the same way the admin screen does. Zones are only reachable from a domain through the `authority.config.org_zone` convention, which is not exclusive — three domains share one org zone on the dev install — so the zone tree is only included when no other domain references it, and the dialog names the domains that caused it to be skipped. Because the removals are destructive, the system index (Elasticsearch, which can lag) only nominates candidates; each one is re-read through `showSystem` and re-checked against the database before it makes the list. Also here: - `ConfirmModalComponent` gains optional checkboxes with lazily resolved detail, reported back on the confirmation event. Existing callers pass no options and are unaffected. - The zone mock filtered on a `parent` query param the API has never sent; it now honours `parent_id`, including comma separated lists and `root`. Mock zones gained the `parent_id` hierarchy their `zones` arrays imply. - `config/proxy.conf.js` accepts `PLACEOS_DOMAIN` so the dev server can point at a local stack. - `ZONES.DELETE_MSG` now describes what actually happens. The stale translation was dropped from the non-English locales so they fall back to the corrected source string rather than repeating the old claim. Verified against a local PlaceOS stack: an org > building > two levels tree with three systems (two enclosed, one straddling an outside zone) and their modules. With the option on, the tree, both enclosed systems and both modules were removed while the straddling system survived holding only its outside zone; with it off, the zone went and the system was left orphaned exactly as before. A domain delete removed its org zone tree, orphaned system, OAuth application and staff API tenant; a domain sharing its org zone with another left the zone, its system and the other domain untouched. --- config/proxy.conf.js | 6 +- e2e/src/cascade-delete.spec.ts | 139 ++++++ e2e/src/pages/base.page.ts | 51 ++ public/assets/locale/ar.json | 1 - public/assets/locale/en-AU.json | 42 +- public/assets/locale/en-GB.json | 42 +- public/assets/locale/en-US.json | 42 +- public/assets/locale/es.json | 1 - public/assets/locale/fr.json | 1 - public/assets/locale/jp.json | 1 - src/app/common/actions.ts | 30 ++ src/app/common/cascade-delete.ts | 401 ++++++++++++++++ src/app/common/item.service.ts | 135 ++++-- src/app/mocks/backend/zones.mock.ts | 12 +- src/app/mocks/data/zones.ts | 7 + src/app/overlays/confirm-modal.component.ts | 182 ++++++- src/tests/common/cascade-delete.spec.ts | 454 ++++++++++++++++++ src/tests/common/cascade-locale.spec.ts | 80 +++ .../overlays/confirm-modal.component.spec.ts | 149 ++++++ 19 files changed, 1715 insertions(+), 61 deletions(-) create mode 100644 e2e/src/cascade-delete.spec.ts create mode 100644 src/app/common/cascade-delete.ts create mode 100644 src/tests/common/cascade-delete.spec.ts create mode 100644 src/tests/common/cascade-locale.spec.ts diff --git a/config/proxy.conf.js b/config/proxy.conf.js index 44c634f24..22855ba21 100644 --- a/config/proxy.conf.js +++ b/config/proxy.conf.js @@ -1,5 +1,7 @@ -const domain = 'placeos-dev.aca.im'; -const secure = true; +// Override to develop against another environment, e.g. the local PlaceOS +// stack: `PLACEOS_DOMAIN=localhost:8443 bun run start` +const domain = process.env.PLACEOS_DOMAIN || 'placeos-dev.aca.im'; +const secure = process.env.PLACEOS_INSECURE !== 'true'; const valid_ssl = false; const PROXY_CONFIG = {}; diff --git a/e2e/src/cascade-delete.spec.ts b/e2e/src/cascade-delete.spec.ts new file mode 100644 index 000000000..7f2b63084 --- /dev/null +++ b/e2e/src/cascade-delete.spec.ts @@ -0,0 +1,139 @@ +import { expect, test } from '@playwright/test'; +import { ZonesPage } from './pages'; + +/** + * Optional cascade delete (PPT-1203) + * + * Deleting a zone has never removed the systems inside it — the zone id was + * just stripped from `sys.zones`, leaving systems orphaned (PROJ-845). The + * delete confirmation now offers to remove them, off by default. + * + * The mock zone tree these tests rely on: + * + * Place Technology (org) + * └── Tower 2 (building) + * ├── Level 30 + * │ └── L30 Activity Spaces + * └── Level 31 + * ├── L31 Activity Spaces + * ├── L31 Multifunction: 31.22 + * └── L31 R7 Activity Space + * New Zone (root, no children, no systems) + */ +test.describe('Cascade delete', () => { + let zonesPage: ZonesPage; + + test.beforeEach(async ({ page }) => { + await page.addInitScript(() => { + localStorage.setItem('BACKOFFICE.mock', 'true'); + }); + zonesPage = new ZonesPage(page); + }); + + const openZone = async (page, zone_id: string) => { + await page.goto(`/?mock=true#/zones/${zone_id}/about`); + await zonesPage.waitForLoad(); + await page.waitForSelector('item-details', { timeout: 20000 }); + }; + + test('offers the option, disabled by default', async ({ page }) => { + await openZone(page, 'zone-lmhh_hVfz0'); + await zonesPage.openDeleteConfirmation(); + + await expect(zonesPage.cascadeCheckbox).toHaveCount(1); + await expect(zonesPage.cascadeCheckbox).not.toBeChecked(); + // Nothing is resolved until the option is switched on + await expect(zonesPage.cascadeSummary).toHaveCount(0); + }); + + test('no longer claims that systems are removed by default', async ({ + page, + }) => { + await openZone(page, 'zone-lmhh_hVfz0'); + await zonesPage.openDeleteConfirmation(); + + const content = await page + .locator('confirm-modal [content]') + .innerText(); + expect(content).toContain('are kept unless you also remove'); + }); + + test('reports nothing to remove for an empty zone', async ({ page }) => { + await openZone(page, 'zone-lmhh_hVfz0'); + await zonesPage.openDeleteConfirmation(); + await zonesPage.enableCascade(); + + await expect(zonesPage.cascadeEmpty).toBeVisible(); + await expect(zonesPage.cascadeSummary).toHaveCount(0); + }); + + test('lists the systems and modules that would be removed', async ({ + page, + }) => { + await openZone(page, 'zone-Kl0E0HmCJ3'); // Place Technology (org) + await zonesPage.openDeleteConfirmation(); + await zonesPage.enableCascade(); + + const summary = await zonesPage.cascadeSummary.innerText(); + expect(summary).toMatch(/\d+ systems? left without a zone/); + expect(summary).toMatch(/\d+ modules? in those systems/); + + const scope = await page.locator('confirm-modal').innerText(); + expect(scope).toMatch(/Scope: this zone and \d+ zones? beneath it/); + }); + + test('keeps systems that also belong to a zone outside the subtree', async ({ + page, + }) => { + // Level 30's systems are also in Tower 2, which is above it — they + // survive, so the cascade must remove nothing and say so. + await openZone(page, 'zone-LEHeo501Er'); + await zonesPage.openDeleteConfirmation(); + await zonesPage.enableCascade(); + + await expect(zonesPage.cascadeEmpty).toBeVisible(); + const warning = await zonesPage.cascadeWarnings.innerText(); + expect(warning).toMatch( + /systems? also belongs? to (a )?zones? outside this one and will be kept/, + ); + }); + + test('removes the orphaned systems when confirmed', async ({ page }) => { + await openZone(page, 'zone-Kl0E0HmCJ3'); + await zonesPage.openDeleteConfirmation(); + await zonesPage.enableCascade(); + + const summary = await zonesPage.cascadeSummary.innerText(); + const expected = Number(summary.match(/(\d+) systems? left/)?.[1] || 0); + expect(expected).toBeGreaterThan(0); + + await zonesPage.acceptButton.click(); + await page.waitForURL(/#\/zones\/-/, { timeout: 30000 }); + + // Every system lived inside this org zone, so the systems list empties + await page.goto('/?mock=true#/systems'); + await zonesPage.waitForLoad(); + await page.waitForTimeout(1000); + await expect(zonesPage.sidebarItems).toHaveCount(0); + }); + + test('leaves systems alone when the option is left off', async ({ + page, + }) => { + await page.goto('/?mock=true#/systems'); + await zonesPage.waitForLoad(); + await page.waitForTimeout(1000); + const before = await zonesPage.sidebarItems.count(); + expect(before).toBeGreaterThan(0); + + await openZone(page, 'zone-Kl0E0HmCJ3'); + await zonesPage.openDeleteConfirmation(); + await zonesPage.acceptButton.click(); + await page.waitForURL(/#\/zones\/-/, { timeout: 30000 }); + + await page.goto('/?mock=true#/systems'); + await zonesPage.waitForLoad(); + await page.waitForTimeout(1000); + await expect(zonesPage.sidebarItems).toHaveCount(before); + }); +}); diff --git a/e2e/src/pages/base.page.ts b/e2e/src/pages/base.page.ts index ec185f940..4e587642d 100644 --- a/e2e/src/pages/base.page.ts +++ b/e2e/src/pages/base.page.ts @@ -233,6 +233,57 @@ export abstract class BasePage { await this.dialog.waitFor({ timeout: 5000 }); } + /** + * Get the "also delete associated resources" checkbox on the delete + * confirmation + */ + get cascadeCheckbox(): Locator { + return this.page.locator( + 'confirm-modal [confirm-option] input[type="checkbox"]', + ); + } + + /** Get the resolved list of what the cascade would remove */ + get cascadeSummary(): Locator { + return this.page.locator('confirm-modal [details-summary]'); + } + + /** Get the "nothing else to remove" message */ + get cascadeEmpty(): Locator { + return this.page.locator('confirm-modal [details-empty]'); + } + + /** Get the lines describing what the cascade will leave alone */ + get cascadeWarnings(): Locator { + return this.page.locator('confirm-modal [details-warning]'); + } + + /** Get the confirmation dialog's accept button */ + get acceptButton(): Locator { + return this.page.locator('confirm-modal button[name="accept"]'); + } + + /** + * Open the delete confirmation without confirming it + */ + async openDeleteConfirmation(): Promise { + await this.page.waitForSelector('item-details', { timeout: 10000 }); + await this.openActionMenu(); + await this.deleteButton.click(); + await this.page.waitForSelector('confirm-modal', { timeout: 5000 }); + } + + /** + * Enable the cascade option and wait for its breakdown to resolve + */ + async enableCascade(): Promise { + await this.cascadeCheckbox.click(); + await this.page.waitForSelector( + 'confirm-modal [details-summary], confirm-modal [details-empty]', + { timeout: 20000 }, + ); + } + /** * Click delete and confirm (requires opening action menu first) */ diff --git a/public/assets/locale/ar.json b/public/assets/locale/ar.json index 1b833b091..11e34574a 100644 --- a/public/assets/locale/ar.json +++ b/public/assets/locale/ar.json @@ -340,7 +340,6 @@ "ADD": "إضافة منطقة", "EDIT": "تحرير منطقة", "DELETE": "حذف منطقة", - "DELETE_MSG": "

هل أنت متأكد أنك تريد حذف هذه المنطقة؟

سيؤدي حذف هذه المنطقة إلى الإزالة الفورية للأنظمة دون منطقة أخرى

", "DELETE_LOADING": "جاري حذف المنطقة...", "DELETE_SUCCESS": "تم حذف المنطقة بنجاح.", "DELETE_ERROR": "فشل حذف المنطقة. الخطأ: {{ error }}", diff --git a/public/assets/locale/en-AU.json b/public/assets/locale/en-AU.json index 2bed2f621..38dc1ea90 100644 --- a/public/assets/locale/en-AU.json +++ b/public/assets/locale/en-AU.json @@ -379,7 +379,7 @@ "ADD": "Add zone", "EDIT": "Edit zone", "DELETE": "Delete zone", - "DELETE_MSG": "

Are you sure you want delete this zone?

Deleting this zone will immediately remove systems without another zone

", + "DELETE_MSG": "

Are you sure you want delete this zone?

Zones beneath it, its triggers, metadata and settings are removed immediately. Systems in this zone are kept unless you also remove associated resources.

", "DELETE_LOADING": "Deleting zone...", "DELETE_SUCCESS": "Successfully deleted zone.", "DELETE_ERROR": "Failed to delete zone. Error: {{ error }}", @@ -436,7 +436,9 @@ "PARENT_ZONE": "Parent Zone", "NAME_REQUIRED": "A unique zone name is required", "DISPLAY_NAME": "Display name", - "MISCONFIGURED": "Tags in zone require a parent zone" + "MISCONFIGURED": "Tags in zone require a parent zone", + "DELETE_CASCADE": "Also delete associated resources", + "DELETE_CASCADE_DESC": "Removes systems that would be left without any zone, along with the modules, triggers and settings belonging to them." }, "DRIVERS": { "SINGULAR": "Driver", @@ -802,7 +804,7 @@ "BULK": "Bulk add domains", "REMOVE": "Remove domain", "DELETE": "Delete domain", - "DELETE_MSG": "

Are you sure you want delete this domain?

The domain will be deleted immediately.

", + "DELETE_MSG": "

Are you sure you want delete this domain?

Its users, auth sources and groups are removed immediately.

", "DELETE_LOADING": "Deleting domain...", "DELETE_SUCCESS": "Successfully deleted domain.", "DELETE_ERROR": "Failed to delete domain. Error: {{ error }}", @@ -913,7 +915,9 @@ "APP_SCOPES": "Access Scopes", "APP_SUBSYSTEMS": "Subsystems", "APP_REDIRECT_URL": "Redirect URL", - "APP_REDIRECT_URL_REQUIRED": "A valid URL is required" + "APP_REDIRECT_URL_REQUIRED": "A valid URL is required", + "DELETE_CASCADE": "Also delete associated resources", + "DELETE_CASCADE_DESC": "Removes this domain's OAuth applications and staff API tenant, plus its org zone tree when no other domain uses it." }, "ADMIN": { "TITLE": "Admin", @@ -1200,5 +1204,35 @@ "BUILD_LIST_REMOVE_LOADING": "Cancelling build job...", "BUILD_LIST_REMOVE_ERROR": "Failed to cancel build job. Error: {{ error }}", "BUILD_LIST_REMOVE_SUCCESS": "Successfully cancelled build job." + }, + "CASCADE": { + "RESOLVING": "Working out what would be removed...", + "NOTHING": "Nothing else to remove.", + "SCOPE_ZONES": "Scope: this zone and {{ count }} zones beneath it.", + "SCOPE_ZONES_1": "Scope: this zone and {{ count }} zone beneath it.", + "SCOPE_ORG_ZONE": "Scope: org zone \"{{ name }}\" and everything beneath it.", + "REMOVE_SYSTEMS": "{{ count }} systems left without a zone", + "REMOVE_SYSTEMS_1": "{{ count }} system left without a zone", + "REMOVE_MODULES": "{{ count }} modules in those systems (any also used by a system being kept will remain)", + "REMOVE_MODULES_1": "{{ count }} module in those systems (kept if another system also uses it)", + "REMOVE_APPLICATIONS": "{{ count }} OAuth applications", + "REMOVE_APPLICATIONS_1": "{{ count }} OAuth application", + "REMOVE_TENANTS": "{{ count }} staff API tenants, with their bookings, guests and survey data", + "REMOVE_TENANTS_1": "{{ count }} staff API tenant, with its bookings, guests and survey data", + "REMOVE_ORG_ZONE": "the org zone and every zone beneath it", + "KEEP_SYSTEMS": "{{ count }} systems also belong to zones outside this one and will be kept.", + "KEEP_SYSTEMS_1": "{{ count }} system also belongs to a zone outside this one and will be kept.", + "NO_ORG_ZONE": "This domain has no \"org_zone\" configured, so no zones can be matched to it. Delete its zones from the Zones page instead.", + "ORG_ZONE_SHARED": "The org zone is also used by {{ names }}, so it will be left alone. Delete it from the Zones page if that is what you want.", + "ORG_ZONE_MISSING": "The configured org zone ({{ id }}) no longer exists, so no zones will be removed.", + "REMOVING_SYSTEM": "Removing system \"{{ name }}\"", + "REMOVING_APPLICATION": "Removing application \"{{ name }}\"", + "REMOVING_TENANT": "Removing staff API tenant \"{{ name }}\"", + "REMOVING_ZONE": "Removing zone \"{{ name }}\"", + "PROGRESS": "{{ step }} ({{ index }} of {{ total }})", + "SUCCESS": "Removed {{ count }} associated resources.", + "SUCCESS_1": "Removed {{ count }} associated resource.", + "FAILED": "Failed to remove {{ count }} associated resources, so nothing further was deleted. Error: {{ error }}", + "FAILED_1": "Failed to remove {{ count }} associated resource, so nothing further was deleted. Error: {{ error }}" } } diff --git a/public/assets/locale/en-GB.json b/public/assets/locale/en-GB.json index 17eab3696..e26ad9a77 100644 --- a/public/assets/locale/en-GB.json +++ b/public/assets/locale/en-GB.json @@ -346,7 +346,7 @@ "ADD": "Add zone", "EDIT": "Edit zone", "DELETE": "Delete zone", - "DELETE_MSG": "

Are you sure you want delete this zone?

Deleting this zone will immediately remove systems without another zone

", + "DELETE_MSG": "

Are you sure you want delete this zone?

Zones beneath it, its triggers, metadata and settings are removed immediately. Systems in this zone are kept unless you also remove associated resources.

", "DELETE_LOADING": "Deleting zone...", "DELETE_SUCCESS": "Successfully deleted zone.", "DELETE_ERROR": "Failed to delete zone. Error: {{ error }}", @@ -385,7 +385,9 @@ "TRIGGERS_EMPTY": "No triggers for selected zone", "PARENT_ZONE": "Parent Zone", "NAME_REQUIRED": "A unique zone name is required", - "DISPLAY_NAME": "Display name" + "DISPLAY_NAME": "Display name", + "DELETE_CASCADE": "Also delete associated resources", + "DELETE_CASCADE_DESC": "Removes systems that would be left without any zone, along with the modules, triggers and settings belonging to them." }, "DRIVERS": { "SINGULAR": "Driver", @@ -645,7 +647,7 @@ "BULK": "Bulk add domains", "REMOVE": "Remove domain", "DELETE": "Delete domain", - "DELETE_MSG": "

Are you sure you want delete this domain?

The domain will be deleted immediately.

", + "DELETE_MSG": "

Are you sure you want delete this domain?

Its users, auth sources and groups are removed immediately.

", "DELETE_LOADING": "Deleting domain...", "DELETE_SUCCESS": "Successfully deleted domain.", "DELETE_ERROR": "Failed to delete domain. Error: {{ error }}", @@ -750,7 +752,9 @@ "APP_SCOPES": "Access Scopes", "APP_SUBSYSTEMS": "Subsystems", "APP_REDIRECT_URL": "Redirect URL", - "APP_REDIRECT_URL_REQUIRED": "A valid URL is required" + "APP_REDIRECT_URL_REQUIRED": "A valid URL is required", + "DELETE_CASCADE": "Also delete associated resources", + "DELETE_CASCADE_DESC": "Removes this domain's OAuth applications and staff API tenant, plus its org zone tree when no other domain uses it." }, "ADMIN": { "TITLE": "Admin", @@ -990,5 +994,35 @@ "UPLOADS_LIB_FIELD_TYPE": "File Type", "UPLOADS_LIB_FIELD_SIZE": "Size", "UPLOADS_LIB_LIST_EMPTY": "No uploads for the selected domain" + }, + "CASCADE": { + "RESOLVING": "Working out what would be removed...", + "NOTHING": "Nothing else to remove.", + "SCOPE_ZONES": "Scope: this zone and {{ count }} zones beneath it.", + "SCOPE_ZONES_1": "Scope: this zone and {{ count }} zone beneath it.", + "SCOPE_ORG_ZONE": "Scope: org zone \"{{ name }}\" and everything beneath it.", + "REMOVE_SYSTEMS": "{{ count }} systems left without a zone", + "REMOVE_SYSTEMS_1": "{{ count }} system left without a zone", + "REMOVE_MODULES": "{{ count }} modules in those systems (any also used by a system being kept will remain)", + "REMOVE_MODULES_1": "{{ count }} module in those systems (kept if another system also uses it)", + "REMOVE_APPLICATIONS": "{{ count }} OAuth applications", + "REMOVE_APPLICATIONS_1": "{{ count }} OAuth application", + "REMOVE_TENANTS": "{{ count }} staff API tenants, with their bookings, guests and survey data", + "REMOVE_TENANTS_1": "{{ count }} staff API tenant, with its bookings, guests and survey data", + "REMOVE_ORG_ZONE": "the org zone and every zone beneath it", + "KEEP_SYSTEMS": "{{ count }} systems also belong to zones outside this one and will be kept.", + "KEEP_SYSTEMS_1": "{{ count }} system also belongs to a zone outside this one and will be kept.", + "NO_ORG_ZONE": "This domain has no \"org_zone\" configured, so no zones can be matched to it. Delete its zones from the Zones page instead.", + "ORG_ZONE_SHARED": "The org zone is also used by {{ names }}, so it will be left alone. Delete it from the Zones page if that is what you want.", + "ORG_ZONE_MISSING": "The configured org zone ({{ id }}) no longer exists, so no zones will be removed.", + "REMOVING_SYSTEM": "Removing system \"{{ name }}\"", + "REMOVING_APPLICATION": "Removing application \"{{ name }}\"", + "REMOVING_TENANT": "Removing staff API tenant \"{{ name }}\"", + "REMOVING_ZONE": "Removing zone \"{{ name }}\"", + "PROGRESS": "{{ step }} ({{ index }} of {{ total }})", + "SUCCESS": "Removed {{ count }} associated resources.", + "SUCCESS_1": "Removed {{ count }} associated resource.", + "FAILED": "Failed to remove {{ count }} associated resources, so nothing further was deleted. Error: {{ error }}", + "FAILED_1": "Failed to remove {{ count }} associated resource, so nothing further was deleted. Error: {{ error }}" } } diff --git a/public/assets/locale/en-US.json b/public/assets/locale/en-US.json index 35030d5f6..d6d6047fd 100644 --- a/public/assets/locale/en-US.json +++ b/public/assets/locale/en-US.json @@ -343,7 +343,7 @@ "ADD": "Add zone", "EDIT": "Edit zone", "DELETE": "Delete zone", - "DELETE_MSG": "

Are you sure you want delete this zone?

Deleting this zone will immediately remove systems without another zone

", + "DELETE_MSG": "

Are you sure you want delete this zone?

Zones beneath it, its triggers, metadata and settings are removed immediately. Systems in this zone are kept unless you also remove associated resources.

", "DELETE_LOADING": "Deleting zone...", "DELETE_SUCCESS": "Successfully deleted zone.", "DELETE_ERROR": "Failed to delete zone. Error: {{ error }}", @@ -382,7 +382,9 @@ "TRIGGERS_EMPTY": "No triggers for selected zone", "PARENT_ZONE": "Parent Zone", "NAME_REQUIRED": "A unique zone name is required", - "DISPLAY_NAME": "Display name" + "DISPLAY_NAME": "Display name", + "DELETE_CASCADE": "Also delete associated resources", + "DELETE_CASCADE_DESC": "Removes systems that would be left without any zone, along with the modules, triggers and settings belonging to them." }, "DRIVERS": { "SINGULAR": "Driver", @@ -642,7 +644,7 @@ "BULK": "Bulk add domains", "REMOVE": "Remove domain", "DELETE": "Delete domain", - "DELETE_MSG": "

Are you sure you want delete this domain?

The domain will be deleted immediately.

", + "DELETE_MSG": "

Are you sure you want delete this domain?

Its users, auth sources and groups are removed immediately.

", "DELETE_LOADING": "Deleting domain...", "DELETE_SUCCESS": "Successfully deleted domain.", "DELETE_ERROR": "Failed to delete domain. Error: {{ error }}", @@ -747,7 +749,9 @@ "APP_SCOPES": "Access Scopes", "APP_SUBSYSTEMS": "Subsystems", "APP_REDIRECT_URL": "Redirect URL", - "APP_REDIRECT_URL_REQUIRED": "A valid URL is required" + "APP_REDIRECT_URL_REQUIRED": "A valid URL is required", + "DELETE_CASCADE": "Also delete associated resources", + "DELETE_CASCADE_DESC": "Removes this domain's OAuth applications and staff API tenant, plus its org zone tree when no other domain uses it." }, "ADMIN": { "TITLE": "Admin", @@ -987,5 +991,35 @@ "UPLOADS_LIB_FIELD_TYPE": "File Type", "UPLOADS_LIB_FIELD_SIZE": "Size", "UPLOADS_LIB_LIST_EMPTY": "No uploads for the selected domain" + }, + "CASCADE": { + "RESOLVING": "Working out what would be removed...", + "NOTHING": "Nothing else to remove.", + "SCOPE_ZONES": "Scope: this zone and {{ count }} zones beneath it.", + "SCOPE_ZONES_1": "Scope: this zone and {{ count }} zone beneath it.", + "SCOPE_ORG_ZONE": "Scope: org zone \"{{ name }}\" and everything beneath it.", + "REMOVE_SYSTEMS": "{{ count }} systems left without a zone", + "REMOVE_SYSTEMS_1": "{{ count }} system left without a zone", + "REMOVE_MODULES": "{{ count }} modules in those systems (any also used by a system being kept will remain)", + "REMOVE_MODULES_1": "{{ count }} module in those systems (kept if another system also uses it)", + "REMOVE_APPLICATIONS": "{{ count }} OAuth applications", + "REMOVE_APPLICATIONS_1": "{{ count }} OAuth application", + "REMOVE_TENANTS": "{{ count }} staff API tenants, with their bookings, guests and survey data", + "REMOVE_TENANTS_1": "{{ count }} staff API tenant, with its bookings, guests and survey data", + "REMOVE_ORG_ZONE": "the org zone and every zone beneath it", + "KEEP_SYSTEMS": "{{ count }} systems also belong to zones outside this one and will be kept.", + "KEEP_SYSTEMS_1": "{{ count }} system also belongs to a zone outside this one and will be kept.", + "NO_ORG_ZONE": "This domain has no \"org_zone\" configured, so no zones can be matched to it. Delete its zones from the Zones page instead.", + "ORG_ZONE_SHARED": "The org zone is also used by {{ names }}, so it will be left alone. Delete it from the Zones page if that is what you want.", + "ORG_ZONE_MISSING": "The configured org zone ({{ id }}) no longer exists, so no zones will be removed.", + "REMOVING_SYSTEM": "Removing system \"{{ name }}\"", + "REMOVING_APPLICATION": "Removing application \"{{ name }}\"", + "REMOVING_TENANT": "Removing staff API tenant \"{{ name }}\"", + "REMOVING_ZONE": "Removing zone \"{{ name }}\"", + "PROGRESS": "{{ step }} ({{ index }} of {{ total }})", + "SUCCESS": "Removed {{ count }} associated resources.", + "SUCCESS_1": "Removed {{ count }} associated resource.", + "FAILED": "Failed to remove {{ count }} associated resources, so nothing further was deleted. Error: {{ error }}", + "FAILED_1": "Failed to remove {{ count }} associated resource, so nothing further was deleted. Error: {{ error }}" } } diff --git a/public/assets/locale/es.json b/public/assets/locale/es.json index 955948eb3..64c6825dd 100644 --- a/public/assets/locale/es.json +++ b/public/assets/locale/es.json @@ -340,7 +340,6 @@ "ADD": "Agregar zona", "EDIT": "Editar zona", "DELETE": "Eliminar zona", - "DELETE_MSG": "

¿Estás seguro de que deseas eliminar esta zona?

Al eliminar esta zona, se eliminarán inmediatamente los sistemas que no tengan otra zona

", "DELETE_LOADING": "Eliminando zona...", "DELETE_SUCCESS": "Zona eliminada correctamente.", "DELETE_ERROR": "No se pudo eliminar la zona. Error: {{ error }}", diff --git a/public/assets/locale/fr.json b/public/assets/locale/fr.json index 414d7bd1a..707e21852 100644 --- a/public/assets/locale/fr.json +++ b/public/assets/locale/fr.json @@ -340,7 +340,6 @@ "ADD": "Ajouter une zone", "EDIT": "Modifier la zone", "DELETE": "Supprimer la zone", - "DELETE_MSG": "

Êtes-vous sûr de vouloir supprimer cette zone ?

La suppression de cette zone va immédiatement retirer les systèmes sans une autre zone

", "DELETE_LOADING": "Suppression de la zone...", "DELETE_SUCCESS": "Zone supprimée avec succès.", "DELETE_ERROR": "Échec de la suppression de la zone. Erreur : {{ error }}", diff --git a/public/assets/locale/jp.json b/public/assets/locale/jp.json index 276c492bd..a15a06a5e 100644 --- a/public/assets/locale/jp.json +++ b/public/assets/locale/jp.json @@ -342,7 +342,6 @@ "ADD": "ゾーンを追加", "EDIT": "ゾーンを編集", "DELETE": "ゾーンを削除", - "DELETE_MSG": "

このゾーンを削除してよろしいですか？

このゾーンを削除すると、他のゾーンに属さないシステムは即時に削除されます

", "DELETE_LOADING": "ゾーンを削除中...", "DELETE_SUCCESS": "ゾーンを正常に削除しました。", "DELETE_ERROR": "ゾーンの削除に失敗しました。エラー：{{ error }}", diff --git a/src/app/common/actions.ts b/src/app/common/actions.ts index 9909051af..9d837f0ca 100644 --- a/src/app/common/actions.ts +++ b/src/app/common/actions.ts @@ -58,6 +58,11 @@ import { updateUser, updateZone, } from '@placeos/ts-client'; +import { + CascadePlan, + planDomainCascade, + planZoneCascade, +} from './cascade-delete'; import { DomainFormComponent } from '../domains/domain-form.component'; import { DriverFormComponent } from '../drivers/driver-form.component'; import { GroupFormComponent } from '../groups/group-form.component'; @@ -68,6 +73,20 @@ import { TriggerFormComponent } from '../triggers/trigger-form.component'; import { UserFormComponent } from '../users/user-form.component'; import { ZoneFormComponent } from '../zones/zone-form.component'; +/** + * Optional "also remove the things associated with this item" behaviour, + * surfaced as a checkbox on the delete confirmation. Off by default — deleting + * an item without touching its associated resources stays the default. + */ +export interface ItemCascade { + /** i18n key for the checkbox label */ + label: string; + /** i18n key for the text shown under the checkbox */ + description: string; + /** Resolves what would be removed alongside the item */ + plan: (_: T) => Promise; +} + export interface ItemActions { query: (_?: string) => QueryResponse; show: (_: string) => Promise; @@ -77,6 +96,7 @@ export interface ItemActions { modalComponent: Type; delete_message: string; delete_extra?: (_: T) => Promise<[string, string]>; + cascade?: ItemCascade; name: string; } @@ -92,6 +112,11 @@ const domains: ItemActions = { itemConstructor: PlaceDomain, modalComponent: DomainFormComponent, delete_message: ``, + cascade: { + label: 'DOMAINS.DELETE_CASCADE', + description: 'DOMAINS.DELETE_CASCADE_DESC', + plan: (item) => planDomainCascade(item), + }, name: 'DOMAINS', }; @@ -319,6 +344,11 @@ const zones: ItemActions = { itemConstructor: PlaceZone, modalComponent: ZoneFormComponent, delete_message: ``, + cascade: { + label: 'ZONES.DELETE_CASCADE', + description: 'ZONES.DELETE_CASCADE_DESC', + plan: (item) => planZoneCascade(item.id), + }, name: 'ZONES', }; diff --git a/src/app/common/cascade-delete.ts b/src/app/common/cascade-delete.ts new file mode 100644 index 000000000..50f81d26c --- /dev/null +++ b/src/app/common/cascade-delete.ts @@ -0,0 +1,401 @@ +import { + del, + get, + PlaceApplication, + PlaceDomain, + PlaceSystem, + PlaceZone, + QueryResponse, + queryApplications, + queryDomains, + querySystems, + queryZones, + removeApplication, + removeSystem, + removeZone, + showSystem, + showZone, +} from '@placeos/ts-client'; +import type { PlaceTenant } from '../admin/staff-api.component'; +import { i18n } from './locale.service'; + +/** + * Resolution and execution of "delete the things associated with this item". + * + * PlaceOS already cascades most relationships server side — deleting a zone + * takes its child zones, trigger instances, metadata, settings and group links; + * deleting a system takes any module used by only that system. The one + * relationship that is *not* cascaded is `sys.zones`, a text array, so a system + * whose only zones are deleted is left orphaned with `zones: []`. Everything + * here exists to close that gap, plus the handful of authority-scoped records + * that have no foreign key back to `authority`. + * + * See tasks/PPT-1203 for the full relationship audit. + */ + +/** Page size used when walking collections. Server caps a page at 10000. */ +const PAGE_SIZE = 500; +/** Ceiling on zones walked in one subtree, guards against a cyclic `parent_id`. */ +const MAX_ZONES = 5000; +/** Ceiling on pages followed for a single query. */ +const MAX_PAGES = 100; +/** Concurrent requests issued while resolving a plan. */ +const READ_CONCURRENCY = 8; + +/** A single removal performed as part of a cascade. */ +export interface CascadeStep { + /** Progress message shown while the step runs */ + label: string; + /** Performs the removal. Rejects on failure. */ + run: () => Promise; +} + +/** What a cascade would do, resolved before the user confirms it. */ +export interface CascadePlan { + /** Lines describing the scope the cascade was resolved over */ + scope: string[]; + /** Lines describing what the cascade will remove */ + summary: string[]; + /** Lines describing what the cascade deliberately leaves alone */ + warnings: string[]; + /** Removals, in the order they must happen */ + steps: CascadeStep[]; +} + +/** Result of executing a `CascadePlan`. */ +export interface CascadeOutcome { + /** Number of steps that completed */ + removed: number; + /** Steps that threw, kept so the caller can report them */ + failures: { label: string; error: unknown }[]; +} + +const emptyPlan = (): CascadePlan => ({ + scope: [], + summary: [], + warnings: [], + steps: [], +}); + +/** Runs `fn` over `list` with at most `limit` requests in flight. */ +async function mapLimit( + list: T[], + limit: number, + fn: (item: T) => Promise, +): Promise { + const results: R[] = new Array(list.length); + let next_index = 0; + const worker = async () => { + while (next_index < list.length) { + const index = next_index++; + results[index] = await fn(list[index]); + } + }; + const size = Math.min(limit, list.length); + await Promise.all(new Array(size).fill(0).map(() => worker())); + return results; +} + +/** Collects every page of a paginated query. */ +async function collectPages(request: QueryResponse): Promise { + const items: T[] = []; + let page = await request; + items.push(...page.data); + let pages = 1; + while (page.next && pages < MAX_PAGES) { + const next_page = page.next(); + if (!next_page) break; + page = await next_page; + if (!page.data.length) break; + items.push(...page.data); + pages += 1; + } + return items; +} + +/** + * IDs of `zone_id` and every zone beneath it, walked breadth first. `parent_id` + * accepts a comma separated list so this costs one request per level of the + * tree rather than one per zone. + */ +export async function zoneSubtreeIds(zone_id: string): Promise { + if (!zone_id) return []; + const found = [zone_id]; + const seen = new Set(found); + let level = [zone_id]; + while (level.length && found.length < MAX_ZONES) { + const children = await collectPages( + queryZones({ parent_id: level.join(','), limit: PAGE_SIZE }), + ); + level = []; + for (const zone of children) { + if (!zone?.id || seen.has(zone.id)) continue; + seen.add(zone.id); + found.push(zone.id); + level.push(zone.id); + } + } + return found; +} + +/** Systems in a zone subtree, split by whether they survive its removal. */ +export interface ZoneSystemSplit { + /** Systems whose every zone is inside the subtree — these would be orphaned */ + orphaned: PlaceSystem[]; + /** Systems that also belong to a zone outside the subtree — these are kept */ + retained: PlaceSystem[]; +} + +/** + * Resolves the systems attached to a zone subtree. + * + * `GET /systems?zone_id=` ANDs its zone list server side, so "in any of these + * zones" needs one query per zone, deduplicated by system id. + * + * The index is Elasticsearch backed and can lag the database. Since a stale + * `zones` array here would mean deleting a system that still belongs + * somewhere, every removal candidate is re-read through `showSystem` (which + * reads the database) and re-checked before it makes the list. + */ +export async function splitZoneSystems( + zone_ids: string[], +): Promise { + const subtree = new Set(zone_ids); + const inside = (system: PlaceSystem) => + (system.zones || []).every((id) => subtree.has(id)); + + const found = new Map(); + const pages = await mapLimit(zone_ids, READ_CONCURRENCY, (zone_id) => + collectPages(querySystems({ zone_id, limit: PAGE_SIZE })).catch( + () => [] as PlaceSystem[], + ), + ); + for (const list of pages) { + for (const system of list) if (system?.id) found.set(system.id, system); + } + + const candidates: PlaceSystem[] = []; + const retained: PlaceSystem[] = []; + for (const system of found.values()) { + (inside(system) ? candidates : retained).push(system); + } + + const confirmed = await mapLimit(candidates, READ_CONCURRENCY, (system) => + showSystem(system.id).catch(() => null), + ); + const orphaned: PlaceSystem[] = []; + confirmed.forEach((current) => { + // A system that has since been deleted, or that has picked up a zone + // outside the subtree, is left alone. + if (!current) return; + (inside(current) ? orphaned : retained).push(current); + }); + return { orphaned, retained }; +} + +/** + * Resolves the removals needed so that deleting `zone_id` does not leave + * orphaned systems behind. Does **not** include removal of the zone itself — + * for a zone delete that is the caller's existing `remove` action, and for a + * domain delete `planDomainCascade` appends it. + */ +export async function planZoneCascade(zone_id: string): Promise { + const plan = emptyPlan(); + const zone_ids = await zoneSubtreeIds(zone_id); + if (!zone_ids.length) return plan; + const { orphaned, retained } = await splitZoneSystems(zone_ids); + const module_count = new Set( + orphaned.flatMap((system) => [...(system.modules || [])]), + ).size; + + const child_count = zone_ids.length - 1; + if (child_count) { + plan.scope.push( + i18n('CASCADE.SCOPE_ZONES', { count: child_count }, child_count), + ); + } + if (orphaned.length) { + plan.summary.push( + i18n( + 'CASCADE.REMOVE_SYSTEMS', + { count: orphaned.length }, + orphaned.length, + ), + ); + if (module_count) { + plan.summary.push( + i18n( + 'CASCADE.REMOVE_MODULES', + { count: module_count }, + module_count, + ), + ); + } + } + if (retained.length) { + plan.warnings.push( + i18n( + 'CASCADE.KEEP_SYSTEMS', + { count: retained.length }, + retained.length, + ), + ); + } + plan.steps = orphaned.map((system) => ({ + label: i18n('CASCADE.REMOVING_SYSTEM', { name: system.name }), + run: () => removeSystem(system.id), + })); + return plan; +} + +/** Tenants configured in the staff API against `domain`. */ +async function domainTenants(domain: string): Promise { + if (!domain) return []; + const tenants = (await get('/api/staff/v1/tenants').catch( + () => [], + )) as PlaceTenant[]; + return (tenants || []).filter((tenant) => tenant?.domain === domain); +} + +/** The `org_zone` a domain points at, if it declares one. */ +function orgZoneId(domain: PlaceDomain): string { + return `${domain?.config?.org_zone || ''}`; +} + +/** + * Resolves the removals associated with a domain. + * + * Users, auth sources, groups, playlists, signage plugins, shorteners, pending + * mail, asset categories and alert dashboards already cascade when the domain + * is deleted (model callbacks and DB foreign keys), so they are not listed + * here. What does not cascade — and so is handled here — is OAuth applications + * (`oauth_applications.owner_id` has no foreign key) and the staff API tenant + * (a separate service, linked only by matching domain name). + * + * Zones are only reachable through the `authority.config.org_zone` convention. + * That convention is not exclusive — multiple domains can and do point at the + * same org zone — so the zone tree is only included when no other domain + * references it. + */ +export async function planDomainCascade( + domain: PlaceDomain, +): Promise { + const plan = emptyPlan(); + const org_zone_id = orgZoneId(domain); + const [applications, tenants, all_domains] = await Promise.all([ + collectPages( + queryApplications({ authority_id: domain.id, limit: PAGE_SIZE }), + ).catch(() => [] as PlaceApplication[]), + domainTenants(domain.domain), + org_zone_id + ? collectPages(queryDomains({ limit: PAGE_SIZE })).catch( + () => [] as PlaceDomain[], + ) + : Promise.resolve([] as PlaceDomain[]), + ]); + + if (applications.length) { + plan.summary.push( + i18n( + 'CASCADE.REMOVE_APPLICATIONS', + { count: applications.length }, + applications.length, + ), + ); + plan.steps.push( + ...applications.map((application) => ({ + label: i18n('CASCADE.REMOVING_APPLICATION', { + name: application.name, + }), + run: () => removeApplication(application.id), + })), + ); + } + + if (tenants.length) { + plan.summary.push( + i18n( + 'CASCADE.REMOVE_TENANTS', + { count: tenants.length }, + tenants.length, + ), + ); + plan.steps.push( + ...tenants.map((tenant) => ({ + label: i18n('CASCADE.REMOVING_TENANT', { + name: tenant.name || tenant.domain, + }), + run: () => del(`/api/staff/v1/tenants/${tenant.id}`), + })), + ); + } + + if (!org_zone_id) { + plan.warnings.push(i18n('CASCADE.NO_ORG_ZONE')); + return plan; + } + + const sharing = all_domains.filter( + (other) => other.id !== domain.id && orgZoneId(other) === org_zone_id, + ); + if (sharing.length) { + plan.warnings.push( + i18n('CASCADE.ORG_ZONE_SHARED', { + names: sharing.map((other) => other.name).join(', '), + }), + ); + return plan; + } + + const org_zone: PlaceZone | null = await showZone(org_zone_id).catch( + () => null, + ); + if (!org_zone) { + plan.warnings.push( + i18n('CASCADE.ORG_ZONE_MISSING', { id: org_zone_id }), + ); + return plan; + } + + const zone_plan = await planZoneCascade(org_zone_id); + plan.scope.push( + i18n('CASCADE.SCOPE_ORG_ZONE', { name: org_zone.name }), + ...zone_plan.scope, + ); + plan.summary.push(...zone_plan.summary, i18n('CASCADE.REMOVE_ORG_ZONE')); + plan.warnings.push(...zone_plan.warnings); + plan.steps.push(...zone_plan.steps, { + label: i18n('CASCADE.REMOVING_ZONE', { name: org_zone.name }), + run: () => removeZone(org_zone_id), + }); + return plan; +} + +/** + * Executes a plan's steps in order. Steps run sequentially — each system + * removal cascades work on the server, and sequential execution gives honest + * progress and lets a partial failure be reported precisely. + */ +export async function runCascade( + plan: CascadePlan, + progress: (message: string) => void = () => undefined, +): Promise { + const outcome: CascadeOutcome = { removed: 0, failures: [] }; + const total = plan.steps.length; + for (const [index, step] of plan.steps.entries()) { + progress( + i18n('CASCADE.PROGRESS', { + step: step.label, + index: index + 1, + total, + }), + ); + try { + await step.run(); + outcome.removed += 1; + } catch (error) { + outcome.failures.push({ label: step.label, error }); + } + } + return outcome; +} diff --git a/src/app/common/item.service.ts b/src/app/common/item.service.ts index a6aa56782..878e2b957 100644 --- a/src/app/common/item.service.ts +++ b/src/app/common/item.service.ts @@ -26,11 +26,15 @@ import { DuplicateModalComponent } from '../overlays/duplicate-modal.component'; import { BackofficeUsersService } from '../users/users.service'; import { ACTIONS, ItemActions } from './actions'; import { AsyncHandler } from './async-handler.class'; +import { CascadePlan, runCascade } from './cascade-delete'; import { log } from './general'; import { i18n } from './locale.service'; import { notifyError, notifySuccess } from './notifications'; import { waitForEvent } from './signals'; +/** Id the "also delete associated resources" toggle is reported under */ +const CASCADE_OPTION = 'cascade'; + export type ResourceType = | 'domains' | 'drivers' @@ -246,57 +250,108 @@ export class ActiveItemService extends AsyncHandler { public async delete() { if (!this._user.current().sys_admin) return; const item = this._active_item(); - if (item) { - const ref = this._dialog.open< - ConfirmModalComponent, - ConfirmModalData - >(ConfirmModalComponent, { + if (!item) return; + const actions = this.actions; + const cascade = actions.cascade; + // Resolved lazily, only if the user enables the option — a cascade + // plan walks the whole zone subtree, which is not free. + let plan: CascadePlan | null = null; + const ref = this._dialog.open( + ConfirmModalComponent, + { ...CONFIRM_METADATA, data: { - title: i18n(`${this.actions.name}.DELETE`), - content: i18n(`${this.actions.name}.DELETE_MSG`, { + title: i18n(`${actions.name}.DELETE`), + content: i18n(`${actions.name}.DELETE_MSG`, { name: (item as PlaceResource & { display_name?: string }) .display_name || item.name, }), - extra: this.actions.delete_extra - ? await this.actions.delete_extra(item) + extra: actions.delete_extra + ? await actions.delete_extra(item) : null, + options: cascade + ? [ + { + id: CASCADE_OPTION, + label: i18n(cascade.label), + description: i18n(cascade.description), + details: async () => { + plan = await cascade.plan(item); + const { scope, summary, warnings } = + plan; + return { scope, summary, warnings }; + }, + }, + ] + : undefined, icon: { type: 'icon', content: 'delete' }, }, - }); - waitForEvent( - ref.componentInstance.event, - (e: DialogEvent) => e.reason === 'done', - ).then(async () => { + }, + ); + waitForEvent( + ref.componentInstance.event, + (e: DialogEvent) => e.reason === 'done', + ).then(async (event: DialogEvent<{ options?: HashMap }>) => { + ref.componentInstance.loading.set( + i18n(`${actions.name}.DELETE_LOADING`), + ); + if (event.metadata?.options?.[CASCADE_OPTION] && plan) { + const outcome = await runCascade(plan, (message) => + ref.componentInstance.loading.set(message), + ); + if (outcome.failures.length) { + ref.componentInstance.loading.set(''); + return notifyError( + i18n( + 'CASCADE.FAILED', + { + count: outcome.failures.length, + error: + (outcome.failures[0].error as Error) + ?.message || outcome.failures[0].label, + }, + outcome.failures.length, + ), + ); + } + if (outcome.removed) { + notifySuccess( + i18n( + 'CASCADE.SUCCESS', + { count: outcome.removed }, + outcome.removed, + ), + ); + } ref.componentInstance.loading.set( - i18n(`${this.actions.name}.DELETE_LOADING`), + i18n(`${actions.name}.DELETE_LOADING`), ); - await this.actions - .remove(item) - .then(() => { - notifySuccess( - i18n(`${this.actions.name}.DELETE_SUCCESS`, { - name: item.name, - }), - ); - this._active_item.set(null); - this.removeItem(item); - this._router.navigate([`/${this._type}`, '-', 'about']); - ref.close(); - }) - .catch((err) => { - ref.componentInstance.loading.set(''); - notifyError( - i18n(`${this.actions.name}.DELETE_ERROR`, { - error: JSON.stringify( - err.response || err.message || err, - ), - }), - ); - }); - }); - } + } + await actions + .remove(item) + .then(() => { + notifySuccess( + i18n(`${actions.name}.DELETE_SUCCESS`, { + name: item.name, + }), + ); + this._active_item.set(null); + this.removeItem(item); + this._router.navigate([`/${this._type}`, '-', 'about']); + ref.close(); + }) + .catch((err) => { + ref.componentInstance.loading.set(''); + notifyError( + i18n(`${actions.name}.DELETE_ERROR`, { + error: JSON.stringify( + err.response || err.message || err, + ), + }), + ); + }); + }); } public duplicate() { diff --git a/src/app/mocks/backend/zones.mock.ts b/src/app/mocks/backend/zones.mock.ts index 00b71bd05..4a674b4ad 100644 --- a/src/app/mocks/backend/zones.mock.ts +++ b/src/app/mocks/backend/zones.mock.ts @@ -14,8 +14,16 @@ const FILTER_FN = (item: Record, q: HashMap) => { .toLowerCase() .indexOf(((q.q as string) || '').toLowerCase()) >= 0; } - if (q.parent) { - match = match && item.parent_id === q.parent; + if (q.parent_id) { + // Matches the API: a comma separated list of parents, plus the + // special `root` value for zones without one. + const parents = `${q.parent_id}`.split(',').filter((_) => !!_); + const parent_id = `${item.parent_id || ''}`; + match = + match && + parents.some((parent) => + parent === 'root' ? !parent_id : parent === parent_id, + ); } if (q.control_system_id) { const system = endpointData(`${API}/systems`).find( diff --git a/src/app/mocks/data/zones.ts b/src/app/mocks/data/zones.ts index 36eed244c..b9bf5d45a 100644 --- a/src/app/mocks/data/zones.ts +++ b/src/app/mocks/data/zones.ts @@ -305,6 +305,7 @@ export const ZONES = [ triggers: [], created_at: 1543374809, id: 'zone-iIdF20naW0', + parent_id: 'zone-LEHeo501Er', }, { name: 'L31 Activity Spaces', @@ -389,6 +390,7 @@ export const ZONES = [ triggers: ['trigger-WzXonXrB4G'], created_at: 1519368108, id: 'zone-WjDE_sLQy8', + parent_id: 'zone-QjLXbYUxuC', }, { name: 'L31 Multifunction: 31.22', @@ -446,6 +448,7 @@ export const ZONES = [ triggers: [], created_at: 1529567548, id: 'zone-beI-19FMdl', + parent_id: 'zone-QjLXbYUxuC', }, { name: 'L31 R7 Activity Space', @@ -530,6 +533,7 @@ export const ZONES = [ triggers: [], created_at: 1547438444, id: 'zone-kG8cn_fkH9', + parent_id: 'zone-QjLXbYUxuC', }, { name: 'Level 30', @@ -539,6 +543,7 @@ export const ZONES = [ triggers: [], created_at: 1495599360, id: 'zone-LEHeo501Er', + parent_id: 'zone-Kl0HN~nDwc', }, { name: 'Level 31', @@ -548,6 +553,7 @@ export const ZONES = [ triggers: [], created_at: 1506945022, id: 'zone-QjLXbYUxuC', + parent_id: 'zone-Kl0HN~nDwc', }, { name: 'New Zone', @@ -811,5 +817,6 @@ export const ZONES = [ triggers: [], created_at: 1494571187, id: 'zone-Kl0HN~nDwc', + parent_id: 'zone-Kl0E0HmCJ3', }, ]; diff --git a/src/app/overlays/confirm-modal.component.ts b/src/app/overlays/confirm-modal.component.ts index f872e87ba..f51091f01 100644 --- a/src/app/overlays/confirm-modal.component.ts +++ b/src/app/overlays/confirm-modal.component.ts @@ -3,6 +3,7 @@ import { EventEmitter, OnInit, Output, + computed, inject, signal, } from '@angular/core'; @@ -13,6 +14,7 @@ import { MatDialogRef, } from '@angular/material/dialog'; +import { MatCheckboxModule } from '@angular/material/checkbox'; import { MatRippleModule } from '@angular/material/core'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; import { lastValueFrom } from 'rxjs'; @@ -22,6 +24,37 @@ import { ApplicationIcon, DialogEvent } from '../common/types'; import { IconComponent } from '../ui/icon.component'; import { TranslatePipe } from '../ui/translate.pipe'; +/** Breakdown of what enabling a `ConfirmModalOption` would do */ +export interface ConfirmModalOptionDetails { + /** Lines describing the scope the option was resolved over */ + scope?: string[]; + /** Lines describing what will additionally be removed */ + summary?: string[]; + /** Lines describing what will deliberately be left alone */ + warnings?: string[]; +} + +/** Opt-in toggle offered alongside the confirmation */ +export interface ConfirmModalOption { + /** Identifier the selection is reported under */ + id: string; + /** Label displayed beside the checkbox */ + label: string; + /** Explanatory text displayed under the checkbox */ + description?: string; + /** Whether the option starts enabled. Defaults to `false` */ + enabled?: boolean; + /** + * Resolves a breakdown of the option's effect. Run the first time the + * option is enabled so the cost is only paid when the user asks for it. + * Confirmation is blocked until it settles. + */ + details?: () => Promise; +} + +/** Options selected on confirmation, keyed by `ConfirmModalOption.id` */ +export type ConfirmModalSelection = Record; + export interface ConfirmModalData { /** Title of the modal */ title: string; @@ -29,6 +62,8 @@ export interface ConfirmModalData { content: string; /** Contents of the modal */ extra?: [string, string]; + /** Opt-in toggles offered alongside the confirmation */ + options?: ConfirmModalOption[]; /** Text displaed on the confirmation button */ confirm_text?: string; /** Text displaed on the confirmation button */ @@ -45,6 +80,7 @@ export const CONFIRM_METADATA = { export interface ConfirmRepsonse { reason: 'done' | '' | null; + metadata?: { options?: ConfirmModalSelection }; loading: (_: string) => void; close: () => void; } @@ -92,6 +128,81 @@ export async function openConfirmModal( [class]="'text-' + extra[0] + ' text-center text-sm'" [innerHTML]="extra[1]" >

+ @for (option of options; track option.id) { +
+ + {{ option.label }} + + @if (option.description) { +

+ {{ option.description }} +

+ } + @if (isSelected(option.id)) { +
+ @if (isLoadingDetails(option.id)) { +
+ + + {{ + 'CASCADE.RESOLVING' | translate + }} + +
+ } @else if (detailsError(option.id)) { +

+ {{ detailsError(option.id) }} +

+ } @else if (detailsFor(option.id); as detail) { + @for (line of detail.scope; track line) { +

+ {{ line }} +

+ } + @if (detail.summary?.length) { +
    + @for ( + line of detail.summary; + track line + ) { +
  • {{ line }}
    • + } +
+ } @else { +

+ {{ 'CASCADE.NOTHING' | translate }} +

+ } + @for (line of detail.warnings; track line) { +

+ {{ line }} +

+ } + } +
+ } +
+ } } @else {
@@ -120,6 +231,7 @@ export async function openConfirmModal( matRipple name="accept" class="flex-1" + [disabled]="resolving()" (click)="onConfirm()" > {{ confirm_text | translate }} @@ -130,6 +242,7 @@ export async function openConfirmModal( styles: [``], imports: [ MatProgressSpinnerModule, + MatCheckboxModule, TranslatePipe, IconComponent, MatRippleModule, @@ -162,6 +275,66 @@ export class ConfirmModalComponent extends AsyncHandler implements OnInit { class: 'material-symbols-rounded', content: 'done', }; + /** Opt-in toggles offered alongside the confirmation */ + public readonly options: ConfirmModalOption[] = this._data.options || []; + + /** Currently enabled options, keyed by option id */ + private readonly _selected = signal( + Object.fromEntries( + (this._data.options || []).map((option) => [ + option.id, + !!option.enabled, + ]), + ), + ); + /** Resolved breakdowns, keyed by option id */ + private readonly _details = signal< + Record + >({}); + /** Options currently resolving their breakdown */ + private readonly _resolving = signal>({}); + /** Failures from resolving a breakdown, keyed by option id */ + private readonly _errors = signal>({}); + + /** Whether any option is still resolving its breakdown */ + public readonly resolving = computed(() => + Object.values(this._resolving()).some((value) => value), + ); + + public readonly isSelected = (id: string) => !!this._selected()[id]; + public readonly isLoadingDetails = (id: string) => !!this._resolving()[id]; + public readonly detailsFor = (id: string) => this._details()[id]; + public readonly detailsError = (id: string) => this._errors()[id]; + + /** Enable or disable an option, resolving its breakdown on first enable */ + public toggleOption(option: ConfirmModalOption, enabled: boolean) { + this._selected.update((state) => ({ ...state, [option.id]: enabled })); + if (!enabled || !option.details || this._details()[option.id]) return; + this._errors.update((state) => ({ ...state, [option.id]: '' })); + this._resolving.update((state) => ({ ...state, [option.id]: true })); + option + .details() + .then((details) => + this._details.update((state) => ({ + ...state, + [option.id]: details, + })), + ) + .catch((error) => + this._errors.update((state) => ({ + ...state, + [option.id]: `${ + (error as Error)?.message || error || 'Unknown error' + }`, + })), + ) + .finally(() => + this._resolving.update((state) => ({ + ...state, + [option.id]: false, + })), + ); + } /** Prevent user from closing the modal */ public readonly disableClose = () => (this._dialog_ref.disableClose = true); /** Allow the user to close the modal */ @@ -179,6 +352,13 @@ export class ConfirmModalComponent extends AsyncHandler implements OnInit { /** User confirmation of the content of the modal */ public onConfirm() { - this.event.emit({ reason: 'done' }); + if (this.resolving()) return; + // Only carry metadata when options were offered, so existing callers + // keep seeing the exact event they always have. + this.event.emit( + this.options.length + ? { reason: 'done', metadata: { options: this._selected() } } + : { reason: 'done' }, + ); } } diff --git a/src/tests/common/cascade-delete.spec.ts b/src/tests/common/cascade-delete.spec.ts new file mode 100644 index 000000000..eb9397348 --- /dev/null +++ b/src/tests/common/cascade-delete.spec.ts @@ -0,0 +1,454 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// `i18n` is stubbed so assertions can read the key and its arguments rather +// than a rendered English sentence. +vi.mock('../../app/common/locale.service', () => ({ + i18n: (key: string, args: Record = {}) => + Object.keys(args).length ? `${key}:${JSON.stringify(args)}` : key, +})); + +vi.mock('@placeos/ts-client', () => ({ + PlaceApplication: class {}, + PlaceDomain: class {}, + PlaceSystem: class {}, + PlaceZone: class {}, + del: vi.fn(() => Promise.resolve()), + get: vi.fn(() => Promise.resolve([])), + queryApplications: vi.fn(() => Promise.resolve({ data: [], total: 0 })), + queryDomains: vi.fn(() => Promise.resolve({ data: [], total: 0 })), + querySystems: vi.fn(() => Promise.resolve({ data: [], total: 0 })), + queryZones: vi.fn(() => Promise.resolve({ data: [], total: 0 })), + removeApplication: vi.fn(() => Promise.resolve()), + removeSystem: vi.fn(() => Promise.resolve()), + removeZone: vi.fn(() => Promise.resolve()), + showSystem: vi.fn(() => Promise.resolve(null)), + showZone: vi.fn(() => Promise.resolve(null)), +})); + +import { + del, + get, + queryApplications, + queryDomains, + querySystems, + queryZones, + removeApplication, + removeSystem, + removeZone, + showSystem, + showZone, +} from '@placeos/ts-client'; +import { + planDomainCascade, + planZoneCascade, + runCascade, + splitZoneSystems, + zoneSubtreeIds, +} from '../../app/common/cascade-delete'; + +type Query = ReturnType; + +const page = (data: unknown[], next: unknown = null) => + Promise.resolve({ data, total: data.length, next: next ? () => next : null }); + +/** Zones keyed by their `parent_id`, matching the API's comma separated filter */ +const zonesByParent = (tree: Record) => + (params: { parent_id?: string }) => { + const parents = (params?.parent_id || '').split(',').filter((_) => !!_); + return page(parents.flatMap((parent) => tree[parent] || [])); + }; + +/** + * Systems keyed by the zone they belong to. Also records them so the mocked + * `showSystem` (the authoritative re-read) can serve the same data. + */ +const system_index = new Map(); +const systemsByZone = (map: Record) => { + system_index.clear(); + for (const list of Object.values(map)) { + for (const item of list as { id: string; zones: string[] }[]) { + system_index.set(item.id, item); + } + } + return (params: { zone_id?: string }) => + page(map[params?.zone_id || ''] || []); +}; + +const system = (id: string, zones: string[], modules: string[] = []) => ({ + id, + name: id, + zones, + modules, +}); + +describe('cascade-delete', () => { + beforeEach(() => { + vi.clearAllMocks(); + (queryZones as Query).mockImplementation(() => page([])); + (querySystems as Query).mockImplementation(() => page([])); + (queryApplications as Query).mockImplementation(() => page([])); + (queryDomains as Query).mockImplementation(() => page([])); + (get as Query).mockImplementation(() => Promise.resolve([])); + // Removal candidates are re-read from the database; by default the + // authoritative copy matches what the index returned. + (showSystem as Query).mockImplementation((id: string) => + Promise.resolve( + [...system_index.values()].find((_) => _.id === id) || null, + ), + ); + }); + + describe('zoneSubtreeIds', () => { + it('returns just the zone when it has no children', async () => { + expect(await zoneSubtreeIds('zone-a')).toEqual(['zone-a']); + }); + + it('returns an empty list for a missing id', async () => { + expect(await zoneSubtreeIds('')).toEqual([]); + }); + + it('walks the whole tree breadth first', async () => { + (queryZones as Query).mockImplementation( + zonesByParent({ + 'zone-a': [{ id: 'zone-b' }, { id: 'zone-c' }], + 'zone-b': [{ id: 'zone-d' }], + }), + ); + expect(await zoneSubtreeIds('zone-a')).toEqual([ + 'zone-a', + 'zone-b', + 'zone-c', + 'zone-d', + ]); + }); + + it('queries a whole level in a single request', async () => { + (queryZones as Query).mockImplementation( + zonesByParent({ + 'zone-a': [{ id: 'zone-b' }, { id: 'zone-c' }], + }), + ); + await zoneSubtreeIds('zone-a'); + // one request for zone-a's children, one for `zone-b,zone-c` + expect((queryZones as Query).mock.calls.length).toBe(2); + expect((queryZones as Query).mock.calls[1][0].parent_id).toBe( + 'zone-b,zone-c', + ); + }); + + it('terminates when a zone is its own ancestor', async () => { + (queryZones as Query).mockImplementation( + zonesByParent({ + 'zone-a': [{ id: 'zone-b' }], + 'zone-b': [{ id: 'zone-a' }], + }), + ); + expect(await zoneSubtreeIds('zone-a')).toEqual([ + 'zone-a', + 'zone-b', + ]); + }); + + it('follows pagination', async () => { + (queryZones as Query).mockImplementation(() => + page([{ id: 'zone-b' }], page([{ id: 'zone-c' }])), + ); + const ids = await zoneSubtreeIds('zone-a'); + expect(ids).toContain('zone-b'); + expect(ids).toContain('zone-c'); + }); + }); + + describe('splitZoneSystems', () => { + it('orphans a system whose every zone is inside the subtree', async () => { + (querySystems as Query).mockImplementation( + systemsByZone({ + 'zone-a': [system('sys-1', ['zone-a', 'zone-b'])], + }), + ); + const split = await splitZoneSystems(['zone-a', 'zone-b']); + expect(split.orphaned.map((_) => _.id)).toEqual(['sys-1']); + expect(split.retained).toEqual([]); + }); + + it('retains a system that also lives outside the subtree', async () => { + (querySystems as Query).mockImplementation( + systemsByZone({ + 'zone-a': [system('sys-1', ['zone-a', 'zone-elsewhere'])], + }), + ); + const split = await splitZoneSystems(['zone-a']); + expect(split.orphaned).toEqual([]); + expect(split.retained.map((_) => _.id)).toEqual(['sys-1']); + }); + + it('deduplicates a system found through several zones', async () => { + const shared = system('sys-1', ['zone-a', 'zone-b']); + (querySystems as Query).mockImplementation( + systemsByZone({ 'zone-a': [shared], 'zone-b': [shared] }), + ); + const split = await splitZoneSystems(['zone-a', 'zone-b']); + expect(split.orphaned.length).toBe(1); + }); + + it('keeps a system the database says has moved outside the subtree', async () => { + (querySystems as Query).mockImplementation( + systemsByZone({ 'zone-a': [system('sys-1', ['zone-a'])] }), + ); + // The index is stale — the database has it in another zone too. + (showSystem as Query).mockImplementation(() => + Promise.resolve(system('sys-1', ['zone-a', 'zone-elsewhere'])), + ); + const split = await splitZoneSystems(['zone-a']); + expect(split.orphaned).toEqual([]); + expect(split.retained.map((_) => _.id)).toEqual(['sys-1']); + }); + + it('skips a system that has already been deleted', async () => { + (querySystems as Query).mockImplementation( + systemsByZone({ 'zone-a': [system('sys-1', ['zone-a'])] }), + ); + (showSystem as Query).mockImplementation(() => + Promise.reject(new Error('404')), + ); + const split = await splitZoneSystems(['zone-a']); + expect(split.orphaned).toEqual([]); + expect(split.retained).toEqual([]); + }); + + it('treats a failed lookup as no systems rather than failing', async () => { + (querySystems as Query).mockImplementation(() => + Promise.reject(new Error('nope')), + ); + const split = await splitZoneSystems(['zone-a']); + expect(split.orphaned).toEqual([]); + expect(split.retained).toEqual([]); + }); + }); + + describe('planZoneCascade', () => { + it('is empty for a zone with no systems', async () => { + const plan = await planZoneCascade('zone-a'); + expect(plan.steps).toEqual([]); + }); + + it('only removes systems that would be left without a zone', async () => { + (queryZones as Query).mockImplementation( + zonesByParent({ 'zone-a': [{ id: 'zone-b' }] }), + ); + (querySystems as Query).mockImplementation( + systemsByZone({ + 'zone-a': [system('sys-inside', ['zone-a'], ['mod-1'])], + 'zone-b': [ + system('sys-straddle', ['zone-b', 'zone-other']), + ], + }), + ); + const plan = await planZoneCascade('zone-a'); + + expect(plan.steps.length).toBe(1); + await plan.steps[0].run(); + expect(removeSystem).toHaveBeenCalledExactlyOnceWith('sys-inside'); + + expect(plan.summary).toContain( + 'CASCADE.REMOVE_SYSTEMS:{"count":1}', + ); + expect(plan.summary).toContain( + 'CASCADE.REMOVE_MODULES:{"count":1}', + ); + expect(plan.warnings).toContain('CASCADE.KEEP_SYSTEMS:{"count":1}'); + expect(plan.scope).toContain('CASCADE.SCOPE_ZONES:{"count":1}'); + }); + + it('counts each module once across the removed systems', async () => { + (querySystems as Query).mockImplementation( + systemsByZone({ + 'zone-a': [ + system('sys-1', ['zone-a'], ['mod-1', 'mod-2']), + system('sys-2', ['zone-a'], ['mod-2']), + ], + }), + ); + const plan = await planZoneCascade('zone-a'); + expect(plan.summary).toContain( + 'CASCADE.REMOVE_MODULES:{"count":2}', + ); + }); + + it('never removes the zone itself', async () => { + (querySystems as Query).mockImplementation( + systemsByZone({ 'zone-a': [system('sys-1', ['zone-a'])] }), + ); + const plan = await planZoneCascade('zone-a'); + await Promise.all(plan.steps.map((step) => step.run())); + expect(removeZone).not.toHaveBeenCalled(); + }); + }); + + describe('planDomainCascade', () => { + const domain = (config: Record = {}) => + ({ + id: 'authority-1', + name: 'Acme', + domain: 'acme.example.com', + config, + }) as never; + + it('removes the domain OAuth applications', async () => { + (queryApplications as Query).mockImplementation(() => + page([{ id: '7', name: 'Workplace' }]), + ); + const plan = await planDomainCascade(domain()); + expect(plan.summary).toContain( + 'CASCADE.REMOVE_APPLICATIONS:{"count":1}', + ); + await plan.steps[0].run(); + expect(removeApplication).toHaveBeenCalledExactlyOnceWith('7'); + }); + + it('removes only the staff API tenants matching the domain name', async () => { + (get as Query).mockImplementation(() => + Promise.resolve([ + { id: '1', name: 'Acme', domain: 'acme.example.com' }, + { id: '2', name: 'Other', domain: 'other.example.com' }, + ]), + ); + const plan = await planDomainCascade(domain()); + expect(plan.summary).toContain( + 'CASCADE.REMOVE_TENANTS:{"count":1}', + ); + await plan.steps[0].run(); + expect(del).toHaveBeenCalledExactlyOnceWith( + '/api/staff/v1/tenants/1', + ); + }); + + it('warns and touches no zones when the domain has no org_zone', async () => { + const plan = await planDomainCascade(domain()); + expect(plan.warnings).toContain('CASCADE.NO_ORG_ZONE'); + expect(queryZones).not.toHaveBeenCalled(); + expect(plan.steps).toEqual([]); + }); + + it('leaves the org zone alone when another domain shares it', async () => { + (queryDomains as Query).mockImplementation(() => + page([ + domain({ org_zone: 'zone-org' }), + { + id: 'authority-2', + name: 'Beta', + domain: 'beta.example.com', + config: { org_zone: 'zone-org' }, + }, + ]), + ); + const plan = await planDomainCascade( + domain({ org_zone: 'zone-org' }), + ); + expect( + plan.warnings.some((line) => + line.startsWith('CASCADE.ORG_ZONE_SHARED'), + ), + ).toBe(true); + expect(plan.warnings[0]).toContain('Beta'); + expect(plan.steps).toEqual([]); + expect(showZone).not.toHaveBeenCalled(); + }); + + it('warns when the configured org zone no longer exists', async () => { + (queryDomains as Query).mockImplementation(() => + page([domain({ org_zone: 'zone-org' })]), + ); + (showZone as Query).mockImplementation(() => + Promise.reject(new Error('404')), + ); + const plan = await planDomainCascade( + domain({ org_zone: 'zone-org' }), + ); + expect(plan.warnings).toContain( + 'CASCADE.ORG_ZONE_MISSING:{"id":"zone-org"}', + ); + expect(plan.steps).toEqual([]); + }); + + it('removes the org zone tree when no other domain uses it', async () => { + (queryDomains as Query).mockImplementation(() => + page([domain({ org_zone: 'zone-org' })]), + ); + (showZone as Query).mockImplementation(() => + Promise.resolve({ id: 'zone-org', name: 'ORG Acme' }), + ); + (queryZones as Query).mockImplementation( + zonesByParent({ 'zone-org': [{ id: 'zone-level' }] }), + ); + (querySystems as Query).mockImplementation( + systemsByZone({ + 'zone-level': [system('sys-1', ['zone-level'])], + }), + ); + + const plan = await planDomainCascade( + domain({ org_zone: 'zone-org' }), + ); + + expect(plan.summary).toContain('CASCADE.REMOVE_ORG_ZONE'); + expect(plan.steps.length).toBe(2); + // systems must go before the zone they belong to + await plan.steps[0].run(); + expect(removeSystem).toHaveBeenCalledExactlyOnceWith('sys-1'); + await plan.steps[1].run(); + expect(removeZone).toHaveBeenCalledExactlyOnceWith('zone-org'); + }); + }); + + describe('runCascade', () => { + it('runs every step in order and reports progress', async () => { + const order: string[] = []; + const messages: string[] = []; + const plan = { + scope: [], + summary: [], + warnings: [], + steps: [ + { + label: 'first', + run: async () => { + order.push('first'); + }, + }, + { + label: 'second', + run: async () => { + order.push('second'); + }, + }, + ], + }; + const outcome = await runCascade(plan, (m) => messages.push(m)); + expect(order).toEqual(['first', 'second']); + expect(outcome.removed).toBe(2); + expect(outcome.failures).toEqual([]); + expect(messages[0]).toContain('"index":1'); + expect(messages[0]).toContain('"total":2'); + }); + + it('continues past a failing step and reports it', async () => { + const plan = { + scope: [], + summary: [], + warnings: [], + steps: [ + { + label: 'broken', + run: () => Promise.reject(new Error('boom')), + }, + { label: 'ok', run: () => Promise.resolve() }, + ], + }; + const outcome = await runCascade(plan); + expect(outcome.removed).toBe(1); + expect(outcome.failures.length).toBe(1); + expect(outcome.failures[0].label).toBe('broken'); + }); + }); +}); diff --git a/src/tests/common/cascade-locale.spec.ts b/src/tests/common/cascade-locale.spec.ts new file mode 100644 index 000000000..a20ee589c --- /dev/null +++ b/src/tests/common/cascade-locale.spec.ts @@ -0,0 +1,80 @@ +import { describe, expect, it, vi } from 'vitest'; + +vi.mock('@placeos/ts-client', () => ({ + showMetadata: vi.fn(() => ({ + toPromise: () => Promise.resolve({ details: {} }), + })), +})); + +vi.mock('../../app/common/general', () => ({ log: vi.fn() })); + +import { LocaleService } from '../../app/common/locale.service'; + +/** + * The cascade summary lines are the only place in the app that leans on the + * locale service's plural lookup (`KEY_` then `KEY_N` then `KEY`), so + * the singular and plural wording is pinned here against the real locale file. + */ +describe('cascade locale strings', () => { + const locale = new LocaleService(); + const get = (key: string, count: number) => + locale.get(key, { count }, count); + + it.each([ + ['CASCADE.SCOPE_ZONES', 1, 'Scope: this zone and 1 zone beneath it.'], + ['CASCADE.SCOPE_ZONES', 4, 'Scope: this zone and 4 zones beneath it.'], + ['CASCADE.REMOVE_SYSTEMS', 1, '1 system left without a zone'], + ['CASCADE.REMOVE_SYSTEMS', 2, '2 systems left without a zone'], + [ + 'CASCADE.KEEP_SYSTEMS', + 1, + '1 system also belongs to a zone outside this one and will be kept.', + ], + [ + 'CASCADE.KEEP_SYSTEMS', + 3, + '3 systems also belong to zones outside this one and will be kept.', + ], + ['CASCADE.REMOVE_APPLICATIONS', 1, '1 OAuth application'], + ['CASCADE.REMOVE_APPLICATIONS', 2, '2 OAuth applications'], + ['CASCADE.SUCCESS', 1, 'Removed 1 associated resource.'], + ['CASCADE.SUCCESS', 5, 'Removed 5 associated resources.'], + ])('renders %s for a count of %i', (key, count, expected) => { + expect(get(key as string, count as number)).toBe(expected); + }); + + it('renders the module line for both counts', () => { + expect(get('CASCADE.REMOVE_MODULES', 1)).toContain('1 module in'); + expect(get('CASCADE.REMOVE_MODULES', 4)).toContain('4 modules in'); + }); + + it('renders the tenant line for both counts', () => { + expect(get('CASCADE.REMOVE_TENANTS', 1)).toContain( + '1 staff API tenant,', + ); + expect(get('CASCADE.REMOVE_TENANTS', 2)).toContain( + '2 staff API tenants,', + ); + }); + + it('resolves the option labels used on the delete confirmation', () => { + expect(locale.get('ZONES.DELETE_CASCADE')).toBe( + 'Also delete associated resources', + ); + expect(locale.get('DOMAINS.DELETE_CASCADE')).toBe( + 'Also delete associated resources', + ); + expect(locale.get('ZONES.DELETE_CASCADE_DESC')).not.toBe( + 'ZONES.DELETE_CASCADE_DESC', + ); + expect(locale.get('DOMAINS.DELETE_CASCADE_DESC')).not.toBe( + 'DOMAINS.DELETE_CASCADE_DESC', + ); + }); + + it('no longer claims that deleting a zone removes its systems', () => { + const message = locale.get('ZONES.DELETE_MSG'); + expect(message).toContain('are kept unless you also remove'); + expect(message).not.toContain('remove systems without another zone'); + }); +}); diff --git a/src/tests/overlays/confirm-modal.component.spec.ts b/src/tests/overlays/confirm-modal.component.spec.ts index 62fa7062f..26588af8a 100644 --- a/src/tests/overlays/confirm-modal.component.spec.ts +++ b/src/tests/overlays/confirm-modal.component.spec.ts @@ -212,6 +212,155 @@ describe('ConfirmModalComponent', () => { }); }); + describe('with options', () => { + let details: ReturnType; + let resolve_details: (value: unknown) => void; + + const build = async (option_overrides = {}) => { + await TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + imports: [ + ConfirmModalComponent, + MatDialogModule, + NoopAnimationsModule, + ], + providers: [ + { provide: MatDialogRef, useValue: dialog_ref_mock }, + { + provide: MAT_DIALOG_DATA, + useValue: { + ...default_data, + options: [ + { + id: 'cascade', + label: 'Also delete associated resources', + description: 'Removes orphaned systems', + details, + ...option_overrides, + }, + ], + }, + }, + ], + }) + .overrideComponent(ConfirmModalComponent, { + remove: { imports: [IconComponent] }, + add: { imports: [mockComponent(IconComponent)] }, + }) + .compileComponents(); + fixture = TestBed.createComponent(ConfirmModalComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }; + + beforeEach(async () => { + details = vi.fn( + () => + new Promise((resolve) => { + resolve_details = resolve; + }), + ); + await build(); + }); + + it('should start with the option disabled', () => { + expect(component.isSelected('cascade')).toBe(false); + }); + + it('should honour an option that defaults to enabled', async () => { + await build({ enabled: true }); + expect(component.isSelected('cascade')).toBe(true); + }); + + it('should not resolve details until the option is enabled', () => { + expect(details).not.toHaveBeenCalled(); + }); + + it('should resolve details when the option is enabled', async () => { + component.toggleOption(component.options[0], true); + expect(details).toHaveBeenCalledOnce(); + expect(component.isLoadingDetails('cascade')).toBe(true); + expect(component.resolving()).toBe(true); + + resolve_details({ summary: ['2 systems'], warnings: [] }); + await Promise.resolve(); + await Promise.resolve(); + + expect(component.resolving()).toBe(false); + expect(component.detailsFor('cascade')).toEqual({ + summary: ['2 systems'], + warnings: [], + }); + }); + + it('should only resolve details once', async () => { + component.toggleOption(component.options[0], true); + resolve_details({ summary: [] }); + await Promise.resolve(); + await Promise.resolve(); + component.toggleOption(component.options[0], false); + component.toggleOption(component.options[0], true); + expect(details).toHaveBeenCalledOnce(); + }); + + it('should surface a failure to resolve details', async () => { + details.mockImplementation(() => + Promise.reject(new Error('lookup failed')), + ); + await build(); + component.toggleOption(component.options[0], true); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + expect(component.detailsError('cascade')).toBe('lookup failed'); + expect(component.resolving()).toBe(false); + }); + + it('should block confirmation while details are resolving', () => { + const event_spy = vi.fn(); + component.event.subscribe(event_spy); + component.toggleOption(component.options[0], true); + + component.onConfirm(); + + expect(event_spy).not.toHaveBeenCalled(); + }); + + it('should report the selection on confirmation', () => { + const event_spy = vi.fn(); + component.event.subscribe(event_spy); + + component.onConfirm(); + + expect(event_spy).toHaveBeenCalledWith({ + reason: 'done', + metadata: { options: { cascade: false } }, + }); + }); + + it('should report an enabled selection on confirmation', async () => { + await build({ enabled: true, details: undefined }); + const event_spy = vi.fn(); + component.event.subscribe(event_spy); + + component.onConfirm(); + + expect(event_spy).toHaveBeenCalledWith({ + reason: 'done', + metadata: { options: { cascade: true } }, + }); + }); + + it('should render the option checkbox', () => { + const option_el = + fixture.nativeElement.querySelector('[confirm-option]'); + expect(option_el).toBeTruthy(); + expect(option_el.textContent).toContain( + 'Also delete associated resources', + ); + }); + }); + describe('disableClose/enableClose', () => { it('should set disableClose to true when calling disableClose', () => { expect(dialog_ref_mock.disableClose).toBe(false); From 99aca42f42f8bad2a55ca02774f3414d43d7a9be Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Tue, 28 Jul 2026 15:44:39 +1000 Subject: [PATCH 2/9] fix(delete): drop the duplicate scope line on a domain cascade The org zone line already reads "and everything beneath it", so appending the nested zone plan's own scope line stacked two "Scope:" sentences on top of each other in the dialog. --- src/app/common/cascade-delete.ts | 7 +++---- src/tests/common/cascade-delete.spec.ts | 4 ++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/app/common/cascade-delete.ts b/src/app/common/cascade-delete.ts index 50f81d26c..f34026d7a 100644 --- a/src/app/common/cascade-delete.ts +++ b/src/app/common/cascade-delete.ts @@ -358,10 +358,9 @@ export async function planDomainCascade( } const zone_plan = await planZoneCascade(org_zone_id); - plan.scope.push( - i18n('CASCADE.SCOPE_ORG_ZONE', { name: org_zone.name }), - ...zone_plan.scope, - ); + // The org zone line already says "and everything beneath it", so the zone + // plan's own scope line would just repeat it. + plan.scope.push(i18n('CASCADE.SCOPE_ORG_ZONE', { name: org_zone.name })); plan.summary.push(...zone_plan.summary, i18n('CASCADE.REMOVE_ORG_ZONE')); plan.warnings.push(...zone_plan.warnings); plan.steps.push(...zone_plan.steps, { diff --git a/src/tests/common/cascade-delete.spec.ts b/src/tests/common/cascade-delete.spec.ts index eb9397348..cc1d6b6af 100644 --- a/src/tests/common/cascade-delete.spec.ts +++ b/src/tests/common/cascade-delete.spec.ts @@ -392,6 +392,10 @@ describe('cascade-delete', () => { ); expect(plan.summary).toContain('CASCADE.REMOVE_ORG_ZONE'); + // one scope line, not the org zone's plus the zone plan's own + expect(plan.scope).toEqual([ + 'CASCADE.SCOPE_ORG_ZONE:{"name":"ORG Acme"}', + ]); expect(plan.steps.length).toBe(2); // systems must go before the zone they belong to await plan.steps[0].run(); From e3621cbe02eb508dc4e9a87f08c9d3f2bd889b86 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Tue, 28 Jul 2026 16:01:47 +1000 Subject: [PATCH 3/9] fix(ui): make the delete confirmation readable on the dark theme MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The checkbox label and the empty box outline both rendered near-black on the dark background. The app already styles `mat-checkbox`, but those rules target `.mdc-checkbox__background` and `.mdc-label`, which never win against Angular Material's own selectors — so the colours were still coming from the prebuilt `indigo-pink` (light) palette. Nothing hit this before because no other `` in the app has label content, and the faint outline was easy to miss. Set at the token layer instead, where Material actually reads them, using `currentColor` so they follow whichever theme is active. Measured on the delete confirmation: label outline dark 1.21:1 -> 10.84:1 (WCAG wants 3:1 for controls) label text dark 1.18:1 -> 10.84:1 (WCAG wants 4.5:1 for body text) Light theme is unchanged at 20.12:1. This affects every checkbox on the dark theme, all of which had the same invisible outline. The cascade warning line used `text-warning` — a yellow that only reads on a dark background (1.39:1 on the light theme). It now tints the block and keeps the inherited text colour, matching how warnings are done elsewhere. Secondary text nudged from 60% to 70% opacity for the same reason. Every line in the dialog now passes WCAG AA in both themes. --- src/app/overlays/confirm-modal.component.ts | 10 +++++++--- src/styles.css | 11 ++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/app/overlays/confirm-modal.component.ts b/src/app/overlays/confirm-modal.component.ts index f51091f01..b837f1dc5 100644 --- a/src/app/overlays/confirm-modal.component.ts +++ b/src/app/overlays/confirm-modal.component.ts @@ -142,7 +142,7 @@ export async function openConfirmModal( {{ option.label }} @if (option.description) { -

+

{{ option.description }}

} @@ -166,7 +166,7 @@ export async function openConfirmModal(

} @else if (detailsFor(option.id); as detail) { @for (line of detail.scope; track line) { -

+

{{ line }}

} @@ -191,9 +191,13 @@ export async function openConfirmModal(

} @for (line of detail.warnings; track line) { +

{{ line }}

diff --git a/src/styles.css b/src/styles.css index 40c2bfe73..d7c587e8e 100644 --- a/src/styles.css +++ b/src/styles.css @@ -1055,8 +1055,17 @@ a[icon] { /* Checkboxes & Radio */ mat-checkbox { + /* Material resolves these from the prebuilt (light) palette, so both the + label and the empty box outline come out near-black — invisible on the + dark theme. The rules below on `.mdc-checkbox__background` never won + against Material's own selectors, so the colours are set at the token + layer instead. `currentColor` follows whichever theme is active. */ + --mat-checkbox-label-text-color: currentColor; + --mat-checkbox-unselected-icon-color: currentColor; + --mat-checkbox-unselected-hover-icon-color: currentColor; + --mat-checkbox-unselected-focus-icon-color: currentColor; + .mdc-checkbox__background { - border-color: var(--neutral); svg { color: var(--info-content); } From 1af811437bdb0d443b3c6c84da1d413f3f266cdb Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 29 Jul 2026 13:43:31 +1000 Subject: [PATCH 4/9] feat(delete): show what a cascade removed, with ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A toast saying "Removed 6 associated resources" is not something you can act on afterwards. When a cascade runs, the confirmation now becomes a receipt listing every resource that went — type, name and id — in the order they were removed, ending with the item itself. A "Copy list" button puts it on the clipboard as tab separated rows for pasting into a ticket. Cascade steps carry a `CascadeResource` ({type, id, name}) instead of a pre-rendered progress string, and `CascadeOutcome.removed` is that list rather than a count, so the receipt reports what actually happened rather than what was planned. Partial failures now show the same receipt with the failures called out separately, and the item is deliberately left in place — previously this was a notification that named only the first failure and left the operator guessing which of the rest had gone. Existing delete flows are untouched: the receipt only appears when a cascade actually ran, so every other resource type still closes the dialog and shows its usual notification. There is an e2e test asserting exactly that. `receiptToTsv` is a pure function so the clipboard formatting is tested without DI. It also keeps `@angular/cdk/clipboard` out of the spec, which upsets vitest's `vi.mock` hoisting analysis and made it warn about mocks it had previously accepted. Verified against the local stack: deleting a domain with two OAuth apps, a staff tenant, an org zone tree and three enclosed systems produced an 8 row receipt, and every id on it was confirmed absent from the API afterwards while the four intended survivors remained. --- e2e/src/cascade-delete.spec.ts | 53 +++++- public/assets/locale/en-AU.json | 17 +- public/assets/locale/en-GB.json | 17 +- public/assets/locale/en-US.json | 17 +- src/app/common/actions.ts | 5 + src/app/common/cascade-delete.ts | 70 ++++++-- src/app/common/item.service.ts | 65 ++++++-- src/app/overlays/confirm-modal.component.ts | 146 ++++++++++++++++- src/tests/common/cascade-delete.spec.ts | 61 ++++++- .../overlays/confirm-modal.component.spec.ts | 152 ++++++++++++++++-- 10 files changed, 545 insertions(+), 58 deletions(-) diff --git a/e2e/src/cascade-delete.spec.ts b/e2e/src/cascade-delete.spec.ts index 7f2b63084..acbb8b61a 100644 --- a/e2e/src/cascade-delete.spec.ts +++ b/e2e/src/cascade-delete.spec.ts @@ -108,7 +108,10 @@ test.describe('Cascade delete', () => { expect(expected).toBeGreaterThan(0); await zonesPage.acceptButton.click(); - await page.waitForURL(/#\/zones\/-/, { timeout: 30000 }); + await page.waitForSelector('confirm-modal [result-items]', { + timeout: 60000, + }); + await page.locator('confirm-modal button[name="close"]').click(); // Every system lived inside this org zone, so the systems list empties await page.goto('/?mock=true#/systems'); @@ -117,6 +120,54 @@ test.describe('Cascade delete', () => { await expect(zonesPage.sidebarItems).toHaveCount(0); }); + test('lists what was removed, with ids, once it has run', async ({ + page, + }) => { + await openZone(page, 'zone-Kl0E0HmCJ3'); + await zonesPage.openDeleteConfirmation(); + await zonesPage.enableCascade(); + + const summary = await zonesPage.cascadeSummary.innerText(); + const systems = Number(summary.match(/(\d+) systems? left/)?.[1] || 0); + + await zonesPage.acceptButton.click(); + await page.waitForSelector('confirm-modal [result-items]', { + timeout: 60000, + }); + + // one row per system, plus the zone itself + const rows = page.locator('confirm-modal [result-items] li'); + await expect(rows).toHaveCount(systems + 1); + + const receipt = await page + .locator('confirm-modal [result-items]') + .innerText(); + expect(receipt).toContain('zone-Kl0E0HmCJ3'); + expect(receipt).toContain('Place Technology'); + // every row carries an id + for (const row of await rows.all()) { + expect(await row.innerText()).toMatch(/(sys|zone)-\S+/); + } + + // the confirmation buttons are replaced by a single close + await expect(zonesPage.acceptButton).toHaveCount(0); + await expect( + page.locator('confirm-modal button[name="close"]'), + ).toBeVisible(); + }); + + test('shows no receipt for a delete without the option', async ({ + page, + }) => { + await openZone(page, 'zone-lmhh_hVfz0'); + await zonesPage.openDeleteConfirmation(); + await zonesPage.acceptButton.click(); + + // unchanged behaviour: the dialog closes itself, no receipt + await page.waitForURL(/#\/zones\/-/, { timeout: 30000 }); + await expect(page.locator('confirm-modal')).toHaveCount(0); + }); + test('leaves systems alone when the option is left off', async ({ page, }) => { diff --git a/public/assets/locale/en-AU.json b/public/assets/locale/en-AU.json index 38dc1ea90..0d37a14f4 100644 --- a/public/assets/locale/en-AU.json +++ b/public/assets/locale/en-AU.json @@ -47,6 +47,7 @@ "NOTES": "Notes", "NONE": "None", "CANCEL": "Cancel", + "CLOSE": "Close", "CONFIRM": "Confirm", "SAVE": "Save", "SAVE_ALL": "Save All", @@ -1233,6 +1234,20 @@ "SUCCESS": "Removed {{ count }} associated resources.", "SUCCESS_1": "Removed {{ count }} associated resource.", "FAILED": "Failed to remove {{ count }} associated resources, so nothing further was deleted. Error: {{ error }}", - "FAILED_1": "Failed to remove {{ count }} associated resource, so nothing further was deleted. Error: {{ error }}" + "FAILED_1": "Failed to remove {{ count }} associated resource, so nothing further was deleted. Error: {{ error }}", + "TYPE_SYSTEM": "System", + "TYPE_ZONE": "Zone", + "TYPE_APPLICATION": "Application", + "TYPE_TENANT": "Tenant", + "TYPE_DOMAIN": "Domain", + "REMOVING_DOMAIN": "Removing domain \"{{ name }}\"", + "RECEIPT_TITLE": "Removed", + "RECEIPT_NOTE": "Modules, triggers, metadata and settings belonging to these resources were removed with them by the server, so they are not listed individually.", + "RECEIPT_PARTIAL_TITLE": "Partly removed", + "RECEIPT_PARTIAL_NOTE": "\"{{ name }}\" was left in place because not everything associated with it could be removed. Resolve the failures above and try again.", + "RECEIPT_FAILED": "Could not be removed", + "RECEIPT_COPY": "Copy list", + "RECEIPT_COPIED": "Copied {{ count }} rows to the clipboard.", + "RECEIPT_COPIED_1": "Copied {{ count }} row to the clipboard." } } diff --git a/public/assets/locale/en-GB.json b/public/assets/locale/en-GB.json index e26ad9a77..38b92940c 100644 --- a/public/assets/locale/en-GB.json +++ b/public/assets/locale/en-GB.json @@ -42,6 +42,7 @@ "NOTES": "Notes", "NONE": "None", "CANCEL": "Cancel", + "CLOSE": "Close", "CONFIRM": "Confirm", "SAVE": "Save", "SAVE_ALL": "Save All", @@ -1023,6 +1024,20 @@ "SUCCESS": "Removed {{ count }} associated resources.", "SUCCESS_1": "Removed {{ count }} associated resource.", "FAILED": "Failed to remove {{ count }} associated resources, so nothing further was deleted. Error: {{ error }}", - "FAILED_1": "Failed to remove {{ count }} associated resource, so nothing further was deleted. Error: {{ error }}" + "FAILED_1": "Failed to remove {{ count }} associated resource, so nothing further was deleted. Error: {{ error }}", + "TYPE_SYSTEM": "System", + "TYPE_ZONE": "Zone", + "TYPE_APPLICATION": "Application", + "TYPE_TENANT": "Tenant", + "TYPE_DOMAIN": "Domain", + "REMOVING_DOMAIN": "Removing domain \"{{ name }}\"", + "RECEIPT_TITLE": "Removed", + "RECEIPT_NOTE": "Modules, triggers, metadata and settings belonging to these resources were removed with them by the server, so they are not listed individually.", + "RECEIPT_PARTIAL_TITLE": "Partly removed", + "RECEIPT_PARTIAL_NOTE": "\"{{ name }}\" was left in place because not everything associated with it could be removed. Resolve the failures above and try again.", + "RECEIPT_FAILED": "Could not be removed", + "RECEIPT_COPY": "Copy list", + "RECEIPT_COPIED": "Copied {{ count }} rows to the clipboard.", + "RECEIPT_COPIED_1": "Copied {{ count }} row to the clipboard." } } diff --git a/public/assets/locale/en-US.json b/public/assets/locale/en-US.json index d6d6047fd..eaecacdff 100644 --- a/public/assets/locale/en-US.json +++ b/public/assets/locale/en-US.json @@ -38,6 +38,7 @@ "NOTES": "Notes", "NONE": "None", "CANCEL": "Cancel", + "CLOSE": "Close", "CONFIRM": "Confirm", "SAVE": "Save", "SAVE_ALL": "Save All", @@ -1020,6 +1021,20 @@ "SUCCESS": "Removed {{ count }} associated resources.", "SUCCESS_1": "Removed {{ count }} associated resource.", "FAILED": "Failed to remove {{ count }} associated resources, so nothing further was deleted. Error: {{ error }}", - "FAILED_1": "Failed to remove {{ count }} associated resource, so nothing further was deleted. Error: {{ error }}" + "FAILED_1": "Failed to remove {{ count }} associated resource, so nothing further was deleted. Error: {{ error }}", + "TYPE_SYSTEM": "System", + "TYPE_ZONE": "Zone", + "TYPE_APPLICATION": "Application", + "TYPE_TENANT": "Tenant", + "TYPE_DOMAIN": "Domain", + "REMOVING_DOMAIN": "Removing domain \"{{ name }}\"", + "RECEIPT_TITLE": "Removed", + "RECEIPT_NOTE": "Modules, triggers, metadata and settings belonging to these resources were removed with them by the server, so they are not listed individually.", + "RECEIPT_PARTIAL_TITLE": "Partly removed", + "RECEIPT_PARTIAL_NOTE": "\"{{ name }}\" was left in place because not everything associated with it could be removed. Resolve the failures above and try again.", + "RECEIPT_FAILED": "Could not be removed", + "RECEIPT_COPY": "Copy list", + "RECEIPT_COPIED": "Copied {{ count }} rows to the clipboard.", + "RECEIPT_COPIED_1": "Copied {{ count }} row to the clipboard." } } diff --git a/src/app/common/actions.ts b/src/app/common/actions.ts index 9d837f0ca..ca92ba218 100644 --- a/src/app/common/actions.ts +++ b/src/app/common/actions.ts @@ -60,6 +60,7 @@ import { } from '@placeos/ts-client'; import { CascadePlan, + CascadeResourceType, planDomainCascade, planZoneCascade, } from './cascade-delete'; @@ -83,6 +84,8 @@ export interface ItemCascade { label: string; /** i18n key for the text shown under the checkbox */ description: string; + /** Type of the item itself, so it can be labelled on the receipt */ + resource_type: CascadeResourceType; /** Resolves what would be removed alongside the item */ plan: (_: T) => Promise; } @@ -115,6 +118,7 @@ const domains: ItemActions = { cascade: { label: 'DOMAINS.DELETE_CASCADE', description: 'DOMAINS.DELETE_CASCADE_DESC', + resource_type: 'domain', plan: (item) => planDomainCascade(item), }, name: 'DOMAINS', @@ -347,6 +351,7 @@ const zones: ItemActions = { cascade: { label: 'ZONES.DELETE_CASCADE', description: 'ZONES.DELETE_CASCADE_DESC', + resource_type: 'zone', plan: (item) => planZoneCascade(item.id), }, name: 'ZONES', diff --git a/src/app/common/cascade-delete.ts b/src/app/common/cascade-delete.ts index f34026d7a..e3e155803 100644 --- a/src/app/common/cascade-delete.ts +++ b/src/app/common/cascade-delete.ts @@ -42,10 +42,38 @@ const MAX_PAGES = 100; /** Concurrent requests issued while resolving a plan. */ const READ_CONCURRENCY = 8; +/** Kinds of resource a cascade can remove directly. */ +export type CascadeResourceType = + | 'system' + | 'zone' + | 'application' + | 'tenant' + | 'domain'; + +/** A resource a cascade removed, or is about to. */ +export interface CascadeResource { + type: CascadeResourceType; + id: string; + name: string; +} + +/** i18n key for the progress message of each resource type */ +const REMOVING_KEY: Record = { + system: 'CASCADE.REMOVING_SYSTEM', + zone: 'CASCADE.REMOVING_ZONE', + application: 'CASCADE.REMOVING_APPLICATION', + tenant: 'CASCADE.REMOVING_TENANT', + domain: 'CASCADE.REMOVING_DOMAIN', +}; + +/** Progress message shown while `resource` is being removed. */ +export const removingLabel = (resource: CascadeResource) => + i18n(REMOVING_KEY[resource.type], { name: resource.name }); + /** A single removal performed as part of a cascade. */ export interface CascadeStep { - /** Progress message shown while the step runs */ - label: string; + /** What the step removes, reported back once it has run */ + resource: CascadeResource; /** Performs the removal. Rejects on failure. */ run: () => Promise; } @@ -64,10 +92,10 @@ export interface CascadePlan { /** Result of executing a `CascadePlan`. */ export interface CascadeOutcome { - /** Number of steps that completed */ - removed: number; + /** Resources that were removed, in the order they went */ + removed: CascadeResource[]; /** Steps that threw, kept so the caller can report them */ - failures: { label: string; error: unknown }[]; + failures: { resource: CascadeResource; error: unknown }[]; } const emptyPlan = (): CascadePlan => ({ @@ -242,7 +270,11 @@ export async function planZoneCascade(zone_id: string): Promise { ); } plan.steps = orphaned.map((system) => ({ - label: i18n('CASCADE.REMOVING_SYSTEM', { name: system.name }), + resource: { + type: 'system' as const, + id: system.id, + name: system.name, + }, run: () => removeSystem(system.id), })); return plan; @@ -304,9 +336,11 @@ export async function planDomainCascade( ); plan.steps.push( ...applications.map((application) => ({ - label: i18n('CASCADE.REMOVING_APPLICATION', { + resource: { + type: 'application' as const, + id: `${application.id}`, name: application.name, - }), + }, run: () => removeApplication(application.id), })), ); @@ -322,9 +356,11 @@ export async function planDomainCascade( ); plan.steps.push( ...tenants.map((tenant) => ({ - label: i18n('CASCADE.REMOVING_TENANT', { + resource: { + type: 'tenant' as const, + id: `${tenant.id}`, name: tenant.name || tenant.domain, - }), + }, run: () => del(`/api/staff/v1/tenants/${tenant.id}`), })), ); @@ -364,7 +400,11 @@ export async function planDomainCascade( plan.summary.push(...zone_plan.summary, i18n('CASCADE.REMOVE_ORG_ZONE')); plan.warnings.push(...zone_plan.warnings); plan.steps.push(...zone_plan.steps, { - label: i18n('CASCADE.REMOVING_ZONE', { name: org_zone.name }), + resource: { + type: 'zone' as const, + id: org_zone_id, + name: org_zone.name, + }, run: () => removeZone(org_zone_id), }); return plan; @@ -379,21 +419,21 @@ export async function runCascade( plan: CascadePlan, progress: (message: string) => void = () => undefined, ): Promise { - const outcome: CascadeOutcome = { removed: 0, failures: [] }; + const outcome: CascadeOutcome = { removed: [], failures: [] }; const total = plan.steps.length; for (const [index, step] of plan.steps.entries()) { progress( i18n('CASCADE.PROGRESS', { - step: step.label, + step: removingLabel(step.resource), index: index + 1, total, }), ); try { await step.run(); - outcome.removed += 1; + outcome.removed.push(step.resource); } catch (error) { - outcome.failures.push({ label: step.label, error }); + outcome.failures.push({ resource: step.resource, error }); } } return outcome; diff --git a/src/app/common/item.service.ts b/src/app/common/item.service.ts index 878e2b957..b2b356cc9 100644 --- a/src/app/common/item.service.ts +++ b/src/app/common/item.service.ts @@ -26,7 +26,12 @@ import { DuplicateModalComponent } from '../overlays/duplicate-modal.component'; import { BackofficeUsersService } from '../users/users.service'; import { ACTIONS, ItemActions } from './actions'; import { AsyncHandler } from './async-handler.class'; -import { CascadePlan, runCascade } from './cascade-delete'; +import { + CascadeOutcome, + CascadePlan, + CascadeResource, + runCascade, +} from './cascade-delete'; import { log } from './general'; import { i18n } from './locale.service'; import { notifyError, notifySuccess } from './notifications'; @@ -35,6 +40,14 @@ import { waitForEvent } from './signals'; /** Id the "also delete associated resources" toggle is reported under */ const CASCADE_OPTION = 'cascade'; +/** Turns removed resources into the receipt rows the confirm modal renders. */ +const receiptItems = (resources: CascadeResource[]) => + resources.map((resource) => ({ + type: i18n(`CASCADE.TYPE_${resource.type.toUpperCase()}`), + id: resource.id, + name: resource.name, + })); + export type ResourceType = | 'domains' | 'drivers' @@ -296,12 +309,25 @@ export class ActiveItemService extends AsyncHandler { ref.componentInstance.loading.set( i18n(`${actions.name}.DELETE_LOADING`), ); + let outcome: CascadeOutcome | null = null; if (event.metadata?.options?.[CASCADE_OPTION] && plan) { - const outcome = await runCascade(plan, (message) => + outcome = await runCascade(plan, (message) => ref.componentInstance.loading.set(message), ); if (outcome.failures.length) { + // Something is still referencing the item, so leave it in + // place and show what did and did not go. ref.componentInstance.loading.set(''); + ref.componentInstance.result.set({ + title: i18n('CASCADE.RECEIPT_PARTIAL_TITLE'), + items: receiptItems(outcome.removed), + failed: receiptItems( + outcome.failures.map((_) => _.resource), + ), + note: i18n('CASCADE.RECEIPT_PARTIAL_NOTE', { + name: item.name, + }), + }); return notifyError( i18n( 'CASCADE.FAILED', @@ -309,21 +335,13 @@ export class ActiveItemService extends AsyncHandler { count: outcome.failures.length, error: (outcome.failures[0].error as Error) - ?.message || outcome.failures[0].label, + ?.message || + outcome.failures[0].resource.name, }, outcome.failures.length, ), ); } - if (outcome.removed) { - notifySuccess( - i18n( - 'CASCADE.SUCCESS', - { count: outcome.removed }, - outcome.removed, - ), - ); - } ref.componentInstance.loading.set( i18n(`${actions.name}.DELETE_LOADING`), ); @@ -331,14 +349,31 @@ export class ActiveItemService extends AsyncHandler { await actions .remove(item) .then(() => { + this._active_item.set(null); + this.removeItem(item); + this._router.navigate([`/${this._type}`, '-', 'about']); + if (outcome) { + // A cascade removed more than the item itself, so show + // the receipt rather than closing on a notification. + ref.componentInstance.result.set({ + title: i18n('CASCADE.RECEIPT_TITLE'), + items: receiptItems([ + ...outcome.removed, + { + type: cascade.resource_type, + id: `${item.id}`, + name: item.name, + }, + ]), + note: i18n('CASCADE.RECEIPT_NOTE'), + }); + return; + } notifySuccess( i18n(`${actions.name}.DELETE_SUCCESS`, { name: item.name, }), ); - this._active_item.set(null); - this.removeItem(item); - this._router.navigate([`/${this._type}`, '-', 'about']); ref.close(); }) .catch((err) => { diff --git a/src/app/overlays/confirm-modal.component.ts b/src/app/overlays/confirm-modal.component.ts index b837f1dc5..ad332a607 100644 --- a/src/app/overlays/confirm-modal.component.ts +++ b/src/app/overlays/confirm-modal.component.ts @@ -14,11 +14,14 @@ import { MatDialogRef, } from '@angular/material/dialog'; +import { Clipboard } from '@angular/cdk/clipboard'; import { MatCheckboxModule } from '@angular/material/checkbox'; import { MatRippleModule } from '@angular/material/core'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; import { lastValueFrom } from 'rxjs'; import { AsyncHandler } from '../common/async-handler.class'; +import { i18n } from '../common/locale.service'; +import { notifyInfo } from '../common/notifications'; import { waitForEvent } from '../common/signals'; import { ApplicationIcon, DialogEvent } from '../common/types'; import { IconComponent } from '../ui/icon.component'; @@ -55,6 +58,31 @@ export interface ConfirmModalOption { /** Options selected on confirmation, keyed by `ConfirmModalOption.id` */ export type ConfirmModalSelection = Record; +/** A resource listed in the post-action receipt */ +export interface ConfirmModalResultItem { + /** Short type label, e.g. "System" */ + type: string; + /** Resource id, so the removal can be traced afterwards */ + id: string; + /** Resource name at the time it was removed */ + name: string; +} + +/** + * Replaces the modal body once the action has run, so the user gets a receipt + * of what actually happened rather than a transient notification. + */ +export interface ConfirmModalResult { + /** Heading above the list */ + title: string; + /** Resources that were removed */ + items: ConfirmModalResultItem[]; + /** Resources that could not be removed */ + failed?: ConfirmModalResultItem[]; + /** Explanatory line under the list */ + note?: string; +} + export interface ConfirmModalData { /** Title of the modal */ title: string; @@ -74,6 +102,24 @@ export interface ConfirmModalData { close_delay?: number; } +/** + * Renders a receipt as tab separated rows, for pasting into a ticket or + * spreadsheet. Failed rows are marked so a partial run is not mistaken for a + * complete one. + */ +export function receiptToTsv(result: ConfirmModalResult): string { + const rows = [ + ...result.items.map((item) => [item.type, item.name, item.id]), + ...(result.failed || []).map((item) => [ + item.type, + item.name, + item.id, + 'FAILED', + ]), + ]; + return rows.map((row) => row.join('\t')).join('\n'); +} + export const CONFIRM_METADATA = { height: 'auto', }; @@ -115,9 +161,80 @@ export async function openConfirmModal(
-

{{ title }}

+

+ {{ result() ? result().title : title }} +

- @if (!loading()) { + @if (result(); as receipt) { +
+ @if (receipt.items.length) { +
    + @for (item of receipt.items; track item.id) { +
  • + + {{ item.type }} + + + {{ item.name }} + + +
    • + } +
+ } + @if (receipt.failed?.length) { +
+

+ {{ 'CASCADE.RECEIPT_FAILED' | translate }} +

+ @for (item of receipt.failed; track item.id) { +

+ {{ item.type }} — {{ item.name }} + + {{ item.id }} + +

+ } +
+ } + @if (receipt.note) { +

{{ receipt.note }}

+ } + +
+
+ +
+ } @else if (!loading()) {
@@ -218,7 +335,7 @@ export async function openConfirmModal(
} - @if (!loading()) { + @if (!loading() && !result()) {