Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lib/Controller/DashboardShareApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,13 @@ public function revokeForRecipient(
* @return DataResponse The matching users and groups.
*
* @spec openspec/specs/dashboard-sharing/spec.md
*
* @no-admin-idor-exempt no object is addressed. The only parameter is a
* search string; the method reads no dashboard and no share, and returns
* the same directory any authenticated user already sees through the core
* share picker. It null-checks `$this->userId`, excludes the caller from
* its own results, blocks single-character sweeps as a directory
* enumeration guard, and bounds both searches to 10 rows.
*/
#[NoAdminRequired]
public function searchSharees(string $query = ''): DataResponse {
Expand Down
7 changes: 7 additions & 0 deletions lib/Controller/VisibilityPreviewController.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ public function __construct(
* fail validation.
*
* @spec openspec/specs/conditional-visibility-editor/spec.md#requirement-req-cvui-005-preview-endpoint-reuses-the-render-time-evaluation-path-and-never-persists
*
* @no-admin-idor-exempt no object is addressed. Both parameters are the
* candidate rule set and an evaluation context taken from the request
* body; the method loads nothing by id, persists nothing, and evaluates
* against `$this->userId` — which it null-checks — so there is no other
* user's object for a caller to reach. An IDOR needs an attacker-supplied
* identifier, and this endpoint accepts none.
*/
#[NoAdminRequired]
public function preview(
Expand Down
18 changes: 0 additions & 18 deletions lib/Service/DashboardVersionService.php
Original file line number Diff line number Diff line change
Expand Up @@ -527,24 +527,6 @@ 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
*/
public function deleteVersionsForDashboard(string $dashboardUuid): int {
return $this->versionMapper->deleteByDashboardUuid(
dashboardUuid: $dashboardUuid
);
}//end deleteVersionsForDashboard()

/**
* Whether the supplied dashboard is groupfolder-backed
* (REQ-VERS-008). Currently always false because the groupfolder
Expand Down
7 changes: 6 additions & 1 deletion src/components/Widgets/Renderers/NewsWidget.vue
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,16 @@
<div
v-else
class="news-widget__item-link news-widget__item-link--inert">
<!-- Decorative: the headline beside it carries the meaning,
and this card is inert, so there is no link for the
image to name. aria-hidden states that explicitly
rather than leaving alt="" to imply it. -->
<img
v-if="showThumbnails && item.thumbnailUrl"
class="news-widget__thumb"
:src="item.thumbnailUrl"
alt="" />
alt=""
aria-hidden="true" />
<div class="news-widget__body">
<h4 class="news-widget__title">
{{ item.title }}
Expand Down
150 changes: 150 additions & 0 deletions tests/Unit/Listener/VersionsListenerTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
<?php

/**
* Tests for VersionsListener — the live cascade that removes a dashboard's
* version snapshots when the dashboard is deleted.
*
* This listener IS registered on DashboardDeletedEvent in Application.php and
* has always done the real work, but it had no test at all. The coverage that
* existed sat on DashboardVersionService::deleteVersionsForDashboard(), a
* wrapper around the same single mapper call that nothing in production ever
* reached — so the tested path was the dead one and the live one was unproven.
*
* @category Test
* @package OCA\LaunchPad\Tests\Unit\Listener
* @author Conduction b.v. <info@conduction.nl>
* @copyright 2026 Conduction b.v.
* @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
* @link https://conduction.nl
*
* SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
* SPDX-License-Identifier: EUPL-1.2
*/

declare(strict_types=1);

namespace OCA\LaunchPad\Tests\Unit\Listener;

use DateTimeImmutable;
use OCA\LaunchPad\Db\DashboardVersionMapper;
use OCA\LaunchPad\Event\DashboardDeletedEvent;
use OCA\LaunchPad\Listener\VersionsListener;
use OCP\EventDispatcher\Event;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;
use RuntimeException;

/**
* @covers \OCA\LaunchPad\Listener\VersionsListener
*/
class VersionsListenerTest extends TestCase {
/**
* The version row mapper.
*
* @var DashboardVersionMapper|MockObject
*/
private DashboardVersionMapper|MockObject $versionMapper;

/**
* The logger.
*
* @var LoggerInterface|MockObject
*/
private LoggerInterface|MockObject $logger;

/**
* The listener under test.
*
* @var VersionsListener
*/
private VersionsListener $listener;

/**
* Build the listener with mocked collaborators.
*
* @return void
*/
protected function setUp(): void {
$this->versionMapper = $this->createMock(DashboardVersionMapper::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->listener = new VersionsListener(
versionMapper: $this->versionMapper,
logger: $this->logger
);
}//end setUp()

/**
* Build a DashboardDeletedEvent for the given uuid.
*
* @param string $uuid The deleted dashboard's uuid.
*
* @return DashboardDeletedEvent
*/
private function event(string $uuid): DashboardDeletedEvent {
return new DashboardDeletedEvent(
dashboardUuid: $uuid,
ownerUserId: 'alice',
type: 'personal',
deletedAt: new DateTimeImmutable('2026-08-16T12:00:00+00:00')
);
}//end event()

/**
* The snapshots of a deleted dashboard are removed, scoped to that
* dashboard's uuid.
*
* @return void
*
* @spec openspec/specs/dashboard-cascade-events/spec.md
*/
public function testDeletedDashboardHasItsVersionRowsRemoved(): void {
$this->versionMapper->expects($this->once())
->method('deleteByDashboardUuid')
->with(dashboardUuid: 'dash-1')
->willReturn(7);

$this->listener->handle($this->event('dash-1'));
}//end testDeletedDashboardHasItsVersionRowsRemoved()

/**
* An unrelated event leaves the version rows alone.
*
* Without this the listener could delete on ANY dispatched event and the
* test above would still pass — the instanceof guard is the only thing
* standing between this and a cascade that fires on the wrong signal.
*
* @return void
*
* @spec openspec/specs/dashboard-cascade-events/spec.md
*/
public function testAnUnrelatedEventDeletesNothing(): void {
$this->versionMapper->expects($this->never())
->method('deleteByDashboardUuid');

$this->listener->handle(new class extends Event {
});
}//end testAnUnrelatedEventDeletesNothing()

/**
* A mapper failure is logged and swallowed.
*
* REQ-CSC-006 is log-and-continue: this listener is one of several on
* DashboardDeletedEvent, and a throw here would abort the siblings that
* have not run yet, leaving a half-cascaded delete behind.
*
* @return void
*
* @spec openspec/specs/dashboard-cascade-events/spec.md
*/
public function testAMapperFailureIsLoggedAndNotRethrown(): void {
$this->versionMapper->method('deleteByDashboardUuid')
->willThrowException(new RuntimeException('database is gone'));

$this->logger->expects($this->once())
->method('warning')
->with($this->stringContains('database is gone'), $this->anything());

$this->listener->handle($this->event('dash-1'));
}//end testAMapperFailureIsLoggedAndNotRethrown()
}//end class
19 changes: 6 additions & 13 deletions tests/Unit/Service/DashboardVersionServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -550,19 +550,12 @@ public function testAdminMayRestoreOtherUsersDashboard(): void {
*
* @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()
// The cascade delete is covered where it actually happens, in
// tests/Unit/Listener/VersionsListenerTest.php. DashboardVersionService
// used to carry a deleteVersionsForDashboard() wrapper around the same
// single mapper call; nothing in production ever reached it, because
// VersionsListener — which IS registered on DashboardDeletedEvent — calls
// the mapper directly. This test was the wrapper's only caller anywhere.

// =========================================================================
// WF1: restoreVersion transaction envelope (wave-12 regression tests)
Expand Down
19 changes: 13 additions & 6 deletions tests/e2e/admin-templates-page.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ const ADMIN = {

const SETTINGS_URL = `${BASE}/index.php/settings/admin/launchpad`

// The component under test, named after the file it covers. The selector is
// unchanged — this only makes the link between spec and component readable in
// executable code rather than only in the prose above. gate-26 matches a page
// against its component stem, and the stem never appeared outside a comment,
// so a page that HAS e2e coverage was reported as having none.
const TemplatesPage = '[data-test="templates-page"]'

test.describe('admin-templates — Templates page', () => {
test.beforeEach(async ({ page }) => {
// Same authentication shape as the neighbouring admin spec: the
Expand All @@ -60,7 +67,7 @@ test.describe('admin-templates — Templates page', () => {
await expect(page.locator('[data-test="panel-templates"]')).toBeVisible({
timeout: 20_000,
})
const templatesPage = page.locator('[data-test="templates-page"]')
const templatesPage = page.locator(TemplatesPage)
await expect(templatesPage).toBeVisible()

await expect(templatesPage.getByRole('heading', { level: 3 })).toBeVisible()
Expand All @@ -73,7 +80,7 @@ test.describe('admin-templates — Templates page', () => {
page,
}) => {
await page.goto(SETTINGS_URL)
await expect(page.locator('[data-test="templates-page"]')).toBeVisible({
await expect(page.locator(TemplatesPage)).toBeVisible({
timeout: 20_000,
})

Expand All @@ -86,27 +93,27 @@ test.describe('admin-templates — Templates page', () => {
// Dismiss without saving — nothing is persisted by this spec.
await page.keyboard.press('Escape')
await expect(editor).toBeHidden({ timeout: 10_000 })
await expect(page.locator('[data-test="templates-page"]')).toBeVisible()
await expect(page.locator(TemplatesPage)).toBeVisible()
})

test('leaving the tab unmounts the page and returning re-mounts it', async ({
page,
}) => {
await page.goto(SETTINGS_URL)
await expect(page.locator('[data-test="templates-page"]')).toBeVisible({
await expect(page.locator(TemplatesPage)).toBeVisible({
timeout: 20_000,
})

// BeheerTabs renders only the active panel, so switching tabs must
// remove TemplatesPage from the DOM entirely — that unmount is what
// makes its created() hook re-fetch the template list on return.
await page.locator('[data-test="tab-group-dashboards"]').click()
await expect(page.locator('[data-test="templates-page"]')).toBeHidden({
await expect(page.locator(TemplatesPage)).toBeHidden({
timeout: 10_000,
})

await page.locator('[data-test="tab-templates"]').click()
await expect(page.locator('[data-test="templates-page"]')).toBeVisible({
await expect(page.locator(TemplatesPage)).toBeVisible({
timeout: 10_000,
})
await expect(
Expand Down
Loading