Skip to content

fix(entries): satisfy required slug/date rules from the entry's own values #40

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
Aug 27, 2026
Merged

fix(entries): satisfy required slug/date rules from the entry's own values #40

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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **`SECURITY.md`** — Private vulnerability reporting via GitHub, an explicit split between what this addon secures and what the operator does, and a limitations section stating plainly that the audit log is append-only by convention with no hash chain (neither tamper-proof nor tamper-evident), that confirmation tokens are replayable within their window, and that `require_https` falls back to off when the published config predates the key

### Fixed
- **Entry updates no longer fail on blueprints with a required slug** (#39) — The #27 fix removed the slug from the validated payload entirely, so Statamic's default slug field (`validate: [required, UniqueEntryValue…]`) could never be satisfied: every update failed with "The Slug field is required", whether the caller omitted the slug or resent the current one. The entry's effective slug is now injected back into the validation payload, and both `FieldsValidator` invocations (including the `TypeError` fallback) resolve the `UniqueEntryValue({collection}, {id}, {site})` placeholders via `withReplacements()`, so the rule excludes the entry being updated — the false positive #27 was about — while a slug owned by another entry is still rejected
- **`date` no longer has to be resent on every update of a dated collection** — Like the slug, the date is an entry property absent from the merged data payload, so a blueprint with a required date field failed any update that did not repeat a date the caller never meant to change. The entry's current date now satisfies the rule when the payload omits it
- **Explicit slug on create actually works** — `createEntry()` read `$arguments['slug']`, but the tool schema never declared the parameter, so no client could send it and the slug was always derived from the title. The schema now declares `slug`, and create also accepts it as `data.slug` — the shape update uses — storing it as an entry property in both cases, never as a data key
- **PHPStan level 9 is no longer partly disabled** — The config carried blanket `ignoreErrors` patterns (`#Method .* should return .* but returns mixed#`, `#Cannot call method .* on .*\|null#`, `#Parameter .* expects .*, mixed given#`, and four more) that suppressed whole error classes across `src/`, so "level 9 clean" meant considerably less than it sounded. Removing them surfaced 54 real errors — almost all method calls on `Blueprint|null`, `Entry|null`, or `GlobalSet|null` after a lookup, because `requireResource()` returned an error array without narrowing the variable. Every site now checks for null explicitly (identical messages, identical behaviour, and the analyser can see it), the untyped Statamic/Eloquent return values are narrowed rather than cast, and the four `@phpstan-ignore` annotations on `abort()` calls are gone. `requireResource()` itself is removed, having no callers left
- **CI verifies formatting instead of rewriting it** — The Tests workflow ran Pint in fix mode, committed the result, and pushed it back to the branch; it now runs `pint --test` and fails on violations, matching what the release workflow already does
- **CI exercises both Laravel majors** — `composer.json` claims Laravel 12 and 13 via `orchestra/testbench: ^10.0 || ^11.0`, but the matrix only varied PHP, so every job resolved testbench 11 and the Laravel 12 claim was never verified. The matrix now spans both. The suite passes on Laravel 12
Expand Down
74 changes: 56 additions & 18 deletions src/Mcp/Tools/Routers/EntriesRouter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ protected function defineSchema(JsonSchemaContract $schema): array
. 'blueprint handle to see required fields, types, and nesting before sending data.'
),

'slug' => JsonSchema::string()
->description('Entry slug for the create action. Defaults to a slug generated from the title. For update, pass "slug" inside data instead'),

'filters' => JsonSchema::object()
->description('Filter conditions as key-value pairs. Keys are field handles from the blueprint. Example: {"status": "published"}'),

Expand Down Expand Up @@ -350,9 +353,13 @@ private function createEntry(array $arguments): array
->collection($collection)
->locale($site);

// Set slug from arguments or generate from title
if (! empty($arguments['slug'])) {
$requestedSlug = $arguments['slug'];
// Set slug from arguments (or data, for callers that send it as a
// field value) or generate from title. Slug is an entry property,
// not a data key, so it never stays in $data.
$requestedSlug = $arguments['slug'] ?? $data['slug'] ?? null;
unset($data['slug']);

if (! empty($requestedSlug)) {
$requestedSlug = is_string($requestedSlug) ? $requestedSlug : '';

// Use Statamic's built-in validation for unique slugs
Expand Down Expand Up @@ -421,13 +428,22 @@ private function createEntry(array $arguments): array
try {
$fields = $blueprint->fields()->addValues($dataWithSlug);

// Resolve blueprint-level rule placeholders — Statamic's
// default slug field carries
// `new UniqueEntryValue({collection}, {id}, {site})`,
// which needs {collection} and {site} filled in ({id}
// resolves to null: nothing to exclude on create).
(new FieldsValidator)
->fields($fields)
->withContext([
'entry' => $entry,
'collection' => $collection,
'site' => $site,
])
->withReplacements([
'collection' => $collection->handle(),
'site' => $site,
])
->validate();

// Process through fieldtypes (Terms strips prefixes,
Expand Down Expand Up @@ -530,14 +546,14 @@ 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.
// Handle slug separately *before* blueprint validation. Slug is
// an entry property, not a data key, so a requested change is
// applied to the entry object and removed from $data. This
// dedicated check passes the entry's id as the $except argument,
// so it gives a precise error without rejecting the entry's own
// slug as "already taken" (issue #27). The blueprint validation
// below re-validates the applied slug with resolved rule
// placeholders.
if (array_key_exists('slug', $data)) {
$newSlug = is_string($data['slug']) ? $data['slug'] : '';
$slugValidator = Validator::make(['slug' => $newSlug], [
Expand Down Expand Up @@ -592,26 +608,47 @@ 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 = $this->sanitizeStoredFieldDataForValidation($blueprint, $mergedData);

// Slug and date are entry properties, not data keys, so the
// merged payload never contains them on its own. Statamic's
// default blueprint declares slug as `required`, so validation
// must see the entry's effective values or every update fails
// with "The Slug field is required" (issue #39). Safe to
// re-validate the slug: withReplacements() below resolves
// `UniqueEntryValue({collection}, {id}, {site})` with this
// entry's id, excluding it from the uniqueness check — the
// exact false positive #27 was about.
$entryPropertyValues = [];
$entrySlug = $entry->slug();
if (is_string($entrySlug) && $entrySlug !== '') {
$entryPropertyValues['slug'] = $entrySlug;
}
$entryDate = $entry->date();
if (! array_key_exists('date', $data) && $entry->collection()->dated() && $entryDate !== null) {
$entryPropertyValues['date'] = $entryDate->format('Y-m-d\TH:i:s.v\Z');
}
$mergedData = array_merge($mergedData, $entryPropertyValues);

$validationContext = [
'entry' => $entry,
'collection' => $entry->collection(),
'site' => $site,
];

$ruleReplacements = [
'id' => $entry->id(),
'collection' => $entry->collectionHandle(),
'site' => $site,
];

try {
(new FieldsValidator)
->fields($blueprint->fields()->addValues($mergedData))
->withContext($validationContext)
->withReplacements($ruleReplacements)
->validate();
} catch (ValidationException $e) {
return $this->formatValidationError($e);
Expand All @@ -623,8 +660,9 @@ private function updateEntry(array $arguments): array
// already valid when it was saved.
try {
(new FieldsValidator)
->fields($blueprint->fields()->addValues($data))
->fields($blueprint->fields()->addValues(array_merge($data, $entryPropertyValues)))
->withContext($validationContext)
->withReplacements($ruleReplacements)
->validate();
} catch (ValidationException $inner) {
return $this->formatValidationError($inner);
Expand Down
Loading
Loading