Skip to content

Commit 2bf1a6b

Browse files
rubenvdlindeConduction Release Bot
andauthored
fix(gate-57): drop the caller-less cascade-delete wrapper (#128)
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. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
1 parent 8f1949f commit 2bf1a6b

4 files changed

Lines changed: 13 additions & 14 deletions

File tree

lib/Service/DashboardVersionService.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,18 @@ public function restoreVersion(
527527
];
528528
}//end restoreVersion()
529529

530+
/*
531+
* NO CASCADE-DELETE WRAPPER HERE.
532+
*
533+
* This service used to carry `deleteVersionsForDashboard()`, a literal
534+
* pass-through to `DashboardVersionMapper::deleteByDashboardUuid()`. It
535+
* never had a caller: the live cascade path is `VersionsListener`, which
536+
* is registered against `DashboardDeletedEvent` in `Application.php`
537+
* (REQ-CSC-002) and calls the mapper directly, inside the
538+
* log-and-continue envelope REQ-CSC-006 requires. Routing that listener
539+
* through a second wrapper would add a hop and nothing else.
540+
*/
541+
530542
/**
531543
* Whether the supplied dashboard is groupfolder-backed
532544
* (REQ-VERS-008). Currently always false because the groupfolder

openspec/specs/dashboard-versioning/spec.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ The system MUST degrade gracefully when version storage is unavailable, returnin
372372

373373
### Schema and storage
374374
- 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)`.
375-
- 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`).
375+
- 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.
376376

377377
### Deferred / future work
378378
- 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.

tests/Unit/Service/DashboardVersionServiceTest.php

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -545,18 +545,6 @@ public function testAdminMayRestoreOtherUsersDashboard(): void {
545545
$this->assertSame(1, $result['version']->getVersionNumber());
546546
}//end testAdminMayRestoreOtherUsersDashboard()
547547

548-
/**
549-
* Cascade cleanup delegates to the mapper.
550-
*
551-
* @return void
552-
*/
553-
// The cascade delete is covered where it actually happens, in
554-
// tests/Unit/Listener/VersionsListenerTest.php. DashboardVersionService
555-
// used to carry a deleteVersionsForDashboard() wrapper around the same
556-
// single mapper call; nothing in production ever reached it, because
557-
// VersionsListener — which IS registered on DashboardDeletedEvent — calls
558-
// the mapper directly. This test was the wrapper's only caller anywhere.
559-
560548
// =========================================================================
561549
// WF1: restoreVersion transaction envelope (wave-12 regression tests)
562550
// =========================================================================

tools/spec-annotations-allowlist.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,6 @@ OCA\LaunchPad\Service\DashboardVersionService::listVersions
539539
OCA\LaunchPad\Service\DashboardVersionService::fetchSnapshot
540540
OCA\LaunchPad\Service\DashboardVersionService::createExplicitSnapshot
541541
OCA\LaunchPad\Service\DashboardVersionService::restoreVersion
542-
OCA\LaunchPad\Service\DashboardVersionService::deleteVersionsForDashboard
543542
OCA\LaunchPad\Service\DashboardVersionService::isGroupfolderBacked
544543
OCA\LaunchPad\Service\DemoShowcasesService::setDataDirForTesting
545544
OCA\LaunchPad\Service\DemoShowcasesService::getDataDir

0 commit comments

Comments
 (0)