From 85a6552295537743a61188f3afdb63a1cc279cd8 Mon Sep 17 00:00:00 2001
From: Zacharias Zacharodimos <zacharias.zacharodimos@cern.ch>
Date: Tue, 10 Dec 2024 18:07:01 +0100
Subject: [PATCH] doi: require a doi after the first minting

---
 .../services/components/pids.py               |  7 ++-
 tests/services/test_rdm_service.py            | 52 +++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/invenio_rdm_records/services/components/pids.py b/invenio_rdm_records/services/components/pids.py
index c41f68c41..c6e6f5690 100644
--- a/invenio_rdm_records/services/components/pids.py
+++ b/invenio_rdm_records/services/components/pids.py
@@ -75,6 +75,11 @@ def publish(self, identity, draft=None, record=None):
         record_schemes = set(record_pids.keys())
         required_schemes = set(self.service.config.pids_required)
 
+        # if a doi was ever minted for the parent record then we always require one
+        # for any version of the record that will be published
+        if "doi" in draft.parent.pids:
+            required_schemes.add("doi")
+
         # Validate the draft PIDs
         self.service.pids.pid_manager.validate(draft_pids, draft, raise_errors=True)
 
@@ -173,7 +178,7 @@ def publish(self, identity, draft=None, record=None):
         required_schemes = set(self.service.config.parent_pids_required)
 
         # if parent DOI is not required in the config, but record DOI is created, we need to create parent DOI as well
-        if "doi" not in required_schemes and draft and draft.get("pids", {}).get("doi"):
+        if draft and draft.get("pids", {}).get("doi"):
             required_schemes.add("doi")
 
         conditional_schemes = self.service.config.parent_pids_conditional
diff --git a/tests/services/test_rdm_service.py b/tests/services/test_rdm_service.py
index ae1158d99..f71f7469a 100644
--- a/tests/services/test_rdm_service.py
+++ b/tests/services/test_rdm_service.py
@@ -12,6 +12,9 @@
 
 import pytest
 
+from copy import deepcopy
+from marshmallow import ValidationError
+
 from invenio_rdm_records.proxies import current_rdm_records
 from invenio_rdm_records.services.errors import EmbargoNotLiftedError
 
@@ -68,6 +71,55 @@ def test_publish_public_record_with_optional_doi(
     draft = service.create(superuser_identity, minimal_record)
     record = service.publish(id_=draft.id, identity=superuser_identity)
     assert "doi" not in record._record.pids
+    assert "doi" not in record._record.parent.pids
+
+    # edit the record and mint the DOI
+    draft = service.edit(superuser_identity, record.id)
+    draft = service.pids.create(superuser_identity, draft.id, "doi")
+
+    assert "doi" in draft._record.pids
+    record = service.publish(id_=draft.id, identity=superuser_identity)
+    assert "doi" in record._record.pids
+    assert "doi" in record._record.parent.pids
+
+    parent_doi = record._record.parent.pids["doi"]
+
+    # create a new version and ensure that doi is minted by default now when you publish
+    draft = service.new_version(superuser_identity, record.id)
+    draft_data = deepcopy(draft.data)
+    draft_data["metadata"]["publication_date"] = "2023-01-01"
+    draft = service.update_draft(superuser_identity, draft.id, data=draft_data)
+
+    record = service.publish(id_=draft.id, identity=superuser_identity)
+    assert "doi" in record._record.pids
+    assert "doi" in record._record.parent.pids
+    assert record._record.parent.pids["doi"] == parent_doi
+
+    # create a new version and try to pass an external DOI
+    draft = service.new_version(superuser_identity, record.id)
+    draft_data = deepcopy(draft.data)
+    draft_data["metadata"]["publication_date"] = "2023-01-01"
+    draft_data["pids"]["doi"] = {
+        "identifier": "10.4321/test.1234",
+        "provider": "external",
+    }
+    draft = service.update_draft(superuser_identity, draft.id, data=draft_data)
+    record = service.publish(id_=draft.id, identity=superuser_identity)
+    assert "doi" in record._record.pids
+    assert "doi" in record._record.parent.pids
+    assert record._record.parent.pids["doi"] == parent_doi
+
+    # create a new version and try to pass unset pids i.e. pids: {}
+    # because of previous minted doi the doi should be required
+    draft = service.new_version(superuser_identity, record.id)
+    draft_data = deepcopy(draft.data)
+    draft_data["metadata"]["publication_date"] = "2023-01-01"
+    draft_data["pids"]["doi"] = {}
+    draft = service.update_draft(superuser_identity, draft.id, data=draft_data)
+
+    with pytest.raises(ValidationError):
+        record = service.publish(id_=draft.id, identity=superuser_identity)
+
     # Reset the running_app config for next tests
     running_app.app.config["RDM_PERSISTENT_IDENTIFIERS"]["doi"]["required"] = True