Track add-author status for handling blank ORCID search results

src/submission/const.py

@@ -0,0 +1,11 @@
+"""
+Constants defined by the submission app
+"""
+
+from utils.const import EnumContains
+
+
+class AddAuthorStatus(EnumContains):
+    OK = "ok"
+    NOT_FOUND = "not_found"
+    ORCID_ERROR = "orcid_error"

src/submission/logic.py

@@ -19,6 +19,7 @@
 from utils import orcid, setting_handler, shared as utils_shared
 from utils.forms import clean_orcid_id
 from submission import models
+from submission.const import AddAuthorStatus
 from submission.forms import EditFrozenAuthor, CreditRecordForm
 
 
@@ -386,7 +387,8 @@ def add_author_using_email(search_term, article):
         email,
         article,
     )
-    return author, created
+    status = AddAuthorStatus.OK if author else AddAuthorStatus.NOT_FOUND
+    return author, created, status
 
 
 def add_author_using_orcid(search_term, article, request):
@@ -396,19 +398,20 @@ def add_author_using_orcid(search_term, article, request):
     try:
         cleaned_orcid = clean_orcid_id(search_term)
     except ValueError:
-        cleaned_orcid = ""
-        return author, created
+        return author, created, AddAuthorStatus.NOT_FOUND
 
     author, created = models.FrozenAuthor.get_or_snapshot_if_orcid_found(
         cleaned_orcid,
         article,
     )
     if author:
-        return author, created
+        return author, created, AddAuthorStatus.OK
 
     # If there is no account or frozen author in Janeway with that orcid and article,
     # get the public ORCID record.
     orcid_details = orcid.get_orcid_record_details(cleaned_orcid)
+    if not orcid_details:
+        return author, created, AddAuthorStatus.ORCID_ERROR
     orcid_emails = orcid_details.get("emails", [])
 
     # Check for accounts by email address.
@@ -422,7 +425,7 @@ def add_author_using_orcid(search_term, article, request):
             account.save()
             author = account.snapshot_as_author(article)
             created = True
-            return author, created
+            return author, created, AddAuthorStatus.OK
         except core_models.Account.DoesNotExist:
             author = None
 
@@ -445,7 +448,8 @@ def add_author_using_orcid(search_term, article, request):
     if created:
         add_author_affiliation_from_orcid(author, orcid_details, request)
 
-    return author, created
+    status = AddAuthorStatus.OK if author else AddAuthorStatus.NOT_FOUND
+    return author, created, status
 
 
 def add_author_affiliation_from_orcid(author, orcid_details, request):
@@ -465,29 +469,42 @@ def add_author_from_search(search_term, request, article):
     """
     Tries to add a FrozenAuthor from a search query.
     """
-    author, created = add_author_using_email(search_term, article)
+    author, created, status = add_author_using_email(search_term, article)
     if not author:
-        author, created = add_author_using_orcid(search_term, article, request)
+        author, created, status = add_author_using_orcid(
+            search_term,
+            article,
+            request,
+        )
 
-    if author:
-        if created:
-            messages.add_message(
-                request,
-                messages.SUCCESS,
-                _("%(author_name)s is now an author.")
-                % {
-                    "author_name": author.full_name(),
-                },
-            )
-        else:
-            messages.add_message(
-                request,
-                messages.INFO,
-                _("%(author_name)s is already an author.")
-                % {
-                    "author_name": author.full_name(),
-                },
+    if author and created:
+        messages.add_message(
+            request,
+            messages.SUCCESS,
+            _("%(author_name)s is now an author.")
+            % {"author_name": author.full_name()},
+        )
+    elif author:
+        messages.add_message(
+            request,
+            messages.INFO,
+            _("%(author_name)s is already an author.")
+            % {"author_name": author.full_name()},
+        )
+    elif status == AddAuthorStatus.ORCID_ERROR:
+        try:
+            cleaned_orcid = clean_orcid_id(search_term)
+        except ValueError:
+            cleaned_orcid = search_term
+        messages.add_message(
+            request,
+            messages.ERROR,
+            _(
+                "We couldn't retrieve a record from ORCID for "
+                "%(orcid)s. Check the ORCID ID and try again later."
             )
+            % {"orcid": cleaned_orcid},
+        )
     else:
         messages.add_message(
             request,

src/submission/tests/test_logic.py

@@ -3,6 +3,7 @@
 __license__ = "AGPL v3"
 __maintainer__ = "Open Library of Humanities"
 
+from django.contrib.messages import constants as message_constants
 from django.shortcuts import reverse
 from django.test import TestCase, client
 from mock import patch
@@ -216,3 +217,72 @@ def test_add_author_from_search_orcid_public_affiliations(self, get_orcid_detail
             last_author.primary_affiliation().__str__(),
             "Birkbeck",
         )
+
+    @patch("utils.orcid.get_orcid_record_details")
+    def test_add_author_from_search_orcid_lookup_fails(
+        self,
+        get_orcid_details,
+    ):
+        """
+        When the ORCID lookup fails (e.g. misconfigured settings or invalid
+        ORCID), get_orcid_record_details returns an empty dict; no
+        FrozenAuthor should be created and an error message should be
+        shown to the user.
+        """
+        from collections import defaultdict
+
+        get_orcid_details.return_value = defaultdict(lambda: None)
+        starting_author_count = self.article.frozenauthor_set.count()
+        self.client.force_login(self.kathleen)
+        post_data = {
+            "search_authors": "",
+            "author_search_text": "0000-5678-5678-5678",
+        }
+        response = self.client.post(
+            reverse("submit_authors", kwargs={"article_id": self.article.pk}),
+            post_data,
+            SERVER_NAME=self.journal_one.domain,
+            follow=True,
+        )
+        self.assertEqual(
+            self.article.frozenauthor_set.count(),
+            starting_author_count,
+        )
+        all_messages = list(response.context["messages"])
+        error_messages = [
+            str(m) for m in all_messages if m.level == message_constants.ERROR
+        ]
+        warning_messages = [
+            str(m) for m in all_messages if m.level == message_constants.WARNING
+        ]
+        self.assertTrue(
+            any("ORCID" in m for m in error_messages),
+            f"Expected ORCID error message, got: {error_messages}",
+        )
+        # The generic "No author found" warning should be suppressed
+        # since we already showed a specific error for the ORCID failure.
+        self.assertFalse(
+            any("No author found" in m for m in warning_messages),
+            f"Did not expect 'No author found' warning, got: {warning_messages}",
+        )
+
+    def test_add_author_from_search_malformed_orcid(self):
+        """
+        A search term that doesn't match the ORCID format should not
+        create a FrozenAuthor (clean_orcid_id raises ValueError).
+        """
+        starting_author_count = self.article.frozenauthor_set.count()
+        self.client.force_login(self.kathleen)
+        post_data = {
+            "search_authors": "",
+            "author_search_text": "not-an-orcid",
+        }
+        self.client.post(
+            reverse("submit_authors", kwargs={"article_id": self.article.pk}),
+            post_data,
+            SERVER_NAME=self.journal_one.domain,
+        )
+        self.assertEqual(
+            self.article.frozenauthor_set.count(),
+            starting_author_count,
+        )

src/utils/tests.py

@@ -1299,6 +1299,12 @@ def test_record_details_min(self, mock_record):
         self.assertIsNone(details["affiliation"])
         self.assertIsNone(details["country"])
 
+    @mock.patch("utils.orcid.get_orcid_record", return_value=None)
+    def test_record_details_empty_on_lookup_failure(self, mock_record):
+        details = get_orcid_record_details("0000-0000-0000-0000")
+        self.assertFalse(details)
+        self.assertEqual(len(details), 0)
+
     @override_settings(URL_CONFIG="domain")
     @mock.patch("utils.logic.get_current_request")
     def test_redirect_uri(self, get_current_request):