Fix image override case: Change behaviour so fronts tool decides replacement image

[ajwl]

The problem

A rare problem came up on the UK front page where the imageReplace was not being shown as the thumbnail for a story. The default CAPI image was being used instead.

Features of the story: -> Opinion/comment piece -> It had no cutout image supplied or available from TagManager -> An imageReplace had been applied in the fronts tool overriding the default image from CAPI.

NB: The standard format for an opinion piece on fronts is to use a cutout image / quote headline / show byline.

Two opinion pieces on a front - 1st with cutout, 2nd with standard image

Other aspects of the problem -> If a cutout image was added, even if it was not used, it solved the problem and image replace worked correctly. This is the normal case, as the vast majority of opinion pieces have a cutout available even if not used -> Fronts tool was correctly sending the data with the imageReplace property set to true and the new image supplied in the url. -> Frontend was correctly rendering what it received from facia-press. -> Facia-press was correctly pressing the uk front. -> The same happens in v1, so this problem has obviously been around for a while, but only turns up infrequently because it applies to a small section of stories.

Suggested fix

From how I see it, the fronts tool should take the decision as to whether a cutout is used, rather than this being hardcoded as the default by this library.

Therefore I am removing line 62 that hardcodes that for opinion/comment pieces. That should be set by line 82 taking it from the fronts tool trailMeta instead.

This cedes control for setting the replaceImage and the cutoutImageReplace to the fronts tool metadata rather than guessing that an Opinion story will definitely have a cutout image.

Setting it to true by default meant that it failed this if statement in facia press:

val (maybeSrc, maybeWidth, maybeHeight) = image match {
      case cutout: Cutout => (Some(cutout.imageSrc), cutout.imageSrcHeight, cutout.imageSrcWidth)
      case replace: Replace => (Some(replace.imageSrc), Some(replace.imageSrcHeight), Some(replace.imageSrcWidth))
      case _ => (None, None, None)
    }

Json produced by fronts

This image override worked because a cutout was present.

"meta": {
"imageSrcWidth": "2625",
"headline": "It will wreck it: Boris Johnson’s electoral gamble risks wrecking the Tory party",
"imageSrcOrigin": "https://media.test.dev-gutools.co.uk/images/94b46e4078134a5b04a868c4054658a33348d5ec",
"imageSrcHeight": "1575",
"imageCutoutSrc": "https://uploads.guim.co.uk/2017/10/06/Jonathan-Freedland,-L.png",
"showByline": true,
"showQuotedHeadline": true,
"imageSrcThumb": "https://s3-eu-west-1.amazonaws.com/media-origin.test.dev-guim.co.uk/94b46e4078134a5b04a868c4054658a33348d5ec/0_104_2625_1575/500.jpg",
"imageSrc": "https://s3-eu-west-1.amazonaws.com/media-origin.test.dev-guim.co.uk/94b46e4078134a5b04a868c4054658a33348d5ec/0_104_2625_1575/master/2625.jpg",
"imageReplace": true,
"imageCutoutReplace": false,
"group": "1"
}

This one didn't because a cutout was not present:

"meta": {
"imageSrcWidth": "2595",
"headline": "It fell: Boris Johnson's blustering strategy has fallen at the first hurdle",
"imageSrcOrigin": "https://media.test.dev-gutools.co.uk/images/6d3a83433a65f38231d8455929ae5c6e47f55883",
"imageSrcHeight": "1558",
"showByline": true,
"showQuotedHeadline": true,
"imageSrcThumb": "https://s3-eu-west-1.amazonaws.com/media-origin.test.dev-guim.co.uk/6d3a83433a65f38231d8455929ae5c6e47f55883/0_87_2595_1558/500.jpg",
"imageSrc": "https://s3-eu-west-1.amazonaws.com/media-origin.test.dev-guim.co.uk/6d3a83433a65f38231d8455929ae5c6e47f55883/0_87_2595_1558/master/2595.jpg",
"imageReplace": true,
"group": "1"
}

Other issues with this library

Variations of this problem have come up before. cf this trello card - https://trello.com/c/nwUyy7Lv/67-content-based-settings-do-not-get-saved-into-database-when-creating-skeleton @aug24 @akash1810

[ajwl]

Have added a 2nd commit which is a more drastic removal of these default styling options. Including defaults setting mainVideo and showByline

[ajwl]

Bumping this as the problem has come up again

[jonathonherbert]

@akash1810 @ajwl this code looks fine, but if we're removing this logic here, I feel like we should take care to make sure it exists elsewhere. For example, AFAIK isCartoonForContent isn't handled in the Fronts tool, and as a result when editors add cartoons they may find that the output on platform isn't what they expect (no byline).

Given that this is causing problems for editors now, would it be safer to remove logic piece by piece, at least in this case, to make sure we don't cause more problems than we solve?

[ajwl]

@jonathonherbert yeah sensible... I can roll back the 2nd commit tomorrow and just fix the bug in hand

[ajwl]

@jonathonherbert @akash1810 - have reduced this PR to the original small scope fix. Lmk if you are happy

[ajwl]

thanks!