Repository navigation
G4a: verify core/post-* blocks fully wired against G3 entities - #425
Merged
Merged
Conversation
Closes the gaps surfaced by the G4a verification pass:
* core-data shim's `useEntityProp` was a no-op stub. Implement the real
read+edit hook so `core/post-*` Edit components round-trip the post
prop through the shim's edits bag instead of always seeing undefined.
* `<BlockContextProvider>` now wraps `<BlockList>` for cms-framework
Post/Page edits so blocks declaring `usesContext: ["postId",
"postType", "queryId"]` pick up the active entity from block context.
The `value` is memoized so editor-app re-renders don't fan a new
context object out to every block (which clobbers the inspector's
Block tab selection state mid-edit).
* `usePersistence` accepts an optional core-data entity identity. The
save loop now flushes any staged `editEntityRecord` edits to the G3
PUT endpoint after the block save lands so sidebar metadata edits
(excerpt, featured_media, slug, status, author, categories, tags,
parent, menu_order, template) and post-title block edits both
round-trip on the same cycle. Sidebar handlers dispatch
`editEntityRecord` next to their existing `setLocalState` mirror.
* New `AttachmentController` at `GET /visual-editor/api/attachments/{id}`
emits the WP REST media envelope (source_url, alt_text, media_details)
by delegating to `apGetMedia()`. `core/post-featured-image` and
`core/cover` resolve their `featured_media` id through the shim's new
`attachment` postType entity. Adapter gains a `toWpRestShape` method
alongside the existing `toGutenberg` (legacy MediaUpload) shape.
* `docs/block-library-audit.md` flips the six entity-wired post blocks
out of the deny-list "Crash or backend-required" section into a new
"Entity-wired" bucket documenting the resolved-attribute contract
the renderers consume.
Tests: 463 PHP + 970 JS, all green. New coverage:
* core-data-shim.test.tsx: useEntityProp read, write, ambient id
fallback, REST resolver path, same-component re-render after fetch.
* GutenbergAttachmentAdapterTest: WP REST shape, sizes reshape, null
caption collapse.
* PostControllerTest: partial metadata-only PUT round-trips excerpt +
featured_media end-to-end.
Closes #400
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR implements entity metadata persistence in the visual editor by integrating core-data with the block editor's persistence layer. It adds API endpoints for saving attachment records, updates the core-data shim to support entity property editing, and wires sidebar metadata changes to dispatch core-data record updates and flush staged edits through a new metadata-save API flow. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Sidebar as Sidebar UI
participant EditorApp as Editor App
participant CoreData as Core-Data Store
participant PersistenceHook as Persistence Hook
participant ApiClient as API Client
participant Backend as Backend API
User->>Sidebar: Edit metadata (title, excerpt, featured_image)
Sidebar->>EditorApp: onChange handler triggered
EditorApp->>CoreData: dispatch editEntityRecord(field, value)
EditorApp->>PersistenceHook: queueMetadataForSave()
PersistenceHook->>CoreData: select getEntityRecordEdits
CoreData-->>PersistenceHook: return staged edits {excerpt, featured_media, ...}
alt Block content also changed
PersistenceHook->>Backend: PUT /posts/{id} with content
Backend-->>PersistenceHook: block save response
end
PersistenceHook->>ApiClient: saveEntityRecord(edits)
ApiClient->>Backend: PUT /posts/{id} with metadata only
Backend-->>ApiClient: updated record JSON
ApiClient-->>PersistenceHook: return parsed response
PersistenceHook->>CoreData: dispatch clearEntityRecord edits
PersistenceHook->>User: metadata persisted
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 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)
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. 🔧 PHPStan (2.1.51)PHPStan was skipped because the sandbox runner could not parse its output. Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
resources/js/visual-editor/editor/editor-app.tsx (1)
971-977: 💤 Low valueConsider using
blockContextValuedirectly in the conditional for type safety.TypeScript won't narrow
blockContextValuebased on checkingdocumentTypeandnumericIdseparately. IfBlockContextProviderstrictly types itsvalueprop as non-nullable, this could cause a compile error in stricter configurations.Suggested refinement
- {documentType !== null && numericId !== null ? ( - <BlockContextProvider value={blockContextValue}> + {blockContextValue !== null ? ( + <BlockContextProvider value={blockContextValue}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/editor/editor-app.tsx` around lines 971 - 977, The conditional should check blockContextValue directly so TypeScript can narrow its nullability before passing into BlockContextProvider; replace the condition (documentType !== null && numericId !== null) with a direct check of blockContextValue (e.g., blockContextValue != null) and wrap BlockList in <BlockContextProvider value={blockContextValue}> only when that value is non-null, referencing BlockContextProvider, blockContextValue and BlockList to locate the change.resources/js/visual-editor/editor/use-persistence.ts (1)
262-317: 💤 Low valueConsider handling partial success more gracefully.
If block content saves successfully but
saveEntityRecordfails, the error status is set andlastSavedAtis not updated, even though the blocks were persisted. This could confuse users who might believe their block changes were lost.A possible improvement: track whether blocks saved successfully and update
lastSavedAtbefore attempting the metadata save, then set status to a "partial error" or still update the timestamp while reporting the metadata failure.This is an edge case, so the current behavior (prioritizing error signaling) is acceptable for now.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/editor/use-persistence.ts` around lines 262 - 317, When saveContent succeeds but saveEntityRecord may fail, update savedAt and call setLastSavedAt(savedAt) immediately after the saveContent/response path (inside the block that sets savedAt and dispatchEditorEvent) so the block timestamp reflects persisted blocks, then wrap the saveEntityRecord call in its own try/catch (referencing saveEntityRecord, readEntityEdits, clearEntityEdits, savedAt, setLastSavedAt, setSaveStatus) and on entity-save failure set a distinct status (e.g., setSaveStatus('partial-error') or keep 'saved' while reporting the metadata error) without clearing the lastSavedAt; ensure the catch still returns early if unmountedRef.current or version mismatch requires aborting and only clears edits on success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@resources/js/visual-editor/editor/editor-app.tsx`:
- Around line 971-977: The conditional should check blockContextValue directly
so TypeScript can narrow its nullability before passing into
BlockContextProvider; replace the condition (documentType !== null && numericId
!== null) with a direct check of blockContextValue (e.g., blockContextValue !=
null) and wrap BlockList in <BlockContextProvider value={blockContextValue}>
only when that value is non-null, referencing BlockContextProvider,
blockContextValue and BlockList to locate the change.
In `@resources/js/visual-editor/editor/use-persistence.ts`:
- Around line 262-317: When saveContent succeeds but saveEntityRecord may fail,
update savedAt and call setLastSavedAt(savedAt) immediately after the
saveContent/response path (inside the block that sets savedAt and
dispatchEditorEvent) so the block timestamp reflects persisted blocks, then wrap
the saveEntityRecord call in its own try/catch (referencing saveEntityRecord,
readEntityEdits, clearEntityEdits, savedAt, setLastSavedAt, setSaveStatus) and
on entity-save failure set a distinct status (e.g.,
setSaveStatus('partial-error') or keep 'saved' while reporting the metadata
error) without clearing the lastSavedAt; ensure the catch still returns early if
unmountedRef.current or version mismatch requires aborting and only clears edits
on success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 104277ac-7b26-46ab-a3c3-b063e4598231
📒 Files selected for processing (12)
database/migrations/testing/2026_04_19_000000_create_test_block_content_tables.phpdocs/block-library-audit.mdresources/js/visual-editor/editor/api-client.tsresources/js/visual-editor/editor/editor-app.tsxresources/js/visual-editor/editor/use-persistence.tsresources/js/visual-editor/vendor/__tests__/core-data-shim.test.tsxresources/js/visual-editor/vendor/core-data-shim.tsroutes/api.phpsrc/Http/Controllers/AttachmentController.phpsrc/MediaBridge/GutenbergAttachmentAdapter.phptests/Feature/VisualEditor/Adapters/CmsFramework/PostControllerTest.phptests/Unit/VisualEditor/MediaBridge/GutenbergAttachmentAdapterTest.php
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
17 of 18 tasks
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
1 participant
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
Verification + cleanup pass for G4a (issue #400). Surfaced and closed three real gaps in the G3 entity adapter that prevented
core/post-*blocks from actually round-tripping their content in the editor canvas.Closes: #400
Type of Change
Related Issue
Issue: #400
Motivation and Context
Per the issue's acceptance criteria, each
core/post-*block needs to render editable content in the editor against cms-framework Post and Page. Verification revealed three blockers in code that had shipped through G0/G3:useEntityPropwas a no-op stub returning[undefined, noop, undefined]. Every block-library Edit component that reads viauseEntityProp(post-title, post-content, post-excerpt, post-author, post-date, post-featured-image, cover) saw nothing.BlockContextProviderwas never wrapped around<BlockList>, so blocks declaringusesContext: ["postId", "postType", "queryId"]hadcontext.postType/context.postIdundefined and short-circuited to their placeholder shells./contentendpoint. Sidebar metadata edits (excerpt, featured_media, author, etc.) accumulated in local React state and were dropped on reload because no save channel sent them anywhere.A fourth gap surfaced once 1–3 landed:
core/post-featured-imageandcore/coverresolve theirfeatured_mediaid viagetEntityRecord('postType', 'attachment', id), which our shim had no entity registration or endpoint for.Changes Made
useEntityProp(core-data-shim.ts) — replace the no-op with a real read+edit hook. Reads viagetEditedEntityRecord(flattened) +getEntityRecord(full shape), returns a setter that dispatcheseditEntityRecord. Falls back to ambientEntityProviderid when the explicit id arg is omitted.<BlockContextProvider>wraps<BlockList>with a memoized{postType, postId}value when the editor mounts against a cms-framework Post/Page. Skipped for non-entity resources.usePersistenceaccepts an optionalentity: {kind, name, id}identity. Save flow now flushes block tree first via the existing/contentPUT, then drains any stagededitEntityRecordedits to a newsaveEntityRecordAPI client method that PUTs partial WP-shape data to/visual-editor/api/{resource}/{id}. Sidebar handlers dispatcheditEntityRecordalongside their existingsetLocalStatemirror; auseSelectsubscription on the entity edits bag re-arms the metadata-save debounce when block-level setters fire (e.g.core/post-titletyping).AttachmentController+ WP REST attachment shape — newGET /visual-editor/api/attachments/{id}endpoint emits the WP REST media envelope (source_url,alt_text,media_details) by delegating toapGetMedia().GutenbergAttachmentAdapter::toWpRestShape()produces the new shape; the existingtoGutenberg()(legacyMediaUploadshape) is unchanged. Shim'sDEFAULT_ENTITIESadds anattachmentpostType entity pointing at the new endpoint.docs/block-library-audit.md— flips the six entity-wired post blocks out of the M5-era "Crash or backend-required" deny-list section into a new "Entity-wired" bucket documenting the resolved-attribute contract that the Blade/React/Vue renderer packages consume. Adds the bucket to the bucket-classification preamble.How Has This Been Tested?
Testing Environment:
Tests Performed:
/packages/visual-editor/m3/post/1):core/post-titleblock reads + edits the saved title and persists on reload./visual-editor/api/posts/1and persist on reload.core/post-featured-imageblock renders the saved image after the newattachmentsentity resolves.BlockContextProvider).PUT /contentfor the block tree andPUT /for the partial entity edits, in order.Accessibility Tests Run
Details: No new UI surfaces; the inspector panels, block toolbar, and canvas markup are unchanged. Existing accessibility coverage in upstream Gutenberg components applies. The new attachment endpoint is a JSON-only API — no a11y surface.
Tests Added
Test details:
core-data-shim.test.tsx: 5 newuseEntityPropcases covering missing-args early return, edited-prop read with{raw, rendered}flattening, full-shape return, setter dispatchingeditEntityRecordand surfacing on next read, ambientEntityProviderid fallback, REST resolver path with mocked fetcher, and same-component re-render after the resolver settles. Plus theattachmententity registration assertion.GutenbergAttachmentAdapterTest.php: 3 new cases fortoWpRestShapecovering core field mapping, sizes reshape into WP{source_url, width, height}shape, and null caption collapse to{rendered: ''}.PostControllerTest.php: 1 new feature test exercising the partial metadata-only PUT path ({excerpt, featured_media}) end-to-end againstTestBlockContentModel.Documentation
Documentation details:
docs/block-library-audit.mdupdated to reflect the Entity-wired bucket. Inline PHPDoc + JSDoc on the new methods (useEntityProp,saveEntityRecordAPI client,AttachmentController::show,toWpRestShape,usePersistenceentity identity), and explanatory comments at each non-obvious wiring site (BlockContextProvider memoization rationale, entity-edits subscription, dual-flush ordering).Companion dev-app changes
To make the manual smoke test possible, the dev-app (
Herd/artisanpack-ui) needs:slug, excerpt, featured_image_idtovisual_editor_postsand the page-only fields tovisual_editor_pages.App\Models\Post/App\Models\Page$fillableupdates to expose those columns.VisualEditorTestController::resolveFeaturedImage()and them3-editor.blade.phptemplate passingdata-excerpt/data-featured-image/data-author-idso the sidebar's local state hydrates from the DB on reload.These are tracked separately on the dev-app working tree and not part of this PR.
Pre-Submission Checklist
Summary by CodeRabbit
New Features
Documentation