From 39df6ed7b97ee1456f1d0344a99cbda34d08b060 Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Sun, 16 Aug 2026 11:48:49 +0200 Subject: [PATCH] fix(gate-57): drop the caller-less cascade-delete wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gate-57 orphaned-write-capability reported 1 finding over 82 lib/Service files: DashboardVersionService::deleteVersionsForDashboard(). It was a literal pass-through to DashboardVersionMapper::deleteByDashboardUuid() with zero callers. The live cascade path is Listener\VersionsListener, which is registered against DashboardDeletedEvent in Application.php per dashboard-cascade-events REQ-CSC-002/REQ-CSC-003 and calls the mapper directly, inside the log-and-continue envelope REQ-CSC-006 requires. Routing the listener through a second wrapper would add a hop and nothing else, so the wrapper is removed rather than wired. The dashboard-versioning spec claimed the wrapper was 'wired and ready for the sibling cascade-events listener; the dashboard delete path itself does not yet invoke them'. That was stale — the listener has shipped. The spec now records the real path and states that there is deliberately no service-layer wrapper. Before: 1 finding over 82 files. After: 0 findings over 82 files. --- lib/Service/DashboardVersionService.php | 26 +++++++------------ openspec/specs/dashboard-versioning/spec.md | 2 +- .../Service/DashboardVersionServiceTest.php | 19 -------------- tools/spec-annotations-allowlist.txt | 1 - 4 files changed, 11 insertions(+), 37 deletions(-) diff --git a/lib/Service/DashboardVersionService.php b/lib/Service/DashboardVersionService.php index d0d6138c6..4448b441d 100644 --- a/lib/Service/DashboardVersionService.php +++ b/lib/Service/DashboardVersionService.php @@ -527,23 +527,17 @@ public function restoreVersion( ]; }//end restoreVersion() - /** - * Cascade-delete every snapshot row for a dashboard. - * - * Designed to be called from the dashboard delete path or, in the - * future, the cascade-events VersionsListener stub. Idempotent. - * - * @param string $dashboardUuid The dashboard UUID. - * - * @return integer The number of rows deleted. - * - * @spec openspec/specs/dashboard-versioning/spec.md + /* + * NO CASCADE-DELETE WRAPPER HERE. + * + * This service used to carry `deleteVersionsForDashboard()`, a literal + * pass-through to `DashboardVersionMapper::deleteByDashboardUuid()`. It + * never had a caller: the live cascade path is `VersionsListener`, which + * is registered against `DashboardDeletedEvent` in `Application.php` + * (REQ-CSC-002) and calls the mapper directly, inside the + * log-and-continue envelope REQ-CSC-006 requires. Routing that listener + * through a second wrapper would add a hop and nothing else. */ - public function deleteVersionsForDashboard(string $dashboardUuid): int { - return $this->versionMapper->deleteByDashboardUuid( - dashboardUuid: $dashboardUuid - ); - }//end deleteVersionsForDashboard() /** * Whether the supplied dashboard is groupfolder-backed diff --git a/openspec/specs/dashboard-versioning/spec.md b/openspec/specs/dashboard-versioning/spec.md index f438b1826..e538f5636 100644 --- a/openspec/specs/dashboard-versioning/spec.md +++ b/openspec/specs/dashboard-versioning/spec.md @@ -372,7 +372,7 @@ The system MUST degrade gracefully when version storage is unavailable, returnin ### Schema and storage - Migration `Version001015Date20260502130000` creates `oc_launchpad_dash_versions` via `DashboardVersionTableBuilder::create()` with columns `id` (BIGINT PK), `dashboard_uuid` (STRING(36)), `version_number` (BIGINT), `snapshot_json` (TEXT — MEDIUMTEXT on MySQL), `created_by` (STRING(64)), `created_at` (DATETIME), `note` (STRING(500) nullable). Indexes: `launchpad_dvers_uuid_num` UNIQUE on `(dashboard_uuid, version_number)` and `launchpad_dvers_uuid_ts` on `(dashboard_uuid, created_at)`. -- Cascade cleanup: `DashboardVersionMapper::deleteByDashboardUuid()` and `DashboardVersionService::deleteVersionsForDashboard()` are wired and ready for the sibling cascade-events listener; the dashboard delete path itself does not yet invoke them (future work alongside `dashboard-cascade-events`). +- Cascade cleanup: `DashboardVersionMapper::deleteByDashboardUuid()` is invoked by `Listener\VersionsListener`, registered against `DashboardDeletedEvent` in `Application.php` per `dashboard-cascade-events` REQ-CSC-002/REQ-CSC-003. There is deliberately no service-layer wrapper for it — the listener owns the log-and-continue envelope REQ-CSC-006 requires, and a pass-through would add a hop with no caller. ### Deferred / future work - REQ-VERS-007 (Activity events): NC `IActivityManager::publish()` integration deferred. `DashboardVersionService::restoreVersion()` already stamps the dashboard `updatedAt` so audit consumers can pick the change up via the existing dashboard mtime; full activity provider registration tracked as a follow-up. diff --git a/tests/Unit/Service/DashboardVersionServiceTest.php b/tests/Unit/Service/DashboardVersionServiceTest.php index b87929ecf..616869b49 100644 --- a/tests/Unit/Service/DashboardVersionServiceTest.php +++ b/tests/Unit/Service/DashboardVersionServiceTest.php @@ -545,25 +545,6 @@ public function testAdminMayRestoreOtherUsersDashboard(): void { $this->assertSame(1, $result['version']->getVersionNumber()); }//end testAdminMayRestoreOtherUsersDashboard() - /** - * Cascade cleanup delegates to the mapper. - * - * @return void - */ - public function testDeleteVersionsForDashboardDelegates(): void { - $this->versionMapper->expects($this->once()) - ->method('deleteByDashboardUuid') - ->with(dashboardUuid: 'd-uuid-1') - ->willReturn(7); - - $this->assertSame( - 7, - $this->service->deleteVersionsForDashboard( - dashboardUuid: 'd-uuid-1' - ) - ); - }//end testDeleteVersionsForDashboardDelegates() - // ========================================================================= // WF1: restoreVersion transaction envelope (wave-12 regression tests) // ========================================================================= diff --git a/tools/spec-annotations-allowlist.txt b/tools/spec-annotations-allowlist.txt index b28995109..1ecac47e5 100644 --- a/tools/spec-annotations-allowlist.txt +++ b/tools/spec-annotations-allowlist.txt @@ -539,7 +539,6 @@ OCA\LaunchPad\Service\DashboardVersionService::listVersions OCA\LaunchPad\Service\DashboardVersionService::fetchSnapshot OCA\LaunchPad\Service\DashboardVersionService::createExplicitSnapshot OCA\LaunchPad\Service\DashboardVersionService::restoreVersion -OCA\LaunchPad\Service\DashboardVersionService::deleteVersionsForDashboard OCA\LaunchPad\Service\DashboardVersionService::isGroupfolderBacked OCA\LaunchPad\Service\DemoShowcasesService::setDataDirForTesting OCA\LaunchPad\Service\DemoShowcasesService::getDataDir