Basic Live-Harness endpoint functionality (tweaked on top of #28417) #28438
+134
−6
Conversation
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
7 tasks
rtyley
force-pushed
the
refactor-prior-to-live-harness
branch
from
9e78e21 to
c413902
Compare
rtyley
force-pushed
the
live-harness-endpoint-refactor
branch
from
a920d3f to
8650909
Compare
Contributor
This is a tweaked version of the work done in #28417, but built upon the refactors done in #28437, which allows this implementation to be smaller. In `LiveHarnessController.renderLiveHarness()`: * The `ArticleController.mapAndRender()` & `InteractiveController.modelAndRender()` methods introduced in #28437 are useful not only within those controllers, but also more suitable for the work needed in `renderLiveHarness()`, meaning that each case (`ContentType.Article` & `ContentType.Interactive`) can be handled much more concisely - a single line each. * We can avoid transforming the `JsResult` to an `Either`, and just match on `JsSuccess`/`JsError` rather than `Right`/`Left` * The `match` statement on the `Option[ContentType]` can be done slightly more concisely using `collect` : https://www.scala-lang.org/api/2.13.18/scala/Option.html#collect[B](pf:PartialFunction[A,B]):Option[B] In `LiveHarness`: * The important code in `turnLocalAtomsIntoAtomBlocksAndElements()` - that makes an `InteractiveAtom` or `BlockElement` out of a `LiveHarnessInteractiveAtom` - has moved onto `LiveHarnessInteractiveAtom` itself (so all the references copying data from `localAtom.xxx` fields become just references to `xxx` field itself) * A fiddly bit of functionality - updating the `content` within a `ContentPage`, which varies depending on whether it's a `ArticlePage` or `InteractivePage` - has been isolated into a `PageUpdater` type class. This allows us to have much clearer code elsewhere - without having to define a content-updating method for every single `ContentPage` type, just the two we care about. Consequently, there's not a need for either of the two `injectInteractiveAtomsIntoPage()` methods that used to exist - just a single `inject()` method, which does the work of `addLocalAtomsToParent()` too. * We use the handy `splitAt()` method to make a bit of code in `addLocalAtomsToParent()` a bit smaller. In `InteractiveController`: * A small new `modelAndRenderHtml()` method is required, to match the behaviour from #28417 which short-circuits a lot of logic to ensure that `renderHtml()` is called. Co-Authored-By: Jonathon Herbert <[email protected]> Co-Authored-By: Frederick O'Brien <[email protected]>
frederickobrien
force-pushed
the
live-harness-endpoint-refactor
branch
from
8650909 to
52c4785
Compare
frederickobrien
approved these changes
Dec 5, 2025
gu-prout
bot
added
Pending-on-ADMIN-PROD
Pending-on-FRONTS-PROD
Seen-on-ADMIN-PROD
and removed
Pending-on-ADMIN-PROD
labels
|
Seen on ADMIN-PROD (created by @rtyley and merged by @frederickobrien 7 minutes and 56 seconds ago)
|
|
Seen on FRONTS-PROD (created by @rtyley and merged by @frederickobrien 10 minutes and 9 seconds ago)
|
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.
This is a tweaked version of the work done in #28417 (so see the functionality description there!), but built upon the refactors done in #28437, which allows this implementation to be a bit smaller.
Changes relative to the original PR #28417
In
LiveHarnessController.renderLiveHarness():ArticleController.mapAndRender()&InteractiveController.modelAndRender()methods introduced in Refactor: IntroduceBlocksOn[Page]parameter object #28437 are useful not only within those controllers, but also more suitable for the work needed inrenderLiveHarness(), meaning that each case (ContentType.Article&ContentType.Interactive) can be handled much more concisely - a single line each.JsResultto anEither, and just match onJsSuccess/JsErrorrather thanRight/Leftmatchstatement on theOption[ContentType]can be done slightly more concisely usingcollect: https://www.scala-lang.org/api/2.13.18/scala/Option.html#collect[B](pf:PartialFunction[A,B]):Option[B]In
LiveHarness:turnLocalAtomsIntoAtomBlocksAndElements()- that makes anInteractiveAtomorBlockElementout of aLiveHarnessInteractiveAtom- has moved ontoLiveHarnessInteractiveAtomitself (so all the references copying data fromlocalAtom.xxxfields become just references toxxxfield itself)contentwithin aContentPage, which varies depending on whether it's aArticlePageorInteractivePage- has been isolated into aPageUpdatertype class. This allows us to have much clearer code elsewhere - without having to define a content-updating method for every singleContentPagetype, just the two we care about. Consequently, there's not a need for either of the twoinjectInteractiveAtomsIntoPage()methods that used to exist - just a singleinject()method, which does the work ofaddLocalAtomsToParent()too.splitAt()method to make a bit of code inaddLocalAtomsToParent()a bit smaller.In
InteractiveController:modelAndRenderHtml()method is required, to match the behaviour from Basic live harness endpoint functionality #28417 which short-circuits a lot of logic to ensure thatrenderHtml()is called.