From 0bb316c4707a3a0f699f30eac2477feaab02519d Mon Sep 17 00:00:00 2001 From: philip Date: Fri, 22 Mar 2024 14:01:23 +0000 Subject: [PATCH 1/6] add chekers no_none_value --- .../unit/autocheckers/test_no_none_value.py | 81 +++++++++++++++++++ portality/autocheck/checkers/no_none_value.py | 38 +++++++++ portality/bll/services/autochecks.py | 7 +- 3 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 doajtest/unit/autocheckers/test_no_none_value.py create mode 100644 portality/autocheck/checkers/no_none_value.py diff --git a/doajtest/unit/autocheckers/test_no_none_value.py b/doajtest/unit/autocheckers/test_no_none_value.py new file mode 100644 index 0000000000..8cf34bbca5 --- /dev/null +++ b/doajtest/unit/autocheckers/test_no_none_value.py @@ -0,0 +1,81 @@ +from doajtest.fixtures import ApplicationFixtureFactory +from doajtest.helpers import DoajTestCase +from portality import models +from portality.autocheck.checkers import no_none_value +from portality.autocheck.checkers.no_none_value import NoNoneValue +from portality.autocheck.resource_bundle import ResourceBundle +from portality.crosswalks.application_form import ApplicationFormXWalk +from portality.models import JournalLikeObject + + +def run_check(checker, form: dict, jla: JournalLikeObject, resources=None): + if resources is None: + resources = ResourceBundle() + + autochecks = models.Autocheck() + checker.check(form, jla, autochecks, resources, logger=print) + return autochecks + + +def fixture_standard_application(modify_fn=None): + source = ApplicationFixtureFactory.make_application_source() + app = models.Application(**source) + modify_fn and modify_fn(app) + form = ApplicationFormXWalk.obj2form(app) + return form, app + + +class TestNoNoneValue(DoajTestCase): + + def test_check__pass(self): + form, app = fixture_standard_application() + autochecks = run_check(NoNoneValue(), form, app) + assert len(autochecks.checks) == 0 + + def test_check__fail_all(self): + def modify_fn(app): + bibjson = app.bibjson() + bibjson.preservation["national_library"] = ['None'] + bibjson.preservation["service"] = ['None'] + bibjson.deposit_policy = ['None'] + bibjson.persistent_identifier_scheme = ['None'] + + form, app = fixture_standard_application(modify_fn) + + autochecks = run_check(NoNoneValue(), form, app) + assert len(autochecks.checks) == 4 + + for check in autochecks.checks: + print(check) + assert check['checked_by'] == NoNoneValue.__identity__ + assert check['advice'] == no_none_value.ADVICE_NONE_VALUE_FOUND + + def test_check__fail_library(self): + def modify_fn(app): + bibjson = app.bibjson() + bibjson.preservation["national_library"] = ['aaa', 'None'] + + form, app = fixture_standard_application(modify_fn) + autochecks = run_check(NoNoneValue(), form, app) + assert len(autochecks.checks) == 1 + assert autochecks.checks[0]['field'] == 'preservation_service_library' + + def test_check__fail_deposit(self): + def modify_fn(app): + bibjson = app.bibjson() + bibjson.deposit_policy = ['None'] + + form, app = fixture_standard_application(modify_fn) + autochecks = run_check(NoNoneValue(), form, app) + assert len(autochecks.checks) == 1 + assert autochecks.checks[0]['field'] == 'deposit_policy_other' + assert autochecks.checks[0]['original_value'] == 'None' + + def test_check__pass_deposit(self): + def modify_fn(app): + bibjson = app.bibjson() + bibjson.deposit_policy = ['aa', 'None'] + + form, app = fixture_standard_application(modify_fn) + autochecks = run_check(NoNoneValue(), form, app) + assert len(autochecks.checks) == 0 diff --git a/portality/autocheck/checkers/no_none_value.py b/portality/autocheck/checkers/no_none_value.py new file mode 100644 index 0000000000..06c0801b0d --- /dev/null +++ b/portality/autocheck/checkers/no_none_value.py @@ -0,0 +1,38 @@ +from typing import Callable + +from portality.autocheck.checker import Checker +from portality.autocheck.resource_bundle import ResourceBundle +from portality.models import JournalLikeObject, Autocheck + +ADVICE_NONE_VALUE_FOUND = 'none_value_found' + + +class NoNoneValue(Checker): + __identity__ = "no_none_value" + + def check(self, form: dict, jla: JournalLikeObject, autochecks: Autocheck, + resources: ResourceBundle, logger: Callable): + fields = [ + 'preservation_service_library', + 'preservation_service_other', + 'deposit_policy_other', + 'persistent_identifiers_other', + ] + + for field in fields: + if field not in form: + logger(f'Field {field} not found in form') + continue + + value = form.get(field) + + if ( + (isinstance(value, str) and value.strip() == 'None') or + (isinstance(value, list) and any([v.strip() == 'None' for v in value])) + ): + autochecks.add_check( + field=field, + original_value=value, + advice=ADVICE_NONE_VALUE_FOUND, + checked_by=self.__identity__, + ) diff --git a/portality/bll/services/autochecks.py b/portality/bll/services/autochecks.py index 0d00fe512c..ec06ef22cd 100644 --- a/portality/bll/services/autochecks.py +++ b/portality/bll/services/autochecks.py @@ -1,12 +1,13 @@ -from portality.crosswalks.application_form import ApplicationFormXWalk, JournalFormXWalk -from portality.autocheck.resource_bundle import ResourceBundle from portality import models - from portality.autocheck.checkers.issn_active import ISSNActive from portality.autocheck.checkers.keepers_registry import KeepersRegistry +from portality.autocheck.checkers.no_none_value import NoNoneValue +from portality.autocheck.resource_bundle import ResourceBundle +from portality.crosswalks.application_form import ApplicationFormXWalk, JournalFormXWalk AUTOCHECK_PLUGINS = [ # (Active on Journal?, Active on Application?, Plugin Class) + (True, True, NoNoneValue), (True, True, ISSNActive), (True, True, KeepersRegistry) ] From 753887c4404abba445c17edbc9157a174b47a556 Mon Sep 17 00:00:00 2001 From: philip Date: Fri, 22 Mar 2024 14:30:11 +0000 Subject: [PATCH 2/6] add DEBUG_BGJOBS_IMMEDIATELY when run app --- doajtest/helpers.py | 6 ++---- portality/app.py | 5 +++++ portality/tasks/redis_huey.py | 20 ++++++++++++++++---- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/doajtest/helpers.py b/doajtest/helpers.py index a8f33d52d5..18e8690099 100644 --- a/doajtest/helpers.py +++ b/doajtest/helpers.py @@ -18,6 +18,7 @@ from portality.lib import paths, dates from portality.lib.dates import FMT_DATE_STD from portality.lib.thread_utils import wait_until +from portality.tasks import redis_huey from portality.tasks.redis_huey import main_queue, long_running from portality.util import url_for @@ -160,10 +161,7 @@ def setUpClass(cls) -> None: # always_eager has been replaced by immediate # for huey version > 2 # https://huey.readthedocs.io/en/latest/guide.html - main_queue.always_eager = True - long_running.always_eager = True - main_queue.immediate = True - long_running.immediate = True + redis_huey.run_bgjobs_immediately() dao.DomainObject.save = dao_proxy(dao.DomainObject.save, type="instance") dao.DomainObject.delete = dao_proxy(dao.DomainObject.delete, type="instance") diff --git a/portality/app.py b/portality/app.py index efd9e5b170..4608a0c68b 100644 --- a/portality/app.py +++ b/portality/app.py @@ -455,6 +455,11 @@ def run_server(host=None, port=None, fake_https=False): port=app.config.get('DEBUG_PYCHARM_PORT', 6000), stdoutToServer=True, stderrToServer=True) + # run background jobs immediately if DEBUG_BGJOBS_IMMEDIATELY is True + if app.config.get('DEBUG_BGJOBS_IMMEDIATELY', False): + from portality.tasks import redis_huey + redis_huey.run_bgjobs_immediately() + run_kwargs = {} if fake_https: run_kwargs['ssl_context'] = 'adhoc' diff --git a/portality/tasks/redis_huey.py b/portality/tasks/redis_huey.py index 6e6b6e9b9d..dcb637f264 100644 --- a/portality/tasks/redis_huey.py +++ b/portality/tasks/redis_huey.py @@ -2,10 +2,12 @@ from portality.core import app # every-day background jobs that take a few minutes each (like, bulk deletes and anything else requested by the user) -main_queue = RedisHuey('doaj_main_queue', host=app.config['HUEY_REDIS_HOST'], port=app.config['HUEY_REDIS_PORT'], always_eager=app.config.get("HUEY_EAGER", False)) +main_queue = RedisHuey('doaj_main_queue', host=app.config['HUEY_REDIS_HOST'], port=app.config['HUEY_REDIS_PORT'], + always_eager=app.config.get("HUEY_EAGER", False)) # jobs that might take a long time, like the harvester or the anon export, which can run for several hours -long_running = RedisHuey('doaj_long_running', host=app.config['HUEY_REDIS_HOST'], port=app.config['HUEY_REDIS_PORT'], always_eager=app.config.get("HUEY_EAGER", False)) +long_running = RedisHuey('doaj_long_running', host=app.config['HUEY_REDIS_HOST'], port=app.config['HUEY_REDIS_PORT'], + always_eager=app.config.get("HUEY_EAGER", False)) """ we put everything we want to be responsive onto the main_queue, @@ -18,7 +20,9 @@ def schedule(action): cfg = app.config.get("HUEY_SCHEDULE", {}) action_cfg = cfg.get(action) if action_cfg is None: - raise RuntimeError("No configuration for scheduled action '{x}'. Define this in HUEY_SCHEDULE first then try again.".format(x=action)) + raise RuntimeError( + "No configuration for scheduled action '{x}'. Define this in HUEY_SCHEDULE first then try again." + .format(x=action)) return crontab(**action_cfg) @@ -27,6 +31,14 @@ def configure(action): cfg = app.config.get("HUEY_TASKS", {}) action_cfg = cfg.get(action) if action_cfg is None: - raise RuntimeError("No task configuration for action '{x}'. Define this in HUEY_TASKS first then try again.".format(x=action)) + raise RuntimeError( + "No task configuration for action '{x}'. Define this in HUEY_TASKS first then try again." + .format(x=action)) return action_cfg + +def run_bgjobs_immediately(): + main_queue.always_eager = True + long_running.always_eager = True + main_queue.immediate = True + long_running.immediate = True From 672b3f6bd7433e7d1bbf0a775f21e2b2ad8e37c3 Mon Sep 17 00:00:00 2001 From: philip Date: Fri, 22 Mar 2024 15:46:41 +0000 Subject: [PATCH 3/6] update frontend to show result of NoNoneValue --- doajtest/testdrive/autocheck.py | 34 ++++++++-------- portality/forms/application_forms.py | 36 ++++++++++++++-- portality/static/js/autochecks.js | 61 ++++++++++++++++++++++++---- 3 files changed, 101 insertions(+), 30 deletions(-) diff --git a/doajtest/testdrive/autocheck.py b/doajtest/testdrive/autocheck.py index 7e9bf4fbbe..931e20312a 100644 --- a/doajtest/testdrive/autocheck.py +++ b/doajtest/testdrive/autocheck.py @@ -1,16 +1,17 @@ -from portality import constants -from doajtest.testdrive.factory import TestDrive +from datetime import datetime + from doajtest.fixtures.v2.applications import ApplicationFixtureFactory from doajtest.fixtures.v2.journals import JournalFixtureFactory +from doajtest.testdrive.factory import TestDrive +from flask import url_for +from portality import constants from portality import models -from datetime import datetime -from portality.autocheck.checkers.issn_active import ISSNActive, ISSNChecker +from portality.autocheck.checkers.issn_active import ISSNChecker from portality.autocheck.resources.issn_org import ISSNOrgData -from portality.autocheck.checkers.keepers_registry import KeepersRegistry from portality.bll import DOAJ -from flask import url_for from portality.core import app + class Autocheck(TestDrive): def setup(self) -> dict: @@ -35,8 +36,13 @@ def setup(self) -> dict: 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(["CLOCKSS", "LOCKSS", "PMC", "PKP PN", "None"], "http://policy.example.com") ap.set_id(ap.makeid()) + + # # data for autocheck no_none_value + apbj.deposit_policy = ['None'] + apbj.persistent_identifier_scheme = ['None'] + ap.save() bj = ap.bibjson() @@ -47,7 +53,7 @@ def setup(self) -> dict: pissn_data = ISSNOrgData({ "mainEntityOfPage": { - "version": "Register" # this means the ISSN is registered at ISSN.org + "version": "Register" # this means the ISSN is registered at ISSN.org }, "subjectOf": [ { @@ -69,7 +75,7 @@ def setup(self) -> dict: eissn_data = ISSNOrgData({ "mainEntityOfPage": { - "version": "Pending" # this means the ISSN is not registered at ISSN.org + "version": "Pending" # this means the ISSN is not registered at ISSN.org }, "subjectOf": [ { @@ -100,13 +106,7 @@ def setup(self) -> dict: pissn_data, False) - acSvc = DOAJ.autochecksService( - autocheck_plugins=[ - # (journal, application, plugin) - (True, True, ISSNActive), - (True, True, KeepersRegistry) - ] - ) + acSvc = DOAJ.autochecksService() ac1 = acSvc.autocheck_application(ap) ################################################## @@ -132,7 +132,7 @@ def setup(self) -> dict: ISSNChecker.retrieve_from_source = lambda *args, **kwargs: ( eissn, "https://portal.issn.org/resource/ISSN/9999-000X", - None, # Don't pass in any data, so we get the Not Found response + None, # Don't pass in any data, so we get the Not Found response False, pissn, "https://portal.issn.org/resource/ISSN/2682-4396", diff --git a/portality/forms/application_forms.py b/portality/forms/application_forms.py index b370d29051..6f7c1d9b0b 100644 --- a/portality/forms/application_forms.py +++ b/portality/forms/application_forms.py @@ -1318,7 +1318,14 @@ class FieldDefinitions: ], "attr": { "class": "input-xlarge unstyled-list" - } + }, + "contexts": { + "admin": { + "widgets": [ + "autocheck", # ~~^-> Autocheck:FormWidget~~ + ] + } + }, } # ~~->$ PreservationServiceOther:FormField~~ @@ -1337,7 +1344,14 @@ class FieldDefinitions: ], "widgets" : [ "trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~ - ] + ], + "contexts": { + "admin": { + "widgets": [ + "autocheck", # ~~^-> Autocheck:FormWidget~~ + ] + } + }, } # ~~->$ PreservationServiceURL:FormField~~ @@ -1434,7 +1448,14 @@ class FieldDefinitions: ], "widgets" : [ "trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~ - ] + ], + "contexts": { + "admin": { + "widgets": [ + "autocheck", # ~~^-> Autocheck:FormWidget~~ + ] + } + }, } # ~~->$ DepositPolicyURL:FormField~~ @@ -1539,7 +1560,14 @@ class FieldDefinitions: ], "widgets" : [ "trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~ - ] + ], + "contexts": { + "admin": { + "widgets": [ + "autocheck", # ~~^-> Autocheck:FormWidget~~ + ] + } + }, } # ~~->$ Orcids:FormField~~ diff --git a/portality/static/js/autochecks.js b/portality/static/js/autochecks.js index 32da79de29..9ed20f2c27 100644 --- a/portality/static/js/autochecks.js +++ b/portality/static/js/autochecks.js @@ -25,14 +25,14 @@ doaj.autocheckers.ISSNActive = class { } draw(autocheck) { - let icon = this.ICONS[autocheck.advice]; let message = this.MESSAGES[autocheck.advice]; message = message.replace("{{ISSN}}", autocheck.original_value); - let style = this.STYLE[autocheck.advice]; + const frag = drawIconMessage( + autocheck, message, + this.ICONS[autocheck.advice], + this.STYLE[autocheck.advice]); - let frag = `
- ${message} (see record)
`; return frag; } } @@ -60,23 +60,66 @@ doaj.autocheckers.KeepersRegistry = class { } draw(autocheck) { - let icon = this.ICONS[autocheck.advice]; let message = this.MESSAGES[autocheck.advice]; let context = JSON.parse(autocheck.context); message = message.replace("{service}", context.service); - let style = this.STYLE[autocheck.advice]; + const frag = drawIconMessage( + autocheck, message, + this.ICONS[autocheck.advice], + this.STYLE[autocheck.advice]); - let frag = `
- ${message} (see record)
`; return frag; } } +doaj.autocheckers.NoNoneValue = class { + MESSAGES = { + "none_value_found": "This field should not contain the value 'None'.", + } + + ICONS = { + "none_value_found": "x-circle", + } + + STYLE = { + "none_value_found": "error", + } + + draw(autocheck) { + let message = this.MESSAGES[autocheck.advice]; + + const frag = drawIconMessage( + autocheck, message, + this.ICONS[autocheck.advice], + this.STYLE[autocheck.advice]); + + return frag; + } +} + +function drawIconMessage(autocheck, message, icon, style) { + let msg_reference_url = ''; + if (autocheck.reference_url) { + msg_reference_url = ` (see record)` + } + + const frag = ` +
+ + + + ${message} ${msg_reference_url} +
+ `; + return frag; +} + doaj.autocheckers.registry = { "issn_active": doaj.autocheckers.ISSNActive, - "keepers_registry": doaj.autocheckers.KeepersRegistry + "keepers_registry": doaj.autocheckers.KeepersRegistry, + "no_none_value": doaj.autocheckers.NoNoneValue, } doaj.autocheckers.AutochecksManager = class { From a66a213997de6b95c964da75afead30b6c64bb43 Mon Sep 17 00:00:00 2001 From: philip Date: Fri, 22 Mar 2024 15:51:36 +0000 Subject: [PATCH 4/6] update testbook autocheck --- doajtest/testbook/autocheck/autocheck.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doajtest/testbook/autocheck/autocheck.yml b/doajtest/testbook/autocheck/autocheck.yml index e13965b5ac..6310f06a6f 100644 --- a/doajtest/testbook/autocheck/autocheck.yml +++ b/doajtest/testbook/autocheck/autocheck.yml @@ -55,6 +55,14 @@ tests: - You are taken to the ISSN.org record. Note that for the purposes of this test, this is a random record on ISSN.org, unrelated to the actual record in DOAJ. - step: Close the ISSN.org window/tab and return to the application form + - step: Scroll to the "Other archiving policy" + results: + - Input box contain value "None" + - This field annotated with red cross, saying "None" value found + - step: Scroll to the "Name of other website where policy is registered" + results: + - Input box contain value "None" + - This field annotated with red cross, saying "None" value found - step: Scroll to the top of the application form results: - There is text which tells you when the Autochecks were made, and an option to Hide All Autochecks From 08018fbc24cc4cb74692ddc5461fb0f9fa887c57 Mon Sep 17 00:00:00 2001 From: philip Date: Fri, 19 Apr 2024 11:53:40 +0100 Subject: [PATCH 5/6] fix admin contexts --- portality/forms/application_forms.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/portality/forms/application_forms.py b/portality/forms/application_forms.py index 6f7c1d9b0b..26aa7e88d5 100644 --- a/portality/forms/application_forms.py +++ b/portality/forms/application_forms.py @@ -1323,6 +1323,8 @@ class FieldDefinitions: "admin": { "widgets": [ "autocheck", # ~~^-> Autocheck:FormWidget~~ + "trim_whitespace", # ~~^-> TrimWhitespace:FormWidget~~ + "multiple_field", ] } }, @@ -1349,6 +1351,7 @@ class FieldDefinitions: "admin": { "widgets": [ "autocheck", # ~~^-> Autocheck:FormWidget~~ + "trim_whitespace", # ~~^-> TrimWhitespace:FormWidget~~ ] } }, @@ -1453,6 +1456,7 @@ class FieldDefinitions: "admin": { "widgets": [ "autocheck", # ~~^-> Autocheck:FormWidget~~ + "trim_whitespace", # ~~^-> TrimWhitespace:FormWidget~~ ] } }, @@ -1565,6 +1569,7 @@ class FieldDefinitions: "admin": { "widgets": [ "autocheck", # ~~^-> Autocheck:FormWidget~~ + "trim_whitespace", # ~~^-> TrimWhitespace:FormWidget~~ ] } }, From e150da889bfa60de279fee8b0cc5c7ffc55af173 Mon Sep 17 00:00:00 2001 From: philip Date: Fri, 19 Apr 2024 11:57:37 +0100 Subject: [PATCH 6/6] add drawIconMessage to scope doaj.autocheckers --- portality/static/js/autochecks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/portality/static/js/autochecks.js b/portality/static/js/autochecks.js index 9ed20f2c27..af1f1b0d49 100644 --- a/portality/static/js/autochecks.js +++ b/portality/static/js/autochecks.js @@ -99,7 +99,7 @@ doaj.autocheckers.NoNoneValue = class { } } -function drawIconMessage(autocheck, message, icon, style) { +doaj.autocheckers.drawIconMessage = function(autocheck, message, icon, style) { let msg_reference_url = ''; if (autocheck.reference_url) { msg_reference_url = ` (see record)`