Address comments

solenoid/records/models.py

@@ -69,6 +69,8 @@ def create_citation(paper_data):
         This method assumes that the supplied paper metadata has already been
         validated; it does not perform any validation.
 
+        The minimal citation is generated using the author's first name and last name
+        and the publication's title and journal (to which it was published).
         If any of the citation fields are missing, the following placeholder text
         will be used in the citation: "<[CITATION FIELD NAME] UNIDENTIFIED>".
 
@@ -81,22 +83,33 @@ def create_citation(paper_data):
             Volume(Issue), pp.-pp. doi:XX.XXXXX.
         We don't appear to get page number information, so we'll skip that.
         """
-        citation_fields = {
-            field: paper_data.get(
-                field, f"<{field.replace('-', ' ').upper()} UNIDENTIFIED>"
-            )
-            for field in Fields.CITATION_DATA
-        }
+        citation_field_placeholder = "<{field} UNIDENTIFIED>"
+
+        first_init = None
+        if first_name := paper_data[Fields.FIRST_NAME]:
+            first_init = first_name[0]
+
         citation = "{last}, {first_init}. ".format(
-            last=citation_fields[Fields.LAST_NAME],
-            first_init=citation_fields[Fields.FIRST_NAME][0],
+            last=paper_data.get(
+                Fields.LAST_NAME,
+                citation_field_placeholder.format(field=Fields.LAST_NAME.upper()),
+            ),
+            first_init=first_init
+            or citation_field_placeholder.format(field=Fields.FIRST_NAME.upper()),
         )
 
         if paper_data[Fields.PUBDATE]:
             citation += f"({paper_data[Fields.PUBDATE][0:4]}). "
 
         citation += "{title}. {journal}".format(
-            title=citation_fields[Fields.TITLE], journal=citation_fields[Fields.JOURNAL]
+            title=paper_data.get(
+                Fields.TITLE,
+                citation_field_placeholder.format(field=Fields.TITLE.upper()),
+            ),
+            journal=paper_data.get(
+                Fields.JOURNAL,
+                citation_field_placeholder.format(field=Fields.JOURNAL.upper()),
+            ),
         )
 
         if paper_data[Fields.VOLUME] and paper_data[Fields.ISSUE]:
@@ -209,32 +222,26 @@ def paper_requested(paper_data):
         return any([record.email.date_sent for record in records if record.email])
 
     @staticmethod
-    def missing_id_fields(paper_data):
-        """Check whether record is missing required ID fields."""
-        # check if record is missing ID fields ('MIT ID', 'PaperID')
+    def get_missing_id_fields(paper_data):
+        """Get missing required ID fields ('MIT ID', 'PaperID')."""
         missing_id_fields = [
             field for field in Fields.REQUIRED_DATA if not paper_data[field]
         ]
         if missing_id_fields:
             return f"Missing required ID fields: {missing_id_fields}."
 
     @staticmethod
-    def missing_citation_fields(paper_data):
-        """Check whether record is citable.
+    def get_missing_citation_fields(paper_data):
+        """Get missing fields needed to create a minimal citation"""
+        if paper_data[Fields.CITATION]:
+            return
 
-        If a citation is provided, it will be used; otherwise a minimal citation
-        is generated using the author's first name and last name and the publication's
-        title and journal (to which it was published).
-        """
-        # check if record is citable
         missing_citation_fields = [
-            field
-            for field in [Fields.CITATION, *Fields.CITATION_DATA]
-            if not paper_data[field]
+            field for field in Fields.CITATION_DATA if not paper_data[field]
         ]
         if missing_citation_fields:
             return (
-                "Citation was not provided and/or "
+                f"Missing data for '{Fields.CITATION}' and "
                 f"missing required fields to generate minimal citation: {missing_citation_fields}."
             )
 

solenoid/records/tasks.py

@@ -107,14 +107,14 @@ def _run_checks_on_paper(paper_data, author):
     author_name = paper_data[Fields.LAST_NAME]
 
     # Check that data provided from Elements is citable
-    if _missing_citation_fields := Record.missing_citation_fields(paper_data):
+    if _missing_citation_fields := Record.get_missing_citation_fields(paper_data):
         logger.info(
             f"Publication #{paper_id} by {author_name} is missing citation fields. "
             f"{_missing_citation_fields}"
         )
 
     # Check that data provided from Elements is complete
-    if _missing_id_fields := Record.missing_id_fields(paper_data):
+    if _missing_id_fields := Record.get_missing_id_fields(paper_data):
         logger.info(f"Paper #{paper_id} missing required data, record not imported")
         return (
             f"Publication #{paper_id} by {author_name} is missing required ID fields. "

solenoid/records/tests/test_models.py

@@ -46,37 +46,31 @@ def test_metadata_not_missing_any_citation_fields(self):
         metadata[Fields.CITATION] = "This is a citation"
         metadata[Fields.TITLE] = "This is a paper title"
         metadata[Fields.JOURNAL] = "Journal of Awesomeness"
-        assert not Record.missing_citation_fields(metadata)
+        assert not Record.get_missing_citation_fields(metadata)
 
     # need to actually test create_citation
     def test_is_cited_metadata_missing_minimal_citation_fields(self):
         metadata = copy.copy(self.paper_data)
         metadata[Fields.CITATION] = "This is a citation"
         metadata[Fields.TITLE] = None
         metadata[Fields.JOURNAL] = None
-        assert Record.missing_citation_fields(metadata) == (
-            "Citation was not provided and/or "
-            f"missing required fields to generate minimal citation: ['{Fields.TITLE}', '{Fields.JOURNAL}']."
-        )
+        assert not Record.get_missing_citation_fields(metadata)
 
     def test_uncited_metadata_with_minimal_citation_fields(self):
         metadata = copy.copy(self.paper_data)
         metadata[Fields.CITATION] = None
         metadata[Fields.TITLE] = "This is a paper title"
         metadata[Fields.JOURNAL] = "Journal of Awesomeness"
-        assert Record.missing_citation_fields(metadata) == (
-            "Citation was not provided and/or "
-            f"missing required fields to generate minimal citation: ['{Fields.CITATION}']."
-        )
+        assert not Record.get_missing_citation_fields(metadata)
 
     def test_is_uncited_metadata_missing_citation_fields(self):
         metadata = copy.copy(self.paper_data)
         metadata[Fields.CITATION] = None
         metadata[Fields.TITLE] = None
         metadata[Fields.JOURNAL] = None
-        assert Record.missing_citation_fields(metadata) == (
-            "Citation was not provided and/or "
-            f"missing required fields to generate minimal citation: ['{Fields.CITATION}', '{Fields.TITLE}', '{Fields.JOURNAL}']."
+        assert Record.get_missing_citation_fields(metadata) == (
+            f"Missing data for '{Fields.CITATION}' and "
+            f"missing required fields to generate minimal citation: ['{Fields.TITLE}', '{Fields.JOURNAL}']."
         )
 
     def test_is_record_creatable(self):