Skip to content

Commit

Permalink
Refactor visibleArticles to support lists
Browse files Browse the repository at this point in the history
  • Loading branch information
Fweddi committed Feb 25, 2025
1 parent ca2d915 commit 973f811
Show file tree
Hide file tree
Showing 11 changed files with 95 additions and 69 deletions.
70 changes: 42 additions & 28 deletions app/services/ContainerService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,19 @@ import play.api.libs.json.{Json, OFormat}
import slices._
import com.gu.facia.client.models.CollectionConfigJson

case class StoriesVisibleResponse(
case class StoriesVisible(
desktop: Option[Int],
mobile: Option[Int]
mobile: Option[Int],
groupName: Option[String] = None
)

object StoriesVisible {
implicit val jsonFormat: OFormat[StoriesVisible] =
Json.format[StoriesVisible]
}

case class StoriesVisibleResponse(
response: List[StoriesVisible]
)

object StoriesVisibleResponse {
Expand All @@ -19,60 +29,64 @@ class ContainerService(val containers: Containers) {
containerType: String,
stories: Seq[Story],
collectionConfigJson: Option[CollectionConfigJson]
) = {
): Option[StoriesVisibleResponse] = {
val numberOfStories = stories.length
containers.all.get(containerType) map {
case Fixed(container) =>
val maxDesktop = container.numItems
val desktopVisible = maxDesktop min numberOfStories

StoriesVisibleResponse(
Some(desktopVisible),
container.mobileShowMore match {
case DesktopBehaviour => Some(desktopVisible)
case RestrictTo(maxMobile) if maxMobile > desktopVisible =>
Some(desktopVisible)
case RestrictTo(maxMobile) => Some(maxMobile min numberOfStories)
}
)

List(StoriesVisible(
Some(desktopVisible),
container.mobileShowMore match {
case DesktopBehaviour => Some(desktopVisible)
case RestrictTo(maxMobile) if maxMobile > desktopVisible =>
Some(desktopVisible)
case RestrictTo(maxMobile) => Some(maxMobile min numberOfStories)
}
)))
case Scrollable(container) =>
val numberVisible = container.storiesVisible(stories)
StoriesVisibleResponse(
Some(numberVisible),
Some(numberVisible)
)
List(StoriesVisible(
Some(numberVisible),
Some(numberVisible)
)))

case Dynamic(container) =>
val slices = container.slicesFor(stories)
val maxItems = slices.map(_.map(_.layout.numItems).sum).getOrElse(0)
val numberVisible = maxItems min numberOfStories
StoriesVisibleResponse(
Some(numberVisible),
Some(numberVisible)
)
List(StoriesVisible(
Some(numberVisible),
Some(numberVisible)
)))

case Flexible(container) =>
val numberVisible = container.storiesVisible(
stories,
collectionConfigJson
)
StoriesVisibleResponse(
Some(numberVisible),
Some(numberVisible)
)
List(StoriesVisible(
Some(numberVisible),
Some(numberVisible)
)))

case MostPopular =>
StoriesVisibleResponse(
Some(10 min numberOfStories),
Some(10 min numberOfStories)
)
List(StoriesVisible(
Some(10 min numberOfStories),
Some(10 min numberOfStories)
)))

case NavList | NavMediaList =>
StoriesVisibleResponse(
None,
None
)
List(StoriesVisible(
None,
None
)))
}
}
}
12 changes: 8 additions & 4 deletions app/slices/FlexibleContainer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@ object FlexibleGeneral extends FlexibleContainer {
stories: Seq[Story],
collectionConfigJson: Option[CollectionConfigJson]
): Int = {
collectionConfigJson
.flatMap(_.displayHints)
.flatMap(_.maxItemsToDisplay)
.getOrElse(9)
val maxItems = for {
configJson <- collectionConfigJson
groupsConfig <- configJson.groupsConfig
// TODO: this should be per group
group <- groupsConfig.find(groupConfig => groupConfig.head)
maxItems <- group.maxItems
} yield maxItems
maxItems.getOrElse(9)
}
}

Expand Down
16 changes: 8 additions & 8 deletions fronts-client/integration/server/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,24 +126,24 @@ module.exports = async () =>
id: req.body[0].id,
collection: { ...collection, id: req.body[0].id },
storiesVisibleByStage: {
live: { desktop: 4, mobile: 4 },
draft: { desktop: 4, mobile: 4 }
live: { response: [{ desktop: 4, mobile: 4 }] },
draft: { response: [{desktop: 4, mobile: 4 }] }
}
},
{
id: req.body[1].id,
collection: { ...collectionTwo, id: req.body[1].id },
storiesVisibleByStage: {
live: { desktop: 4, mobile: 4 },
draft: { desktop: 4, mobile: 4 }
live: { response: [{ desktop: 4, mobile: 4 }] },
draft: { response: [{desktop: 4, mobile: 4 }] }
}
},
{
id: req.body[2].id,
collection: { ...collectionFour, id: req.body[2].id },
storiesVisibleByStage: {
live: { desktop: 4, mobile: 4 },
draft: { desktop: 4, mobile: 4 }
live: { response: [{ desktop: 4, mobile: 4 }] },
draft: { response: [{desktop: 4, mobile: 4 }] }
}
}
]);
Expand Down Expand Up @@ -185,8 +185,8 @@ module.exports = async () =>
// Collection three is empty, simulating a discard event
collection: { ...collectionThree, id: req.body.collectionId },
storiesVisibleByStage: {
live: { desktop: 4, mobile: 4 },
draft: { desktop: 4, mobile: 4 }
live: { response: [{ desktop: 4, mobile: 4 }] },
draft: { response: [{desktop: 4, mobile: 4 }] }
}
});
});
Expand Down
4 changes: 2 additions & 2 deletions fronts-client/src/actions/Fronts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ function recordStaleFronts(frontId: string, frontIsStale: boolean): Action {

function recordVisibleArticles(
collectionId: string,
visibleArticles: VisibleArticlesResponse,
visibleArticlesResponse: VisibleArticlesResponse,
stage: Stages,
): Action {
return {
type: 'FETCH_VISIBLE_ARTICLES_SUCCESS',
payload: {
collectionId,
visibleArticles,
visibleArticlesResponse,
stage,
},
};
Expand Down
10 changes: 5 additions & 5 deletions fronts-client/src/actions/__tests__/Collections.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('Collection actions', () => {
type: 'FETCH_VISIBLE_ARTICLES_SUCCESS',
payload: {
collectionId: 'exampleCollection',
visibleArticles: {},
visibleArticlesResponse: {},
stage: 'draft',
},
});
Expand Down Expand Up @@ -219,7 +219,7 @@ describe('Collection actions', () => {
platform: undefined,
previously: ['uuid'],
previouslyCardIds: [],
groups: undefined,
groupsConfig: undefined,
frontsToolSettings: undefined,
targetedRegions: [],
excludedRegions: [],
Expand All @@ -229,7 +229,7 @@ describe('Collection actions', () => {
displayName: 'testCollection',
draft: ['uuid'],
frontsToolSettings: undefined,
groups: undefined,
groupsConfig: undefined,
id: 'testCollection1',
lastUpdated: 1547479667115,
live: ['uuid'],
Expand All @@ -243,7 +243,7 @@ describe('Collection actions', () => {
displayName: 'testCollection',
draft: ['uuid'],
frontsToolSettings: undefined,
groups: undefined,
groupsConfig: undefined,
id: 'testCollection2',
lastUpdated: 1547479667115,
live: ['uuid'],
Expand All @@ -257,7 +257,7 @@ describe('Collection actions', () => {
displayName: 'New Zealand News',
draft: ['uuid'],
frontsToolSettings: undefined,
groups: undefined,
groupsConfig: undefined,
id: 'geoLocatedCollection',
lastUpdated: 1547479667115,
live: ['uuid'],
Expand Down
8 changes: 4 additions & 4 deletions fronts-client/src/components/FrontsEdit/Collection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,10 @@ const createMapStateToProps = () => {
collectionSet: props.browsingStage,
});

return {
lastDesktopArticle: articleVisibilityDetails.desktop,
lastMobileArticle: articleVisibilityDetails.mobile,
};
return articleVisibilityDetails.map(_ => ({
lastDesktopArticle: _.desktop,
lastMobileArticle: _.mobile,
}));
};
};

Expand Down
13 changes: 7 additions & 6 deletions fronts-client/src/reducers/frontsReducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,18 @@ const reducer = (
);
}
case 'FETCH_VISIBLE_ARTICLES_SUCCESS': {
const { collectionId, visibleArticles, stage } = action.payload;
const { collectionId, visibleArticlesResponse, stage } = action.payload;

const collectionVisibilities = state.collectionVisibility[stage];
const prevVisibilities = collectionVisibilities[collectionId] || {};
if (
visibleArticles.mobile === prevVisibilities.mobile &&
visibleArticles.desktop === visibleArticles.desktop
) {

if(visibleArticlesResponse.every((visibleArticles, index) => {
return visibleArticles.mobile === prevVisibilities[index].mobile
&& visibleArticles.desktop === prevVisibilities[index].desktop;
})) {
return newState;
}
const newCollectionVisibility = { [collectionId]: visibleArticles };
const newCollectionVisibility = { [collectionId]: visibleArticlesResponse };
const newVisibilities = {
...collectionVisibilities,
...newCollectionVisibility,
Expand Down
19 changes: 11 additions & 8 deletions fronts-client/src/selectors/frontsSelectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
FrontConfig,
CollectionConfig,
VisibleArticlesResponse,
VisibleArticles,
} from 'types/FaciaApi';
import type { State } from 'types/State';
import { AlsoOnDetail } from 'types/Collection';
Expand Down Expand Up @@ -350,10 +351,10 @@ const selectVisibleArticles = createSelector(
},
);

const defaultVisibleFrontArticles = {
const defaultVisibleFrontArticles = [{
desktop: undefined,
mobile: undefined,
};
}];

const createSelectArticleVisibilityDetails = () => {
const selectArticlesInCollection = createSelectCardsInCollection();
Expand All @@ -380,16 +381,18 @@ const createSelectArticleVisibilityDetails = () => {
if (collectionSet === 'previously') {
return defaultVisibleFrontArticles;
}
const visibilities = collectionVisibilities[collectionSet][collectionId];
const visibleArticlesResponse: VisibleArticlesResponse = collectionVisibilities[collectionSet][collectionId];

if (!visibilities) {
if (!visibleArticlesResponse) {
return defaultVisibleFrontArticles;
}

return {
desktop: articles[visibilities.desktop - 1],
mobile: articles[visibilities.mobile - 1],
};
return visibleArticlesResponse.map((visibleArticles: VisibleArticles) => {
return {
desktop: articles[visibleArticles.desktop - 1],
mobile: articles[visibleArticles.mobile - 1],
}
})
},
);
};
Expand Down
4 changes: 2 additions & 2 deletions fronts-client/src/strategies/fetch-collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export const editionCollectionToCollection = (
},
storiesVisibleByStage: {
// TODO - remove me once we figure out what to do here!
live: { mobile: 0, desktop: 0 },
draft: { mobile: 0, desktop: 0 },
live: [],
draft: [],
},
};
};
Expand Down
2 changes: 1 addition & 1 deletion fronts-client/src/types/Action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ interface FetchVisibleArticlesSuccess {
type: 'FETCH_VISIBLE_ARTICLES_SUCCESS';
payload: {
collectionId: string;
visibleArticles: VisibleArticlesResponse;
visibleArticlesResponse: VisibleArticlesResponse;
stage: Stages;
};
}
Expand Down
6 changes: 5 additions & 1 deletion fronts-client/src/types/FaciaApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,12 @@ interface EditionCollectionResponse {
collection: EditionCollectionFromResponse;
}

interface VisibleArticlesResponse {
type VisibleArticlesResponse = VisibleArticles[];

interface VisibleArticles {
desktop: number;
mobile: number;
groupName?: string
}

export {
Expand All @@ -143,6 +146,7 @@ export {
ArticleDetails,
CollectionResponse,
EditionCollectionResponse,
VisibleArticles,
VisibleArticlesResponse,
FrontsToolSettings,
DisplayHints,
Expand Down

0 comments on commit 973f811

Please sign in to comment.