From 9f359da01a284f9e40b39c9aa86fbcab32f49896 Mon Sep 17 00:00:00 2001 From: fweddi Date: Fri, 21 Feb 2025 15:14:29 +0100 Subject: [PATCH] Use GroupsConfig over Groups (WIP) --- app/model/frontsapi.scala | 4 ++-- build.sbt | 2 +- fronts-client/src/fixtures/shared.ts | 17 +++++++++++++++-- fronts-client/src/types/Collection.ts | 8 +++++++- fronts-client/src/types/FaciaApi.ts | 4 ++-- fronts-client/src/util/__tests__/shared.spec.ts | 10 +++++----- fronts-client/src/util/frontsUtils.ts | 2 +- fronts-client/src/util/shared.ts | 14 +++++++------- test/config/TransformationsSpec.scala | 9 +++------ test/services/CollectionServiceTest.scala | 14 ++++++-------- 10 files changed, 49 insertions(+), 35 deletions(-) diff --git a/app/model/frontsapi.scala b/app/model/frontsapi.scala index 9e567518efb..a47821fa4b8 100644 --- a/app/model/frontsapi.scala +++ b/app/model/frontsapi.scala @@ -421,8 +421,8 @@ trait UpdateActionsTrait extends Logging { collectionId: String, collectionJson: CollectionJson ): CollectionJson = { - configAgent.getConfig(collectionId).flatMap(_.groups) match { - case Some(groups) if groups.groups.nonEmpty => collectionJson + configAgent.getConfig(collectionId).flatMap(_.groupsConfig) match { + case Some(groupsConfig) if groupsConfig.nonEmpty => collectionJson case _ => collectionJson.copy( live = collectionJson.live.map(removeGroupsFromTrail), diff --git a/build.sbt b/build.sbt index 68e830484d1..7822fcb8e37 100644 --- a/build.sbt +++ b/build.sbt @@ -83,7 +83,7 @@ libraryDependencies ++= Seq( "com.gu" %% "content-api-client-aws" % "0.7.6", "com.gu" %% "content-api-client-default" % capiClientVersion, "com.gu" %% "editorial-permissions-client" % "3.0.0", - "com.gu" %% "fapi-client-play30" % "15.0.0", + "com.gu" %% "fapi-client-play30" % "16.0.0-PREVIEW.fpadd-groups-config-field.2025-02-21T1227.15f48131", "com.gu" %% "mobile-notifications-api-models" % "3.0.0", "com.gu" %% "pan-domain-auth-play_3-0" % "7.0.0", "org.scanamo" %% "scanamo" % "1.1.1" exclude ("org.scala-lang.modules", "scala-java8-compat_2.13"), diff --git a/fronts-client/src/fixtures/shared.ts b/fronts-client/src/fixtures/shared.ts index e3b27c0f189..9684af313db 100644 --- a/fronts-client/src/fixtures/shared.ts +++ b/fronts-client/src/fixtures/shared.ts @@ -1,3 +1,5 @@ +import {CollectionConfig} from "../types/FaciaApi"; + const capiArticle = { id: 'world/live/2018/sep/13/florence-hurricane-latest-live-news-updates-weather-path-storm-surge-north-carolina', type: 'liveblog', @@ -591,11 +593,22 @@ const normalisedCollectonWithPreviously = { excludedRegions: [], }; -const collectionConfig = { +const collectionConfig: CollectionConfig = { id: 'exampleCollection', displayName: 'Example Collection', type: 'any', - groups: ['large', 'medium', 'small'], + groupsConfig: [{ + name: 'large', + maxItems: 10, + }, + { + name: 'medium', + maxItems: 10, + }, + { + name: 'small', + maxItems: 10, + }] }; const collectionWithSupportingArticles = { diff --git a/fronts-client/src/types/Collection.ts b/fronts-client/src/types/Collection.ts index 77739c24c7d..0e0bf8ff318 100644 --- a/fronts-client/src/types/Collection.ts +++ b/fronts-client/src/types/Collection.ts @@ -21,6 +21,11 @@ interface Group { cards: string[]; } +interface GroupConfig { + name: string; + maxItems: number | null; +} + /** CardSets represent all of the lists of cards available in a collection. */ type CardSets = 'draft' | 'live' | 'previously'; /** Stages represent only those lists which are curated by the user.*/ @@ -183,7 +188,7 @@ interface Collection { updatedEmail?: string; platform?: string; displayName: string; - groups?: string[]; + groupsConfig?: GroupConfig[]; metadata?: Array<{ type: string }>; uneditable?: boolean; type?: string; @@ -213,6 +218,7 @@ export { Collection, CardSizes, Group, + GroupConfig, Stages, CardSets, ArticleTag, diff --git a/fronts-client/src/types/FaciaApi.ts b/fronts-client/src/types/FaciaApi.ts index 8772ce53140..7b0cc1b8884 100644 --- a/fronts-client/src/types/FaciaApi.ts +++ b/fronts-client/src/types/FaciaApi.ts @@ -1,5 +1,5 @@ import { $Diff } from 'utility-types'; -import { CollectionFromResponse, NestedCard } from 'types/Collection'; +import {CollectionFromResponse, GroupConfig, NestedCard} from 'types/Collection'; import { EditionsPrefill } from './Edition'; interface FrontConfigResponse { @@ -43,7 +43,7 @@ interface CollectionConfigResponse { type?: string; backfill?: unknown; href?: string; - groups?: string[]; + groupsConfig?: GroupConfig[]; metadata?: unknown[]; uneditable?: boolean; showTags?: boolean; diff --git a/fronts-client/src/util/__tests__/shared.spec.ts b/fronts-client/src/util/__tests__/shared.spec.ts index 85270a26176..a1d53525646 100644 --- a/fronts-client/src/util/__tests__/shared.spec.ts +++ b/fronts-client/src/util/__tests__/shared.spec.ts @@ -109,7 +109,7 @@ describe('Shared utilities', () => { expect(result.cards[prevGroup3.cards[0]].id).toBe('article/live/3'); }); it('should insert a default group for empty collections', () => { - const { groups, ...collectionConfigWithoutGroups } = collectionConfig; + const { groupsConfig, ...collectionConfigWithoutGroups } = collectionConfig; const result = normaliseCollectionWithNestedArticles( { ...collection, @@ -145,16 +145,16 @@ describe('Shared utilities', () => { collectionWithoutGroups, { ...collectionConfig, - groups: undefined, + groupsConfig: undefined, }, ); const groupId = result.normalisedCollection.live![0]; expect(result.groups[groupId].cards).toHaveLength(3); }); - it('should create different groups for cards belonging to different groups even if they are not specificied in the config', () => { + it('should create different groups for cards belonging to different groups even if they are not specified in the config', () => { const result = normaliseCollectionWithNestedArticles(collection, { ...collectionConfig, - groups: undefined, + groupsConfig: undefined, }); const groupId1 = result.normalisedCollection.live![0]; expect(result.groups[groupId1].cards).toHaveLength(1); @@ -168,7 +168,7 @@ describe('Shared utilities', () => { }; const result = normaliseCollectionWithNestedArticles(collection, { ...configWithExtraGroup, - groups: undefined, + groupsConfig: undefined, }); expect(Object.keys(result.groups)).toHaveLength(4); }); diff --git a/fronts-client/src/util/frontsUtils.ts b/fronts-client/src/util/frontsUtils.ts index 5167e38892b..2b9ec19d8a5 100644 --- a/fronts-client/src/util/frontsUtils.ts +++ b/fronts-client/src/util/frontsUtils.ts @@ -38,7 +38,7 @@ const combineCollectionWithConfig = ( displayName: useCollectionDisplayName ? collection.displayName : collectionConfig.displayName, - groups: collectionConfig.groups, + groupsConfig: collectionConfig.groupsConfig, type: collectionConfig.type, frontsToolSettings: collectionConfig.frontsToolSettings, platform: collectionConfig.platform, diff --git a/fronts-client/src/util/shared.ts b/fronts-client/src/util/shared.ts index 27b4902046e..8601906e8b1 100644 --- a/fronts-client/src/util/shared.ts +++ b/fronts-client/src/util/shared.ts @@ -53,20 +53,20 @@ const addGroupsForStage = ( let name: string | null = null; const groupNumberAsInt = getGroupIndex(group.id); if ( - collectionConfig.groups && - groupNumberAsInt < collectionConfig.groups.length + collectionConfig.groupsConfig && + groupNumberAsInt < collectionConfig.groupsConfig.length ) { - name = collectionConfig.groups[groupNumberAsInt]; + name = collectionConfig.groupsConfig[groupNumberAsInt].name; } return { ...group, name }; }); // We may have empty groups in the config which would not show up in the normalised // groups result. We need to add these into the groups array. - if (collectionConfig.groups) { - collectionConfig.groups.forEach((group, configGroupIndex) => { - if (!configGroupIndexExistsInGroups(groupsWithNames, configGroupIndex)) { - groupsWithNames.push(createGroup(`${configGroupIndex}`, group)); + if (collectionConfig.groupsConfig) { + collectionConfig.groupsConfig.forEach((groupConfig, groupConfigIndex) => { + if (!configGroupIndexExistsInGroups(groupsWithNames, groupConfigIndex)) { + groupsWithNames.push(createGroup(`${groupConfigIndex}`, groupConfig.name)); } }); } diff --git a/test/config/TransformationsSpec.scala b/test/config/TransformationsSpec.scala index ff01da2f0dc..3a864fc987c 100644 --- a/test/config/TransformationsSpec.scala +++ b/test/config/TransformationsSpec.scala @@ -1,10 +1,6 @@ package config -import com.gu.facia.client.models.{ - CollectionConfigJson => CollectionConfig, - ConfigJson => Config, - FrontJson => Front -} +import com.gu.facia.client.models.{GroupsConfigJson, CollectionConfigJson => CollectionConfig, ConfigJson => Config, FrontJson => Front} import org.scalatest._ import updates.CreateFront @@ -13,7 +9,7 @@ class TransformationsSpec extends FlatSpec with Matchers { displayName = Some("New collection"), `type` = Some("???"), href = Some("newfront"), - groups = Some(List("1", "2")), + groupsConfig = Some(List(GroupsConfigJson(name = "1", maxItems = Some(10)), GroupsConfigJson(name = "2", maxItems = Some(10)))), uneditable = Some(false), showTags = Some(true), showSections = Some(false), @@ -60,6 +56,7 @@ class TransformationsSpec extends FlatSpec with Matchers { None, None, None, + None, None ) diff --git a/test/services/CollectionServiceTest.scala b/test/services/CollectionServiceTest.scala index 6e7e587bd09..804ddf3b52f 100644 --- a/test/services/CollectionServiceTest.scala +++ b/test/services/CollectionServiceTest.scala @@ -1,12 +1,6 @@ package services -import com.gu.facia.client.models.{ - CollectionConfigJson, - CollectionJson, - ConfigJson, - FrontJson, - Trail -} +import com.gu.facia.client.models.{CollectionConfigJson, CollectionJson, ConfigJson, FrontJson, GroupsConfigJson, Trail} import conf.ApplicationConfiguration import org.joda.time.DateTime import org.scalatest.{FreeSpec, Matchers} @@ -89,7 +83,11 @@ class CollectionServiceTest extends FreeSpec with Matchers { `type` = Some("dynamic/slow"), href = None, description = None, - groups = Some(List("Group 1", "Group 2")), + groups = None, + groupsConfig = Some(List( + GroupsConfigJson(name = "Group 1", maxItems = Some(10)), + GroupsConfigJson(name = "Group 2", maxItems = Some(10)) + )), uneditable = None, showTags = None, showSections = None,