Merge branch 'feature/3933_autocheck_keepers_update' into develop

doajtest/testbook/autocheck/autocheck.yml

@@ -44,11 +44,12 @@ tests:
   - step: Close the ISSN.org window/tab and return to the application form
   - step: Scroll to the "Best Practice" section of the application form, and look at the "Long-term preservation services" question
     results:
-      - "4 checks are visible attached to this question, for: CLOCKSS, LOCKSS, PMC and PKP PN"
-      - CLOCKSS is annotated with a green tick, saying it is archived
-      - LOCKSS is annotated with a red cross, saying it is not current
-      - PMC is annotated with a grey info symbol, saying it is not currently recorded by Keepers
+      - "5 checks are visible attached to this question, for: CLOCKSS, LOCKSS, Internet Archive, PMC and PKP PN"
+      - CLOCKSS is annotated with a red exclamation, saying it is archived but not selected in the form
+      - LOCKSS is annotated with a green tick, saying it is currently archived
+      - Internet Archiive is annotated with a red cross, saying it is archived but not current
       - PKP PN is annotated with a red cross, saying it is not archived
+      - PMC is annotated with a grey info symbol, saying it is not currently recorded by Keepers
   - step: Click on one of the "see record" links in the annotation
     results:
       - The link is opened in a new window/tab
@@ -90,11 +91,12 @@ tests:
   - step: Close the ISSN.org window/tab and return to the journal form
   - step: Scroll to the "Best Practice" section of the application form, and look at the "Long-term preservation services" question
     results:
-      - "4 checks are visible attached to this question, for: CLOCKSS, LOCKSS, PMC and PKP PN"
-      - CLOCKSS is annotated with a green tick, saying it is archived
-      - LOCKSS is annotated with a red cross, saying it is not current
-      - PMC is annotated with a grey info symbol, saying it is not currently recorded by Keepers
+      - "5 checks are visible attached to this question, for: CLOCKSS, LOCKSS, Internet Archive, PMC and PKP PN"
+      - CLOCKSS is annotated with a red exclamation, saying it is archived but not selected in the form
+      - LOCKSS is annotated with a green tick, saying it is currently archived
+      - Internet Archiive is annotated with a red cross, saying it is archived but not current
       - PKP PN is annotated with a red cross, saying it is not archived
+      - PMC is annotated with a grey info symbol, saying it is not currently recorded by Keepers
   - step: Click on one of the "see record" links in the annotation
     results:
       - The link is opened in a new window/tab

doajtest/testdrive/autocheck.py

@@ -25,17 +25,19 @@ def setup(self) -> dict:
         ##
         ## - Print ISSN registered at ISSN.org
         ## - Electronic ISSN not registered at ISSN.org
-        ## - 3 preservation services:
+        ## - 5 preservation services:
         ##    - CLOCKSS - currently archived
         ##    - LOCKSS - not currently archived
         ##    - PMC - not registered
+        ##     - PKP PN - no info
+        ##     - national_library - BL
         source = ApplicationFixtureFactory.make_application_source()
         ap = models.Application(**source)
         ap.application_type = constants.APPLICATION_TYPE_NEW_APPLICATION
         ap.remove_current_journal()
         ap.remove_related_journal()
         apbj = ap.bibjson()
-        apbj.set_preservation(["CLOCKSS", "LOCKSS", "PMC", "PKP PN"], "http://policy.example.com")
+        apbj.set_preservation(["LOCKSS", "Internet Archive", "PKP PN", "PMC", ["a national library", "BL"]], "http://policy.example.com")
         ap.set_id(ap.makeid())
         ap.save()
 
@@ -62,6 +64,13 @@ def setup(self) -> dict:
                     "holdingArchive": {
                         "@id": "http://issn.org/organization/keepers#lockss"
                     },
+                    "temporalCoverage": "2022/" + str(thisyear)
+                },
+                {
+                    "@type": "ArchiveComponent",
+                    "holdingArchive": {
+                        "@id": "http://issn.org/organization/keepers#internetarchive"
+                    },
                     "temporalCoverage": "2019/2020"
                 }
             ]
@@ -84,6 +93,13 @@ def setup(self) -> dict:
                     "holdingArchive": {
                         "@id": "http://issn.org/organization/keepers#lockss"
                     },
+                    "temporalCoverage": "2022/" + str(thisyear)
+                },
+                {
+                    "@type": "ArchiveComponent",
+                    "holdingArchive": {
+                        "@id": "http://issn.org/organization/keepers#internetarchive"
+                    },
                     "temporalCoverage": "2019/2020"
                 }
             ]
@@ -114,15 +130,20 @@ def setup(self) -> dict:
         ##
         ## - Print ISSN registered at ISSN.org
         ## - Electronic ISSN not found
-        ## - 3 preservation services:
+        ## - 5 preservation services:
         ##    - CLOCKSS - currently archived
         ##    - LOCKSS - not currently archived
         ##    - PMC - not registered
+        ##     - PKP PN - no info
+        ##     - national_library - BL
 
         source = JournalFixtureFactory.make_journal_source()
         j = models.Journal(**source)
         j.remove_current_application()
         j.set_id(ap.makeid())
+        jbj = j.bibjson()
+        jbj.set_preservation(["LOCKSS", "Internet Archive", "PKP PN", "PMC", ["a national library", "BL"]],
+                              "http://policy.example.com")
         j.save()
 
         bj = j.bibjson()

doajtest/unit/autocheckers/test_keepers_registry.py

@@ -31,7 +31,8 @@ def test_01_registered(self):
         form = {
             "pissn": "1234-5678",
             "eissn": "9876-5432",
-            "preservation_service": ["LOCKSS", "Internet Archive", "PKP PN"]
+            "preservation_service": ["LOCKSS", "Internet Archive", "PKP PN", "PMC", "national_library"],
+            "preservation_service_library": "BL"
         }
 
         source = ApplicationFixtureFactory.make_application_source()
@@ -42,20 +43,54 @@ def test_01_registered(self):
 
         kr.check(form, app, autochecks, resources, logger=lambda x: x)
 
-        assert len(autochecks.checks) == 3
+        assert len(autochecks.checks) == 5
 
-        checks = [False, False, False]
+        checks = [False, False, False, False, False]
         for check in autochecks.checks:
+            if check["context"]["service"] == "CLOCKSS":
+                assert check["advice"] == kr.SHOULD_SELECT
+                checks[0] = True
+
             if check["context"]["service"] == "LOCKSS":
                 assert check["advice"] == kr.PRESENT
-                checks[0] = True
+                checks[1] = True
 
             if check["context"]["service"] == "Internet Archive":
                 assert check["advice"] == kr.OUTDATED
-                checks[1] = True
+                checks[2] = True
 
             if check["context"]["service"] == "PKP PN":
                 assert check["advice"] == kr.MISSING
-                checks[2] = True
+                checks[3] = True
+
+            if check["context"]["service"] == "PMC":
+                assert check["advice"] == kr.NOT_RECORDED
+                checks[4] = True
+
+        assert all(checks)
+
+    def test_02_none(self):
+        Resource.fetch = ResourceBundleResourceMockFactory.no_contact_resource_fetch(archive_components={
+            "CLOCKSS": True,
+            "LOCKSS": True,
+            "Internet Archive": False
+        })
+
+        kr = KeepersRegistry()
+
+        form = {
+            "pissn": "1234-5678",
+            "eissn": "9876-5432",
+            "preservation_service": ["none"]
+        }
+
+        source = ApplicationFixtureFactory.make_application_source()
+        app = models.Application(**source)
+
+        autochecks = models.Autocheck()
+        resources = ResourceBundle()
+
+        kr.check(form, app, autochecks, resources, logger=lambda x: x)
 
-        assert all(checks)
+        # should just get the CLOCKSS and LOCKSS results
+        assert len(autochecks.checks) == 2

portality/autocheck/checkers/keepers_registry.py

@@ -16,10 +16,13 @@ class KeepersRegistry(ISSNChecker):
         "Portico": "http://issn.org/organization/keepers#portico"
     }
 
+    REVERSE_ID_MAP = {v: k for k, v in ID_MAP.items()}
+
     MISSING = "missing"
     PRESENT = "present"
     OUTDATED = "outdated"
     NOT_RECORDED = "not_recorded"
+    SHOULD_SELECT = "should_select"
 
     def _get_archive_components(self, eissn_data, pissn_data):
         acs = []
@@ -59,9 +62,52 @@ def check(self, form: dict,
         acs = self._get_archive_components(eissn_data, pissn_data)
         ad = self._extract_archive_data(acs)
         services = form.get("preservation_service", [])
+        service_ids = [self.ID_MAP.get(s) for s in services if s in self.ID_MAP]
+
         logger("There are {x} preservation services on the record: {y}".format(x=len(services), y=",".join(services)))
+
+        for archive_id, end_date in ad.items():
+            if archive_id not in service_ids and end_date >= datetime.utcnow().year - 1:
+                service_name = self.REVERSE_ID_MAP.get(archive_id)
+                logger("Service '{x}' has not been selected, but is registered and current in Keepers".format(x=service_name))
+                autochecks.add_check(
+                    field="preservation_service",
+                    advice=self.SHOULD_SELECT,
+                    reference_url=url,
+                    context={"service": service_name},
+                    checked_by=self.__identity__
+                )
+                continue
+
+            if archive_id in service_ids:
+                service_name = self.REVERSE_ID_MAP.get(archive_id)
+                if end_date >= datetime.utcnow().year - 1:
+                    logger("Service '{x}' is registered and current in Keepers".format(x=service_name))
+                    autochecks.add_check(
+                        field="preservation_service",
+                        advice=self.PRESENT,
+                        reference_url=url,
+                        context={"service": service_name},
+                        checked_by=self.__identity__
+                    )
+                else:
+                    # the temporal coverage is too old
+                    logger(
+                        "Service {x} is registerd as issn.org for this record, but the archive is not recent enough".format(x=service_name))
+                    autochecks.add_check(
+                        field="preservation_service",
+                        advice=self.OUTDATED,
+                        reference_url=url,
+                        context={"service": service_name},
+                        checked_by=self.__identity__
+                    )
+
         for service in services:
+            if service in ["none", "national_library"]:
+                continue
+
             id = self.ID_MAP.get(service)
+
             if not id:
                 logger("Service {x} is not recorded by Keepers Registry".format(x=service))
                 autochecks.add_check(
@@ -73,8 +119,7 @@ def check(self, form: dict,
                 )
                 continue
 
-            coverage = ad.get(id)
-            if coverage is None:
+            if id not in ad:
                 # the archive is not mentioned in issn.org
                 logger("Service {x} is not registered at issn.org for this record".format(x=service))
                 autochecks.add_check(
@@ -84,25 +129,3 @@ def check(self, form: dict,
                     context={"service": service},
                     checked_by=self.__identity__
                 )
-                continue
-
-            if coverage < datetime.utcnow().year - 1:
-                # the temporal coverage is too old
-                logger("Service {x} is registerd as issn.org for this record, but the archive is not recent enough".format(x=service))
-                autochecks.add_check(
-                    field="preservation_service",
-                    advice=self.OUTDATED,
-                    reference_url=url,
-                    context={"service": service},
-                    checked_by=self.__identity__
-                )
-            else:
-                # the coverage is within a reasonable period
-                logger("Service {x} is registerd as issn.org for this record".format(x=service))
-                autochecks.add_check(
-                    field="preservation_service",
-                    advice=self.PRESENT,
-                    reference_url=url,
-                    context={"service": service},
-                    checked_by=self.__identity__
-                )

portality/static/js/autochecks.js

@@ -32,31 +32,34 @@ doaj.autocheckers.ISSNActive = class {
         let style = this.STYLE[autocheck.advice];
 
         let frag = `<div><span class="icon-container icon-container--${autocheck.advice} icon-container--${style}"><span data-feather="${icon}" aria-hidden="true"></span></span>
-                    ${message} (<a href="${autocheck.reference_url}" target="_blank">see record</a>)</div>`;
+                    ${message} (<a href="${autocheck.reference_url}" target="_blank">see record</a>) [last checked: ${doaj.humanDate(doaj.autochecks.created_date)}]</div>`;
         return frag;
     }
 }
 
 doaj.autocheckers.KeepersRegistry = class {
     MESSAGES = {
-        "missing": "Keepers does not show any content archived in {service}.",
-        "present": "The journal content is actively archived in {service}.",
-        "outdated": "The journal has content archived in {service} but it's not current.",
-        "not_recorded": "Keepers Registry does not currently record information about {service}."
+        "missing": "{service} is not shown as receiving content, according to Keepers Registry, but is selected in the form.",
+        "present": "{service} is actively archiving content, according to Keepers Registry.",
+        "outdated": "{service} shows content archived historically, <strong>but not currently</strong>, according to Keepers Registry.",
+        "not_recorded": "{service} is not currently covered by Keepers Registry, no information available.",
+        "should_select": "{service} is shown as receiving content, according to Keepers Registry, but is not selected in the form."
     }
 
     ICONS = {
         "missing": "x-circle",
         "present": "check-circle",
         "outdated": "x-circle",
-        "not_recorded": "info"
+        "not_recorded": "info",
+        "should_select": "alert-circle"
     }
 
     STYLE = {
         "missing": "error",
         "present": "success",
         "outdated": "error",
-        "not_recorded": "info"
+        "not_recorded": "info",
+        "should_select": "warn"
     }
 
     draw(autocheck) {
@@ -69,7 +72,7 @@ doaj.autocheckers.KeepersRegistry = class {
         let style = this.STYLE[autocheck.advice];
 
         let frag = `<div><span class="icon-container icon-container--${autocheck.advice} icon-container--${style}""><span data-feather="${icon}" aria-hidden="true"></span></span>
-                    ${message} (<a href="${autocheck.reference_url}" target="_blank">see record</a>)</div>`;
+                    ${message} (<a href="${autocheck.reference_url}" target="_blank">see record</a>) [last checked: ${doaj.humanDate(doaj.autochecks.created_date)}]</div>`;
         return frag;
     }
 }

portality/templates-v2/management/_application-form/includes/_editorial_form_body.html

@@ -1,6 +1,5 @@
 {% include "management/_application-form/includes/_edit_status.html" %}
 {% include "_application-form/includes/_backend_validation.html" %}
-{% include "management/_application-form/includes/_autochecks.html" %}
 {% import "_application-form/includes/_application_warning_msg.html" as _msg %}
 
 {% if obj and (obj.es_type == 'journal' and obj.is_in_doaj()) %}

portality/templates-v2/management/_application-form/includes/_editorial_side_panel.html

@@ -1,4 +1,5 @@
 <div class="editorial-panel__wrapper">
+    {% include "management/_application-form/includes/_autochecks.html" %}
     {% include "management/_application-form/includes/_contact.html" %}
     {% if obj %}
         <h3 class="label"> Locked for editing until <span id="lock_expires"