Fixes literal 'None' encoding of some empty Crop fields.

Treat all fields the same way then explicitly filter out None fields to protect ourselves from any misunderstandings about how collect treats values of type Option.
guardian · Nov 29, 2024 · d294f9e · d294f9e
1 parent 14fa02b
commit d294f9e
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 3 deletions.
diff --git a/cropper/app/lib/CropSpecMetadata.scala b/cropper/app/lib/CropSpecMetadata.scala
@@ -17,9 +17,14 @@ trait CropSpecMetadata {
       "date" -> crop.date.map(printDateTime),
       "width" -> dimensions.width,
       "height" -> dimensions.height,
-    ) ++ r.map("aspect-ratio" -> _)
+      "aspect-ratio" -> r)
 
-    val flattenedMetadata = metadata.collect {
+    val nonEmptyMetadata = metadata.filter {
+      case (_, None) => false
+      case _ => true
+    }
+
+    val flattenedMetadata = nonEmptyMetadata.collect {
       case (key, Some(value)) => key -> value
       case (key, value) => key -> value
     }.view.mapValues(_.toString).toMap

diff --git a/cropper/test/lib/CropSpecMetadataTest.scala b/cropper/test/lib/CropSpecMetadataTest.scala
@@ -37,7 +37,7 @@ class CropSpecMetadataTest extends AnyFunSpec with Matchers with CropSpecMetadat
       val metadata = metadataForCrop(crop.copy(specification = withEmptyField, author = None), dimensions)
 
       metadata.get("aspect-ratio") shouldBe None
-      metadata.get("author") shouldBe Some("None")  // TODO this does not look intentional!
+      metadata.get("author") shouldBe None
     }
 
     it("should round trip metadata back to crop spec") {