diff --git a/src/Mcp/Tools/Routers/EntriesRouter.php b/src/Mcp/Tools/Routers/EntriesRouter.php index d92d251..710c293 100644 --- a/src/Mcp/Tools/Routers/EntriesRouter.php +++ b/src/Mcp/Tools/Routers/EntriesRouter.php @@ -455,6 +455,36 @@ private function updateEntry(array $arguments): array unset($data['published']); } + // Handle slug separately *before* blueprint validation so the + // FieldsValidator never sees the slug column. Letting the merged + // payload include the slug forces UniqueEntryValue to compare the + // current slug against the entry being updated and reject it as + // "already taken" — even when the caller never asked to change + // the slug. Mirrors the createEntry() flow but passes the + // entry's id as the $except argument so the rule excludes the + // current entry from the uniqueness check. + if (array_key_exists('slug', $data)) { + $newSlug = is_string($data['slug']) ? $data['slug'] : ''; + $slugValidator = Validator::make(['slug' => $newSlug], [ + 'slug' => [ + 'required', + 'string', + new UniqueEntryValue($entry->collectionHandle(), $entry->id(), $site), + ], + ]); + + if ($slugValidator->fails()) { + $errors = $slugValidator->errors()->get('slug'); + /** @var array $flatErrors */ + $flatErrors = array_map(fn ($error) => is_string($error) ? $error : implode(', ', (array) $error), $errors); + + return $this->createErrorResponse('Slug validation failed: ' . implode(', ', $flatErrors))->toArray(); + } + + $entry->slug($newSlug); + unset($data['slug']); + } + // For dated collections, parse the date and set it on the entry. // Keep a normalized copy in data so the FieldsValidator sees the required field. if (array_key_exists('date', $data) && $entry->collection()->dated()) { @@ -487,9 +517,14 @@ private function updateEntry(array $arguments): array // TypeError (common with third-party fieldtypes like SEO Pro // whose preProcessValidatable can't handle stored data formats), // we fall back to validating only the incoming fields. + // NOTE: do NOT inject slug into mergedData. Any slug change + // was already applied to the entry object above; the + // FieldsValidator does not need the slug column and including + // it forces UniqueEntryValue to reject the current entry's + // own slug. See the slug-handling block earlier in this + // method (issue #27). /** @var array $mergedData */ $mergedData = array_merge($entry->data()->all(), $data); - $mergedData['slug'] = $entry->slug(); $mergedData = $this->sanitizeStoredFieldDataForValidation($blueprint, $mergedData); $validationContext = [ diff --git a/tests/Feature/Routers/EntriesRouterTest.php b/tests/Feature/Routers/EntriesRouterTest.php index 64de092..8884333 100644 --- a/tests/Feature/Routers/EntriesRouterTest.php +++ b/tests/Feature/Routers/EntriesRouterTest.php @@ -184,6 +184,143 @@ public function test_create_entry_without_slug_generates_from_title(): void $this->assertEquals('auto-slug-generation-test', $result['data']['entry']['slug']); } + /** + * Regression for https://github.com/cboxdk/statamic-mcp/issues/27. + * + * The previous updateEntry() flow injected the entry's current + * slug into the FieldsValidator payload, which made + * UniqueEntryValue compare the slug against the entry being + * updated and reject it as "already taken" — even when the caller + * never passed a slug at all. + */ + public function test_update_entry_without_slug_in_data_succeeds(): void + { + $entry = Entry::make() + ->collection($this->collectionHandle) + ->slug("update-no-slug-{$this->testId}") + ->data(['title' => 'Original Title']); + $entry->save(); + + $result = $this->router->execute([ + 'action' => 'update', + 'collection' => $this->collectionHandle, + 'id' => $entry->id(), + 'data' => [ + 'title' => 'Updated Title', + ], + ]); + + $this->assertTrue( + $result['success'], + 'updating an entry without changing the slug must succeed; got: ' + . json_encode($result['errors'] ?? []), + ); + + $reloaded = Entry::find($entry->id()); + $this->assertSame('Updated Title', $reloaded->get('title')); + $this->assertSame("update-no-slug-{$this->testId}", $reloaded->slug()); + } + + /** + * Resending the entry's *current* slug as part of `data` is + * idempotent and must not trigger UniqueEntryValue against the + * entry itself. + */ + public function test_update_entry_with_unchanged_slug_succeeds(): void + { + $slug = "update-same-slug-{$this->testId}"; + $entry = Entry::make() + ->collection($this->collectionHandle) + ->slug($slug) + ->data(['title' => 'Original']); + $entry->save(); + + $result = $this->router->execute([ + 'action' => 'update', + 'collection' => $this->collectionHandle, + 'id' => $entry->id(), + 'data' => [ + 'title' => 'Updated', + 'slug' => $slug, + ], + ]); + + $this->assertTrue( + $result['success'], + 'resending the existing slug must be idempotent; got: ' + . json_encode($result['errors'] ?? []), + ); + + $reloaded = Entry::find($entry->id()); + $this->assertSame($slug, $reloaded->slug()); + $this->assertSame('Updated', $reloaded->get('title')); + } + + /** + * Renaming an entry to a slug that does not collide with any + * other entry in the collection succeeds. + */ + public function test_update_entry_with_new_unique_slug_succeeds(): void + { + $entry = Entry::make() + ->collection($this->collectionHandle) + ->slug("update-rename-from-{$this->testId}") + ->data(['title' => 'Will Be Renamed']); + $entry->save(); + + $newSlug = "update-rename-to-{$this->testId}"; + $result = $this->router->execute([ + 'action' => 'update', + 'collection' => $this->collectionHandle, + 'id' => $entry->id(), + 'data' => [ + 'slug' => $newSlug, + ], + ]); + + $this->assertTrue( + $result['success'], + 'updating to a unique new slug must succeed; got: ' + . json_encode($result['errors'] ?? []), + ); + + $reloaded = Entry::find($entry->id()); + $this->assertSame($newSlug, $reloaded->slug()); + } + + /** + * Renaming an entry to a slug that another entry already owns + * must surface the validation error. + */ + public function test_update_entry_to_existing_slug_returns_error(): void + { + $other = Entry::make() + ->collection($this->collectionHandle) + ->slug("update-collision-{$this->testId}") + ->data(['title' => 'Other']); + $other->save(); + + $entry = Entry::make() + ->collection($this->collectionHandle) + ->slug("update-target-{$this->testId}") + ->data(['title' => 'Target']); + $entry->save(); + + $result = $this->router->execute([ + 'action' => 'update', + 'collection' => $this->collectionHandle, + 'id' => $entry->id(), + 'data' => [ + 'slug' => $other->slug(), + ], + ]); + + $this->assertFalse( + $result['success'], + 'updating to a slug already owned by another entry must fail', + ); + } + public function test_update_nonexistent_entry_returns_error(): void { $result = $this->router->execute([