Repository navigation
feat: C3 · Global styles model, migration, API, schema pin - #364
Merged
ViewFromTheBox merged 3 commits intoApr 23, 2026
Merged
Conversation
Lands the server-side backing for the globalStyles singleton the B1 core-data shim addresses at /visual-editor/api/global-styles: - visual_editor_global_styles table (version, settings, styles, theme — unique) + VisualEditorGlobalStyles model with a resolveSingleton() helper that lazily seeds the row on first lookup. - Four endpoints — lookup, show, update, base — mounted under /visual-editor/api/global-styles behind the auth middleware stack. Routes are declared in the order required for the static segments to win over wildcard model binding. - UpdateGlobalStylesRequest validates against the pinned theme.json schema version (configurable via artisanpack.visual-editor.global_styles.schema_version), required settings / styles shape, palette entry fields, and unique preset slugs across color / typography families. The controller persists the full settings / styles blob verbatim so extensible theme.json keys (layout, spacing, etc.) are not silently dropped by validated(). - Default base payload bundled at resources/theme-json/default-base.php; host apps can swap it via a config path override. - VisualEditorGlobalStylesPolicy gates access (auth required). - docs/global-styles.md documents the pinned schema version (3), what "compatible" means, and the steps we take when upstream theme.json bumps. - 18 Pest feature tests cover lookup idempotency + theme scoping, show / 404, update happy path + 422s (missing version, mismatched version, missing settings / styles, duplicate palette slugs, malformed palette entry), base payload shape, base_path config override, and B2 global- styles fixture round-trip. Auth gating verified on every endpoint. Closes #359
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds server-side support for a singleton, theme.json-shaped Changes
Sequence DiagramsequenceDiagram
actor Client
participant Controller as GlobalStylesController
participant Validator as UpdateGlobalStylesRequest
participant Model as VisualEditorGlobalStyles
participant DB as Database
participant Resource as GlobalStylesResource
Client->>Controller: PUT /visual-editor/api/global-styles/{id}
Controller->>Validator: validate($request)
Validator->>Validator: ensure version == config schema_version
Validator->>Validator: ensure settings/styles arrays and unique preset slugs
alt validation fails
Validator-->>Client: 422 Unprocessable Entity (errors)
else validation passes
Validator-->>Controller: validated data
Controller->>Model: set version, settings, styles
Model->>DB: save()
DB-->>Model: persisted record
Controller->>Resource: new GlobalStylesResource(model)
Resource-->>Client: 200 OK (id, version, settings, styles)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/theme-json/default-base.php`:
- Around line 7-8: The header comment references the wrong config key name;
update the comment that mentions `artisanpack.visual-editor.global_styles.base`
to the correct key `artisanpack.visual-editor.global_styles.base_path` so
host-app overrides and docs reference the actual config entry used by the code
(look for the header/comment block in resources/theme-json/default-base.php and
replace the mistaken symbol).
In `@src/Http/Controllers/GlobalStylesController.php`:
- Around line 56-59: The singleton resolution in
VisualEditorGlobalStyles::resolveSingleton (used by lookup()) is vulnerable to
race conditions because it uses firstOrCreate() on the unique theme key; wrap
the resolution in a DB transaction and perform a SELECT ... FOR UPDATE (e.g.,
use DB::transaction + lockForUpdate() on a query for the theme) before calling
firstOrCreate(), or replace the create path with a single atomic upsert call
supported by your driver so that concurrent cold-start requests cannot both
attempt to insert the same theme for activeTheme() / basePayload().
In `@src/Http/Requests/UpdateGlobalStylesRequest.php`:
- Around line 55-56: The current validation for settings.typography.fontFamilies
and .fontSizes only enforces array + unique slugs, so incomplete items slip
through; update the request validation in UpdateGlobalStylesRequest.php to
require each array item contains the expected keys (e.g. for fontFamilies
require each array.*.name and array.*.fontFamily as required strings; for
fontSizes require array.*.name and array.*.size as required numbers/strings as
appropriate) in addition to keeping the existing
'settings.typography.fontFamilies'/'settings.typography.fontSizes' array rules
and the $this->uniqueSlugRule('fontFamilies'|'fontSizes') checks so entries must
be fully shaped like theme.json presets.
In `@tests/Feature/VisualEditor/GlobalStylesControllerTest.php`:
- Around line 258-270: The temp file is leaked because the code appends ".php"
to the path returned by tempnam() and only unlinks the appended path; change the
creation to keep tempnam()'s returned path (i.e. set $override =
tempnam(sys_get_temp_dir(), 'base-') without appending '.php'), write the PHP
return contents into that same $override with file_put_contents, leave the
existing try/finally that calls `@unlink`($override), and keep the
config()->set('artisanpack.visual-editor.global_styles.base_path', $override)
and the getJson('/visual-editor/api/global-styles/base') assertions as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c78862e-c194-4e5e-8933-56d42843a6ca
📒 Files selected for processing (12)
config/visual-editor.phpdatabase/migrations/2026_04_23_200000_create_visual_editor_global_styles_table.phpdocs/global-styles.mdresources/theme-json/default-base.phproutes/api.phpsrc/Http/Controllers/GlobalStylesController.phpsrc/Http/Requests/UpdateGlobalStylesRequest.phpsrc/Http/Resources/GlobalStylesResource.phpsrc/Models/VisualEditorGlobalStyles.phpsrc/Policies/VisualEditorGlobalStylesPolicy.phpsrc/VisualEditorServiceProvider.phptests/Feature/VisualEditor/GlobalStylesControllerTest.php
Comment on lines
+56
to
+59
| $record = VisualEditorGlobalStyles::resolveSingleton( | ||
| $this->activeTheme(), | ||
| $this->basePayload() | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
model="$(fd -a 'VisualEditorGlobalStyles.php' src | head -n1)"
migration="$(fd -a 'visual_editor_global_styles' database | head -n1)"
test -n "$model"
test -n "$migration"
echo "== resolveSingleton() =="
nl -ba "$model" | sed -n '70,90p'
echo
echo "== visual_editor_global_styles migration =="
nl -ba "$migration" | sed -n '1,200p'Repository: ArtisanPack-UI/visual-editor
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the model file
model=$(fd -a 'VisualEditorGlobalStyles.php' src 2>/dev/null | head -n1)
echo "Model file: $model"
# Find the migration file
migration=$(fd -a 'visual_editor_global_styles' database 2>/dev/null | head -n1)
echo "Migration file: $migration"
if [ -z "$model" ] || [ -z "$migration" ]; then
echo "Files not found, searching more broadly..."
echo "=== Searching for VisualEditorGlobalStyles ==="
find . -name '*VisualEditorGlobalStyles*' -type f 2>/dev/null | head -20
echo "=== Searching for visual_editor_global_styles ==="
find . -name '*visual_editor_global_styles*' -type f 2>/dev/null | head -20
fi
# Also check the GlobalStylesController
echo "=== Searching for GlobalStylesController ==="
find . -name '*GlobalStylesController*' -type f 2>/dev/nullRepository: ArtisanPack-UI/visual-editor
Length of output: 415
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== GlobalStylesController.php (lines 40-75) ==="
cat -n ./src/Http/Controllers/GlobalStylesController.php | head -75 | tail -36
echo ""
echo "=== VisualEditorGlobalStyles.php (entire file) ==="
wc -l ./src/Models/VisualEditorGlobalStyles.php
echo ""
echo "=== VisualEditorGlobalStyles.php content ==="
cat -n ./src/Models/VisualEditorGlobalStyles.php
echo ""
echo "=== Migration file content ==="
cat -n ./database/migrations/2026_04_23_200000_create_visual_editor_global_styles_table.phpRepository: ArtisanPack-UI/visual-editor
Length of output: 7081
Make singleton resolution race-safe.
lookup() currently relies on resolveSingleton(), which uses firstOrCreate() on a unique theme key. Two cold-start requests can both miss the row and one will 500 on the unique index instead of returning the same singleton.
Consider wrapping the resolution in database-level locking (e.g., firstOrCreate() within a transaction with lockForUpdate()) or using an upsert operation if supported by your database driver, to ensure the operation is atomic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Http/Controllers/GlobalStylesController.php` around lines 56 - 59, The
singleton resolution in VisualEditorGlobalStyles::resolveSingleton (used by
lookup()) is vulnerable to race conditions because it uses firstOrCreate() on
the unique theme key; wrap the resolution in a DB transaction and perform a
SELECT ... FOR UPDATE (e.g., use DB::transaction + lockForUpdate() on a query
for the theme) before calling firstOrCreate(), or replace the create path with a
single atomic upsert call supported by your driver so that concurrent cold-start
requests cannot both attempt to insert the same theme for activeTheme() /
basePayload().
- Correct the header comment in resources/theme-json/default-base.php to reference the actual config key — artisanpack.visual-editor. global_styles.base_path — that the controller honors for base-payload overrides. The earlier `.base` was a stale symbol. - Tighten UpdateGlobalStylesRequest so typography preset entries must carry their full shape: fontFamilies entries now require slug, name, and fontFamily; fontSizes entries require slug, name, and size. The palette already enforced per-item required keys, and the typography presets are addressed the same way (CSS variables keyed by slug), so partial entries would have silently produced broken var(--…) tokens. - Plug a temp-file leak in the base_path override test. tempnam() creates the file at the path it returns; appending ".php" before writing meant the finally block unlinked a sibling file and orphaned the tempnam-created stub. Dropped the extension append — require does not care. - Document that the cold-start race on VisualEditorGlobalStyles:: resolveSingleton() is already covered by Laravel's race-safe firstOrCreate (which catches UniqueConstraintViolationException and re-queries) plus the unique theme index, so future reviewers do not re-raise the concern. Two new feature tests exercise the fontFamilies / fontSizes malformed- entry cases. Full suite: 273 passed (828 assertions).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Http/Requests/UpdateGlobalStylesRequest.php (1)
95-95: Silence the unused-parameter warning in the closure signature.
$attributeis required by the rule signature but currently unused; renaming to$_attributekeeps intent clear and avoids PHPMD noise.Suggested tweak
- return function ( string $attribute, mixed $value, Closure $fail ) use ( $label ): void { + return function ( string $_attribute, mixed $value, Closure $fail ) use ( $label ): void {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Http/Requests/UpdateGlobalStylesRequest.php` at line 95, The closure returned in UpdateGlobalStylesRequest:: (the function signature starting "function ( string $attribute, mixed $value, Closure $fail ) use ( $label )") should rename the unused $attribute parameter to $_attribute to silence PHPMD/unused-parameter warnings; update the closure signature to "function ( string $_attribute, mixed $value, Closure $fail ) use ( $label )" and leave the rest of the closure body unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Models/VisualEditorGlobalStyles.php`:
- Line 86: Replace the hardcoded fallback version on the 'version' assignment in
VisualEditorGlobalStyles (the line "'version' => isset( $defaults['version'] )
? (int) $defaults['version'] : 3") so it reads the canonical schema version from
configuration; use
config('artisanpack.visual-editor.global_styles.schema_version') (cast to int)
as the fallback (optionally with a numeric default in config(...) to preserve
safety) instead of the literal 3.
---
Nitpick comments:
In `@src/Http/Requests/UpdateGlobalStylesRequest.php`:
- Line 95: The closure returned in UpdateGlobalStylesRequest:: (the function
signature starting "function ( string $attribute, mixed $value, Closure $fail )
use ( $label )") should rename the unused $attribute parameter to $_attribute to
silence PHPMD/unused-parameter warnings; update the closure signature to
"function ( string $_attribute, mixed $value, Closure $fail ) use ( $label )"
and leave the rest of the closure body unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 24b8f598-0ca8-4f29-b68e-668fb1b29f4d
📒 Files selected for processing (4)
resources/theme-json/default-base.phpsrc/Http/Requests/UpdateGlobalStylesRequest.phpsrc/Models/VisualEditorGlobalStyles.phptests/Feature/VisualEditor/GlobalStylesControllerTest.php
✅ Files skipped from review due to trivial changes (1)
- resources/theme-json/default-base.php
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Feature/VisualEditor/GlobalStylesControllerTest.php
The singleton auto-create path was falling back to a literal `3` when the caller did not supply a `version` in the seed payload, while the controller and form-request already read artisanpack.visual-editor.global_styles.schema_version from config. Unified the three callers on the same config key so the pinned version lives in exactly one place — the config default still reads 3, so behavior is unchanged for default installs.
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
15 tasks done
13 of 20 tasks
3 of 4 tasks
14 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Lands the server-side backing for the
globalStylessingleton the B1 core-data shim (#353) addresses at/visual-editor/api/global-styles. The record is a theme.json-shaped{ version, settings, styles }blob, scoped by theme to support future multi-theme hosting. Four endpoints —lookup,show,update,base— unblock the D3 site-editor style UI and E1 front-end CSS emission.Closes: #359
Type of Change
Related Issue
Issue: #359
Motivation and Context
The B1 core-data shim dispatches three endpoint patterns against
globalStyles—lookup, CRUD on the singleton, and a separatebaseendpoint for the theme defaults the site-editor style UI diffs against. Phase C is the backend work that makes each entity's endpoints return real payloads; C3 is the global-styles slice.Per §4.2 of the V1 plan doc, the theme.json schema is pinned to a specific version so upstream
@wordpress/*bumps do not silently drift the schema. The pin is recorded as configurable data (artisanpack.visual-editor.global_styles.schema_version, default3) and enforced by thePUTform-request, not a hard-coded constant.Changes Made
visual_editor_global_styles(version,settings,styles,themeunique).VisualEditorGlobalStylesmodel with aresolveSingleton($theme, $defaults)helper that lazily seeds the singleton row on first lookup — no manual seeder needed.GlobalStylesControllerwithlookup,show,update,baseactions; routes declared so static segments (lookup,base) win over the wildcard model-binding segment.UpdateGlobalStylesRequestvalidates: pinned-version match, requiredsettings/styles, palette entry shape, unique preset slugs across color/typography families.settings/stylesblob verbatim via$request->input()— usingvalidated()would silently strip extensible theme.json keys (layout,spacing, etc.) that the form-request doesn't enforce shape on.resources/theme-json/default-base.php; host apps can override viaartisanpack.visual-editor.global_styles.base_path.VisualEditorGlobalStylesPolicygates access (auth required on every endpoint).docs/global-styles.mddocuments the pinned v3 schema, what "compatible" means, and the steps for a future schema bump.How Has This Been Tested?
Testing Environment:
release/1.0Tests Performed:
./vendor/bin/pest --compact→ 271 passed (822 assertions)../vendor/bin/pest tests/Feature/VisualEditor/GlobalStylesControllerTest.php→ 18 passed (72 assertions).Accessibility Tests Run
Details: Not applicable — backend REST endpoints, no UI surface. D3 (site-editor global-styles UI) will cover accessibility testing for the consumer of these endpoints.
Tests Added
Test details: 18 new Pest feature tests covering:
lookupauth gate, idempotency, theme scopingshowauth gate, full record shape, 404updateauth gate, happy path, 422 on schema-version mismatch, missingversion, missingsettings/styles, duplicate palette slugs, malformed palette entrybaseauth gate, shape assertion,base_pathconfig overridetests/Fixtures/sample-content/global-styles/default.json) round-trips throughPUT+GETwithout reshapingDocumentation
Documentation details: New
docs/global-styles.mdrecords the pinned schema version, the relationship to the vendored@wordpress/*packages, the validation constraints, the multi-theme scoping model, and an explicit upgrade checklist for future schema bumps. File-level and method-level PHPDoc blocks follow the same convention as the C1 / C2 siblings.Screenshots
Not applicable — backend-only work.
Pre-Submission Checklist
Summary by CodeRabbit
New Features
Documentation
Tests