Skip to content

Commit

Permalink
Use GroupsConfig over Groups (WIP)
Browse files Browse the repository at this point in the history
  • Loading branch information
Fweddi committed Feb 21, 2025
1 parent 046c225 commit 9f359da
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 35 deletions.
4 changes: 2 additions & 2 deletions app/model/frontsapi.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
17 changes: 15 additions & 2 deletions fronts-client/src/fixtures/shared.ts
Original file line number Diff line number Diff line change
@@ -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',
Expand Down Expand Up @@ -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 = {
Expand Down
8 changes: 7 additions & 1 deletion fronts-client/src/types/Collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.*/
Expand Down Expand Up @@ -183,7 +188,7 @@ interface Collection {
updatedEmail?: string;
platform?: string;
displayName: string;
groups?: string[];
groupsConfig?: GroupConfig[];
metadata?: Array<{ type: string }>;
uneditable?: boolean;
type?: string;
Expand Down Expand Up @@ -213,6 +218,7 @@ export {
Collection,
CardSizes,
Group,
GroupConfig,
Stages,
CardSets,
ArticleTag,
Expand Down
4 changes: 2 additions & 2 deletions fronts-client/src/types/FaciaApi.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -43,7 +43,7 @@ interface CollectionConfigResponse {
type?: string;
backfill?: unknown;
href?: string;
groups?: string[];
groupsConfig?: GroupConfig[];
metadata?: unknown[];
uneditable?: boolean;
showTags?: boolean;
Expand Down
10 changes: 5 additions & 5 deletions fronts-client/src/util/__tests__/shared.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -168,7 +168,7 @@ describe('Shared utilities', () => {
};
const result = normaliseCollectionWithNestedArticles(collection, {
...configWithExtraGroup,
groups: undefined,
groupsConfig: undefined,
});
expect(Object.keys(result.groups)).toHaveLength(4);
});
Expand Down
2 changes: 1 addition & 1 deletion fronts-client/src/util/frontsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions fronts-client/src/util/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
});
}
Expand Down
9 changes: 3 additions & 6 deletions test/config/TransformationsSpec.scala
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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),
Expand Down Expand Up @@ -60,6 +56,7 @@ class TransformationsSpec extends FlatSpec with Matchers {
None,
None,
None,
None,
None
)

Expand Down
14 changes: 6 additions & 8 deletions test/services/CollectionServiceTest.scala
Original file line number Diff line number Diff line change
@@ -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}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 9f359da

Please sign in to comment.