Enable limiting the number of stories in flexible general containers

[Georges-GNM]

What's changed?

Part of this fairground ticket

This allows editorial to set the number of stories in a flexible general container, by making use of an existing value, maxItemsToDisplay, nested in the DisplayHints section of a collection's config.

The input has a placeholder of 9, which will also be a default/fallback number used throughout the codebases, to replicate existing behaviour. At the most there can be up to 20 stories, at the least there can be a single one.

Aside from adding the input, this PR uses the value to adjust the dividing line separation in the fronts tool (see screenshot); this is currently used by editorial as an indicator of which stories will appear - those below the line are effectively queued up, and so not intended to be displayed.

As the amount of stories actually displayed on the platforms is determined in their respective codebases, follow up work is in process in frontend/mapi so they can also pick up on the desired value and ensure the right amount of stories gets displayed.

maxItemsToDisplay usage note

As mentioned this is an existing value, but it is currently only used for email fronts (which don't have access to flexible general containers), and has a similar intention to limit the number of visible stories. I would normally feel uneasy about piggybacking like this, but it feels like there's enough similarities while also having sufficient separation between the two use cases that this can be a working solution.

This wasn't my first choice though, I intended to add a separate field (through this facia scala client PR) but implementing this proved to be non trivial, hence this approach.

Fronts tool (v2) implementation note

The dividing lines are getting adjusted depending on the maxItemsToDisplay value, which for the most part seems to be working as intended, but the behaviour can be temperamental when manipulating splash cards (e.g. adding one or more, or moving a splash card to the list of standard stories, which seems to shift the dividing line up or down rather than keeping it at the desired number of stories).

There's a separate ticket to limit the number of splash cards to one story, which feels best to tackle first and separately. I would expect the undesired behaviours to be resolved as part of that PR (or possibly a follow up).

Screenshots

Config tool:

Fronts tool - showing the dividing line appearing below the 5th standard story (with 1 splash)

How to test

In the config tool, on a front, create a new flexible general container. You should see the field appear with the placeholder 9, and if you save the container, the tool should display a dividing line at 9 standard stories (you may need to refresh the tool once or twice). If you then edit the field in the config tool to a lower value, the dividing line should go up to the relevant spot.

If these frontend and DCR PRs are also deployed to code, you can check that the value is getting correctly used and updated by the web platform.

General

• 🤖 Relevant tests added
• ✅ CI checks / tests run locally
• 🔍 Checked on CODE

Client

• 🚫 No obvious console errors on the client (i.e. React dev mode errors)
• 🎛️ No regressions with existing user interactions (i.e. all existing buttons, inputs etc. work)
• 📷 Screenshots / GIFs of relevant UI changes included

[github-actions]

Deploy build 8789 of cms-fronts::facia-tool to CODE

All deployment options

• Deploy build 8789 of cms-fronts::facia-tool to CODE
• Deploy parts of build 8789 to CODE by previewing it first
• What's on CODE right now?

---

From guardian/actions-riff-raff.

[davidfurey]

I think using maxItemsToDisplay is reasonable, although slightly unfortunate that it is nolonger the actual max but in fact the max standard. Still, I don't think it justifies a new field for now.

I had no memory of adding this field 9 years ago. I'm slightly surprised that no further display hints have been added.

[davidfurey]

If this is a valid situations where collectionConfigJson is not provided can you make the field optional and pass None instead of null?

[davidfurey]

This will throw an exception of the collection is not found, please refactor to handle the failure case and avoid the .get

[davidfurey]

Why is the type annotation required here?

[davidfurey]

I think it would be easier to read as numOfSplash + min(numOfStandard, standardStoryLimit)

[davidfurey]

This will throw an exception if the collection is not present, and is exactly equivalent to the config.collections.get(collectionId).get you had before.

Instead, you could do val cConfigJson = config.collections.get(collectionId) and drop the Some(...) that you have wrapped cConfigJson in below.

[Georges-GNM]

I do prefer the look of that suggestion, but just to check my understanding - wouldn't this code only get executed after line 102, which should guarantee the collection is present at that point?

[davidfurey]

That is an excellent point. I missed that you already know that the collection exists. But much like me, the complier isn't clever enough to detect that your actually accessing it in a safe place way.

Looking at this again, you could refactor:

    for {
      cConfigJson <- config.collections.get(collectionId)
      cType <- cConfigJson.`type`
    } yield {
        StoriesVisibleByStage(
          containerService
            .getStoriesVisible(cType, stages._1, Some(cConfigJson)),
          containerService.getStoriesVisible(
            cType,
            stages._2,
            Some(cConfigJson)
          )
        )
    }

[Georges-GNM]

Thanks, that does look a bit more concise and seems to work!

[Georges-GNM]

I think using maxItemsToDisplay is reasonable, although slightly unfortunate that it is nolonger the actual max but in fact the max standard.

Yeah, I hesitated about that, and thought that since the number of splash stories was already limited, that really it was the number of standard stories we wanted to affect.

However, from what I can tell the platforms, when they determine how many stories to show, don't easily have a way to access information about splash and standard stories, which we would need in order to get the right number of stories to come up (in frontend for example, the proposed implementation looks like this), so in the end I think the best way is for the value to indeed reflect the actual max. Just in the process of revisiting this and testing it on code.

[davidfurey]

Great work!

[prout-bot]

Seen on PROD (merged by @Georges-GNM 7 minutes and 39 seconds ago) Please check your changes!