From c393106ebb5f83a6d897a5b9f22fddff10f560bb Mon Sep 17 00:00:00 2001
From: Tony McCrae <tony@eelpieconsulting.co.uk>
Date: Sat, 31 Aug 2024 19:56:17 +0100
Subject: [PATCH] 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.
---
 cropper/app/lib/CropSpecMetadata.scala      | 11 ++++++++---
 cropper/test/lib/CropSpecMetadataTest.scala |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/cropper/app/lib/CropSpecMetadata.scala b/cropper/app/lib/CropSpecMetadata.scala
index 5c4b32dda6..f6875a5816 100644
--- a/cropper/app/lib/CropSpecMetadata.scala
+++ b/cropper/app/lib/CropSpecMetadata.scala
@@ -17,14 +17,19 @@ trait CropSpecMetadata {
       "date" -> crop.date.map(printDateTime),
       "width" -> dimensions.width,
       "height" -> dimensions.height,
-    ) ++ r.map("aspect-ratio" -> _)
+      "aspect-ratio" -> r)
 
-    val nonEmptyMetadata = 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
 
-    nonEmptyMetadata
+    flattenedMetadata
   }
 
   def cropSpecFromMetadata(userMetadata: Map[String, String]): Option[CropSpec] = {
diff --git a/cropper/test/lib/CropSpecMetadataTest.scala b/cropper/test/lib/CropSpecMetadataTest.scala
index 421bc31441..bfd3c8c8d1 100644
--- 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") {