From f0f89d8f2a4fb9b932b2f32d8cf66754fde3a077 Mon Sep 17 00:00:00 2001 From: axolotle Date: Thu, 13 Apr 2023 13:58:24 +0200 Subject: [PATCH] form: restrict choices to select, tags, domain, app, user + group --- src/tests/test_questions.py | 85 +++---------------------- src/utils/form.py | 121 +++++++++++++++++++++++------------- 2 files changed, 84 insertions(+), 122 deletions(-) diff --git a/src/tests/test_questions.py b/src/tests/test_questions.py index 9e7be5db4..3e7927dce 100644 --- a/src/tests/test_questions.py +++ b/src/tests/test_questions.py @@ -17,9 +17,9 @@ from yunohost import app, domain, user from yunohost.utils.form import ( OPTIONS, ask_questions_and_parse_answers, + BaseChoicesOption, BaseInputOption, BaseReadonlyOption, - DisplayTextOption, PasswordOption, DomainOption, WebPathOption, @@ -490,14 +490,12 @@ class BaseTest: option, value = _fill_or_prompt_one_option(raw_option, None) expected_message = option.ask["en"] + choices = [] - if option.choices: - choices = ( - option.choices - if isinstance(option.choices, list) - else option.choices.keys() - ) - expected_message += f" [{' | '.join(choices)}]" + if isinstance(option, BaseChoicesOption): + choices = option.choices + if choices: + expected_message += f" [{' | '.join(choices)}]" if option.type == "boolean": expected_message += " [yes | no]" @@ -507,7 +505,7 @@ class BaseTest: confirm=False, # FIXME no confirm? prefill=prefill, is_multiline=option.type == "text", - autocomplete=option.choices or [], + autocomplete=choices, help=option.help["en"], ) @@ -1972,75 +1970,6 @@ def test_question_string_input_test_ask_with_example(): assert example_text in prompt.call_args[1]["message"] -def test_question_string_with_choice(): - questions = {"some_string": {"type": "string", "choices": ["fr", "en"]}} - answers = {"some_string": "fr"} - out = ask_questions_and_parse_answers(questions, answers)[0] - - assert out.name == "some_string" - assert out.type == "string" - assert out.value == "fr" - - -def test_question_string_with_choice_prompt(): - questions = {"some_string": {"type": "string", "choices": ["fr", "en"]}} - answers = {"some_string": "fr"} - with patch.object(Moulinette, "prompt", return_value="fr"), patch.object( - os, "isatty", return_value=True - ): - out = ask_questions_and_parse_answers(questions, answers)[0] - - assert out.name == "some_string" - assert out.type == "string" - assert out.value == "fr" - - -def test_question_string_with_choice_bad(): - questions = {"some_string": {"type": "string", "choices": ["fr", "en"]}} - answers = {"some_string": "bad"} - - with pytest.raises(YunohostError), patch.object(os, "isatty", return_value=False): - ask_questions_and_parse_answers(questions, answers) - - -def test_question_string_with_choice_ask(): - ask_text = "some question" - choices = ["fr", "en", "es", "it", "ru"] - questions = { - "some_string": { - "ask": ask_text, - "choices": choices, - } - } - answers = {} - - with patch.object(Moulinette, "prompt", return_value="ru") as prompt, patch.object( - os, "isatty", return_value=True - ): - ask_questions_and_parse_answers(questions, answers) - assert ask_text in prompt.call_args[1]["message"] - - for choice in choices: - assert choice in prompt.call_args[1]["message"] - - -def test_question_string_with_choice_default(): - questions = { - "some_string": { - "type": "string", - "choices": ["fr", "en"], - "default": "en", - } - } - answers = {} - with patch.object(os, "isatty", return_value=False): - out = ask_questions_and_parse_answers(questions, answers)[0] - - assert out.name == "some_string" - assert out.type == "string" - assert out.value == "en" - - @pytest.mark.skip # we should do something with this example def test_question_password_input_test_ask_with_example(): ask_text = "some question" diff --git a/src/utils/form.py b/src/utils/form.py index 57d4cabcc..02f51b6c4 100644 --- a/src/utils/form.py +++ b/src/utils/form.py @@ -306,8 +306,6 @@ class BaseInputOption(BaseOption): super().__init__(question, hooks) self.default = question.get("default", None) self.optional = question.get("optional", False) - # Don't restrict choices if there's none specified - self.choices = question.get("choices", None) self.pattern = question.get("pattern", self.pattern) self.help = question.get("help") self.redact = question.get("redact", False) @@ -338,33 +336,7 @@ class BaseInputOption(BaseOption): if self.readonly: message = colorize(message, "purple") - if self.choices: - choice = self.current_value - if isinstance(self.choices, dict) and choice is not None: - choice = self.choices[choice] - return f"{message} {choice}" - return message + f" {self.humanize(self.current_value)}" - elif self.choices: - # Prevent displaying a shitload of choices - # (e.g. 100+ available users when choosing an app admin...) - choices = ( - list(self.choices.keys()) - if isinstance(self.choices, dict) - else self.choices - ) - choices_to_display = choices[:20] - remaining_choices = len(choices[20:]) - - if remaining_choices > 0: - choices_to_display += [ - m18n.n("other_available_options", n=remaining_choices) - ] - - choices_to_display = " | ".join( - str(choice) for choice in choices_to_display - ) - - message += f" [{choices_to_display}]" + return f"{message} {self.humanize(self.current_value)}" return message @@ -374,13 +346,6 @@ class BaseInputOption(BaseOption): # we have an answer, do some post checks if self.value not in [None, ""]: - if self.choices and self.value not in self.choices: - raise YunohostValidationError( - "app_argument_choice_invalid", - name=self.name, - value=self.value, - choices=", ".join(str(choice) for choice in self.choices), - ) if self.pattern and not re.match(self.pattern["regexp"], str(self.value)): raise YunohostValidationError( self.pattern["error"], @@ -746,7 +711,72 @@ class FileOption(BaseInputOption): # ─ CHOICES ─────────────────────────────────────────────── -class TagsOption(BaseInputOption): +class BaseChoicesOption(BaseInputOption): + def __init__( + self, + question: Dict[str, Any], + hooks: Dict[str, Callable] = {}, + ): + super().__init__(question, hooks) + # Don't restrict choices if there's none specified + self.choices = question.get("choices", None) + + def _get_prompt_message(self) -> str: + message = super()._get_prompt_message() + + if self.readonly: + message = message + choice = self.current_value + + if isinstance(self.choices, dict) and choice is not None: + choice = self.choices[choice] + + return f"{colorize(message, 'purple')} {choice}" + + if self.choices: + # Prevent displaying a shitload of choices + # (e.g. 100+ available users when choosing an app admin...) + choices = ( + list(self.choices.keys()) + if isinstance(self.choices, dict) + else self.choices + ) + choices_to_display = choices[:20] + remaining_choices = len(choices[20:]) + + if remaining_choices > 0: + choices_to_display += [ + m18n.n("other_available_options", n=remaining_choices) + ] + + choices_to_display = " | ".join( + str(choice) for choice in choices_to_display + ) + + return f"{message} [{choices_to_display}]" + + return message + + def _value_pre_validator(self): + super()._value_pre_validator() + + # we have an answer, do some post checks + if self.value not in [None, ""]: + if self.choices and self.value not in self.choices: + raise YunohostValidationError( + "app_argument_choice_invalid", + name=self.name, + value=self.value, + choices=", ".join(str(choice) for choice in self.choices), + ) + + +class SelectOption(BaseChoicesOption): + argument_type = "select" + default_value = "" + + +class TagsOption(BaseChoicesOption): argument_type = "tags" default_value = "" @@ -799,7 +829,7 @@ class TagsOption(BaseInputOption): # ─ ENTITIES ────────────────────────────────────────────── -class DomainOption(BaseInputOption): +class DomainOption(BaseChoicesOption): argument_type = "domain" def __init__(self, question, hooks: Dict[str, Callable] = {}): @@ -828,7 +858,7 @@ class DomainOption(BaseInputOption): return value -class AppOption(BaseInputOption): +class AppOption(BaseChoicesOption): argument_type = "app" def __init__(self, question, hooks: Dict[str, Callable] = {}): @@ -853,7 +883,7 @@ class AppOption(BaseInputOption): self.choices.update({app["id"]: _app_display(app) for app in apps}) -class UserOption(BaseInputOption): +class UserOption(BaseChoicesOption): argument_type = "user" def __init__(self, question, hooks: Dict[str, Callable] = {}): @@ -884,7 +914,7 @@ class UserOption(BaseInputOption): break -class GroupOption(BaseInputOption): +class GroupOption(BaseChoicesOption): argument_type = "group" def __init__(self, question, hooks: Dict[str, Callable] = {}): @@ -926,7 +956,7 @@ OPTIONS = { "path": WebPathOption, "url": URLOption, "file": FileOption, - "select": StringOption, + "select": SelectOption, "tags": TagsOption, "domain": DomainOption, "app": AppOption, @@ -997,6 +1027,9 @@ def prompt_or_validate_form( for i in range(5): if interactive and option.value is None: prefill = "" + choices = ( + option.choices if isinstance(option, BaseChoicesOption) else [] + ) if option.current_value is not None: prefill = option.humanize(option.current_value, option) @@ -1009,7 +1042,7 @@ def prompt_or_validate_form( confirm=False, prefill=prefill, is_multiline=(option.type == "text"), - autocomplete=option.choices or [], + autocomplete=choices, help=_value_for_locale(option.help), ) @@ -1094,7 +1127,7 @@ def hydrate_questions_with_choices(raw_questions: List) -> List: for raw_question in raw_questions: question = OPTIONS[raw_question.get("type", "string")](raw_question) - if isinstance(question, BaseInputOption) and question.choices: + if isinstance(question, BaseChoicesOption) and question.choices: raw_question["choices"] = question.choices raw_question["default"] = question.default out.append(raw_question)