Save metadata(page data) field with the entity revision

I have started work on top of [##3478565] for nwo

wim leers → credited bnjmnm → .

I think this MR is quite a bit too optimistic?! 😅🙈

1. Easy: AFAICT the special read-only handling is simply because this MR is not yet performing the proper Entity Field API access checks: https://git.drupalcode.org/project/experience_builder/-/merge_requests/4...
2. Hard: I have strong doubts about the (undocumented!) assumption this MR makes: that the client sends Entity Field property values. AFAICT it does NOT send those, it just happens to for simple Field Types/Widgets, where the shape of the data in the Field Widget's HTML form happens to match that of the corresponding Field Type's prop. But that's definitely NOT guaranteed. See: all \Drupal\Core\Field\WidgetInterface::massageFormValues() implementations.

   I'm fine with not fully solving this here. But not acknowledging that is not acceptable IMO.

   In addition to acknowledging that, this code should point to a follow-up issue where that does get fixed. And 🐛 Experiments in rendering Twig as React Active landed 2 days ago, and documented precisely this in docs/redux-integrated-field-widgets.md.

@tedbow and I discussed this in light of the 'publish' functionality.
As we understand it, the publish feature will be similar to publishing a workspace. In the first instance it will take all auto-saved items and turn them into entity revisions. This won't make use of workspaces in the original implementation but it is hoped that it will by the end state.

If that's the case, then is there any need for this issue/controller? The only thing that writes to the entity will be the publish operation, i.e. all other controllers only write to the tempstore. In that case should we focus more on the publish operation taking all of the auto-saved items and 'promoting' them to entity-revisions?

And if so, that raises a number of UX questions:

• If there are multiple pages being published, what happens when some of them pass validation but others don't
• How do we convey that to the user
• What happens if we're publishing a large number of entities and we hit resource constraints - we likely don't want to reach for batch API here because the UX isn't on par with what XB is delivering

So thinking out loud, perhaps the frontend needs to keep track of the list of unsaved entities that a publish operation will actually publish? A list controller in Drupal can expose the list of unsaved entities so that this can be syncronized with the frontend. This would likely need to support pagination for the case of a large number of items. This list could deal with the reality that some pages won't have an entity ID until they are published - so the autosave controller will need to deal in UUIDs or similar. This list could be used by the editor to navigate between pages being edited. It would also allow for validation to provide meaningful actions for the user. e.g. they attempt to publish 7 pages, but pages 2 and 3 are invalid. Being able to present the list of unsaved pages to the user will allow them to navigate to the invalid ones. We could also possibly show invalid components in the tree outline in the editor. This also probably means that the controller endpoint for fetching the current layout needs to work with the auto-saved ID rather than entity ID.
In terms of addressing the 'large number of entities' I think if the publish controller is configured to return a 307 while each group is processed that should just work with the fetch API. But that puts us into a state where several entities could save and one could fail. Probably post 0.2 to consider once we have workspaces support.

So tl;dr I think we likely need a list controller that returns the current 'auto-save'/in-progress content - i.e. the things that publishing will publish. This will allow nice UI affordances like `Publish (3)` and also a UI to navigate between pages in progress from with XB.

I talked to @larowlan a bit about this and said I would write my thoughts up.

First thanks to @larowlan @bnjmnm and @wimleers for the reviews

But I no longer think we should do this issue as is, here is why

1. For 📌 Save metadata(page data) field with the entity revision Active we are not going to use Workspaces. All the ongoing saves will use auto-save. There will be no option to save an individual entity directly from client data as a real entity save.

   All the saves will happen when the user clicks "Publish now" to save all the entities that have auto-save states (at least the valid ones)

2. When we first started ✨ AutoSave Entity Revision POC Active where we made ApiContentUpdateController the above was not clear (at least to me) or decided. Therefore it was reasonable to think the user would be able to save an individual entity directly from client data and ApiContentUpdateController made sense
3. Now ApiContentUpdateController no longer makes sense. For the endstate of 0.2 there will never be a request from the client with the XB field and other entity fields which should then directly result in a real entity save.
4. In 0.2 all real entity saves will actually be auto-save states to entity saves. Never with the data directly from client to entity save. There will always be the auto-saved version first
5. So ultimately we will need an saveEntityFromAutoSave() to be triggered for each auto-save state when the user hits the "Publish all button"
6. Triggering saveEntityFromAutoSave() on many may not be something that can be done in a single request, but because of the following point I don't think we should worry about that.
7. After 0.2 when eventually we are using Workspaces it is likely saveEntityFromAutoSave() will happen before the user clicks the "Publish all" button. It may be that all the "Publish all" button will do at that point will be move real entities to the live site in the regular Workspace way (though probably with our UI)

   Because of this the problem calling saveEntityFromAutoSave() many times in 1 request is likely only a problem for 0.2 or at least pre-XB-Workspaces

   At the very least we don't know this will be a problem once we start using Workspaces.

8. Therefore I don't think we should worry too much about what would happen in 0.2 if you had 100 entities being saved in 1 request.

   Since 0.2 is truley a preview I think it would be reasonable to put some limit on the number entity changes we support in XB at once. Because otherwise we likely will be solving performance problems that might go away when using Workspaces.

All this makes me think more work on ApiContentUpdateController is just churn and what should be doing.

1. Continue work on 📌 Create AutoSave service and HTTP API to retrieve all entities with pending changes Active

<

2. Create separate issue to save the metadata field in the auto-save states or fold that into #3489743
3. Create a follow-up issue Create Endpoint to Publish all auto-save state to real entity saves

   In this we would create a service that would do something like saveEntityFromAutoSave(). Hopefully we could start that from what we have now in ApiContentUpdateController as the data this controller gets from the request is exactly the same as what we currently save in the auto-save state. #3489743 will add more metadata about the autosave but it should still retain the data the auto-save has now.

Of coures all this would delay the ability to actually save the XB data to the DB and therefore be able to view a node(an eventually a Page entity) with the components that were added in the XB UI. I understand that even if we eventually remove it having a "Publish" button on the XB UI that saves an individual node makes XB seem much more real. It also would let people demoing XB to create multiple different nodes that would show what you could make with XB.

Therefore while we are doing the above I think we should make an issue for Create a temporary Publish button to save an individual node via ApiContentUpdateController. Hopefully that issue could just use ApiContentUpdateController and not worry about the metafields as they are not being sent from the client right now anyways. This would be stop-gap until we do the steps above to convert auto-saves to real entity saves.

Assigning to @wim leers because I think my proposal in #8 needs review. Then I would make issues accordingly if he agrees

✨ AutoSave Entity Revision POC Active got committed 🎉

so my suggestion above

Create a temporary Publish button to save an individual node via ApiContentUpdateController.

has already been done

@larowlan in #7

In that case should we focus more on the publish operation taking all of the auto-saved items and 'promoting' them to entity-revisions?

But if all those auto-saved-to-tempstore items exist only in tempstore, how can you preview them together? I guess that's your point: you won't "in the first instance" — i.e. that's for a later phase/release?

The first 2 UX questions you raise are exactly what not saving to tempstore but to a workspace would address.

The third UX question is a really good point! That requires clarification from Product Manager @lauriii and the design team, because not only is that a performance/scalability matter, it's also a UX concern: what if >100 changes are accumulated prior to publishing them all? 😨

@larowlan on the MR: in almost every of your comments on the MR you refer to the inability of this controller to update the Pattern config entities. But that's not relevant here. The controller being updated exists specifically for updating content entities that have an XB field. The Pattern config entity type already has full CRUD support via an HTTP API, see \XbConfigEntityHttpApiTest::testPattern() 😊

Created 📌 Create an endpoint to publish all auto-saved entities Active that I believe we should do before this issue so don't have refactor the controller again. See my proposal in 📌 Create an endpoint to publish all auto-saved entities Active

@tedbow in #8

1. This is my understanding for 0.2 too, but it will need to be supported later, right?
2. This is AFAICT still necessary, to power the "Publish" button?
3. I don't understand this leap. 😅
4. Even if there's an auto-saved version first, that may be invalid, and so we'll still need the client to talk to the server (to the controller that ✨ AutoSave Entity Revision POC Active introduced) to do a "real" save, including actual validation?!
5. This part I disagree with I think — see the prior point. The client must have all the data anyway. So expecting the client to perform an "actual/real" entity save through an explicit HTTP API request to a controller, to receive validation errors etc. seems A) logical, B) reasonable, C) in line with eventually adding Workspaces support?
6. You say "before" because that way the XB UI would be informed of validation errors sufficiently early?

So I'm not at all following how we conclude 📌 Create AutoSave service and HTTP API to retrieve all entities with pending changes Active is the correct next sttep.

#8.3/#13.3

@tedbow just talked me through this. I now understand it! The future button will publish an entire workspace, meaning 1, 2 or possibly 100 content entities (and perhaps even config entities) all at once, with a single click of a button. Given that, a single request to publish everything auto-saved into tempstore is the closest current equivalent. So +1.

Per @tedbow and @larowlan, this should hence be postponed on 📌 Create an endpoint to publish all auto-saved entities Active , which will refactor the logic that ✨ AutoSave Entity Revision POC Active introduced into an explicit ClientToServerConverter service.

After that is done, this issue will then update that ClientToServerConverter service to also support (metadata) fields.

The third UX question is a really good point! That requires clarification from Product Manager @lauriii and the design team, because not only is that a performance/scalability matter, it's also a UX concern: what if >100 changes are accumulated prior to publishing them all? 😨

Needs input from @lauriii and/or design.

Blocking this issue on those clarifications, but it really should be spun off into its own issue.

If there are multiple pages being published, what happens when some of them pass validation but others don't
How do we convey that to the user

This is demonstrated by this user flow: https://www.figma.com/design/1sj0mnVdLFdYihrkAhR43u/Experience-Builder-%.... Let me know if you want me to record a short video to walk through that since this is one of the more complex flows and it might be helpful.

What happens if we're publishing a large number of entities and we hit resource constraints - we likely don't want to reach for batch API here because the UX isn't on par with what XB is delivering

We can accept the scalability problems for this initial state. We'd probably publish at maximum 10-15 entities with the intermediate solution. We could probably accept even if it was just 5-10 but then we'd need to document it somewhere.

Do these scalability concerns exist with the Workspace based solution too? From design perspective, it seems that the current solution is good enough for now. It would probably be worth opening a spinoff issue to define in a bit more detail how we'd expect the panel to behave in this scenario.

@larowlan on the MR: in almost every of your comments on the MR you refer to the inability of this controller to update the Pattern config entities. But that's not relevant here. The controller being updated exists specifically for updating content entities that have an XB field

Thanks, personally in my head there's an end-state where that should be one and the same and abstracted behind routing/storage layers - much like how LB has little distinction between writing to config entities (defaults), content entities (overrides) or simple config (navigation module). But that's for the future.

Do these scalability concerns exist with the Workspace based solution too? From design perspective, it seems that the current solution is good enough for now. It would probably be worth opening a spinoff issue to define in a bit more detail how we'd expect the panel to behave in this scenario.

In my opinion the issue still exists for Workspaces - it makes use of set_time_limit to continue to increase the execution time on a workspace publish event. This might work for low-end setups but for most HA setups the CDN won't hang around waiting for a response from the backend that long. E.g. Cloudfront, Cloudflare all have time-out limits after which they return a 504. So we will likely need to resolve that too if this is going to be for enterprise setups. I created a small POC yesterday to check if the fetch API respects a 307 header and it indeed does. So there is scope for us to craft a controller for the publish that processes chunks (e.g. we could use the existing settings.php flag for entity_update_batch_size) and returns a 307 with a Location header and JSON body that contains the progress. AKA an API based batch API. In my testing, I created a controller that responded with 4 307s and then a 201, and the FE did indeed make the subsequent requests to the Location header until the 200 was returned. So I think there's a world with nice UX where we put the operations in a queue and process them.

This is demonstrated by this user flow: https://www.figma.com/design/1sj0mnVdLFdYihrkAhR43u/Experience-Builder-%.... Let me know if you want me to record a short video to walk through that since this is one of the more complex flows and it might be helpful.

I don't have access to that design @lauriii - so yes please to a short video

much like how LB has little distinction between writing to config entities (defaults), content entities (overrides) or simple config (navigation module). But that's for the future.

LB writes to config entities and simple config?! 🤯

Where? (Do you have a URL? 🙏)

How? (Zero config was validatable at the time Layout Builder shipped!)

set_time_limit()

Woah! 😱 Indeed: \Drupal\workspaces\WorkspacePublisher::publish() uses that…

In my testing, I created a controller that responded with 4 307s and then a 201, and the FE did indeed make the subsequent requests to the Location header until the 200 was returned. So I think there's a world with nice UX where we put the operations in a queue and process them.

🤩👏

I don't have access to that design @lauriii - so yes please to a short video

😬 And no response yet ~24 hours later, escalated internally. Sorry about this 😞

@larowlan Here's the correct Figma link: https://www.figma.com/design/Ps3m4APGHIILsBrm0Uj31N/Experience-Builder?n.... Updated the original link too.

But if all those auto-saved-to-tempstore items exist only in tempstore, how can you preview them together? I guess that's your point: you won't "in the first instance" — i.e. that's for a later phase/release?

For 0.2.0, you won't be able to preview them outside of XB. However, you'll be able to preview them inside of XB. For example, if there's auto-saved changes to both the Page content entity ( 🌱 [Research] Landing page integration Active ) that you're currently viewing in XB, and the PageTemplate config entity, and one or more JS component config entities (once ✨ [exploratory] PoC of Preact+Tailwind components editable via CodeMirror or Monaco Active is done), then the XB canvas will reflect all of that. There's also been some ideation (not sure if a d.o. issue has been opened yet for the implementation) for a button in XB to remove most of XB's UI and just show its canvas full-screen to make it even closer of a "show me what this will look like on the real site" preview.

LB writes to config entities and simple config?! 🤯

Where? (Do you have a URL? 🙏)

How? (Zero config was validatable at the time Layout Builder shipped!)

• Simple config in core's navigation module
• Config entities in core entity view display and layout library module

Thanks for access @lauriii

@lauriii thanks, it looks like the design matches my mental model of this and your refinement questions match mine in #7

I think it would be good if in the list of 'unpublished changes' there was some sort of icon to indicate a page had an error, and if these could be links to lead the user to loading that page in the editor. Or even better if the errors could link to both the page and open the component in the side panel

The designers suggested that the errors should have a have a link to the page where the error appears, and it should indeed focus the error that the user clicked. I think it's a good point that it might be beneficial to additionally highlight the pages that have errors in the list of pages. 👍

@lauriii one thing the design is missing is translations, it is feasible that there will be multiple translations of a page in the auto-save store, so we might need to convey that in the review changes list somehow. Or should the list only return items in the current language?

We should display translations there but we need to determine how, because the design doesn't account for how translations are displayed in there (yet). I can think of at least two ways to display this:

1. Each translation becomes it's own row
2. Each row indicates which translations of the page has been changed

comitted 📌 Create an endpoint to publish all auto-saved entities Active