Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/3672 autocheck none value #2362

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open
6 changes: 2 additions & 4 deletions doajtest/helpers.py
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Expand Down
8 changes: 8 additions & 0 deletions doajtest/testbook/autocheck/autocheck.yml
Expand Up @@ -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
Expand Down
34 changes: 17 additions & 17 deletions 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:
Expand All @@ -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()
Expand All @@ -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": [
{
Expand All @@ -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": [
{
Expand Down Expand Up @@ -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)

##################################################
Expand All @@ -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",
Expand Down
81 changes: 81 additions & 0 deletions 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
5 changes: 5 additions & 0 deletions portality/app.py
Expand Up @@ -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'
Expand Down
38 changes: 38 additions & 0 deletions 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__,
)
7 changes: 4 additions & 3 deletions 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)
]
Expand Down
41 changes: 37 additions & 4 deletions portality/forms/application_forms.py
Expand Up @@ -1318,7 +1318,16 @@ class FieldDefinitions:
],
"attr": {
"class": "input-xlarge unstyled-list"
}
},
"contexts": {
"admin": {
"widgets": [
"autocheck", # ~~^-> Autocheck:FormWidget~~
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The context fields overwrite the base fields, so in this case this would remove the trim_whitespace and multiple_field widgets, and replace them with just the autocheck field. Instead, this should list all 3 required widgets.

"trim_whitespace", # ~~^-> TrimWhitespace:FormWidget~~
"multiple_field",
]
}
},
}

# ~~->$ PreservationServiceOther:FormField~~
Expand All @@ -1337,7 +1346,15 @@ class FieldDefinitions:
],
"widgets" : [
"trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~
]
],
"contexts": {
"admin": {
"widgets": [
"autocheck", # ~~^-> Autocheck:FormWidget~~
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above

"trim_whitespace", # ~~^-> TrimWhitespace:FormWidget~~
]
}
},
}

# ~~->$ PreservationServiceURL:FormField~~
Expand Down Expand Up @@ -1434,7 +1451,15 @@ class FieldDefinitions:
],
"widgets" : [
"trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~
]
],
"contexts": {
"admin": {
"widgets": [
"autocheck", # ~~^-> Autocheck:FormWidget~~
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above

"trim_whitespace", # ~~^-> TrimWhitespace:FormWidget~~
]
}
},
}

# ~~->$ DepositPolicyURL:FormField~~
Expand Down Expand Up @@ -1539,7 +1564,15 @@ class FieldDefinitions:
],
"widgets" : [
"trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~
]
],
"contexts": {
"admin": {
"widgets": [
"autocheck", # ~~^-> Autocheck:FormWidget~~
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above

"trim_whitespace", # ~~^-> TrimWhitespace:FormWidget~~
]
}
},
}

# ~~->$ Orcids:FormField~~
Expand Down