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. + +