diff --git a/article/app/controllers/LiveBlogController.scala b/article/app/controllers/LiveBlogController.scala index df57e4eb8957..e01efa710359 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.{TopMentionsService} +import topmentions.TopMentionsService import views.support.RenderOtherStatus import scala.concurrent.Future @@ -45,7 +45,7 @@ class LiveBlogController( def renderEmail(path: String): Action[AnyContent] = { Action.async { implicit request => - mapModel(path, ArticleBlocks) { + mapModel(path, ArticleBlocks, topMentionResult = 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 = getTopMentionsByTopics(path, topics) + val topMentions = if (filter) None else getTopMentions(path, topics) val topicList = topMentionsService.getTopics(path) page.map(ParseBlockId.fromPageParam) match { case Some(ParsedBlockId(id)) => @@ -71,7 +71,13 @@ class LiveBlogController( Future.successful( Cached(10)(WithoutRevalidationResult(NotFound)), ) // page param there but couldn't extract a block id - case None => renderWithRange(path, CanonicalLiveBlog, filter, topMentions, topicList) // no page param + case None => { + topMentions match { + case Some(value) => + renderWithRange(path, TopicsLiveBlog, filter, Some(value), topicList) // no page param + case None => renderWithRange(path, CanonicalLiveBlog, filter, None, topicList) // no page param + } + } } } } @@ -87,7 +93,8 @@ class LiveBlogController( Action.async { implicit request: Request[AnyContent] => val filter = shouldFilter(filterKeyEvents) val range = getRange(lastUpdate, page) - mapModel(path, range, filter) { + + mapModel(path, range, filter, None) { case (blog: LiveBlogPage, _) if rendered.contains(false) => getJsonForFronts(blog) case (blog: LiveBlogPage, blocks) if request.forceDCR && lastUpdate.isEmpty => Future.successful(renderGuuiJson(blog, blocks, filter)) @@ -112,7 +119,7 @@ class LiveBlogController( )(implicit request: RequestHeader, ): Future[Result] = { - mapModel(path, range, filterKeyEvents) { (page, blocks) => + mapModel(path, range, filterKeyEvents, topMentionResult) { (page, blocks) => { val isAmpSupported = page.article.content.shouldAmplify val pageType: PageType = PageType(page, request, context) @@ -307,12 +314,17 @@ class LiveBlogController( common.renderJson(json, blog).as("application/json") } - private[this] def mapModel(path: String, range: BlockRange, filterKeyEvents: Boolean = false)( + private[this] def mapModel( + path: String, + range: BlockRange, + filterKeyEvents: Boolean = false, + topMentionResult: Option[TopMentionsResult], + )( render: (PageWithStoryPackage, Blocks) => Future[Result], )(implicit request: RequestHeader): Future[Result] = { capiLookup .lookup(path, Some(range)) - .map(responseToModelOrResult(range, filterKeyEvents)) + .map(responseToModelOrResult(range, filterKeyEvents, topMentionResult)) .recover(convertApiExceptions) .flatMap { case Left((model, blocks)) => render(model, blocks) @@ -323,6 +335,7 @@ class LiveBlogController( private[this] def responseToModelOrResult( range: BlockRange, filterKeyEvents: Boolean, + topMentionResult: Option[TopMentionsResult], )(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) @@ -339,6 +352,7 @@ class LiveBlogController( response, range, filterKeyEvents, + topMentionResult, ).left .map(_ -> blocks) case unknown => @@ -353,7 +367,7 @@ class LiveBlogController( filterKeyEvents.getOrElse(false) } - def getTopMentionsByTopics(blogId: String, topics: Option[String]) = { + def getTopMentions(blogId: String, topics: Option[String]) = { val topMentionsResult = for { topMentionTopic <- TopMentionsTopic.fromString(topics) topMentions <- topMentionsService.getTopMentionsByTopic(blogId, topMentionTopic) diff --git a/article/app/model/LiveBlogHelpers.scala b/article/app/model/LiveBlogHelpers.scala index b644e1111ee5..07ada2c58138 100644 --- a/article/app/model/LiveBlogHelpers.scala +++ b/article/app/model/LiveBlogHelpers.scala @@ -1,6 +1,7 @@ package model import com.gu.contentapi.client.model.v1.ItemResponse +import common.GuLogging import common.`package`._ import model.liveblog.BodyBlock import model.ParseBlockId.ParsedBlockId @@ -8,7 +9,7 @@ import org.joda.time.DateTime import play.api.libs.functional.syntax._ import play.api.libs.json.{JsValue, Json, _} -object LiveBlogHelpers { +object LiveBlogHelpers extends GuLogging { // Get a Seq[BodyBlock] given an article and the "page" request parameter on live-blog pages. @@ -19,7 +20,7 @@ object LiveBlogHelpers { ): Seq[BodyBlock] = { def modelWithRange(range: BlockRange) = - LiveBlogHelpers.createLiveBlogModel(article, range, filterKeyEvents) + LiveBlogHelpers.createLiveBlogModel(article, range, filterKeyEvents, None) val lbcp = param.map(ParseBlockId.fromPageParam) match { case Some(ParsedBlockId(id)) => modelWithRange(PageWithBlock(id)) @@ -39,6 +40,7 @@ object LiveBlogHelpers { liveBlog: Article, range: BlockRange, filterKeyEvents: Boolean, + topMentionResult: Option[TopMentionsResult], ): Option[LiveBlogCurrentPage] = { val pageSize = if (liveBlog.content.tags.tags.map(_.id).contains("sport/sport")) 30 else 10 @@ -49,6 +51,7 @@ object LiveBlogHelpers { _, range, filterKeyEvents, + topMentionResult, ), ) @@ -61,6 +64,7 @@ object LiveBlogHelpers { response: ItemResponse, range: BlockRange, filterKeyEvents: Boolean, + topMentionResult: Option[TopMentionsResult], ): Either[LiveBlogPage, Status] = { val pageSize = if (liveBlog.content.tags.tags.map(_.id).contains("sport/sport")) 30 else 10 @@ -72,6 +76,7 @@ object LiveBlogHelpers { blocks, range, filterKeyEvents, + topMentionResult, ) } getOrElse None diff --git a/article/test/DCRFake.scala b/article/test/DCRFake.scala index fb4baed2d2d0..39c6029999fb 100644 --- a/article/test/DCRFake.scala +++ b/article/test/DCRFake.scala @@ -8,10 +8,15 @@ import play.api.libs.ws.WSClient import play.api.mvc.{RequestHeader, Result} import play.twirl.api.Html +import scala.collection.mutable +import scala.collection.mutable.Queue import scala.concurrent.{ExecutionContext, Future} // It is always a mistake to rely on actual DCR output for tests. class DCRFake(implicit context: ApplicationContext) extends renderers.DotcomRenderingService { + + val requestedBlogs: Queue[PageWithStoryPackage] = new Queue[PageWithStoryPackage]() + override def getArticle( ws: WSClient, article: PageWithStoryPackage, @@ -22,6 +27,7 @@ class DCRFake(implicit context: ApplicationContext) extends renderers.DotcomRend topics: Option[Seq[TopicWithCount]], )(implicit request: RequestHeader): Future[Result] = { implicit val ec = ExecutionContext.global + requestedBlogs.enqueue(article) Future( Cached(article)(RevalidatableResult.Ok(Html("FakeRemoteRender has found you out if you rely on this markup!"))), ) diff --git a/article/test/LiveBlogControllerTest.scala b/article/test/LiveBlogControllerTest.scala index 24ca1bd9f868..5a073f48c1f1 100644 --- a/article/test/LiveBlogControllerTest.scala +++ b/article/test/LiveBlogControllerTest.scala @@ -2,14 +2,14 @@ package test import controllers.LiveBlogController import org.mockito.Mockito._ -import org.mockito.Matchers.any +import org.mockito.Matchers.{any, anyObject, anyString} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import play.api.test._ import play.api.test.Helpers._ import org.scalatest.{BeforeAndAfterAll, DoNotDiscover} import org.scalatestplus.mockito.MockitoSugar -import model.{TopMentionsResult, TopMentionsTopic, TopMentionsTopicType} +import model.{LiveBlogPage, TopMentionsResult, TopMentionsTopic, TopMentionsTopicType, TopicsLiveBlog} import topmentions.{TopMentionsS3Client, TopMentionsService} import scala.concurrent.Future @@ -28,28 +28,38 @@ import scala.concurrent.Future val liveBlogUrl = "global/middle-east-live/2013/sep/09/syria-crisis-russia-kerry-us-live" val path = "/football/live/2016/feb/26/fifa-election-who-will-succeed-sepp-blatter-president-live" - val fakeTopMentionsService = mock[TopMentionsService] + var fakeTopMentionsService = mock[TopMentionsService] val topMentionResult = TopMentionsResult( - name = "nhs", + name = "Fifa", `type` = TopMentionsTopicType.Org, - blocks = Seq("blockId1"), + blocks = Seq("56d02bd2e4b0d38537b1f5fa"), count = 1, percentage_blocks = 1.2f, ) - when( - fakeTopMentionsService.getTopMentionsByTopic(path, TopMentionsTopic(TopMentionsTopicType.Org, "nhs")), - ) thenReturn Some( - topMentionResult, - ) + var fakeDcr = new DCRFake() lazy val liveBlogController = new LiveBlogController( testContentApiClient, play.api.test.Helpers.stubControllerComponents(), wsClient, - new DCRFake(), + fakeDcr, fakeTopMentionsService, ) + override def beforeAll(): Unit = { + // This is to assure on every test we have a fresh instance of DCR + // and requestedBlogs is only having the calls for the relevant test + fakeDcr = new DCRFake() + + // This is to assure on every test we have a fresh instance of TopicService + fakeTopMentionsService = mock[TopMentionsService] + when( + fakeTopMentionsService.getTopMentionsByTopic(path, TopMentionsTopic(TopMentionsTopicType.Org, "Fifa")), + ) thenReturn Some( + topMentionResult, + ) + } + it should "return the latest blocks of a live blog" in { val lastUpdateBlock = "block-56d03169e4b074a9f6b35baa" val fakeRequest = FakeRequest( @@ -82,7 +92,7 @@ import scala.concurrent.Future } - it should "return the latest blocks of a live blog using DCR" in { + "renderJson" should "return the latest blocks of a live blog using DCR" in { val lastUpdateBlock = "block-56d03169e4b074a9f6b35baa" val fakeRequest = FakeRequest( GET, @@ -229,15 +239,58 @@ import scala.concurrent.Future liveBlogController.shouldFilter(None) should be(false) } - "getTopMentionsForFilters" should "return none given no automatic filter query parameter" in { - liveBlogController.getTopMentionsByTopics(path, None) should be(None) + "getTopMentionsForFilters" should "returns none given no automatic filter query parameter" in { + liveBlogController.getTopMentions(path, None) should be(None) } - "getTopMentionsForFilters" should "return none given an incorrect automatic filter query parameter" in { - liveBlogController.getTopMentionsByTopics(path, Some("orgnhs")) should be(None) + "getTopMentionsForFilters" should "returns none given an incorrect automatic filter query parameter" in { + liveBlogController.getTopMentions(path, Some("orgFifa")) should be(None) } - "getTopMentionsForFilters" should "return correct topMentionResult given a correct automatic filter query parameter" in { - liveBlogController.getTopMentionsByTopics(path, Some("org:nhs")) should be(Some(topMentionResult)) + "getTopMentionsForFilters" should "returns correct topMentionResult given a correct automatic filter query parameter" in { + liveBlogController.getTopMentions(path, Some("org:Fifa")) should be(Some(topMentionResult)) + } + + "renderArticle" should "returns the first page of filtered blog by topics" in { + val fakeRequest = FakeRequest( + GET, + s"${path}", + ).withHeaders("host" -> "localhost:9000") + + val result = liveBlogController.renderArticle( + path, + page = None, + filterKeyEvents = Some(false), + topics = Some("org:Fifa"), + )(fakeRequest) + + status(result) should be(200) + assertDcrCalledForLiveBlogWithBlocks(expectedBlocks = Seq("56d02bd2e4b0d38537b1f5fa")) + } + + "renderArticle" should "doesn't call getTopMentionsByTopic given filterKeyEvents and topics query params are provided" in { + reset(fakeTopMentionsService) + val fakeRequest = FakeRequest(GET, s"${path}").withHeaders("host" -> "localhost:9000") + + val result = liveBlogController.renderArticle( + path, + page = None, + filterKeyEvents = Some(true), + topics = Some("org:Fifa"), + )(fakeRequest) + + verify(fakeTopMentionsService, times(0)).getTopMentionsByTopic(anyString(), anyObject()) + status(result) should be(200) + } + + private def assertDcrCalledForLiveBlogWithBlocks(expectedBlocks: Seq[String]) = { + val liveblog = fakeDcr.requestedBlogs.dequeue() + + liveblog match { + case LiveBlogPage(_, currentPage, _, _) => { + currentPage.currentPage.blocks.map(_.id) should be(expectedBlocks) + } + case _ => fail("DCR was not called with a LiveBlogPage") + } } } diff --git a/article/test/model/LiveBlogCurrentPageTest.scala b/article/test/model/LiveBlogCurrentPageTest.scala index 693ee4c44cdc..e7993dc61057 100644 --- a/article/test/model/LiveBlogCurrentPageTest.scala +++ b/article/test/model/LiveBlogCurrentPageTest.scala @@ -2,6 +2,7 @@ package model import model.liveblog.BodyBlock.{KeyEvent, SummaryEvent} import model.liveblog._ +import model.TopMentionsTopicType.TopMentionsTopicType import org.joda.time.DateTime import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -55,12 +56,34 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { regular ++ keyEvents ++ pinnedBlocks ++ summaries } + "LiveBlogCurrentPage.apply" should "create first page given a block range of TopicsLiveBlog" in { + val range = TopicsLiveBlog + val result = LiveBlogCurrentPage.apply( + pageSize = 10, + blocks = Blocks(1, Seq(), None, Map()), + range, + filterKeyEvents = false, + topMentionResult = Some( + TopMentionsResult( + `type` = TopMentionsTopicType.Org, + name = "someName", + blocks = Seq(), + count = 0, + percentage_blocks = 0, + ), + ), + ) + + result.get.currentPage shouldBe (a[FirstPage]) + } + "firstPage" should "allow 1 block on one page" in { val result = { LiveBlogCurrentPage.firstPage( 2, Blocks(1, Nil, None, Map(CanonicalLiveBlog.firstPage -> Seq(fakeBlock(1)))), false, + None, ) } @@ -103,6 +126,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { ), ), false, + None, ) result.get.pinnedBlock should be(Some(latestPinnedBlock)) result.get.pinnedBlock should not be (Some(olderPinnedBlock)) @@ -128,6 +152,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { ), ), false, + None, ) result.get.pinnedBlock should be(Some(expectedPinnedBlock)) @@ -153,6 +178,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { requestedBodyBlocks, ), true, + None, ) result.get.pinnedBlock should be(Some(expectedPinnedBlock)) @@ -175,6 +201,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { requestedBodyBlocks, ), true, + None, ) result should be(None) @@ -197,6 +224,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { requestedBodyBlocks, ), true, + None, ) result should be(None) @@ -220,6 +248,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { requestedBodyBlocks, ), true, + None, ) should(result, currentPage = FirstPage(List(), true), pagination = None) @@ -237,6 +266,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { Map(CanonicalLiveBlog.firstPage -> blocks.take(4), CanonicalLiveBlog.oldestPage -> blocks.lastOption.toSeq), ), false, + None, ) should(result, FirstPage(blocks, filterKeyEvents = false), None) @@ -253,6 +283,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { Map(CanonicalLiveBlog.firstPage -> blocks.take(4), CanonicalLiveBlog.oldestPage -> blocks.lastOption.toSeq), ), false, + None, ) val expected = blocks.take(2) @@ -282,6 +313,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { Map(CanonicalLiveBlog.firstPage -> blocks.take(4), CanonicalLiveBlog.oldestPage -> blocks.lastOption.toSeq), ), false, + None, ) val expectedCurrentPage = FirstPage(blocks = blocks.take(3), filterKeyEvents = false) @@ -318,6 +350,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { ), ), true, + None, ) val expectedCurrentPage = FirstPage(blocks = keyBlocks.take(3), filterKeyEvents = true) @@ -336,9 +369,57 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { should(result, currentPage = expectedCurrentPage, pagination = expectedPagination) } + 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 testFakeBlocks = TestFakeBlocks(numberOfBlocks = 5, numberOfKeyEventsBlocks = 2) + val result = + LiveBlogCurrentPage.firstPage( + pageSize = 2, + blocks = testFakeBlocks.blocksType, + filterKeyEvents = true, + topMentionResult = Some(topMentions), + ) + + result.get should be( + LiveBlogCurrentPage( + currentPage = FirstPage(testFakeBlocks.blocksSequence.slice(3, 5), filterKeyEvents = true), + pagination = None, + pinnedBlock = None, + ), + ) + } + + it should "returns the 1st page of the topic filtered blocks" in { + val topMentions = getTopMentionsForTopicAndBlocks(TopMentionsTopicType.Org, "tfl", Seq("1", "2", "3", "4")) + val testFakeBlocks = TestFakeBlocks(numberOfBlocks = 5, numberOfKeyEventsBlocks = 2) + val result = + LiveBlogCurrentPage.firstPage( + pageSize = 2, + blocks = testFakeBlocks.blocksType, + filterKeyEvents = false, + topMentionResult = Some(topMentions), + ) + + val expectedPagination = Some( + N1Pagination( + newest = None, + newer = None, + older = Some(BlockPage(blocks = Nil, blockId = "2", pageNumber = 2, filterKeyEvents = false)), + oldest = Some(BlockPage(blocks = Nil, blockId = "1", pageNumber = 2, filterKeyEvents = false)), + numberOfPages = 2, + ), + ) + + should( + result, + currentPage = FirstPage(testFakeBlocks.blocksSequence.slice(1, 3), filterKeyEvents = false), + pagination = expectedPagination, + ) + } + "findPageWithBlock" should "put 4 blocks on two pages - older page link" in { val blocks = fakeBlocks(4) - val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", false) + val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", false, None) val expectedCurrentPage = BlockPage(blocks = blocks.takeRight(2), blockId = "2", pageNumber = 2, filterKeyEvents = false) @@ -358,7 +439,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { it should "put 4 blocks on two pages - link to another block on the page" in { val blocks = fakeBlocks(4) - val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "1", false) + val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "1", false, None) val expectedCurrentPage = BlockPage(blocks = blocks.takeRight(2), blockId = "2", pageNumber = 2, filterKeyEvents = false) @@ -378,7 +459,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { it should "put 5 blocks on two pages (block 3 from oldest page)" in { val blocks = fakeBlocks(5) - val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", false) + val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", false, None) val expectedCurrentPage = BlockPage(blocks = blocks.takeRight(2), blockId = "2", pageNumber = 2, filterKeyEvents = false) @@ -407,6 +488,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { Map(CanonicalLiveBlog.firstPage -> blocks.take(4), CanonicalLiveBlog.oldestPage -> blocks.lastOption.toSeq), ), false, + None, ) val expectedCurrentPage = FirstPage(blocks = blocks.take(2), filterKeyEvents = false) @@ -427,7 +509,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { it should "put 6 blocks on 3 pages (middle page)" in { val blocks = fakeBlocks(6) - val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "4", false) + val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "4", false, None) val expectedCurrentPage = BlockPage(blocks = blocks.slice(2, 4), blockId = "4", pageNumber = 2, filterKeyEvents = false) @@ -449,7 +531,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { it should "put 6 blocks on 3 pages (oldest page)" in { val blocks = fakeBlocks(6) - val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", false) + val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", false, None) val expectedCurrentPage = BlockPage(blocks = blocks.takeRight(2), blockId = "2", pageNumber = 3, filterKeyEvents = false) @@ -472,7 +554,7 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { it should "display only key events and summaries when the filter is on" in { val blocks = fakeBlocks(12, 4, 0, 2) val keyAndSummaryBlocks = blocks.filter(block => block.eventType == KeyEvent || block.eventType == SummaryEvent) - val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", true) + val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", true, None) val expectedCurrentPage = { BlockPage(blocks = keyAndSummaryBlocks.takeRight(2), blockId = "2", pageNumber = 3, filterKeyEvents = true) @@ -495,9 +577,69 @@ class LiveBlogCurrentPageTest extends AnyFlatSpec with Matchers { it should "display nothing when no key events exist and the filter is on" in { val blocks = fakeBlocks(6, 0) - val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", true) + val result = LiveBlogCurrentPage.findPageWithBlock(2, blocks, "2", true, None) result should be(None) } + 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 testFakeBlocks = TestFakeBlocks(numberOfBlocks = 8, numberOfKeyEventsBlocks = 2) + + val expectedFirstPage = FirstPage(testFakeBlocks.blocksSequence.slice(2, 5), filterKeyEvents = false) + val expectedCurrentPage = BlockPage( + blocks = testFakeBlocks.blocksSequence.slice(5, 7), + blockId = "3", + pageNumber = 2, + filterKeyEvents = false, + ) + val expectedPagination = Some( + N1Pagination( + newest = Some(expectedFirstPage), + newer = Some(expectedFirstPage), + older = None, + oldest = None, + numberOfPages = 2, + ), + ) + + val result = LiveBlogCurrentPage.findPageWithBlock( + pageSize = 2, + blocks = testFakeBlocks.blocksSequence, + isRequestedBlock = "3", + filterKeyEvents = false, + topMentionsResult = Some(topMentions), + ) + + should(result, expectedCurrentPage, expectedPagination) + } + + private def getTopMentionsForTopicAndBlocks( + tpoicType: TopMentionsTopicType, + topicName: String, + blocks: Seq[String], + ) = { + TopMentionsResult( + `type` = tpoicType, + name = topicName, + blocks = blocks, + count = 0, + percentage_blocks = 0, + ) + } + + case class TestFakeBlocks(numberOfBlocks: Int, numberOfKeyEventsBlocks: Int) { + val blocksSequence = fakeBlocks(numberOfBlocks, numberOfKeyEventsBlocks).toSeq + + val blocksType = Blocks( + blocksSequence.length, + blocksSequence, + None, + Map( + CanonicalLiveBlog.timeline -> blocksSequence.slice(numberOfBlocks - numberOfKeyEventsBlocks, numberOfBlocks), + CanonicalLiveBlog.summary -> Seq(), + ), + ) + } + } diff --git a/common/app/model/ArticleBlocks.scala b/common/app/model/ArticleBlocks.scala index baa3c530440f..5eca4f69c3ec 100644 --- a/common/app/model/ArticleBlocks.scala +++ b/common/app/model/ArticleBlocks.scala @@ -19,6 +19,16 @@ case object CanonicalLiveBlog extends BlockRange { val query = Some(Seq(mainBlock, firstPage, oldestPage, timeline, summary, pinned)) } +case object TopicsLiveBlog extends BlockRange { + val mainBlock = "main" + val oldestPage = "body:oldest:1" + val timeline = "body:key-events" + val summary = "body:summary" + val pinned = "body:pinned" + val body = "body" // this gets all blocks that's needed for filtering blog by topics + val query = Some(Seq(mainBlock, oldestPage, timeline, summary, pinned, body)) +} + // Created to handle ArticleController (for preview) specifically, where we may render an // article or a liveblog (that doesn't get caught by the liveblog specific routes, which may // or may not actually happen in the wild) so we request both specific blocks and the whole body diff --git a/common/app/model/LiveBlogCurrentPage.scala b/common/app/model/LiveBlogCurrentPage.scala index 68be27b24d2f..ab9e37100950 100644 --- a/common/app/model/LiveBlogCurrentPage.scala +++ b/common/app/model/LiveBlogCurrentPage.scala @@ -18,11 +18,12 @@ object LiveBlogCurrentPage { blocks: Blocks, range: BlockRange, filterKeyEvents: Boolean, + topMentionResult: Option[TopMentionsResult], ): Option[LiveBlogCurrentPage] = { range match { - case CanonicalLiveBlog => firstPage(pageSize, blocks, filterKeyEvents) + case CanonicalLiveBlog | TopicsLiveBlog => firstPage(pageSize, blocks, filterKeyEvents, topMentionResult) case PageWithBlock(isRequestedBlock) => - findPageWithBlock(pageSize, blocks.body, isRequestedBlock, filterKeyEvents) + findPageWithBlock(pageSize, blocks.body, isRequestedBlock, filterKeyEvents, topMentionResult) case SinceBlockId(blockId) => updates(blocks, SinceBlockId(blockId), filterKeyEvents) case ArticleBlocks => None case GenericFallback => None @@ -37,7 +38,7 @@ object LiveBlogCurrentPage { filterKeyEvents: Boolean, ): Option[LiveBlogCurrentPage] = { val bodyBlocks = blocks.requestedBodyBlocks.get(sinceBlockId.around).toSeq.flatMap { bodyBlocks => - applyFilters(bodyBlocks, filterKeyEvents).takeWhile(_.id != sinceBlockId.lastUpdate) + applyFilters(bodyBlocks, filterKeyEvents, None).takeWhile(_.id != sinceBlockId.lastUpdate) } Some( LiveBlogCurrentPage(FirstPage(bodyBlocks, filterKeyEvents), None, None), @@ -45,29 +46,14 @@ object LiveBlogCurrentPage { } // turns the slimmed down (to save bandwidth) capi response into a first page model object - def firstPage(pageSize: Int, blocks: Blocks, filterKeyEvents: Boolean): Option[LiveBlogCurrentPage] = { - val (maybeRequestedBodyBlocks, blockCount, oldestPageBlockId) = if (filterKeyEvents) { - - val keyEventsAndSummaries = for { - keyEvents <- blocks.requestedBodyBlocks.get(CanonicalLiveBlog.timeline) - summaries <- blocks.requestedBodyBlocks.get(CanonicalLiveBlog.summary) - } yield { - (keyEvents ++ summaries).sortBy(_.publishedCreatedTimestamp).reverse - } - - val keyEventsAndSummariesCount = keyEventsAndSummaries.getOrElse(Seq.empty).size - - val oldestPageBlockId = keyEventsAndSummaries.flatMap(_.lastOption map (_.id)) - - (keyEventsAndSummaries, keyEventsAndSummariesCount, oldestPageBlockId) - - } else { - val firstPageBlocks = blocks.requestedBodyBlocks.get(CanonicalLiveBlog.firstPage) - val oldestPageBlockId = - blocks.requestedBodyBlocks.get(CanonicalLiveBlog.oldestPage) flatMap (_.headOption.map(_.id)) - - (firstPageBlocks, blocks.totalBodyBlocks, oldestPageBlockId) - } + def firstPage( + pageSize: Int, + blocks: Blocks, + filterKeyEvents: Boolean, + topMentionResult: Option[TopMentionsResult], + ): Option[LiveBlogCurrentPage] = { + val (maybeRequestedBodyBlocks, blockCount, oldestPageBlockId) = + extractFirstPageBlocks(blocks, filterKeyEvents, topMentionResult) val remainder = blockCount % pageSize val numPages = blockCount / pageSize @@ -106,6 +92,62 @@ object LiveBlogCurrentPage { } } + private def extractFirstPageBlocks( + blocks: Blocks, + filterKeyEvents: Boolean, + topMentionResult: Option[TopMentionsResult], + ) = { + if (filterKeyEvents) { + getKeyEventsBlocks(blocks) + } else if (topMentionResult.isDefined) { + getTopMentionsBlocks(blocks, topMentionResult.get) + } else { + getStandardBlocks(blocks) + } + } + + private def isTopMentionBlock(topMentionsResult: TopMentionsResult)(bodyBlock: BodyBlock): Boolean = { + topMentionsResult.blocks.contains(bodyBlock.id) + } + + private def filterBlocksByTopMentions(blocks: Seq[BodyBlock], topMentionsResult: TopMentionsResult) = { + blocks.filter(isTopMentionBlock(topMentionsResult)).sortBy(_.publishedCreatedTimestamp).reverse + } + + private def getTopMentionsBlocks( + blocks: Blocks, + topMentionsResult: TopMentionsResult, + ): (Option[Seq[BodyBlock]], Int, Option[String]) = { + val bodyBlocks = blocks.body + + val filteredBodyBlocks = filterBlocksByTopMentions(bodyBlocks, topMentionsResult) + + (Some(filteredBodyBlocks), filteredBodyBlocks.length, filteredBodyBlocks.lastOption.map(_.id)) + } + + private def getStandardBlocks(blocks: Blocks): (Option[Seq[BodyBlock]], Int, Option[String]) = { + val firstPageBlocks = blocks.requestedBodyBlocks.get(CanonicalLiveBlog.firstPage) + val oldestPageBlockId = + blocks.requestedBodyBlocks.get(CanonicalLiveBlog.oldestPage) flatMap (_.headOption.map(_.id)) + + (firstPageBlocks, blocks.totalBodyBlocks, oldestPageBlockId) + } + + private def getKeyEventsBlocks(blocks: Blocks) = { + val keyEventsAndSummaries = for { + keyEvents <- blocks.requestedBodyBlocks.get(CanonicalLiveBlog.timeline) + summaries <- blocks.requestedBodyBlocks.get(CanonicalLiveBlog.summary) + } yield { + (keyEvents ++ summaries).sortBy(_.publishedCreatedTimestamp).reverse + } + + val keyEventsAndSummariesCount = keyEventsAndSummaries.getOrElse(Seq.empty).size + + val oldestPageBlockId = keyEventsAndSummaries.flatMap(_.lastOption map (_.id)) + + (keyEventsAndSummaries, keyEventsAndSummariesCount, oldestPageBlockId) + } + private def removeFirstBlockIfPinned(firstPageBlocks: Seq[BodyBlock], pinnedBlock: Option[BodyBlock]) = { firstPageBlocks match { case firstBlock +: otherBlocks if pinnedBlock.contains(firstBlock) => otherBlocks @@ -119,9 +161,10 @@ object LiveBlogCurrentPage { blocks: Seq[BodyBlock], isRequestedBlock: String, filterKeyEvents: Boolean, + topMentionsResult: Option[TopMentionsResult], ): Option[LiveBlogCurrentPage] = { val pinnedBlock = blocks.find(_.attributes.pinned).map(renamePinnedBlock) - val filteredBlocks = applyFilters(blocks, filterKeyEvents) + val filteredBlocks = applyFilters(blocks, filterKeyEvents, topMentionsResult) val (mainPageBlocks, restPagesBlocks) = getPages(pageSize, filteredBlocks) val newestPage = FirstPage(mainPageBlocks, filterKeyEvents) val pages = newestPage :: restPagesBlocks.zipWithIndex @@ -168,9 +211,15 @@ object LiveBlogCurrentPage { pinnedBlock.copy(id = s"${pinnedBlock.id}-pinned") } - private def applyFilters(blocks: Seq[BodyBlock], filterKeyEvents: Boolean) = { + private def applyFilters( + blocks: Seq[BodyBlock], + filterKeyEvents: Boolean, + topMentionsResult: Option[TopMentionsResult], + ) = { if (filterKeyEvents) { blocks.filter(block => block.eventType == KeyEvent || block.eventType == SummaryEvent) + } else if (topMentionsResult.isDefined) { + filterBlocksByTopMentions(blocks, topMentionsResult.get) } else { blocks } diff --git a/data/database/82e438e354154ed818acaa143c00ee0ffd67ba9bbe659a99dda3b44630638cba b/data/database/82e438e354154ed818acaa143c00ee0ffd67ba9bbe659a99dda3b44630638cba new file mode 100644 index 000000000000..c29fad002e0f Binary files /dev/null and b/data/database/82e438e354154ed818acaa143c00ee0ffd67ba9bbe659a99dda3b44630638cba differ