form: fix readonly prompting + + choices + tests

src/tests/test_questions.py

@@ -662,9 +662,7 @@ class TestString(BaseTest):
             (" ##value \n \tvalue\n  ", "##value \n \tvalue"),
         ], reason=r"should fail or without `\n`?"),
         # readonly
-        *xfail(scenarios=[
+        ("overwrite", "expected value", {"readonly": True, "current_value": "expected value"}),
-            ("overwrite", "expected value", {"readonly": True, "default": "expected value"}),
-        ], reason="Should not be overwritten"),
     ]
     # fmt: on
 
@@ -701,9 +699,7 @@ class TestText(BaseTest):
             (r" ##value \n \tvalue\n  ", r"##value \n \tvalue\n"),
         ], reason="Should not be stripped"),
         # readonly
-        *xfail(scenarios=[
+        ("overwrite", "expected value", {"readonly": True, "current_value": "expected value"}),
-            ("overwrite", "expected value", {"readonly": True, "default": "expected value"}),
-        ], reason="Should not be overwritten"),
     ]
     # fmt: on
 
@@ -737,9 +733,7 @@ class TestPassword(BaseTest):
         ("secret", FAIL),
         *[("supersecret" + char, FAIL) for char in PasswordOption.forbidden_chars],  # FIXME maybe add ` \n` to the list?
         # readonly
-        *xpass(scenarios=[
+        ("s3cr3t!!", YunohostError, {"readonly": True, "current_value": "isforbidden"}),  # readonly is forbidden
-            ("s3cr3t!!", "s3cr3t!!", {"readonly": True}),
-        ], reason="Should fail since readonly is forbidden"),
     ]
     # fmt: on
 
@@ -780,9 +774,7 @@ class TestColor(BaseTest):
             ("yellow", "#ffff00"),
         ], reason="Should work with pydantic"),
         # readonly
-        *xfail(scenarios=[
+        ("#ffff00", "#fe100", {"readonly": True, "current_value": "#fe100"}),
-            ("#ffff00", "#fe100", {"readonly": True, "default": "#fe100"}),
-        ], reason="Should not be overwritten"),
     ]
     # fmt: on
 
@@ -824,9 +816,7 @@ class TestNumber(BaseTest):
         (-10, -10, {"default": 10}),
         (-10, -10, {"default": 10, "optional": True}),
         # readonly
-        *xfail(scenarios=[
+        (1337, 10000, {"readonly": True, "current_value": 10000}),
-            (1337, 10000, {"readonly": True, "default": 10000}),
-        ], reason="Should not be overwritten"),
     ]
     # fmt: on
     # FIXME should `step` be some kind of "multiple of"?
@@ -891,9 +881,7 @@ class TestBoolean(BaseTest):
             "scenarios": all_fails("", "y", "n", error=AssertionError),
         },
         # readonly
-        *xfail(scenarios=[
+        (1, 0, {"readonly": True, "current_value": 0}),
-            (1, 0, {"readonly": True, "default": 0}),
-        ], reason="Should not be overwritten"),
     ]
 
 
@@ -931,9 +919,7 @@ class TestDate(BaseTest):
         ("12-01-10", FAIL),
         ("2022-02-29", FAIL),
         # readonly
-        *xfail(scenarios=[
+        ("2070-12-31", "2024-02-29", {"readonly": True, "current_value": "2024-02-29"}),
-            ("2070-12-31", "2024-02-29", {"readonly": True, "default": "2024-02-29"}),
-        ], reason="Should not be overwritten"),
     ]
     # fmt: on
 
@@ -966,9 +952,7 @@ class TestTime(BaseTest):
         ("23:1", FAIL),
         ("23:005", FAIL),
         # readonly
-        *xfail(scenarios=[
+        ("00:00", "08:00", {"readonly": True, "current_value": "08:00"}),
-            ("00:00", "08:00", {"readonly": True, "default": "08:00"}),
-        ], reason="Should not be overwritten"),
     ]
     # fmt: on
 
@@ -992,9 +976,7 @@ class TestEmail(BaseTest):
         *nones(None, "", output=""),
         ("\n Abc@example.tld  ", "Abc@example.tld"),
         # readonly
-        *xfail(scenarios=[
+        ("Abc@example.tld", "admin@ynh.local", {"readonly": True, "current_value": "admin@ynh.local"}),
-            ("Abc@example.tld", "admin@ynh.local", {"readonly": True, "default": "admin@ynh.local"}),
-        ], reason="Should not be overwritten"),
 
         # Next examples are from https://github.com/JoshData/python-email-validator/blob/main/tests/test_syntax.py
         # valid email values
@@ -1107,9 +1089,7 @@ class TestWebPath(BaseTest):
             ("https://example.com/folder", "/https://example.com/folder")
         ], reason="Should fail or scheme+domain removed"),
         # readonly
-        *xfail(scenarios=[
+        ("/overwrite", "/value", {"readonly": True, "current_value": "/value"}),
-            ("/overwrite", "/value", {"readonly": True, "default": "/value"}),
-        ], reason="Should not be overwritten"),
         # FIXME should path have forbidden_chars?
     ]
     # fmt: on
@@ -1134,9 +1114,7 @@ class TestUrl(BaseTest):
         *nones(None, "", output=""),
         ("http://some.org/folder/file.txt", "http://some.org/folder/file.txt"),
         # readonly
-        *xfail(scenarios=[
+        ("https://overwrite.org", "https://example.org", {"readonly": True, "current_value": "https://example.org"}),
-            ("https://overwrite.org", "https://example.org", {"readonly": True, "default": "https://example.org"}),
-        ], reason="Should not be overwritten"),
         # rest is taken from https://github.com/pydantic/pydantic/blob/main/tests/test_networks.py
         # valid
         *unchanged(
@@ -1426,9 +1404,7 @@ class TestSelect(BaseTest):
             ]
         },
         # readonly
-        *xfail(scenarios=[
+        ("one", "two", {"readonly": True, "choices": ["one", "two"], "current_value": "two"}),
-            ("one", "two", {"readonly": True, "choices": ["one", "two"], "default": "two"}),
-        ], reason="Should not be overwritten"),
     ]
     # fmt: on
 
@@ -1476,9 +1452,7 @@ class TestTags(BaseTest):
         *all_fails(*([t] for t in [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]), raw_option={"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}),
         *all_fails(*([str(t)] for t in [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]), raw_option={"choices": [False, True, -1, 0, 1, 1337, 13.37, [], ["one"], {}]}),
         # readonly
-        *xfail(scenarios=[
+        ("one", "one,two", {"readonly": True, "choices": ["one", "two"], "current_value": "one,two"}),
-            ("one", "one,two", {"readonly": True, "choices": ["one", "two"], "default": "one,two"}),
-        ], reason="Should not be overwritten"),
     ]
     # fmt: on
 
@@ -1526,9 +1500,7 @@ class TestDomain(BaseTest):
                 ("doesnt_exist.pouet", FAIL, {}),
                 ("fake.com", FAIL, {"choices": ["fake.com"]}),
                 # readonly
-                *xpass(scenarios=[
+                (domains1[0], YunohostError, {"readonly": True}),  # readonly is forbidden
-                    (domains1[0], domains1[0], {"readonly": True}),
-                ], reason="Should fail since readonly is forbidden"),
             ]
         },
         {
@@ -1625,9 +1597,7 @@ class TestApp(BaseTest):
                 (installed_non_webapp["id"], installed_non_webapp["id"]),
                 (installed_non_webapp["id"], FAIL, {"filter": "is_webapp"}),
                 # readonly
-                *xpass(scenarios=[
+                (installed_non_webapp["id"], YunohostError, {"readonly": True}),  # readonly is forbidden
-                    (installed_non_webapp["id"], installed_non_webapp["id"], {"readonly": True}),
-                ], reason="Should fail since readonly is forbidden"),
             ]
         },
     ]
@@ -1744,9 +1714,7 @@ class TestUser(BaseTest):
                     ("", regular_username, {"default": regular_username})
                 ], reason="Should throw 'no default allowed'"),
                 # readonly
-                *xpass(scenarios=[
+                (admin_username, YunohostError, {"readonly": True}),  # readonly is forbidden
-                    (admin_username, admin_username, {"readonly": True}),
-                ], reason="Should fail since readonly is forbidden"),
             ]
         },
     ]
@@ -1821,9 +1789,7 @@ class TestGroup(BaseTest):
                     ("", "custom_group", {"default": "custom_group"}),
                 ], reason="Should throw 'default must be in (None, 'all_users', 'visitors', 'admins')"),
                 # readonly
-                *xpass(scenarios=[
+                ("admins", YunohostError, {"readonly": True}),  # readonly is forbidden
-                    ("admins", "admins", {"readonly": True}),
-                ], reason="Should fail since readonly is forbidden"),
             ]
         },
     ]

src/utils/configpanel.py

@@ -465,20 +465,10 @@ class ConfigPanel:
             "max_progression",
         ]
         forbidden_keywords += format_description["sections"]
-        forbidden_readonly_types = ["password", "app", "domain", "user", "file"]
 
         for _, _, option in self._iterate():
             if option["id"] in forbidden_keywords:
                 raise YunohostError("config_forbidden_keyword", keyword=option["id"])
-            if (
-                option.get("readonly", False)
-                and option.get("type", "string") in forbidden_readonly_types
-            ):
-                raise YunohostError(
-                    "config_forbidden_readonly_type",
-                    type=option["type"],
-                    id=option["id"],
-                )
 
         return self.config
 

src/utils/form.py

@@ -204,7 +204,16 @@ class BaseOption:
         self.hooks = hooks
         self.type = question.get("type", "string")
         self.visible = question.get("visible", True)
+
         self.readonly = question.get("readonly", False)
+        if self.readonly and self.type in {"password", "app", "domain", "user", "group", "file"}:
+            # FIXME i18n
+            raise YunohostError(
+                "config_forbidden_readonly_type",
+                type=self.type,
+                id=self.name,
+            )
+
         self.ask = question.get("ask", self.name)
         if not isinstance(self.ask, dict):
             self.ask = {"en": self.ask}
@@ -328,9 +337,10 @@ class BaseInputOption(BaseOption):
         if self.readonly:
             text_for_user_input_in_cli = colorize(text_for_user_input_in_cli, "purple")
             if self.choices:
-                return (
+                choice = self.current_value
-                    text_for_user_input_in_cli + f" {self.choices[self.current_value]}"
+                if isinstance(self.choices, dict) and choice is not None:
-                )
+                    choice = self.choices[choice]
+                return f"{text_for_user_input_in_cli} {choice}"
             return text_for_user_input_in_cli + f" {self.humanize(self.current_value)}"
         elif self.choices:
             # Prevent displaying a shitload of choices
@@ -348,7 +358,9 @@ class BaseInputOption(BaseOption):
                     m18n.n("other_available_options", n=remaining_choices)
                 ]
 
-            choices_to_display = " | ".join(choices_to_display)
+            choices_to_display = " | ".join(
+                str(choice) for choice in choices_to_display
+            )
 
             text_for_user_input_in_cli += f" [{choices_to_display}]"
 
@@ -946,7 +958,7 @@ def prompt_or_validate_form(
         interactive = Moulinette.interface.type == "cli" and os.isatty(1)
 
         if isinstance(option, ButtonOption):
-            if option.is_enabled(context):
+            if option.is_visible(context) and option.is_enabled(context):
                 continue
             else:
                 raise YunohostValidationError(
@@ -955,32 +967,49 @@ def prompt_or_validate_form(
                     help=_value_for_locale(option.help),
                 )
 
-        if option.is_visible(context):
+        # FIXME not sure why we do not append Buttons to returned options
+        options.append(option)
+
+        if not option.is_visible(context):
+            if isinstance(option, BaseInputOption):
+                # FIXME There could be several use case if the question is not displayed:
+                # - we doesn't want to give a specific value
+                # - we want to keep the previous value
+                # - we want the default value
+                option.value = context[option.name] = None
+
+            continue
+
+        message = option._format_text_for_user_input_in_cli()
+
+        if option.readonly:
+            if interactive:
+                Moulinette.display(message)
+
+            if isinstance(option, BaseInputOption):
+                option.value = context[option.name] = option.current_value
+
+            continue
+
+        if isinstance(option, BaseInputOption):
             for i in range(5):
-                # Display question if no value filled or if it's a readonly message
+                if interactive and option.value is None:
-                if interactive:
+                    prefill = ""
-                    text_for_user_input_in_cli = (
+
-                        option._format_text_for_user_input_in_cli()
+                    if option.current_value is not None:
+                        prefill = option.humanize(option.current_value, option)
+                    elif option.default is not None:
+                        prefill = option.humanize(option.default, option)
+
+                    option.value = Moulinette.prompt(
+                        message=message,
+                        is_password=isinstance(option, PasswordOption),
+                        confirm=False,
+                        prefill=prefill,
+                        is_multiline=(option.type == "text"),
+                        autocomplete=option.choices or [],
+                        help=_value_for_locale(option.help),
                     )
-                    if option.readonly:
-                        Moulinette.display(text_for_user_input_in_cli)
-                        option.value = option.current_value
-                        break
-                    elif option.value is None:
-                        prefill = ""
-                        if option.current_value is not None:
-                            prefill = option.humanize(option.current_value, option)
-                        elif option.default is not None:
-                            prefill = option.humanize(option.default, option)
-                        option.value = Moulinette.prompt(
-                            message=text_for_user_input_in_cli,
-                            is_password=option.hide_user_input_in_prompt,
-                            confirm=False,
-                            prefill=prefill,
-                            is_multiline=(option.type == "text"),
-                            autocomplete=option.choices or [],
-                            help=_value_for_locale(option.help),
-                        )
 
                 # Apply default value
                 class_default = getattr(option, "default_value", None)
@@ -1013,16 +1042,9 @@ def prompt_or_validate_form(
             post_hook = f"post_ask__{option.name}"
             if post_hook in option.hooks:
                 option.values.update(option.hooks[post_hook](option))
-        else:
-            # FIXME There could be several use case if the question is not displayed:
-            # - we doesn't want to give a specific value
-            # - we want to keep the previous value
-            # - we want the default value
-            option.value = option.values[option.name] = None
 
-        answers.update(option.values)
+            answers.update(option.values)
-        context.update(option.values)
+            context.update(option.values)
-        options.append(option)
 
     return options
 
@@ -1070,7 +1092,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 question.choices:
+        if isinstance(question, BaseInputOption) and question.choices:
             raw_question["choices"] = question.choices
             raw_question["default"] = question.default
         out.append(raw_question)