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 2 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
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)
125 changes: 90 additions & 35 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 blind peer review used.")
}} # ~~^-> ForbiddenWord:FormValidator~~
],
"widgets": [
"trim_whitespace" # ~~^-> TrimWhitespace:FormWidget~~
],
"asynchronous_warning": [
{"warn_on_value": {"value": "None"}}
]
}

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,32 @@ 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):
html_attrs["data-parsley-forbidden-word"] = settings.get("word", "")
if "message" in settings:
html_attrs["data-parsley-forbidden-word-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 +3308,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 +3337,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
36 changes: 36 additions & 0 deletions portality/forms/validate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,42 @@ def __call__(self, form, field):
raise validators.StopValidation(self.message.format(stop_word=v))


class NotValue(object):
"""
~~NotValue:FormValidator~~
Rejects if the field value equals a forbidden string (case-insensitive).
"""
def __init__(self, value, message=None):
self.forbidden = value
if not message:
message = "'{value}' is not a valid answer for this question. Leave blank.".format(value=value)
self.message = message

def __call__(self, form, field):
if self.forbidden is None:
return
if field.data and field.data.strip().lower() == self.forbidden.strip().lower():
raise validators.ValidationError(self.message)


class ForbiddenWord(object):
"""
~~ForbiddenWord:FormValidator~~
Rejects if the field value contains a forbidden word (case-insensitive).
"""
def __init__(self, word, message=None):
self.word = word
if not message:
message = "You may not enter '{word}' in this field".format(word=word)
self.message = message

def __call__(self, form, field):
if self.word is None:
return
if field.data and self.word.lower() in field.data.lower():
raise validators.ValidationError(self.message)


class DifferentTo(MultiFieldValidator):
"""
~~DifferentTo:FormValidator~~
Expand Down
Loading
Loading