fix: Theme Builder minors of the final review (M-4, M-5, M-6, M-9, M-10, M-11, M-12 c)

- tb-list applies the detached limit after the edit filter (reads up to 500 IDs) and notes when more are not listed (M-4); an invalid template_id is invalid input, shared with ThemeBuilderAccess::template() (M-5). - Templates carry their status; tb-list notes that a live template outside publish (e.g. trashed) can only be switched off (M-10). - changeableTemplate() refuses without canUse() first, like tb-list and tb-create-template; the layout/template refusal difference is documented (M-9). - tb-create-template reads the default template from editableLiveTemplates() (M-11); the condition-ID character set (role slugs) is documented (M-6); tb-assign documents that a failed reattach keeps Divi's unused marks removed (M-12 c). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
2026-09-27 23:12:14 +02:00
parent e7bbe1e9d7
commit 14bc954ecc
10 changed files with 176 additions and 18 deletions
@@ -94,6 +94,70 @@ final class TbListAbilityTest extends TestCase
        self::assertToolError(self::ability($store)->execute(['template_id' => 4711]), 'not_found');
    }

+    /** Review M-5: an invalid template_id is named as invalid, not as template 0. */
+    public function testAnInvalidTemplateIdIsInvalidInput(): void
+    {
+        $result = self::ability(ThemeBuilderFixtures::store([ThemeBuilderFixtures::template(1065)]))
+            ->execute(['template_id' => 'abc']);
+
+        self::assertToolError($result, 'validation_failed');
+        self::assertSame('template_id fehlt oder ist keine ID.', self::clientError($result)['message']);
+    }
+
+    /**
+     * Review M-4: the detached limit applies to the templates the user may edit – foreign detached ones do not
+     * crowd them out – and a note says when more are not listed.
+     */
+    public function testTheDetachedLimitCountsOnlyEditableTemplates(): void
+    {
+        $foreign = range(3000, 3000 + TbListAbility::DETACHED_LIMIT - 1);
+        $own = range(4000, 4000 + TbListAbility::DETACHED_LIMIT);
+        $detached = array_map(
+            static fn (int $id): \LIT\MCP\Integrations\Divi\ThemeBuilder\Template => ThemeBuilderFixtures::template($id, ['attached' => false]),
+            array_merge($foreign, $own)
+        );
+        $store = ThemeBuilderFixtures::store([], 0, $detached);
+        $asked = [];
+        $store->allows('detachedIds')->andReturnUsing(static function (int $limit) use (&$asked, $detached): array {
+            $asked[] = $limit;
+
+            return array_slice(array_map(static fn ($t): int => $t->id, $detached), 0, $limit);
+        });
+        self::denyEditing($foreign);
+
+        $result = self::ability($store)->execute([]);
+
+        self::assertSame(array_slice($own, 0, TbListAbility::DETACHED_LIMIT), array_column($result['detached'], 'id'));
+        self::assertStringContainsString('Weitere entfernte Vorlagen nicht aufgeführt', implode(' ', $result['notes']));
+        self::assertSame([TbListAbility::DETACHED_SCAN], $asked);
+
+        $few = ThemeBuilderFixtures::store([], 0, [ThemeBuilderFixtures::template(1066, ['attached' => false])]);
+        self::assertStringNotContainsString(
+            'Weitere entfernte Vorlagen',
+            implode(' ', self::ability($few)->execute([])['notes'])
+        );
+    }
+
+    /**
+     * Review M-10: a live template in the trash is listed with its status and the note that it can only be switched
+     * off (tb-assign and switching on read it as missing).
+     */
+    public function testATrashedLiveTemplateSaysItCanOnlyBeSwitchedOff(): void
+    {
+        $store = ThemeBuilderFixtures::store([ThemeBuilderFixtures::template(1066, ['enabled' => true, 'status' => 'trash'])]);
+
+        $result = self::ability($store)->execute([]);
+
+        self::assertSame('trash', $result['templates'][0]['status']);
+        self::assertStringContainsString('lit/tb-activate enabled: false', implode(' ', $result['notes']));
+        self::assertStringContainsString('1066', implode(' ', $result['notes']));
+
+        $published = ThemeBuilderFixtures::store([ThemeBuilderFixtures::template(1066, ['enabled' => true])]);
+        $plain = self::ability($published)->execute([]);
+        self::assertSame('publish', $plain['templates'][0]['status']);
+        self::assertStringNotContainsString('Papierkorb', implode(' ', $plain['notes']));
+    }
+
    public function testNeedsTheThemeBuilderRole(): void
    {
        $ability = self::ability(ThemeBuilderFixtures::store(), ['theme_builder']);
@@ -208,6 +208,31 @@ final class ThemeBuilderAccessTest extends TestCase
        );
    }

+    /**
+     * Review M-9: without the Theme Builder rights tb-assign and tb-activate refuse like tb-list and
+     * tb-create-template (forbidden, independent of the ID) instead of "Vorlage X gibt es nicht".
+     */
+    public function testChangeableTemplateRefusesWithoutTheThemeBuilderRightsFirst(): void
+    {
+        LibraryFixtures::given([ThemeBuilderFixtures::templatePost(1066)]);
+        $store = ThemeBuilderFixtures::store([ThemeBuilderFixtures::template(1066)]);
+        $rev = ThemeBuilderFixtures::template(1066)->rev();
+
+        foreach ([1066, 4711, 'abc'] as $id) {
+            $refused = ThemeBuilderFixtures::access(['theme_builder'], true, $store)
+                ->changeableTemplate(['template_id' => $id, 'template_rev' => $rev]);
+            self::assertToolError($refused, 'forbidden');
+            self::assertStringNotContainsString((string) $id, self::clientError($refused)['message']);
+        }
+        foreach ([true, false] as $offSwitch) {
+            self::assertToolError(
+                ThemeBuilderFixtures::access(['theme_builder'], true, $store)
+                    ->changeableTemplate(['template_id' => 1066, 'template_rev' => $rev], $offSwitch),
+                'forbidden'
+            );
+        }
+    }
+
    /** Ruling Q18: a template the user may not edit answers like a missing one, also for writing. */
    public function testChangeableTemplateHidesTemplatesTheUserMayNotEdit(): void
    {
@@ -242,13 +267,19 @@ final class ThemeBuilderAccessTest extends TestCase
            ThemeBuilderFixtures::access([], true, ThemeBuilderFixtures::store([
                ThemeBuilderFixtures::template(1066, ['status' => 'trash']),
            ])),
-            ThemeBuilderFixtures::access(['theme_builder'], true, ThemeBuilderFixtures::store([$trashed])),
+            ThemeBuilderFixtures::access([], true, ThemeBuilderFixtures::store([$trashed]), false),
        ];
        foreach ($notLive as $index => $other) {
            $refused = $other->changeableTemplate($input, true);
            self::assertToolError($refused, 'not_found', (string) $index);
            self::assertSame(self::clientError($missing), self::clientError($refused), (string) $index);
        }
+        // Review M-9: without the Theme Builder rights the refusal does not depend on the ID.
+        self::assertToolError(
+            ThemeBuilderFixtures::access(['theme_builder'], true, ThemeBuilderFixtures::store([$trashed]))
+                ->changeableTemplate($input, true),
+            'forbidden'
+        );

        self::assertToolError(
            ThemeBuilderFixtures::access([], false, ThemeBuilderFixtures::store([$trashed]))->changeableTemplate($input, true),