diff --git a/appinfo/info.xml b/appinfo/info.xml index 2194da7f..ad3c9da5 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -161,6 +161,21 @@ Vrij en open source onder de EUPL-1.2-licentie. OCA\Planninq\Repair\MigrateAppConfigKeys OCA\Planninq\Repair\MigrateUserPreferences OCA\Planninq\Repair\MigrateRegisterSlug + + OCA\Planninq\Repair\RenameTimeEntrySchemaSlug OCA\Planninq\Repair\InitializeSettings OCA\Planninq\Repair\ReconcileDueReminderOverrides @@ -178,6 +193,8 @@ Vrij en open source onder de EUPL-1.2-licentie. happen here or the import below forks a second, empty register. --> OCA\Planninq\Repair\MigrateRegisterSlug + + OCA\Planninq\Repair\RenameTimeEntrySchemaSlug OCA\Planninq\Repair\InitializeSettings diff --git a/lib/Portal/PortalContributionProvider.php b/lib/Portal/PortalContributionProvider.php index e55a8057..88c80166 100644 --- a/lib/Portal/PortalContributionProvider.php +++ b/lib/Portal/PortalContributionProvider.php @@ -183,7 +183,7 @@ private function externalEmployeeContribution(): array { [ 'id' => 'contractorTimeEntries', 'register' => self::REGISTER, - 'schema' => 'timeEntry', + 'schema' => 'plannedTimeEntry', 'scopeField' => 'contractorRef', 'scopeClaim' => 'contractorRef', 'label' => 'My time entries', @@ -219,7 +219,7 @@ private function externalEmployeeContribution(): array { 'type' => 'create', 'label' => 'Log time', 'register' => self::REGISTER, - 'schema' => 'timeEntry', + 'schema' => 'plannedTimeEntry', 'fields' => [ 'task', 'date', diff --git a/lib/Repair/RenameTimeEntrySchemaSlug.php b/lib/Repair/RenameTimeEntrySchemaSlug.php new file mode 100644 index 00000000..25aeb6d3 --- /dev/null +++ b/lib/Repair/RenameTimeEntrySchemaSlug.php @@ -0,0 +1,206 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * @version GIT: + * + * @link https://planninq.nl + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Repair; + +use OCP\DB\Exception; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; +use Psr\Log\LoggerInterface; + +/** + * Renames the time-entry schema slug in place, ahead of the register import. + * + * @spec exclude No canonical spec covers the fleet-wide slug collision + * migration. Pointing this at an existing spec would report conformance to a + * requirement that says nothing about it. + */ +class RenameTimeEntrySchemaSlug implements IRepairStep { + /** + * The slug this step migrates away from. + * + * @var string + */ + private const OLD_SLUG = 'timeEntry'; + + /** + * The namespaced slug the register fragment now declares. + * + * @var string + */ + private const NEW_SLUG = 'plannedTimeEntry'; + + /** + * The owning application, as stored on the schema row. + * + * @var string + */ + private const APPLICATION = 'planninq'; + + /** + * Constructor. + * + * @param IDBConnection $db Database connection. + * @param LoggerInterface $logger Logger. + */ + public function __construct( + private readonly IDBConnection $db, + private readonly LoggerInterface $logger, + ) { + }//end __construct() + + /** + * Human-readable step name. + * + * @return string + * + * @spec exclude No canonical spec covers the fleet-wide slug collision + * migration. Pointing this at an existing spec would report conformance to a + * requirement that says nothing about it. + */ + public function getName(): string { + return 'Namespace the planninq time-entry schema slug'; + }//end getName() + + /** + * Rename the slug, unless doing so would be ambiguous. + * + * @param IOutput $output Repair output. + * + * @return void + * + * @spec exclude No canonical spec covers the fleet-wide slug collision + * migration. Pointing this at an existing spec would report conformance to a + * requirement that says nothing about it. + */ + public function run(IOutput $output): void { + $old = $this->schemaIds(slug: self::OLD_SLUG); + $new = $this->schemaIds(slug: self::NEW_SLUG); + + if ($old === null || $new === null) { + $output->info('RenameTimeEntrySchemaSlug: schema table unreadable; leaving the slug alone.'); + return; + } + + if ($old === []) { + $output->info('RenameTimeEntrySchemaSlug: no time-entry schema on this install; nothing to do.'); + return; + } + + if ($new !== []) { + // Both slugs present: each may own objects, and renaming would collide + // with the English row. Abandoning either set of objects is not a call + // a repair step gets to make without being asked. + $this->logger->warning( + 'RenameTimeEntrySchemaSlug: both slugs exist; refusing to merge them.', + ['old' => $old, 'new' => $new] + ); + $output->warning( + 'RenameTimeEntrySchemaSlug: both `' . self::OLD_SLUG . '` and `' . self::NEW_SLUG + . '` exist; refusing to merge them. Resolve by hand.' + ); + return; + } + + if (count($old) > 1) { + $this->logger->warning( + 'RenameTimeEntrySchemaSlug: duplicate time-entry slugs; refusing to guess.', + ['ids' => $old] + ); + $output->warning('RenameTimeEntrySchemaSlug: duplicate `' . self::OLD_SLUG . '` schemas; refusing to guess.'); + return; + } + + try { + $this->db->executeStatement( + 'UPDATE `*PREFIX*openregister_schemas` SET slug = ? WHERE id = ?', + [self::NEW_SLUG, $old[0]] + ); + } catch (Exception $e) { + // A failure here is safe: the import then creates a new schema rather + // than updating this one, which is the pre-existing behaviour. Loud, + // because the objects on the old schema stop being reachable. + $this->logger->error( + 'RenameTimeEntrySchemaSlug: slug rename failed; the import will create a second schema.', + ['id' => $old[0], 'exception' => $e->getMessage()] + ); + $output->warning('RenameTimeEntrySchemaSlug: slug rename failed; see the log.'); + return; + } + + $output->info( + 'RenameTimeEntrySchemaSlug: schema ' . $old[0] . ' renamed `' + . self::OLD_SLUG . '` -> `' . self::NEW_SLUG . '`; its objects stay attached.' + ); + }//end run() + + /** + * Ids of this application's schemas carrying the given slug. + * + * @param string $slug The schema slug to look for. + * + * @return array|null The ids, or null when the table cannot be read. + */ + private function schemaIds(string $slug): ?array { + try { + $rows = $this->db->executeQuery( + 'SELECT id FROM `*PREFIX*openregister_schemas` WHERE slug = ? AND application = ?', + [$slug, self::APPLICATION] + )->fetchAll(\PDO::FETCH_COLUMN); + + return array_values((array)$rows); + } catch (Exception $e) { + $this->logger->warning( + 'RenameTimeEntrySchemaSlug: could not read the schema table; skipping.', + ['exception' => $e->getMessage()] + ); + return null; + } + }//end schemaIds() +}//end class diff --git a/lib/Settings/planninq_mock_register.json b/lib/Settings/planninq_mock_register.json index 86e46522..1fb2c22f 100644 --- a/lib/Settings/planninq_mock_register.json +++ b/lib/Settings/planninq_mock_register.json @@ -324,7 +324,7 @@ { "@self": { "register": "planninq", - "schema": "timeEntry", + "schema": "plannedTimeEntry", "slug": "timeentry-timeentry-1-1" }, "task": "00000000-0000-4000-8000-000000000000", @@ -337,7 +337,7 @@ { "@self": { "register": "planninq", - "schema": "timeEntry", + "schema": "plannedTimeEntry", "slug": "timeentry-timeentry-2-2" }, "task": "00000000-0000-4000-8000-000000000001", @@ -350,7 +350,7 @@ { "@self": { "register": "planninq", - "schema": "timeEntry", + "schema": "plannedTimeEntry", "slug": "timeentry-timeentry-3-3" }, "task": "00000000-0000-4000-8000-000000000002", diff --git a/lib/Settings/planninq_register.json b/lib/Settings/planninq_register.json index cf1e5765..2d6a918c 100644 --- a/lib/Settings/planninq_register.json +++ b/lib/Settings/planninq_register.json @@ -24,7 +24,7 @@ "project", "projectPhase", "column", - "timeEntry", + "plannedTimeEntry", "label", "dependency" ], @@ -901,8 +901,8 @@ } } }, - "timeEntry": { - "slug": "timeEntry", + "plannedTimeEntry": { + "slug": "plannedTimeEntry", "icon": "TimerOutline", "version": "0.2.0", "title": "Time Entry", @@ -990,6 +990,15 @@ ] }, "properties": { + "timeEntry": { + "title": "Urenboeking", + "title:en": "Time entry", + "type": "string", + "description": "De humaniq `TimeEntry` waarop deze uren geboekt zijn. Een platte uuid en geen `$ref`: humaniq's register is een ander register, en ADR-062 regel 7 geeft een doel buiten dit register geen `$ref`.", + "description:en": "The humaniq `TimeEntry` these hours are booked on. A plain uuid and not a `$ref`: humaniq's register is a different register, and ADR-062 rule 7 gives a cross-register target no `$ref`.", + "format": "uuid", + "facetable": true + }, "task": { "title": "Task", "type": "string", @@ -1533,7 +1542,7 @@ { "@self": { "register": "planninq", - "schema": "timeEntry", + "schema": "plannedTimeEntry", "slug": "te-fix-login-2026-04-01" }, "task": "00000000-0000-4000-d000-000000000001", @@ -1545,7 +1554,7 @@ { "@self": { "register": "planninq", - "schema": "timeEntry", + "schema": "plannedTimeEntry", "slug": "te-fix-login-2026-04-02" }, "task": "00000000-0000-4000-d000-000000000001", @@ -1557,7 +1566,7 @@ { "@self": { "register": "planninq", - "schema": "timeEntry", + "schema": "plannedTimeEntry", "slug": "te-k8s-2026-04-01" }, "task": "00000000-0000-4000-d000-000000000004", diff --git a/src/manifest.json b/src/manifest.json index e0994587..22c5f332 100644 --- a/src/manifest.json +++ b/src/manifest.json @@ -198,10 +198,10 @@ "title": "Time spent", "config": { "widgets": [ - { "id": "time-entries", "title": "Entries", "type": "stat", "content": { "label": "Entries", "icon": "ClockOutline", "route": { "name": "Timesheet" }, "format": { "style": "decimal", "decimals": 0 }, "source": { "register": "planninq", "schema": "timeEntry", "metric": "count" } } }, - { "id": "time-minutes", "title": "Minutes logged", "type": "stat", "content": { "label": "Minutes logged", "icon": "TimerOutline", "route": { "name": "Timesheet" }, "format": { "style": "decimal", "decimals": 0 }, "source": { "register": "planninq", "schema": "timeEntry", "metric": "sum", "field": "duration" } } }, - { "id": "time-by-user", "title": "Per person", "type": "chart", "content": { "chartKind": "bar", "legendPosition": "bottom", "emptyLabel": "Nothing logged yet", "dataSource": { "register": "planninq", "schema": "timeEntry", "aggregate": { "groupBy": "user", "metric": "sum", "sumField": "duration", "topN": 10, "otherBucket": true } } } }, - { "id": "time-recent", "title": "Most recent", "type": "object-table", "content": { "register": "planninq", "schema": "timeEntry", "filter": {}, "sort": { "field": "date", "dir": "desc" }, "limit": 8, "hideHeader": true, "emptyText": "Nothing logged yet", "viewAllRoute": { "name": "Timesheet" }, "viewAllLabel": "Open the timesheet", "columns": [ { "key": "date", "label": "Date" }, { "key": "user", "label": "Who" }, { "key": "duration", "label": "Minutes" }, { "key": "description", "label": "What" } ] } } + { "id": "time-entries", "title": "Entries", "type": "stat", "content": { "label": "Entries", "icon": "ClockOutline", "route": { "name": "Timesheet" }, "format": { "style": "decimal", "decimals": 0 }, "source": { "register": "planninq", "schema": "plannedTimeEntry", "metric": "count" } } }, + { "id": "time-minutes", "title": "Minutes logged", "type": "stat", "content": { "label": "Minutes logged", "icon": "TimerOutline", "route": { "name": "Timesheet" }, "format": { "style": "decimal", "decimals": 0 }, "source": { "register": "planninq", "schema": "plannedTimeEntry", "metric": "sum", "field": "duration" } } }, + { "id": "time-by-user", "title": "Per person", "type": "chart", "content": { "chartKind": "bar", "legendPosition": "bottom", "emptyLabel": "Nothing logged yet", "dataSource": { "register": "planninq", "schema": "plannedTimeEntry", "aggregate": { "groupBy": "user", "metric": "sum", "sumField": "duration", "topN": 10, "otherBucket": true } } } }, + { "id": "time-recent", "title": "Most recent", "type": "object-table", "content": { "register": "planninq", "schema": "plannedTimeEntry", "filter": {}, "sort": { "field": "date", "dir": "desc" }, "limit": 8, "hideHeader": true, "emptyText": "Nothing logged yet", "viewAllRoute": { "name": "Timesheet" }, "viewAllLabel": "Open the timesheet", "columns": [ { "key": "date", "label": "Date" }, { "key": "user", "label": "Who" }, { "key": "duration", "label": "Minutes" }, { "key": "description", "label": "What" } ] } } ], "layout": [ { "id": "1", "widgetId": "time-entries", "gridX": 0, "gridY": 0, "gridWidth": 6, "gridHeight": 2, "showTitle": false }, @@ -296,7 +296,7 @@ }, { "registerSlug": "planninq", - "schemaSlug": "timeEntry", + "schemaSlug": "plannedTimeEntry", "urlTemplate": "/apps/planninq/projects/{project}?task={task}", "displayName": "Time entry" } diff --git a/src/store/projects.js b/src/store/projects.js index e0f1c30b..507d7dfa 100644 --- a/src/store/projects.js +++ b/src/store/projects.js @@ -27,7 +27,7 @@ const REGISTER = 'planninq' const PROJECT_SCHEMA = 'project' const COLUMN_SCHEMA = 'column' const TASK_SCHEMA = 'task' -const TIME_ENTRY_SCHEMA = 'timeEntry' +const TIME_ENTRY_SCHEMA = 'plannedTimeEntry' /** * Largest page OpenRegister will return. Asking for more is silently capped. diff --git a/src/store/timeEntries.js b/src/store/timeEntries.js index 5d788c0a..fbf22c86 100644 --- a/src/store/timeEntries.js +++ b/src/store/timeEntries.js @@ -23,7 +23,7 @@ import { useObjectStore } from './objectStore.js' // the register ROW: OR resolves a register by slug and by nothing else, so the // literal and the row move in the same release or neither resolves. const REGISTER = 'planninq' -const TIME_ENTRY_SCHEMA = 'timeEntry' +const TIME_ENTRY_SCHEMA = 'plannedTimeEntry' export const useTimeEntriesStore = defineStore('timeEntries', { state: () => ({ diff --git a/tests/unit/Portal/PortalContributionProviderTest.php b/tests/unit/Portal/PortalContributionProviderTest.php index ed15bd36..a84ef25c 100644 --- a/tests/unit/Portal/PortalContributionProviderTest.php +++ b/tests/unit/Portal/PortalContributionProviderTest.php @@ -208,7 +208,7 @@ public function testCollectionsAreContractorScoped(): void { ['contractorTasks', 'contractorTimeEntries', 'contractorProjects'], array_column($collections, 'id') ); - $this->assertSame(['task', 'timeEntry', 'project'], array_column($collections, 'schema')); + $this->assertSame(['task', 'plannedTimeEntry', 'project'], array_column($collections, 'schema')); $this->assertSame( ['contractorRef', 'contractorRef', 'contractorRefs'], array_column($collections, 'scopeField') @@ -239,7 +239,7 @@ public function testLogTimeActionWhitelist(): void { $this->assertSame('logTime', $action['id']); $this->assertSame('create', $action['type']); - $this->assertSame('timeEntry', $action['schema']); + $this->assertSame('plannedTimeEntry', $action['schema']); $this->assertSame(['task', 'date', 'duration', 'description'], $action['fields']); $this->assertNotContains('user', $action['fields'], 'the logging NC user is server-authoritative'); $this->assertNotContains('contractorRef', $action['fields'], 'the scope key is derived from the claim, not submitted'); @@ -256,7 +256,7 @@ public function testLogTimeActionWhitelist(): void { public function testRegisterCarriesContractorRefProperties(): void { $schemas = $this->register['components']['schemas']; - foreach (['task', 'timeEntry'] as $schemaName) { + foreach (['task', 'plannedTimeEntry'] as $schemaName) { $prop = $schemas[$schemaName]['properties']['contractorRef']; $this->assertSame('string', $prop['type'], "{$schemaName}.contractorRef must be a string"); $this->assertSame('uuid', $prop['format'], "{$schemaName}.contractorRef must be format uuid"); @@ -275,7 +275,7 @@ public function testRegisterCarriesContractorRefProperties(): void { // The NC-uid props they sit ALONGSIDE are kept, not replaced. $this->assertArrayHasKey('assignedTo', $schemas['task']['properties']); - $this->assertArrayHasKey('user', $schemas['timeEntry']['properties']); + $this->assertArrayHasKey('user', $schemas['plannedTimeEntry']['properties']); $this->assertArrayHasKey('members', $schemas['project']['properties']); }//end testRegisterCarriesContractorRefProperties() diff --git a/tests/unit/Repair/RenameTimeEntrySchemaSlugTest.php b/tests/unit/Repair/RenameTimeEntrySchemaSlugTest.php new file mode 100644 index 00000000..a87f0036 --- /dev/null +++ b/tests/unit/Repair/RenameTimeEntrySchemaSlugTest.php @@ -0,0 +1,150 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://planninq.nl + */ + +declare(strict_types=1); + +namespace OCA\Planninq\Tests\Unit\Repair; + +use OCA\Planninq\Repair\RenameTimeEntrySchemaSlug; +use OCP\DB\IResult; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; + +/** + * Guards the time-entry schema slug migration. + * + * The step exists because OpenRegister matches a schema by (application, slug) and + * CREATES a new one when that misses — so a slug rename in the shipped register + * fragment orphans the old schema and every object on it, without erroring. These + * tests pin the one case that must write and the two that must refuse. + */ +final class RenameTimeEntrySchemaSlugTest extends TestCase { + /** + * Mocked database connection. + * + * @var IDBConnection + */ + private $db; + + /** + * The step under test. + * + * @var RenameTimeEntrySchemaSlug + */ + private RenameTimeEntrySchemaSlug $step; + + /** + * Build the step with mocked collaborators. + * + * @return void + */ + protected function setUp(): void { + parent::setUp(); + $this->db = $this->createMock(IDBConnection::class); + $this->step = new RenameTimeEntrySchemaSlug($this->db, $this->createMock(LoggerInterface::class)); + }//end setUp() + + /** + * Queue the two slug lookups the step performs, in order. + * + * @param array $old Ids returned for the old slug. + * @param array $new Ids returned for the namespaced slug. + * + * @return void + */ + private function queueLookups(array $old, array $new): void { + $oldResult = $this->createMock(IResult::class); + $oldResult->method('fetchAll')->willReturn($old); + $newResult = $this->createMock(IResult::class); + $newResult->method('fetchAll')->willReturn($new); + $this->db->method('executeQuery')->willReturnOnConsecutiveCalls($oldResult, $newResult); + }//end queueLookups() + + /** + * The old slug alone is renamed in place, keeping the schema id. + * + * Keeping the id is the whole point: the shard table is named after it, so a new + * schema would leave every existing planned entry behind a slug nothing reads. + * + * @return void + */ + public function testRenamesTheOldSlugInPlace(): void { + $this->queueLookups(old: [495], new: []); + + $statements = []; + $this->db->method('executeStatement')->willReturnCallback( + function (string $sql, array $params = []) use (&$statements): int { + $statements[] = [$sql, $params]; + return 1; + } + ); + + $this->step->run($this->createMock(IOutput::class)); + + $this->assertCount(1, $statements, 'exactly one row may be rewritten'); + $this->assertStringContainsString('openregister_schemas', $statements[0][0]); + $this->assertStringContainsString('SET slug', $statements[0][0]); + $this->assertSame(['plannedTimeEntry', 495], $statements[0][1]); + }//end testRenamesTheOldSlugInPlace() + + /** + * An install already on the namespaced slug is left alone. + * + * @return void + */ + public function testIsANoOpWhenTheOldSlugIsAbsent(): void { + $this->queueLookups(old: [], new: [495]); + $this->db->expects($this->never())->method('executeStatement'); + + $this->step->run($this->createMock(IOutput::class)); + }//end testIsANoOpWhenTheOldSlugIsAbsent() + + /** + * Both slugs present is a refusal, not a merge. + * + * Each schema may own objects. Renaming one onto the other would decide which set + * to abandon, so the step declines and says so rather than picking silently. + * + * @return void + */ + public function testRefusesWhenBothSlugsExist(): void { + $this->queueLookups(old: [495], new: [512]); + $this->db->expects($this->never())->method('executeStatement'); + + $output = $this->createMock(IOutput::class); + $output->expects($this->once())->method('warning'); + + $this->step->run($output); + }//end testRefusesWhenBothSlugsExist() + + /** + * Duplicate old slugs are a refusal too — the step must not guess. + * + * @return void + */ + public function testRefusesWhenTheOldSlugIsDuplicated(): void { + $this->queueLookups(old: [495, 496], new: []); + $this->db->expects($this->never())->method('executeStatement'); + + $output = $this->createMock(IOutput::class); + $output->expects($this->once())->method('warning'); + + $this->step->run($output); + }//end testRefusesWhenTheOldSlugIsDuplicated() +}//end class diff --git a/tests/unit/Settings/PlanninqRegisterSchemaTest.php b/tests/unit/Settings/PlanninqRegisterSchemaTest.php index cd3320f4..d03ccef3 100644 --- a/tests/unit/Settings/PlanninqRegisterSchemaTest.php +++ b/tests/unit/Settings/PlanninqRegisterSchemaTest.php @@ -140,7 +140,7 @@ public function testProjectSchemaHasOwnerField(): void { * @return void */ public function testAllDataSchemasHaveAuthorizationBlocks(): void { - $schemasRequiringAuth = ['project', 'task', 'column', 'timeEntry']; + $schemasRequiringAuth = ['project', 'task', 'column', 'plannedTimeEntry']; foreach ($schemasRequiringAuth as $schemaSlug) { self::assertArrayHasKey( @@ -260,7 +260,7 @@ public function testProjectDeleteAuthorizationEnforcesOwner(): void { * @return void */ public function testTimeEntryAuthorizationRestrictsWriteToOwningUser(): void { - $auth = $this->register['components']['schemas']['timeEntry']['authorization']; + $auth = $this->register['components']['schemas']['plannedTimeEntry']['authorization']; foreach (['update', 'delete'] as $action) { $hasUserRule = false; @@ -368,7 +368,7 @@ public function testDueSoonRecipientFieldExistsOnSchema(): void { * @spec openspec/changes/task-dependencies/specs/register-schemas/spec.md */ public function testRegisterDeclaresExactlySevenSchemas(): void { - $expected = ['task', 'project', 'projectPhase', 'column', 'timeEntry', 'label', 'dependency']; + $expected = ['task', 'project', 'projectPhase', 'column', 'plannedTimeEntry', 'label', 'dependency']; $listed = $this->register['components']['registers']['planninq']['schemas']; sort($listed);