Skip to content

Publisher application form: prevent 'blind' and 'none' being entered into the form #2589

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.

Sign up for GitHub

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

Open
varadekd wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Publisher application form: prevent 'blind' and 'none' being entered into the form #2589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1e5077a
Blind and None are added in validation
varadekd May 1, 2026
44bbebb
Migrations added
varadekd May 1, 2026
b242806
Merge branch 'develop' into feature/4232_prevent_blind_none
richard-jones May 14, 2026
f3d11e0
added functional test and slight wording change
richard-jones May 18, 2026
e96d6b5
Ensured all the fields are tested and reverfied the feedback provided.
varadekd Jun 15, 2026
d57a91f
Merge branch 'feature/4232_prevent_blind_none' of github.com:DOAJ/doa…
varadekd Jun 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions doajtest/testbook/new_application_form/publishers_form.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,35 @@ tests:
- step: Check the consent box and click Submit
results:
- The application submits and you are shown the confirmation page

- title: Disallowed values
context:
role: publisher
steps:
- step: Go to the application form
path: /en/apply
- step: Skip to section 2 (About) using the progress bar
- step: In the "Other organisation's name" field, enter the value "None"
- step: Click "Next" at the bottom of the page
results:
- There is an error by the "Other organisation's name" field that says the value is not permitted
- step: Skip to section 4 (Editorial) using the progress bar
- step: Select "Other" from the Peer Review checkboxes
- step: In the "Other peer review" field, enter the value "None"
- step: Click "Next" at the bottom of the page
results:
- There is an error by the "Other peer review" field that says the value is not permitted
- step: Go back to the "Other peer review" field and change the text value to "blind peer review" and click Next
- step: Click "Next" at the bottom of the page
results:
- There is an error by the "Other peer review" field that asks you to use a structured option instead
- step: Skip to section 6 (Best practice) using the progress bar
- step: Select "A National Library" and "Other" from the Archiving services checkboxes
- step: Enter "None" in both boxes that appear
- step: Select "Other" from the Repository policy checkboxes
- step: Enter "None" in the "Other repository policy" field
- step: Select "Other" from the Persistent identifier checkboxes
- step: Enter "None" in the "Other identifier" field
- step: Click "Next" at the bottom of the page
results:
- There is an error by all relevant fields that says the value is not permitted
124 changes: 123 additions & 1 deletion doajtest/unit/test_form_validate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from doajtest.helpers import DoajTestCase
from werkzeug.datastructures import MultiDict
from portality.forms.application_forms import ApplicationFormFactory, STOP_WORDS
from portality.forms.validate import NotValue, ForbiddenWord

class TestFormValidate(DoajTestCase):
def setUp(self):
Expand Down Expand Up @@ -87,4 +88,125 @@ def test_05_required_if(self):
valid = wtf.validate()

pidof = wtf.persistent_identifiers_other
assert len(pidof.errors) == 0
assert len(pidof.errors) == 0

def test_06_not_value(self):
# "None" in review_process_other triggers error
formdata = MultiDict({
"review_process": "other",
"review_process_other": "None"
})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
rpo = wtf.review_process_other
assert any("not a valid answer" in e for e in rpo.errors)

# Case-insensitive: "none" also triggers error
formdata = MultiDict({
"review_process": "other",
"review_process_other": "none"
})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
rpo = wtf.review_process_other
assert any("not a valid answer" in e for e in rpo.errors)

# A valid value passes
formdata = MultiDict({
"review_process": "other",
"review_process_other": "Registered reports"
})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
rpo = wtf.review_process_other
assert not any("not a valid answer" in e for e in rpo.errors)

# Empty value passes
formdata = MultiDict({})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
rpo = wtf.review_process_other
assert not any("not a valid answer" in e for e in rpo.errors)

# Validator with None forbidden-value is a no-op (defensive guard)
validator = NotValue(None)
class FakeField:
data = "None"
errors = []
validator(None, FakeField()) # should not raise

def test_07_forbidden_word(self):
# "blind" in review_process_other triggers error
formdata = MultiDict({
"review_process": "other",
"review_process_other": "blind peer review"
})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
rpo = wtf.review_process_other
assert any("structured options" in e for e in rpo.errors)

# Case-insensitive: "Double Blind" also triggers
formdata = MultiDict({
"review_process": "other",
"review_process_other": "Double Blind"
})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
rpo = wtf.review_process_other
assert any("structured options" in e for e in rpo.errors)

# Value without "blind" passes
formdata = MultiDict({
"review_process": "other",
"review_process_other": "Open review"
})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
rpo = wtf.review_process_other
assert not any("structured options" in e for e in rpo.errors)

# Empty value passes
formdata = MultiDict({})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
rpo = wtf.review_process_other
assert not any("structured options" in e for e in rpo.errors)

# Validator with None word is a no-op (defensive guard)
validator = ForbiddenWord(None)
class FakeField:
data = "blind review"
errors = []
validator(None, FakeField()) # should not raise

def test_08_not_value_on_other_fields(self):
# "None" in persistent_identifiers_other triggers error when other is selected
formdata = MultiDict({
"persistent_identifiers": "other",
"persistent_identifiers_other": "None"
})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
pido = wtf.persistent_identifiers_other
assert any("not a valid answer" in e for e in pido.errors)

# "None" in deposit_policy_other triggers error when other is selected
formdata = MultiDict({
"deposit_policy": "other",
"deposit_policy_other": "None"
})
pap = ApplicationFormFactory.context("public")
wtf = pap.wtform(formdata)
wtf.validate()
dpo = wtf.deposit_policy_other
assert any("not a valid answer" in e for e in dpo.errors)
131 changes: 95 additions & 36 deletions portality/forms/application_forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@
Year,
CurrentISOCurrency,
CurrentISOLanguage,
DateInThePast
DateInThePast,
NotValue,
ForbiddenWord
)
from portality.lib import dates
from portality.lib.formulaic import Formulaic, WTFormsBuilder, FormulaicContext, FormulaicField
Expand Down Expand Up @@ -608,18 +610,34 @@ class FieldDefinitions:
# ~~^-> Autocomplete:FormWidget~~
"full_contents" # ~~^->FullContents:FormWidget~~
],
"validate": [
{"not_value": {
"value": "None",
"message": lazy_gettext("'None' is not a valid answer for this question. Leave blank.")
}} # ~~^-> NotValue:FormValidator~~
],
"contexts": {
"public": {
"validate": [{"different_to": {"field": "publisher_name",
"message": lazy_gettext("The Publisher's name and Other organisation's name cannot be the same.")}}]
# ~~^-> DifferetTo:FormValidator~~

"validate": [
{"different_to": {"field": "publisher_name",
"message": lazy_gettext("The Publisher's name and Other organisation's name cannot be the same.")}},
# ~~^-> DifferetTo:FormValidator~~
{"not_value": {
"value": "None",
"message": lazy_gettext("'None' is not a valid answer for this question. Leave blank.")
}} # ~~^-> NotValue:FormValidator~~
]
},
"update_request": {
"validate": [{"different_to": {"field": "publisher_name",
"message": lazy_gettext("The Publisher's name and Other organisation's name cannot be the same.")}}]
# ~~^-> DifferetTo:FormValidator~~

"validate": [
{"different_to": {"field": "publisher_name",
"message": lazy_gettext("The Publisher's name and Other organisation's name cannot be the same.")}},
# ~~^-> DifferetTo:FormValidator~~
{"not_value": {
"value": "None",
"message": lazy_gettext("'None' is not a valid answer for this question. Leave blank.")
}} # ~~^-> NotValue:FormValidator~~
]
},
"admin": {
"widgets": [
Expand Down Expand Up @@ -892,9 +910,9 @@ class FieldDefinitions:
"multiple": True,
"options": [
{"display": lazy_gettext("Editorial review"), "value": "Editorial review"},
{"display": lazy_gettext("Peer review"), "value": "Peer review"},
{"display": lazy_gettext("Anonymous peer review"), "value": "Anonymous peer review"},
{"display": lazy_gettext("Single anonymous peer review"), "value": "Single anonymous peer review"},
{"display": lazy_gettext("Double anonymous peer review"), "value": "Double anonymous peer review"},
{"display": lazy_gettext("Triple anonymous peer review"), "value": "Triple anonymous peer review"},
{"display": lazy_gettext("Post-publication peer review"), "value": "Post-publication peer review"},
{"display": lazy_gettext("Open peer review"), "value": "Open peer review"},
{"display": lazy_gettext("Other"), "value": "other", "subfields": ["review_process_other"]}
Expand Down Expand Up @@ -926,14 +944,18 @@ class FieldDefinitions:
"field": "review_process",
"value": "other",
"message": lazy_gettext("Enter the name of another type of peer review")
}
}
}},
{"not_value": {
"value": "None",
"message": lazy_gettext("'None' is not a valid answer for this question. Leave blank.")
}}, # ~~^-> NotValue:FormValidator~~
{"forbidden_word": {
"word": "blind",
"message": lazy_gettext("Please use the structured options above to indicate the type of peer review used.")
}} # ~~^-> ForbiddenWord:FormValidator~~
],
"widgets": [
"trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~
],
"asynchronous_warning": [
{"warn_on_value": {"value": "None"}}
]
}

Expand Down Expand Up @@ -994,7 +1016,7 @@ class FieldDefinitions:
"services(s) used on your website.")],
},
"options": [
{"display": lazy_gettext("Yes"), "value": "y", "subfields": ["review_process_other"]},
{"display": lazy_gettext("Yes"), "value": "y"},
{"display": lazy_gettext("No"), "value": "n"}
],
"validate": [
Expand Down Expand Up @@ -1389,8 +1411,11 @@ class FieldDefinitions:
"field": "preservation_service",
"value": "national_library",
"message": lazy_gettext("Enter the name(s) of the national library or libraries where the journal is archived")
}
}
}},
{"not_value": {
"value": "None",
"message": lazy_gettext("'None' is not a valid answer for this question. Leave blank.")
}} # ~~^-> NotValue:FormValidator~~

@richard-jones richard-jones May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For reasons that aren't clear to me, this validation isn't working client-side. Possibly it's interacting with the multiple_field widget?

],
"widgets": [
"trim_whitespace", # ~~^-> TrimWhitespace:FormWidget~~
Expand All @@ -1412,11 +1437,11 @@ class FieldDefinitions:
"field": "preservation_service",
"value": "other",
"message": lazy_gettext("Enter the name of another archiving policy")
}
}
],
"asynchronous_warning": [
{"warn_on_value": {"value": "None"}}
}},
{"not_value": {
"value": "None",
"message": lazy_gettext("'None' is not a valid answer for this question. Leave blank.")
}} # ~~^-> NotValue:FormValidator~~
],
"widgets": [
"trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~
Expand Down Expand Up @@ -1512,11 +1537,11 @@ class FieldDefinitions:
"field": "deposit_policy",
"value": "other",
"message": lazy_gettext("Enter the name of another repository policy")
}
}
],
"asynchronous_warning": [
{"warn_on_value": {"value": "None"}}
}},
{"not_value": {
"value": "None",
"message": lazy_gettext("'None' is not a valid answer for this question. Leave blank.")
}} # ~~^-> NotValue:FormValidator~~
],
"widgets": [
"trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~
Expand Down Expand Up @@ -1621,11 +1646,11 @@ class FieldDefinitions:
"field": "persistent_identifiers",
"value": "other",
"message": lazy_gettext("Enter the name of another type of identifier")
}
}
],
"asynchronous_warning": [
{"warn_on_value": {"value": "None"}}
}},
{"not_value": {
"value": "None",
"message": lazy_gettext("'None' is not a valid answer for this question. Leave blank.")
}} # ~~^-> NotValue:FormValidator~~
],
"widgets": [
"trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~
Expand Down Expand Up @@ -3216,6 +3241,36 @@ def wtforms(field, settings):
return CurrentISOLanguage(settings.get("message"))


class NotValueBuilder:
# ~~->$ NotValue:FormValidator~~
@staticmethod
def render(settings, html_attrs):
html_attrs["data-parsley-not-value"] = settings.get("value", "")
if "message" in settings:
html_attrs["data-parsley-not-value-message"] = "<p><small>" + settings["message"] + "</small></p>"

@staticmethod
def wtforms(field, settings):
return NotValue(settings.get("value"), settings.get("message"))


class ForbiddenWordBuilder:
# ~~->$ ForbiddenWord:FormValidator~~
@staticmethod
def render(settings, html_attrs):
word = settings.get("word", "")
if word:
# Use Parsley's built-in pattern validator with a case-insensitive negative lookahead.
# The /i flag is picked up by Parsley's regexp parser via the /pattern/i literal format.
html_attrs["data-parsley-pattern"] = "/^(?!.*" + word + ").*$/i"
if "message" in settings and word:
html_attrs["data-parsley-pattern-message"] = "<p><small>" + settings["message"] + "</small></p>"

@staticmethod
def wtforms(field, settings):
return ForbiddenWord(settings.get("word"), settings.get("message"))


#########################################################
# Crosswalks
#########################################################
Expand Down Expand Up @@ -3257,7 +3312,9 @@ def wtforms(field, settings):
"bigenddate": BigEndDateBuilder.render,
"no_script_tag": NoScriptTagBuilder.render,
"year": YearBuilder.render,
"date_in_the_past": DateInThePastBuilder.render
"date_in_the_past": DateInThePastBuilder.render,
"not_value": NotValueBuilder.render,
"forbidden_word": ForbiddenWordBuilder.render
},
"wtforms": {
"required": RequiredBuilder.wtforms,
Expand All @@ -3284,7 +3341,9 @@ def wtforms(field, settings):
"year": YearBuilder.wtforms,
"current_iso_currency": CurrentISOCurrencyBuilder.wtforms,
"current_iso_language": CurrentISOLanguageBuilder.wtforms,
"date_in_the_past": DateInThePastBuilder.wtforms
"date_in_the_past": DateInThePastBuilder.wtforms,
"not_value": NotValueBuilder.wtforms,
"forbidden_word": ForbiddenWordBuilder.wtforms
}
}
}
Expand Down
Loading
Loading