Merge pull request #1308 from datacite/blank-alt-id-fix

Remove alternateIdentifiers from XML when alternateIdentifiers parameter is blank or nil
datacite · Jan 14, 2025 · 14d68b0 · 14d68b0
2 parents 6accc8f + 93ac254
commit 14d68b0
Show file tree

Hide file tree

Showing 2 changed files with 175 additions and 9 deletions.
diff --git a/app/controllers/datacite_dois_controller.rb b/app/controllers/datacite_dois_controller.rb
@@ -757,16 +757,23 @@ def safe_params
       # alternateIdentifiers as alias for identifiers
       # easier before strong_parameters are checked
       if params.dig(:data, :attributes).present? &&
-          params.dig(:data, :attributes, :identifiers).blank? &&
-          !params.dig(:data, :attributes, :alternateIdentifiers).blank?
+          !params.dig(:data, :attributes)&.key?(:identifiers) &&
+          params.dig(:data, :attributes)&.key?(:alternateIdentifiers)
+
+        alternate_identifiers = params.dig(:data, :attributes, :alternateIdentifiers)
+
         params[:data][:attributes][:identifiers] =
-          Array.wrap(params.dig(:data, :attributes, :alternateIdentifiers)).
-            map do |a|
-            {
-              identifier: a[:alternateIdentifier],
-              identifierType: a[:alternateIdentifierType],
-            }
-          end
+          alternate_identifiers.nil? ? nil :
+            Array.wrap(alternate_identifiers).map do |a|
+              if a.respond_to?(:fetch)
+                {
+                  identifier: a.fetch(:alternateIdentifier),
+                  identifierType: a.fetch(:alternateIdentifierType),
+                }
+              else
+                a
+              end
+            end
       end
 
       ParamsSanitizer.sanitize_nameIdentifiers(params[:creators])

diff --git a/spec/requests/datacite_dois/patch_spec.rb b/spec/requests/datacite_dois/patch_spec.rb
@@ -775,4 +775,163 @@
       expect(json.dig("data", "attributes", "subjects")).to eq([])
     end
   end
+
+  context "when a doi has alternateIdentifier/identifier values" do
+    let(:valid_attributes) do
+      {
+        "data" => {
+          "type" => "dois",
+          "attributes" => {
+            "alternateIdentifiers" => nil,
+          },
+        },
+      }
+    end
+
+    it "the xml and doi record contain the values" do
+      xml = Maremma.from_xml(doi.xml).fetch("resource", {})
+      expect(xml.dig("alternateIdentifiers")).not_to eq(nil)
+      expect(doi.identifiers).not_to eq(nil)
+    end
+
+    it "the values are removed when nil values are sent in json" do
+      patch "/dois/#{doi.doi}", valid_attributes, headers
+
+      xml = Maremma.from_xml(Base64.decode64(json.dig("data", "attributes", "xml"))).fetch("resource", {})
+      expect(xml.dig("alternateIdentifiers")).to eq(nil)
+      expect(json.dig("data", "attributes", "identifiers")).to eq([])
+      expect(json.dig("data", "attributes", "alternateIdentifiers")).to eq([])
+    end
+  end
+
+  context "when a doi has alternateIdentifier/identifier values" do
+    let(:valid_attributes) do
+      {
+        "data" => {
+          "type" => "dois",
+          "attributes" => {
+            "alternateIdentifiers" => [],
+          },
+        },
+      }
+    end
+
+    it "the xml and doi record contain the values" do
+      xml = Maremma.from_xml(doi.xml).fetch("resource", {})
+      expect(xml.dig("alternateIdentifiers")).not_to eq(nil)
+      expect(doi.identifiers).not_to eq(nil)
+    end
+
+    it "the values are removed when blank values are sent in json" do
+      patch "/dois/#{doi.doi}", valid_attributes, headers
+
+      xml = Maremma.from_xml(Base64.decode64(json.dig("data", "attributes", "xml"))).fetch("resource", {})
+      expect(xml.dig("alternateIdentifiers")).to eq(nil)
+      expect(json.dig("data", "attributes", "identifiers")).to eq([])
+      expect(json.dig("data", "attributes", "alternateIdentifiers")).to eq([])
+    end
+  end
+
+  context "when a doi has alternateIdentifier/identifier values" do
+    let(:valid_attributes) do
+      {
+        "data" => {
+          "type" => "dois",
+          "attributes" => {
+            "alternateIdentifiers" => [
+              {
+                "alternateIdentifier" => "identifier",
+                "alternateIdentifierType" => "identifierType",
+              },
+              {
+                "alternateIdentifier" => "identifier_2",
+                "alternateIdentifierType" => "identifierType_2",
+              },
+            ],
+          },
+        },
+      }
+    end
+
+    it "the xml and doi record contain the values" do
+      xml = Maremma.from_xml(doi.xml).fetch("resource", {})
+      expect(xml.dig("alternateIdentifiers")).not_to eq(nil)
+      expect(doi.identifiers).not_to eq(nil)
+    end
+
+    it "the values are changed when new values are sent in json" do
+      patch "/dois/#{doi.doi}", valid_attributes, headers
+
+      xml = Maremma.from_xml(Base64.decode64(json.dig("data", "attributes", "xml"))).fetch("resource", {})
+      expect(xml.dig("alternateIdentifiers")).to eq(
+        "alternateIdentifier" =>
+          [
+            { "__content__" => "identifier", "alternateIdentifierType" => "identifierType" },
+            { "__content__" => "identifier_2", "alternateIdentifierType" => "identifierType_2" }
+          ]
+      )
+      expect(json.dig("data", "attributes", "identifiers")).to eq([
+        {
+          "identifier" => "identifier",
+          "identifierType" => "identifierType"
+        },
+        {
+          "identifier" => "identifier_2",
+          "identifierType" => "identifierType_2"
+        }
+      ])
+      expect(json.dig("data", "attributes", "alternateIdentifiers")).to eq([
+        {
+          "alternateIdentifier" => "identifier",
+          "alternateIdentifierType" => "identifierType"
+        },
+        {
+          "alternateIdentifier" => "identifier_2",
+          "alternateIdentifierType" => "identifierType_2"
+        }
+      ])
+    end
+
+    context "when a doi has alternateIdentifier/identifier values" do
+      let(:valid_attributes) do
+        {
+          "data" => {
+            "type" => "dois",
+            "attributes" => {
+              "subjects" => nil,
+            },
+          },
+        }
+      end
+
+      it "the xml and doi record contain the values" do
+        xml = Maremma.from_xml(doi.xml).fetch("resource", {})
+        expect(xml.dig("alternateIdentifiers")).not_to eq(nil)
+        expect(doi.identifiers).not_to eq(nil)
+      end
+
+      it "the values are the same when no values are sent in json" do
+        patch "/dois/#{doi.doi}", valid_attributes, headers
+
+        xml = Maremma.from_xml(Base64.decode64(json.dig("data", "attributes", "xml"))).fetch("resource", {})
+        expect(xml.dig("alternateIdentifiers")).to eq(
+          "alternateIdentifier" => {
+            "__content__" => "pk-1234", "alternateIdentifierType" => "publisher ID"
+          }
+        )
+        expect(json.dig("data", "attributes", "identifiers")).to eq([
+          {
+            "identifier" => "pk-1234",
+            "identifierType" => "publisher ID"
+          },
+        ])
+        expect(json.dig("data", "attributes", "alternateIdentifiers")).to eq([
+          {
+            "alternateIdentifier" => "pk-1234",
+            "alternateIdentifierType" => "publisher ID"
+          },
+        ])
+      end
+    end
+  end
 end