From 5538a608b124357656f6bc81fb6bed1a50ed3f08 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Fri, 30 Aug 2024 08:51:59 +0100 Subject: [PATCH 1/3] extend the behaviour of the keepers registry checker to review all available entries --- .../autocheckers/test_keepers_registry.py | 20 ++++-- .../autocheck/checkers/keepers_registry.py | 71 ++++++++++++------- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/doajtest/unit/autocheckers/test_keepers_registry.py b/doajtest/unit/autocheckers/test_keepers_registry.py index 7e49f4612f..69a4e67630 100644 --- a/doajtest/unit/autocheckers/test_keepers_registry.py +++ b/doajtest/unit/autocheckers/test_keepers_registry.py @@ -31,7 +31,7 @@ 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"] } source = ApplicationFixtureFactory.make_application_source() @@ -42,20 +42,28 @@ 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) \ No newline at end of file diff --git a/portality/autocheck/checkers/keepers_registry.py b/portality/autocheck/checkers/keepers_registry.py index c722c2d864..517e849085 100644 --- a/portality/autocheck/checkers/keepers_registry.py +++ b/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 == "none": + 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__ - ) \ No newline at end of file From 527e588aab3156ce328564c702107e482e6bd7b7 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Fri, 30 Aug 2024 14:06:18 +0100 Subject: [PATCH 2/3] update test stack for new setup for keepers autochecks --- doajtest/testbook/autocheck/autocheck.yml | 18 ++++++----- doajtest/testdrive/autocheck.py | 27 ++++++++++++++-- .../autocheckers/test_keepers_registry.py | 31 +++++++++++++++++-- .../autocheck/checkers/keepers_registry.py | 2 +- portality/static/js/autochecks.js | 15 +++++---- 5 files changed, 73 insertions(+), 20 deletions(-) diff --git a/doajtest/testbook/autocheck/autocheck.yml b/doajtest/testbook/autocheck/autocheck.yml index e13965b5ac..1cebe14fda 100644 --- a/doajtest/testbook/autocheck/autocheck.yml +++ b/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 diff --git a/doajtest/testdrive/autocheck.py b/doajtest/testdrive/autocheck.py index 7e9bf4fbbe..cdc9d113e4 100644 --- a/doajtest/testdrive/autocheck.py +++ b/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() diff --git a/doajtest/unit/autocheckers/test_keepers_registry.py b/doajtest/unit/autocheckers/test_keepers_registry.py index 69a4e67630..f8f3a777fd 100644 --- a/doajtest/unit/autocheckers/test_keepers_registry.py +++ b/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", "PMC"] + "preservation_service": ["LOCKSS", "Internet Archive", "PKP PN", "PMC", "national_library"], + "preservation_service_library": "BL" } source = ApplicationFixtureFactory.make_application_source() @@ -66,4 +67,30 @@ def test_01_registered(self): assert check["advice"] == kr.NOT_RECORDED checks[4] = True - assert all(checks) \ No newline at end of file + 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) + + # should just get the CLOCKSS and LOCKSS results + assert len(autochecks.checks) == 2 \ No newline at end of file diff --git a/portality/autocheck/checkers/keepers_registry.py b/portality/autocheck/checkers/keepers_registry.py index 517e849085..d66b724e40 100644 --- a/portality/autocheck/checkers/keepers_registry.py +++ b/portality/autocheck/checkers/keepers_registry.py @@ -103,7 +103,7 @@ def check(self, form: dict, ) for service in services: - if service == "none": + if service in ["none", "national_library"]: continue id = self.ID_MAP.get(service) diff --git a/portality/static/js/autochecks.js b/portality/static/js/autochecks.js index 32da79de29..2cae4139cf 100644 --- a/portality/static/js/autochecks.js +++ b/portality/static/js/autochecks.js @@ -39,24 +39,27 @@ doaj.autocheckers.ISSNActive = class { 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, but not currently, 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) { From 2c2e5d2c1ddc5e66ff03e6936684dcfe76ff3153 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Tue, 7 Jan 2025 15:01:10 +0000 Subject: [PATCH 3/3] add date to all autochecks, and move autocheck controls to sidebar --- portality/static/js/autochecks.js | 4 ++-- .../_application-form/includes/_editorial_form_body.html | 1 - .../_application-form/includes/_editorial_side_panel.html | 1 + 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/portality/static/js/autochecks.js b/portality/static/js/autochecks.js index 32da79de29..0b58c55012 100644 --- a/portality/static/js/autochecks.js +++ b/portality/static/js/autochecks.js @@ -32,7 +32,7 @@ doaj.autocheckers.ISSNActive = class { let style = this.STYLE[autocheck.advice]; let frag = `
- ${message} (see record)
`; + ${message} (see record) [last checked: ${doaj.humanDate(doaj.autochecks.created_date)}]`; return frag; } } @@ -69,7 +69,7 @@ doaj.autocheckers.KeepersRegistry = class { let style = this.STYLE[autocheck.advice]; let frag = `
- ${message} (see record)
`; + ${message} (see record) [last checked: ${doaj.humanDate(doaj.autochecks.created_date)}]`; return frag; } } diff --git a/portality/templates-v2/management/_application-form/includes/_editorial_form_body.html b/portality/templates-v2/management/_application-form/includes/_editorial_form_body.html index 5e4d1d07b2..809bc7180b 100644 --- a/portality/templates-v2/management/_application-form/includes/_editorial_form_body.html +++ b/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()) %} diff --git a/portality/templates-v2/management/_application-form/includes/_editorial_side_panel.html b/portality/templates-v2/management/_application-form/includes/_editorial_side_panel.html index f7047bd426..853f21e123 100644 --- a/portality/templates-v2/management/_application-form/includes/_editorial_side_panel.html +++ b/portality/templates-v2/management/_application-form/includes/_editorial_side_panel.html @@ -1,4 +1,5 @@
+ {% include "management/_application-form/includes/_autochecks.html" %} {% include "management/_application-form/includes/_contact.html" %} {% if obj %}

Locked for editing until