Skip to content

fix(entries): exclude current entry from slug uniqueness on update (#27) #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sylvesterdamgaard merged 1 commit into from
May 5, 2026
Merged

fix(entries): exclude current entry from slug uniqueness on update (#27) #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion src/Mcp/Tools/Routers/EntriesRouter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> $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()) {
Expand Down Expand Up @@ -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<string, mixed> $mergedData */
$mergedData = array_merge($entry->data()->all(), $data);
$mergedData['slug'] = $entry->slug();
$mergedData = $this->sanitizeStoredFieldDataForValidation($blueprint, $mergedData);

$validationContext = [
Expand Down
137 changes: 137 additions & 0 deletions tests/Feature/Routers/EntriesRouterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
Loading