Repository navigation
feat(blocks): preview live entity values in site-* and post-* edits - #508
Conversation
The Phase I5 entity-cluster fork (#413) rendered the artisanpack/site-* and artisanpack/post-* blocks as crash-free but generic placeholder chips in the editor canvas because @wordpress/core-data was backed by the V1 shim that didn't expose the site or current-page entity. This adds two new fallback paths to createEntityPlaceholderEdit so those blocks preview real values in the canvas, while keeping the existing #483 query-loop path intact: - A singleton root/__unstableBase site-meta entity, served by a new GET /visual-editor/api/site/{id} endpoint backed by a new ArtisanPackUI\VisualEditor\Resources\SiteMetaResolver that reads from apGetSetting('site.*') with a config('artisanpack.visual-editor .site_meta') fallback. site-title / site-tagline / site-logo edits read the live record via useEntityRecord and preview its title / description / logoUrl. - A live post-entity read via useEntityRecord('postType', postType, postId), keyed off the postId/postType block context the editor stamps on the canvas (editor-app.tsx). post-title / post-excerpt / post-date / post-author / post-featured-image edits surface the active page's actual title / excerpt / formatted date / author name / featured image when placed outside a query loop. Priority order in the edit helper: stamped _resolved* attribute > artisanpack/postPreview query-loop context (#483) > live post entity (#481) > live site entity (#481) > placeholder label. Closes #481
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR implements live entity preview rendering for visual editor blocks. It adds a singleton site-meta API endpoint backed by SiteMetaResolver, extends the core-data shim with a site entity and SITE_ENTITY_ID, introduces live entity fetch/extract hooks and readEntityString, and updates post-/site- placeholder editors plus tests to prefer stamped resolved attributes, then query-preview, then live entity values, then placeholders. ChangesLive Entity Preview
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Http/Controllers/SiteController.php (1)
50-50: 💤 Low valueSimplify type hint to match route constraint.
The route constraint
[A-Za-z0-9_-]+ensures the$idparameter is always a string. Theint|stringunion type is broader than necessary and could mislead readers about the actual runtime type.♻️ Proposed refinement
-public function show( int|string $id ): JsonResponse +public function show( string $id ): JsonResponse🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Http/Controllers/SiteController.php` at line 50, The show method's union type hint int|string is broader than the route guarantee; update the SiteController::show method signature to accept only string (change int|string $id to string $id) and adjust any internal logic in show that assumes numeric $id (e.g., casting or strict type checks) so it operates on a string input to match the route constraint [A-Za-z0-9_-]+.resources/js/visual-editor/blocks/_shared/__tests__/entity-placeholder-live-entity.test.tsx (1)
79-93: ⚡ Quick winFix object spread order to prevent
idparameter override.In both helper functions, the current spread order
{ id, ...record }allows therecordparameter to override the explicitidargument. While current test cases don't passidin the record, this pattern could cause subtle bugs if future tests do. Reversing the spread order ensures the parameter always takes precedence.♻️ Proposed fix
function seedPostEntity( postType: 'post' | 'page', id: number, record: Record<string, unknown>, ): void { coreDispatch().receiveEntityRecords('postType', postType, [ - { id, ...record }, + { ...record, id }, ]); } function seedSiteEntity(record: Record<string, unknown>): void { coreDispatch().receiveEntityRecords('root', '__unstableBase', [ - { id: SITE_ENTITY_ID, ...record }, + { ...record, id: SITE_ENTITY_ID }, ]); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/blocks/_shared/__tests__/entity-placeholder-live-entity.test.tsx` around lines 79 - 93, The seedPostEntity and seedSiteEntity helpers use object spread in the order { id, ...record } which lets record.id override the explicit id argument; change the spread order in seedPostEntity and seedSiteEntity to merge record first and then id (i.e., ensure the explicit id takes precedence by using record spread before id) so the passed id parameter cannot be overridden by properties in record.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Resources/SiteMetaResolver.php`:
- Around line 130-139: The call to apGetMediaUrl() in resolveMediaUrl currently
can throw and crash requests; wrap the apGetMediaUrl($id) invocation in a
try-catch that catches \Throwable (or \Exception) and returns '' on error,
preserving the existing string return type; in the catch block optionally log or
trigger_error the exception using any available logger (or error_log) so
failures are recorded but do not propagate. Ensure resolveMediaUrl( ?int $id )
still returns '' when $id is null, when apGetMediaUrl is unavailable, or when an
exception is thrown.
---
Nitpick comments:
In
`@resources/js/visual-editor/blocks/_shared/__tests__/entity-placeholder-live-entity.test.tsx`:
- Around line 79-93: The seedPostEntity and seedSiteEntity helpers use object
spread in the order { id, ...record } which lets record.id override the explicit
id argument; change the spread order in seedPostEntity and seedSiteEntity to
merge record first and then id (i.e., ensure the explicit id takes precedence by
using record spread before id) so the passed id parameter cannot be overridden
by properties in record.
In `@src/Http/Controllers/SiteController.php`:
- Line 50: The show method's union type hint int|string is broader than the
route guarantee; update the SiteController::show method signature to accept only
string (change int|string $id to string $id) and adjust any internal logic in
show that assumes numeric $id (e.g., casting or strict type checks) so it
operates on a string input to match the route constraint [A-Za-z0-9_-]+.
🪄 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: e9bf02c6-c06f-4bfe-bbce-12328958698e
📒 Files selected for processing (16)
resources/js/visual-editor/blocks/_shared/__tests__/entity-placeholder-live-entity.test.tsxresources/js/visual-editor/blocks/_shared/entity-placeholder-edit.tsxresources/js/visual-editor/blocks/post-author/edit.tsxresources/js/visual-editor/blocks/post-date/edit.tsxresources/js/visual-editor/blocks/post-excerpt/edit.tsxresources/js/visual-editor/blocks/post-featured-image/edit.tsxresources/js/visual-editor/blocks/post-title/edit.tsxresources/js/visual-editor/blocks/site-logo/edit.tsxresources/js/visual-editor/blocks/site-tagline/edit.tsxresources/js/visual-editor/blocks/site-title/edit.tsxresources/js/visual-editor/vendor/__tests__/core-data-shim.test.tsxresources/js/visual-editor/vendor/core-data-shim.tsroutes/api.phpsrc/Http/Controllers/SiteController.phpsrc/Resources/SiteMetaResolver.phptests/Feature/VisualEditor/SiteControllerTest.php
- Wrap apGetMediaUrl() in try/catch in SiteMetaResolver so a host
media-library helper that throws doesn't crash the editor's
site-meta fetch. Mirrors the defensive pattern in
WpEntityResource::featuredImageEnvelope().
- Tighten SiteController::show signature from int|string to string —
the {id} route is constrained to [A-Za-z0-9_-]+, so Laravel never
hands the controller a numeric value.
- Reverse the spread order in the test seeders so an explicit `id`
argument always wins over any `id` a future test might pass
inside the record payload.
Description
Adds two new fallback paths to
createEntityPlaceholderEditso theartisanpack/site-*andartisanpack/post-*blocks preview real values in the editor canvas. The Phase I5 entity-cluster fork (#413) rendered them as generic placeholder chips because@wordpress/core-datais backed by the V1 shim that doesn't expose the site or current-page entity. The existing #483 query-loop preview path is left intact.Closes: #481
Type of Change
Related Issue
Issue: #481
Motivation and Context
Authors building a page template (or any non-loop layout) couldn't see real site title / logo / tagline or the page's actual title / excerpt / date / author / featured image in the canvas — only labelled placeholder chips. This closes the parity gap between the editor canvas and the rendered front end for the entity display blocks.
Changes Made
ArtisanPackUI\VisualEditor\Resources\SiteMetaResolverreadingapGetSetting('site.*')with aconfig('artisanpack.visual-editor.site_meta')fallback (mirrors the renderer-blade sibling so visual-editor stands alone).SiteControllerexposing the singleton site-meta envelope atGET /visual-editor/api/site/{id}(id segment is a sentinel — always returns the same record).root/__unstableBaseentity inDEFAULT_ENTITIESplus aSITE_ENTITY_ID = 'self'export so consumers canuseEntityRecord('root', '__unstableBase', SITE_ENTITY_ID).createEntityPlaceholderEditextended withfromPostEntity/fromSiteEntityextractor options backed byuseEntityRecord. Priority chain: stamped_resolved*→artisanpack/postPreviewquery-loop context (Editor preview: show real post data in post-* block edits instead of placeholder labels #483) → live post entity (frompostId/postTypeblock context) → live site entity → placeholder label.readEntityStringhelper handles the{raw, rendered}envelope shape returned byuseEntityRecord(unflattened) as well as the flattened raw string fromgetEditedEntityRecord.site-{title,tagline,logo}andpost-{title,excerpt,date,author,featured-image}edit wires its extractor against the relevant fields (title,excerpt,_preview.dateFormatted,_preview.author.name,_preview.featuredImage,logoUrl).How Has This Been Tested?
Testing Environment:
~/Herd/jmwd-keystone-cmsafternpm run build+sync:visual-editorTests Performed:
F j, Ydate / author name / featured image.artisanpack/postPreviewblock-context preview still wins inside a Query loop (no regression to Editor preview: show real post data in post-* block edits instead of placeholder labels #483).Accessibility Tests Run
Details: This change only affects the editor canvas preview; the front-end rendered markup is unchanged. The preview reuses the existing
useBlockProps()wrapper and the same DOM nesting the previous placeholder used, so editor a11y is unchanged. No new toolbar / inspector controls were added.Tests Added
Test details:
tests/Feature/VisualEditor/SiteControllerTest.php— 4 feature tests covering the configured-meta envelope, empty-config fallback, sentinel-id behaviour, and auth gate.resources/js/visual-editor/blocks/_shared/__tests__/entity-placeholder-live-entity.test.tsx— 20 vitest tests covering each post-* and site-* block's live-entity read path, the{ raw, rendered }shape handling, the priority order vs. stamped_resolved*+ query-preview context, and edge cases (missing context, malformedpostId, empty entity values).resources/js/visual-editor/vendor/__tests__/core-data-shim.test.tsx— updated the entity-registry assertion to include the newroot|__unstableBaseentry.Full suite: 684 PHP tests + 1508 JS tests all passing.
Documentation
Documentation details: All new public surfaces (
SiteMetaResolver,SiteController,SITE_ENTITY_ID, the newEntityPlaceholderConfigoptions,readEntityString, the new live-entity hooks) have PHPDoc / TSDoc blocks. ThecreateEntityPlaceholderEditmodule header documents the new 5-level resolution priority chain.Pre-Submission Checklist
Summary by CodeRabbit
New Features
Tests