From 36b171d2fece98a6186ac72b7cd2eceb1eaabceb Mon Sep 17 00:00:00 2001 From: Charlotte <charlotte.emms@theguardian.com> Date: Mon, 5 Feb 2024 14:56:41 +0000 Subject: [PATCH 1/2] refactor: make newsletters page async incl futures and swap DCR endpoint to rendering app --- .../controllers/SignupPageController.scala | 75 +++++++++---------- applications/conf/routes | 4 +- .../renderers/DotcomRenderingService.scala | 2 +- 3 files changed, 37 insertions(+), 44 deletions(-) diff --git a/applications/app/controllers/SignupPageController.scala b/applications/app/controllers/SignupPageController.scala index bb3d3fa299fe..4a1c1a6a9aea 100644 --- a/applications/app/controllers/SignupPageController.scala +++ b/applications/app/controllers/SignupPageController.scala @@ -1,7 +1,7 @@ package controllers import common.{GuLogging, ImplicitControllerExecutionContext} -import conf.switches.Switches.{UseDcrNewslettersPage} +import conf.switches.Switches.UseDcrNewslettersPage import model.{ApplicationContext, Cached, NoCache} import model.Cached.RevalidatableResult import pages.NewsletterHtmlPage @@ -13,12 +13,12 @@ import services.newsletters.GroupedNewslettersResponse.GroupedNewslettersRespons import services.newsletters.NewsletterSignupAgent import services.newsletters.model.NewsletterResponseV2 import staticpages.StaticPages +import implicits.{HtmlFormat, JsonFormat} import implicits.Requests.RichRequestHeader -import scala.concurrent.{Await, ExecutionContext, Future} +import scala.concurrent.{ExecutionContext, Future} import scala.concurrent.duration._ import model.dotcomrendering.DotcomNewslettersPageRenderingDataModel -import model.SimplePage import model.CacheTime class SignupPageController( @@ -36,61 +36,48 @@ class SignupPageController( private def localRenderNewslettersPage()(implicit request: RequestHeader, - ): Result = { + ): Future[Result] = { val groupedNewsletters: Either[String, GroupedNewslettersResponse] = newsletterSignupAgent.getGroupedNewsletters() + groupedNewsletters match { case Right(groupedNewsletters) => - Cached(defaultCacheDuration)( - RevalidatableResult.Ok( - NewsletterHtmlPage.html(StaticPages.simpleNewslettersPage(request.path, groupedNewsletters)), + Future.successful( + Cached(defaultCacheDuration)( + RevalidatableResult.Ok( + NewsletterHtmlPage.html(StaticPages.simpleNewslettersPage(request.path, groupedNewsletters)), + ), ), ) case Left(e) => log.error(s"API call to get newsletters failed: $e") - NoCache(InternalServerError) + Future(NoCache(InternalServerError)) } } private def remoteRenderNewslettersPage()(implicit request: RequestHeader, - ): Result = { + ): Future[Result] = { val newsletters: Either[String, List[NewsletterResponseV2]] = newsletterSignupAgent.getV2Newsletters() newsletters match { case Right(newsletters) => - Await.result( - remoteRenderer.getEmailNewsletters( - ws = wsClient, - newsletters = newsletters, - page = StaticPages.dcrSimpleNewsletterPage(request.path), - ), - 3.seconds, + remoteRenderer.getEmailNewsletters( + ws = wsClient, + newsletters = newsletters, + page = StaticPages.dcrSimpleNewsletterPage(request.path), ) case Left(e) => log.error(s"API call to get newsletters failed: $e") - NoCache(InternalServerError) + Future(NoCache(InternalServerError)) } } - def renderNewslettersPage()(implicit - executionContext: ExecutionContext = this.executionContext, - ): Action[AnyContent] = - csrfAddToken { - Action { implicit request => - if (request.forceDCR || UseDcrNewslettersPage.isSwitchedOn) { - remoteRenderNewslettersPage() - } else { - localRenderNewslettersPage() - } - } - } - private def renderDCRNewslettersJson()(implicit request: RequestHeader, - ): Result = { + ): Future[Result] = { val newsletters: Either[String, List[NewsletterResponseV2]] = newsletterSignupAgent.getV2Newsletters() @@ -100,7 +87,7 @@ class SignupPageController( val dataModel = DotcomNewslettersPageRenderingDataModel.apply(page, newsletters, request) val dataJson = DotcomNewslettersPageRenderingDataModel.toJson(dataModel) - common.renderJson(dataJson, page).as("application/json") + Future.successful(common.renderJson(dataJson, page).as("application/json")) } case Left(e) => log.error(s"API call to get newsletters failed: $e") @@ -108,21 +95,27 @@ class SignupPageController( } } - private def renderNewslettersJson()(implicit + private def notFoundPage()(implicit request: RequestHeader, - ): Result = { - Cached(CacheTime.NotFound)(Cached.WithoutRevalidationResult(NotFound)) + ): Future[Result] = { + Future(Cached(CacheTime.NotFound)(Cached.WithoutRevalidationResult(NotFound))) } - def renderNewslettersJson()(implicit + def renderNewsletters()(implicit executionContext: ExecutionContext = this.executionContext, ): Action[AnyContent] = csrfAddToken { - Action { implicit request => - if (request.forceDCR || UseDcrNewslettersPage.isSwitchedOn) { - renderDCRNewslettersJson() - } else { - renderNewslettersJson() + Action.async { implicit request => + val useDCR = request.forceDCR || UseDcrNewslettersPage.isSwitchedOn + + request.getRequestFormat match { + case HtmlFormat if useDCR => + remoteRenderNewslettersPage() + case HtmlFormat => + localRenderNewslettersPage() + case JsonFormat if useDCR => + renderDCRNewslettersJson() + case _ => notFoundPage() } } } diff --git a/applications/conf/routes b/applications/conf/routes index 6754d70706fd..316c38ca8d1a 100644 --- a/applications/conf/routes +++ b/applications/conf/routes @@ -10,8 +10,8 @@ GET /_healthcheck GET /sitemaps/news.xml controllers.SiteMapController.renderNewsSiteMap() GET /sitemaps/video.xml controllers.SiteMapController.renderVideoSiteMap() -GET /email-newsletters.json controllers.SignupPageController.renderNewslettersJson() -GET /email-newsletters controllers.SignupPageController.renderNewslettersPage() +GET /email-newsletters.json controllers.SignupPageController.renderNewsletters() +GET /email-newsletters controllers.SignupPageController.renderNewsletters() GET /survey/:formName/show controllers.SurveyPageController.renderFormStackSurvey(formName) GET /survey/thankyou controllers.SurveyPageController.thankYou() diff --git a/common/app/renderers/DotcomRenderingService.scala b/common/app/renderers/DotcomRenderingService.scala index 2385f3d4cc51..40ed2f83ea8f 100644 --- a/common/app/renderers/DotcomRenderingService.scala +++ b/common/app/renderers/DotcomRenderingService.scala @@ -347,7 +347,7 @@ class DotcomRenderingService extends GuLogging with ResultWithPreconnectPreload val dataModel = DotcomNewslettersPageRenderingDataModel.apply(page, newsletters, request) val json = DotcomNewslettersPageRenderingDataModel.toJson(dataModel) - post(ws, json, Configuration.rendering.faciaBaseURL + "/EmailNewsletters", CacheTime.Facia) + post(ws, json, Configuration.rendering.baseURL + "/EmailNewsletters", CacheTime.Facia) } def getImageContent( From 464cf02bfc00b3e796ec22a84a6f7f1e28ed24fd Mon Sep 17 00:00:00 2001 From: Charlotte <charlotte.emms@theguardian.com> Date: Mon, 5 Feb 2024 15:59:46 +0000 Subject: [PATCH 2/2] keep email newsletters page using facia baseurl --- common/app/renderers/DotcomRenderingService.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/app/renderers/DotcomRenderingService.scala b/common/app/renderers/DotcomRenderingService.scala index 40ed2f83ea8f..2385f3d4cc51 100644 --- a/common/app/renderers/DotcomRenderingService.scala +++ b/common/app/renderers/DotcomRenderingService.scala @@ -347,7 +347,7 @@ class DotcomRenderingService extends GuLogging with ResultWithPreconnectPreload val dataModel = DotcomNewslettersPageRenderingDataModel.apply(page, newsletters, request) val json = DotcomNewslettersPageRenderingDataModel.toJson(dataModel) - post(ws, json, Configuration.rendering.baseURL + "/EmailNewsletters", CacheTime.Facia) + post(ws, json, Configuration.rendering.faciaBaseURL + "/EmailNewsletters", CacheTime.Facia) } def getImageContent(