From 9f03bcadb7f44d6744b9bab35cefee9d4c5a1949 Mon Sep 17 00:00:00 2001 From: Juan Claude Date: Wed, 11 Nov 2026 18:27:48 +0000 Subject: [PATCH 1/2] test(dashboard): pin DashboardService group delete, publication visibility and group-shared pick before the split Assisted-by: ClaudeCode:claude-opus-5-5 --- .../DashboardServiceStepsGoldenTest.php | 377 ++++++++++++++++++ 1 file changed, 377 insertions(+) create mode 100644 tests/Unit/Service/DashboardServiceStepsGoldenTest.php diff --git a/tests/Unit/Service/DashboardServiceStepsGoldenTest.php b/tests/Unit/Service/DashboardServiceStepsGoldenTest.php new file mode 100644 index 000000000..077aa7014 --- /dev/null +++ b/tests/Unit/Service/DashboardServiceStepsGoldenTest.php @@ -0,0 +1,377 @@ + + * @copyright 2026 Conduction b.v. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * SPDX-FileCopyrightText: 2026 LaunchPad Contributors + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace Unit\Service; + +use DateTime; +use Exception; +use InvalidArgumentException; +use OCA\LaunchPad\Db\AdminSettingMapper; +use OCA\LaunchPad\Db\Dashboard; +use OCA\LaunchPad\Db\DashboardMapper; +use OCA\LaunchPad\Db\WidgetPlacementMapper; +use OCA\LaunchPad\Event\DashboardDeletedEvent; +use OCA\LaunchPad\Service\AdminTemplateService; +use OCA\LaunchPad\Service\DashboardBuilders; +use OCA\LaunchPad\Service\DashboardFactory; +use OCA\LaunchPad\Service\DashboardOutlets; +use OCA\LaunchPad\Service\DashboardPlatform; +use OCA\LaunchPad\Service\DashboardResolver; +use OCA\LaunchPad\Service\DashboardService; +use OCA\LaunchPad\Service\DashboardStores; +use OCA\LaunchPad\Service\DashboardStructure; +use OCA\LaunchPad\Service\DashboardTreeService; +use OCA\LaunchPad\Service\FooterService; +use OCA\LaunchPad\Service\PersonalLayerService; +use OCA\LaunchPad\Service\TemplateService; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\IConfig; +use OCP\IDBConnection; +use OCP\IGroupManager; +use OCP\L10N\IFactory; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use ReflectionMethod; + +class DashboardServiceStepsGoldenTest extends TestCase { + private DashboardMapper $dashboardMapper; + private WidgetPlacementMapper $placementMapper; + private IGroupManager $groupManager; + private ?IEventDispatcher $dispatcher; + + protected function setUp(): void { + $this->dashboardMapper = $this->createMock(DashboardMapper::class); + $this->placementMapper = $this->createMock(WidgetPlacementMapper::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->dispatcher = $this->createMock(IEventDispatcher::class); + } + + private function service(): DashboardService { + return new DashboardService( + new DashboardStores($this->dashboardMapper, $this->placementMapper, $this->createMock(AdminSettingMapper::class)), + new DashboardBuilders( + $this->createMock(TemplateService::class), + $this->createMock(DashboardFactory::class), + $this->createMock(DashboardResolver::class), + $this->createMock(AdminTemplateService::class), + ), + new DashboardStructure( + $this->createMock(DashboardTreeService::class), + $this->createMock(PersonalLayerService::class), + null, + $this->createMock(FooterService::class), + ), + new DashboardPlatform($this->groupManager, $this->createMock(IDBConnection::class), $this->createMock(IConfig::class), $this->createMock(IFactory::class)), + new DashboardOutlets($this->createMock(LoggerInterface::class), $this->dispatcher), + ); + } + + private function call(string $method, array $args): mixed { + $reflection = new ReflectionMethod(DashboardService::class, $method); + + return $reflection->invokeArgs($this->service(), $args); + } + + private function groupDashboard(string $groupId, string $uuid = 'g-1', int $isDefault = 0): Dashboard { + $dashboard = new Dashboard(); + $dashboard->setId(70); + $dashboard->setUuid($uuid); + $dashboard->setType(Dashboard::TYPE_GROUP_SHARED); + $dashboard->setGroupId($groupId); + $dashboard->setIsDefault($isDefault); + + return $dashboard; + } + + private function stamp(string $modify): string { + return (new DateTime())->modify($modify)->format('Y-m-d H:i:s'); + } + + public function testNonAdminCannotDeleteAndNothingIsRead(): void { + $this->groupManager->method('isAdmin')->willReturn(false); + $this->dashboardMapper->expects($this->never())->method('findByUuid'); + $this->expectException(Exception::class); + $this->expectExceptionMessage('Forbidden: admin only'); + + $this->service()->deleteGroupShared(actorUserId: 'bob', groupId: 'sales', uuid: 'g-1'); + } + + public function testGroupMismatchIsNotFound(): void { + $this->groupManager->method('isAdmin')->willReturn(true); + $this->dashboardMapper->method('findByUuid')->willReturn($this->groupDashboard('other')); + $this->dashboardMapper->expects($this->never())->method('delete'); + $this->expectException(DoesNotExistException::class); + + $this->service()->deleteGroupShared(actorUserId: 'admin', groupId: 'sales', uuid: 'g-1'); + } + + public function testLastDashboardOfAGroupIsKept(): void { + $this->groupManager->method('isAdmin')->willReturn(true); + $this->dashboardMapper->method('findByUuid')->willReturn($this->groupDashboard('sales')); + $this->dashboardMapper->method('countByGroup')->willReturn(1); + $this->placementMapper->expects($this->never())->method('deleteByDashboardId'); + $this->expectException(Exception::class); + $this->expectExceptionMessage('Cannot delete the only dashboard in the group'); + + $this->service()->deleteGroupShared(actorUserId: 'admin', groupId: 'sales', uuid: 'g-1'); + } + + public function testDefaultGroupDeleteSkipsTheCountAndCascades(): void { + $this->groupManager->method('isAdmin')->willReturn(true); + $dashboard = $this->groupDashboard(Dashboard::DEFAULT_GROUP_ID); + $this->dashboardMapper->method('findByUuid')->willReturn($dashboard); + $this->dashboardMapper->expects($this->never())->method('countByGroup'); + $order = []; + $this->placementMapper->method('deleteByDashboardId')->willReturnCallback(static function (int $dashboardId) use (&$order) { + $order[] = 'placements:' . $dashboardId; + return 0; + }); + $this->dashboardMapper->method('delete')->willReturnCallback(static function ($entity) use (&$order) { + $order[] = 'dashboard'; + return $entity; + }); + $this->dispatcher->expects($this->once())->method('dispatchTyped')->willReturnCallback(static function ($event) use (&$order): void { + $order[] = 'event:' . $event->getDashboardUuid() . ':' . $event->getOwnerUserId() . ':' . $event->getType() . ':' . $event->getDashboardId(); + }); + + $this->service()->deleteGroupShared(actorUserId: 'admin', groupId: Dashboard::DEFAULT_GROUP_ID, uuid: 'g-1'); + + $this->assertSame(['placements:70', 'dashboard', 'event:g-1:admin:group_shared:70'], $order); + } + + public function testDeleteWithSiblingsDispatchesNoEventWithoutDispatcherOrUuid(): void { + $this->groupManager->method('isAdmin')->willReturn(true); + $this->dashboardMapper->method('findByUuid')->willReturn($this->groupDashboard('sales', '')); + $this->dashboardMapper->method('countByGroup')->willReturn(2); + $this->dashboardMapper->expects($this->once())->method('delete'); + $this->dispatcher->expects($this->never())->method('dispatchTyped'); + + $this->service()->deleteGroupShared(actorUserId: 'admin', groupId: 'sales', uuid: ''); + + $this->dispatcher = null; + $this->dashboardMapper = $this->createMock(DashboardMapper::class); + $this->dashboardMapper->method('findByUuid')->willReturn($this->groupDashboard('sales')); + $this->dashboardMapper->method('countByGroup')->willReturn(2); + $this->dashboardMapper->expects($this->once())->method('delete'); + $this->service()->deleteGroupShared(actorUserId: 'admin', groupId: 'sales', uuid: 'g-1'); + } + + /** + * Publication states and their effective status (and stored status after). + * + * @return array + */ + public static function publicationStates(): array { + return [ + 'legacy empty' => ['', null, null, 'published', ''], + 'draft' => ['draft', '-1 day', null, 'draft', 'draft'], + 'scheduled due' => ['scheduled', '-1 minute', null, 'published', 'published'], + 'scheduled future' => ['scheduled', '+1 day', null, 'scheduled', 'scheduled'], + 'scheduled no time' => ['scheduled', '', null, 'scheduled', 'scheduled'], + 'scheduled bad time' => ['scheduled', 'not a time', null, 'scheduled', 'scheduled'], + 'published live' => ['published', null, '+1 day', 'published', 'published'], + 'published down' => ['published', null, '-1 minute', 'draft', 'draft'], + 'published bad down' => ['published', null, 'nope', 'published', 'published'], + 'due then down' => ['scheduled', '-2 days', '-1 day', 'draft', 'draft'], + 'legacy taken down' => ['', null, '-1 day', 'draft', 'draft'], + ]; + } + + /** + * @dataProvider publicationStates + */ + public function testEffectivePublicationStatus(string $stored, ?string $publishAt, ?string $unpublishAt, string $effective, string $after): void { + $dashboard = new Dashboard(); + $dashboard->setPublicationStatus($stored); + $dashboard->setPublishAt($this->timeOrRaw($publishAt)); + $dashboard->setUnpublishAt($this->timeOrRaw($unpublishAt)); + + $this->assertSame($effective, $this->call('effectivePublicationStatus', [$dashboard, new DateTime()])); + $this->assertSame($after, $dashboard->getPublicationStatus()); + } + + private function timeOrRaw(?string $value): ?string { + if ($value === null || $value === '' || preg_match('/^[+-]\d/', $value) !== 1) { + return $value; + } + + return $this->stamp($value); + } + + public function testPublicationFilterKeepsPublishedAndOwnOrAdminDrafts(): void { + $entries = []; + foreach ([['published', 'bob'], ['draft', 'alice'], ['draft', 'bob'], ['scheduled', null], ['', 'bob']] as $i => [$status, $owner]) { + $dashboard = new Dashboard(); + $dashboard->setPublicationStatus($status); + $dashboard->setPublishAt($this->stamp('+1 day')); + $dashboard->setUserId($owner); + $entries[] = ['dashboard' => $dashboard, 'source' => 'n' . $i]; + } + + $asAlice = $this->call('filterByPublicationState', [$entries, 'alice', false]); + $asAdmin = $this->call('filterByPublicationState', [$entries, 'carol', true]); + $asNobody = $this->call('filterByPublicationState', [$entries, '', false]); + + $this->assertSame(['n0', 'n1', 'n4'], array_column($asAlice, 'source')); + $this->assertSame(['n0', 'n1', 'n2', 'n3', 'n4'], array_column($asAdmin, 'source')); + $this->assertSame(['n0', 'n4'], array_column($asNobody, 'source')); + } + + /** + * Scheduled materialisation cases: stored status, publishAt, status after. + * + * @return array + */ + public static function schedules(): array { + return [ + 'not scheduled' => ['draft', '-1 day', 'draft'], + 'no time' => ['scheduled', null, 'scheduled'], + 'empty time' => ['scheduled', '', 'scheduled'], + 'due' => ['scheduled', '-1 minute', 'published'], + 'future' => ['scheduled', '+1 day', 'scheduled'], + 'malformed' => ['scheduled', 'never', 'scheduled'], + ]; + } + + /** + * @dataProvider schedules + */ + public function testMaterialiseDueSchedule(string $stored, ?string $publishAt, string $after): void { + $dashboard = new Dashboard(); + $dashboard->setPublicationStatus($stored); + $dashboard->setPublishAt($this->timeOrRaw($publishAt)); + + $this->call('materialiseDueSchedule', [$dashboard]); + + $this->assertSame($after, $dashboard->getPublicationStatus()); + } + + /** + * Visible sets and the uuid picked for a viewer in group `sales`. + * + * @return array,1:string,2:?string}> + */ + public static function groupSharedPicks(): array { + $primaryDefault = ['group', 'sales', Dashboard::TYPE_GROUP_SHARED, 1, 'p-def']; + $primaryFirst = ['group', 'sales', Dashboard::TYPE_GROUP_SHARED, 0, 'p-first']; + $defaultDefault = ['default', 'default', Dashboard::TYPE_GROUP_SHARED, 1, 'd-def']; + $defaultFirst = ['default', 'default', Dashboard::TYPE_GROUP_SHARED, 0, 'd-first']; + $personal = ['group', 'sales', 'user', 1, 'personal']; + $wrongSource = ['user', 'sales', Dashboard::TYPE_GROUP_SHARED, 1, 'wrong-source']; + $otherGroup = ['group', 'hr', Dashboard::TYPE_GROUP_SHARED, 1, 'other-group']; + return [ + 'primary default first' => [[$defaultFirst, $defaultDefault, $primaryFirst, $primaryDefault], 'sales', 'p-def'], + 'default default next' => [[$defaultFirst, $primaryFirst, $defaultDefault], 'sales', 'd-def'], + 'primary first next' => [[$defaultFirst, $primaryFirst], 'sales', 'p-first'], + 'default first last' => [[$personal, $wrongSource, $otherGroup, $defaultFirst], 'sales', 'd-first'], + 'nothing' => [[$personal, $wrongSource, $otherGroup], 'sales', null], + 'default group only' => [[$primaryDefault, $defaultFirst], 'default', 'd-first'], + ]; + } + + /** + * @dataProvider groupSharedPicks + */ + public function testGroupSharedPickOrder(array $rows, string $groupId, ?string $expected): void { + $visible = []; + foreach ($rows as [$source, $group, $type, $isDefault, $uuid]) { + $dashboard = $this->groupDashboard($group, $uuid, $isDefault); + $dashboard->setType($type); + $visible[] = ['dashboard' => $dashboard, 'source' => $source]; + } + + $result = $this->call('resolveGroupSharedDashboard', [$visible, $groupId]); + + $this->assertSame($expected, $result === null ? null : $result['dashboard']->getUuid()); + } + + /** + * Footer modes: dashboard mode, patch, provided flag, result or error. + * + * @return array,2:bool,3:string}> + */ + public static function footerModes(): array { + return [ + 'provided custom' => ['hidden', ['dashboardFooterMode' => 'custom'], true, 'custom'], + 'kept stored' => ['hidden', ['dashboardFooterMode' => 'custom'], false, 'hidden'], + 'stored empty' => ['', [], false, 'inherit'], + 'provided empty' => ['hidden', ['dashboardFooterMode' => ''], true, 'inherit'], + 'provided null' => ['hidden', ['dashboardFooterMode' => null], true, 'inherit'], + 'unknown' => ['hidden', ['dashboardFooterMode' => 'loud'], true, 'error'], + 'not a string' => ['hidden', ['dashboardFooterMode' => 3], true, 'error'], + 'stored unknown' => ['loud', [], false, 'error'], + ]; + } + + /** + * @dataProvider footerModes + */ + public function testFooterMode(?string $stored, array $data, bool $provided, string $expected): void { + $dashboard = new Dashboard(); + $dashboard->setDashboardFooterMode($stored); + if ($expected === 'error') { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('dashboardFooterMode must be one of: ' . implode(', ', Dashboard::FOOTER_MODES)); + } + + $this->assertSame($expected, $this->call('resolveFooterMode', [$dashboard, $data, $provided])); + } + + public function testPlainFieldUpdates(): void { + $dashboard = new Dashboard(); + $dashboard->setName('Old'); + $dashboard->setDescription('Old description'); + $dashboard->setIcon('old-icon'); + $dashboard->setGridColumns(12); + $this->placementMapper->expects($this->once())->method('updatePositions')->with([['id' => 1, 'x' => 2]]); + + $this->call('applyDashboardUpdates', [$dashboard, [ + 'name' => 'New', + 'description' => null, + 'icon' => 5, + 'gridColumns' => 6, + 'placements' => [['id' => 1, 'x' => 2]], + ]]); + + $this->assertSame(['New', 'Old description', null, 6], [$dashboard->getName(), $dashboard->getDescription(), $dashboard->getIcon(), $dashboard->getGridColumns()]); + $this->assertNotNull($dashboard->getUpdatedAt()); + } + + public function testIconStringAndAbsentKeysAndScalarPlacements(): void { + $dashboard = new Dashboard(); + $dashboard->setName('Keep'); + $dashboard->setIcon('old-icon'); + $this->placementMapper->expects($this->never())->method('updatePositions'); + + $this->call('applyDashboardUpdates', [$dashboard, ['icon' => 'star', 'placements' => 'x', 'name' => null]]); + + $this->assertSame(['Keep', 'star'], [$dashboard->getName(), $dashboard->getIcon()]); + + $this->call('applyDashboardUpdates', [$dashboard, []]); + + $this->assertSame('star', $dashboard->getIcon()); + } +}//end class From 4bf932f43f8dc61a520c15784e528648300806f6 Mon Sep 17 00:00:00 2001 From: Juan Claude Date: Wed, 11 Nov 2026 18:29:32 +0000 Subject: [PATCH 2/2] refactor(dashboard): split DashboardService group delete, publication visibility, group-shared pick and updates into named steps Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Service/DashboardService.php | 414 +++++++++++++++++++++---------- 1 file changed, 284 insertions(+), 130 deletions(-) diff --git a/lib/Service/DashboardService.php b/lib/Service/DashboardService.php index 23414fc88..b4a2c7ff6 100644 --- a/lib/Service/DashboardService.php +++ b/lib/Service/DashboardService.php @@ -1325,31 +1325,80 @@ public function deleteGroupShared( string $groupId, string $uuid, ): void { - if ($this->isAdmin(userId: $actorUserId) === false) { - throw new Exception(message: self::ERR_FORBIDDEN_NOT_ADMIN); - } + $this->assertAdmin(userId: $actorUserId); $dashboard = $this->findGroupDashboard( groupId: $groupId, uuid: $uuid ); - if ($groupId !== Dashboard::DEFAULT_GROUP_ID) { - $count = $this->stores->dashboardMapper->countByGroup( - groupId: $groupId - ); - if ($count <= 1) { - throw new Exception(message: self::ERR_LAST_IN_GROUP); - } + $this->assertNotLastInGroup(groupId: $groupId); + $this->deleteWithPlacements(dashboard: $dashboard); + $this->dispatchGroupSharedDeleted(dashboard: $dashboard, actorUserId: $actorUserId); + }//end deleteGroupShared() + + /** + * Refuse an admin-only mutation for a non-admin actor. + * + * @param string $userId The acting user ID. + * + * @return void + * + * @throws Exception When the actor is not an admin. + */ + private function assertAdmin(string $userId): void { + if ($this->isAdmin(userId: $userId) === false) { + throw new Exception(message: self::ERR_FORBIDDEN_NOT_ADMIN); + } + }//end assertAdmin() + + /** + * Refuse removing the only group-shared dashboard of a group; the + * `default` group is exempt (REQ-DASH-014). + * + * @param string $groupId The group ID. + * + * @return void + * + * @throws Exception When the group has one dashboard or fewer. + */ + private function assertNotLastInGroup(string $groupId): void { + if ($groupId === Dashboard::DEFAULT_GROUP_ID) { + return; } + $count = $this->stores->dashboardMapper->countByGroup( + groupId: $groupId + ); + if ($count <= 1) { + throw new Exception(message: self::ERR_LAST_IN_GROUP); + } + }//end assertNotLastInGroup() + + /** + * Delete a dashboard's placements, then the dashboard itself. + * + * @param Dashboard $dashboard The dashboard to delete. + * + * @return void + */ + private function deleteWithPlacements(Dashboard $dashboard): void { $this->stores->placementMapper->deleteByDashboardId( dashboardId: $dashboard->getId() ); $this->stores->dashboardMapper->delete(entity: $dashboard); + }//end deleteWithPlacements() - // SB1 fix: dispatch DashboardDeletedEvent for cascade cleanup - // (REQ-CSC-001). Actor is the admin who performed the delete. + /** + * SB1 fix: dispatch DashboardDeletedEvent for cascade cleanup + * (REQ-CSC-001). Actor is the admin who performed the delete. + * + * @param Dashboard $dashboard The deleted group-shared dashboard. + * @param string $actorUserId The admin who deleted it. + * + * @return void + */ + private function dispatchGroupSharedDeleted(Dashboard $dashboard, string $actorUserId): void { $deletedUuid = (string)$dashboard->getUuid(); if ($this->outlets->eventDispatcher !== null && $deletedUuid !== '') { $this->outlets->eventDispatcher->dispatchTyped( @@ -1362,7 +1411,7 @@ public function deleteGroupShared( ) ); } - }//end deleteGroupShared() + }//end dispatchGroupSharedDeleted() /** * Promote a single group-shared dashboard to the group's default. @@ -1536,19 +1585,33 @@ private function materialiseDueSchedule(Dashboard $dashboard): void { return; } + if ($this->isDue(dashboard: $dashboard, now: new DateTime()) === true) { + $dashboard->setPublicationStatus(Dashboard::STATUS_PUBLISHED); + } + }//end materialiseDueSchedule() + + /** + * Whether a dashboard's `publishAt` has passed. A missing or + * malformed timestamp is never due (the row stays scheduled). + * + * @param Dashboard $dashboard The dashboard. + * @param DateTime $now The moment to compare against. + * + * @return bool True when the scheduled publication is due. + */ + private function isDue(Dashboard $dashboard, DateTime $now): bool { $publishAt = $dashboard->getPublishAt(); if ($publishAt === null || $publishAt === '') { - return; + return false; } try { - if (new DateTime($publishAt) <= new DateTime()) { - $dashboard->setPublicationStatus(Dashboard::STATUS_PUBLISHED); - } + return new DateTime($publishAt) <= $now; } catch (Exception) { // Malformed timestamp — leave the row as scheduled. + return false; } - }//end materialiseDueSchedule() + }//end isDue() /** * Defensive admin check for the visibility filter. @@ -1597,28 +1660,32 @@ private function filterByPublicationState( ): array { $now = new DateTime(); $filtered = []; - foreach ($entries as $entry) { - $dashboard = $entry['dashboard']; - $status = $this->effectivePublicationStatus(dashboard: $dashboard, now: $now); - - if ($status === Dashboard::STATUS_PUBLISHED) { - $filtered[] = $entry; - continue; - } - - // Status is now draft or (still) scheduled — hide from + $published = $this->effectivePublicationStatus(dashboard: $entry['dashboard'], now: $now) === Dashboard::STATUS_PUBLISHED; + // A draft or (still) scheduled dashboard is hidden from // non-owner non-admin viewers. - $ownerId = $dashboard->getUserId(); - $isOwner = ($ownerId !== null && $ownerId === $actorUserId); - if ($isOwner === true || $actorIsAdmin === true) { + if ($published === true || $this->isOwnedBy(dashboard: $entry['dashboard'], userId: $actorUserId) === true || $actorIsAdmin === true) { $filtered[] = $entry; } - }//end foreach + } return $filtered; }//end filterByPublicationState() + /** + * Whether a dashboard has an owner and it is the given user. + * + * @param Dashboard $dashboard The dashboard. + * @param string $userId The viewing user. + * + * @return bool True for the owner. + */ + private function isOwnedBy(Dashboard $dashboard, string $userId): bool { + $ownerId = $dashboard->getUserId(); + + return $ownerId !== null && $ownerId === $userId; + }//end isOwnedBy() + /** * The publication state a dashboard has at read time, applied to the * entity in memory only: an empty legacy status reads as published @@ -1634,44 +1701,48 @@ private function filterByPublicationState( * @spec openspec/specs/dashboards/spec.md */ private function effectivePublicationStatus(Dashboard $dashboard, DateTime $now): string { + $status = $this->materialisedStatus(dashboard: $dashboard, now: $now); + + // REQ-SCHEDUI-002 (sharing-dashboard-schedule-screen): a published + // dashboard whose take-down time has passed reads as a draft, so + // only its owner and administrators keep seeing it. + if ($status === Dashboard::STATUS_PUBLISHED && $this->isTakenDown(dashboard: $dashboard, now: $now) === true) { + $dashboard->setPublicationStatus(Dashboard::STATUS_DRAFT); + return Dashboard::STATUS_DRAFT; + } + + return $status; + }//end effectivePublicationStatus() + + /** + * The stored publication status, with legacy empty rows read as + * published and due scheduled rows published in place. + * + * @param Dashboard $dashboard The dashboard (mutated when due). + * @param DateTime $now The moment to compare against. + * + * @return string The status before the take-down check. + */ + private function materialisedStatus(Dashboard $dashboard, DateTime $now): string { $status = $dashboard->getPublicationStatus(); // Pre-migration / legacy rows that never set the column // semantically remain visible (REQ-DASH-035): treat an // empty string as `'published'` so backwards compatibility // holds even if an entity is hydrated without the column. if ($status === '') { - $status = Dashboard::STATUS_PUBLISHED; + return Dashboard::STATUS_PUBLISHED; } // REQ-DASH-034: lazy materialisation of due scheduled rows. - if ($status === Dashboard::STATUS_SCHEDULED) { - $publishAt = $dashboard->getPublishAt(); - if ($publishAt !== null && $publishAt !== '') { - try { - $when = new DateTime($publishAt); - if ($when <= $now) { - $dashboard->setPublicationStatus( - Dashboard::STATUS_PUBLISHED - ); - $status = Dashboard::STATUS_PUBLISHED; - } - } catch (Exception) { - // Malformed timestamp — leave as scheduled and - // fall through to the visibility check below. - } - } - } - - // REQ-SCHEDUI-002 (sharing-dashboard-schedule-screen): a published - // dashboard whose take-down time has passed reads as a draft, so - // only its owner and administrators keep seeing it. - if ($status === Dashboard::STATUS_PUBLISHED && $this->isTakenDown(dashboard: $dashboard, now: $now) === true) { - $dashboard->setPublicationStatus(Dashboard::STATUS_DRAFT); - $status = Dashboard::STATUS_DRAFT; + if ($status === Dashboard::STATUS_SCHEDULED && $this->isDue(dashboard: $dashboard, now: $now) === true) { + $dashboard->setPublicationStatus( + Dashboard::STATUS_PUBLISHED + ); + return Dashboard::STATUS_PUBLISHED; } return $status; - }//end effectivePublicationStatus() + }//end materialisedStatus() /** * Resolve the active dashboard for a user using the 7-step precedence @@ -2042,26 +2113,7 @@ private function describeTemplateRung(array $rung): array { * @spec openspec/specs/dashboards/spec.md */ private function resolveGroupSharedDashboard(array $visible, string $groupId): ?array { - $hasPrimaryGroup = ($groupId !== Dashboard::DEFAULT_GROUP_ID); - - $rungs = []; - if ($hasPrimaryGroup === true) { - // Step 2: primary group default. - $rungs[] = [$groupId, Dashboard::SOURCE_GROUP, true]; - } - - // Step 3: default-group default. - $rungs[] = [Dashboard::DEFAULT_GROUP_ID, Dashboard::SOURCE_DEFAULT, true]; - - if ($hasPrimaryGroup === true) { - // Step 4: primary group first. - $rungs[] = [$groupId, Dashboard::SOURCE_GROUP, false]; - } - - // Step 5: default-group first. - $rungs[] = [Dashboard::DEFAULT_GROUP_ID, Dashboard::SOURCE_DEFAULT, false]; - - foreach ($rungs as $rung) { + foreach (self::groupSharedRungs(groupId: $groupId) as $rung) { $result = $this->findFirstGroupSharedWhere( visible: $visible, groupId: $rung[0], @@ -2076,6 +2128,32 @@ private function resolveGroupSharedDashboard(array $visible, string $groupId): ? return null; }//end resolveGroupSharedDashboard() + /** + * The group-shared fallback rungs in order, each `[groupId, source, + * requireDefault]`. Steps 2 and 4 (the primary group's default, then + * its first dashboard) only exist for a viewer whose primary group is + * not the default group; steps 3 and 5 are the default group's. + * + * @param string $groupId The viewer's primary group. + * + * @return list The rungs. + */ + private static function groupSharedRungs(string $groupId): array { + if ($groupId === Dashboard::DEFAULT_GROUP_ID) { + return [ + [Dashboard::DEFAULT_GROUP_ID, Dashboard::SOURCE_DEFAULT, true], + [Dashboard::DEFAULT_GROUP_ID, Dashboard::SOURCE_DEFAULT, false], + ]; + } + + return [ + [$groupId, Dashboard::SOURCE_GROUP, true], + [Dashboard::DEFAULT_GROUP_ID, Dashboard::SOURCE_DEFAULT, true], + [$groupId, Dashboard::SOURCE_GROUP, false], + [Dashboard::DEFAULT_GROUP_ID, Dashboard::SOURCE_DEFAULT, false], + ]; + }//end groupSharedRungs() + /** * Return the first visible entry carrying the given source, in * iteration order. @@ -2961,32 +3039,57 @@ private function findFirstGroupSharedWhere( string $source, bool $requireDefault, ): ?array { - foreach ($visible as $entry) { - if ($entry['source'] !== $source) { - continue; - } - - $dashboard = $entry['dashboard']; - if ($dashboard->getType() !== Dashboard::TYPE_GROUP_SHARED) { - continue; - } - - if ($dashboard->getGroupId() !== $groupId) { - continue; - } - - if ($requireDefault === true - && (int)$dashboard->getIsDefault() !== 1 + foreach (self::entriesFromSource(visible: $visible, source: $source) as $entry) { + if (self::isGroupSharedIn(dashboard: $entry['dashboard'], groupId: $groupId) === true + && self::meetsDefaultRequirement(dashboard: $entry['dashboard'], requireDefault: $requireDefault) === true ) { - continue; + return $entry; } - - return $entry; - }//end foreach + } return null; }//end findFirstGroupSharedWhere() + /** + * The visible entries resolved through the given source, in order. + * + * @param array $visible The visible entries. + * @param string $source The source. + * + * @return array The matching entries. + */ + private static function entriesFromSource(array $visible, string $source): array { + return array_filter( + array: $visible, + callback: static fn (array $entry): bool => $entry['source'] === $source + ); + }//end entriesFromSource() + + /** + * Whether a dashboard is a group-shared dashboard of the given group. + * + * @param Dashboard $dashboard The dashboard. + * @param string $groupId The group ID. + * + * @return bool True when it is. + */ + private static function isGroupSharedIn(Dashboard $dashboard, string $groupId): bool { + return $dashboard->getType() === Dashboard::TYPE_GROUP_SHARED + && $dashboard->getGroupId() === $groupId; + }//end isGroupSharedIn() + + /** + * Whether a dashboard satisfies the rung's default requirement. + * + * @param Dashboard $dashboard The dashboard. + * @param bool $requireDefault Whether only a default dashboard qualifies. + * + * @return bool True when it qualifies. + */ + private static function meetsDefaultRequirement(Dashboard $dashboard, bool $requireDefault): bool { + return $requireDefault === false || (int)$dashboard->getIsDefault() === 1; + }//end meetsDefaultRequirement() + /** * A root slug for this user's auto-provisioned dashboard that no sibling holds. * @@ -3338,6 +3441,43 @@ private function applyDashboardUpdates( Dashboard $dashboard, array $data, ): void { + $this->applyFieldUpdates(dashboard: $dashboard, data: $data); + $this->applyTreeUpdates(dashboard: $dashboard, data: $data); + $this->applyFooterUpdates(dashboard: $dashboard, data: $data); + + $dashboard->setUpdatedAt( + (new DateTime())->format(format: 'Y-m-d H:i:s') + ); + + $this->applyPlacementUpdates(data: $data); + }//end applyDashboardUpdates() + + /** + * Apply a patched placement position list, when the payload has one. + * + * @param array $data The update payload. + * + * @return void + */ + private function applyPlacementUpdates(array $data): void { + if (isset($data['placements']) === true + && is_array($data['placements']) === true + ) { + $this->stores->placementMapper->updatePositions( + updates: $data['placements'] + ); + } + }//end applyPlacementUpdates() + + /** + * Apply the patched name, description, icon and grid columns. + * + * @param Dashboard $dashboard The dashboard being updated. + * @param array $data The update payload. + * + * @return void + */ + private function applyFieldUpdates(Dashboard $dashboard, array $data): void { if (isset($data['name']) === true) { $dashboard->setName($data['name']); } @@ -3346,39 +3486,39 @@ private function applyDashboardUpdates( $dashboard->setDescription($data['description']); } - // Icon may be NULL/empty (use default), a registry key, or a URL. - // The discriminator + lookup live frontend-side in the - // `dashboard-icons` capability — we just store the opaque string. - if (array_key_exists(key: 'icon', array: $data) === true) { - $iconValue = $data['icon']; - $iconToStore = null; - if (is_string($iconValue) === true) { - $iconToStore = $iconValue; - } - - // phpcs:ignore CustomSniffs.Functions.NamedParameters.RequireNamedParameters - $dashboard->setIcon($iconToStore); - } + $this->applyIconUpdate(dashboard: $dashboard, data: $data); if (isset($data['gridColumns']) === true) { $dashboard->setGridColumns($data['gridColumns']); } + }//end applyFieldUpdates() - $this->applyTreeUpdates(dashboard: $dashboard, data: $data); - $this->applyFooterUpdates(dashboard: $dashboard, data: $data); - - $dashboard->setUpdatedAt( - (new DateTime())->format(format: 'Y-m-d H:i:s') - ); + /** + * Apply a patched icon: a string is stored as given, anything else + * clears it. + * + * Icon may be NULL/empty (use default), a registry key, or a URL. + * The discriminator + lookup live frontend-side in the + * `dashboard-icons` capability — we just store the opaque string. + * + * @param Dashboard $dashboard The dashboard being updated. + * @param array $data The update payload. + * + * @return void + */ + private function applyIconUpdate(Dashboard $dashboard, array $data): void { + if (array_key_exists(key: 'icon', array: $data) === false) { + return; + } - if (isset($data['placements']) === true - && is_array($data['placements']) === true - ) { - $this->stores->placementMapper->updatePositions( - updates: $data['placements'] - ); + $iconToStore = null; + if (is_string($data['icon']) === true) { + $iconToStore = $data['icon']; } - }//end applyDashboardUpdates() + + // phpcs:ignore CustomSniffs.Functions.NamedParameters.RequireNamedParameters + $dashboard->setIcon($iconToStore); + }//end applyIconUpdate() /** * Apply per-dashboard footer override updates (REQ-FTR-006). @@ -3463,20 +3603,34 @@ private function resolveFooterMode( $newMode = $data['dashboardFooterMode']; } - if ($newMode === null || $newMode === '') { - $newMode = Dashboard::FOOTER_MODE_INHERIT; + return self::validFooterMode(mode: $newMode); + }//end resolveFooterMode() + + /** + * A footer mode with null / empty read as `inherit`, refused unless it + * is one of the known modes. + * + * @param mixed $mode The candidate mode. + * + * @return string The mode. + * + * @throws InvalidArgumentException When the mode is unknown. + */ + private static function validFooterMode(mixed $mode): string { + if ($mode === null || $mode === '') { + $mode = Dashboard::FOOTER_MODE_INHERIT; } - if (is_string($newMode) === false - || in_array(needle: $newMode, haystack: Dashboard::FOOTER_MODES, strict: true) === false + if (is_string($mode) === false + || in_array(needle: $mode, haystack: Dashboard::FOOTER_MODES, strict: true) === false ) { throw new InvalidArgumentException( message: 'dashboardFooterMode must be one of: ' . implode(separator: ', ', array: Dashboard::FOOTER_MODES) ); } - return $newMode; - }//end resolveFooterMode() + return $mode; + }//end validFooterMode() /** * Resolve the raw custom-footer HTML an update is asking to store.