From 7795c33f671bce29be5340477e60a1bbeee79e84 Mon Sep 17 00:00:00 2001
From: Tom Richards <tom.richards@guardian.co.uk>
Date: Tue, 1 Oct 2024 14:12:39 +0100
Subject: [PATCH] =?UTF-8?q?WIP=20add=20`points=20of=20interest=20=E1=B4=AE?=
 =?UTF-8?q?=E1=B4=B1=E1=B5=80=E1=B4=AC`=20crop=20type?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Frederick O'Brien <frederick.obrien@guardian.co.uk>
---
 .../com/gu/mediaservice/model/Crop.scala      | 28 ++++++++++++++++---
 .../app/controllers/CropperController.scala   |  5 ++--
 cropper/app/lib/Crops.scala                   | 10 +++----
 cropper/app/model/ExportRequest.scala         | 10 +++++++
 cropper/test/lib/CropsTest.scala              | 10 +++----
 kahuna/public/js/crop/controller.js           |  6 ++--
 kahuna/public/js/crop/view.html               |  6 ++++
 kahuna/public/js/image/controller.js          |  2 +-
 .../public/js/services/api/media-cropper.js   |  4 +--
 .../public/js/util/constants/cropOptions.js   |  3 +-
 10 files changed, 62 insertions(+), 22 deletions(-)

diff --git a/common-lib/src/main/scala/com/gu/mediaservice/model/Crop.scala b/common-lib/src/main/scala/com/gu/mediaservice/model/Crop.scala
index 879ddd4c17..9aad174283 100644
--- a/common-lib/src/main/scala/com/gu/mediaservice/model/Crop.scala
+++ b/common-lib/src/main/scala/com/gu/mediaservice/model/Crop.scala
@@ -12,10 +12,28 @@ case class Crop(id: Option[String], author: Option[String], date: Option[DateTim
 object Crop {
   import com.gu.mediaservice.lib.formatting._
 
-  def getCropId(b: Bounds) = List(b.x, b.y, b.width, b.height).mkString("_")
-
-  def createFromCropSource(by: Option[String], timeRequested: Option[DateTime], specification: CropSpec, master: Option[Asset] = None, cropSizings: List[Asset] = Nil): Crop =
-    Crop(Some(getCropId(specification.bounds)), by, timeRequested, specification, master, cropSizings)
+  private def getCropId(b: Bounds): String = List(b.x, b.y, b.width, b.height).mkString("_")
+  private def getPoiCropId(b: Bounds, fullDimensions: Dimensions): String = s"${getCropId(b)}_${fullDimensions.width}_${fullDimensions.height}"
+
+  def createFromCropSource(by: Option[String], timeRequested: Option[DateTime], specification: CropSpec, master: Option[Asset] = None, cropSizings: List[Asset] = Nil, maybeFullDimensions: Option[Dimensions] = None): Crop =
+    (specification.`type`, maybeFullDimensions) match {
+      case (PointsOfInterestExport, Some(fullDimensions)) => Crop(
+        id = Some(getPoiCropId(specification.bounds, fullDimensions)),
+        author = by,
+        date = timeRequested,
+        specification = specification.copy(bounds = Bounds(0, 0, fullDimensions.width, fullDimensions.height)),
+        master = master,
+        assets = cropSizings
+      )
+      case _ => Crop(
+        id = Some(getCropId(specification.bounds)),
+        author = by,
+        date = timeRequested,
+        specification = specification,
+        master = master,
+        assets = cropSizings
+      )
+    }
 
   def createFromCrop(crop: Crop, master: Asset, assets: List[Asset]): Crop =
     Crop(crop.id, crop.author, crop.date, crop.specification, Some(master), assets)
@@ -43,6 +61,7 @@ object Crop {
 sealed trait ExportType { val name: String }
 case object CropExport extends ExportType { val name = "crop" }
 case object FullExport extends ExportType { val name = "full" }
+case object PointsOfInterestExport extends ExportType { val name = "poi" }
 
 object ExportType {
 
@@ -51,6 +70,7 @@ object ExportType {
   def valueOf(name: String): ExportType = name match {
     case "crop" => CropExport
     case "full" => FullExport
+    case "poi" => PointsOfInterestExport
   }
 
   implicit val exportTypeWrites: Writes[ExportType] = Writes[ExportType](t => JsString(t.name))
diff --git a/cropper/app/controllers/CropperController.scala b/cropper/app/controllers/CropperController.scala
index 2830948f3c..6951147140 100644
--- a/cropper/app/controllers/CropperController.scala
+++ b/cropper/app/controllers/CropperController.scala
@@ -173,9 +173,10 @@ class CropperController(auth: Authentication, crops: Crops, store: CropStore, no
       crop = Crop.createFromCropSource(
         by = Some(Authentication.getIdentity(user)),
         timeRequested = Some(new DateTime()),
-        specification = cropSpec
+        specification = cropSpec,
+        maybeFullDimensions = Some(dimensions)
       )
-      markersWithCropDetails = logMarker ++ Map("imageId" -> apiImage.id, "cropId" -> Crop.getCropId(cropSpec.bounds))
+      markersWithCropDetails = logMarker ++ Map("imageId" -> apiImage.id, "cropId" -> crop.id)
       ExportResult(id, masterSizing, sizings) <- crops.export(apiImage, crop)(markersWithCropDetails)
       finalCrop = Crop.createFromCrop(crop, masterSizing, sizings)
     } yield (id, finalCrop)
diff --git a/cropper/app/lib/Crops.scala b/cropper/app/lib/Crops.scala
index d79dcb4a94..529c4df946 100644
--- a/cropper/app/lib/Crops.scala
+++ b/cropper/app/lib/Crops.scala
@@ -23,9 +23,9 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
   private val cropQuality = 75d
   private val masterCropQuality = 95d
 
-  def outputFilename(source: SourceImage, bounds: Bounds, outputWidth: Int, fileType: MimeType, isMaster: Boolean = false): String = {
+  def outputFilename(source: SourceImage, cropId: String, outputWidth: Int, fileType: MimeType, isMaster: Boolean = false): String = {
     val masterString: String = if (isMaster) "master/" else ""
-    s"${source.id}/${Crop.getCropId(bounds)}/${masterString}$outputWidth${fileType.fileExtension}"
+    s"${source.id}/$cropId/$masterString$outputWidth${fileType.fileExtension}"
   }
 
   def createMasterCrop(
@@ -49,7 +49,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
       )
       file: File <- imageOperations.appendMetadata(strip, metadata)
       dimensions  = Dimensions(source.bounds.width, source.bounds.height)
-      filename    = outputFilename(apiImage, source.bounds, dimensions.width, mediaType, isMaster = true)
+      filename    = outputFilename(apiImage, crop.id.get, dimensions.width, mediaType, isMaster = true)
       sizing      = store.storeCropSizing(file, filename, mediaType, crop, dimensions)
       dirtyAspect = source.bounds.width.toFloat / source.bounds.height
       aspect      = crop.specification.aspectRatio.flatMap(AspectRatio.clean).getOrElse(dirtyAspect)
@@ -64,7 +64,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
       for {
         file          <- imageOperations.resizeImage(sourceFile, apiImage.source.mimeType, dimensions, cropQuality, config.tempDir, cropType)
         optimisedFile = imageOperations.optimiseImage(file, cropType)
-        filename      = outputFilename(apiImage, crop.specification.bounds, dimensions.width, cropType)
+        filename      = outputFilename(apiImage, crop.id.get, dimensions.width, cropType)
         sizing        <- store.storeCropSizing(optimisedFile, filename, cropType, crop, dimensions)
         _             <- delete(file)
         _             <- delete(optimisedFile)
@@ -97,7 +97,7 @@ class Crops(config: CropperConfig, store: CropStore, imageOperations: ImageOpera
     val hasAlpha = apiImage.fileMetadata.colourModelInformation.get("hasAlpha").flatMap(a => Try(a.toBoolean).toOption).getOrElse(true)
     val cropType = Crops.cropType(mimeType, colourType, hasAlpha)
 
-    Stopwatch(s"making crop assets for ${apiImage.id} ${Crop.getCropId(source.bounds)}") {
+    Stopwatch(s"making crop assets for ${apiImage.id} ${crop.id}") {
       for {
         sourceFile <- tempFileFromURL(secureUrl, "cropSource", "", config.tempDir)
         colourModel <- ImageOperations.identifyColourModel(sourceFile, mimeType)
diff --git a/cropper/app/model/ExportRequest.scala b/cropper/app/model/ExportRequest.scala
index f99bc6c019..88b015ac9f 100644
--- a/cropper/app/model/ExportRequest.scala
+++ b/cropper/app/model/ExportRequest.scala
@@ -16,6 +16,8 @@ case class FullExportRequest(uri: String) extends ExportRequest
 
 case class CropRequest(uri: String, bounds: Bounds, aspectRatio: Option[String]) extends ExportRequest
 
+case class PointsOfInterest(uri: String, bounds: Bounds) extends ExportRequest
+
 
 object ExportRequest {
 
@@ -27,12 +29,18 @@ object ExportRequest {
     (__ \ "aspectRatio").readNullable[String](pattern(aspectRatioLike))
   )(CropRequest.apply _)
 
+  private val readPointsOfInterestRequest: Reads[PointsOfInterest] = (
+    (__ \ "source").read[String] ~
+    __.read[Bounds]
+  )(PointsOfInterest.apply _)
+
   private val readFullExportRequest: Reads[FullExportRequest] =
     (__ \ "source").read[String].map(FullExportRequest.apply)
 
   implicit val readExportRequest: Reads[ExportRequest] = Reads[ExportRequest](jsValue =>
     (jsValue \ "type").validate[String] match {
       case JsSuccess("crop", _) => readCropRequest.reads(jsValue)
+      case JsSuccess("poi", _) => readPointsOfInterestRequest.reads(jsValue)
       case JsSuccess("full", _) => readFullExportRequest.reads(jsValue)
       case _ => JsError("invalid type")
     }
@@ -41,6 +49,8 @@ object ExportRequest {
   def boundsFill(dimensions: Dimensions): Bounds = Bounds(0, 0, dimensions.width, dimensions.height)
 
   def toCropSpec(cropRequest: ExportRequest, dimensions: Dimensions): CropSpec = cropRequest match {
+    case PointsOfInterest(uri, bounds) =>
+      CropSpec(uri, bounds, None, PointsOfInterestExport)
     case FullExportRequest(uri)          =>
       CropSpec(
         uri,
diff --git a/cropper/test/lib/CropsTest.scala b/cropper/test/lib/CropsTest.scala
index a0d0ba8a62..961a4c7866 100644
--- a/cropper/test/lib/CropsTest.scala
+++ b/cropper/test/lib/CropsTest.scala
@@ -46,27 +46,27 @@ class CropsTest extends AnyFunSpec with Matchers with MockitoSugar {
   private val store = mock[CropStore]
   private val imageOperations: ImageOperations = mock[ImageOperations]
   private val source: SourceImage = SourceImage("test", mock[Asset], valid = true, mock[ImageMetadata], mock[FileMetadata])
-  private val bounds: Bounds = Bounds(10, 20, 30, 40)
+  private val cropId: String = "10_20_30_40"
   private val outputWidth = 1234
 
   it("should should construct a correct address for a master jpg") {
     val outputFilename = new Crops(config, store, imageOperations)
-      .outputFilename(source, bounds, outputWidth, Jpeg, isMaster = true)
+      .outputFilename(source, cropId, outputWidth, Jpeg, isMaster = true)
     outputFilename shouldBe "test/10_20_30_40/master/1234.jpg"
   }
   it("should should construct a correct address for a non-master jpg") {
     val outputFilename = new Crops(config, store, imageOperations)
-      .outputFilename(source, bounds, outputWidth, Jpeg)
+      .outputFilename(source, cropId, outputWidth, Jpeg)
     outputFilename shouldBe "test/10_20_30_40/1234.jpg"
   }
   it("should should construct a correct address for a non-master tiff") {
     val outputFilename = new Crops(config, store, imageOperations)
-      .outputFilename(source, bounds, outputWidth, Tiff)
+      .outputFilename(source, cropId, outputWidth, Tiff)
     outputFilename shouldBe "test/10_20_30_40/1234.tiff"
   }
   it("should should construct a correct address for a non-master png") {
     val outputFilename = new Crops(config, store, imageOperations)
-      .outputFilename(source, bounds, outputWidth, Png)
+      .outputFilename(source, cropId, outputWidth, Png)
     outputFilename shouldBe "test/10_20_30_40/1234.png"
   }
 }
diff --git a/kahuna/public/js/crop/controller.js b/kahuna/public/js/crop/controller.js
index 8c939342e6..d93737c5d0 100644
--- a/kahuna/public/js/crop/controller.js
+++ b/kahuna/public/js/crop/controller.js
@@ -3,7 +3,7 @@ import angular from 'angular';
 import '../components/gr-keyboard-shortcut/gr-keyboard-shortcut';
 import {radioList} from '../components/gr-radio-list/gr-radio-list';
 import {cropUtil} from "../util/crop";
-import {cropOptions} from "../util/constants/cropOptions";
+import {cropOptions, pointsOfInterestBeta} from "../util/constants/cropOptions";
 import {getFeatureSwitchActive} from "../components/gr-feature-switch-panel/gr-feature-switch-panel";
 
 const crop = angular.module('kahuna.crop.controller', [
@@ -150,7 +150,7 @@ crop.controller('ImageCropCtrl', [
 
         ctrl.cropping = true;
 
-        mediaCropper.createCrop(ctrl.image, coords, ratioString)
+        mediaCropper.createCrop(ctrl.image, coords, ratioString, ctrl.cropType === pointsOfInterestBeta.key ? 'poi' : 'crop')
         .then(crop => {
            // Global notification of action
            $rootScope.$emit('events:crop-created', {
@@ -187,6 +187,8 @@ crop.controller('ImageCropCtrl', [
           && getFeatureSwitchActive("show-cropping-gutters-switch")
           && maybeCropRatioIfStandard === "5:3";
 
+        ctrl.isPointsOfInterestCrop = newCropType === pointsOfInterestBeta.key; /* TODO adjust the height of easel to avoid scrollbar from explainer */
+
         if (isCropTypeDisabled) {
           ctrl.cropType = oldCropType;
         } else {
diff --git a/kahuna/public/js/crop/view.html b/kahuna/public/js/crop/view.html
index 5a98a5d46f..920533337a 100644
--- a/kahuna/public/js/crop/view.html
+++ b/kahuna/public/js/crop/view.html
@@ -106,6 +106,12 @@
     there is nothing important in the striped sides as these might be clipped.
 </div>
 
+
+<div class="warning status--info" ng-if="ctrl.isPointsOfInterestCrop">
+    This is a 'Points of Interest ᴮᴱᵀᴬ` crop. Use the crop rectangle to cover all the vital points of interest, but nothing more.
+    The frontend can use the crop in spaces of different shapes/ratios ensuring the important parts are always visible.
+</div>
+
 <div class="easel" role="main" aria-label="Image cropper"
      ng-class="{'vertical-warning-gutters': ctrl.shouldShowVerticalWarningGutters}">
     <div class="easel__canvas">
diff --git a/kahuna/public/js/image/controller.js b/kahuna/public/js/image/controller.js
index 47a8ecbb16..dc62462903 100644
--- a/kahuna/public/js/image/controller.js
+++ b/kahuna/public/js/image/controller.js
@@ -297,7 +297,7 @@ image.controller('ImageCtrl', [
       }
       ctrl.crop = crops.find(crop => crop.id === cropKey);
       ctrl.fullCrop = crops.find(crop => crop.specification.type === 'full');
-      ctrl.crops = crops.filter(crop => crop.specification.type === 'crop');
+      ctrl.crops = crops.filter(crop => crop.specification.type === 'crop' || 'poi');
       ctrl.image.allCrops = ctrl.fullCrop ? [ctrl.fullCrop].concat(ctrl.crops) : ctrl.crops;
       //boolean version for use in template
       ctrl.hasFullCrop = angular.isDefined(ctrl.fullCrop);
diff --git a/kahuna/public/js/services/api/media-cropper.js b/kahuna/public/js/services/api/media-cropper.js
index d49cfe1fd4..c78d9bdba7 100644
--- a/kahuna/public/js/services/api/media-cropper.js
+++ b/kahuna/public/js/services/api/media-cropper.js
@@ -16,9 +16,9 @@ cropperApi.factory('mediaCropper', ['mediaApi', function(mediaApi) {
         return cropperRoot;
     }
 
-    function createCrop(image, coords, ratio) {
+    function createCrop(image, coords, ratio, type = 'crop') {
         return getCropperRoot().follow('crop').post({
-            type: 'crop',
+            type,
             source: image.uri,
             x: coords.x,
             y: coords.y,
diff --git a/kahuna/public/js/util/constants/cropOptions.js b/kahuna/public/js/util/constants/cropOptions.js
index 017ec87522..90bb419c0e 100644
--- a/kahuna/public/js/util/constants/cropOptions.js
+++ b/kahuna/public/js/util/constants/cropOptions.js
@@ -4,5 +4,6 @@ export const portrait = {key: 'portrait', ratio: 4 / 5, ratioString: '4:5'};
 export const video = {key: 'video', ratio: 16 / 9, ratioString: '16:9'};
 export const square = {key: 'square', ratio: 1, ratioString: '1:1'};
 export const freeform = {key: 'freeform', ratio: null};
+export const pointsOfInterestBeta = {key: 'points of interest ᴮᴱᵀᴬ', ratio:null};
 
-export const cropOptions = [landscape, portrait, video, square, freeform];
+export const cropOptions = [landscape, portrait, video, square, freeform, pointsOfInterestBeta];