[PP-1] Send page data to Drupal for storage in auto-save store

larowlan → changed the visibility of the branch 3495752-pp-1-send-page to hidden.

📌 Maintain a per-component set of prop expressions/sources Active and 📌 Split model values into resolved and raw Active also block this as they prevent removing the hard-coded tree item used by preview controller.
Without removing that, we can't see changes to dynamic fields based off the node/entity fields in the preview.

I'll start on those next

✨ Save page data form values in application state with support for undo/redo Active is in.

@effulgentsia indicated we can do this without seeing updates to dynamic components in the preview, so this no-longer depends on 📌 Maintain a per-component set of prop expressions/sources Active and 📌 Split model values into resolved and raw Active

I've moved the new e2e test to 📌 Maintain a per-component set of prop expressions/sources Active for that.

That now means this is ready for review 🙌

This looks good but when I try to manually test it I get errors. Investigating

Created related 📌 Missing e2e for Publish button hides bugs Active

working this

I have fixed the bugs I found in manual tests. Now in manual testing change update the page data fields(at least have tried title and body) the "Publish" button works, and when I view the node outside of experience builder I see both the XB and other field changes.🎉

Assigning back to @wim leers since @larowlan assigned to him but I will continue to review.

I have idea why the test is failing. Will work on it

PHPUnit tests are passing now, the e2e contextual-panel.cy.js test failed, Hoping was just a fluke so retesting

I think this is good now, I started this and @larowlan did a lot of the work too, so I can't approve

I didn't get to this today, between tying up the many loose ends that ✨ [PP-1] Integrate saving sections with the backend Postponed revealed, which I finished up just now in 📌 Improve or remove ComponentSourceInterface::getClientSideInfo() Active (thanks, @longwave!).

I had deprioritized reviewing this issue in favor of other issues: because I need to see the big picture to be able to properly review this, and pretty much all of this infrastructure landed while I was on paternity leave with no involvement from me. That's why I've been asking @tedbow to update https://www.drupal.org/project/experience_builder/issues/3455753#save-dr... 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active , which he has been doing! 👏

I promise a thorough review tomorrow.

Meanwhile, this seems to have conflicted with 🐛 Cannot save Page in XB when content_moderation is installed Active , so a rebase would be appreciated 🙏😊

This is ready for review again @Wim

Thanks!

(Also thanks to @tedbow for getting https://www.drupal.org/project/experience_builder/issues/3455753#save-dr... 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active in good shape!)

Post-humously reviewed the predecessor

Reviewed the ✨ Save page data form values in application state with support for undo/redo Active MR to be able to review this one. High-level observations from there:

• \Drupal\experience_builder\Controller\ApiLayoutController::getEntityData blew my mind.
• Editing the node title in the tab does not result in the auto-saved entity getting its label updated (nor does it result in an updated preview, but that'd be the page title block needing to get updated — for that we have 📌 [later phase] ApiPreviewController must use the previewed route's controller, and override canonical content entity routes' received entity object Postponed 👍
• I would've liked 📌 [PP-1] Extend test coverage for page data form elements Postponed to be a blocking issue, because A) adding those was out of scope and caused confusion on the issue, B) now we risk never getting to those. Which increases technical debt/risk. That's something we need to get better at everywhere though, but in an MR as complex as this one, I think we as a team need to become more disciplined at that.
• In other cases, the temporary work-arounds in the code do not point to the fixing issue, example: ✨ Save page data form values in application state with support for undo/redo Active .
• Updates/additions to docs/ would've been nice 😇🙏 A lot of the React code is beyond me, especially because comments are sparse.
• 🤩 Impressive work on undo-redo-timeline.cy.js and all the things it proves! Undoing across both the edited content entity object's fields and the XB component tree ("layout+model") is crucial, and it now works!

Review

1. Some really impressive work here, @larowlan and @tedbow! 😄👏 It looks pretty hacky at this point, but that's because we're bootstrapping all this. I'm confident it'll get better over time. 👍
2. Bug #1: When the server responds with validation errors, those are not displayed anywhere: not in the button (that's in scope for ✨ [PP-1] Implement the "Publish All" button Postponed , and not in the form. I cannot find an issue for that. →
3. Bug #2: Buggy behavior for boolean fields, as described in detail at https://git.drupalcode.org/project/experience_builder/-/merge_requests/5....

Can you please create a follow-up issue for those 2? 🙏

Given how important this is for https://www.drupal.org/project/experience_builder/issues/3455753#save-dr... 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active , and that this definitely gets us to a better place than HEAD: merging :)

Opened
📌 Harden check for boolean fields in ClientDataToEntityConverter Active
📌 [Needs design] Display validation errors in page data form Active

FWIW I found the qs npm package after this and have started using it in 📌 Move clientside assumptions about prop form data shape into a series of prop specific transforms Active which eventually will allow us to remove the hacky \parse_str \http_build_query dance.

Thanks for those! 🙏

I quite liked/was impressed with that "hacky dance" 😅🙈 IDK what that says about me 🤣

Automatically closed - issue fixed for 2 weeks with no activity.