From cafa83de941549b8f1767d584ba4b3cbcb0b2bd3 Mon Sep 17 00:00:00 2001 From: Joe Cowton Date: Wed, 29 Jun 2022 14:37:43 +0100 Subject: [PATCH 1/5] Rename TopMentionsResult to Topic --- .../app/controllers/LiveBlogController.scala | 16 ++++++------- article/app/model/LiveBlogHelpers.scala | 4 ++-- article/app/topmentions/TopicService.scala | 4 ++-- article/test/LiveBlogControllerTest.scala | 4 ++-- .../test/model/LiveBlogCurrentPageTest.scala | 4 ++-- .../topmentions/TopMentionsServiceTest.scala | 8 +++---- common/app/model/LiveBlogCurrentPage.scala | 24 +++++++++---------- common/app/model/TopMentionsModel.scala | 6 ++--- 8 files changed, 35 insertions(+), 35 deletions(-) diff --git a/article/app/controllers/LiveBlogController.scala b/article/app/controllers/LiveBlogController.scala index a2ef865d8b70..bdd191a0fbad 100644 --- a/article/app/controllers/LiveBlogController.scala +++ b/article/app/controllers/LiveBlogController.scala @@ -140,7 +140,7 @@ class LiveBlogController( path: String, range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], availableTopics: Option[Seq[AvailableTopic]], selectedTopics: Option[String], )(implicit @@ -203,7 +203,7 @@ class LiveBlogController( private[this] def getRange( lastUpdate: Option[String], page: Option[String], - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ): BlockRange = { (lastUpdate.map(ParseBlockId.fromBlockId), page.map(ParseBlockId.fromPageParam), topMentionResult) match { case (Some(ParsedBlockId(id)), _, _) => SinceBlockId(id) @@ -225,7 +225,7 @@ class LiveBlogController( isLivePage: Option[Boolean], filterKeyEvents: Boolean, requestedBodyBlocks: scala.collection.Map[String, Seq[Block]] = Map.empty, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], )(implicit request: RequestHeader): Future[Result] = { val remoteRender = !request.forceDCROff @@ -248,7 +248,7 @@ class LiveBlogController( page: PageWithStoryPackage, lastUpdateBlockId: SinceBlockId, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ): Seq[BodyBlock] = { val requestedBlocks = page.article.fields.blocks.toSeq.flatMap { _.requestedBodyBlocks.getOrElse(lastUpdateBlockId.around, Seq()) @@ -270,7 +270,7 @@ class LiveBlogController( requestedBodyBlocks: scala.collection.Map[String, Seq[Block]], lastUpdateBlockId: SinceBlockId, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ): Seq[Block] = { val blocksAround = requestedBodyBlocks.getOrElse(lastUpdateBlockId.around, Seq.empty).takeWhile { block => block.id != lastUpdateBlockId.lastUpdate @@ -300,7 +300,7 @@ class LiveBlogController( filterKeyEvents: Boolean, remoteRender: Boolean, requestedBodyBlocks: scala.collection.Map[String, Seq[Block]], - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], )(implicit request: RequestHeader): Future[Result] = { val newBlocks = getNewBlocks(page, lastUpdateBlockId, filterKeyEvents, topMentionResult) val newCapiBlocks = getNewBlocks(requestedBodyBlocks, lastUpdateBlockId, filterKeyEvents, topMentionResult) @@ -361,7 +361,7 @@ class LiveBlogController( path: String, range: BlockRange, filterKeyEvents: Boolean = false, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], )( render: (PageWithStoryPackage, Blocks) => Future[Result], )(implicit request: RequestHeader): Future[Result] = { @@ -378,7 +378,7 @@ class LiveBlogController( private[this] def responseToModelOrResult( range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], )(response: ItemResponse)(implicit request: RequestHeader): Either[(PageWithStoryPackage, Blocks), Result] = { val supportedContent: Option[ContentType] = response.content.filter(isSupported).map(Content(_)) val supportedContentResult: Either[ContentType, Result] = ModelOrResult(supportedContent, response) diff --git a/article/app/model/LiveBlogHelpers.scala b/article/app/model/LiveBlogHelpers.scala index 07ada2c58138..e8bf49e3bb45 100644 --- a/article/app/model/LiveBlogHelpers.scala +++ b/article/app/model/LiveBlogHelpers.scala @@ -40,7 +40,7 @@ object LiveBlogHelpers extends GuLogging { liveBlog: Article, range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ): Option[LiveBlogCurrentPage] = { val pageSize = if (liveBlog.content.tags.tags.map(_.id).contains("sport/sport")) 30 else 10 @@ -64,7 +64,7 @@ object LiveBlogHelpers extends GuLogging { response: ItemResponse, range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ): Either[LiveBlogPage, Status] = { val pageSize = if (liveBlog.content.tags.tags.map(_.id).contains("sport/sport")) 30 else 10 diff --git a/article/app/topmentions/TopicService.scala b/article/app/topmentions/TopicService.scala index eb37a1965ab6..dfa201d8b849 100644 --- a/article/app/topmentions/TopicService.scala +++ b/article/app/topmentions/TopicService.scala @@ -1,7 +1,7 @@ package topmentions import common.{Box, GuLogging} -import model.{TopicsApiResponse, TopMentionsResult, SelectedTopic, AvailableTopic} +import model.{TopicsApiResponse, Topic, SelectedTopic, AvailableTopic} import scala.concurrent.{ExecutionContext, Future} @@ -41,7 +41,7 @@ class TopicService(topicS3Client: TopicS3Client) extends GuLogging { def getSelectedTopic( blogId: String, topMentionEntity: SelectedTopic, - ): Option[TopMentionsResult] = { + ): Option[Topic] = { getBlogTopicsApiResponse(blogId).flatMap(_.results.find(result => { result.`type` == topMentionEntity.`type` && result.name == topMentionEntity.value })) diff --git a/article/test/LiveBlogControllerTest.scala b/article/test/LiveBlogControllerTest.scala index 4f314e92a679..468e17fce5ce 100644 --- a/article/test/LiveBlogControllerTest.scala +++ b/article/test/LiveBlogControllerTest.scala @@ -9,7 +9,7 @@ import play.api.test._ import play.api.test.Helpers._ import org.scalatest.{BeforeAndAfterAll, DoNotDiscover} import org.scalatestplus.mockito.MockitoSugar -import model.{LiveBlogPage, TopMentionsResult, SelectedTopic, TopMentionsTopicType, AvailableTopic, TopicsLiveBlog} +import model.{LiveBlogPage, Topic, SelectedTopic, TopMentionsTopicType, AvailableTopic, TopicsLiveBlog} import topmentions.{TopicS3Client, TopicService} import scala.concurrent.Future @@ -31,7 +31,7 @@ import scala.concurrent.Future trait Setup { var fakeTopMentionsService = mock[TopicService] var fakeDcr = new DCRFake() - val topMentionResult = TopMentionsResult( + val topMentionResult = Topic( name = "Fifa", `type` = TopMentionsTopicType.Org, blocks = Seq("56d08042e4b0d38537b1f70b"), diff --git a/article/test/model/LiveBlogCurrentPageTest.scala b/article/test/model/LiveBlogCurrentPageTest.scala index dbaf4d4370ff..5a24b666d61a 100644 --- a/article/test/model/LiveBlogCurrentPageTest.scala +++ b/article/test/model/LiveBlogCurrentPageTest.scala @@ -64,7 +64,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { range, filterKeyEvents = false, topMentionResult = Some( - TopMentionsResult( + Topic( `type` = TopMentionsTopicType.Org, name = "someName", blocks = Seq(), @@ -744,7 +744,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { topicName: String, blocks: Seq[String], ) = { - TopMentionsResult( + Topic( `type` = tpoicType, name = topicName, blocks = blocks, diff --git a/article/test/topmentions/TopMentionsServiceTest.scala b/article/test/topmentions/TopMentionsServiceTest.scala index fa33ac1592fd..243cabdb3bdf 100644 --- a/article/test/topmentions/TopMentionsServiceTest.scala +++ b/article/test/topmentions/TopMentionsServiceTest.scala @@ -1,6 +1,6 @@ package topmentions -import model.{TopicsApiResponse, TopMentionsResult, SelectedTopic, TopMentionsTopicType, AvailableTopic} +import model.{TopicsApiResponse, Topic, SelectedTopic, TopMentionsTopicType, AvailableTopic} import org.scalatest.{BeforeAndAfterAll} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -20,7 +20,7 @@ class TopMentionsServiceTest val fakeClient = mock[TopicS3Client] val topMentionResult = - TopMentionsResult( + Topic( name = "name1", `type` = TopMentionsTopicType.Org, blocks = Seq("blockId1"), @@ -29,14 +29,14 @@ class TopMentionsServiceTest ) val topMentionResults = Seq( - TopMentionsResult( + Topic( name = "name1", `type` = TopMentionsTopicType.Org, blocks = Seq("blockId1"), count = 1, percentage_blocks = 1.2f, ), - TopMentionsResult( + Topic( name = "name2", `type` = TopMentionsTopicType.Person, blocks = Seq("blockId1"), diff --git a/common/app/model/LiveBlogCurrentPage.scala b/common/app/model/LiveBlogCurrentPage.scala index 8c88b202054e..9c34876ce35f 100644 --- a/common/app/model/LiveBlogCurrentPage.scala +++ b/common/app/model/LiveBlogCurrentPage.scala @@ -18,7 +18,7 @@ object LiveBlogCurrentPage { blocks: Blocks, range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ): Option[LiveBlogCurrentPage] = { range match { case CanonicalLiveBlog | TopicsLiveBlog => firstPage(pageSize, blocks, filterKeyEvents, topMentionResult) @@ -36,7 +36,7 @@ object LiveBlogCurrentPage { blocks: Blocks, sinceBlockId: SinceBlockId, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ): Option[LiveBlogCurrentPage] = { val bodyBlocks = blocks.requestedBodyBlocks.get(sinceBlockId.around).toSeq.flatMap { bodyBlocks => @@ -53,7 +53,7 @@ object LiveBlogCurrentPage { pageSize: Int, blocks: Blocks, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ): Option[LiveBlogCurrentPage] = { val (maybeRequestedBodyBlocks, blockCount, oldestPageBlockId) = extractFirstPageBlocks(blocks, filterKeyEvents, topMentionResult) @@ -98,7 +98,7 @@ object LiveBlogCurrentPage { private def extractFirstPageBlocks( blocks: Blocks, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ) = { if (filterKeyEvents) { getKeyEventsBlocks(blocks) @@ -109,17 +109,17 @@ object LiveBlogCurrentPage { } } - private def isTopMentionBlock(topMentionsResult: TopMentionsResult)(bodyBlock: BodyBlock): Boolean = { + private def isTopMentionBlock(topMentionsResult: Topic)(bodyBlock: BodyBlock): Boolean = { topMentionsResult.blocks.contains(bodyBlock.id) } - private def filterBlocksByTopMentions(blocks: Seq[BodyBlock], topMentionsResult: TopMentionsResult) = { + private def filterBlocksByTopMentions(blocks: Seq[BodyBlock], topMentionsResult: Topic) = { blocks.filter(isTopMentionBlock(topMentionsResult)).sortBy(_.publishedCreatedTimestamp).reverse } private def getTopMentionsBlocks( blocks: Blocks, - topMentionsResult: TopMentionsResult, + topMentionsResult: Topic, ): (Option[Seq[BodyBlock]], Int, Option[String]) = { val bodyBlocks = blocks.body @@ -164,7 +164,7 @@ object LiveBlogCurrentPage { blocks: Seq[BodyBlock], isRequestedBlock: String, filterKeyEvents: Boolean, - topMentionsResult: Option[TopMentionsResult], + topMentionsResult: Option[Topic], ): Option[LiveBlogCurrentPage] = { val pinnedBlock = blocks.find(_.attributes.pinned).map(renamePinnedBlock) val filteredBlocks = applyFilters(blocks, filterKeyEvents, topMentionsResult) @@ -217,7 +217,7 @@ object LiveBlogCurrentPage { private def applyFilters( blocks: Seq[BodyBlock], filterKeyEvents: Boolean, - topMentionsResult: Option[TopMentionsResult], + topMentionsResult: Option[Topic], ) = { if (filterKeyEvents) { blocks.filter(block => block.eventType == KeyEvent || block.eventType == SummaryEvent) @@ -247,7 +247,7 @@ sealed trait PageReference { def isArchivePage: Boolean - def buildQueryParam(topMentionResult: Option[TopMentionsResult]) = { + def buildQueryParam(topMentionResult: Option[Topic]) = { topMentionResult match { case Some(value) => s"&topics=${topMentionResult.get.`type`}:${value.name}" case None => "" @@ -263,7 +263,7 @@ case class N1Pagination( numberOfPages: Int, ) -case class FirstPage(blocks: Seq[BodyBlock], filterKeyEvents: Boolean, topMentionResult: Option[TopMentionsResult]) +case class FirstPage(blocks: Seq[BodyBlock], filterKeyEvents: Boolean, topMentionResult: Option[Topic]) extends PageReference { val suffix = s"?filterKeyEvents=$filterKeyEvents${buildQueryParam(topMentionResult)}" val pageNumber = 1 @@ -275,7 +275,7 @@ case class BlockPage( blockId: String, pageNumber: Int, filterKeyEvents: Boolean, - topMentionResult: Option[TopMentionsResult], + topMentionResult: Option[Topic], ) extends PageReference { val suffix = s"?page=with:block-$blockId&filterKeyEvents=$filterKeyEvents${buildQueryParam(topMentionResult)}" val isArchivePage = true diff --git a/common/app/model/TopMentionsModel.scala b/common/app/model/TopMentionsModel.scala index d9d1f88059a4..101fcd4a3b45 100644 --- a/common/app/model/TopMentionsModel.scala +++ b/common/app/model/TopMentionsModel.scala @@ -4,19 +4,19 @@ import common.GuLogging import model.TopMentionsTopicType.TopMentionsTopicType import play.api.libs.json.{Format, Json} -case class TopMentionsResult( +case class Topic( name: String, `type`: TopMentionsTopicType, blocks: Seq[String], count: Int, percentage_blocks: Float, ) -case class TopicsApiResponse(entity_types: Seq[TopMentionsTopicType], results: Seq[TopMentionsResult], model: String) +case class TopicsApiResponse(entity_types: Seq[TopMentionsTopicType], results: Seq[Topic], model: String) case class TopMentionJsonParseException(message: String) extends Exception(message) object TopicsApiResponse { - implicit val TopMentionsResultJf: Format[TopMentionsResult] = Json.format[TopMentionsResult] + implicit val TopicJf: Format[Topic] = Json.format[Topic] implicit val TopicsApiResponseJf: Format[TopicsApiResponse] = Json.format[TopicsApiResponse] } From fe4cc391e66e44081d6306a13a366b0c0a0b404f Mon Sep 17 00:00:00 2001 From: Joe Cowton Date: Wed, 29 Jun 2022 15:26:27 +0100 Subject: [PATCH 2/5] Renaming --- .../app/controllers/LiveBlogController.scala | 72 ++++++++--------- article/app/topmentions/TopicService.scala | 2 +- article/test/LiveBlogControllerTest.scala | 6 +- .../test/model/LiveBlogCurrentPageTest.scala | 80 +++++++++---------- common/app/model/LiveBlogCurrentPage.scala | 70 ++++++++-------- 5 files changed, 115 insertions(+), 115 deletions(-) diff --git a/article/app/controllers/LiveBlogController.scala b/article/app/controllers/LiveBlogController.scala index bdd191a0fbad..523acaf5283c 100644 --- a/article/app/controllers/LiveBlogController.scala +++ b/article/app/controllers/LiveBlogController.scala @@ -30,7 +30,7 @@ class LiveBlogController( val controllerComponents: ControllerComponents, ws: WSClient, remoteRenderer: renderers.DotcomRenderingService = DotcomRenderingService(), - topMentionsService: TopicService, + topicService: TopicService, )(implicit context: ApplicationContext) extends BaseController with GuLogging @@ -45,7 +45,7 @@ class LiveBlogController( def renderEmail(path: String): Action[AnyContent] = { Action.async { implicit request => - mapModel(path, ArticleBlocks, topMentionResult = None) { + mapModel(path, ArticleBlocks, maybeTopic = None) { case (minute: MinutePage, _) => Future.successful(common.renderEmail(ArticleEmailHtmlPage.html(minute), minute)) case (blog: LiveBlogPage, _) => Future.successful(common.renderEmail(LiveBlogHtmlPage.html(blog), blog)) @@ -62,8 +62,8 @@ class LiveBlogController( ): Action[AnyContent] = { Action.async { implicit request => val filter = shouldFilter(filterKeyEvents) - val topMentions = if (filter) None else getTopMentions(path, topics) - val availableTopics = topMentionsService.getTopics(path) + val topMentions = if (filter) None else getTopics(path, topics) + val availableTopics = topicService.getAvailableTopics(path) page.map(ParseBlockId.fromPageParam) match { case Some(ParsedBlockId(id)) => @@ -116,16 +116,16 @@ class LiveBlogController( ): Action[AnyContent] = { Action.async { implicit request: Request[AnyContent] => val filter = shouldFilter(filterKeyEvents) - val topMentionResult = getTopMentions(path, topics) - val range = getRange(lastUpdate, page, topMentionResult) - val availableTopics = topMentionsService.getTopics(path) + val maybeTopic = getTopics(path, topics) + val range = getRange(lastUpdate, page, maybeTopic) + val availableTopics = topicService.getAvailableTopics(path) - mapModel(path, range, filter, topMentionResult) { + mapModel(path, range, filter, maybeTopic) { case (blog: LiveBlogPage, _) if rendered.contains(false) => getJsonForFronts(blog) case (blog: LiveBlogPage, blocks) if request.forceDCR && lastUpdate.isEmpty => Future.successful(renderGuuiJson(blog, blocks, filter, availableTopics, selectedTopics = topics)) case (blog: LiveBlogPage, blocks) => - getJson(blog, range, isLivePage, filter, blocks.requestedBodyBlocks.getOrElse(Map.empty), topMentionResult) + getJson(blog, range, isLivePage, filter, blocks.requestedBodyBlocks.getOrElse(Map.empty), maybeTopic) case (minute: MinutePage, _) => Future.successful(common.renderJson(views.html.fragments.minuteBody(minute), minute)) case _ => @@ -140,13 +140,13 @@ class LiveBlogController( path: String, range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], availableTopics: Option[Seq[AvailableTopic]], selectedTopics: Option[String], )(implicit request: RequestHeader, ): Future[Result] = { - mapModel(path, range, filterKeyEvents, topMentionResult) { (page, blocks) => + mapModel(path, range, filterKeyEvents, maybeTopic) { (page, blocks) => { val isAmpSupported = page.article.content.shouldAmplify val pageType: PageType = PageType(page, request, context) @@ -203,9 +203,9 @@ class LiveBlogController( private[this] def getRange( lastUpdate: Option[String], page: Option[String], - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], ): BlockRange = { - (lastUpdate.map(ParseBlockId.fromBlockId), page.map(ParseBlockId.fromPageParam), topMentionResult) match { + (lastUpdate.map(ParseBlockId.fromBlockId), page.map(ParseBlockId.fromPageParam), maybeTopic) match { case (Some(ParsedBlockId(id)), _, _) => SinceBlockId(id) case (_, Some(ParsedBlockId(id)), _) => PageWithBlock(id) case (_, _, Some(_)) => TopicsLiveBlog @@ -225,7 +225,7 @@ class LiveBlogController( isLivePage: Option[Boolean], filterKeyEvents: Boolean, requestedBodyBlocks: scala.collection.Map[String, Seq[Block]] = Map.empty, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], )(implicit request: RequestHeader): Future[Result] = { val remoteRender = !request.forceDCROff @@ -238,7 +238,7 @@ class LiveBlogController( filterKeyEvents, remoteRender, requestedBodyBlocks, - topMentionResult, + maybeTopic, ) case _ => Future.successful(common.renderJson(views.html.liveblog.liveBlogBody(liveblog), liveblog)) } @@ -248,7 +248,7 @@ class LiveBlogController( page: PageWithStoryPackage, lastUpdateBlockId: SinceBlockId, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], ): Seq[BodyBlock] = { val requestedBlocks = page.article.fields.blocks.toSeq.flatMap { _.requestedBodyBlocks.getOrElse(lastUpdateBlockId.around, Seq()) @@ -260,8 +260,8 @@ class LiveBlogController( if (filterKeyEvents) { latestBlocks.filter(block => block.eventType == KeyEvent || block.eventType == SummaryEvent) - } else if (topMentionResult.isDefined) { - latestBlocks.filter(block => topMentionResult.get.blocks.contains(block.id)) + } else if (maybeTopic.isDefined) { + latestBlocks.filter(block => maybeTopic.get.blocks.contains(block.id)) } else latestBlocks } @@ -270,7 +270,7 @@ class LiveBlogController( requestedBodyBlocks: scala.collection.Map[String, Seq[Block]], lastUpdateBlockId: SinceBlockId, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], ): Seq[Block] = { val blocksAround = requestedBodyBlocks.getOrElse(lastUpdateBlockId.around, Seq.empty).takeWhile { block => block.id != lastUpdateBlockId.lastUpdate @@ -280,8 +280,8 @@ class LiveBlogController( blocksAround.filter(block => block.attributes.keyEvent.getOrElse(false) || block.attributes.summary.getOrElse(false), ) - } else if (topMentionResult.isDefined) { - blocksAround.filter(block => topMentionResult.get.blocks.contains(block.id)) + } else if (maybeTopic.isDefined) { + blocksAround.filter(block => maybeTopic.get.blocks.contains(block.id)) } else blocksAround } @@ -300,10 +300,10 @@ class LiveBlogController( filterKeyEvents: Boolean, remoteRender: Boolean, requestedBodyBlocks: scala.collection.Map[String, Seq[Block]], - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], )(implicit request: RequestHeader): Future[Result] = { - val newBlocks = getNewBlocks(page, lastUpdateBlockId, filterKeyEvents, topMentionResult) - val newCapiBlocks = getNewBlocks(requestedBodyBlocks, lastUpdateBlockId, filterKeyEvents, topMentionResult) + val newBlocks = getNewBlocks(page, lastUpdateBlockId, filterKeyEvents, maybeTopic) + val newCapiBlocks = getNewBlocks(requestedBodyBlocks, lastUpdateBlockId, filterKeyEvents, maybeTopic) val timelineHtml = views.html.liveblog.keyEvents( "", @@ -361,13 +361,13 @@ class LiveBlogController( path: String, range: BlockRange, filterKeyEvents: Boolean = false, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], )( render: (PageWithStoryPackage, Blocks) => Future[Result], )(implicit request: RequestHeader): Future[Result] = { capiLookup .lookup(path, Some(range)) - .map(responseToModelOrResult(range, filterKeyEvents, topMentionResult)) + .map(responseToModelOrResult(range, filterKeyEvents, maybeTopic)) .recover(convertApiExceptions) .flatMap { case Left((model, blocks)) => render(model, blocks) @@ -378,7 +378,7 @@ class LiveBlogController( private[this] def responseToModelOrResult( range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], )(response: ItemResponse)(implicit request: RequestHeader): Either[(PageWithStoryPackage, Blocks), Result] = { val supportedContent: Option[ContentType] = response.content.filter(isSupported).map(Content(_)) val supportedContentResult: Either[ContentType, Result] = ModelOrResult(supportedContent, response) @@ -395,7 +395,7 @@ class LiveBlogController( response, range, filterKeyEvents, - topMentionResult, + maybeTopic, ).left .map(_ -> blocks) case unknown => @@ -410,17 +410,17 @@ class LiveBlogController( filterKeyEvents.getOrElse(false) } - def getTopMentions(blogId: String, maybeTopic: Option[String]) = { - val topMentionsResult = for { + def getTopics(blogId: String, maybeTopic: Option[String]) = { + val topics = for { selectedTopic <- SelectedTopic.fromString(maybeTopic) - topMentions <- topMentionsService.getSelectedTopic(blogId, selectedTopic) - } yield topMentions + topics <- topicService.getSelectedTopic(blogId, selectedTopic) + } yield topics - topMentionsResult match { - case Some(_) => log.info(s"top mention result was successfully retrieved for ${maybeTopic.get}") - case None => if (maybeTopic.isDefined) log.warn(s"top mention result couldn't be retrieved for ${maybeTopic.get}") + topics match { + case Some(_) => log.info(s"topic was successfully retrieved for ${maybeTopic.get}") + case None => if (maybeTopic.isDefined) log.warn(s"topic result couldn't be retrieved for ${maybeTopic.get}") } - topMentionsResult + topics } } diff --git a/article/app/topmentions/TopicService.scala b/article/app/topmentions/TopicService.scala index dfa201d8b849..9032dfaef3ac 100644 --- a/article/app/topmentions/TopicService.scala +++ b/article/app/topmentions/TopicService.scala @@ -28,7 +28,7 @@ class TopicService(topicS3Client: TopicS3Client) extends GuLogging { topicsDetails.get().flatMap(_.get(blogId)) } - def getTopics(blogId: String): Option[Seq[AvailableTopic]] = { + def getAvailableTopics(blogId: String): Option[Seq[AvailableTopic]] = { getBlogTopicsApiResponse(blogId).map(mentions => mentions.results.map(mention => AvailableTopic(mention.`type`, mention.name, mention.count)), ) diff --git a/article/test/LiveBlogControllerTest.scala b/article/test/LiveBlogControllerTest.scala index 468e17fce5ce..9736ee1e2e58 100644 --- a/article/test/LiveBlogControllerTest.scala +++ b/article/test/LiveBlogControllerTest.scala @@ -286,15 +286,15 @@ import scala.concurrent.Future } "getTopMentionsForFilters" should "returns none given no automatic filter query parameter" in new Setup { - liveBlogController.getTopMentions(path, None) should be(None) + liveBlogController.getTopics(path, None) should be(None) } "getTopMentionsForFilters" should "returns none given an incorrect automatic filter query parameter" in new Setup { - liveBlogController.getTopMentions(path, Some("orgFifa")) should be(None) + liveBlogController.getTopics(path, Some("orgFifa")) should be(None) } "getTopMentionsForFilters" should "returns correct topMentionResult given a correct automatic filter query parameter" in new Setup { - liveBlogController.getTopMentions(path, Some("org:Fifa")) should be(Some(topMentionResult)) + liveBlogController.getTopics(path, Some("org:Fifa")) should be(Some(topMentionResult)) } "renderArticle" should "returns the first page of filtered blog by topics" in new Setup { diff --git a/article/test/model/LiveBlogCurrentPageTest.scala b/article/test/model/LiveBlogCurrentPageTest.scala index 5a24b666d61a..e8a684e93b67 100644 --- a/article/test/model/LiveBlogCurrentPageTest.scala +++ b/article/test/model/LiveBlogCurrentPageTest.scala @@ -63,7 +63,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blocks = Blocks(1, Seq(), None, Map()), range, filterKeyEvents = false, - topMentionResult = Some( + maybeTopic = Some( Topic( `type` = TopMentionsTopicType.Org, name = "someName", @@ -90,7 +90,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { result should be( Some( LiveBlogCurrentPage( - currentPage = FirstPage(Seq(fakeBlock(1)), filterKeyEvents = false, topMentionResult = None), + currentPage = FirstPage(Seq(fakeBlock(1)), filterKeyEvents = false, maybeTopic = None), pagination = None, pinnedBlock = None, ), @@ -251,7 +251,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - should(result, currentPage = FirstPage(List(), true, topMentionResult = None), pagination = None) + should(result, currentPage = FirstPage(List(), true, maybeTopic = None), pagination = None) } it should "allow 3 blocks on one page" in { @@ -269,7 +269,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - should(result, FirstPage(blocks, filterKeyEvents = false, topMentionResult = None), None) + should(result, FirstPage(blocks, filterKeyEvents = false, maybeTopic = None), None) } it should "put 4 blocks on two pages (main page)" in { @@ -288,9 +288,9 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { val expected = blocks.take(2) val expectedOldestPage = - BlockPage(blocks = Nil, blockId = "1", pageNumber = 2, filterKeyEvents = false, topMentionResult = None) + BlockPage(blocks = Nil, blockId = "1", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) val expectedOlderPage = - BlockPage(blocks = Nil, blockId = "2", pageNumber = 2, filterKeyEvents = false, topMentionResult = None) + BlockPage(blocks = Nil, blockId = "2", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) val expectedPagination = Some( N1Pagination( newest = None, @@ -318,11 +318,11 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - val expectedCurrentPage = FirstPage(blocks = blocks.take(3), filterKeyEvents = false, topMentionResult = None) + val expectedCurrentPage = FirstPage(blocks = blocks.take(3), filterKeyEvents = false, maybeTopic = None) val expectedOldestPage = - BlockPage(blocks = Nil, blockId = "1", pageNumber = 2, filterKeyEvents = false, topMentionResult = None) + BlockPage(blocks = Nil, blockId = "1", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) val expectedOlderPage = - BlockPage(blocks = Nil, blockId = "2", pageNumber = 2, filterKeyEvents = false, topMentionResult = None) + BlockPage(blocks = Nil, blockId = "2", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) val expectedPagination = Some( N1Pagination( newest = None, @@ -357,11 +357,11 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - val expectedCurrentPage = FirstPage(blocks = keyBlocks.take(3), filterKeyEvents = true, topMentionResult = None) + val expectedCurrentPage = FirstPage(blocks = keyBlocks.take(3), filterKeyEvents = true, maybeTopic = None) val expectedOldestPage = - BlockPage(blocks = Nil, blockId = "1", pageNumber = 3, filterKeyEvents = true, topMentionResult = None) + BlockPage(blocks = Nil, blockId = "1", pageNumber = 3, filterKeyEvents = true, maybeTopic = None) val expectedOlderPage = - BlockPage(blocks = Nil, blockId = "5", pageNumber = 2, filterKeyEvents = true, topMentionResult = None) + BlockPage(blocks = Nil, blockId = "5", pageNumber = 2, filterKeyEvents = true, maybeTopic = None) val expectedPagination = Some( N1Pagination( newest = None, @@ -383,7 +383,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { pageSize = 2, blocks = testFakeBlocks.blocksType, filterKeyEvents = true, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ) result.get should be( @@ -391,7 +391,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { currentPage = FirstPage( testFakeBlocks.blocksSequence.slice(3, 5), filterKeyEvents = true, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ), pagination = None, pinnedBlock = None, @@ -407,7 +407,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { pageSize = 2, blocks = testFakeBlocks.blocksType, filterKeyEvents = false, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ) val expectedPagination = Some( @@ -420,7 +420,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 2, filterKeyEvents = false, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ), ), oldest = Some( @@ -429,7 +429,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "1", pageNumber = 2, filterKeyEvents = false, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ), ), numberOfPages = 2, @@ -441,7 +441,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { currentPage = FirstPage( testFakeBlocks.blocksSequence.slice(1, 3), filterKeyEvents = false, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ), pagination = expectedPagination, ) @@ -457,9 +457,9 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 2, filterKeyEvents = false, - topMentionResult = None, + maybeTopic = None, ) - val expectedNewestPage = FirstPage(blocks.take(2), filterKeyEvents = false, topMentionResult = None) + val expectedNewestPage = FirstPage(blocks.take(2), filterKeyEvents = false, maybeTopic = None) val expectedPagination = Some( N1Pagination( newest = Some(expectedNewestPage), @@ -483,9 +483,9 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 2, filterKeyEvents = false, - topMentionResult = None, + maybeTopic = None, ) - val expectedNewestPage = FirstPage(blocks.take(2), filterKeyEvents = false, topMentionResult = None) + val expectedNewestPage = FirstPage(blocks.take(2), filterKeyEvents = false, maybeTopic = None) val expectedPagination = Some( N1Pagination( newest = Some(expectedNewestPage), @@ -509,9 +509,9 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 2, filterKeyEvents = false, - topMentionResult = None, + maybeTopic = None, ) - val expectedNewestPage = FirstPage(blocks.take(3), filterKeyEvents = false, topMentionResult = None) + val expectedNewestPage = FirstPage(blocks.take(3), filterKeyEvents = false, maybeTopic = None) val expectedPagination = Some( N1Pagination( newest = Some(expectedNewestPage), @@ -539,11 +539,11 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - val expectedCurrentPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, topMentionResult = None) + val expectedCurrentPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, maybeTopic = None) val expectedMiddlePage = - BlockPage(blocks = Nil, blockId = "4", pageNumber = 2, filterKeyEvents = false, topMentionResult = None) + BlockPage(blocks = Nil, blockId = "4", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) val expectedOldestPage = - BlockPage(blocks = Nil, blockId = "1", pageNumber = 3, filterKeyEvents = false, topMentionResult = None) + BlockPage(blocks = Nil, blockId = "1", pageNumber = 3, filterKeyEvents = false, maybeTopic = None) val expectedPagination = Some( N1Pagination( newest = None, @@ -567,16 +567,16 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "4", pageNumber = 2, filterKeyEvents = false, - topMentionResult = None, + maybeTopic = None, ) - val expectedFirstPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, topMentionResult = None) + val expectedFirstPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, maybeTopic = None) val expectedOlderPage = BlockPage( blocks = blocks.takeRight(2), blockId = "2", pageNumber = 3, filterKeyEvents = false, - topMentionResult = None, + maybeTopic = None, ) val expectedPagination = Some( N1Pagination( @@ -601,16 +601,16 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 3, filterKeyEvents = false, - topMentionResult = None, + maybeTopic = None, ) - val expectedFirstPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, topMentionResult = None) + val expectedFirstPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, maybeTopic = None) val expectedMiddlePage = BlockPage( blocks = blocks.slice(2, 4), blockId = "4", pageNumber = 2, filterKeyEvents = false, - topMentionResult = None, + maybeTopic = None, ) val expectedPagination = Some( N1Pagination( @@ -636,18 +636,18 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 3, filterKeyEvents = true, - topMentionResult = None, + maybeTopic = None, ) } val expectedFirstPage = - FirstPage(blocks = keyAndSummaryBlocks.take(2), filterKeyEvents = true, topMentionResult = None) + FirstPage(blocks = keyAndSummaryBlocks.take(2), filterKeyEvents = true, maybeTopic = None) val expectedMiddlePage = BlockPage( blocks = keyAndSummaryBlocks.slice(2, 4), blockId = "4", pageNumber = 2, filterKeyEvents = true, - topMentionResult = None, + maybeTopic = None, ) val expectedPagination = Some( N1Pagination( @@ -676,14 +676,14 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { val expectedFirstPage = FirstPage( testFakeBlocks.blocksSequence.slice(2, 5), filterKeyEvents = false, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ) val expectedCurrentPage = BlockPage( blocks = testFakeBlocks.blocksSequence.slice(5, 7), blockId = "3", pageNumber = 2, filterKeyEvents = false, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ) val expectedPagination = Some( N1Pagination( @@ -715,7 +715,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blocks = testFakeBlocks.blocksType, sinceBlockId = SinceBlockId("5"), filterKeyEvents = false, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ) result.get.currentPage.blocks.length should be(1) @@ -731,7 +731,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blocks = testFakeBlocks.blocksType, sinceBlockId = SinceBlockId(sinceBlockId), filterKeyEvents = false, - topMentionResult = Some(topMentions), + maybeTopic = Some(topMentions), ) result.get.currentPage.blocks.length should be(2) diff --git a/common/app/model/LiveBlogCurrentPage.scala b/common/app/model/LiveBlogCurrentPage.scala index 9c34876ce35f..ff7223c0f19b 100644 --- a/common/app/model/LiveBlogCurrentPage.scala +++ b/common/app/model/LiveBlogCurrentPage.scala @@ -18,13 +18,13 @@ object LiveBlogCurrentPage { blocks: Blocks, range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], ): Option[LiveBlogCurrentPage] = { range match { - case CanonicalLiveBlog | TopicsLiveBlog => firstPage(pageSize, blocks, filterKeyEvents, topMentionResult) + case CanonicalLiveBlog | TopicsLiveBlog => firstPage(pageSize, blocks, filterKeyEvents, maybeTopic) case PageWithBlock(isRequestedBlock) => - findPageWithBlock(pageSize, blocks.body, isRequestedBlock, filterKeyEvents, topMentionResult) - case SinceBlockId(blockId) => updates(blocks, SinceBlockId(blockId), filterKeyEvents, topMentionResult) + findPageWithBlock(pageSize, blocks.body, isRequestedBlock, filterKeyEvents, maybeTopic) + case SinceBlockId(blockId) => updates(blocks, SinceBlockId(blockId), filterKeyEvents, maybeTopic) case ArticleBlocks => None case GenericFallback => None case _ => None @@ -36,15 +36,15 @@ object LiveBlogCurrentPage { blocks: Blocks, sinceBlockId: SinceBlockId, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], ): Option[LiveBlogCurrentPage] = { val bodyBlocks = blocks.requestedBodyBlocks.get(sinceBlockId.around).toSeq.flatMap { bodyBlocks => val onlyBlocksAfterLastUpdated = bodyBlocks.takeWhile(_.id != sinceBlockId.lastUpdate) - applyFilters(onlyBlocksAfterLastUpdated, filterKeyEvents, topMentionResult) + applyFilters(onlyBlocksAfterLastUpdated, filterKeyEvents, maybeTopic) } Some( - LiveBlogCurrentPage(FirstPage(bodyBlocks, filterKeyEvents, topMentionResult), None, None), + LiveBlogCurrentPage(FirstPage(bodyBlocks, filterKeyEvents, maybeTopic), None, None), ) // just pretend to be the first page, it'll be ignored } @@ -53,10 +53,10 @@ object LiveBlogCurrentPage { pageSize: Int, blocks: Blocks, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], ): Option[LiveBlogCurrentPage] = { val (maybeRequestedBodyBlocks, blockCount, oldestPageBlockId) = - extractFirstPageBlocks(blocks, filterKeyEvents, topMentionResult) + extractFirstPageBlocks(blocks, filterKeyEvents, maybeTopic) val remainder = blockCount % pageSize val numPages = blockCount / pageSize @@ -65,11 +65,11 @@ object LiveBlogCurrentPage { val (firstPageBlocks, startOfSecondPageBlocks) = requestedBodyBlocks.splitAt(remainder + pageSize) val olderPage = startOfSecondPageBlocks.headOption.map { block => - BlockPage(blocks = Nil, blockId = block.id, pageNumber = 2, filterKeyEvents, topMentionResult) + BlockPage(blocks = Nil, blockId = block.id, pageNumber = 2, filterKeyEvents, maybeTopic) } val oldestPage = oldestPageBlockId map { blockId => - BlockPage(blocks = Nil, blockId = blockId, pageNumber = numPages, filterKeyEvents, topMentionResult) + BlockPage(blocks = Nil, blockId = blockId, pageNumber = numPages, filterKeyEvents, maybeTopic) } val pinnedBlocks = blocks.requestedBodyBlocks.get(CanonicalLiveBlog.pinned) @@ -91,39 +91,39 @@ object LiveBlogCurrentPage { else None } - LiveBlogCurrentPage(FirstPage(blocksToDisplay, filterKeyEvents, topMentionResult), pagination, pinnedBlockRenamed) + LiveBlogCurrentPage(FirstPage(blocksToDisplay, filterKeyEvents, maybeTopic), pagination, pinnedBlockRenamed) } } private def extractFirstPageBlocks( blocks: Blocks, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], ) = { if (filterKeyEvents) { getKeyEventsBlocks(blocks) - } else if (topMentionResult.isDefined) { - getTopMentionsBlocks(blocks, topMentionResult.get) + } else if (maybeTopic.isDefined) { + getTopMentionsBlocks(blocks, maybeTopic.get) } else { getStandardBlocks(blocks) } } - private def isTopMentionBlock(topMentionsResult: Topic)(bodyBlock: BodyBlock): Boolean = { - topMentionsResult.blocks.contains(bodyBlock.id) + private def isTopicBlock(topic: Topic)(bodyBlock: BodyBlock): Boolean = { + topic.blocks.contains(bodyBlock.id) } - private def filterBlocksByTopMentions(blocks: Seq[BodyBlock], topMentionsResult: Topic) = { - blocks.filter(isTopMentionBlock(topMentionsResult)).sortBy(_.publishedCreatedTimestamp).reverse + private def filterBlocksByTopic(blocks: Seq[BodyBlock], topic: Topic) = { + blocks.filter(isTopicBlock(topic)).sortBy(_.publishedCreatedTimestamp).reverse } private def getTopMentionsBlocks( blocks: Blocks, - topMentionsResult: Topic, + topic: Topic, ): (Option[Seq[BodyBlock]], Int, Option[String]) = { val bodyBlocks = blocks.body - val filteredBodyBlocks = filterBlocksByTopMentions(bodyBlocks, topMentionsResult) + val filteredBodyBlocks = filterBlocksByTopic(bodyBlocks, topic) (Some(filteredBodyBlocks), filteredBodyBlocks.length, filteredBodyBlocks.lastOption.map(_.id)) } @@ -164,17 +164,17 @@ object LiveBlogCurrentPage { blocks: Seq[BodyBlock], isRequestedBlock: String, filterKeyEvents: Boolean, - topMentionsResult: Option[Topic], + maybeTopic: Option[Topic], ): Option[LiveBlogCurrentPage] = { val pinnedBlock = blocks.find(_.attributes.pinned).map(renamePinnedBlock) - val filteredBlocks = applyFilters(blocks, filterKeyEvents, topMentionsResult) + val filteredBlocks = applyFilters(blocks, filterKeyEvents, maybeTopic) val (mainPageBlocks, restPagesBlocks) = getPages(pageSize, filteredBlocks) - val newestPage = FirstPage(mainPageBlocks, filterKeyEvents, topMentionsResult) + val newestPage = FirstPage(mainPageBlocks, filterKeyEvents, maybeTopic) val pages = newestPage :: restPagesBlocks.zipWithIndex .map { case (page, index) => // page number is index + 2 to account for first page and 0 based index - BlockPage(blocks = page, blockId = page.head.id, pageNumber = index + 2, filterKeyEvents, topMentionsResult) + BlockPage(blocks = page, blockId = page.head.id, pageNumber = index + 2, filterKeyEvents, maybeTopic) } val oldestPage = pages.lastOption.getOrElse(newestPage) @@ -217,12 +217,12 @@ object LiveBlogCurrentPage { private def applyFilters( blocks: Seq[BodyBlock], filterKeyEvents: Boolean, - topMentionsResult: Option[Topic], + maybeTopic: Option[Topic], ) = { if (filterKeyEvents) { blocks.filter(block => block.eventType == KeyEvent || block.eventType == SummaryEvent) - } else if (topMentionsResult.isDefined) { - filterBlocksByTopMentions(blocks, topMentionsResult.get) + } else if (maybeTopic.isDefined) { + filterBlocksByTopic(blocks, maybeTopic.get) } else { blocks } @@ -247,9 +247,9 @@ sealed trait PageReference { def isArchivePage: Boolean - def buildQueryParam(topMentionResult: Option[Topic]) = { - topMentionResult match { - case Some(value) => s"&topics=${topMentionResult.get.`type`}:${value.name}" + def buildQueryParam(maybeTopic: Option[Topic]) = { + maybeTopic match { + case Some(value) => s"&topics=${maybeTopic.get.`type`}:${value.name}" case None => "" } } @@ -263,9 +263,9 @@ case class N1Pagination( numberOfPages: Int, ) -case class FirstPage(blocks: Seq[BodyBlock], filterKeyEvents: Boolean, topMentionResult: Option[Topic]) +case class FirstPage(blocks: Seq[BodyBlock], filterKeyEvents: Boolean, maybeTopic: Option[Topic]) extends PageReference { - val suffix = s"?filterKeyEvents=$filterKeyEvents${buildQueryParam(topMentionResult)}" + val suffix = s"?filterKeyEvents=$filterKeyEvents${buildQueryParam(maybeTopic)}" val pageNumber = 1 val isArchivePage = false } @@ -275,9 +275,9 @@ case class BlockPage( blockId: String, pageNumber: Int, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + maybeTopic: Option[Topic], ) extends PageReference { - val suffix = s"?page=with:block-$blockId&filterKeyEvents=$filterKeyEvents${buildQueryParam(topMentionResult)}" + val suffix = s"?page=with:block-$blockId&filterKeyEvents=$filterKeyEvents${buildQueryParam(maybeTopic)}" val isArchivePage = true } From 2d1d6518a4ee0f47cb43aa0436c70a8ad4aa4264 Mon Sep 17 00:00:00 2001 From: Joe Cowton Date: Wed, 29 Jun 2022 16:34:35 +0100 Subject: [PATCH 3/5] Rename all topMentions --- .../app/controllers/LiveBlogController.scala | 72 ++++++------- article/app/model/LiveBlogHelpers.scala | 8 +- article/app/topmentions/TopicService.scala | 4 +- article/test/LiveBlogControllerTest.scala | 32 +++--- .../test/model/LiveBlogCurrentPageTest.scala | 102 +++++++++--------- .../topmentions/TopMentionsServiceTest.scala | 42 ++++---- common/app/model/LiveBlogCurrentPage.scala | 64 +++++------ common/app/model/TopMentionsModel.scala | 6 +- 8 files changed, 165 insertions(+), 165 deletions(-) diff --git a/article/app/controllers/LiveBlogController.scala b/article/app/controllers/LiveBlogController.scala index 523acaf5283c..6250ec79effd 100644 --- a/article/app/controllers/LiveBlogController.scala +++ b/article/app/controllers/LiveBlogController.scala @@ -45,7 +45,7 @@ class LiveBlogController( def renderEmail(path: String): Action[AnyContent] = { Action.async { implicit request => - mapModel(path, ArticleBlocks, maybeTopic = None) { + mapModel(path, ArticleBlocks, topicResult = None) { case (minute: MinutePage, _) => Future.successful(common.renderEmail(ArticleEmailHtmlPage.html(minute), minute)) case (blog: LiveBlogPage, _) => Future.successful(common.renderEmail(LiveBlogHtmlPage.html(blog), blog)) @@ -62,7 +62,7 @@ class LiveBlogController( ): Action[AnyContent] = { Action.async { implicit request => val filter = shouldFilter(filterKeyEvents) - val topMentions = if (filter) None else getTopics(path, topics) + val topicResult = if (filter) None else getTopicResult(path, topics) val availableTopics = topicService.getAvailableTopics(path) page.map(ParseBlockId.fromPageParam) match { @@ -71,7 +71,7 @@ class LiveBlogController( path, PageWithBlock(id), filter, - topMentions, + topicResult, availableTopics, selectedTopics = topics, ) // we know the id of a block @@ -80,7 +80,7 @@ class LiveBlogController( Cached(10)(WithoutRevalidationResult(NotFound)), ) // page param there but couldn't extract a block id case None => { - topMentions match { + topicResult match { case Some(value) => renderWithRange( path, @@ -116,16 +116,16 @@ class LiveBlogController( ): Action[AnyContent] = { Action.async { implicit request: Request[AnyContent] => val filter = shouldFilter(filterKeyEvents) - val maybeTopic = getTopics(path, topics) - val range = getRange(lastUpdate, page, maybeTopic) + val topicResult = getTopicResult(path, topics) + val range = getRange(lastUpdate, page, topicResult) val availableTopics = topicService.getAvailableTopics(path) - mapModel(path, range, filter, maybeTopic) { + mapModel(path, range, filter, topicResult) { case (blog: LiveBlogPage, _) if rendered.contains(false) => getJsonForFronts(blog) case (blog: LiveBlogPage, blocks) if request.forceDCR && lastUpdate.isEmpty => Future.successful(renderGuuiJson(blog, blocks, filter, availableTopics, selectedTopics = topics)) case (blog: LiveBlogPage, blocks) => - getJson(blog, range, isLivePage, filter, blocks.requestedBodyBlocks.getOrElse(Map.empty), maybeTopic) + getJson(blog, range, isLivePage, filter, blocks.requestedBodyBlocks.getOrElse(Map.empty), topicResult) case (minute: MinutePage, _) => Future.successful(common.renderJson(views.html.fragments.minuteBody(minute), minute)) case _ => @@ -140,13 +140,13 @@ class LiveBlogController( path: String, range: BlockRange, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], availableTopics: Option[Seq[AvailableTopic]], selectedTopics: Option[String], )(implicit request: RequestHeader, ): Future[Result] = { - mapModel(path, range, filterKeyEvents, maybeTopic) { (page, blocks) => + mapModel(path, range, filterKeyEvents, topicResult) { (page, blocks) => { val isAmpSupported = page.article.content.shouldAmplify val pageType: PageType = PageType(page, request, context) @@ -203,9 +203,9 @@ class LiveBlogController( private[this] def getRange( lastUpdate: Option[String], page: Option[String], - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ): BlockRange = { - (lastUpdate.map(ParseBlockId.fromBlockId), page.map(ParseBlockId.fromPageParam), maybeTopic) match { + (lastUpdate.map(ParseBlockId.fromBlockId), page.map(ParseBlockId.fromPageParam), topicResult) match { case (Some(ParsedBlockId(id)), _, _) => SinceBlockId(id) case (_, Some(ParsedBlockId(id)), _) => PageWithBlock(id) case (_, _, Some(_)) => TopicsLiveBlog @@ -225,7 +225,7 @@ class LiveBlogController( isLivePage: Option[Boolean], filterKeyEvents: Boolean, requestedBodyBlocks: scala.collection.Map[String, Seq[Block]] = Map.empty, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], )(implicit request: RequestHeader): Future[Result] = { val remoteRender = !request.forceDCROff @@ -238,7 +238,7 @@ class LiveBlogController( filterKeyEvents, remoteRender, requestedBodyBlocks, - maybeTopic, + topicResult, ) case _ => Future.successful(common.renderJson(views.html.liveblog.liveBlogBody(liveblog), liveblog)) } @@ -248,7 +248,7 @@ class LiveBlogController( page: PageWithStoryPackage, lastUpdateBlockId: SinceBlockId, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ): Seq[BodyBlock] = { val requestedBlocks = page.article.fields.blocks.toSeq.flatMap { _.requestedBodyBlocks.getOrElse(lastUpdateBlockId.around, Seq()) @@ -260,8 +260,8 @@ class LiveBlogController( if (filterKeyEvents) { latestBlocks.filter(block => block.eventType == KeyEvent || block.eventType == SummaryEvent) - } else if (maybeTopic.isDefined) { - latestBlocks.filter(block => maybeTopic.get.blocks.contains(block.id)) + } else if (topicResult.isDefined) { + latestBlocks.filter(block => topicResult.get.blocks.contains(block.id)) } else latestBlocks } @@ -270,7 +270,7 @@ class LiveBlogController( requestedBodyBlocks: scala.collection.Map[String, Seq[Block]], lastUpdateBlockId: SinceBlockId, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ): Seq[Block] = { val blocksAround = requestedBodyBlocks.getOrElse(lastUpdateBlockId.around, Seq.empty).takeWhile { block => block.id != lastUpdateBlockId.lastUpdate @@ -280,8 +280,8 @@ class LiveBlogController( blocksAround.filter(block => block.attributes.keyEvent.getOrElse(false) || block.attributes.summary.getOrElse(false), ) - } else if (maybeTopic.isDefined) { - blocksAround.filter(block => maybeTopic.get.blocks.contains(block.id)) + } else if (topicResult.isDefined) { + blocksAround.filter(block => topicResult.get.blocks.contains(block.id)) } else blocksAround } @@ -300,10 +300,10 @@ class LiveBlogController( filterKeyEvents: Boolean, remoteRender: Boolean, requestedBodyBlocks: scala.collection.Map[String, Seq[Block]], - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], )(implicit request: RequestHeader): Future[Result] = { - val newBlocks = getNewBlocks(page, lastUpdateBlockId, filterKeyEvents, maybeTopic) - val newCapiBlocks = getNewBlocks(requestedBodyBlocks, lastUpdateBlockId, filterKeyEvents, maybeTopic) + val newBlocks = getNewBlocks(page, lastUpdateBlockId, filterKeyEvents, topicResult) + val newCapiBlocks = getNewBlocks(requestedBodyBlocks, lastUpdateBlockId, filterKeyEvents, topicResult) val timelineHtml = views.html.liveblog.keyEvents( "", @@ -361,13 +361,13 @@ class LiveBlogController( path: String, range: BlockRange, filterKeyEvents: Boolean = false, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], )( render: (PageWithStoryPackage, Blocks) => Future[Result], )(implicit request: RequestHeader): Future[Result] = { capiLookup .lookup(path, Some(range)) - .map(responseToModelOrResult(range, filterKeyEvents, maybeTopic)) + .map(responseToModelOrResult(range, filterKeyEvents, topicResult)) .recover(convertApiExceptions) .flatMap { case Left((model, blocks)) => render(model, blocks) @@ -378,7 +378,7 @@ class LiveBlogController( private[this] def responseToModelOrResult( range: BlockRange, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], )(response: ItemResponse)(implicit request: RequestHeader): Either[(PageWithStoryPackage, Blocks), Result] = { val supportedContent: Option[ContentType] = response.content.filter(isSupported).map(Content(_)) val supportedContentResult: Either[ContentType, Result] = ModelOrResult(supportedContent, response) @@ -395,7 +395,7 @@ class LiveBlogController( response, range, filterKeyEvents, - maybeTopic, + topicResult, ).left .map(_ -> blocks) case unknown => @@ -410,17 +410,17 @@ class LiveBlogController( filterKeyEvents.getOrElse(false) } - def getTopics(blogId: String, maybeTopic: Option[String]) = { - val topics = for { - selectedTopic <- SelectedTopic.fromString(maybeTopic) - topics <- topicService.getSelectedTopic(blogId, selectedTopic) - } yield topics + def getTopicResult(blogId: String, topic: Option[String]) = { + val topicResult = for { + selectedTopic <- SelectedTopic.fromString(topic) + topicResult <- topicService.getSelectedTopic(blogId, selectedTopic) + } yield topicResult - topics match { - case Some(_) => log.info(s"topic was successfully retrieved for ${maybeTopic.get}") - case None => if (maybeTopic.isDefined) log.warn(s"topic result couldn't be retrieved for ${maybeTopic.get}") + topicResult match { + case Some(_) => log.info(s"topic was successfully retrieved for ${topic.get}") + case None => if (topic.isDefined) log.warn(s"topic result couldn't be retrieved for ${topic.get}") } - topics + topicResult } } diff --git a/article/app/model/LiveBlogHelpers.scala b/article/app/model/LiveBlogHelpers.scala index e8bf49e3bb45..b226a2b634c9 100644 --- a/article/app/model/LiveBlogHelpers.scala +++ b/article/app/model/LiveBlogHelpers.scala @@ -40,7 +40,7 @@ object LiveBlogHelpers extends GuLogging { liveBlog: Article, range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + topicResult: Option[TopicResult], ): Option[LiveBlogCurrentPage] = { val pageSize = if (liveBlog.content.tags.tags.map(_.id).contains("sport/sport")) 30 else 10 @@ -51,7 +51,7 @@ object LiveBlogHelpers extends GuLogging { _, range, filterKeyEvents, - topMentionResult, + topicResult, ), ) @@ -64,7 +64,7 @@ object LiveBlogHelpers extends GuLogging { response: ItemResponse, range: BlockRange, filterKeyEvents: Boolean, - topMentionResult: Option[Topic], + topicResult: Option[TopicResult], ): Either[LiveBlogPage, Status] = { val pageSize = if (liveBlog.content.tags.tags.map(_.id).contains("sport/sport")) 30 else 10 @@ -76,7 +76,7 @@ object LiveBlogHelpers extends GuLogging { blocks, range, filterKeyEvents, - topMentionResult, + topicResult, ) } getOrElse None diff --git a/article/app/topmentions/TopicService.scala b/article/app/topmentions/TopicService.scala index 9032dfaef3ac..d2637ff0c012 100644 --- a/article/app/topmentions/TopicService.scala +++ b/article/app/topmentions/TopicService.scala @@ -1,7 +1,7 @@ package topmentions import common.{Box, GuLogging} -import model.{TopicsApiResponse, Topic, SelectedTopic, AvailableTopic} +import model.{TopicsApiResponse, TopicResult, SelectedTopic, AvailableTopic} import scala.concurrent.{ExecutionContext, Future} @@ -41,7 +41,7 @@ class TopicService(topicS3Client: TopicS3Client) extends GuLogging { def getSelectedTopic( blogId: String, topMentionEntity: SelectedTopic, - ): Option[Topic] = { + ): Option[TopicResult] = { getBlogTopicsApiResponse(blogId).flatMap(_.results.find(result => { result.`type` == topMentionEntity.`type` && result.name == topMentionEntity.value })) diff --git a/article/test/LiveBlogControllerTest.scala b/article/test/LiveBlogControllerTest.scala index 9736ee1e2e58..88fe949a236c 100644 --- a/article/test/LiveBlogControllerTest.scala +++ b/article/test/LiveBlogControllerTest.scala @@ -9,7 +9,7 @@ import play.api.test._ import play.api.test.Helpers._ import org.scalatest.{BeforeAndAfterAll, DoNotDiscover} import org.scalatestplus.mockito.MockitoSugar -import model.{LiveBlogPage, Topic, SelectedTopic, TopMentionsTopicType, AvailableTopic, TopicsLiveBlog} +import model.{LiveBlogPage, TopicResult, SelectedTopic, TopMentionsTopicType, AvailableTopic, TopicsLiveBlog} import topmentions.{TopicS3Client, TopicService} import scala.concurrent.Future @@ -29,9 +29,9 @@ import scala.concurrent.Future val path = "/football/live/2016/feb/26/fifa-election-who-will-succeed-sepp-blatter-president-live" trait Setup { - var fakeTopMentionsService = mock[TopicService] + var fakeTopicService = mock[TopicService] var fakeDcr = new DCRFake() - val topMentionResult = Topic( + val topicResult = TopicResult( name = "Fifa", `type` = TopMentionsTopicType.Org, blocks = Seq("56d08042e4b0d38537b1f70b"), @@ -39,7 +39,7 @@ import scala.concurrent.Future percentage_blocks = 1.2f, ) - val topics = Vector( + val fakeAvailableTopics = Vector( AvailableTopic(TopMentionsTopicType.Gpe, "United Kingdom", 6), AvailableTopic(TopMentionsTopicType.Gpe, "Russia", 4), AvailableTopic(TopMentionsTopicType.Org, "KPMG", 4), @@ -53,15 +53,15 @@ import scala.concurrent.Future ); when( - fakeTopMentionsService.getSelectedTopic(path, SelectedTopic(TopMentionsTopicType.Org, "Fifa")), + fakeTopicService.getSelectedTopic(path, SelectedTopic(TopMentionsTopicType.Org, "Fifa")), ) thenReturn Some( - topMentionResult, + topicResult, ) when( - fakeTopMentionsService.getTopics(path), + fakeTopicService.getAvailableTopics(path), ) thenReturn Some( - topics, + fakeAvailableTopics, ) lazy val liveBlogController = new LiveBlogController( @@ -69,7 +69,7 @@ import scala.concurrent.Future play.api.test.Helpers.stubControllerComponents(), wsClient, fakeDcr, - fakeTopMentionsService, + fakeTopicService, ) } @@ -285,16 +285,16 @@ import scala.concurrent.Future liveBlogController.shouldFilter(None) should be(false) } - "getTopMentionsForFilters" should "returns none given no automatic filter query parameter" in new Setup { - liveBlogController.getTopics(path, None) should be(None) + "getTopicResult" should "returns none given no automatic filter query parameter" in new Setup { + liveBlogController.getTopicResult(path, None) should be(None) } - "getTopMentionsForFilters" should "returns none given an incorrect automatic filter query parameter" in new Setup { - liveBlogController.getTopics(path, Some("orgFifa")) should be(None) + "getTopicResult" should "returns none given an incorrect automatic filter query parameter" in new Setup { + liveBlogController.getTopicResult(path, Some("orgFifa")) should be(None) } - "getTopMentionsForFilters" should "returns correct topMentionResult given a correct automatic filter query parameter" in new Setup { - liveBlogController.getTopics(path, Some("org:Fifa")) should be(Some(topMentionResult)) + "getTopicResult" should "returns correct topicResult given a correct automatic filter query parameter" in new Setup { + liveBlogController.getTopicResult(path, Some("org:Fifa")) should be(Some(topicResult)) } "renderArticle" should "returns the first page of filtered blog by topics" in new Setup { @@ -324,7 +324,7 @@ import scala.concurrent.Future topics = Some("org:Fifa"), )(fakeRequest) - verify(fakeTopMentionsService, times(0)).getSelectedTopic(anyString(), anyObject()) + verify(fakeTopicService, times(0)).getSelectedTopic(anyString(), anyObject()) status(result) should be(200) } diff --git a/article/test/model/LiveBlogCurrentPageTest.scala b/article/test/model/LiveBlogCurrentPageTest.scala index e8a684e93b67..590ba5e004b0 100644 --- a/article/test/model/LiveBlogCurrentPageTest.scala +++ b/article/test/model/LiveBlogCurrentPageTest.scala @@ -63,8 +63,8 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blocks = Blocks(1, Seq(), None, Map()), range, filterKeyEvents = false, - maybeTopic = Some( - Topic( + topicResult = Some( + TopicResult( `type` = TopMentionsTopicType.Org, name = "someName", blocks = Seq(), @@ -90,7 +90,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { result should be( Some( LiveBlogCurrentPage( - currentPage = FirstPage(Seq(fakeBlock(1)), filterKeyEvents = false, maybeTopic = None), + currentPage = FirstPage(Seq(fakeBlock(1)), filterKeyEvents = false, topicResult = None), pagination = None, pinnedBlock = None, ), @@ -251,7 +251,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - should(result, currentPage = FirstPage(List(), true, maybeTopic = None), pagination = None) + should(result, currentPage = FirstPage(List(), true, topicResult = None), pagination = None) } it should "allow 3 blocks on one page" in { @@ -269,7 +269,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - should(result, FirstPage(blocks, filterKeyEvents = false, maybeTopic = None), None) + should(result, FirstPage(blocks, filterKeyEvents = false, topicResult = None), None) } it should "put 4 blocks on two pages (main page)" in { @@ -288,9 +288,9 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { val expected = blocks.take(2) val expectedOldestPage = - BlockPage(blocks = Nil, blockId = "1", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) + BlockPage(blocks = Nil, blockId = "1", pageNumber = 2, filterKeyEvents = false, topicResult = None) val expectedOlderPage = - BlockPage(blocks = Nil, blockId = "2", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) + BlockPage(blocks = Nil, blockId = "2", pageNumber = 2, filterKeyEvents = false, topicResult = None) val expectedPagination = Some( N1Pagination( newest = None, @@ -318,11 +318,11 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - val expectedCurrentPage = FirstPage(blocks = blocks.take(3), filterKeyEvents = false, maybeTopic = None) + val expectedCurrentPage = FirstPage(blocks = blocks.take(3), filterKeyEvents = false, topicResult = None) val expectedOldestPage = - BlockPage(blocks = Nil, blockId = "1", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) + BlockPage(blocks = Nil, blockId = "1", pageNumber = 2, filterKeyEvents = false, topicResult = None) val expectedOlderPage = - BlockPage(blocks = Nil, blockId = "2", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) + BlockPage(blocks = Nil, blockId = "2", pageNumber = 2, filterKeyEvents = false, topicResult = None) val expectedPagination = Some( N1Pagination( newest = None, @@ -357,11 +357,11 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - val expectedCurrentPage = FirstPage(blocks = keyBlocks.take(3), filterKeyEvents = true, maybeTopic = None) + val expectedCurrentPage = FirstPage(blocks = keyBlocks.take(3), filterKeyEvents = true, topicResult = None) val expectedOldestPage = - BlockPage(blocks = Nil, blockId = "1", pageNumber = 3, filterKeyEvents = true, maybeTopic = None) + BlockPage(blocks = Nil, blockId = "1", pageNumber = 3, filterKeyEvents = true, topicResult = None) val expectedOlderPage = - BlockPage(blocks = Nil, blockId = "5", pageNumber = 2, filterKeyEvents = true, maybeTopic = None) + BlockPage(blocks = Nil, blockId = "5", pageNumber = 2, filterKeyEvents = true, topicResult = None) val expectedPagination = Some( N1Pagination( newest = None, @@ -376,14 +376,14 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { } it should "only filters blocks by key events given both key events and top mentions are provided" in { - val topMentions = getTopMentionsForTopicAndBlocks(TopMentionsTopicType.Org, "tfl", Seq("1", "3")) + val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("1", "3")) val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 5, numberOfKeyEventsBlocks = 2, None) val result = LiveBlogCurrentPage.firstPage( pageSize = 2, blocks = testFakeBlocks.blocksType, filterKeyEvents = true, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ) result.get should be( @@ -391,7 +391,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { currentPage = FirstPage( testFakeBlocks.blocksSequence.slice(3, 5), filterKeyEvents = true, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ), pagination = None, pinnedBlock = None, @@ -400,14 +400,14 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { } it should "returns the 1st page of the topic filtered blocks" in { - val topMentions = getTopMentionsForTopicAndBlocks(TopMentionsTopicType.Org, "tfl", Seq("1", "2", "3", "4")) + val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("1", "2", "3", "4")) val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 5, numberOfKeyEventsBlocks = 2, None) val result = LiveBlogCurrentPage.firstPage( pageSize = 2, blocks = testFakeBlocks.blocksType, filterKeyEvents = false, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ) val expectedPagination = Some( @@ -420,7 +420,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 2, filterKeyEvents = false, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ), ), oldest = Some( @@ -429,7 +429,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "1", pageNumber = 2, filterKeyEvents = false, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ), ), numberOfPages = 2, @@ -441,7 +441,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { currentPage = FirstPage( testFakeBlocks.blocksSequence.slice(1, 3), filterKeyEvents = false, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ), pagination = expectedPagination, ) @@ -457,9 +457,9 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 2, filterKeyEvents = false, - maybeTopic = None, + topicResult = None, ) - val expectedNewestPage = FirstPage(blocks.take(2), filterKeyEvents = false, maybeTopic = None) + val expectedNewestPage = FirstPage(blocks.take(2), filterKeyEvents = false, topicResult = None) val expectedPagination = Some( N1Pagination( newest = Some(expectedNewestPage), @@ -483,9 +483,9 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 2, filterKeyEvents = false, - maybeTopic = None, + topicResult = None, ) - val expectedNewestPage = FirstPage(blocks.take(2), filterKeyEvents = false, maybeTopic = None) + val expectedNewestPage = FirstPage(blocks.take(2), filterKeyEvents = false, topicResult = None) val expectedPagination = Some( N1Pagination( newest = Some(expectedNewestPage), @@ -509,9 +509,9 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 2, filterKeyEvents = false, - maybeTopic = None, + topicResult = None, ) - val expectedNewestPage = FirstPage(blocks.take(3), filterKeyEvents = false, maybeTopic = None) + val expectedNewestPage = FirstPage(blocks.take(3), filterKeyEvents = false, topicResult = None) val expectedPagination = Some( N1Pagination( newest = Some(expectedNewestPage), @@ -539,11 +539,11 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { None, ) - val expectedCurrentPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, maybeTopic = None) + val expectedCurrentPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, topicResult = None) val expectedMiddlePage = - BlockPage(blocks = Nil, blockId = "4", pageNumber = 2, filterKeyEvents = false, maybeTopic = None) + BlockPage(blocks = Nil, blockId = "4", pageNumber = 2, filterKeyEvents = false, topicResult = None) val expectedOldestPage = - BlockPage(blocks = Nil, blockId = "1", pageNumber = 3, filterKeyEvents = false, maybeTopic = None) + BlockPage(blocks = Nil, blockId = "1", pageNumber = 3, filterKeyEvents = false, topicResult = None) val expectedPagination = Some( N1Pagination( newest = None, @@ -567,16 +567,16 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "4", pageNumber = 2, filterKeyEvents = false, - maybeTopic = None, + topicResult = None, ) - val expectedFirstPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, maybeTopic = None) + val expectedFirstPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, topicResult = None) val expectedOlderPage = BlockPage( blocks = blocks.takeRight(2), blockId = "2", pageNumber = 3, filterKeyEvents = false, - maybeTopic = None, + topicResult = None, ) val expectedPagination = Some( N1Pagination( @@ -601,16 +601,16 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 3, filterKeyEvents = false, - maybeTopic = None, + topicResult = None, ) - val expectedFirstPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, maybeTopic = None) + val expectedFirstPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false, topicResult = None) val expectedMiddlePage = BlockPage( blocks = blocks.slice(2, 4), blockId = "4", pageNumber = 2, filterKeyEvents = false, - maybeTopic = None, + topicResult = None, ) val expectedPagination = Some( N1Pagination( @@ -636,18 +636,18 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blockId = "2", pageNumber = 3, filterKeyEvents = true, - maybeTopic = None, + topicResult = None, ) } val expectedFirstPage = - FirstPage(blocks = keyAndSummaryBlocks.take(2), filterKeyEvents = true, maybeTopic = None) + FirstPage(blocks = keyAndSummaryBlocks.take(2), filterKeyEvents = true, topicResult = None) val expectedMiddlePage = BlockPage( blocks = keyAndSummaryBlocks.slice(2, 4), blockId = "4", pageNumber = 2, filterKeyEvents = true, - maybeTopic = None, + topicResult = None, ) val expectedPagination = Some( N1Pagination( @@ -670,20 +670,20 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { } it should "returns the correct 2nd page of the topic filtered blocks" in { - val topMentions = getTopMentionsForTopicAndBlocks(TopMentionsTopicType.Org, "tfl", Seq("2", "3", "4", "5", "6")) + val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("2", "3", "4", "5", "6")) val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 8, numberOfKeyEventsBlocks = 2, None) val expectedFirstPage = FirstPage( testFakeBlocks.blocksSequence.slice(2, 5), filterKeyEvents = false, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ) val expectedCurrentPage = BlockPage( blocks = testFakeBlocks.blocksSequence.slice(5, 7), blockId = "3", pageNumber = 2, filterKeyEvents = false, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ) val expectedPagination = Some( N1Pagination( @@ -700,14 +700,14 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blocks = testFakeBlocks.blocksSequence, isRequestedBlock = "3", filterKeyEvents = false, - topMentionsResult = Some(topMentions), + topicResult = Some(topicResult), ) should(result, expectedCurrentPage, expectedPagination) } "updates" should "return only the topic filtered blocks after the lastUpdated block given lastUpdated is also a topic block" in { - val topMentions = getTopMentionsForTopicAndBlocks(TopMentionsTopicType.Org, "tfl", Seq("2", "3", "4", "5", "6")) + val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("2", "3", "4", "5", "6")) val sinceBlock = SinceBlockId("5") val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 8, numberOfKeyEventsBlocks = 2, Some("5")) @@ -715,7 +715,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blocks = testFakeBlocks.blocksType, sinceBlockId = SinceBlockId("5"), filterKeyEvents = false, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ) result.get.currentPage.blocks.length should be(1) @@ -723,7 +723,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { } "updates" should "return only the topic filtered blocks after the lastUpdated block given lastUpdated is NOT a topic block" in { - val topMentions = getTopMentionsForTopicAndBlocks(TopMentionsTopicType.Org, "tfl", Seq("2", "3", "5", "6")) + val topicResult = getFakeTopicResult(TopMentionsTopicType.Org, "tfl", Seq("2", "3", "5", "6")) val sinceBlockId = "4" val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 8, numberOfKeyEventsBlocks = 2, Some(sinceBlockId)) @@ -731,7 +731,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { blocks = testFakeBlocks.blocksType, sinceBlockId = SinceBlockId(sinceBlockId), filterKeyEvents = false, - maybeTopic = Some(topMentions), + topicResult = Some(topicResult), ) result.get.currentPage.blocks.length should be(2) @@ -739,13 +739,13 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { result.get.currentPage.blocks(1).id should be("5") } - private def getTopMentionsForTopicAndBlocks( - tpoicType: TopMentionsTopicType, + private def getFakeTopicResult( + topicType: TopMentionsTopicType, topicName: String, blocks: Seq[String], ) = { - Topic( - `type` = tpoicType, + TopicResult( + `type` = topicType, name = topicName, blocks = blocks, count = 0, diff --git a/article/test/topmentions/TopMentionsServiceTest.scala b/article/test/topmentions/TopMentionsServiceTest.scala index 243cabdb3bdf..6a58c4ab42e4 100644 --- a/article/test/topmentions/TopMentionsServiceTest.scala +++ b/article/test/topmentions/TopMentionsServiceTest.scala @@ -1,6 +1,6 @@ package topmentions -import model.{TopicsApiResponse, Topic, SelectedTopic, TopMentionsTopicType, AvailableTopic} +import model.{TopicsApiResponse, TopicResult, SelectedTopic, TopMentionsTopicType, AvailableTopic} import org.scalatest.{BeforeAndAfterAll} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -19,8 +19,8 @@ class TopMentionsServiceTest with MockitoSugar { val fakeClient = mock[TopicS3Client] - val topMentionResult = - Topic( + val topicResult = + TopicResult( name = "name1", `type` = TopMentionsTopicType.Org, blocks = Seq("blockId1"), @@ -28,15 +28,15 @@ class TopMentionsServiceTest percentage_blocks = 1.2f, ) - val topMentionResults = Seq( - Topic( + val topicResults = Seq( + TopicResult( name = "name1", `type` = TopMentionsTopicType.Org, blocks = Seq("blockId1"), count = 1, percentage_blocks = 1.2f, ), - Topic( + TopicResult( name = "name2", `type` = TopMentionsTopicType.Person, blocks = Seq("blockId1"), @@ -45,12 +45,12 @@ class TopMentionsServiceTest ), ) val successResponse = - TopicsApiResponse(entity_types = Seq(TopMentionsTopicType.Org), results = Seq(topMentionResult), model = "model") + TopicsApiResponse(entity_types = Seq(TopMentionsTopicType.Org), results = Seq(topicResult), model = "model") val successMultiResponse = - TopicsApiResponse(entity_types = Seq(TopMentionsTopicType.Org), results = topMentionResults, model = "model") + TopicsApiResponse(entity_types = Seq(TopMentionsTopicType.Org), results = topicResults, model = "model") - "refreshTopMentions" should "return successful future given getListOfKeys s3 call fails" in { + "refreshTopics" should "return successful future given getListOfKeys s3 call fails" in { when(fakeClient.getListOfKeys()) thenReturn Future.failed(new Throwable("")) val topMentionService = new TopicService(fakeClient) @@ -60,7 +60,7 @@ class TopMentionsServiceTest results should be(None) } - "refreshTopMentions" should "return successful future given one of the S3 object calls fails" in { + "refreshTopics" should "return successful future given one of the S3 object calls fails" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1", "key2")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) when(fakeClient.getObject("key2")) thenReturn Future.failed(new Throwable("error happend")) @@ -74,7 +74,7 @@ class TopMentionsServiceTest results should be(None) } - "refreshTopMentions" should "update in memory top mentions and return successful future given one of the S3 object calls fails" in { + "refreshTopics" should "update in memory topics and return successful future given one of the S3 object calls fails" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) @@ -88,7 +88,7 @@ class TopMentionsServiceTest results.get.get("key1") should equal(Some(successResponse)) } - "getTopMentionsByTopic" should "return the correct top mention result given correct blog id, filter entity and filter value" in { + "getSelectedTopic" should "return the correct topic result given correct blog id, filter entity and filter value" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) @@ -97,10 +97,10 @@ class TopMentionsServiceTest val result = topMentionService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "name1")) - result.get should equal(topMentionResult) + result.get should equal(topicResult) } - "getTopMentionsByTopic" should "return none given correct blog id, filter entity and with same filter value but different case" in { + "getSelectedTopic" should "return none given correct blog id, filter entity and with same filter value but different case" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) @@ -112,7 +112,7 @@ class TopMentionsServiceTest result should equal(None) } - "getTopMentionsByTopic" should "return none given a blog id that doesn't exist in cache" in { + "getSelectedTopic" should "return none given a blog id that doesn't exist in cache" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) @@ -124,7 +124,7 @@ class TopMentionsServiceTest result should equal(None) } - "getTopMentionsByTopic" should "return none given a filter entity type that doesn't exist in cache for the relevant blog" in { + "getSelectedTopic" should "return none given a filter entity type that doesn't exist in cache for the relevant blog" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) @@ -137,7 +137,7 @@ class TopMentionsServiceTest result should equal(None) } - "getTopMentionsByTopic" should "return none given a filter entity value that doesn't exist in cache for the relevant blog" in { + "getSelectedTopic" should "return none given a filter entity value that doesn't exist in cache for the relevant blog" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) @@ -150,7 +150,7 @@ class TopMentionsServiceTest result should equal(None) } - "getTopics" should "return a list of topics given a blog id" in { + "getAvailableTopics" should "return a list of available topics given a blog id" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successMultiResponse) val expectedTopics = @@ -164,18 +164,18 @@ class TopMentionsServiceTest val topMentionService = new TopicService(fakeClient) val refreshJob = Await.result(topMentionService.refreshTopics(), 1.second) val topicList = - topMentionService.getTopics("key1") + topMentionService.getAvailableTopics("key1") topicList should equal(expectedTopics) } - "getTopics" should "return none given a blog id that doesn't exist in cache" in { + "getAvailableTopics" should "return none given a blog id that doesn't exist in cache" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successMultiResponse) val topMentionService = new TopicService(fakeClient) val topicList = - topMentionService.getTopics("key1") + topMentionService.getAvailableTopics("key1") topicList should equal(None) } diff --git a/common/app/model/LiveBlogCurrentPage.scala b/common/app/model/LiveBlogCurrentPage.scala index ff7223c0f19b..f4bf36d463e6 100644 --- a/common/app/model/LiveBlogCurrentPage.scala +++ b/common/app/model/LiveBlogCurrentPage.scala @@ -18,13 +18,13 @@ object LiveBlogCurrentPage { blocks: Blocks, range: BlockRange, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ): Option[LiveBlogCurrentPage] = { range match { - case CanonicalLiveBlog | TopicsLiveBlog => firstPage(pageSize, blocks, filterKeyEvents, maybeTopic) + case CanonicalLiveBlog | TopicsLiveBlog => firstPage(pageSize, blocks, filterKeyEvents, topicResult) case PageWithBlock(isRequestedBlock) => - findPageWithBlock(pageSize, blocks.body, isRequestedBlock, filterKeyEvents, maybeTopic) - case SinceBlockId(blockId) => updates(blocks, SinceBlockId(blockId), filterKeyEvents, maybeTopic) + findPageWithBlock(pageSize, blocks.body, isRequestedBlock, filterKeyEvents, topicResult) + case SinceBlockId(blockId) => updates(blocks, SinceBlockId(blockId), filterKeyEvents, topicResult) case ArticleBlocks => None case GenericFallback => None case _ => None @@ -36,15 +36,15 @@ object LiveBlogCurrentPage { blocks: Blocks, sinceBlockId: SinceBlockId, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ): Option[LiveBlogCurrentPage] = { val bodyBlocks = blocks.requestedBodyBlocks.get(sinceBlockId.around).toSeq.flatMap { bodyBlocks => val onlyBlocksAfterLastUpdated = bodyBlocks.takeWhile(_.id != sinceBlockId.lastUpdate) - applyFilters(onlyBlocksAfterLastUpdated, filterKeyEvents, maybeTopic) + applyFilters(onlyBlocksAfterLastUpdated, filterKeyEvents, topicResult) } Some( - LiveBlogCurrentPage(FirstPage(bodyBlocks, filterKeyEvents, maybeTopic), None, None), + LiveBlogCurrentPage(FirstPage(bodyBlocks, filterKeyEvents, topicResult), None, None), ) // just pretend to be the first page, it'll be ignored } @@ -53,10 +53,10 @@ object LiveBlogCurrentPage { pageSize: Int, blocks: Blocks, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ): Option[LiveBlogCurrentPage] = { val (maybeRequestedBodyBlocks, blockCount, oldestPageBlockId) = - extractFirstPageBlocks(blocks, filterKeyEvents, maybeTopic) + extractFirstPageBlocks(blocks, filterKeyEvents, topicResult) val remainder = blockCount % pageSize val numPages = blockCount / pageSize @@ -65,11 +65,11 @@ object LiveBlogCurrentPage { val (firstPageBlocks, startOfSecondPageBlocks) = requestedBodyBlocks.splitAt(remainder + pageSize) val olderPage = startOfSecondPageBlocks.headOption.map { block => - BlockPage(blocks = Nil, blockId = block.id, pageNumber = 2, filterKeyEvents, maybeTopic) + BlockPage(blocks = Nil, blockId = block.id, pageNumber = 2, filterKeyEvents, topicResult) } val oldestPage = oldestPageBlockId map { blockId => - BlockPage(blocks = Nil, blockId = blockId, pageNumber = numPages, filterKeyEvents, maybeTopic) + BlockPage(blocks = Nil, blockId = blockId, pageNumber = numPages, filterKeyEvents, topicResult) } val pinnedBlocks = blocks.requestedBodyBlocks.get(CanonicalLiveBlog.pinned) @@ -91,35 +91,35 @@ object LiveBlogCurrentPage { else None } - LiveBlogCurrentPage(FirstPage(blocksToDisplay, filterKeyEvents, maybeTopic), pagination, pinnedBlockRenamed) + LiveBlogCurrentPage(FirstPage(blocksToDisplay, filterKeyEvents, topicResult), pagination, pinnedBlockRenamed) } } private def extractFirstPageBlocks( blocks: Blocks, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ) = { if (filterKeyEvents) { getKeyEventsBlocks(blocks) - } else if (maybeTopic.isDefined) { - getTopMentionsBlocks(blocks, maybeTopic.get) + } else if (topicResult.isDefined) { + getTopMentionsBlocks(blocks, topicResult.get) } else { getStandardBlocks(blocks) } } - private def isTopicBlock(topic: Topic)(bodyBlock: BodyBlock): Boolean = { + private def isTopicBlock(topic: TopicResult)(bodyBlock: BodyBlock): Boolean = { topic.blocks.contains(bodyBlock.id) } - private def filterBlocksByTopic(blocks: Seq[BodyBlock], topic: Topic) = { + private def filterBlocksByTopic(blocks: Seq[BodyBlock], topic: TopicResult) = { blocks.filter(isTopicBlock(topic)).sortBy(_.publishedCreatedTimestamp).reverse } private def getTopMentionsBlocks( blocks: Blocks, - topic: Topic, + topic: TopicResult, ): (Option[Seq[BodyBlock]], Int, Option[String]) = { val bodyBlocks = blocks.body @@ -164,17 +164,17 @@ object LiveBlogCurrentPage { blocks: Seq[BodyBlock], isRequestedBlock: String, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ): Option[LiveBlogCurrentPage] = { val pinnedBlock = blocks.find(_.attributes.pinned).map(renamePinnedBlock) - val filteredBlocks = applyFilters(blocks, filterKeyEvents, maybeTopic) + val filteredBlocks = applyFilters(blocks, filterKeyEvents, topicResult) val (mainPageBlocks, restPagesBlocks) = getPages(pageSize, filteredBlocks) - val newestPage = FirstPage(mainPageBlocks, filterKeyEvents, maybeTopic) + val newestPage = FirstPage(mainPageBlocks, filterKeyEvents, topicResult) val pages = newestPage :: restPagesBlocks.zipWithIndex .map { case (page, index) => // page number is index + 2 to account for first page and 0 based index - BlockPage(blocks = page, blockId = page.head.id, pageNumber = index + 2, filterKeyEvents, maybeTopic) + BlockPage(blocks = page, blockId = page.head.id, pageNumber = index + 2, filterKeyEvents, topicResult) } val oldestPage = pages.lastOption.getOrElse(newestPage) @@ -217,12 +217,12 @@ object LiveBlogCurrentPage { private def applyFilters( blocks: Seq[BodyBlock], filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ) = { if (filterKeyEvents) { blocks.filter(block => block.eventType == KeyEvent || block.eventType == SummaryEvent) - } else if (maybeTopic.isDefined) { - filterBlocksByTopic(blocks, maybeTopic.get) + } else if (topicResult.isDefined) { + filterBlocksByTopic(blocks, topicResult.get) } else { blocks } @@ -247,9 +247,9 @@ sealed trait PageReference { def isArchivePage: Boolean - def buildQueryParam(maybeTopic: Option[Topic]) = { - maybeTopic match { - case Some(value) => s"&topics=${maybeTopic.get.`type`}:${value.name}" + def buildQueryParam(topicResult: Option[TopicResult]) = { + topicResult match { + case Some(value) => s"&topics=${topicResult.get.`type`}:${value.name}" case None => "" } } @@ -263,9 +263,9 @@ case class N1Pagination( numberOfPages: Int, ) -case class FirstPage(blocks: Seq[BodyBlock], filterKeyEvents: Boolean, maybeTopic: Option[Topic]) +case class FirstPage(blocks: Seq[BodyBlock], filterKeyEvents: Boolean, topicResult: Option[TopicResult]) extends PageReference { - val suffix = s"?filterKeyEvents=$filterKeyEvents${buildQueryParam(maybeTopic)}" + val suffix = s"?filterKeyEvents=$filterKeyEvents${buildQueryParam(topicResult)}" val pageNumber = 1 val isArchivePage = false } @@ -275,9 +275,9 @@ case class BlockPage( blockId: String, pageNumber: Int, filterKeyEvents: Boolean, - maybeTopic: Option[Topic], + topicResult: Option[TopicResult], ) extends PageReference { - val suffix = s"?page=with:block-$blockId&filterKeyEvents=$filterKeyEvents${buildQueryParam(maybeTopic)}" + val suffix = s"?page=with:block-$blockId&filterKeyEvents=$filterKeyEvents${buildQueryParam(topicResult)}" val isArchivePage = true } diff --git a/common/app/model/TopMentionsModel.scala b/common/app/model/TopMentionsModel.scala index 101fcd4a3b45..d7ab4780b80b 100644 --- a/common/app/model/TopMentionsModel.scala +++ b/common/app/model/TopMentionsModel.scala @@ -4,19 +4,19 @@ import common.GuLogging import model.TopMentionsTopicType.TopMentionsTopicType import play.api.libs.json.{Format, Json} -case class Topic( +case class TopicResult( name: String, `type`: TopMentionsTopicType, blocks: Seq[String], count: Int, percentage_blocks: Float, ) -case class TopicsApiResponse(entity_types: Seq[TopMentionsTopicType], results: Seq[Topic], model: String) +case class TopicsApiResponse(entity_types: Seq[TopMentionsTopicType], results: Seq[TopicResult], model: String) case class TopMentionJsonParseException(message: String) extends Exception(message) object TopicsApiResponse { - implicit val TopicJf: Format[Topic] = Json.format[Topic] + implicit val TopicResultJf: Format[TopicResult] = Json.format[TopicResult] implicit val TopicsApiResponseJf: Format[TopicsApiResponse] = Json.format[TopicsApiResponse] } From 72d7d6d8138b5790fcec9a0be08ea8d626317eb3 Mon Sep 17 00:00:00 2001 From: Joe Cowton Date: Wed, 29 Jun 2022 16:46:28 +0100 Subject: [PATCH 4/5] Rename top mentions serices --- article/app/AppLoader.scala | 8 +-- .../app/controllers/ArticleControllers.scala | 4 +- .../app/controllers/LiveBlogController.scala | 2 +- article/app/jobs/TopicLifecycle.scala | 8 +-- .../TopicS3Client.scala | 4 +- .../TopicService.scala | 2 +- article/test/LiveBlogControllerTest.scala | 2 +- .../TopicServiceTest.scala} | 62 +++++++++---------- common/app/model/LiveBlogCurrentPage.scala | 4 +- dev-build/app/AppLoader.scala | 2 +- preview/app/AppLoader.scala | 2 +- 11 files changed, 50 insertions(+), 50 deletions(-) rename article/app/{topmentions => topics}/TopicS3Client.scala (97%) rename article/app/{topmentions => topics}/TopicService.scala (98%) rename article/test/{topmentions/TopMentionsServiceTest.scala => topics/TopicServiceTest.scala} (70%) diff --git a/article/app/AppLoader.scala b/article/app/AppLoader.scala index b10097f6c327..bb85867d2c6a 100644 --- a/article/app/AppLoader.scala +++ b/article/app/AppLoader.scala @@ -22,19 +22,19 @@ import router.Routes import services.ophan.SurgingContentAgentLifecycle import services.{NewspaperBooksAndSectionsAutoRefresh, OphanApi, SkimLinksCacheLifeCycle} import jobs.{StoreNavigationLifecycleComponent, TopicLifecycle} -import topmentions.{TopicS3Client, TopicS3ClientImpl, TopicService} +import topics.{TopicS3Client, TopicS3ClientImpl, TopicService} class AppLoader extends FrontendApplicationLoader { override def buildComponents(context: Context): FrontendComponents = new BuiltInComponentsFromContext(context) with AppComponents } -trait TopMentionsServices { +trait TopicServices { lazy val topicS3Client: TopicS3Client = wire[TopicS3ClientImpl] - lazy val topMentionsService = wire[TopicService] + lazy val topicService = wire[TopicService] } -trait AppComponents extends FrontendComponents with ArticleControllers with TopMentionsServices { +trait AppComponents extends FrontendComponents with ArticleControllers with TopicServices { lazy val capiHttpClient: HttpClient = wire[CapiHttpClient] lazy val contentApiClient = wire[ContentApiClient] diff --git a/article/app/controllers/ArticleControllers.scala b/article/app/controllers/ArticleControllers.scala index 04a29ec52089..ac6d91114639 100644 --- a/article/app/controllers/ArticleControllers.scala +++ b/article/app/controllers/ArticleControllers.scala @@ -7,7 +7,7 @@ import play.api.libs.ws.WSClient import play.api.mvc.ControllerComponents import renderers.DotcomRenderingService import services.{NewspaperBookSectionTagAgent, NewspaperBookTagAgent} -import topmentions.{TopicS3Client, TopicService} +import topics.{TopicS3Client, TopicService} trait ArticleControllers { def contentApiClient: ContentApiClient @@ -15,7 +15,7 @@ trait ArticleControllers { def wsClient: WSClient def remoteRender: DotcomRenderingService def topicS3Client: TopicS3Client - def topMentionsService: TopicService + def topicService: TopicService implicit def appContext: ApplicationContext lazy val bookAgent: NewspaperBookTagAgent = wire[NewspaperBookTagAgent] lazy val bookSectionAgent: NewspaperBookSectionTagAgent = wire[NewspaperBookSectionTagAgent] diff --git a/article/app/controllers/LiveBlogController.scala b/article/app/controllers/LiveBlogController.scala index 6250ec79effd..22275dcf864e 100644 --- a/article/app/controllers/LiveBlogController.scala +++ b/article/app/controllers/LiveBlogController.scala @@ -19,7 +19,7 @@ import play.twirl.api.Html import renderers.DotcomRenderingService import services.CAPILookup import services.dotcomponents.DotcomponentsLogger -import topmentions.TopicService +import topics.TopicService import views.support.RenderOtherStatus import scala.concurrent.Future diff --git a/article/app/jobs/TopicLifecycle.scala b/article/app/jobs/TopicLifecycle.scala index 8807538dfb05..e1eb2439006b 100644 --- a/article/app/jobs/TopicLifecycle.scala +++ b/article/app/jobs/TopicLifecycle.scala @@ -3,7 +3,7 @@ package jobs import app.LifecycleComponent import common.{AkkaAsync, JobScheduler} import play.api.inject.ApplicationLifecycle -import topmentions.TopicService +import topics.TopicService import scala.concurrent.duration._ import scala.concurrent.{ExecutionContext, Future} @@ -12,7 +12,7 @@ class TopicLifecycle( appLifeCycle: ApplicationLifecycle, jobs: JobScheduler, akkaAsync: AkkaAsync, - topMentionService: TopicService, + topicService: TopicService, )(implicit ec: ExecutionContext) extends LifecycleComponent { @@ -27,14 +27,14 @@ class TopicLifecycle( // refresh top mentions when app starts akkaAsync.after1s { - topMentionService.refreshTopics() + topicService.refreshTopics() } } private def scheduleJobs(): Unit = { // This job runs every 2 minutes jobs.scheduleEvery("TopMentionsAgentRefreshJob", 2.minutes) { - topMentionService.refreshTopics() + topicService.refreshTopics() } } diff --git a/article/app/topmentions/TopicS3Client.scala b/article/app/topics/TopicS3Client.scala similarity index 97% rename from article/app/topmentions/TopicS3Client.scala rename to article/app/topics/TopicS3Client.scala index ce13982cfe2c..b30b66747584 100644 --- a/article/app/topmentions/TopicS3Client.scala +++ b/article/app/topics/TopicS3Client.scala @@ -1,4 +1,4 @@ -package topmentions +package topics import com.amazonaws.services.s3.AmazonS3 import com.amazonaws.services.s3.model.{GetObjectRequest, S3Object} @@ -8,7 +8,7 @@ import conf.Configuration import model.{TopMentionJsonParseException, TopicsApiResponse} import play.api.libs.json.{JsError, JsSuccess, Json} import services.S3 -import topmentions.S3ObjectImplicits.RichS3Object +import topics.S3ObjectImplicits.RichS3Object import model.TopicsApiResponse._ import scala.collection.JavaConverters._ diff --git a/article/app/topmentions/TopicService.scala b/article/app/topics/TopicService.scala similarity index 98% rename from article/app/topmentions/TopicService.scala rename to article/app/topics/TopicService.scala index d2637ff0c012..4f732dc98096 100644 --- a/article/app/topmentions/TopicService.scala +++ b/article/app/topics/TopicService.scala @@ -1,4 +1,4 @@ -package topmentions +package topics import common.{Box, GuLogging} import model.{TopicsApiResponse, TopicResult, SelectedTopic, AvailableTopic} diff --git a/article/test/LiveBlogControllerTest.scala b/article/test/LiveBlogControllerTest.scala index 88fe949a236c..3514c698484e 100644 --- a/article/test/LiveBlogControllerTest.scala +++ b/article/test/LiveBlogControllerTest.scala @@ -10,7 +10,7 @@ import play.api.test.Helpers._ import org.scalatest.{BeforeAndAfterAll, DoNotDiscover} import org.scalatestplus.mockito.MockitoSugar import model.{LiveBlogPage, TopicResult, SelectedTopic, TopMentionsTopicType, AvailableTopic, TopicsLiveBlog} -import topmentions.{TopicS3Client, TopicService} +import topics.{TopicS3Client, TopicService} import scala.concurrent.Future diff --git a/article/test/topmentions/TopMentionsServiceTest.scala b/article/test/topics/TopicServiceTest.scala similarity index 70% rename from article/test/topmentions/TopMentionsServiceTest.scala rename to article/test/topics/TopicServiceTest.scala index 6a58c4ab42e4..89d14e98ce46 100644 --- a/article/test/topmentions/TopMentionsServiceTest.scala +++ b/article/test/topics/TopicServiceTest.scala @@ -1,4 +1,4 @@ -package topmentions +package topics import model.{TopicsApiResponse, TopicResult, SelectedTopic, TopMentionsTopicType, AvailableTopic} import org.scalatest.{BeforeAndAfterAll} @@ -11,7 +11,7 @@ import org.scalatestplus.mockito.MockitoSugar import scala.concurrent.duration._ import scala.concurrent.{Await, Future} -class TopMentionsServiceTest +class TopicServiceTest extends AnyFlatSpec with Matchers with BeforeAndAfterAll @@ -52,10 +52,10 @@ class TopMentionsServiceTest "refreshTopics" should "return successful future given getListOfKeys s3 call fails" in { when(fakeClient.getListOfKeys()) thenReturn Future.failed(new Throwable("")) - val topMentionService = new TopicService(fakeClient) + val topicService = new TopicService(fakeClient) - Await.result(topMentionService.refreshTopics(), 1.second) - val results = topMentionService.getAllTopics + Await.result(topicService.refreshTopics(), 1.second) + val results = topicService.getAllTopics results should be(None) } @@ -65,10 +65,10 @@ class TopMentionsServiceTest when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) when(fakeClient.getObject("key2")) thenReturn Future.failed(new Throwable("error happend")) - val topMentionService = new TopicService(fakeClient) + val topicService = new TopicService(fakeClient) - val refreshJob = Await.result(topMentionService.refreshTopics(), 1.second) - val results = topMentionService.getAllTopics + val refreshJob = Await.result(topicService.refreshTopics(), 1.second) + val results = topicService.getAllTopics refreshJob shouldBe a[Unit] results should be(None) @@ -78,10 +78,10 @@ class TopMentionsServiceTest when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) - val topMentionService = new TopicService(fakeClient) + val topicService = new TopicService(fakeClient) - val refreshJob = Await.result(topMentionService.refreshTopics(), 1.second) - val results = topMentionService.getAllTopics + val refreshJob = Await.result(topicService.refreshTopics(), 1.second) + val results = topicService.getAllTopics refreshJob shouldBe a[Unit] results.isDefined should be(true) @@ -92,10 +92,10 @@ class TopMentionsServiceTest when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) - val topMentionService = new TopicService(fakeClient) - val refreshJob = Await.result(topMentionService.refreshTopics(), 1.second) + val topicService = new TopicService(fakeClient) + val refreshJob = Await.result(topicService.refreshTopics(), 1.second) - val result = topMentionService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "name1")) + val result = topicService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "name1")) result.get should equal(topicResult) } @@ -104,10 +104,10 @@ class TopMentionsServiceTest when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) - val topMentionService = new TopicService(fakeClient) - val refreshJob = Await.result(topMentionService.refreshTopics(), 1.second) + val topicService = new TopicService(fakeClient) + val refreshJob = Await.result(topicService.refreshTopics(), 1.second) - val result = topMentionService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "NAME1")) + val result = topicService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "NAME1")) result should equal(None) } @@ -116,10 +116,10 @@ class TopMentionsServiceTest when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) - val topMentionService = new TopicService(fakeClient) - val refreshJob = Await.result(topMentionService.refreshTopics(), 1.second) + val topicService = new TopicService(fakeClient) + val refreshJob = Await.result(topicService.refreshTopics(), 1.second) - val result = topMentionService.getSelectedTopic("key2", SelectedTopic(TopMentionsTopicType.Org, "name1")) + val result = topicService.getSelectedTopic("key2", SelectedTopic(TopMentionsTopicType.Org, "name1")) result should equal(None) } @@ -128,11 +128,11 @@ class TopMentionsServiceTest when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) - val topMentionService = new TopicService(fakeClient) - val refreshJob = Await.result(topMentionService.refreshTopics(), 1.second) + val topicService = new TopicService(fakeClient) + val refreshJob = Await.result(topicService.refreshTopics(), 1.second) val result = - topMentionService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Person, "Boris")) + topicService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Person, "Boris")) result should equal(None) } @@ -141,11 +141,11 @@ class TopMentionsServiceTest when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successResponse) - val topMentionService = new TopicService(fakeClient) - val refreshJob = Await.result(topMentionService.refreshTopics(), 1.second) + val topicService = new TopicService(fakeClient) + val refreshJob = Await.result(topicService.refreshTopics(), 1.second) val result = - topMentionService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "someRandomOrg")) + topicService.getSelectedTopic("key1", SelectedTopic(TopMentionsTopicType.Org, "someRandomOrg")) result should equal(None) } @@ -161,10 +161,10 @@ class TopMentionsServiceTest ), ) - val topMentionService = new TopicService(fakeClient) - val refreshJob = Await.result(topMentionService.refreshTopics(), 1.second) + val topicService = new TopicService(fakeClient) + val refreshJob = Await.result(topicService.refreshTopics(), 1.second) val topicList = - topMentionService.getAvailableTopics("key1") + topicService.getAvailableTopics("key1") topicList should equal(expectedTopics) } @@ -172,10 +172,10 @@ class TopMentionsServiceTest "getAvailableTopics" should "return none given a blog id that doesn't exist in cache" in { when(fakeClient.getListOfKeys()) thenReturn Future.successful(List("key1")) when(fakeClient.getObject("key1")) thenReturn Future.successful(successMultiResponse) - val topMentionService = new TopicService(fakeClient) + val topicService = new TopicService(fakeClient) val topicList = - topMentionService.getAvailableTopics("key1") + topicService.getAvailableTopics("key1") topicList should equal(None) } diff --git a/common/app/model/LiveBlogCurrentPage.scala b/common/app/model/LiveBlogCurrentPage.scala index f4bf36d463e6..0dc2e8a6061f 100644 --- a/common/app/model/LiveBlogCurrentPage.scala +++ b/common/app/model/LiveBlogCurrentPage.scala @@ -103,7 +103,7 @@ object LiveBlogCurrentPage { if (filterKeyEvents) { getKeyEventsBlocks(blocks) } else if (topicResult.isDefined) { - getTopMentionsBlocks(blocks, topicResult.get) + getTopicBlocks(blocks, topicResult.get) } else { getStandardBlocks(blocks) } @@ -117,7 +117,7 @@ object LiveBlogCurrentPage { blocks.filter(isTopicBlock(topic)).sortBy(_.publishedCreatedTimestamp).reverse } - private def getTopMentionsBlocks( + private def getTopicBlocks( blocks: Blocks, topic: TopicResult, ): (Option[Seq[BodyBlock]], Int, Option[String]) = { diff --git a/dev-build/app/AppLoader.scala b/dev-build/app/AppLoader.scala index 5666b23dde39..5bbb65cb6a09 100644 --- a/dev-build/app/AppLoader.scala +++ b/dev-build/app/AppLoader.scala @@ -72,7 +72,7 @@ trait AppComponents with OnwardServices with FapiServices with ApplicationsServices - with TopMentionsServices { + with TopicServices { //Overriding conflicting members override lazy val ophanApi = wire[OphanApi] diff --git a/preview/app/AppLoader.scala b/preview/app/AppLoader.scala index e66ad06c890e..249a0cdfaeda 100644 --- a/preview/app/AppLoader.scala +++ b/preview/app/AppLoader.scala @@ -38,7 +38,7 @@ trait PreviewLifecycleComponents with FapiServices with OnwardServices with ApplicationsServices - with TopMentionsServices { + with TopicServices { self: FrontendComponents => //Override conflicting members From bc466e6d6a5099952da69dd97eaaf61044d0b763 Mon Sep 17 00:00:00 2001 From: Joe Cowton Date: Thu, 30 Jun 2022 08:45:19 +0100 Subject: [PATCH 5/5] Fix naming --- common/app/model/LiveBlogCurrentPage.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/app/model/LiveBlogCurrentPage.scala b/common/app/model/LiveBlogCurrentPage.scala index 0dc2e8a6061f..7e3856028e2d 100644 --- a/common/app/model/LiveBlogCurrentPage.scala +++ b/common/app/model/LiveBlogCurrentPage.scala @@ -249,7 +249,7 @@ sealed trait PageReference { def buildQueryParam(topicResult: Option[TopicResult]) = { topicResult match { - case Some(value) => s"&topics=${topicResult.get.`type`}:${value.name}" + case Some(value) => s"&topics=${value.`type`}:${value.name}" case None => "" } }