From 81736ab75cfe13f7bfe52c3b853d273e06ab6245 Mon Sep 17 00:00:00 2001 From: Daniel Gorges Date: Tue, 27 Oct 2026 07:21:46 +0100 Subject: [PATCH 1/7] [BUGFIX] Version prompt fragments in workspaces --- CHANGELOG.md | 7 +++++++ Configuration/TCA/tx_aim_page_prompt_fragment.php | 1 - Configuration/TCA/tx_aim_prompt_fragment.php | 1 - 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index adb209e..fdbbc6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +- **Bugfix "Workspace versioning of prompt fragments"**: `tx_aim_prompt_fragment` and +`tx_aim_page_prompt_fragment` set `versioningWS_alwaysAllowLiveEdit` next to `versioningWS`, so an edit made in +a workspace went straight to live, unlike the workspace behaviour `Documentation/ToneOfVoice.md` describes. +TYPO3 14.3.8 removes the option from workspace aware tables and logs a TCA migration deprecation for both +tables, which made test suites with `failOnDeprecation` fail as soon as aim was loaded. The option is gone; on +TYPO3 12.4 and 13.4 a fragment or an assignment edited in a workspace is now versioned like on 14.3. + - **Bugfix "Smart routing with dated model names"**: Smart routing never downgraded a request to OpenAI or Anthropic. The request log stores the model name a provider reports, a dated snapshot (`gpt-4o-mini-2024-07-18`) or the target of an alias (`claude-sonnet-4-20250514` for `claude-sonnet-4-0`), diff --git a/Configuration/TCA/tx_aim_page_prompt_fragment.php b/Configuration/TCA/tx_aim_page_prompt_fragment.php index 9ee1344..b7f8c5f 100644 --- a/Configuration/TCA/tx_aim_page_prompt_fragment.php +++ b/Configuration/TCA/tx_aim_page_prompt_fragment.php @@ -17,7 +17,6 @@ 'disabled' => 'hidden', ], 'versioningWS' => true, - 'versioningWS_alwaysAllowLiveEdit' => true, 'security' => [ 'ignorePageTypeRestriction' => true, ], diff --git a/Configuration/TCA/tx_aim_prompt_fragment.php b/Configuration/TCA/tx_aim_prompt_fragment.php index 45c889d..44ad269 100644 --- a/Configuration/TCA/tx_aim_prompt_fragment.php +++ b/Configuration/TCA/tx_aim_prompt_fragment.php @@ -19,7 +19,6 @@ 'disabled' => 'hidden', ], 'versioningWS' => true, - 'versioningWS_alwaysAllowLiveEdit' => true, 'security' => [ 'ignorePageTypeRestriction' => true, ], From db67a880ff2e63263d45e234b90e75a2214e7d4c Mon Sep 17 00:00:00 2001 From: Oliver Bartsch Date: Tue, 27 Oct 2026 09:08:54 +0100 Subject: [PATCH 2/7] [BUGFIX] Read prompt fragments workspace-aware With versioningWS_alwaysAllowLiveEdit gone, workspace versions of tx_aim_prompt_fragment and tx_aim_page_prompt_fragment exist on every supported TYPO3 version. PagePromptResolver now reads live records plus the ones created in the current workspace and drops hidden rows only after the workspace overlay, so an assignment activated, deactivated or created in a workspace resolves the way that workspace sees it. The assignment listing, the auto-detected fragment lookup, the assignment counts and the aim:calibrateVoice upsert read live records only, so they no longer return workspace versions or update one directly. Signed-off-by: Oliver Bartsch --- CHANGELOG.md | 5 +- .../PagePromptFragmentRepository.php | 9 +++ Classes/Prompt/PagePromptResolver.php | 67 +++++++++++----- Documentation/ToneOfVoice.md | 2 +- .../PagePromptFragmentRepositoryTest.php | 74 ++++++++++++++++++ .../Prompt/PagePromptResolverTest.php | 77 +++++++++++++++++++ ...aPromptFragmentWorkspaceVersioningTest.php | 41 ++++++++++ 7 files changed, 253 insertions(+), 22 deletions(-) create mode 100644 Tests/Unit/Configuration/TcaPromptFragmentWorkspaceVersioningTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index fdbbc6d..8e23c0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,10 @@ a workspace went straight to live, unlike the workspace behaviour `Documentation/ToneOfVoice.md` describes. TYPO3 14.3.8 removes the option from workspace aware tables and logs a TCA migration deprecation for both tables, which made test suites with `failOnDeprecation` fail as soon as aim was loaded. The option is gone; on -TYPO3 12.4 and 13.4 a fragment or an assignment edited in a workspace is now versioned like on 14.3. +TYPO3 12.4, 13.4 and 14.0 to 14.3.7 a fragment or an assignment edited in a workspace is now versioned like on +14.3.8 and later. The tone of voice resolved within a workspace now also follows an assignment activated, +deactivated or created in that workspace, and the Prompt Management listings, the assignment counts and +`aim:calibrateVoice` read live records only instead of mixing in workspace versions. - **Bugfix "Smart routing with dated model names"**: Smart routing never downgraded a request to OpenAI or Anthropic. The request log stores the model name a provider reports, a dated snapshot diff --git a/Classes/Domain/Repository/PagePromptFragmentRepository.php b/Classes/Domain/Repository/PagePromptFragmentRepository.php index 48f1ca0..17dc4f9 100644 --- a/Classes/Domain/Repository/PagePromptFragmentRepository.php +++ b/Classes/Domain/Repository/PagePromptFragmentRepository.php @@ -197,6 +197,7 @@ public function enrichWithFragmentSummary(array $pageRows): array public function findAssignmentsForPage(int $pageId): array { $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); + $qb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); // The module reviews and activates hidden assignments, so it has to see them, unlike PagePromptResolver. $qb->getRestrictions()->removeByType(HiddenRestriction::class); return $qb->select( @@ -228,6 +229,7 @@ public function findAssignmentsForPage(int $pageId): array public function findAutoDetectedFragmentForRootPage(int $rootPageId): ?array { $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); + $qb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); // A freshly calibrated fragment is hidden, and this is what shows it. $qb->getRestrictions()->removeByType(HiddenRestriction::class); $row = $qb->select( @@ -264,11 +266,13 @@ public function findAutoDetectedFragmentForRootPage(int $rootPageId): ?array public function countAllAssignments(): array { $totalQb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); + $totalQb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); $totalQb->getRestrictions()->removeByType(HiddenRestriction::class); $total = (int)$totalQb->count('uid')->from(self::ASSIGNMENT_TABLE) ->executeQuery()->fetchOne(); $inheritedQb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); + $inheritedQb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); $inheritedQb->getRestrictions()->removeByType(HiddenRestriction::class); $inherited = (int)$inheritedQb->count('uid')->from(self::ASSIGNMENT_TABLE) ->where($inheritedQb->expr()->eq('inherit_to_subpages', $inheritedQb->createNamedParameter(1, Connection::PARAM_INT))) @@ -312,6 +316,9 @@ public function upsertAutoDetectedFragment( bool $inheritToSubpages = true, ): int { $assignmentQb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); + // Live rows only: this write bypasses DataHandler, so it must never + // pick up and overwrite a workspace version of the assignment. + $assignmentQb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); $assignmentQb->getRestrictions()->removeByType(HiddenRestriction::class); $existingAssignment = $assignmentQb ->select('uid', 'fragment') @@ -320,6 +327,8 @@ public function upsertAutoDetectedFragment( $assignmentQb->expr()->eq('parent_page', $assignmentQb->createNamedParameter($pageId, Connection::PARAM_INT)), $assignmentQb->expr()->eq('auto_generated', $assignmentQb->createNamedParameter(1, Connection::PARAM_INT)), ) + ->orderBy('uid') + ->setMaxResults(1) ->executeQuery() ->fetchAssociative(); diff --git a/Classes/Prompt/PagePromptResolver.php b/Classes/Prompt/PagePromptResolver.php index 3ceda64..71959fa 100644 --- a/Classes/Prompt/PagePromptResolver.php +++ b/Classes/Prompt/PagePromptResolver.php @@ -17,6 +17,7 @@ use TYPO3\CMS\Core\Context\Context; use TYPO3\CMS\Core\Database\Connection; use TYPO3\CMS\Core\Database\ConnectionPool; +use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction; use TYPO3\CMS\Core\Database\Query\Restriction\WorkspaceRestriction; use TYPO3\CMS\Core\Domain\Repository\PageRepository; use TYPO3\CMS\Core\Utility\GeneralUtility; @@ -55,8 +56,9 @@ * boundaries; that's an established, technical-audience convention * (integrators can already use the `>` clear operator). * - * DB fragments respect workspace overlays for edits to either the - * assignment or the library fragment itself. Not translatable: neither + * DB fragments respect workspaces for either the assignment or the library + * fragment itself: an edit, an activation, a deactivation, a deletion and + * a record created within the current workspace. Not translatable: neither * TCA has language/transOrig configuration, since nothing in the * resolution pipeline is language-aware yet. * @@ -98,8 +100,7 @@ public function resolve(int $pageId, PromptFragmentScope $scope): ?string // can legitimately resolve differently per workspace; caching by // pageId+scope alone would let a live-workspace resolution leak into // a later draft-workspace one within the same instance's lifetime. - $workspaceId = (int)$this->context->getPropertyFromAspect('workspace', 'id'); - $cacheKey = $pageId . ':' . $scope->value . ':' . $workspaceId; + $cacheKey = $pageId . ':' . $scope->value . ':' . $this->getWorkspaceId(); if (array_key_exists($cacheKey, $this->cache)) { return $this->cache[$cacheKey]; } @@ -217,11 +218,12 @@ private function fetchPageTsConfigFragments(int $pageId, PromptFragmentScope $sc * instead lets an invalid value be normalized to "all" with a logged * warning, the same treatment as the TSconfig/registry sources. * - * Restricted to the live workspace at the query level (a table with no - * such restriction would otherwise mix in every other workspace's - * versions too, not just the current one), then overlaid per-row with - * the current workspace's version via versionOL(), the standard core - * idiom for "show the workspace-draft edit of this record, if any." + * Restricted to live records plus the ones created new in the current + * workspace at the query level, then overlaid per-row with the current + * workspace's version via versionOL(), the standard core idiom for + * "show the workspace-draft edit of this record, if any." Hidden rows + * are only dropped after the overlay, so an assignment activated or + * deactivated within a workspace resolves the way that workspace sees it. * * @param list $pageIds * @return array> keyed by parent_page, ordered by sorting @@ -229,8 +231,10 @@ private function fetchPageTsConfigFragments(int $pageId, PromptFragmentScope $sc private function fetchFragmentsByPage(array $pageIds, PromptFragmentScope $scope): array { $assignmentQueryBuilder = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - $assignmentQueryBuilder->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); - $assignmentRows = $assignmentQueryBuilder->select('uid', 'pid', 't3ver_oid', 't3ver_state', 'parent_page', 'fragment', 'inherit_to_subpages') + $assignmentQueryBuilder->getRestrictions() + ->removeByType(HiddenRestriction::class) + ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId())); + $assignmentRows = $assignmentQueryBuilder->select('uid', 'pid', 't3ver_wsid', 't3ver_oid', 't3ver_state', 'hidden', 'parent_page', 'fragment', 'inherit_to_subpages') ->from(self::ASSIGNMENT_TABLE) ->where( $assignmentQueryBuilder->expr()->in('parent_page', $assignmentQueryBuilder->createNamedParameter($pageIds, Connection::PARAM_INT_ARRAY)), @@ -244,11 +248,10 @@ private function fetchFragmentsByPage(array $pageIds, PromptFragmentScope $scope $liveAssignments = []; foreach ($assignmentRows as $row) { - $pageRepository->versionOL(self::ASSIGNMENT_TABLE, $row); - if ($row === false) { - continue; // deleted within the current workspace + $row = $this->overlay(self::ASSIGNMENT_TABLE, $row, $pageRepository); + if ($row !== null) { + $liveAssignments[] = $row; } - $liveAssignments[] = $row; } $fragmentUids = array_values(array_unique(array_map( @@ -286,8 +289,10 @@ private function fetchFragmentsByUid(array $fragmentUids, PageRepository $pageRe } $queryBuilder = $this->connectionPool->getQueryBuilderForTable(self::FRAGMENT_TABLE); - $queryBuilder->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); - $rows = $queryBuilder->select('uid', 'pid', 't3ver_oid', 't3ver_state', 'prompt', 'examples', 'scope') + $queryBuilder->getRestrictions() + ->removeByType(HiddenRestriction::class) + ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId())); + $rows = $queryBuilder->select('uid', 'pid', 't3ver_wsid', 't3ver_oid', 't3ver_state', 'hidden', 'prompt', 'examples', 'scope') ->from(self::FRAGMENT_TABLE) ->where( $queryBuilder->expr()->in('uid', $queryBuilder->createNamedParameter($fragmentUids, Connection::PARAM_INT_ARRAY)), @@ -297,9 +302,9 @@ private function fetchFragmentsByUid(array $fragmentUids, PageRepository $pageRe $byUid = []; foreach ($rows as $row) { - $pageRepository->versionOL(self::FRAGMENT_TABLE, $row); - if ($row === false) { - continue; // deleted within the current workspace + $row = $this->overlay(self::FRAGMENT_TABLE, $row, $pageRepository); + if ($row === null) { + continue; } $byUid[(int)$row['uid']] = [ 'prompt' => trim((string)$row['prompt']), @@ -309,4 +314,26 @@ private function fetchFragmentsByUid(array $fragmentUids, PageRepository $pageRe } return $byUid; } + + /** + * A move pointer is skipped: versionOL() of its live row already carries + * the move. Null for a row deleted or hidden as the current workspace + * sees it. + */ + private function overlay(string $table, array $row, PageRepository $pageRepository): ?array + { + if ((int)$row['t3ver_oid'] > 0) { + return null; + } + $pageRepository->versionOL($table, $row); + if (!is_array($row) || (int)$row['hidden'] === 1) { + return null; + } + return $row; + } + + private function getWorkspaceId(): int + { + return (int)$this->context->getPropertyFromAspect('workspace', 'id'); + } } diff --git a/Documentation/ToneOfVoice.md b/Documentation/ToneOfVoice.md index ab7c2e7..13c0aa7 100644 --- a/Documentation/ToneOfVoice.md +++ b/Documentation/ToneOfVoice.md @@ -87,7 +87,7 @@ A page can also check **"Disable inherited prompt fragments"** to skip every anc DB fragment inheritance stops at the nearest `is_siteroot` ancestor: in a nested-site install (one site's page tree living under another site's), a page's own assignments never leak into an unrelated site. Page TSconfig fragments deliberately don't get this treatment; `getPagesTSconfig()` has never respected site boundaries, and that's an established, technical-audience convention this extension doesn't override. -DB fragments respect workspace overlays for edits to *either* the assignment or the library fragment's own content: an edit made in a workspace is visible when resolving within that workspace. A fragment or assignment created entirely new within a workspace (no live counterpart yet) won't appear until published. +DB fragments respect workspaces for *either* the assignment or the library fragment's own content: an edit, an activation or deactivation, a deletion and a fragment or assignment created new within a workspace are visible when resolving within that workspace, and only there until published. To have a request resolve against a page, pass `pageId`: diff --git a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php index 624c627..ef9b68c 100644 --- a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php +++ b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php @@ -1072,6 +1072,80 @@ public function upsertAutoDetectedFragmentRefreshesInPlaceWhenOnlyAWorkspaceDraf self::assertSame('Refreshed tone.', $refreshedFragment['prompt']); } + #[Test] + public function assignmentsOfAPageAndTheirCountsLeaveWorkspaceVersionsOut(): void + { + $repository = $this->repository(); + $countsBefore = $repository->countAllAssignments(); + $liveAssignment = $this->assignmentRow(['parent_page' => 1]); + + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 1, + 'parent_page' => 1, + 'fragment' => (int)$liveAssignment['fragment'], + 'inherit_to_subpages' => 1, + 't3ver_wsid' => 3, + 't3ver_oid' => (int)$liveAssignment['uid'], + ]); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 1, + 'parent_page' => 1, + 'fragment' => (int)$liveAssignment['fragment'], + 'inherit_to_subpages' => 1, + 't3ver_wsid' => 3, + 't3ver_state' => 1, + ]); + + self::assertSame( + [(int)$liveAssignment['uid']], + array_map(static fn(array $row): int => (int)$row['uid'], $repository->findAssignmentsForPage(1)), + ); + self::assertSame($countsBefore, $repository->countAllAssignments()); + } + + /** + * The workspace version gets the lower uid, so a lookup without a + * workspace restriction finds it before the live row. + */ + #[Test] + public function theAutoDetectedFragmentIsReadAndRefreshedOnTheLiveAssignmentOnly(): void + { + $fragments = $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'); + $liveFragmentUid = $this->insertFragment($fragments, 'Tone of voice (auto-detected)', 'Live tone.', 'text'); + $draftFragmentUid = $this->insertFragment($fragments, 'Tone of voice (auto-detected)', 'Draft tone.', 'text'); + + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'uid' => 100, + 'pid' => 3, + 'parent_page' => 3, + 'fragment' => $draftFragmentUid, + 'auto_generated' => 1, + 't3ver_wsid' => 3, + 't3ver_oid' => 200, + ]); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'uid' => 200, + 'pid' => 3, + 'parent_page' => 3, + 'fragment' => $liveFragmentUid, + 'auto_generated' => 1, + ]); + + $repository = $this->repository(); + self::assertSame($liveFragmentUid, (int)$repository->findAutoDetectedFragmentForRootPage(3)['uid']); + + $refreshedUid = $repository->upsertAutoDetectedFragment(3, 'Tone of voice (auto-detected)', 'Refreshed tone.', '', ['text'], AutoGeneratedFragmentSource::Cli); + + self::assertSame($liveFragmentUid, $refreshedUid); + self::assertSame($liveFragmentUid, (int)$this->assignmentRow(['uid' => 200])['fragment']); + self::assertSame($draftFragmentUid, (int)$this->assignmentRow(['uid' => 100])['fragment']); + $prompts = $fragments->select(['uid', 'prompt'], 'tx_aim_prompt_fragment', [])->fetchAllKeyValue(); + self::assertSame('Refreshed tone.', $prompts[$liveFragmentUid]); + self::assertSame('Draft tone.', $prompts[$draftFragmentUid]); + } + /** * The audit entry is the only trace an auto-detected fragment leaves, * because the write bypasses DataHandler. It needs a backend user for diff --git a/Tests/Functional/Prompt/PagePromptResolverTest.php b/Tests/Functional/Prompt/PagePromptResolverTest.php index 974f366..3f40847 100644 --- a/Tests/Functional/Prompt/PagePromptResolverTest.php +++ b/Tests/Functional/Prompt/PagePromptResolverTest.php @@ -293,6 +293,83 @@ public function workspaceModifiedFragmentIsVisibleWhenResolvingWithinThatWorkspa ); } + #[Test] + public function anAssignmentActivatedInAWorkspaceIsComposedOnlyWithinThatWorkspace(): void + { + $fragmentUid = $this->insertWorkspaceTestPage(20, 'Activated tone.'); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', ['pid' => 20, 'parent_page' => 20, 'fragment' => $fragmentUid, 'hidden' => 1]); + $liveAssignmentUid = (int)$assignments->lastInsertId(); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 20, + 'parent_page' => 20, + 'fragment' => $fragmentUid, + 'hidden' => 0, + 't3ver_wsid' => 1, + 't3ver_oid' => $liveAssignmentUid, + ]); + + self::assertNull($this->get(PagePromptResolver::class)->resolve(20, PromptFragmentScope::All)); + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(1)); + self::assertSame('Activated tone.', $this->get(PagePromptResolver::class)->resolve(20, PromptFragmentScope::All)); + } + + #[Test] + public function anAssignmentDeactivatedInAWorkspaceIsOnlyComposedInLive(): void + { + $fragmentUid = $this->insertWorkspaceTestPage(21, 'Deactivated tone.'); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', ['pid' => 21, 'parent_page' => 21, 'fragment' => $fragmentUid]); + $liveAssignmentUid = (int)$assignments->lastInsertId(); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 21, + 'parent_page' => 21, + 'fragment' => $fragmentUid, + 'hidden' => 1, + 't3ver_wsid' => 1, + 't3ver_oid' => $liveAssignmentUid, + ]); + + self::assertSame('Deactivated tone.', $this->get(PagePromptResolver::class)->resolve(21, PromptFragmentScope::All)); + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(1)); + self::assertNull($this->get(PagePromptResolver::class)->resolve(21, PromptFragmentScope::All)); + } + + #[Test] + public function aFragmentAndAssignmentCreatedInAWorkspaceAreComposedOnlyWithinThatWorkspace(): void + { + $this->getConnectionPool()->getConnectionForTable('pages') + ->insert('pages', ['uid' => 22, 'pid' => 0, 'title' => 'New in workspace siteroot', 'is_siteroot' => 1]); + $newInWorkspace = ['t3ver_wsid' => 1, 't3ver_oid' => 0, 't3ver_state' => 1]; + $fragmentUid = $this->insertFragment( + $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'), + 'Workspace-new fragment', + 'Workspace-new tone.', + '', + PromptFragmentScope::All->value, + $newInWorkspace, + ); + $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment') + ->insert('tx_aim_page_prompt_fragment', ['pid' => 22, 'parent_page' => 22, 'fragment' => $fragmentUid, ...$newInWorkspace]); + + self::assertNull($this->get(PagePromptResolver::class)->resolve(22, PromptFragmentScope::All)); + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(1)); + self::assertSame('Workspace-new tone.', $this->get(PagePromptResolver::class)->resolve(22, PromptFragmentScope::All)); + } + + private function insertWorkspaceTestPage(int $pageId, string $prompt): int + { + $this->getConnectionPool()->getConnectionForTable('pages') + ->insert('pages', ['uid' => $pageId, 'pid' => 0, 'title' => 'Workspace visibility siteroot', 'is_siteroot' => 1]); + return $this->insertFragment( + $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'), + 'Workspace visibility fragment', + $prompt, + '', + PromptFragmentScope::All->value, + ); + } + #[Test] public function examplesAreAppendedAfterThatSameFragmentsPromptUnderAnExamplesHeading(): void { diff --git a/Tests/Unit/Configuration/TcaPromptFragmentWorkspaceVersioningTest.php b/Tests/Unit/Configuration/TcaPromptFragmentWorkspaceVersioningTest.php new file mode 100644 index 0000000..f2adc58 --- /dev/null +++ b/Tests/Unit/Configuration/TcaPromptFragmentWorkspaceVersioningTest.php @@ -0,0 +1,41 @@ + ['tx_aim_prompt_fragment']; + yield 'tx_aim_page_prompt_fragment' => ['tx_aim_page_prompt_fragment']; + } + + #[Test] + #[DataProvider('tableDataProvider')] + public function tableIsVersionedInWorkspacesWithoutLiveEdit(string $table): void + { + $tca = require __DIR__ . '/../../../Configuration/TCA/' . $table . '.php'; + + self::assertTrue($tca['ctrl']['versioningWS'] ?? false); + self::assertArrayNotHasKey('versioningWS_alwaysAllowLiveEdit', $tca['ctrl']); + } +} From 31c87965d4a079ddfafa3a479e132e66fe903dde Mon Sep 17 00:00:00 2001 From: Oliver Bartsch Date: Tue, 27 Oct 2026 09:23:31 +0100 Subject: [PATCH 3/7] [BUGFIX] Read prompt management data in the user's workspace PagePromptFragmentRepository read live records only, so the Prompt Management module and PromptManagementApi hid what a backend user had changed in their workspace. Reads now follow the workspace of the current Context. Assignments, the auto-detected fragment, the assignment counts, the page summary and the library usage see edits, activations, deactivations, deletions, moves and records created in that workspace. The paginated Pages and Library listings filter, sort and count live records plus the ones created in the workspace, like the core Records module, and overlay the rows they return. The auto-detected fragment upsert writes live without DataHandler and keeps reading live records only. Signed-off-by: Oliver Bartsch --- CHANGELOG.md | 7 +- .../PagePromptFragmentRepository.php | 455 ++++++++++-------- .../PagePromptFragmentRepositoryTest.php | 164 +++++++ phpstan-baseline.neon | 18 - 4 files changed, 411 insertions(+), 233 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e23c0c..656b993 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,11 @@ TYPO3 14.3.8 removes the option from workspace aware tables and logs a TCA migra tables, which made test suites with `failOnDeprecation` fail as soon as aim was loaded. The option is gone; on TYPO3 12.4, 13.4 and 14.0 to 14.3.7 a fragment or an assignment edited in a workspace is now versioned like on 14.3.8 and later. The tone of voice resolved within a workspace now also follows an assignment activated, -deactivated or created in that workspace, and the Prompt Management listings, the assignment counts and -`aim:calibrateVoice` read live records only instead of mixing in workspace versions. +deactivated or created in that workspace. The Prompt Management module and `PromptManagementApi` show the +fragments and assignments of the backend user's workspace instead of mixing live records and workspace versions; +the paginated Pages and Library listings filter, sort and count live records plus the ones created in that +workspace, like the core Records module. `aim:calibrateVoice` and `PromptManagementApi::saveAutoDetectedFragment()` +still write live and read live records only. - **Bugfix "Smart routing with dated model names"**: Smart routing never downgraded a request to OpenAI or Anthropic. The request log stores the model name a provider reports, a dated snapshot diff --git a/Classes/Domain/Repository/PagePromptFragmentRepository.php b/Classes/Domain/Repository/PagePromptFragmentRepository.php index 17dc4f9..055824b 100644 --- a/Classes/Domain/Repository/PagePromptFragmentRepository.php +++ b/Classes/Domain/Repository/PagePromptFragmentRepository.php @@ -15,14 +15,14 @@ use B13\Aim\Prompt\AutoGeneratedFragmentSource; use B13\Aim\Prompt\PromptFragmentScope; use TYPO3\CMS\Core\Authentication\BackendUserAuthentication; +use TYPO3\CMS\Core\Context\Context; use TYPO3\CMS\Core\Database\Connection; use TYPO3\CMS\Core\Database\ConnectionPool; use TYPO3\CMS\Core\Database\Query\QueryBuilder; use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction; -use TYPO3\CMS\Core\Database\Query\Restriction\EndTimeRestriction; use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction; -use TYPO3\CMS\Core\Database\Query\Restriction\StartTimeRestriction; use TYPO3\CMS\Core\Database\Query\Restriction\WorkspaceRestriction; +use TYPO3\CMS\Core\Domain\Repository\PageRepository; use TYPO3\CMS\Core\SysLog\Action as SystemLogGenericAction; use TYPO3\CMS\Core\SysLog\Error as SystemLogErrorClassification; use TYPO3\CMS\Core\SysLog\Type as SystemLogType; @@ -31,6 +31,13 @@ /** * Finds `pages` records that have at least one `tx_aim_prompt_fragment` * assigned via a `tx_aim_page_prompt_fragment` record. + * + * Reads follow the workspace of the current Context. The paginated listings + * (findByDemand(), findLibraryByDemand() and their counts) filter, sort and + * count live records plus the ones created in that workspace, like the core + * Records module, and overlay the rows they return. Every other read sees + * edits, moves and deletions as well. upsertAutoDetectedFragment() writes + * live without DataHandler and therefore reads live records only. */ class PagePromptFragmentRepository { @@ -48,6 +55,7 @@ class PagePromptFragmentRepository public function __construct( private readonly ConnectionPool $connectionPool, + private readonly Context $context, ) {} /** @@ -80,13 +88,16 @@ public function findByDemand(PagePromptFragmentDemand $demand, ?array $accessibl 'pages.perms_user', 'pages.perms_group', 'pages.perms_everybody', + 'pages.t3ver_oid', + 'pages.t3ver_wsid', + 'pages.t3ver_state', ) ->orderBy('pages.' . $demand->getOrderField(), $demand->getOrderDirection()) ->addOrderBy('pages.uid', 'asc') ->setMaxResults($demand->getLimit()) ->setFirstResult($demand->getOffset()); - return $qb->executeQuery()->fetchAllAssociative(); + return $this->overlay(self::PAGES_TABLE, $qb->executeQuery()->fetchAllAssociative()); } /** @@ -122,8 +133,8 @@ public function countByDemand(PagePromptFragmentDemand $demand, ?array $accessib * (list, the individual * fragments themselves, for the Preview list's own "N fragment(s)" * disclosure, mirroring the Library module's "used on N page(s)" one) to - * each given page row, from a single follow-up query scoped to just - * those page ids, not the whole matching set, so this never grows with + * each given page row, from follow-up queries scoped to just those + * page ids, not the whole matching set, so this never grows with * total fragment/page count, only with the current result page size. * * @param list> $pageRows @@ -136,47 +147,30 @@ public function enrichWithFragmentSummary(array $pageRows): array } $pageIds = array_map(static fn(array $row): int => (int)$row['uid'], $pageRows); - $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - $qb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); - // Counted like the listing. A page whose only assignment is inactive - // would report otherwise "no fragments" while holding one. - $qb->getRestrictions()->removeByType(HiddenRestriction::class); - $rows = $qb->select( - self::ASSIGNMENT_TABLE . '.parent_page', - self::FRAGMENT_TABLE . '.uid AS fragment_uid', - self::FRAGMENT_TABLE . '.title AS fragment_title', - self::FRAGMENT_TABLE . '.pid AS fragment_pid', - self::FRAGMENT_TABLE . '.scope', - ) - ->from(self::ASSIGNMENT_TABLE) - ->innerJoin( - self::ASSIGNMENT_TABLE, - self::FRAGMENT_TABLE, - self::FRAGMENT_TABLE, - $qb->expr()->eq(self::FRAGMENT_TABLE . '.uid', $qb->quoteIdentifier(self::ASSIGNMENT_TABLE . '.fragment')), - ) - ->where( - $qb->expr()->in(self::ASSIGNMENT_TABLE . '.parent_page', $qb->createNamedParameter($pageIds, Connection::PARAM_INT_ARRAY)), - ) - ->orderBy(self::ASSIGNMENT_TABLE . '.sorting') - ->addOrderBy(self::ASSIGNMENT_TABLE . '.uid') - ->executeQuery() - ->fetchAllAssociative(); + // Counted like the listing, inactive assignments included. A page whose + // only assignment is inactive would report otherwise "no fragments" + // while holding one. + $assignments = $this->sortAssignments($this->findInWorkspace(self::ASSIGNMENT_TABLE, 'parent_page', $pageIds)); + $fragments = $this->findFragmentsByUid(array_map(static fn(array $row): int => (int)$row['fragment'], $assignments)); $countByPage = []; $scopesByPage = []; $fragmentsByPage = []; - foreach ($rows as $row) { - $pageId = (int)$row['parent_page']; + foreach ($assignments as $assignment) { + $fragment = $fragments[(int)$assignment['fragment']] ?? null; + if ($fragment === null) { + continue; + } + $pageId = (int)$assignment['parent_page']; $countByPage[$pageId] = ($countByPage[$pageId] ?? 0) + 1; - foreach (PromptFragmentScope::parseList((string)$row['scope']) as $scopeValue) { + foreach (PromptFragmentScope::parseList((string)$fragment['scope']) as $scopeValue) { $scopesByPage[$pageId][$scopeValue] = true; } $fragmentsByPage[$pageId][] = [ - 'uid' => (int)$row['fragment_uid'], - 'title' => (string)$row['fragment_title'], - 'pid' => (int)$row['fragment_pid'], - 'scope' => (string)$row['scope'], + 'uid' => (int)$fragment['uid'], + 'title' => (string)$fragment['title'], + 'pid' => (int)$fragment['pid'], + 'scope' => (string)$fragment['scope'], ]; } @@ -196,28 +190,25 @@ public function enrichWithFragmentSummary(array $pageRows): array */ public function findAssignmentsForPage(int $pageId): array { - $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - $qb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); // The module reviews and activates hidden assignments, so it has to see them, unlike PagePromptResolver. - $qb->getRestrictions()->removeByType(HiddenRestriction::class); - return $qb->select( - self::ASSIGNMENT_TABLE . '.uid', - self::ASSIGNMENT_TABLE . '.inherit_to_subpages', - self::ASSIGNMENT_TABLE . '.hidden', - self::FRAGMENT_TABLE . '.title', - self::FRAGMENT_TABLE . '.scope', - ) - ->from(self::ASSIGNMENT_TABLE) - ->innerJoin( - self::ASSIGNMENT_TABLE, - self::FRAGMENT_TABLE, - self::FRAGMENT_TABLE, - $qb->expr()->eq(self::FRAGMENT_TABLE . '.uid', $qb->quoteIdentifier(self::ASSIGNMENT_TABLE . '.fragment')), - ) - ->where($qb->expr()->eq(self::ASSIGNMENT_TABLE . '.parent_page', $qb->createNamedParameter($pageId, Connection::PARAM_INT))) - ->orderBy(self::ASSIGNMENT_TABLE . '.sorting') - ->executeQuery() - ->fetchAllAssociative(); + $assignments = $this->sortAssignments($this->findInWorkspace(self::ASSIGNMENT_TABLE, 'parent_page', [$pageId])); + $fragments = $this->findFragmentsByUid(array_map(static fn(array $row): int => (int)$row['fragment'], $assignments)); + + $result = []; + foreach ($assignments as $assignment) { + $fragment = $fragments[(int)$assignment['fragment']] ?? null; + if ($fragment === null) { + continue; + } + $result[] = [ + 'uid' => (int)$assignment['uid'], + 'inherit_to_subpages' => (int)$assignment['inherit_to_subpages'], + 'title' => (string)$fragment['title'], + 'scope' => (string)$fragment['scope'], + 'hidden' => (int)$assignment['hidden'], + ]; + } + return $result; } /** @@ -228,33 +219,25 @@ public function findAssignmentsForPage(int $pageId): array */ public function findAutoDetectedFragmentForRootPage(int $rootPageId): ?array { - $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - $qb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); // A freshly calibrated fragment is hidden, and this is what shows it. - $qb->getRestrictions()->removeByType(HiddenRestriction::class); - $row = $qb->select( - self::FRAGMENT_TABLE . '.uid', - self::FRAGMENT_TABLE . '.title', - self::FRAGMENT_TABLE . '.prompt', - self::FRAGMENT_TABLE . '.examples', - self::ASSIGNMENT_TABLE . '.hidden', - self::ASSIGNMENT_TABLE . '.auto_generated_source', - ) - ->from(self::ASSIGNMENT_TABLE) - ->innerJoin( - self::ASSIGNMENT_TABLE, - self::FRAGMENT_TABLE, - self::FRAGMENT_TABLE, - $qb->expr()->eq(self::FRAGMENT_TABLE . '.uid', $qb->quoteIdentifier(self::ASSIGNMENT_TABLE . '.fragment')), - ) - ->where( - $qb->expr()->eq(self::ASSIGNMENT_TABLE . '.parent_page', $qb->createNamedParameter($rootPageId, Connection::PARAM_INT)), - $qb->expr()->eq(self::ASSIGNMENT_TABLE . '.auto_generated', $qb->createNamedParameter(1, Connection::PARAM_INT)), - ) - ->executeQuery() - ->fetchAssociative(); - - return $row === false ? null : $row; + foreach ($this->findInWorkspace(self::ASSIGNMENT_TABLE, 'parent_page', [$rootPageId]) as $assignment) { + if ((int)$assignment['auto_generated'] !== 1) { + continue; + } + $fragment = $this->findFragmentsByUid([(int)$assignment['fragment']])[(int)$assignment['fragment']] ?? null; + if ($fragment === null) { + continue; + } + return [ + 'uid' => (int)$fragment['uid'], + 'title' => (string)$fragment['title'], + 'prompt' => (string)$fragment['prompt'], + 'examples' => (string)$fragment['examples'], + 'auto_generated_source' => (string)$assignment['auto_generated_source'], + 'hidden' => (int)$assignment['hidden'], + ]; + } + return null; } /** @@ -265,20 +248,11 @@ public function findAutoDetectedFragmentForRootPage(int $rootPageId): ?array */ public function countAllAssignments(): array { - $totalQb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - $totalQb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); - $totalQb->getRestrictions()->removeByType(HiddenRestriction::class); - $total = (int)$totalQb->count('uid')->from(self::ASSIGNMENT_TABLE) - ->executeQuery()->fetchOne(); - - $inheritedQb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - $inheritedQb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); - $inheritedQb->getRestrictions()->removeByType(HiddenRestriction::class); - $inherited = (int)$inheritedQb->count('uid')->from(self::ASSIGNMENT_TABLE) - ->where($inheritedQb->expr()->eq('inherit_to_subpages', $inheritedQb->createNamedParameter(1, Connection::PARAM_INT))) - ->executeQuery()->fetchOne(); - - return ['total' => $total, 'inherited' => $inherited]; + $assignments = $this->findAllInWorkspace(self::ASSIGNMENT_TABLE); + return [ + 'total' => count($assignments), + 'inherited' => count(array_filter($assignments, static fn(array $row): bool => (bool)$row['inherit_to_subpages'])), + ]; } /** @@ -455,9 +429,9 @@ private function logFragmentWrite(int $fragmentUid, int $pageId, AutoGeneratedFr private function isFragmentSharedWithAnotherAssignment(int $fragmentUid, int $excludeAssignmentUid): bool { $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - // See getQueryBuilderForDemand()'s identical comment: without this, - // a workspace-only assignment reusing $fragmentUid would count as - // "shared", even though it has no live counterpart yet. + // Live rows only, like the upsert this guards: a workspace-only + // assignment reusing $fragmentUid has no live counterpart yet and + // must not count as "shared". $qb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); // An inactive assignment is a pending reuse, which is the case this // guard most needs to see: missing it means a re-run rewrites the @@ -481,7 +455,7 @@ private function isFragmentSharedWithAnotherAssignment(int $fragmentUid, int $ex * DefaultRestrictionContainer) excludes a soft-deleted one exactly as * it would a hard-deleted/never-existing one. HiddenRestriction is * explicitly removed, matching this class's own established convention - * (see countUsagePagesByFragment()/findUsagePagesForFragment()): a + * (see enrichLibraryWithUsage()): a * fragment an editor merely disabled is still a real row an * in-place UPDATE can safely refresh, not an orphan needing a detach - * treating it as gone would silently undo the editor's own hidden=1 @@ -506,7 +480,7 @@ private function fragmentStillExists(int $fragmentUid): bool private function getQueryBuilderForDemand(PagePromptFragmentDemand $demand, ?array $accessiblePageIds): QueryBuilder { $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - $qb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); + $qb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId())); // The module is where a human reviews and activates an inactive // assignment, so it has to list one. Filtering here would hide exactly // the rows aim:calibrateVoice tells the operator to come and look at. @@ -576,13 +550,13 @@ public function findLibraryByDemand(PagePromptFragmentDemand $demand, ?array $ac // those global fragments from a restricted user with zero // accessible pages, when the query itself would still show them. $qb = $this->getLibraryQueryBuilderForDemand($demand, $accessiblePageIds, $usagePageIds); - $qb->select('uid', 'pid', 'title', 'prompt', 'examples', 'scope', 'hidden') + $qb->select('uid', 'pid', 'title', 'prompt', 'examples', 'scope', 'hidden', 't3ver_oid', 't3ver_wsid', 't3ver_state') ->orderBy($demand->getOrderField(), $demand->getOrderDirection()) ->addOrderBy('uid', 'asc') ->setMaxResults($demand->getLimit()) ->setFirstResult($demand->getOffset()); - return $qb->executeQuery()->fetchAllAssociative(); + return $this->overlay(self::FRAGMENT_TABLE, $qb->executeQuery()->fetchAllAssociative()); } /** @@ -607,23 +581,17 @@ public function countLibraryByDemand(PagePromptFragmentDemand $demand, ?array $a * reference. * * Deliberately bypasses the `pages` table's own hidden/start/end-time - * restrictions (its deleted one, the only other one TYPO3 applies by - * default here, still stays in effect). + * restrictions; a deleted page, and so an orphaned assignment whose + * parent_page no longer exists, is not counted. * * One page can (rarely) have more than one assignment pointing at the - * same fragment (e.g. two assignments with different scopes); rows are - * deduplicated by page uid so such a page is listed once, not twice. + * same fragment (e.g. two assignments with different scopes); such a + * page is listed and counted once, not twice. * - * `usedOnPageCount` always reflects the TRUE total (a cheap, GROUP BY - * aggregate joined to `pages` so an orphaned assignment - one whose - * parent_page no longer exists - can't inflate it, unaffected by - * $maxPagesShown); `usedOnPages` is capped to $maxPagesShown per - * fragment. Deliberately one detail query per fragment (bounded by - * however many fragments are on this one demanded page, typically the - * same handful as $demand->getLimit()) rather than a single unbounded - * query for the whole page-of-fragments: a fragment inherited across a - * large site could otherwise return thousands of usage rows just to - * render $maxPagesShown of them, with no setMaxResults() to stop it. + * `usedOnPageCount` is the true total, `usedOnPages` is capped to + * $maxPagesShown per fragment. The assignments and pages of the + * fragments given are read once for all of them: only the fragments on + * one demanded page of the listing are passed in. * * @param list> $fragmentRows * @param list|null $accessiblePageIds @@ -643,108 +611,49 @@ public function enrichLibraryWithUsage(array $fragmentRows, ?array $accessiblePa } $fragmentUids = array_map(static fn(array $row): int => (int)$row['uid'], $fragmentRows); - $countsByFragment = $this->countUsagePagesByFragment($fragmentUids, $accessiblePageIds); - - return array_map(function (array $row) use ($accessiblePageIds, $maxPagesShown, $countsByFragment): array { - $fragmentUid = (int)$row['uid']; - $row['usedOnPageCount'] = $countsByFragment[$fragmentUid] ?? 0; - $row['usedOnPages'] = $row['usedOnPageCount'] > 0 - ? $this->findUsagePagesForFragment($fragmentUid, $accessiblePageIds, $maxPagesShown) - : []; + $accessible = $accessiblePageIds === null ? null : array_flip($accessiblePageIds); + $pageIdsByFragment = []; + foreach ($this->findInWorkspace(self::ASSIGNMENT_TABLE, 'fragment', $fragmentUids) as $assignment) { + $pageId = (int)$assignment['parent_page']; + if ($accessible === null || isset($accessible[$pageId])) { + $pageIdsByFragment[(int)$assignment['fragment']][$pageId] = $pageId; + } + } + $pages = $this->findUsagePages(array_merge([], ...array_values(array_map('array_values', $pageIdsByFragment)))); + + return array_map(static function (array $row) use ($pageIdsByFragment, $pages, $maxPagesShown): array { + $usagePages = array_values(array_intersect_key($pages, $pageIdsByFragment[(int)$row['uid']] ?? [])); + $row['usedOnPageCount'] = count($usagePages); + $row['usedOnPages'] = array_slice($usagePages, 0, $maxPagesShown); return $row; }, $fragmentRows); } /** - * @param list $fragmentUids - * @param list|null $accessiblePageIds - * @return array fragment uid => distinct usage page count + * @param list $pageIds + * @return array keyed by page uid, sorted by title */ - private function countUsagePagesByFragment(array $fragmentUids, ?array $accessiblePageIds): array + private function findUsagePages(array $pageIds): array { - $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - $qb->getRestrictions() - ->removeByType(HiddenRestriction::class) - ->removeByType(StartTimeRestriction::class) - ->removeByType(EndTimeRestriction::class) - ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); - $qb->selectLiteral( - $qb->quoteIdentifier(self::ASSIGNMENT_TABLE . '.fragment') . ' AS fragment', - 'COUNT(DISTINCT ' . $qb->quoteIdentifier(self::PAGES_TABLE . '.uid') . ') AS page_count', - ) - ->from(self::ASSIGNMENT_TABLE) - ->innerJoin( - self::ASSIGNMENT_TABLE, - self::PAGES_TABLE, - self::PAGES_TABLE, - $qb->expr()->eq(self::PAGES_TABLE . '.uid', $qb->quoteIdentifier(self::ASSIGNMENT_TABLE . '.parent_page')), - ) - ->where($qb->expr()->in(self::ASSIGNMENT_TABLE . '.fragment', $qb->createNamedParameter($fragmentUids, Connection::PARAM_INT_ARRAY))) - ->groupBy(self::ASSIGNMENT_TABLE . '.fragment'); - - if ($accessiblePageIds !== null) { - $qb->andWhere($qb->expr()->in( - self::ASSIGNMENT_TABLE . '.parent_page', - $qb->createNamedParameter($accessiblePageIds, Connection::PARAM_INT_ARRAY), - )); - } - - $counts = []; - foreach ($qb->executeQuery()->fetchAllAssociative() as $row) { - $counts[(int)$row['fragment']] = (int)$row['page_count']; + if ($pageIds === []) { + return []; } - return $counts; - } - - /** - * @param list|null $accessiblePageIds - * @return list - */ - private function findUsagePagesForFragment(int $fragmentUid, ?array $accessiblePageIds, int $maxPagesShown): array - { - $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); + $qb = $this->connectionPool->getQueryBuilderForTable(self::PAGES_TABLE); $qb->getRestrictions() - ->removeByType(HiddenRestriction::class) - ->removeByType(StartTimeRestriction::class) - ->removeByType(EndTimeRestriction::class) - ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); - $qb->distinct() - ->select( - self::PAGES_TABLE . '.uid', - self::PAGES_TABLE . '.pid', - self::PAGES_TABLE . '.title', - self::PAGES_TABLE . '.hidden', - self::PAGES_TABLE . '.doktype', - self::PAGES_TABLE . '.is_siteroot', - self::PAGES_TABLE . '.starttime', - self::PAGES_TABLE . '.endtime', - self::PAGES_TABLE . '.nav_hide', - self::PAGES_TABLE . '.fe_group', - self::PAGES_TABLE . '.module', - self::PAGES_TABLE . '.content_from_pid', - ) - ->from(self::ASSIGNMENT_TABLE) - ->innerJoin( - self::ASSIGNMENT_TABLE, - self::PAGES_TABLE, - self::PAGES_TABLE, - $qb->expr()->eq(self::PAGES_TABLE . '.uid', $qb->quoteIdentifier(self::ASSIGNMENT_TABLE . '.parent_page')), - ) - ->where($qb->expr()->eq(self::ASSIGNMENT_TABLE . '.fragment', $qb->createNamedParameter($fragmentUid, Connection::PARAM_INT))) - ->orderBy(self::PAGES_TABLE . '.title', 'asc') - ->addOrderBy(self::PAGES_TABLE . '.uid', 'asc') - ->setMaxResults($maxPagesShown); - - if ($accessiblePageIds !== null) { - $qb->andWhere($qb->expr()->in( - self::ASSIGNMENT_TABLE . '.parent_page', - $qb->createNamedParameter($accessiblePageIds, Connection::PARAM_INT_ARRAY), - )); - } + ->removeAll() + ->add(GeneralUtility::makeInstance(DeletedRestriction::class)) + ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId())); + $rows = $qb->select('uid', 'pid', 'title', 'hidden', 'doktype', 'is_siteroot', 'starttime', 'endtime', 'nav_hide', 'fe_group', 'module', 'content_from_pid', 't3ver_oid', 't3ver_wsid', 't3ver_state') + ->from(self::PAGES_TABLE) + ->where($qb->expr()->in('uid', $qb->createNamedParameter(array_values(array_unique($pageIds)), Connection::PARAM_INT_ARRAY))) + ->executeQuery() + ->fetchAllAssociative(); + $rows = $this->overlay(self::PAGES_TABLE, $rows); + usort($rows, static fn(array $a, array $b): int => [(string)$a['title'], (int)$a['uid']] <=> [(string)$b['title'], (int)$b['uid']]); $pages = []; - foreach ($qb->executeQuery()->fetchAllAssociative() as $row) { - $pages[] = [ + foreach ($rows as $row) { + $pages[(int)$row['uid']] = [ 'uid' => (int)$row['uid'], 'pid' => (int)$row['pid'], 'title' => (string)$row['title'], @@ -779,7 +688,7 @@ private function getLibraryQueryBuilderForDemand(PagePromptFragmentDemand $deman $qb->getRestrictions() ->removeAll() ->add(GeneralUtility::makeInstance(DeletedRestriction::class)) - ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); + ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId())); $qb->from(self::FRAGMENT_TABLE); if ($demand->hasScope() && $demand->getScope() !== PromptFragmentScope::All) { @@ -807,7 +716,7 @@ private function getLibraryQueryBuilderForDemand(PagePromptFragmentDemand $deman if ($usagePageIds !== null) { $usageSubQuery = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - $usageSubQuery->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); + $usageSubQuery->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId())); $usageSubQuery->getRestrictions()->removeByType(HiddenRestriction::class); $usageSubQuery->select('fragment') ->from(self::ASSIGNMENT_TABLE) @@ -824,6 +733,126 @@ private function getLibraryQueryBuilderForDemand(PagePromptFragmentDemand $deman return $qb; } + /** + * @param list $fragmentUids + * @return array> keyed by fragment uid + */ + private function findFragmentsByUid(array $fragmentUids): array + { + $fragments = []; + foreach ($this->findInWorkspace(self::FRAGMENT_TABLE, 'uid', $fragmentUids) as $fragment) { + $fragments[(int)$fragment['uid']] = $fragment; + } + return $fragments; + } + + /** + * Rows of $table whose $column holds one of $values as the current + * workspace sees them: edited, moved, activated, deactivated, deleted + * or created there. Hidden rows are included. A workspace version can + * hold a value its live row does not, so the candidates are looked up + * across the versions as well, then read again as live or new rows, + * overlaid and checked once more. + * + * @param list $values + * @return list> ordered by uid + */ + private function findInWorkspace(string $table, string $column, array $values): array + { + $values = array_values(array_unique($values)); + if ($values === []) { + return []; + } + + $candidateQueryBuilder = $this->getQueryBuilderInWorkspace($table, true); + $candidates = $candidateQueryBuilder->select('uid', 't3ver_oid') + ->from($table) + ->where($candidateQueryBuilder->expr()->in($column, $candidateQueryBuilder->createNamedParameter($values, Connection::PARAM_INT_ARRAY))) + ->executeQuery() + ->fetchAllAssociative(); + $uids = array_values(array_unique(array_map( + static fn(array $row): int => (int)$row['t3ver_oid'] > 0 ? (int)$row['t3ver_oid'] : (int)$row['uid'], + $candidates, + ))); + if ($uids === []) { + return []; + } + + $queryBuilder = $this->getQueryBuilderInWorkspace($table, false); + $rows = $queryBuilder->select('*') + ->from($table) + ->where($queryBuilder->expr()->in('uid', $queryBuilder->createNamedParameter($uids, Connection::PARAM_INT_ARRAY))) + ->orderBy('uid') + ->executeQuery() + ->fetchAllAssociative(); + + return array_values(array_filter( + $this->overlay($table, $rows), + static fn(array $row): bool => in_array((int)$row[$column], $values, true), + )); + } + + /** + * @return list> ordered by uid + */ + private function findAllInWorkspace(string $table): array + { + $queryBuilder = $this->getQueryBuilderInWorkspace($table, false); + return $this->overlay( + $table, + $queryBuilder->select('*')->from($table)->orderBy('uid')->executeQuery()->fetchAllAssociative(), + ); + } + + private function getQueryBuilderInWorkspace(string $table, bool $includeVersions): QueryBuilder + { + $queryBuilder = $this->connectionPool->getQueryBuilderForTable($table); + $queryBuilder->getRestrictions() + ->removeAll() + ->add(GeneralUtility::makeInstance(DeletedRestriction::class)) + ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId(), $includeVersions)); + return $queryBuilder; + } + + /** + * Overlays live and new rows with their version in the current + * workspace and drops the ones deleted there. A move pointer is + * skipped: the overlay of its live row already carries the move. + * + * @param list> $rows + * @return list> + */ + private function overlay(string $table, array $rows): array + { + $pageRepository = GeneralUtility::makeInstance(PageRepository::class, $this->context); + $overlaid = []; + foreach ($rows as $row) { + if ((int)$row['t3ver_oid'] > 0) { + continue; + } + $pageRepository->versionOL($table, $row, false, true); + if (is_array($row)) { + $overlaid[] = $row; + } + } + return $overlaid; + } + + /** + * @param list> $assignments + * @return list> + */ + private function sortAssignments(array $assignments): array + { + usort($assignments, static fn(array $a, array $b): int => [(int)$a['sorting'], (int)$a['uid']] <=> [(int)$b['sorting'], (int)$b['uid']]); + return $assignments; + } + + private function getWorkspaceId(): int + { + return (int)$this->context->getPropertyFromAspect('workspace', 'id'); + } + private function getBackendUser(): ?BackendUserAuthentication { return $GLOBALS['BE_USER'] ?? null; diff --git a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php index ef9b68c..5fd9738 100644 --- a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php +++ b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php @@ -17,6 +17,8 @@ use B13\Aim\Prompt\AutoGeneratedFragmentSource; use B13\Aim\Prompt\PromptFragmentScope; use PHPUnit\Framework\Attributes\Test; +use TYPO3\CMS\Core\Context\Context; +use TYPO3\CMS\Core\Context\WorkspaceAspect; use TYPO3\CMS\Core\Database\Connection; use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction; use TYPO3\CMS\Core\SysLog\Type as SystemLogType; @@ -1146,6 +1148,168 @@ public function theAutoDetectedFragmentIsReadAndRefreshedOnTheLiveAssignmentOnly self::assertSame('Draft tone.', $prompts[$draftFragmentUid]); } + #[Test] + public function theAssignmentsOfAPageAreReadAsTheCurrentWorkspaceSeesThem(): void + { + $uids = $this->createWorkspaceScenario(); + $repository = $this->repository(); + + $assignmentsOn = static fn(int $pageId): array => array_map( + static fn(array $row): array => [$row['uid'], $row['title'], $row['hidden']], + $repository->findAssignmentsForPage($pageId), + ); + self::assertSame( + [ + [$uids['editedAssignment'], 'Live title', 0], + [$uids['deletedAssignment'], 'Untouched fragment', 0], + [$uids['activatedAssignment'], 'Untouched fragment', 1], + [$uids['movedAssignment'], 'Live title', 0], + ], + $assignmentsOn(60), + ); + self::assertSame([], $assignmentsOn(61)); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(3)); + + self::assertSame( + [ + [$uids['editedAssignment'], 'Draft title', 0], + [$uids['activatedAssignment'], 'Untouched fragment', 0], + [$uids['newAssignment'], 'New fragment', 0], + ], + $assignmentsOn(60), + ); + self::assertSame( + [ + [$uids['movedAssignment'], 'Draft title', 0], + [$uids['newAssignmentOnSecondPage'], 'Draft title', 0], + ], + $assignmentsOn(61), + ); + } + + #[Test] + public function theAssignmentCountsAndTheAutoDetectedFragmentFollowTheCurrentWorkspace(): void + { + $repository = $this->repository(); + $liveTotal = $repository->countAllAssignments()['total']; + $this->createWorkspaceScenario(); + self::assertSame($liveTotal + 4, $repository->countAllAssignments()['total']); + self::assertSame('Live title', $repository->findAutoDetectedFragmentForRootPage(60)['title']); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(3)); + + // One deleted, two created. + self::assertSame($liveTotal + 5, $repository->countAllAssignments()['total']); + self::assertSame('Draft title', $repository->findAutoDetectedFragmentForRootPage(60)['title']); + } + + #[Test] + public function theLibraryAndItsUsageFollowTheCurrentWorkspace(): void + { + $uids = $this->createWorkspaceScenario(); + $repository = $this->repository(); + $usage = static fn(): array => array_map( + static fn(array $page): int => $page['uid'], + $repository->enrichLibraryWithUsage([['uid' => $uids['editedFragment']]], null, 10)[0]['usedOnPages'], + ); + $library = static fn(): array => array_column( + $repository->findLibraryByDemand(new PagePromptFragmentDemand(), null), + 'title', + 'uid', + ); + + self::assertSame([60], $usage()); + self::assertSame('Live title', $library()[$uids['editedFragment']]); + self::assertArrayHasKey($uids['deletedFragment'], $library()); + self::assertArrayNotHasKey($uids['newFragment'], $library()); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(3)); + + self::assertSame([60, 61], $usage()); + self::assertSame('Draft title', $library()[$uids['editedFragment']]); + self::assertArrayNotHasKey($uids['deletedFragment'], $library()); + self::assertSame('New fragment', $library()[$uids['newFragment']]); + } + + #[Test] + public function thePagesListingAndItsSummaryFollowTheCurrentWorkspace(): void + { + $this->createWorkspaceScenario(); + $repository = $this->repository(); + $summaryOf = static function (int $pageId) use ($repository): ?array { + $pageRows = $repository->findByDemand(new PagePromptFragmentDemand(), null); + foreach ($repository->enrichWithFragmentSummary($pageRows) as $pageRow) { + if ((int)$pageRow['uid'] === $pageId) { + return array_column($pageRow['fragments'], 'title'); + } + } + return null; + }; + + self::assertNull($summaryOf(61)); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(3)); + + self::assertSame(['Draft title', 'Draft title'], $summaryOf(61)); + self::assertSame(['Draft title', 'Untouched fragment', 'New fragment'], $summaryOf(60)); + } + + /** + * Pages 60 and 61, and in workspace 3: a fragment and an assignment + * edited, a fragment and an assignment deleted, an assignment activated, + * an assignment moved from page 60 to page 61, and a fragment and two + * assignments created. + * + * @return array + */ + private function createWorkspaceScenario(): array + { + $pages = $this->getConnectionPool()->getConnectionForTable('pages'); + $pages->insert('pages', ['uid' => 60, 'pid' => 0, 'title' => 'Workspace scenario page']); + $pages->insert('pages', ['uid' => 61, 'pid' => 0, 'title' => 'Workspace scenario second page']); + + $fragments = $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $insert = static function (Connection $connection, string $table, array $row): int { + $connection->insert($table, $row); + return (int)$connection->lastInsertId(); + }; + $fragment = static fn(string $title, array $workspace = []): int => $insert($fragments, 'tx_aim_prompt_fragment', [ + 'pid' => 0, + 'title' => $title, + 'prompt' => $title . '.', + 'scope' => 'all', + ...$workspace, + ]); + $assignment = static fn(int $pageId, int $fragmentUid, array $values = []): int => $insert($assignments, 'tx_aim_page_prompt_fragment', [ + 'pid' => $pageId, + 'parent_page' => $pageId, + 'fragment' => $fragmentUid, + ...$values, + ]); + + $uids = []; + $uids['editedFragment'] = $fragment('Live title'); + $fragment('Draft title', ['t3ver_wsid' => 3, 't3ver_oid' => $uids['editedFragment']]); + $untouchedFragment = $fragment('Untouched fragment'); + $uids['deletedFragment'] = $fragment('Deleted fragment'); + $fragment('Deleted fragment', ['t3ver_wsid' => 3, 't3ver_oid' => $uids['deletedFragment'], 't3ver_state' => 2]); + $uids['newFragment'] = $fragment('New fragment', ['t3ver_wsid' => 3, 't3ver_state' => 1]); + + $uids['editedAssignment'] = $assignment(60, $uids['editedFragment'], ['auto_generated' => 1]); + $uids['deletedAssignment'] = $assignment(60, $untouchedFragment); + $assignment(60, $untouchedFragment, ['t3ver_wsid' => 3, 't3ver_oid' => $uids['deletedAssignment'], 't3ver_state' => 2]); + $uids['activatedAssignment'] = $assignment(60, $untouchedFragment, ['hidden' => 1]); + $assignment(60, $untouchedFragment, ['t3ver_wsid' => 3, 't3ver_oid' => $uids['activatedAssignment']]); + $uids['movedAssignment'] = $assignment(60, $uids['editedFragment']); + $assignment(60, $uids['editedFragment'], ['parent_page' => 61, 't3ver_wsid' => 3, 't3ver_oid' => $uids['movedAssignment']]); + $uids['newAssignment'] = $assignment(60, $uids['newFragment'], ['t3ver_wsid' => 3, 't3ver_state' => 1]); + $uids['newAssignmentOnSecondPage'] = $assignment(61, $uids['editedFragment'], ['t3ver_wsid' => 3, 't3ver_state' => 1]); + + return $uids; + } + /** * The audit entry is the only trace an auto-detected fragment leaves, * because the write bypasses DataHandler. It needs a backend user for diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ce5f3ba..92b2798 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -78,24 +78,6 @@ parameters: count: 1 path: Classes/Domain/Repository/PagePromptFragmentDemand.php - - - rawMessage: 'Method B13\Aim\Domain\Repository\PagePromptFragmentRepository::findAssignmentsForPage() should return list but returns list>.' - identifier: return.type - count: 1 - path: Classes/Domain/Repository/PagePromptFragmentRepository.php - - - - rawMessage: 'Method B13\Aim\Domain\Repository\PagePromptFragmentRepository::findAutoDetectedFragmentForRootPage() should return array{uid: int, title: string, prompt: string, examples: string, auto_generated_source: string, hidden: int}|null but returns array|null.' - identifier: return.type - count: 1 - path: Classes/Domain/Repository/PagePromptFragmentRepository.php - - - - rawMessage: 'Method B13\Aim\Domain\Repository\PagePromptFragmentRepository::findUsagePagesForFragment() should return list but returns list.' - identifier: return.type - count: 1 - path: Classes/Domain/Repository/PagePromptFragmentRepository.php - - rawMessage: 'Method TYPO3\CMS\Core\Database\Connection::lastInsertId() invoked with 1 parameter, 0 required.' identifier: arguments.count From 8919be05570239a886f581ac18e52bed2b5cbd34 Mon Sep 17 00:00:00 2001 From: Oliver Bartsch Date: Tue, 27 Oct 2026 10:04:56 +0100 Subject: [PATCH 4/7] [BUGFIX] Close workspace gaps in prompt fragments The auto-detected fragment upsert counted only live assignments as sharing the fragment. An assignment created or edited in a workspace that reuses it was missed, so a re-run rewrote the fragment in place and that page carried the new text unreviewed once published. Rows of every workspace now count, except versions of the upserted assignment itself. The Library listing selected move pointers next to their live rows, so a fragment moved in a workspace was counted twice and could be counted without being listed. It now reads live and new rows only and lets the overlay apply the move. PagePromptResolver sorted assignments before the workspace overlay, so a reorder in a workspace did not change the composed prompt. It sorts the overlaid rows now. The upsert keeps writing live. Its docblock and the one of PromptManagementApi::saveAutoDetectedFragment() say that a workspace version of the fragment replaces that write once published. Signed-off-by: Oliver Bartsch --- CHANGELOG.md | 5 +- .../PagePromptFragmentRepository.php | 23 +++-- Classes/Prompt/PagePromptResolver.php | 4 +- Classes/Service/PromptManagementApi.php | 5 + .../PagePromptFragmentRepositoryTest.php | 96 ++++++++++++++++--- .../Prompt/PagePromptResolverTest.php | 29 ++++++ 6 files changed, 135 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 656b993..2cb2d0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,10 @@ deactivated or created in that workspace. The Prompt Management module and `Prom fragments and assignments of the backend user's workspace instead of mixing live records and workspace versions; the paginated Pages and Library listings filter, sort and count live records plus the ones created in that workspace, like the core Records module. `aim:calibrateVoice` and `PromptManagementApi::saveAutoDetectedFragment()` -still write live and read live records only. +still write live and read live records only, but detach the auto-detected fragment into a new one when an +assignment created or edited in any workspace reuses it. A workspace version of the fragment itself still replaces +what they wrote once it is published. Within a workspace, the tone of voice follows the assignment order of that +workspace, and the Library counts a fragment moved there once. - **Bugfix "Smart routing with dated model names"**: Smart routing never downgraded a request to OpenAI or Anthropic. The request log stores the model name a provider reports, a dated snapshot diff --git a/Classes/Domain/Repository/PagePromptFragmentRepository.php b/Classes/Domain/Repository/PagePromptFragmentRepository.php index 055824b..ce403ca 100644 --- a/Classes/Domain/Repository/PagePromptFragmentRepository.php +++ b/Classes/Domain/Repository/PagePromptFragmentRepository.php @@ -277,7 +277,9 @@ public function countAllAssignments(): array * so it always reflects whichever caller produced the CURRENT content, * not just whoever created the row first. * - * Bypasses DataHandler deliberately. + * Bypasses DataHandler deliberately, and so writes live in any workspace. + * Where an editor has a workspace version of the fragment, publishing + * that version replaces what this wrote: the editor's pending edit wins. */ public function upsertAutoDetectedFragment( int $pageId, @@ -429,19 +431,18 @@ private function logFragmentWrite(int $fragmentUid, int $pageId, AutoGeneratedFr private function isFragmentSharedWithAnotherAssignment(int $fragmentUid, int $excludeAssignmentUid): bool { $qb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); - // Live rows only, like the upsert this guards: a workspace-only - // assignment reusing $fragmentUid has no live counterpart yet and - // must not count as "shared". - $qb->getRestrictions()->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, 0)); - // An inactive assignment is a pending reuse, which is the case this - // guard most needs to see: missing it means a re-run rewrites the - // shared fragment and silently replaces content awaiting review. - $qb->getRestrictions()->removeByType(HiddenRestriction::class); + // An inactive assignment and one created or edited in any workspace + // are pending reuses, which is the case this guard most needs to see: + // missing one means a re-run rewrites the shared fragment and the + // content awaiting review or publishing silently changes. A workspace + // version of the excluded assignment itself is not another one. + $qb->getRestrictions()->removeAll()->add(GeneralUtility::makeInstance(DeletedRestriction::class)); $count = $qb->count('uid') ->from(self::ASSIGNMENT_TABLE) ->where( $qb->expr()->eq('fragment', $qb->createNamedParameter($fragmentUid, Connection::PARAM_INT)), $qb->expr()->neq('uid', $qb->createNamedParameter($excludeAssignmentUid, Connection::PARAM_INT)), + $qb->expr()->neq('t3ver_oid', $qb->createNamedParameter($excludeAssignmentUid, Connection::PARAM_INT)), ) ->executeQuery() ->fetchOne(); @@ -689,7 +690,9 @@ private function getLibraryQueryBuilderForDemand(PagePromptFragmentDemand $deman ->removeAll() ->add(GeneralUtility::makeInstance(DeletedRestriction::class)) ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId())); - $qb->from(self::FRAGMENT_TABLE); + // A move pointer would be counted next to its live row, which the overlay already moves. + $qb->from(self::FRAGMENT_TABLE) + ->where($qb->expr()->eq('t3ver_oid', $qb->createNamedParameter(0, Connection::PARAM_INT))); if ($demand->hasScope() && $demand->getScope() !== PromptFragmentScope::All) { $qb->andWhere($qb->expr()->or( diff --git a/Classes/Prompt/PagePromptResolver.php b/Classes/Prompt/PagePromptResolver.php index 71959fa..50e7f46 100644 --- a/Classes/Prompt/PagePromptResolver.php +++ b/Classes/Prompt/PagePromptResolver.php @@ -234,7 +234,7 @@ private function fetchFragmentsByPage(array $pageIds, PromptFragmentScope $scope $assignmentQueryBuilder->getRestrictions() ->removeByType(HiddenRestriction::class) ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId())); - $assignmentRows = $assignmentQueryBuilder->select('uid', 'pid', 't3ver_wsid', 't3ver_oid', 't3ver_state', 'hidden', 'parent_page', 'fragment', 'inherit_to_subpages') + $assignmentRows = $assignmentQueryBuilder->select('uid', 'pid', 't3ver_wsid', 't3ver_oid', 't3ver_state', 'hidden', 'sorting', 'parent_page', 'fragment', 'inherit_to_subpages') ->from(self::ASSIGNMENT_TABLE) ->where( $assignmentQueryBuilder->expr()->in('parent_page', $assignmentQueryBuilder->createNamedParameter($pageIds, Connection::PARAM_INT_ARRAY)), @@ -253,6 +253,8 @@ private function fetchFragmentsByPage(array $pageIds, PromptFragmentScope $scope $liveAssignments[] = $row; } } + // The query sorted live rows; a workspace can reorder them. + usort($liveAssignments, static fn(array $a, array $b): int => [(int)$a['sorting'], (int)$a['uid']] <=> [(int)$b['sorting'], (int)$b['uid']]); $fragmentUids = array_values(array_unique(array_map( static fn(array $row): int => (int)$row['fragment'], diff --git a/Classes/Service/PromptManagementApi.php b/Classes/Service/PromptManagementApi.php index 0fa3b5f..8d5648d 100644 --- a/Classes/Service/PromptManagementApi.php +++ b/Classes/Service/PromptManagementApi.php @@ -116,6 +116,11 @@ public function countAllFragmentAssignments(): FragmentAssignmentCounts * is saved. Pass $hidden = false only where a human has already approved * the text. Re-running never re-hides an assignment somebody activated. * + * The fragment is written live, also when the backend user works in a + * workspace, while findToneOfVoice() reads that workspace. A workspace + * version of the fragment therefore still shows its own text there, and + * publishing it replaces what was saved here. + * * @param list $scope */ public function saveAutoDetectedFragment( diff --git a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php index 5fd9738..142f7a0 100644 --- a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php +++ b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php @@ -1045,33 +1045,99 @@ public function enrichLibraryWithUsageIgnoresAWorkspaceOnlyAssignment(): void } /** - * Regression test: isFragmentSharedWithAnotherAssignment() (used by - * upsertAutoDetectedFragment() to decide whether the existing - * auto-detected fragment is safe to refresh in place, or must be - * detached into a fresh one because another page already relies on it) - * ran without WorkspaceRestriction. A page reusing the fragment only - * via a workspace DRAFT assignment - not yet published, no live - * counterpart - must not be treated as "sharing" it: the CLI upsert - * should still refresh the fragment in place for the live workspace. + * A reuse created in a workspace is live once published, so refreshing + * the fragment in place would hand that page the new text unreviewed. */ #[Test] - public function upsertAutoDetectedFragmentRefreshesInPlaceWhenOnlyAWorkspaceDraftReusesIt(): void + public function upsertAutoDetectedFragmentDetachesWhenAnAssignmentCreatedInAWorkspaceReusesIt(): void { $repository = $this->repository(); $firstUid = $repository->upsertAutoDetectedFragment(3, 'Tone of voice (auto-detected)', 'First tone.', '', ['text'], AutoGeneratedFragmentSource::Cli); - // Reused on a different page, but only as a workspace draft - not - // (yet) a live assignment. $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); - $assignments->insert('tx_aim_page_prompt_fragment', ['pid' => 2, 'parent_page' => 2, 'fragment' => $firstUid, 't3ver_wsid' => 3]); + $assignments->insert('tx_aim_page_prompt_fragment', ['pid' => 2, 'parent_page' => 2, 'fragment' => $firstUid, 't3ver_wsid' => 3, 't3ver_state' => 1]); + + $this->assertDetachedOnReRun($firstUid); + } + + #[Test] + public function upsertAutoDetectedFragmentDetachesWhenAnAssignmentEditedInAWorkspaceReusesIt(): void + { + $repository = $this->repository(); + $firstUid = $repository->upsertAutoDetectedFragment(3, 'Tone of voice (auto-detected)', 'First tone.', '', ['text'], AutoGeneratedFragmentSource::Cli); + + $liveAssignment = $this->assignmentRow(['parent_page' => 1]); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 1, + 'parent_page' => 1, + 'fragment' => $firstUid, + 't3ver_wsid' => 3, + 't3ver_oid' => (int)$liveAssignment['uid'], + ]); + + $this->assertDetachedOnReRun($firstUid); + } + + #[Test] + public function upsertAutoDetectedFragmentRefreshesInPlaceWhenOnlyItsOwnAssignmentHasAWorkspaceVersion(): void + { + $repository = $this->repository(); + $firstUid = $repository->upsertAutoDetectedFragment(3, 'Tone of voice (auto-detected)', 'First tone.', '', ['text'], AutoGeneratedFragmentSource::Cli); + + $ownAssignment = $this->assignmentRow(['parent_page' => 3, 'auto_generated' => 1]); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 3, + 'parent_page' => 3, + 'fragment' => $firstUid, + 'auto_generated' => 1, + 'hidden' => 0, + 't3ver_wsid' => 3, + 't3ver_oid' => (int)$ownAssignment['uid'], + ]); $secondUid = $repository->upsertAutoDetectedFragment(3, 'Tone of voice (auto-detected)', 'Refreshed tone.', '', ['text'], AutoGeneratedFragmentSource::Cli); - self::assertSame($firstUid, $secondUid, 'A workspace-only draft reuse must not trigger a detach for the live workspace.'); - $refreshedFragment = $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment') + self::assertSame($firstUid, $secondUid); + } + + private function assertDetachedOnReRun(int $firstUid): void + { + $secondUid = $this->repository()->upsertAutoDetectedFragment(3, 'Tone of voice (auto-detected)', 'Refreshed tone.', '', ['text'], AutoGeneratedFragmentSource::Cli); + + self::assertNotSame($firstUid, $secondUid); + $originalFragment = $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment') ->select(['prompt'], 'tx_aim_prompt_fragment', ['uid' => $firstUid]) ->fetchAssociative(); - self::assertSame('Refreshed tone.', $refreshedFragment['prompt']); + self::assertSame('First tone.', $originalFragment['prompt']); + } + + #[Test] + public function aFragmentMovedInAWorkspaceIsListedAndCountedOnceInTheLibrary(): void + { + $fragments = $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'); + $fragments->insert('tx_aim_prompt_fragment', ['pid' => 500, 'title' => 'Moved fragment', 'prompt' => 'Tone.', 'scope' => 'all']); + $movedUid = (int)$fragments->lastInsertId(); + $fragments->insert('tx_aim_prompt_fragment', [ + 'pid' => 501, + 'title' => 'Moved fragment', + 'prompt' => 'Tone.', + 'scope' => 'all', + 't3ver_wsid' => 3, + 't3ver_oid' => $movedUid, + 't3ver_state' => 4, + ]); + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(3)); + + $repository = $this->repository(); + $rows = $repository->findLibraryByDemand(new PagePromptFragmentDemand(), null); + + self::assertSame(count($rows), $repository->countLibraryByDemand(new PagePromptFragmentDemand(), null)); + self::assertSame([$movedUid], array_values(array_filter( + array_map(static fn(array $row): int => (int)$row['uid'], $rows), + static fn(int $uid): bool => $uid === $movedUid, + ))); } #[Test] diff --git a/Tests/Functional/Prompt/PagePromptResolverTest.php b/Tests/Functional/Prompt/PagePromptResolverTest.php index 3f40847..a6fc988 100644 --- a/Tests/Functional/Prompt/PagePromptResolverTest.php +++ b/Tests/Functional/Prompt/PagePromptResolverTest.php @@ -357,6 +357,35 @@ public function aFragmentAndAssignmentCreatedInAWorkspaceAreComposedOnlyWithinTh self::assertSame('Workspace-new tone.', $this->get(PagePromptResolver::class)->resolve(22, PromptFragmentScope::All)); } + #[Test] + public function assignmentsReorderedInAWorkspaceAreComposedInThatWorkspacesOrder(): void + { + $firstUid = $this->insertWorkspaceTestPage(23, 'First tone.'); + $secondUid = $this->insertFragment( + $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'), + 'Second fragment', + 'Second tone.', + '', + PromptFragmentScope::All->value, + ); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', ['pid' => 23, 'parent_page' => 23, 'fragment' => $firstUid, 'sorting' => 1]); + $firstAssignmentUid = (int)$assignments->lastInsertId(); + $assignments->insert('tx_aim_page_prompt_fragment', ['pid' => 23, 'parent_page' => 23, 'fragment' => $secondUid, 'sorting' => 2]); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 23, + 'parent_page' => 23, + 'fragment' => $firstUid, + 'sorting' => 3, + 't3ver_wsid' => 1, + 't3ver_oid' => $firstAssignmentUid, + ]); + + self::assertSame("First tone.\n\nSecond tone.", $this->get(PagePromptResolver::class)->resolve(23, PromptFragmentScope::All)); + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(1)); + self::assertSame("Second tone.\n\nFirst tone.", $this->get(PagePromptResolver::class)->resolve(23, PromptFragmentScope::All)); + } + private function insertWorkspaceTestPage(int $pageId, string $prompt): int { $this->getConnectionPool()->getConnectionForTable('pages') From bae01bf84eb1e472b1f847e44f57c8b732c76bac Mon Sep 17 00:00:00 2001 From: Oliver Bartsch Date: Tue, 27 Oct 2026 17:51:09 +0100 Subject: [PATCH 5/7] [BUGFIX] Keep workspace edits of prompt fragments out of live upsertAutoDetectedFragment() writes live without DataHandler. With both tables versioned, a call from within a workspace changed the live tone of voice without review. It now throws there; the CLI runs live and is unaffected. When the upsert detaches the fragment, a pending workspace version of the assignment that still points at the old fragment follows the detach, so publishing it no longer points the live assignment back. The Pages and Library listings leave out records deleted in the workspace before counting and paginating, so totals match the rows shown. countAllAssignments() counts in live and reads only the columns it needs within a workspace. Signed-off-by: Oliver Bartsch --- CHANGELOG.md | 10 +- .../PagePromptFragmentRepository.php | 91 +++++++++++++++++-- Classes/Service/PromptManagementApi.php | 9 +- Documentation/ToneOfVoice.md | 2 + .../PagePromptFragmentRepositoryTest.php | 86 ++++++++++++++++++ .../Prompt/PagePromptResolverTest.php | 64 +++++++++++++ 6 files changed, 244 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cb2d0f..e794e43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,12 @@ TYPO3 12.4, 13.4 and 14.0 to 14.3.7 a fragment or an assignment edited in a work deactivated or created in that workspace. The Prompt Management module and `PromptManagementApi` show the fragments and assignments of the backend user's workspace instead of mixing live records and workspace versions; the paginated Pages and Library listings filter, sort and count live records plus the ones created in that -workspace, like the core Records module. `aim:calibrateVoice` and `PromptManagementApi::saveAutoDetectedFragment()` -still write live and read live records only, but detach the auto-detected fragment into a new one when an -assignment created or edited in any workspace reuses it. A workspace version of the fragment itself still replaces -what they wrote once it is published. Within a workspace, the tone of voice follows the assignment order of that +workspace, like the core Records module, and leave out what is deleted there. `aim:calibrateVoice` and +`PromptManagementApi::saveAutoDetectedFragment()` still write live and read live records only, but detach the +auto-detected fragment into a new one when an assignment created or edited in any workspace reuses it, and a +pending workspace version of the assignment follows that detach. `saveAutoDetectedFragment()` now throws a +`LogicException` when called within a workspace, instead of writing live from there. A workspace version of the +fragment itself still replaces what they wrote once it is published. Within a workspace, the tone of voice follows the assignment order of that workspace, and the Library counts a fragment moved there once. - **Bugfix "Smart routing with dated model names"**: Smart routing never downgraded a request to OpenAI diff --git a/Classes/Domain/Repository/PagePromptFragmentRepository.php b/Classes/Domain/Repository/PagePromptFragmentRepository.php index ce403ca..4848eee 100644 --- a/Classes/Domain/Repository/PagePromptFragmentRepository.php +++ b/Classes/Domain/Repository/PagePromptFragmentRepository.php @@ -34,10 +34,11 @@ * * Reads follow the workspace of the current Context. The paginated listings * (findByDemand(), findLibraryByDemand() and their counts) filter, sort and - * count live records plus the ones created in that workspace, like the core - * Records module, and overlay the rows they return. Every other read sees - * edits, moves and deletions as well. upsertAutoDetectedFragment() writes - * live without DataHandler and therefore reads live records only. + * count live records plus the ones created in that workspace, without the + * ones deleted there, like the core Records module, and overlay the rows + * they return. Every other read sees edits, moves and deletions as well. + * upsertAutoDetectedFragment() writes live without DataHandler, so it reads + * live records only and refuses to run within a workspace. */ class PagePromptFragmentRepository { @@ -53,6 +54,11 @@ class PagePromptFragmentRepository private const MAX_EXAMPLES_LENGTH = 6000; private const FRAGMENT_TABLE = 'tx_aim_prompt_fragment'; + /** + * VersionState::DELETE_PLACEHOLDER, a class constant on v12 and an enum from v13 on. + */ + private const DELETE_PLACEHOLDER = 2; + public function __construct( private readonly ConnectionPool $connectionPool, private readonly Context $context, @@ -248,13 +254,29 @@ public function findAutoDetectedFragmentForRootPage(int $rootPageId): ?array */ public function countAllAssignments(): array { - $assignments = $this->findAllInWorkspace(self::ASSIGNMENT_TABLE); + if ($this->getWorkspaceId() === 0) { + return [ + 'total' => $this->countLiveAssignments(false), + 'inherited' => $this->countLiveAssignments(true), + ]; + } + $assignments = $this->findAllInWorkspace(self::ASSIGNMENT_TABLE, ['inherit_to_subpages']); return [ 'total' => count($assignments), 'inherited' => count(array_filter($assignments, static fn(array $row): bool => (bool)$row['inherit_to_subpages'])), ]; } + private function countLiveAssignments(bool $inheritedOnly): int + { + $queryBuilder = $this->getQueryBuilderInWorkspace(self::ASSIGNMENT_TABLE, false); + $queryBuilder->count('uid')->from(self::ASSIGNMENT_TABLE); + if ($inheritedOnly) { + $queryBuilder->where($queryBuilder->expr()->eq('inherit_to_subpages', $queryBuilder->createNamedParameter(1, Connection::PARAM_INT))); + } + return (int)$queryBuilder->executeQuery()->fetchOne(); + } + /** * Creates or updates the single auto-detected fragment for a page * (marked via auto_generated=1 on the ASSIGNMENT, since that's what's @@ -277,9 +299,12 @@ public function countAllAssignments(): array * so it always reflects whichever caller produced the CURRENT content, * not just whoever created the row first. * - * Bypasses DataHandler deliberately, and so writes live in any workspace. - * Where an editor has a workspace version of the fragment, publishing - * that version replaces what this wrote: the editor's pending edit wins. + * Bypasses DataHandler deliberately and writes live, so it refuses to run + * within a workspace, where both tables are versioned. Where an editor + * has a workspace version of the fragment, publishing that version + * replaces what this wrote: the editor's pending edit wins. + * + * @throws \LogicException within a workspace */ public function upsertAutoDetectedFragment( int $pageId, @@ -291,6 +316,13 @@ public function upsertAutoDetectedFragment( bool $hidden = true, bool $inheritToSubpages = true, ): int { + if ($this->getWorkspaceId() !== 0) { + throw new \LogicException( + sprintf('The auto-detected prompt fragment of page %d is written live and cannot be saved within workspace %d.', $pageId, $this->getWorkspaceId()), + 1793117896, + ); + } + $assignmentQb = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); // Live rows only: this write bypasses DataHandler, so it must never // pick up and overwrite a workspace version of the assignment. @@ -371,6 +403,13 @@ public function upsertAutoDetectedFragment( ['fragment' => $newFragmentUid] + $assignmentValues, ['uid' => $assignmentUid], ); + // A pending version still pointing at the old fragment would + // point the live assignment back at it once published. + $assignmentConnection->update( + self::ASSIGNMENT_TABLE, + ['fragment' => $newFragmentUid], + ['t3ver_oid' => $assignmentUid, 'fragment' => $fragmentUid], + ); $this->logFragmentWrite($newFragmentUid, $pageId, $source, 'detached into a new fragment'); return $newFragmentUid; } @@ -499,6 +538,9 @@ private function getQueryBuilderForDemand(PagePromptFragmentDemand $demand, ?arr self::FRAGMENT_TABLE, $qb->expr()->eq(self::FRAGMENT_TABLE . '.uid', $qb->quoteIdentifier(self::ASSIGNMENT_TABLE . '.fragment')), ); + $this->excludeDeletedInWorkspace($qb, self::ASSIGNMENT_TABLE); + $this->excludeDeletedInWorkspace($qb, self::PAGES_TABLE); + $this->excludeDeletedInWorkspace($qb, self::FRAGMENT_TABLE); if ($demand->hasScope() && $demand->getScope() !== PromptFragmentScope::All) { $qb->andWhere($qb->expr()->or( @@ -693,6 +735,7 @@ private function getLibraryQueryBuilderForDemand(PagePromptFragmentDemand $deman // A move pointer would be counted next to its live row, which the overlay already moves. $qb->from(self::FRAGMENT_TABLE) ->where($qb->expr()->eq('t3ver_oid', $qb->createNamedParameter(0, Connection::PARAM_INT))); + $this->excludeDeletedInWorkspace($qb, self::FRAGMENT_TABLE); if ($demand->hasScope() && $demand->getScope() !== PromptFragmentScope::All) { $qb->andWhere($qb->expr()->or( @@ -796,17 +839,45 @@ private function findInWorkspace(string $table, string $column, array $values): } /** + * @param list $fields read next to the ones the overlay needs * @return list> ordered by uid */ - private function findAllInWorkspace(string $table): array + private function findAllInWorkspace(string $table, array $fields): array { $queryBuilder = $this->getQueryBuilderInWorkspace($table, false); return $this->overlay( $table, - $queryBuilder->select('*')->from($table)->orderBy('uid')->executeQuery()->fetchAllAssociative(), + $queryBuilder->select('uid', 'pid', 't3ver_oid', 't3ver_wsid', 't3ver_state', ...$fields) + ->from($table) + ->orderBy('uid') + ->executeQuery() + ->fetchAllAssociative(), ); } + /** + * Leaves out the live rows of $table deleted in the current workspace. + * Their delete placeholder is a version, which the workspace restriction + * filters out, so the overlay would drop them only after the query + * counted and paginated them. + */ + private function excludeDeletedInWorkspace(QueryBuilder $queryBuilder, string $table): void + { + $workspaceId = $this->getWorkspaceId(); + if ($workspaceId === 0) { + return; + } + $placeholders = $this->connectionPool->getQueryBuilderForTable($table); + $placeholders->getRestrictions()->removeAll()->add(GeneralUtility::makeInstance(DeletedRestriction::class)); + $placeholders->select('placeholder.t3ver_oid') + ->from($table, 'placeholder') + ->where( + $placeholders->expr()->eq('placeholder.t3ver_wsid', $queryBuilder->createNamedParameter($workspaceId, Connection::PARAM_INT)), + $placeholders->expr()->eq('placeholder.t3ver_state', $queryBuilder->createNamedParameter(self::DELETE_PLACEHOLDER, Connection::PARAM_INT)), + ); + $queryBuilder->andWhere($queryBuilder->expr()->notIn($table . '.uid', $placeholders->getSQL())); + } + private function getQueryBuilderInWorkspace(string $table, bool $includeVersions): QueryBuilder { $queryBuilder = $this->connectionPool->getQueryBuilderForTable($table); diff --git a/Classes/Service/PromptManagementApi.php b/Classes/Service/PromptManagementApi.php index 8d5648d..a60e038 100644 --- a/Classes/Service/PromptManagementApi.php +++ b/Classes/Service/PromptManagementApi.php @@ -116,12 +116,13 @@ public function countAllFragmentAssignments(): FragmentAssignmentCounts * is saved. Pass $hidden = false only where a human has already approved * the text. Re-running never re-hides an assignment somebody activated. * - * The fragment is written live, also when the backend user works in a - * workspace, while findToneOfVoice() reads that workspace. A workspace - * version of the fragment therefore still shows its own text there, and - * publishing it replaces what was saved here. + * The fragment is written live, so this throws within a workspace, where + * both tables are versioned. A workspace version of the fragment still + * shows its own text in findToneOfVoice() there, and publishing it + * replaces what was saved here. * * @param list $scope + * @throws \LogicException within a workspace */ public function saveAutoDetectedFragment( int $rootPageId, diff --git a/Documentation/ToneOfVoice.md b/Documentation/ToneOfVoice.md index 13c0aa7..47ed7d6 100644 --- a/Documentation/ToneOfVoice.md +++ b/Documentation/ToneOfVoice.md @@ -157,6 +157,8 @@ Requests without a `pageId` (the default) use the `defaultSystemPrompt` Extensio `saveAutoDetectedFragment()` writes **inactive by default** (`$hidden = true`). What it saves is machine-derived, usually from page copy nobody has vetted, and it would otherwise apply to every AI request in that tree the moment it is written. Pass `$hidden = false` only where a human has already approved the text. A re-run never re-hides an assignment somebody activated. `$source` identifies the calling feature and drives `findToneOfVoice()`'s `isCliCalibrated`; it is not derived from `$title`, which an editor stays free to rename. +`saveAutoDetectedFragment()` writes live, without DataHandler, so it throws a `LogicException` when the backend user works in a workspace. Offer the action in the live workspace only. + ### Robustness notes - **Unknown scope values normalize to `all` with a logged warning**, across every source (DB, Page/User TSconfig, code-registered): a typo (`imageGeneraton`) makes a fragment apply everywhere rather than silently never firing. Far more noticeable, and thus fixable. diff --git a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php index 142f7a0..bb8f3cd 100644 --- a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php +++ b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php @@ -1113,6 +1113,92 @@ private function assertDetachedOnReRun(int $firstUid): void self::assertSame('First tone.', $originalFragment['prompt']); } + #[Test] + public function upsertAutoDetectedFragmentRefusesToWriteLiveFromAWorkspace(): void + { + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(3)); + + try { + $this->repository()->upsertAutoDetectedFragment(3, 'Tone', 'Be warm.', '', ['text'], AutoGeneratedFragmentSource::Cli); + self::fail('A workspace must not write the live fragment.'); + } catch (\LogicException $e) { + self::assertSame(1793117896, $e->getCode()); + } + self::assertSame(0, $this->countAssignments(['parent_page' => 3])); + } + + /** + * Publishing a version that still points at the fragment detached from + * would point the live assignment back at it. + */ + #[Test] + public function aPendingVersionOfTheAssignmentFollowsTheDetach(): void + { + $repository = $this->repository(); + $firstUid = $repository->upsertAutoDetectedFragment(3, 'Tone of voice (auto-detected)', 'First tone.', '', ['text'], AutoGeneratedFragmentSource::Cli); + $ownAssignmentUid = (int)$this->assignmentRow(['parent_page' => 3, 'auto_generated' => 1])['uid']; + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $this->insertAssignment($assignments, 2, $firstUid); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 3, + 'parent_page' => 3, + 'fragment' => $firstUid, + 'auto_generated' => 1, + 't3ver_wsid' => 3, + 't3ver_oid' => $ownAssignmentUid, + ]); + $otherFragmentUid = $this->insertFragment($this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'), 'Chosen in a workspace', 'Chosen.', 'text'); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 3, + 'parent_page' => 3, + 'fragment' => $otherFragmentUid, + 'auto_generated' => 1, + 't3ver_wsid' => 4, + 't3ver_oid' => $ownAssignmentUid, + ]); + + $secondUid = $repository->upsertAutoDetectedFragment(3, 'Tone of voice (auto-detected)', 'Refreshed tone.', '', ['text'], AutoGeneratedFragmentSource::Cli); + + self::assertNotSame($firstUid, $secondUid); + self::assertSame($secondUid, (int)$this->assignmentRow(['t3ver_oid' => $ownAssignmentUid, 't3ver_wsid' => 3])['fragment']); + self::assertSame($otherFragmentUid, (int)$this->assignmentRow(['t3ver_oid' => $ownAssignmentUid, 't3ver_wsid' => 4])['fragment']); + } + + #[Test] + public function theListingsLeaveOutWhatIsDeletedInTheWorkspaceBeforeCountingAndPaginating(): void + { + $uids = $this->createWorkspaceScenario(); + $pages = $this->getConnectionPool()->getConnectionForTable('pages'); + $pages->insert('pages', ['uid' => 62, 'pid' => 0, 'title' => 'Only assignment deleted in workspace']); + $pages->insert('pages', ['uid' => 63, 'pid' => 0, 'title' => 'Page deleted in workspace']); + $pages->insert('pages', ['pid' => 0, 'title' => 'Page deleted in workspace', 't3ver_wsid' => 3, 't3ver_oid' => 63, 't3ver_state' => 2]); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $this->insertAssignment($assignments, 62, $uids['editedFragment']); + $onlyAssignmentUid = (int)$this->assignmentRow(['parent_page' => 62])['uid']; + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 62, + 'parent_page' => 62, + 'fragment' => $uids['editedFragment'], + 't3ver_wsid' => 3, + 't3ver_oid' => $onlyAssignmentUid, + 't3ver_state' => 2, + ]); + $this->insertAssignment($assignments, 63, $uids['editedFragment']); + $this->insertAssignment($assignments, 63, $uids['deletedFragment']); + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(3)); + + $repository = $this->repository(); + $demand = new PagePromptFragmentDemand(); + $pageUids = array_map(static fn(array $row): int => (int)$row['uid'], $repository->findByDemand($demand, null)); + $library = $repository->findLibraryByDemand($demand, null); + + self::assertNotContains(62, $pageUids); + self::assertNotContains(63, $pageUids); + self::assertSame(count($pageUids), $repository->countByDemand($demand, null)); + self::assertNotContains($uids['deletedFragment'], array_map(static fn(array $row): int => (int)$row['uid'], $library)); + self::assertSame(count($library), $repository->countLibraryByDemand($demand, null)); + } + #[Test] public function aFragmentMovedInAWorkspaceIsListedAndCountedOnceInTheLibrary(): void { diff --git a/Tests/Functional/Prompt/PagePromptResolverTest.php b/Tests/Functional/Prompt/PagePromptResolverTest.php index a6fc988..1d50a98 100644 --- a/Tests/Functional/Prompt/PagePromptResolverTest.php +++ b/Tests/Functional/Prompt/PagePromptResolverTest.php @@ -386,6 +386,70 @@ public function assignmentsReorderedInAWorkspaceAreComposedInThatWorkspacesOrder self::assertSame("Second tone.\n\nFirst tone.", $this->get(PagePromptResolver::class)->resolve(23, PromptFragmentScope::All)); } + #[Test] + public function anAssignmentDeletedInAWorkspaceIsOnlyComposedInLive(): void + { + $fragmentUid = $this->insertWorkspaceTestPage(24, 'Deleted assignment tone.'); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', ['pid' => 24, 'parent_page' => 24, 'fragment' => $fragmentUid]); + $liveAssignmentUid = (int)$assignments->lastInsertId(); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 24, + 'parent_page' => 24, + 'fragment' => $fragmentUid, + 't3ver_wsid' => 1, + 't3ver_oid' => $liveAssignmentUid, + 't3ver_state' => 2, + ]); + + self::assertSame('Deleted assignment tone.', $this->get(PagePromptResolver::class)->resolve(24, PromptFragmentScope::All)); + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(1)); + self::assertNull($this->get(PagePromptResolver::class)->resolve(24, PromptFragmentScope::All)); + } + + #[Test] + public function aFragmentDeletedInAWorkspaceIsOnlyComposedInLive(): void + { + $fragmentUid = $this->insertWorkspaceTestPage(25, 'Deleted fragment tone.'); + $fragments = $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'); + $this->insertFragment($fragments, 'Workspace visibility fragment', 'Deleted fragment tone.', '', PromptFragmentScope::All->value, [ + 't3ver_wsid' => 1, + 't3ver_oid' => $fragmentUid, + 't3ver_state' => 2, + ]); + $this->insertAssignment($this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'), 25, $fragmentUid, 1); + + self::assertSame('Deleted fragment tone.', $this->get(PagePromptResolver::class)->resolve(25, PromptFragmentScope::All)); + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(1)); + self::assertNull($this->get(PagePromptResolver::class)->resolve(25, PromptFragmentScope::All)); + } + + #[Test] + public function anAssignmentAndAFragmentMovedInAWorkspaceAreComposedOnce(): void + { + $fragmentUid = $this->insertWorkspaceTestPage(26, 'Moved tone.'); + $this->insertFragment($this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'), 'Workspace visibility fragment', 'Moved tone.', '', PromptFragmentScope::All->value, [ + 'pid' => 26, + 't3ver_wsid' => 1, + 't3ver_oid' => $fragmentUid, + 't3ver_state' => 4, + ]); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', ['pid' => 26, 'parent_page' => 26, 'fragment' => $fragmentUid]); + $liveAssignmentUid = (int)$assignments->lastInsertId(); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 0, + 'parent_page' => 26, + 'fragment' => $fragmentUid, + 't3ver_wsid' => 1, + 't3ver_oid' => $liveAssignmentUid, + 't3ver_state' => 4, + ]); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(1)); + self::assertSame('Moved tone.', $this->get(PagePromptResolver::class)->resolve(26, PromptFragmentScope::All)); + } + private function insertWorkspaceTestPage(int $pageId, string $prompt): int { $this->getConnectionPool()->getConnectionForTable('pages') From 5bc6a89852cf871c5ca69cbbe5b71ba9e0220721 Mon Sep 17 00:00:00 2001 From: Oliver Bartsch Date: Tue, 27 Oct 2026 22:05:56 +0100 Subject: [PATCH 6/7] [BUGFIX] Leave workspace deletions out of the Library page filter The page filter of the Library listing read the assignments of the current workspace without leaving out the ones deleted there, so a fragment whose only assignment on the selected page was deleted in the workspace was still listed and counted. excludeDeletedInWorkspace() now uses integer literals, so it also applies to that subquery. The docblocks and the changelog no longer claim that publishing a workspace version of the fragment replaces what the upsert wrote after it detached the assignment into a new fragment. Signed-off-by: Oliver Bartsch --- CHANGELOG.md | 4 +-- .../PagePromptFragmentRepository.php | 10 ++++-- Classes/Service/PromptManagementApi.php | 3 +- .../PagePromptFragmentRepositoryTest.php | 32 +++++++++++++++++++ 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e794e43..c5bd4d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,8 +17,8 @@ workspace, like the core Records module, and leave out what is deleted there. `a auto-detected fragment into a new one when an assignment created or edited in any workspace reuses it, and a pending workspace version of the assignment follows that detach. `saveAutoDetectedFragment()` now throws a `LogicException` when called within a workspace, instead of writing live from there. A workspace version of the -fragment itself still replaces what they wrote once it is published. Within a workspace, the tone of voice follows the assignment order of that -workspace, and the Library counts a fragment moved there once. +fragment itself still replaces what they refreshed in place once it is published. Within a workspace, the tone +of voice follows the assignment order of that workspace, and the Library counts a fragment moved there once. - **Bugfix "Smart routing with dated model names"**: Smart routing never downgraded a request to OpenAI or Anthropic. The request log stores the model name a provider reports, a dated snapshot diff --git a/Classes/Domain/Repository/PagePromptFragmentRepository.php b/Classes/Domain/Repository/PagePromptFragmentRepository.php index 4848eee..e87b289 100644 --- a/Classes/Domain/Repository/PagePromptFragmentRepository.php +++ b/Classes/Domain/Repository/PagePromptFragmentRepository.php @@ -302,7 +302,8 @@ private function countLiveAssignments(bool $inheritedOnly): int * Bypasses DataHandler deliberately and writes live, so it refuses to run * within a workspace, where both tables are versioned. Where an editor * has a workspace version of the fragment, publishing that version - * replaces what this wrote: the editor's pending edit wins. + * replaces what a refresh in place wrote. After a detach the assignment + * points at the new fragment, and that version changes the old one only. * * @throws \LogicException within a workspace */ @@ -770,6 +771,7 @@ private function getLibraryQueryBuilderForDemand(PagePromptFragmentDemand $deman 'parent_page', $qb->createNamedParameter($usagePageIds, Connection::PARAM_INT_ARRAY), )); + $this->excludeDeletedInWorkspace($usageSubQuery, self::ASSIGNMENT_TABLE); $qb->andWhere($qb->expr()->or( $qb->expr()->eq('pid', $qb->createNamedParameter(0, Connection::PARAM_INT)), $qb->expr()->in('uid', $usageSubQuery->getSQL()), @@ -869,11 +871,13 @@ private function excludeDeletedInWorkspace(QueryBuilder $queryBuilder, string $t } $placeholders = $this->connectionPool->getQueryBuilderForTable($table); $placeholders->getRestrictions()->removeAll()->add(GeneralUtility::makeInstance(DeletedRestriction::class)); + // Integer literals like WorkspaceRestriction's own, so this also + // works for a $queryBuilder that is itself used as a subquery. $placeholders->select('placeholder.t3ver_oid') ->from($table, 'placeholder') ->where( - $placeholders->expr()->eq('placeholder.t3ver_wsid', $queryBuilder->createNamedParameter($workspaceId, Connection::PARAM_INT)), - $placeholders->expr()->eq('placeholder.t3ver_state', $queryBuilder->createNamedParameter(self::DELETE_PLACEHOLDER, Connection::PARAM_INT)), + $placeholders->expr()->eq('placeholder.t3ver_wsid', $workspaceId), + $placeholders->expr()->eq('placeholder.t3ver_state', self::DELETE_PLACEHOLDER), ); $queryBuilder->andWhere($queryBuilder->expr()->notIn($table . '.uid', $placeholders->getSQL())); } diff --git a/Classes/Service/PromptManagementApi.php b/Classes/Service/PromptManagementApi.php index a60e038..cdddbf9 100644 --- a/Classes/Service/PromptManagementApi.php +++ b/Classes/Service/PromptManagementApi.php @@ -119,7 +119,8 @@ public function countAllFragmentAssignments(): FragmentAssignmentCounts * The fragment is written live, so this throws within a workspace, where * both tables are versioned. A workspace version of the fragment still * shows its own text in findToneOfVoice() there, and publishing it - * replaces what was saved here. + * replaces what was saved here, unless the save detached the assignment + * into a new fragment. * * @param list $scope * @throws \LogicException within a workspace diff --git a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php index bb8f3cd..fabb350 100644 --- a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php +++ b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php @@ -1199,6 +1199,38 @@ public function theListingsLeaveOutWhatIsDeletedInTheWorkspaceBeforeCountingAndP self::assertSame(count($library), $repository->countLibraryByDemand($demand, null)); } + #[Test] + public function theLibraryUsageFilterLeavesOutAnAssignmentDeletedInTheWorkspace(): void + { + $this->getConnectionPool()->getConnectionForTable('pages')->insert('pages', ['uid' => 64, 'pid' => 0, 'title' => 'Usage deleted in workspace']); + $fragments = $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'); + $fragments->insert('tx_aim_prompt_fragment', ['pid' => 64, 'title' => 'Used on page 64 only', 'prompt' => 'Tone.', 'scope' => 'all']); + $fragmentUid = (int)$fragments->lastInsertId(); + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $this->insertAssignment($assignments, 64, $fragmentUid); + $assignments->insert('tx_aim_page_prompt_fragment', [ + 'pid' => 64, + 'parent_page' => 64, + 'fragment' => $fragmentUid, + 't3ver_wsid' => 3, + 't3ver_oid' => (int)$this->assignmentRow(['parent_page' => 64])['uid'], + 't3ver_state' => 2, + ]); + $repository = $this->repository(); + $demand = new PagePromptFragmentDemand(); + $filteredUids = static fn(): array => array_map( + static fn(array $row): int => (int)$row['uid'], + $repository->findLibraryByDemand($demand, null, [64]), + ); + + self::assertContains($fragmentUid, $filteredUids()); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(3)); + + self::assertNotContains($fragmentUid, $filteredUids()); + self::assertSame(count($filteredUids()), $repository->countLibraryByDemand($demand, null, [64])); + } + #[Test] public function aFragmentMovedInAWorkspaceIsListedAndCountedOnceInTheLibrary(): void { From 2b08794eaa88d356c0a755d7ee664e9ab4dd391b Mon Sep 17 00:00:00 2001 From: Oliver Bartsch Date: Tue, 27 Oct 2026 23:33:02 +0100 Subject: [PATCH 7/7] [BUGFIX] Resolve prompt fragments in workspaces with PlainDataResolver PagePromptFragmentRepository overlaid every row on its own with versionOL(), one query per row, and sorted the result in PHP. It now resolves the uids of a workspace through PlainDataResolver and reads the resolved rows in one query, sorted in SQL, so a read costs the same few queries however many rows it returns. countAllAssignments() loaded the whole assignment table within a workspace to count it. It counts in SQL now, each assignment once by its version in that workspace, without the ones deleted there. PagePromptResolver overlays with bypassEnableFieldsCheck, since it checks hidden itself afterwards, which saves a query per row on 12.4 and 13.4. A functional test now edits and deletes through DataHandler within a workspace and checks that live stays untouched and that the Prompt Management reads see the workspace only there. It replaces the unit test that checked the TCA keys. PlainDataResolver resolves nothing and DataHandler versions nothing without EXT:workspaces, so it is a dev dependency now and part of the 12.4 and 13.4 CI legs. Signed-off-by: Oliver Bartsch --- .github/workflows/ci.yml | 6 +- .../PagePromptFragmentRepository.php | 165 ++++++++++-------- Classes/Prompt/PagePromptResolver.php | 2 +- .../PromptFragmentWorkspaceVersioningTest.php | 149 ++++++++++++++++ .../PagePromptFragmentRepositoryTest.php | 4 + ...aPromptFragmentWorkspaceVersioningTest.php | 41 ----- composer.json | 1 + 7 files changed, 249 insertions(+), 119 deletions(-) create mode 100644 Tests/Functional/DataHandling/PromptFragmentWorkspaceVersioningTest.php delete mode 100644 Tests/Unit/Configuration/TcaPromptFragmentWorkspaceVersioningTest.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5349d7..a5b7bb0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,7 +88,8 @@ jobs: run: | composer require --dev --no-progress --no-interaction -W \ typo3/cms-core:^13.4 typo3/cms-backend:^13.4 \ - typo3/cms-install:^13.4 typo3/cms-dashboard:^13.4 + typo3/cms-install:^13.4 typo3/cms-dashboard:^13.4 \ + typo3/cms-workspaces:^13.4 # Two more constraints than the v13 leg needs. phpunit has to come down # with the rest: typo3/cms-install ^12.4 and phpunit ^11 cannot be @@ -105,7 +106,8 @@ jobs: composer config policy.advisories.block false composer require --dev --no-progress --no-interaction -W \ typo3/cms-core:^12.4 typo3/cms-backend:^12.4 typo3/cms-install:^12.4 \ - typo3/cms-dashboard:^12.4 typo3/testing-framework:^8.0 \ + typo3/cms-dashboard:^12.4 typo3/cms-workspaces:^12.4 \ + typo3/testing-framework:^8.0 \ phpunit/phpunit:^10.5 - name: Lint diff --git a/Classes/Domain/Repository/PagePromptFragmentRepository.php b/Classes/Domain/Repository/PagePromptFragmentRepository.php index e87b289..8ddc208 100644 --- a/Classes/Domain/Repository/PagePromptFragmentRepository.php +++ b/Classes/Domain/Repository/PagePromptFragmentRepository.php @@ -22,7 +22,7 @@ use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction; use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction; use TYPO3\CMS\Core\Database\Query\Restriction\WorkspaceRestriction; -use TYPO3\CMS\Core\Domain\Repository\PageRepository; +use TYPO3\CMS\Core\DataHandling\PlainDataResolver; use TYPO3\CMS\Core\SysLog\Action as SystemLogGenericAction; use TYPO3\CMS\Core\SysLog\Error as SystemLogErrorClassification; use TYPO3\CMS\Core\SysLog\Type as SystemLogType; @@ -156,7 +156,7 @@ public function enrichWithFragmentSummary(array $pageRows): array // Counted like the listing, inactive assignments included. A page whose // only assignment is inactive would report otherwise "no fragments" // while holding one. - $assignments = $this->sortAssignments($this->findInWorkspace(self::ASSIGNMENT_TABLE, 'parent_page', $pageIds)); + $assignments = $this->findInWorkspace(self::ASSIGNMENT_TABLE, 'parent_page', $pageIds, ['sorting', 'uid']); $fragments = $this->findFragmentsByUid(array_map(static fn(array $row): int => (int)$row['fragment'], $assignments)); $countByPage = []; @@ -197,7 +197,7 @@ public function enrichWithFragmentSummary(array $pageRows): array public function findAssignmentsForPage(int $pageId): array { // The module reviews and activates hidden assignments, so it has to see them, unlike PagePromptResolver. - $assignments = $this->sortAssignments($this->findInWorkspace(self::ASSIGNMENT_TABLE, 'parent_page', [$pageId])); + $assignments = $this->findInWorkspace(self::ASSIGNMENT_TABLE, 'parent_page', [$pageId], ['sorting', 'uid']); $fragments = $this->findFragmentsByUid(array_map(static fn(array $row): int => (int)$row['fragment'], $assignments)); $result = []; @@ -254,26 +254,35 @@ public function findAutoDetectedFragmentForRootPage(int $rootPageId): ?array */ public function countAllAssignments(): array { - if ($this->getWorkspaceId() === 0) { - return [ - 'total' => $this->countLiveAssignments(false), - 'inherited' => $this->countLiveAssignments(true), - ]; - } - $assignments = $this->findAllInWorkspace(self::ASSIGNMENT_TABLE, ['inherit_to_subpages']); return [ - 'total' => count($assignments), - 'inherited' => count(array_filter($assignments, static fn(array $row): bool => (bool)$row['inherit_to_subpages'])), + 'total' => $this->countAssignmentsInWorkspace(false), + 'inherited' => $this->countAssignmentsInWorkspace(true), ]; } - private function countLiveAssignments(bool $inheritedOnly): int + /** + * Counts each assignment once, by its version in the current workspace + * where it has one, without the ones deleted there. + */ + private function countAssignmentsInWorkspace(bool $inheritedOnly): int { - $queryBuilder = $this->getQueryBuilderInWorkspace(self::ASSIGNMENT_TABLE, false); + $queryBuilder = $this->getQueryBuilderInWorkspace(self::ASSIGNMENT_TABLE, true); $queryBuilder->count('uid')->from(self::ASSIGNMENT_TABLE); if ($inheritedOnly) { $queryBuilder->where($queryBuilder->expr()->eq('inherit_to_subpages', $queryBuilder->createNamedParameter(1, Connection::PARAM_INT))); } + $workspaceId = $this->getWorkspaceId(); + if ($workspaceId !== 0) { + $versions = $this->connectionPool->getQueryBuilderForTable(self::ASSIGNMENT_TABLE); + $versions->getRestrictions()->removeAll()->add(GeneralUtility::makeInstance(DeletedRestriction::class)); + $versions->select('ws_version.t3ver_oid') + ->from(self::ASSIGNMENT_TABLE, 'ws_version') + ->where( + $versions->expr()->eq('ws_version.t3ver_wsid', $workspaceId), + $versions->expr()->gt('ws_version.t3ver_oid', 0), + ); + $queryBuilder->andWhere($queryBuilder->expr()->notIn('uid', $versions->getSQL())); + } return (int)$queryBuilder->executeQuery()->fetchOne(); } @@ -682,18 +691,12 @@ private function findUsagePages(array $pageIds): array if ($pageIds === []) { return []; } - $qb = $this->connectionPool->getQueryBuilderForTable(self::PAGES_TABLE); - $qb->getRestrictions() - ->removeAll() - ->add(GeneralUtility::makeInstance(DeletedRestriction::class)) - ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->getWorkspaceId())); - $rows = $qb->select('uid', 'pid', 'title', 'hidden', 'doktype', 'is_siteroot', 'starttime', 'endtime', 'nav_hide', 'fe_group', 'module', 'content_from_pid', 't3ver_oid', 't3ver_wsid', 't3ver_state') - ->from(self::PAGES_TABLE) - ->where($qb->expr()->in('uid', $qb->createNamedParameter(array_values(array_unique($pageIds)), Connection::PARAM_INT_ARRAY))) - ->executeQuery() - ->fetchAllAssociative(); - $rows = $this->overlay(self::PAGES_TABLE, $rows); - usort($rows, static fn(array $a, array $b): int => [(string)$a['title'], (int)$a['uid']] <=> [(string)$b['title'], (int)$b['uid']]); + $rows = $this->findByUidInWorkspace( + self::PAGES_TABLE, + $pageIds, + ['uid', 'pid', 'title', 'hidden', 'doktype', 'is_siteroot', 'starttime', 'endtime', 'nav_hide', 'fe_group', 'module', 'content_from_pid'], + ['title', 'uid'], + ); $pages = []; foreach ($rows as $row) { @@ -799,13 +802,13 @@ private function findFragmentsByUid(array $fragmentUids): array * workspace sees them: edited, moved, activated, deactivated, deleted * or created there. Hidden rows are included. A workspace version can * hold a value its live row does not, so the candidates are looked up - * across the versions as well, then read again as live or new rows, - * overlaid and checked once more. + * across the versions as well and checked again once resolved. * * @param list $values - * @return list> ordered by uid + * @param list $orderBy + * @return list> */ - private function findInWorkspace(string $table, string $column, array $values): array + private function findInWorkspace(string $table, string $column, array $values, array $orderBy = ['uid']): array { $values = array_values(array_unique($values)); if ($values === []) { @@ -818,43 +821,59 @@ private function findInWorkspace(string $table, string $column, array $values): ->where($candidateQueryBuilder->expr()->in($column, $candidateQueryBuilder->createNamedParameter($values, Connection::PARAM_INT_ARRAY))) ->executeQuery() ->fetchAllAssociative(); - $uids = array_values(array_unique(array_map( + $uids = array_map( static fn(array $row): int => (int)$row['t3ver_oid'] > 0 ? (int)$row['t3ver_oid'] : (int)$row['uid'], $candidates, - ))); - if ($uids === []) { - return []; - } - - $queryBuilder = $this->getQueryBuilderInWorkspace($table, false); - $rows = $queryBuilder->select('*') - ->from($table) - ->where($queryBuilder->expr()->in('uid', $queryBuilder->createNamedParameter($uids, Connection::PARAM_INT_ARRAY))) - ->orderBy('uid') - ->executeQuery() - ->fetchAllAssociative(); + ); return array_values(array_filter( - $this->overlay($table, $rows), + $this->findByUidInWorkspace($table, $uids, ['*'], $orderBy), static fn(array $row): bool => in_array((int)$row[$column], $values, true), )); } /** - * @param list $fields read next to the ones the overlay needs - * @return list> ordered by uid + * Rows of $table with the given live or new uids as the current + * workspace sees them, in $orderBy order of that workspace. A version + * keeps the uid of its live record, as after versionOL(), and the ones + * deleted there are left out. + * + * @param list $uids + * @param list $fields + * @param list $orderBy + * @return array> keyed by uid */ - private function findAllInWorkspace(string $table, array $fields): array + private function findByUidInWorkspace(string $table, array $uids, array $fields, array $orderBy): array { - $queryBuilder = $this->getQueryBuilderInWorkspace($table, false); - return $this->overlay( - $table, - $queryBuilder->select('uid', 'pid', 't3ver_oid', 't3ver_wsid', 't3ver_state', ...$fields) - ->from($table) - ->orderBy('uid') - ->executeQuery() - ->fetchAllAssociative(), - ); + $uids = array_values(array_unique($uids)); + $workspaceId = $this->getWorkspaceId(); + if ($uids !== [] && $workspaceId !== 0) { + $resolver = GeneralUtility::makeInstance(PlainDataResolver::class, $table, $uids); + $resolver->setWorkspaceId($workspaceId); + $uids = array_values($resolver->get()); + } + if ($uids === []) { + return []; + } + + $queryBuilder = $this->connectionPool->getQueryBuilderForTable($table); + $queryBuilder->getRestrictions()->removeAll()->add(GeneralUtility::makeInstance(DeletedRestriction::class)); + $queryBuilder->select(...array_unique([...$fields, 'uid', 't3ver_oid'])) + ->from($table) + ->where($queryBuilder->expr()->in('uid', $queryBuilder->createNamedParameter($uids, Connection::PARAM_INT_ARRAY))); + foreach ($orderBy as $field) { + $queryBuilder->addOrderBy($field); + } + + $rows = []; + foreach ($queryBuilder->executeQuery()->fetchAllAssociative() as $row) { + if ((int)$row['t3ver_oid'] > 0) { + $row['_ORIG_uid'] = (int)$row['uid']; + $row['uid'] = (int)$row['t3ver_oid']; + } + $rows[(int)$row['uid']] = $row; + } + return $rows; } /** @@ -894,38 +913,34 @@ private function getQueryBuilderInWorkspace(string $table, bool $includeVersions /** * Overlays live and new rows with their version in the current - * workspace and drops the ones deleted there. A move pointer is - * skipped: the overlay of its live row already carries the move. + * workspace and drops the ones deleted there, keeping their order. A + * move pointer is skipped: the overlay of its live row carries the move. * * @param list> $rows * @return list> */ private function overlay(string $table, array $rows): array { - $pageRepository = GeneralUtility::makeInstance(PageRepository::class, $this->context); + if ($rows === [] || $this->getWorkspaceId() === 0) { + return $rows; + } + $fields = array_keys($rows[0]); + $rows = array_filter($rows, static fn(array $row): bool => (int)$row['t3ver_oid'] === 0); + $resolved = $this->findByUidInWorkspace( + $table, + array_map(static fn(array $row): int => (int)$row['uid'], $rows), + $fields, + [], + ); $overlaid = []; foreach ($rows as $row) { - if ((int)$row['t3ver_oid'] > 0) { - continue; - } - $pageRepository->versionOL($table, $row, false, true); - if (is_array($row)) { - $overlaid[] = $row; + if (isset($resolved[(int)$row['uid']])) { + $overlaid[] = $resolved[(int)$row['uid']]; } } return $overlaid; } - /** - * @param list> $assignments - * @return list> - */ - private function sortAssignments(array $assignments): array - { - usort($assignments, static fn(array $a, array $b): int => [(int)$a['sorting'], (int)$a['uid']] <=> [(int)$b['sorting'], (int)$b['uid']]); - return $assignments; - } - private function getWorkspaceId(): int { return (int)$this->context->getPropertyFromAspect('workspace', 'id'); diff --git a/Classes/Prompt/PagePromptResolver.php b/Classes/Prompt/PagePromptResolver.php index 50e7f46..781eee2 100644 --- a/Classes/Prompt/PagePromptResolver.php +++ b/Classes/Prompt/PagePromptResolver.php @@ -327,7 +327,7 @@ private function overlay(string $table, array $row, PageRepository $pageReposito if ((int)$row['t3ver_oid'] > 0) { return null; } - $pageRepository->versionOL($table, $row); + $pageRepository->versionOL($table, $row, false, true); if (!is_array($row) || (int)$row['hidden'] === 1) { return null; } diff --git a/Tests/Functional/DataHandling/PromptFragmentWorkspaceVersioningTest.php b/Tests/Functional/DataHandling/PromptFragmentWorkspaceVersioningTest.php new file mode 100644 index 0000000..e73bd75 --- /dev/null +++ b/Tests/Functional/DataHandling/PromptFragmentWorkspaceVersioningTest.php @@ -0,0 +1,149 @@ +getConnectionPool()->getConnectionForTable('be_users')->insert('be_users', ['uid' => 1, 'username' => 'admin', 'admin' => 1]); + $this->getConnectionPool()->getConnectionForTable('sys_workspace')->insert('sys_workspace', ['uid' => 1, 'pid' => 0, 'title' => 'Draft']); + $this->getConnectionPool()->getConnectionForTable('pages')->insert('pages', ['uid' => 1, 'pid' => 0, 'title' => 'Standard page']); + + $fragments = $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment'); + $fragments->insert('tx_aim_prompt_fragment', ['pid' => 0, 'title' => 'Live fragment', 'prompt' => 'Live tone.', 'scope' => PromptFragmentScope::All->value]); + $this->liveFragmentUid = (int)$fragments->lastInsertId(); + $fragments->insert('tx_aim_prompt_fragment', ['pid' => 0, 'title' => 'Other fragment', 'prompt' => 'Other tone.', 'scope' => PromptFragmentScope::All->value]); + $this->otherFragmentUid = (int)$fragments->lastInsertId(); + + $assignments = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment'); + $assignments->insert('tx_aim_page_prompt_fragment', ['pid' => 1, 'parent_page' => 1, 'fragment' => $this->liveFragmentUid, 'inherit_to_subpages' => 1]); + $this->liveAssignmentUid = (int)$assignments->lastInsertId(); + } + + #[Test] + public function editsMadeInAWorkspaceAreVersionedAndLeaveLiveUntouched(): void + { + $this->processInWorkspace([ + 'tx_aim_prompt_fragment' => [ + $this->liveFragmentUid => ['prompt' => 'Draft tone.'], + ], + 'tx_aim_page_prompt_fragment' => [ + $this->liveAssignmentUid => ['fragment' => $this->otherFragmentUid, 'inherit_to_subpages' => 0], + ], + ]); + + $liveFragment = $this->getConnectionPool()->getConnectionForTable('tx_aim_prompt_fragment') + ->select(['prompt'], 'tx_aim_prompt_fragment', ['uid' => $this->liveFragmentUid]) + ->fetchAssociative(); + self::assertSame('Live tone.', $liveFragment['prompt']); + $liveAssignment = $this->getConnectionPool()->getConnectionForTable('tx_aim_page_prompt_fragment') + ->select(['fragment', 'inherit_to_subpages'], 'tx_aim_page_prompt_fragment', ['uid' => $this->liveAssignmentUid]) + ->fetchAssociative(); + self::assertSame($this->liveFragmentUid, (int)$liveAssignment['fragment']); + self::assertSame(1, (int)$liveAssignment['inherit_to_subpages']); + + self::assertSame(1, $this->countVersions('tx_aim_prompt_fragment', $this->liveFragmentUid)); + self::assertSame(1, $this->countVersions('tx_aim_page_prompt_fragment', $this->liveAssignmentUid)); + } + + #[Test] + public function promptManagementReadsTheWorkspaceVersionsOnlyWithinTheWorkspace(): void + { + $this->processInWorkspace([ + 'tx_aim_page_prompt_fragment' => [ + $this->liveAssignmentUid => ['fragment' => $this->otherFragmentUid, 'inherit_to_subpages' => 0], + ], + ]); + $repository = $this->get(PagePromptFragmentRepository::class); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(1)); + $assignments = $repository->findAssignmentsForPage(1); + self::assertCount(1, $assignments); + self::assertSame($this->liveAssignmentUid, $assignments[0]['uid']); + self::assertSame('Other fragment', $assignments[0]['title']); + self::assertSame(['total' => 1, 'inherited' => 0], $repository->countAllAssignments()); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(0)); + self::assertSame('Live fragment', $repository->findAssignmentsForPage(1)[0]['title']); + self::assertSame(['total' => 1, 'inherited' => 1], $repository->countAllAssignments()); + } + + #[Test] + public function anAssignmentDeletedInAWorkspaceIsGoneThereOnly(): void + { + $this->processInWorkspace([], [ + 'tx_aim_page_prompt_fragment' => [ + $this->liveAssignmentUid => ['delete' => 1], + ], + ]); + $repository = $this->get(PagePromptFragmentRepository::class); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(1)); + self::assertSame([], $repository->findAssignmentsForPage(1)); + self::assertSame(['total' => 0, 'inherited' => 0], $repository->countAllAssignments()); + + $this->get(Context::class)->setAspect('workspace', new WorkspaceAspect(0)); + self::assertCount(1, $repository->findAssignmentsForPage(1)); + } + + /** + * @param array>> $dataMap + * @param array>> $commandMap + */ + private function processInWorkspace(array $dataMap, array $commandMap = []): void + { + $backendUser = $this->setUpBackendUser(1); + $backendUser->setWorkspace(1); + $dataHandler = GeneralUtility::makeInstance(DataHandler::class); + $dataHandler->start($dataMap, $commandMap, $backendUser); + $dataHandler->process_datamap(); + $dataHandler->process_cmdmap(); + self::assertSame([], $dataHandler->errorLog, implode('; ', $dataHandler->errorLog)); + } + + private function countVersions(string $table, int $liveUid): int + { + return $this->getConnectionPool()->getConnectionForTable($table) + ->count('uid', $table, ['t3ver_oid' => $liveUid, 't3ver_wsid' => 1]); + } +} diff --git a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php index fabb350..90f3d49 100644 --- a/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php +++ b/Tests/Functional/Domain/Repository/PagePromptFragmentRepositoryTest.php @@ -37,6 +37,10 @@ */ final class PagePromptFragmentRepositoryTest extends FunctionalTestCase { + protected array $coreExtensionsToLoad = [ + 'workspaces', + ]; + protected array $testExtensionsToLoad = [ 'b13/aim', ]; diff --git a/Tests/Unit/Configuration/TcaPromptFragmentWorkspaceVersioningTest.php b/Tests/Unit/Configuration/TcaPromptFragmentWorkspaceVersioningTest.php deleted file mode 100644 index f2adc58..0000000 --- a/Tests/Unit/Configuration/TcaPromptFragmentWorkspaceVersioningTest.php +++ /dev/null @@ -1,41 +0,0 @@ - ['tx_aim_prompt_fragment']; - yield 'tx_aim_page_prompt_fragment' => ['tx_aim_page_prompt_fragment']; - } - - #[Test] - #[DataProvider('tableDataProvider')] - public function tableIsVersionedInWorkspacesWithoutLiveEdit(string $table): void - { - $tca = require __DIR__ . '/../../../Configuration/TCA/' . $table . '.php'; - - self::assertTrue($tca['ctrl']['versioningWS'] ?? false); - self::assertArrayNotHasKey('versioningWS_alwaysAllowLiveEdit', $tca['ctrl']); - } -} diff --git a/composer.json b/composer.json index c609579..df2ae4f 100644 --- a/composer.json +++ b/composer.json @@ -24,6 +24,7 @@ "saschaegerer/phpstan-typo3": "^3.1", "typo3/coding-standards": "^0.9.0", "typo3/cms-dashboard": "^12.4 || ^13.4 || ^14.0", + "typo3/cms-workspaces": "^12.4 || ^13.4 || ^14.0", "symfony/http-client-contracts": "^3.7" }, "conflict": {