From f5a07604e54a7721bd2bb4147cc6669df197d323 Mon Sep 17 00:00:00 2001 From: Dale Wahl Date: Fri, 17 Jul 2026 09:17:08 +0200 Subject: [PATCH 01/15] processors add validate_query --- processors/machine_learning/audio_to_text.py | 33 +++++++++++++++-- processors/text-analysis/topic_modeling.py | 28 +++++++++++++-- processors/visualisation/word-cloud.py | 37 +++++++++++++++++--- 3 files changed, 89 insertions(+), 9 deletions(-) diff --git a/processors/machine_learning/audio_to_text.py b/processors/machine_learning/audio_to_text.py index ed4078ba0..ed43d74cc 100644 --- a/processors/machine_learning/audio_to_text.py +++ b/processors/machine_learning/audio_to_text.py @@ -9,7 +9,8 @@ from backend.lib.processor import BasicProcessor from common.lib.compatibility import Compatibility from common.lib.dmi_service_manager import DmiServiceManager, DmiServiceManagerException, DsmOutOfMemory -from common.lib.exceptions import ProcessorInterruptedException +from common.lib.exceptions import ProcessorInterruptedException, QueryParametersException, \ + QueryNeedsExplicitConfirmationException from common.lib.user_input import UserInput from common.lib.item_mapping import MappedItem @@ -260,6 +261,32 @@ def get_options(cls, parent_dataset=None, config=None): return options + @staticmethod + def validate_query(query, request, config): + """ + Check OpenAI requirements before the job is queued + + The API key field is only part of the form when no site-wide key is + configured, so when it is part of the submission the user has to fill + it in. Sending audio to OpenAI also shares data with a third party + and is likely to cost money, so ask for an explicit confirmation. + + :param dict query: Query parameters, from client-side. + :param request: Flask request + :param ConfigManager|None config: Configuration reader (context-aware) + :return dict: Safe query parameters + """ + if query.get("model_host") == "openai": + if "api_key" in query and not (query["api_key"] or "").strip(): + raise QueryParametersException("You need to provide an OpenAI API key to use OpenAI models.") + + if not query.get("frontend-confirm"): + raise QueryNeedsExplicitConfirmationException( + "Your audio files will be sent to OpenAI for processing. This shares your data with a third " + "party and is likely to incur costs. Do you want to continue?") + + return query + def process(self): """ This takes a zipped set of audio files and uses a Whisper docker image to identify speech and convert to text, @@ -318,7 +345,9 @@ def process(self): return else: - # Check for API key if using OpenAI models + # the key is also checked when the job is queued, but jobs started + # by presets and `next` steps skip that check, and a site-wide key + # can be removed between queueing and running api_key = self.parameters.get("api_key") if not api_key: api_key = self.config.get("api.openai.api_key") diff --git a/processors/text-analysis/topic_modeling.py b/processors/text-analysis/topic_modeling.py index d9070ab67..94ca5dae0 100644 --- a/processors/text-analysis/topic_modeling.py +++ b/processors/text-analysis/topic_modeling.py @@ -5,7 +5,7 @@ from common.lib.helpers import UserInput from backend.lib.processor import BasicProcessor from common.lib.compatibility import Compatibility -from common.lib.exceptions import ProcessorInterruptedException +from common.lib.exceptions import ProcessorInterruptedException, QueryParametersException import json import pickle @@ -80,7 +80,7 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: }, "max_df": { "type": UserInput.OPTION_TEXT, - "min": 0.0, + "min": 0.0, # real min is > 0 but 0.00000000...000001?; we'll check for that in validate_query() "max": 1.0, "default": 0.8, "help": "Maximum document frequency", @@ -88,6 +88,30 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: } } + @staticmethod + def validate_query(query, request, config): + """ + Check that the document frequency bounds work together + + Both bounds are already limited to numbers between 0 and 1 when the + form input is parsed, but they can still contradict each other, which + would otherwise only surface as an error halfway through the run. + + :param dict query: Query parameters, from client-side. + :param request: Flask request + :param ConfigManager|None config: Configuration reader (context-aware) + :return dict: Safe query parameters + """ + if query.get("max_df") == 0: + raise QueryParametersException("The maximum document frequency must be above zero, or every token " + "would be ignored.") + + if "min_df" in query and "max_df" in query and query["min_df"] > query["max_df"]: + raise QueryParametersException("The minimum document frequency cannot be higher than the maximum " + "document frequency, or every token would be ignored.") + + return query + def process(self): """ Unzips token sets and builds topic models for each one. Model data is diff --git a/processors/visualisation/word-cloud.py b/processors/visualisation/word-cloud.py index be922a0a5..c9fb9db3d 100644 --- a/processors/visualisation/word-cloud.py +++ b/processors/visualisation/word-cloud.py @@ -7,7 +7,8 @@ from backend.lib.processor import BasicProcessor from common.lib.compatibility import Compatibility -from common.lib.helpers import UserInput +from common.lib.exceptions import QueryParametersException +from common.lib.helpers import UserInput, convert_to_int __author__ = "Sal Hagen" __credits__ = ["Sal Hagen"] @@ -62,11 +63,35 @@ def get_options(cls, parent_dataset=None, config=None): "max_words": { "type": UserInput.OPTION_TEXT, "default": 200, + "min": 1, "help": "Max words to show" } } return options + @staticmethod + def validate_query(query, request, config): + """ + Check that a word and count column were chosen + + The column choices default to an empty value when the parent dataset + has no obvious word and count columns, so the form can be submitted + with an empty choice. Catch that here so the user can fix it in the + form, instead of the processor failing after it has started. + + :param dict query: Query parameters, from client-side. + :param request: Flask request + :param ConfigManager|None config: Configuration reader (context-aware) + :return dict: Safe query parameters + """ + if "word_column" in query and not query.get("word_column"): + raise QueryParametersException("Select the column containing the words for the word cloud.") + + if "count_column" in query and not query.get("count_column"): + raise QueryParametersException("Select the column containing the word counts.") + + return query + def process(self): """ Render an SVG histogram/bar chart using a previous frequency analysis @@ -78,13 +103,15 @@ def process(self): word_column = self.parameters.get("word_column") count_column = self.parameters.get("count_column") to_lower = self.parameters.get("to_lower") - try: - max_words = int(self.parameters.get("max_words")) - except (ValueError, TypeError): - max_words = self.parameters["max_words"]["default"] + # form input is checked when the job is queued, but presets and other + # code can start this processor with unchecked values, so make sure + # the amount is a usable number here as well + max_words = max(1, convert_to_int(self.parameters.get("max_words"), 200)) self.dataset.update_status("Extracting words and counts.") + # also checked when the job is queued, but kept for jobs started by + # presets and other code, which skip that check if not word_column: self.dataset.finish_with_error("Please set a valid word column.") return From be47d12c5c7204efee37ebf2055d067377c28127 Mon Sep 17 00:00:00 2001 From: Dale Wahl Date: Fri, 17 Jul 2026 11:06:55 +0200 Subject: [PATCH 02/15] add comments to all known option keys --- common/lib/user_input.py | 47 +++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/common/lib/user_input.py b/common/lib/user_input.py index 8f02655fc..43d6fa1e6 100644 --- a/common/lib/user_input.py +++ b/common/lib/user_input.py @@ -48,16 +48,43 @@ class UserInput: # "coerce_type"), which is silently ignored and so has no effect - the # module test flags these so they are caught rather than shipped. KNOWN_OPTION_KEYS = frozenset({ - # read by the parser / module loader - "type", "default", "options", "requires", "min", "max", "coerce_type", - "indirect", "delegated", "dict_key", "columns", "sensitive", - # read by the interface templates - "help", "tooltip", "label", "inline", "original_default", "value", - "saturation", "accept", "update", "password", "multiple", "cache", - # annotation options - "to_parent", "hide_in_explorer", - # datasource options - "board_specific", + # -- read by the parser / module loader -- + "type", # UI/control type of the option (toggle, choice, string, ...) + "default", # value used when the option is not present in submitted input + "options", # valid choices for choice/multi/annotation options + "requires", # condition(s) controlling when the option is shown/parsed + "min", # minimum allowed value for numeric text options + "max", # maximum allowed value for numeric text options + "coerce_type", # Python type (int, float) the parsed value is coerced to + "indirect", # derived from other settings; skipped during parsing + "delegated", # shown/parsed only when dynamically delegated by another option + "dict_key", # sub-field or callable used as key for multi_option items + "columns", # sub-option definitions for table-style options + "sensitive", # treated as sensitive; excluded from public displays/logs and deleted on run + + # -- read by the interface templates -- + "help", # primary label/help text shown next to the form control + "tooltip", # longer explanatory text shown as tooltip or placeholder + "label", # display label for annotation badges + "inline", # render multi/multi_select choices inline + "original_default", # configured default preserved when the live value is overridden (settings panel) + "value", # HSV lightness/value component for hue colour pickers + "saturation", # HSV saturation component for hue colour pickers + "accept", # accepted file type for file inputs + "update", # CSS selector to update with hue picker changes + "password", # render string input as a password field + "multiple", # allow selecting multiple files + "cache", # tell the frontend to cache the value locally + + # -- annotation options -- + "to_parent", # attach annotations to the parent dataset + "hide_in_explorer", # hide annotation column from the Explorer UI + + # -- datasource options -- + "board_specific", # list of board IDs for which the option is shown + + # -- config/admin settings -- + "global", # setting applies across all users (not user-scoped) }) @staticmethod From 45e0931473973c7650c6d453b267a4a672a3fb9b Mon Sep 17 00:00:00 2001 From: Dale Wahl Date: Fri, 17 Jul 2026 11:29:54 +0200 Subject: [PATCH 03/15] add mandatory user option flag/tag --- common/lib/user_input.py | 33 +++++ tests/test_user_input_mandatory.py | 212 +++++++++++++++++++++++++++++ 2 files changed, 245 insertions(+) create mode 100644 tests/test_user_input_mandatory.py diff --git a/common/lib/user_input.py b/common/lib/user_input.py index 43d6fa1e6..692af5962 100644 --- a/common/lib/user_input.py +++ b/common/lib/user_input.py @@ -61,6 +61,7 @@ class UserInput: "dict_key", # sub-field or callable used as key for multi_option items "columns", # sub-option definitions for table-style options "sensitive", # treated as sensitive; excluded from public displays/logs and deleted on run + "mandatory", # user-submitted value must not be blank/empty # -- read by the interface templates -- "help", # primary label/help text shown next to the form control @@ -206,6 +207,9 @@ def parse_all(options, input, silently_correct=False, log=None): if option_max not in input or input.get(option_max) == "-1": option_max += "_proxy" + if settings.get("mandatory") and UserInput.is_empty_value(input.get(option_min)) and UserInput.is_empty_value(input.get(option_max)): + raise QueryParametersException("%s: This field is required." % (settings.get("help") or option)) + # save as a tuple of unix timestamps (or None) try: after, before = (UserInput.parse_value(settings, input.get(option_min), parsed_input, silently_correct), UserInput.parse_value(settings, input.get(option_max), parsed_input, silently_correct)) @@ -279,6 +283,10 @@ def parse_all(options, input, silently_correct=False, log=None): # i.e. forms within forms!!! item_options = settings["options"] input_items = {} + + if settings.get("mandatory") and not any(re.match(f"{option}-([0-9]+)-(.+)", key) for key in input): + raise QueryParametersException("%s: This field is required." % (settings.get("help") or option)) + for key, value in input.items(): if key_match := re.match(f"{option}-([0-9]+)-(.+)", key): input_index = int(key_match[1]) @@ -319,6 +327,9 @@ def parse_all(options, input, silently_correct=False, log=None): else: # normal parsing and sanitisation try: + if settings.get("mandatory") and UserInput.is_empty_value(input[option]): + raise QueryParametersException("This field is required.") + parsed_input[option] = UserInput.parse_value(settings, input[option], parsed_input, silently_correct) except RequirementsNotMetException: pass @@ -329,6 +340,26 @@ def parse_all(options, input, silently_correct=False, log=None): return parsed_input + @staticmethod + def is_empty_value(value): + """ + Check whether a raw submitted value counts as blank for mandatory fields + + None, empty strings, and empty lists are treated as blank. Whitespace- + only strings are also considered blank. This is intentionally lenient + for other types; parse_value handles detailed validation. + + :param value: Raw submitted value + :return bool: True if the value is blank + """ + if value is None: + return True + if isinstance(value, str) and value.strip() == "": + return True + if isinstance(value, list) and len(value) == 0: + return True + return False + @staticmethod def requirements_met(requires, other_input): """ @@ -519,6 +550,8 @@ def parse_value(settings, choice, other_input=None, silently_correct=False): # callers), so accept both shapes. (OPTION_MULTI used to assume a # string and crashed on a real list.) if not choice: + if settings.get("mandatory"): + raise QueryParametersException("This field is required.") return settings.get("default", []) if type(choice) is str: diff --git a/tests/test_user_input_mandatory.py b/tests/test_user_input_mandatory.py new file mode 100644 index 000000000..92a082b60 --- /dev/null +++ b/tests/test_user_input_mandatory.py @@ -0,0 +1,212 @@ +""" +Tests for the UserInput.mandatory option setting. + +This validates that options marked as mandatory raise QueryParametersException +when the user submits an empty value, while still allowing absent/null defaults +and non-empty values. +""" + +import pytest + +from common.lib.user_input import UserInput +from common.lib.exceptions import QueryParametersException + + +class TestMandatoryOption: + """ + Test the mandatory option key for a variety of input types. + """ + + def test_mandatory_text_raises_when_empty(self): + """ + A mandatory string option with an empty submitted value should raise. + """ + options = { + "query": { + "type": UserInput.OPTION_TEXT, + "help": "Search query", + "mandatory": True, + } + } + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, {"query": ""}) + + def test_mandatory_text_raises_when_whitespace_only(self): + """ + Whitespace-only input should be treated as empty for mandatory fields. + """ + options = { + "query": { + "type": UserInput.OPTION_TEXT, + "help": "Search query", + "mandatory": True, + } + } + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, {"query": " "}) + + def test_mandatory_text_accepts_value(self): + """ + A mandatory text option with a non-empty value should parse normally. + """ + options = { + "query": { + "type": UserInput.OPTION_TEXT, + "help": "Search query", + "mandatory": True, + } + } + parsed = UserInput.parse_all(options, {"query": "hello"}) + assert parsed == {"query": "hello"} + + def test_mandatory_text_missing_default_does_not_raise(self): + """ + If the option was never submitted and has no default, mandatory should + not raise. + """ + options = { + "query": { + "type": UserInput.OPTION_TEXT, + "help": "Search query", + "mandatory": True, + } + } + parsed = UserInput.parse_all(options, {}) + assert parsed == {"query": None} + + def test_mandatory_text_with_default_missing_input_does_not_raise(self): + """ + If the option was not submitted but has a default, the default is used. + """ + options = { + "query": { + "type": UserInput.OPTION_TEXT, + "help": "Search query", + "default": "default query", + "mandatory": True, + } + } + parsed = UserInput.parse_all(options, {}) + assert parsed == {"query": "default query"} + + def test_non_mandatory_text_empty_is_allowed(self): + """ + Without mandatory=True, an empty value should fall back to the default. + """ + options = { + "query": { + "type": UserInput.OPTION_TEXT, + "help": "Search query", + "default": "default query", + } + } + parsed = UserInput.parse_all(options, {"query": ""}) + assert parsed == {"query": "default query"} + + def test_mandatory_multi_raises_when_empty(self): + """ + A mandatory multi-select with no selected values should raise. + """ + options = { + "boards": { + "type": UserInput.OPTION_MULTI, + "help": "Boards", + "options": {"pol": "/pol/", "v": "/v/"}, + "mandatory": True, + } + } + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, {"boards": ""}) + + def test_mandatory_multi_accepts_value(self): + """ + A mandatory multi-select with at least one selected value should parse. + """ + options = { + "boards": { + "type": UserInput.OPTION_MULTI, + "help": "Boards", + "options": {"pol": "/pol/", "v": "/v/"}, + "mandatory": True, + } + } + parsed = UserInput.parse_all(options, {"boards": "pol"}) + assert parsed == {"boards": ["pol"]} + + def test_mandatory_daterange_raises_when_both_empty(self): + """ + A mandatory date range with neither bound provided should raise. + """ + options = { + "daterange": { + "type": UserInput.OPTION_DATERANGE, + "help": "Date range", + "mandatory": True, + } + } + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, { + "daterange-min": "", + "daterange-max": "", + }) + + def test_mandatory_daterange_accepts_partial_range(self): + """ + A mandatory date range with only one bound should parse that bound. + """ + options = { + "daterange": { + "type": UserInput.OPTION_DATERANGE, + "help": "Date range", + "mandatory": True, + } + } + parsed = UserInput.parse_all(options, { + "daterange-min": "2024-01-01", + "daterange-max": "", + }) + assert parsed["daterange"][0] is not None + assert parsed["daterange"][1] is None + + def test_mandatory_multi_option_raises_when_empty(self): + """ + A mandatory multi_option with no submitted sub-items should raise. + """ + options = { + "filters": { + "type": UserInput.OPTION_MULTI_OPTION, + "help": "Filters", + "mandatory": True, + "options": { + "column": { + "type": UserInput.OPTION_TEXT, + "default": "", + "help": "Column", + } + } + } + } + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, {}) + + def test_mandatory_multi_option_accepts_item(self): + """ + A mandatory multi_option with at least one submitted item should parse. + """ + options = { + "filters": { + "type": UserInput.OPTION_MULTI_OPTION, + "help": "Filters", + "mandatory": True, + "options": { + "column": { + "type": UserInput.OPTION_TEXT, + "default": "", + "help": "Column", + } + } + } + } + parsed = UserInput.parse_all(options, {"filters-1-column": "body"}) + assert len(parsed["filters"]) == 1 + assert parsed["filters"][0]["column"] == "body" From bd423210d6fca1c3f0f80ba9c673861ace1a48e3 Mon Sep 17 00:00:00 2001 From: Dale Wahl Date: Fri, 17 Jul 2026 13:03:55 +0200 Subject: [PATCH 04/15] options mandatory check: add missing multi-select and update test --- common/lib/user_input.py | 8 ++++++++ tests/test_user_input_mandatory.py | 31 +++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/common/lib/user_input.py b/common/lib/user_input.py index 692af5962..2b5cbf17c 100644 --- a/common/lib/user_input.py +++ b/common/lib/user_input.py @@ -324,6 +324,14 @@ def parse_all(options, input, silently_correct=False, log=None): # about above, but is not silently replaced with something else) parsed_input[option] = settings.get("default", None) + # a mandatory option that was not submitted at all is only + # satisfied if its default fills it in. this is the shape an + # unselected multi-select arrives in: like an unchecked + # checkbox, the form leaves the field out altogether rather + # than sending it empty + if settings.get("mandatory") and UserInput.is_empty_value(parsed_input[option]): + raise QueryParametersException("%s: This field is required." % (settings.get("help") or option)) + else: # normal parsing and sanitisation try: diff --git a/tests/test_user_input_mandatory.py b/tests/test_user_input_mandatory.py index 92a082b60..7d8aca844 100644 --- a/tests/test_user_input_mandatory.py +++ b/tests/test_user_input_mandatory.py @@ -59,10 +59,12 @@ def test_mandatory_text_accepts_value(self): parsed = UserInput.parse_all(options, {"query": "hello"}) assert parsed == {"query": "hello"} - def test_mandatory_text_missing_default_does_not_raise(self): + def test_mandatory_text_missing_raises(self): """ - If the option was never submitted and has no default, mandatory should - not raise. + A mandatory option that was not submitted at all, and whose default + does not fill it in, is not satisfied. This is how an API caller + omitting a field arrives, and it matches what a mandatory date range + or multi_option already does when absent. """ options = { "query": { @@ -71,8 +73,8 @@ def test_mandatory_text_missing_default_does_not_raise(self): "mandatory": True, } } - parsed = UserInput.parse_all(options, {}) - assert parsed == {"query": None} + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, {}) def test_mandatory_text_with_default_missing_input_does_not_raise(self): """ @@ -118,6 +120,25 @@ def test_mandatory_multi_raises_when_empty(self): with pytest.raises(QueryParametersException): UserInput.parse_all(options, {"boards": ""}) + def test_mandatory_multi_raises_when_nothing_selected(self): + """ + When a user selects nothing in a multi-select, the form leaves the + field out of the submission altogether rather than sending it empty - + the same way an unchecked checkbox behaves. That absent shape, not an + empty value, is what actually reaches us, so it is the one that has to + be caught. + """ + options = { + "boards": { + "type": UserInput.OPTION_MULTI, + "help": "Boards", + "options": {"pol": "/pol/", "v": "/v/"}, + "mandatory": True, + } + } + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, {}) + def test_mandatory_multi_accepts_value(self): """ A mandatory multi-select with at least one selected value should parse. From 85a7103df48029206d97ea17934167b6c91045ff Mon Sep 17 00:00:00 2001 From: Dale Wahl Date: Fri, 17 Jul 2026 13:39:38 +0200 Subject: [PATCH 05/15] processors: use mandatory flag for options --- processors/conversion/convert_text.py | 1 + processors/conversion/merge_datasets.py | 1 + processors/filtering/lexical_filter.py | 1 + processors/machine_learning/audio_to_text.py | 27 ++++++++----------- processors/machine_learning/clarifai_api.py | 1 + .../clip_categorize_images.py | 1 + .../machine_learning/google_vision_api.py | 1 + processors/machine_learning/perspective.py | 3 ++- processors/metrics/youtube_metadata.py | 3 ++- processors/presets/annotate-images.py | 1 + processors/text-analysis/similar_words.py | 1 + processors/visualisation/histwords.py | 1 + processors/visualisation/word-trees.py | 19 +------------ .../visualisation/youtube_thumbnails.py | 3 ++- 14 files changed, 27 insertions(+), 37 deletions(-) diff --git a/processors/conversion/convert_text.py b/processors/conversion/convert_text.py index 007c245e6..676554f58 100644 --- a/processors/conversion/convert_text.py +++ b/processors/conversion/convert_text.py @@ -41,6 +41,7 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: "find": { "type": UserInput.OPTION_TEXT, "default": "", + "mandatory": True, "help": "Text to replace", "tooltip": "Multiple values can be replaced; separate with comma.", }, diff --git a/processors/conversion/merge_datasets.py b/processors/conversion/merge_datasets.py index a2022b57e..0184020ec 100644 --- a/processors/conversion/merge_datasets.py +++ b/processors/conversion/merge_datasets.py @@ -67,6 +67,7 @@ def get_options(cls, parent_dataset=None, config=None): options = { "source": { "type": UserInput.OPTION_TEXT_LARGE, + "mandatory": True, "help": "Source dataset URLs", "tooltip": "This should be the URL(s) of the result pages of the 4CAT dataset you want to merge with this " "dataset. Note that all datasets need to have the same format! Separate URLs with new lines or " diff --git a/processors/filtering/lexical_filter.py b/processors/filtering/lexical_filter.py index 4c0129f65..e461f7ae3 100644 --- a/processors/filtering/lexical_filter.py +++ b/processors/filtering/lexical_filter.py @@ -45,6 +45,7 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: "lexicon-custom": { "type": UserInput.OPTION_TEXT, "default": "", + "mandatory": True, "help": "Custom word list (separate with commas)" }, "as_regex": { diff --git a/processors/machine_learning/audio_to_text.py b/processors/machine_learning/audio_to_text.py index ed43d74cc..bc1d6046e 100644 --- a/processors/machine_learning/audio_to_text.py +++ b/processors/machine_learning/audio_to_text.py @@ -9,8 +9,7 @@ from backend.lib.processor import BasicProcessor from common.lib.compatibility import Compatibility from common.lib.dmi_service_manager import DmiServiceManager, DmiServiceManagerException, DsmOutOfMemory -from common.lib.exceptions import ProcessorInterruptedException, QueryParametersException, \ - QueryNeedsExplicitConfirmationException +from common.lib.exceptions import ProcessorInterruptedException, QueryNeedsExplicitConfirmationException from common.lib.user_input import UserInput from common.lib.item_mapping import MappedItem @@ -256,7 +255,8 @@ def get_options(cls, parent_dataset=None, config=None): "help": "OpenAI API key", "tooltip": "Can be created on platform.openapi.com", "requires": "model_host==openai", - "sensitive": True + "sensitive": True, + "mandatory": True } return options @@ -264,26 +264,21 @@ def get_options(cls, parent_dataset=None, config=None): @staticmethod def validate_query(query, request, config): """ - Check OpenAI requirements before the job is queued + Ask for confirmation before audio is sent to OpenAI - The API key field is only part of the form when no site-wide key is - configured, so when it is part of the submission the user has to fill - it in. Sending audio to OpenAI also shares data with a third party - and is likely to cost money, so ask for an explicit confirmation. + Sending audio to OpenAI shares data with a third party and is likely to + cost money, so ask the user to confirm that first. That the API key is + actually filled in is handled by the option's `mandatory` setting. :param dict query: Query parameters, from client-side. :param request: Flask request :param ConfigManager|None config: Configuration reader (context-aware) :return dict: Safe query parameters """ - if query.get("model_host") == "openai": - if "api_key" in query and not (query["api_key"] or "").strip(): - raise QueryParametersException("You need to provide an OpenAI API key to use OpenAI models.") - - if not query.get("frontend-confirm"): - raise QueryNeedsExplicitConfirmationException( - "Your audio files will be sent to OpenAI for processing. This shares your data with a third " - "party and is likely to incur costs. Do you want to continue?") + if query.get("model_host") == "openai" and not query.get("frontend-confirm"): + raise QueryNeedsExplicitConfirmationException( + "Your audio files will be sent to OpenAI for processing. This shares your data with a third " + "party and is likely to incur costs. Do you want to continue?") return query diff --git a/processors/machine_learning/clarifai_api.py b/processors/machine_learning/clarifai_api.py index 34ebe67cf..be7dc8d75 100644 --- a/processors/machine_learning/clarifai_api.py +++ b/processors/machine_learning/clarifai_api.py @@ -67,6 +67,7 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: "help": "API Key", "cache": True, "sensitive": True, + "mandatory": True, "tooltip": "The API key for your Clarifai account. You'll need to go to clarifai.com and create a new " "project to generate a key." }, diff --git a/processors/machine_learning/clip_categorize_images.py b/processors/machine_learning/clip_categorize_images.py index 99aa084d1..9d3e7299e 100644 --- a/processors/machine_learning/clip_categorize_images.py +++ b/processors/machine_learning/clip_categorize_images.py @@ -103,6 +103,7 @@ def get_options(cls, parent_dataset=None, config=None): "type": UserInput.OPTION_TEXT, "help": "Categories (comma seperated list)", "default": "", + "mandatory": True, "tooltip": "The CLIP model will estimate the probability an image belongs to every category, adding up to 100% across categories. It is quite robust and can accept proper nouns, some celebrities, as well as understand syntax such as \"animal\" vs \"not animal\"" }, } diff --git a/processors/machine_learning/google_vision_api.py b/processors/machine_learning/google_vision_api.py index 1f6f6eeea..675c42258 100644 --- a/processors/machine_learning/google_vision_api.py +++ b/processors/machine_learning/google_vision_api.py @@ -64,6 +64,7 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: "api_key": { "type": UserInput.OPTION_TEXT, "sensitive": True, + "mandatory": True, "help": "API Key", "tooltip": "The API Key for the Google API account you want to query with. You can generate and find this" "key on console.cloud.google.com. You also need to enable billing and Vision API." diff --git a/processors/machine_learning/perspective.py b/processors/machine_learning/perspective.py index 4b88035d3..e9f9e774a 100644 --- a/processors/machine_learning/perspective.py +++ b/processors/machine_learning/perspective.py @@ -72,7 +72,8 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: "default": "", "help": "Google API key", "tooltip": "Can be created on console.cloud.google.com", - "sensitive": True + "sensitive": True, + "mandatory": True } return options diff --git a/processors/metrics/youtube_metadata.py b/processors/metrics/youtube_metadata.py index 6dacf0fe3..19f793fe9 100644 --- a/processors/metrics/youtube_metadata.py +++ b/processors/metrics/youtube_metadata.py @@ -132,7 +132,8 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: "default": "", "help": "YouTube API key", "tooltip": "Can be created on https://developers.google.com/youtube/v3", - "sensitive": True + "sensitive": True, + "mandatory": True } return options diff --git a/processors/presets/annotate-images.py b/processors/presets/annotate-images.py index 57acbc55e..61241b686 100644 --- a/processors/presets/annotate-images.py +++ b/processors/presets/annotate-images.py @@ -48,6 +48,7 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: "api_key": { "type": UserInput.OPTION_TEXT, "sensitive": True, + "mandatory": True, "help": "API Key", "tooltip": "The API Key for your Google API account. You can generate and find this " "key on the API dashboard." diff --git a/processors/text-analysis/similar_words.py b/processors/text-analysis/similar_words.py index ad8f7db8a..f25e5f426 100644 --- a/processors/text-analysis/similar_words.py +++ b/processors/text-analysis/similar_words.py @@ -45,6 +45,7 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: return { "words": { "type": UserInput.OPTION_TEXT, + "mandatory": True, "help": "Words", "tooltip": "Separate with commas." }, diff --git a/processors/visualisation/histwords.py b/processors/visualisation/histwords.py index 93846d0cd..c910a013d 100644 --- a/processors/visualisation/histwords.py +++ b/processors/visualisation/histwords.py @@ -67,6 +67,7 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: return { "words": { "type": UserInput.OPTION_TEXT, + "mandatory": True, "help": "Word(s)", "tooltip": "Nearest neighbours for these words will be charted, and the position of the words will be highlighted" }, diff --git a/processors/visualisation/word-trees.py b/processors/visualisation/word-trees.py index f1b493570..a7da2de36 100644 --- a/processors/visualisation/word-trees.py +++ b/processors/visualisation/word-trees.py @@ -8,7 +8,6 @@ from backend.lib.processor import BasicProcessor from common.lib.helpers import UserInput, convert_to_int, get_4cat_canvas -from common.lib.exceptions import QueryParametersException from common.lib.compatibility import Compatibility from nltk.tokenize import word_tokenize, TweetTokenizer @@ -256,6 +255,7 @@ def get_options(cls, parent_dataset=None, config=None): "query": { "type": UserInput.OPTION_TEXT, "default": "", + "mandatory": True, "help": "Word tree root query", "tooltip": "Enter a text here to serve as the root of the word tree. The context of this query will be " "visualised as a tree graph. You can use wildcards: 'politic*' will match 'politician' and " @@ -451,23 +451,6 @@ def process(self): return self.dataset.finish(len(list(PreOrderIter(root)))) - @staticmethod - def validate_query(query, request, config): - """ - Validate input - - Checks if everything needed is filled in. - - :param query: - :param request: - :param config: - :return: - """ - if not query.get("query", "").strip(): - raise QueryParametersException("Query cannot be empty.") - - return query - def build_tree(self, query: str) -> tuple[TreeNode, int]: """ Build a tree of text fragments to then visualise diff --git a/processors/visualisation/youtube_thumbnails.py b/processors/visualisation/youtube_thumbnails.py index 111cd6a43..e8026a4bc 100644 --- a/processors/visualisation/youtube_thumbnails.py +++ b/processors/visualisation/youtube_thumbnails.py @@ -48,7 +48,8 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: "default": "", "help": "YouTube API key", "tooltip": "Can be created on https://developers.google.com/youtube/v3", - "sensitive": True + "sensitive": True, + "mandatory": True } } From 2fe5fa19a4cfeb1b5a0cb4fa43322d99c1108dd2 Mon Sep 17 00:00:00 2001 From: Dale Wahl Date: Fri, 17 Jul 2026 13:42:33 +0200 Subject: [PATCH 06/15] convert_text: fix default --- processors/conversion/convert_text.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processors/conversion/convert_text.py b/processors/conversion/convert_text.py index 676554f58..35c652dfa 100644 --- a/processors/conversion/convert_text.py +++ b/processors/conversion/convert_text.py @@ -35,7 +35,7 @@ def get_options(cls, parent_dataset=None, config=None) -> dict: options = { "columns": { "type": UserInput.OPTION_TEXT, - "default": "body", + "default": ["body"], "help": "Columns with texts to replace", }, "find": { From 697747623175ec40468fc3756a762c6b09a17f44 Mon Sep 17 00:00:00 2001 From: Dale Wahl Date: Fri, 17 Jul 2026 15:56:24 +0200 Subject: [PATCH 07/15] user_input: never fill in a value the user did not send A default is the form's starting value. It was also being filled back in while parsing, which meant a value the user never sent could pass the checks that follow. Three ways in: an option missing from the submission, a cleared text field, and an unticked multi-select all quietly became the default again. Parsing strictly now says so instead: - a missing option raises, naming it, since our own form submits every field it shows. Options a form legitimately leaves out stay exempt: unchecked toggles, file uploads (sent outside the form values), and fields gated by an unmet requires or board_specific condition. - a cleared text field stays empty; a cleared numeric field raises, as there is no honest empty number. - unticking every option of a multi-select means an empty selection. A browser leaves a select with nothing selected out of the submission altogether, which is indistinguishable from the field never having been shown, so the form now submits an empty marker alongside it; parsing discards the marker. The chan scope fields were kept out of submissions by the form disabling them, which is a hand-rolled "requires"; they now declare it instead. Parsing tolerantly (silently_correct) still fills the default in, so internal callers that ask to be corrected quietly are unaffected. --- common/lib/user_input.py | 99 ++++++++++--- datasources/eightchan/search_8chan.py | 6 +- datasources/eightkun/search_8kun.py | 6 +- datasources/fourchan/search_4chan.py | 22 ++- tests/test_modules.py | 10 +- tests/test_user_input_mandatory.py | 137 +++++++++++++++++- .../components/datasource-option.html | 7 + .../components/processor-option.html | 7 + 8 files changed, 250 insertions(+), 44 deletions(-) diff --git a/common/lib/user_input.py b/common/lib/user_input.py index 2b5cbf17c..3bada4489 100644 --- a/common/lib/user_input.py +++ b/common/lib/user_input.py @@ -111,11 +111,23 @@ def parse_all(options, input, silently_correct=False, log=None): Ignores all input not belonging to any of the defined options: parses and sanitises the rest, and returns a dictionary with the sanitised - options. If an option is *not* present in the input, the default value - is used, and if that is absent, `None`. + options. + + An option that is *not* present in the input at all is an error when + parsing strictly: our own form always submits every field it shows, so + a missing option means an incomplete submission (e.g. an API call that + left it out), and quietly proceeding without it would let later checks + pass on values the caller never sent. Exceptions are options a form + legitimately leaves out: unchecked toggles (submitted as nothing, read + as False), file uploads (sent outside the form values), fields gated + by an unmet "requires" condition or by "board_specific" (left out of + the result), and unticked multi-selects (the form submits an empty + marker so they are still present). When parsing tolerantly + (silently_correct), a missing option gets its default instead. In other words, this ensures a dictionary with 1) only white-listed - keys, 2) a value of an expected type for each key. + keys, 2) a value of an expected type for each key, and 3) when strict, + a complete submission. :param dict options: Options, as a name -> settings dictionary :param dict input: Input, as a form field -> value dictionary @@ -320,18 +332,41 @@ def parse_all(options, input, silently_correct=False, log=None): parsed_input[option] = {value[settings["dict_key"]]: {**value, "_id": value[settings["dict_key"]]} for value in parsed_input[option]} elif option not in input: - # not provided? use the default (an invalid one was warned - # about above, but is not silently replaced with something else) - parsed_input[option] = settings.get("default", None) - - # a mandatory option that was not submitted at all is only - # satisfied if its default fills it in. this is the shape an - # unselected multi-select arrives in: like an unchecked - # checkbox, the form leaves the field out altogether rather - # than sending it empty - if settings.get("mandatory") and UserInput.is_empty_value(parsed_input[option]): + # the option was not part of the submission at all. our own + # form always submits every field it shows - fields hidden by + # an unmet "requires" condition were already dropped above - + # so apart from the two exceptions below, a missing option + # means an incomplete submission. + if settings.get("type") == UserInput.OPTION_FILE: + # a file input's content does not travel among the form + # values (it is sent separately, in request.files), so it + # is never present here; whether a file was actually + # uploaded is for the module's validate_query to check + pass + + elif settings.get("board_specific"): + # only shown for particular boards: the form disables the + # field when another board is selected, and a disabled + # field is not submitted. like an unmet "requires", the + # user never saw it, so leave it out + pass + + elif settings.get("mandatory"): raise QueryParametersException("%s: This field is required." % (settings.get("help") or option)) + elif silently_correct: + # a caller that asked to be corrected quietly gets the + # default filled in + parsed_input[option] = settings.get("default", None) + + else: + # strict parsing: quietly filling in the default - or + # quietly leaving the option out - would let later checks + # pass on a value the caller never sent. an incomplete + # submission is the caller's mistake, so say so. + raise QueryParametersException( + "%s: this field was missing from the submitted form." % (settings.get("help") or option)) + else: # normal parsing and sanitisation try: @@ -557,13 +592,25 @@ def parse_value(settings, choice, other_input=None, silently_correct=False): # helpers) or as an actual list (real multi-selects and raw API # callers), so accept both shapes. (OPTION_MULTI used to assume a # string and crashed on a real list.) + if type(choice) is str: + choice = choice.split(",") + + # the form submits an empty value alongside whatever is selected, so + # that the field is present even when nothing is: a browser leaves a + # select with no selection out of the submission entirely, which is + # indistinguishable from the field never having been shown. discard + # that empty marker, and treat what remains as the selection. + choice = [item for item in (choice or []) if item != ""] + if not choice: if settings.get("mandatory"): raise QueryParametersException("This field is required.") - return settings.get("default", []) - - if type(choice) is str: - choice = choice.split(",") + if silently_correct: + return settings.get("default", []) + # a submitted-but-empty selection means the user unticked + # everything: a choice of nothing, not a request for the + # default back + return [] options = settings.get("options", []) invalid = [item for item in choice if item not in options] @@ -624,10 +671,20 @@ def parse_value(settings, choice, other_input=None, silently_correct=False): number_type = None if choice is None or choice == "": - # no value given: fall back to the default - choice = settings.get("default") - if choice is None: - choice = 0 if has_range else "" + # an empty value that was actively submitted - an option left + # out of the submission never reaches this point. the default + # is not quietly filled back in: the form showed it, and the + # user removed it. for a text field, empty is an answer + # ("nothing"); for a numeric field it is not one. a tolerant + # caller gets the correcting behaviour, i.e. the default. + if silently_correct: + choice = settings.get("default") + if choice is None: + choice = 0 if number_type is not None else "" + elif number_type is None: + return "" + else: + raise QueryParametersException("This field needs a number.") elif number_type is not None: # a number is expected: coerce it first, then keep it in range. diff --git a/datasources/eightchan/search_8chan.py b/datasources/eightchan/search_8chan.py index 2a079e29d..23dffe8a8 100644 --- a/datasources/eightchan/search_8chan.py +++ b/datasources/eightchan/search_8chan.py @@ -101,13 +101,15 @@ def get_options(cls, parent_dataset=None, config=None): "min": 0, "max": 100, "default": 15, - "tooltip": "At least this many % of messages in the thread must match the query" + "tooltip": "At least this many % of messages in the thread must match the query", + "requires": "search_scope==dense-threads" }, "scope_length": { "type": UserInput.OPTION_TEXT, "help": "Min. dense thread length", "min": 30, "default": 30, - "tooltip": "A thread must at least be this many messages long to qualify as a 'dense thread'" + "tooltip": "A thread must at least be this many messages long to qualify as a 'dense thread'", + "requires": "search_scope==dense-threads" } } diff --git a/datasources/eightkun/search_8kun.py b/datasources/eightkun/search_8kun.py index dab7c6fbb..4e4d9f2a3 100644 --- a/datasources/eightkun/search_8kun.py +++ b/datasources/eightkun/search_8kun.py @@ -104,13 +104,15 @@ def get_options(cls, parent_dataset=None, config=None): "min": 0, "max": 100, "default": 15, - "tooltip": "At least this many % of messages in the thread must match the query" + "tooltip": "At least this many % of messages in the thread must match the query", + "requires": "search_scope==dense-threads" }, "scope_length": { "type": UserInput.OPTION_TEXT, "help": "Min. dense thread length", "min": 30, "default": 30, - "tooltip": "A thread must at least be this many messages long to qualify as a 'dense thread'" + "tooltip": "A thread must at least be this many messages long to qualify as a 'dense thread'", + "requires": "search_scope==dense-threads" } } diff --git a/datasources/fourchan/search_4chan.py b/datasources/fourchan/search_4chan.py index ef8409d27..d6235c6c1 100644 --- a/datasources/fourchan/search_4chan.py +++ b/datasources/fourchan/search_4chan.py @@ -390,18 +390,21 @@ def get_options(cls, parent_dataset=None, config=None): "min": 0, "max": 100, "default": 15, - "tooltip": "At least this many % of posts in the thread must match the query" + "tooltip": "At least this many % of posts in the thread must match the query", + "requires": "search_scope==dense-threads" }, "scope_length": { "type": UserInput.OPTION_TEXT, "help": "Min. dense thread length", "min": 30, "default": 30, - "tooltip": "A thread must at least be this many posts long to qualify as a 'dense thread'" + "tooltip": "A thread must at least be this many posts long to qualify as a 'dense thread'", + "requires": "search_scope==dense-threads" }, "valid_ids": { "type": UserInput.OPTION_TEXT, - "help": "Post IDs (comma-separated)" + "help": "Post IDs (comma-separated)", + "requires": "search_scope==match-ids" } } @@ -870,12 +873,17 @@ def validate_query(query, request, config): query["max_date"] = before del query["daterange"] + + # the scope fields are declared with a "requires" on the matching + # search scope, so on the web path they are already left out of the + # parsed input when their scope is not selected; this also covers + # callers that skip that parsing and pass them along anyway if query.get("search_scope") not in ("dense-threads",): - del query["scope_density"] - del query["scope_length"] + query.pop("scope_density", None) + query.pop("scope_length", None) - if query.get("search_scope") not in ("match-ids",) and "valid_ids" in query.keys(): - del query["valid_ids"] + if query.get("search_scope") not in ("match-ids",): + query.pop("valid_ids", None) return query diff --git a/tests/test_modules.py b/tests/test_modules.py index 7577804b6..c08ecfa03 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -641,12 +641,14 @@ def test_parse_all_gated_options(): assert parsed == {"gate": False, "plain": "y"} # gate off, gated field submitted anyway (e.g. hidden field still posts): - # still absent - parsed = UserInput.parse_all(options, {"option-gated": ";"}) + # still absent. (plain must be submitted: a missing non-gated option is an + # incomplete submission and raises) + parsed = UserInput.parse_all(options, {"option-plain": "y", "option-gated": ";"}) assert "gated" not in parsed and "gated_toggle" not in parsed - # gate on: gated options are parsed (submitted value) or defaulted (absent) - parsed = UserInput.parse_all(options, {"option-gate": "on", "option-gated": ";"}) + # gate on: the gated text field is parsed; the gated toggle is absent from + # the submission, which for a toggle simply means unchecked + parsed = UserInput.parse_all(options, {"option-gate": "on", "option-plain": "y", "option-gated": ";"}) assert parsed["gated"] == ";" and parsed["gated_toggle"] is False and parsed["gate"] is True diff --git a/tests/test_user_input_mandatory.py b/tests/test_user_input_mandatory.py index 7d8aca844..37cfd60f8 100644 --- a/tests/test_user_input_mandatory.py +++ b/tests/test_user_input_mandatory.py @@ -76,9 +76,12 @@ def test_mandatory_text_missing_raises(self): with pytest.raises(QueryParametersException): UserInput.parse_all(options, {}) - def test_mandatory_text_with_default_missing_input_does_not_raise(self): + def test_mandatory_text_with_default_missing_input_raises(self): """ - If the option was not submitted but has a default, the default is used. + A default is the form's starting value, not a stand-in for an answer. + Defaults are no longer filled in for a strict caller, so an option that + was not submitted has not been given - and a mandatory option that was + not given fails, whether or not it has a default. """ options = { "query": { @@ -88,12 +91,17 @@ def test_mandatory_text_with_default_missing_input_does_not_raise(self): "mandatory": True, } } - parsed = UserInput.parse_all(options, {}) - assert parsed == {"query": "default query"} + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, {}) - def test_non_mandatory_text_empty_is_allowed(self): + def test_missing_option_raises_unless_correcting_silently(self): """ - Without mandatory=True, an empty value should fall back to the default. + Our own form always submits every field it shows, so an option that is + missing from the input altogether means an incomplete submission (e.g. + an API call that left it out). Strict parsing refuses it, naming the + option - quietly proceeding would let later checks pass on values the + caller never sent. A caller that asked to be corrected quietly gets + the default filled in instead. """ options = { "query": { @@ -102,8 +110,68 @@ def test_non_mandatory_text_empty_is_allowed(self): "default": "default query", } } - parsed = UserInput.parse_all(options, {"query": ""}) - assert parsed == {"query": "default query"} + + assert UserInput.parse_all(options, {}, silently_correct=True) == {"query": "default query"} + with pytest.raises(QueryParametersException) as raised: + UserInput.parse_all(options, {}) + assert "Search query" in str(raised.value) + + def test_cleared_text_stays_cleared(self): + """ + The form pre-fills the default into the field, so an empty submitted + value means the user deliberately removed it. That is an answer + ("nothing"), and it is stored as such - not quietly swapped back for + the default the user just deleted. A tolerant caller still gets the + default. + """ + options = { + "query": { + "type": UserInput.OPTION_TEXT, + "help": "Search query", + "default": "default query", + } + } + assert UserInput.parse_all(options, {"query": ""}) == {"query": ""} + assert UserInput.parse_all(options, {"query": ""}, silently_correct=True) == {"query": "default query"} + + def test_cleared_number_field_raises(self): + """ + There is no honest empty number: a user who clears a numeric field and + submits is told to enter a number, rather than having the deleted + default quietly restored. + """ + options = { + "amount": { + "type": UserInput.OPTION_TEXT, + "help": "Number of items", + "coerce_type": int, + "min": 1, + "default": 10, + } + } + with pytest.raises(QueryParametersException) as raised: + UserInput.parse_all(options, {"amount": ""}) + assert "Number of items" in str(raised.value) + + assert UserInput.parse_all(options, {"amount": ""}, silently_correct=True) == {"amount": 10} + + def test_unticking_an_optional_multi_means_none(self): + """ + Unticking every option of a non-mandatory multi-select is a choice of + nothing - the result is an empty selection, not the default the user + just unticked. + """ + options = { + "boards": { + "type": UserInput.OPTION_MULTI, + "help": "Boards", + "options": {"pol": "/pol/", "v": "/v/"}, + "default": ["pol"], + } + } + # the form's empty marker on its own: nothing selected + assert UserInput.parse_all(options, {"boards": ""}) == {"boards": []} + assert UserInput.parse_all(options, {"boards": ""}, silently_correct=True) == {"boards": ["pol"]} def test_mandatory_multi_raises_when_empty(self): """ @@ -139,6 +207,59 @@ def test_mandatory_multi_raises_when_nothing_selected(self): with pytest.raises(QueryParametersException): UserInput.parse_all(options, {}) + def test_mandatory_multi_raises_when_default_is_unselected(self): + """ + A user who unticks every option, including a selected default, has + chosen nothing and should be told so rather than quietly handed the + default back. + + A browser submits nothing at all for a select with no selection, which + would be indistinguishable from the field never having been shown, so + the form submits an empty value alongside it. That empty marker on its + own is what "I unticked everything" looks like here. + """ + options = { + "boards": { + "type": UserInput.OPTION_MULTI, + "help": "Boards", + "options": {"pol": "/pol/", "v": "/v/"}, + "default": ["pol"], + "mandatory": True, + } + } + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, {"boards": ""}) + + def test_multi_discards_the_empty_marker(self): + """ + The empty value the form submits alongside the selected ones is not a + selection, and must not be mistaken for an invalid choice. + """ + options = { + "boards": { + "type": UserInput.OPTION_MULTI, + "help": "Boards", + "options": {"pol": "/pol/", "v": "/v/"}, + } + } + parsed = UserInput.parse_all(options, {"boards": ["", "pol"]}) + assert parsed == {"boards": ["pol"]} + + def test_multi_still_rejects_an_invalid_choice_alongside_the_marker(self): + """ + Discarding the marker must not become a way to smuggle bad values past + validation. + """ + options = { + "boards": { + "type": UserInput.OPTION_MULTI, + "help": "Boards", + "options": {"pol": "/pol/", "v": "/v/"}, + } + } + with pytest.raises(QueryParametersException): + UserInput.parse_all(options, {"boards": ["", "nope"]}) + def test_mandatory_multi_accepts_value(self): """ A mandatory multi-select with at least one selected value should parse. diff --git a/webtool/templates/components/datasource-option.html b/webtool/templates/components/datasource-option.html index cd8694fec..3130c8204 100644 --- a/webtool/templates/components/datasource-option.html +++ b/webtool/templates/components/datasource-option.html @@ -103,6 +103,11 @@

{{ settings.help }}

{% endif %} {% elif settings.type == "multi" %}
+ {# a browser submits nothing at all for a select with nothing selected, which + would be indistinguishable from the field never having been shown. this + empty value keeps the field in the submission either way; it is discarded + while parsing #} + {% elif option_settings.type in ("multi", "annotations") %}
+ {# a browser submits nothing at all for a select with nothing selected, which + would be indistinguishable from the field never having been shown. this + empty value keeps the field in the submission either way; it is discarded + while parsing #} +