From b3e7bd88880367a49347ad604a01885291bd0e93 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 15 Oct 2020 03:34:51 -0400 Subject: [PATCH 1/2] support reloading allow all imports through reload service and config entry options UI --- custom_components/pyscript/__init__.py | 2 +- custom_components/pyscript/config_flow.py | 43 ++++++++++++- custom_components/pyscript/eval.py | 14 +++-- custom_components/pyscript/strings.json | 16 ++++- .../pyscript/translations/en.json | 16 ++++- tests/test_config_flow.py | 62 +++++++++++++++++++ tests/test_unit_eval.py | 4 ++ 7 files changed, 149 insertions(+), 8 deletions(-) diff --git a/custom_components/pyscript/__init__.py b/custom_components/pyscript/__init__.py index b87809a..43c88d4 100644 --- a/custom_components/pyscript/__init__.py +++ b/custom_components/pyscript/__init__.py @@ -65,7 +65,7 @@ async def async_setup_entry(hass, config_entry): await hass.async_add_executor_job(os.makedirs, pyscript_folder) hass.data.setdefault(DOMAIN, {}) - hass.data[DOMAIN][CONF_ALLOW_ALL_IMPORTS] = config_entry.data.get(CONF_ALLOW_ALL_IMPORTS) + hass.data[DOMAIN] = config_entry State.set_pyscript_config(config_entry.data) diff --git a/custom_components/pyscript/config_flow.py b/custom_components/pyscript/config_flow.py index 2da34a0..381166b 100644 --- a/custom_components/pyscript/config_flow.py +++ b/custom_components/pyscript/config_flow.py @@ -5,7 +5,8 @@ import voluptuous as vol from homeassistant import config_entries -from homeassistant.config_entries import SOURCE_IMPORT +from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry +from homeassistant.core import callback from .const import CONF_ALLOW_ALL_IMPORTS, DOMAIN @@ -14,12 +15,52 @@ ) +class PyscriptOptionsConfigFlow(config_entries.OptionsFlow): + """Handle a pyscript options flow.""" + + def __init__(self, config_entry: ConfigEntry) -> None: + """Initialize pyscript options flow.""" + self.config_entry = config_entry + + async def async_step_init(self, user_input: Dict[str, Any] = None) -> Dict[str, Any]: + """Manage the pyscript options.""" + if self.config_entry.source == SOURCE_IMPORT: + return self.async_abort(reason="no_ui_configuration_allowed") + + if user_input is None: + return self.async_show_form( + step_id="init", + data_schema=vol.Schema( + { + vol.Optional( + CONF_ALLOW_ALL_IMPORTS, default=self.config_entry.data[CONF_ALLOW_ALL_IMPORTS], + ): bool + }, + extra=vol.ALLOW_EXTRA, + ), + ) + + if user_input[CONF_ALLOW_ALL_IMPORTS] != self.config_entry.data[CONF_ALLOW_ALL_IMPORTS]: + updated_data = self.config_entry.data.copy() + updated_data.update(user_input) + self.hass.config_entries.async_update_entry(entry=self.config_entry, data=updated_data) + return self.async_create_entry(title="", data={}) + + return self.async_abort(reason="no_update") + + class PyscriptConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a pyscript config flow.""" VERSION = 1 CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_PUSH + @staticmethod + @callback + def async_get_options_flow(config_entry: ConfigEntry) -> PyscriptOptionsConfigFlow: + """Get the options flow for this handler.""" + return PyscriptOptionsConfigFlow(config_entry) + async def async_step_user(self, user_input: Dict[str, Any] = None) -> Dict[str, Any]: """Handle a flow initialized by the user.""" if user_input is not None: diff --git a/custom_components/pyscript/eval.py b/custom_components/pyscript/eval.py index 7e0aff7..249e9b8 100644 --- a/custom_components/pyscript/eval.py +++ b/custom_components/pyscript/eval.py @@ -16,7 +16,7 @@ from homeassistant.const import SERVICE_RELOAD from homeassistant.helpers.service import async_set_service_schema -from .const import ALLOWED_IMPORTS, DOMAIN, LOGGER_PATH, SERVICE_JUPYTER_KERNEL_START +from .const import ALLOWED_IMPORTS, CONF_ALLOW_ALL_IMPORTS, DOMAIN, LOGGER_PATH, SERVICE_JUPYTER_KERNEL_START from .function import Function from .state import State @@ -721,7 +721,7 @@ def __init__(self, name, global_ctx, logger_name=None): self.logger_handlers = set() self.logger = None self.set_logger_name(logger_name if logger_name is not None else self.name) - self.allow_all_imports = Function.hass.data.get(DOMAIN, {}).get("allow_all_imports", False) + self.config_entry = Function.hass.data.get(DOMAIN, {}) async def ast_not_implemented(self, arg, *args): """Raise NotImplementedError exception for unimplemented AST types.""" @@ -769,7 +769,10 @@ async def ast_import(self, arg): self.exception_long = error_ctx.exception_long raise self.exception_obj if not mod: - if not self.allow_all_imports and imp.name not in ALLOWED_IMPORTS: + if ( + not self.config_entry.data.get(CONF_ALLOW_ALL_IMPORTS, False) + and imp.name not in ALLOWED_IMPORTS + ): raise ModuleNotFoundError(f"import of {imp.name} not allowed") if imp.name not in sys.modules: mod = await Function.hass.async_add_executor_job(importlib.import_module, imp.name) @@ -799,7 +802,10 @@ async def ast_importfrom(self, arg): self.exception_long = error_ctx.exception_long raise self.exception_obj if not mod: - if not self.allow_all_imports and arg.module not in ALLOWED_IMPORTS: + if ( + not self.config_entry.data.get(CONF_ALLOW_ALL_IMPORTS, False) + and arg.module not in ALLOWED_IMPORTS + ): raise ModuleNotFoundError(f"import from {arg.module} not allowed") if arg.module not in sys.modules: mod = await Function.hass.async_add_executor_job(importlib.import_module, arg.module) diff --git a/custom_components/pyscript/strings.json b/custom_components/pyscript/strings.json index 47dd727..81b8080 100644 --- a/custom_components/pyscript/strings.json +++ b/custom_components/pyscript/strings.json @@ -14,5 +14,19 @@ "single_instance_allowed": "Already configured. Only a single configuration possible.", "updated_entry": "This entry has already been setup but the configuration has been updated." } + }, + "options": { + "step": { + "init": { + "title": "Update pyscript configuration", + "data": { + "allow_all_imports": "Allow All Imports?" + } + } + }, + "abort": { + "no_ui_configuration_allowed": "This entry was created via `configuration.yaml`, so all configuration parameters must be updated there. The [`pyscript.reload`](developer-tools/service) service will allow you to apply the changes you make to `configuration.yaml` without restarting your Home Assistant instance.", + "no_update": "There is nothing to update." + } } -} \ No newline at end of file +} diff --git a/custom_components/pyscript/translations/en.json b/custom_components/pyscript/translations/en.json index 47dd727..81b8080 100644 --- a/custom_components/pyscript/translations/en.json +++ b/custom_components/pyscript/translations/en.json @@ -14,5 +14,19 @@ "single_instance_allowed": "Already configured. Only a single configuration possible.", "updated_entry": "This entry has already been setup but the configuration has been updated." } + }, + "options": { + "step": { + "init": { + "title": "Update pyscript configuration", + "data": { + "allow_all_imports": "Allow All Imports?" + } + } + }, + "abort": { + "no_ui_configuration_allowed": "This entry was created via `configuration.yaml`, so all configuration parameters must be updated there. The [`pyscript.reload`](developer-tools/service) service will allow you to apply the changes you make to `configuration.yaml` without restarting your Home Assistant instance.", + "no_update": "There is nothing to update." + } } -} \ No newline at end of file +} diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 4b54607..f518036 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -174,3 +174,65 @@ async def test_import_flow_update_import(hass): assert result["reason"] == "updated_entry" assert hass.config_entries.async_entries(DOMAIN)[0].data == {"apps": {"test_app": {"param": 1}}} + + +async def test_options_flow_import(hass): + """Test options flow aborts because configuration needs to be managed via configuration.yaml.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_IMPORT}, data=PYSCRIPT_SCHEMA({CONF_ALLOW_ALL_IMPORTS: True}) + ) + await hass.async_block_till_done() + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + entry = result["result"] + + result = await hass.config_entries.options.async_init(entry.entry_id, data=None) + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "no_ui_configuration_allowed" + + +async def test_options_flow_user_change(hass): + """Test options flow updates config entry when options change.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER}, data=PYSCRIPT_SCHEMA({CONF_ALLOW_ALL_IMPORTS: True}) + ) + await hass.async_block_till_done() + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + entry = result["result"] + + result = await hass.config_entries.options.async_init(entry.entry_id) + + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "init" + + result = await hass.config_entries.options.async_configure( + result["flow_id"], user_input={CONF_ALLOW_ALL_IMPORTS: False} + ) + await hass.async_block_till_done() + + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["title"] == "" + + assert entry.data[CONF_ALLOW_ALL_IMPORTS] is False + + +async def test_options_flow_user_no_change(hass): + """Test options flow aborts when options don't change.""" + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USER}, data=PYSCRIPT_SCHEMA({CONF_ALLOW_ALL_IMPORTS: True}) + ) + await hass.async_block_till_done() + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + entry = result["result"] + + result = await hass.config_entries.options.async_init(entry.entry_id) + + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "init" + + result = await hass.config_entries.options.async_configure( + result["flow_id"], user_input={CONF_ALLOW_ALL_IMPORTS: True} + ) + + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "no_update" diff --git a/tests/test_unit_eval.py b/tests/test_unit_eval.py index 2873f50..b0f08c9 100644 --- a/tests/test_unit_eval.py +++ b/tests/test_unit_eval.py @@ -1,9 +1,11 @@ """Unit tests for Python interpreter.""" +from custom_components.pyscript.const import CONF_ALLOW_ALL_IMPORTS, DOMAIN from custom_components.pyscript.eval import AstEval from custom_components.pyscript.function import Function from custom_components.pyscript.global_ctx import GlobalContext, GlobalContextMgr from custom_components.pyscript.state import State +from pytest_homeassistant_custom_component.common import MockConfigEntry evalTests = [ ["1", 1], @@ -882,6 +884,7 @@ async def run_one_test(test_data): async def test_eval(hass): """Test interpreter.""" + hass.data[DOMAIN] = MockConfigEntry(domain=DOMAIN, data={CONF_ALLOW_ALL_IMPORTS: False}) Function.init(hass) State.init(hass) State.register_functions() @@ -1062,6 +1065,7 @@ async def run_one_test_exception(test_data): async def test_eval_exceptions(hass): """Test interpreter exceptions.""" + hass.data[DOMAIN] = MockConfigEntry(domain=DOMAIN, data={CONF_ALLOW_ALL_IMPORTS: False}) Function.init(hass) State.init(hass) State.register_functions() From b22e0084af01bf33bbbee3fd66a97376777f38d2 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 15 Oct 2020 22:14:58 -0400 Subject: [PATCH 2/2] use backwards compatible approach --- custom_components/pyscript/config_flow.py | 25 +++++++++++++++++-- custom_components/pyscript/strings.json | 12 ++++++--- .../pyscript/translations/en.json | 12 ++++++--- tests/test_config_flow.py | 18 ++++++++++--- 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/custom_components/pyscript/config_flow.py b/custom_components/pyscript/config_flow.py index 381166b..821f192 100644 --- a/custom_components/pyscript/config_flow.py +++ b/custom_components/pyscript/config_flow.py @@ -21,11 +21,13 @@ class PyscriptOptionsConfigFlow(config_entries.OptionsFlow): def __init__(self, config_entry: ConfigEntry) -> None: """Initialize pyscript options flow.""" self.config_entry = config_entry + self._show_form = False async def async_step_init(self, user_input: Dict[str, Any] = None) -> Dict[str, Any]: """Manage the pyscript options.""" if self.config_entry.source == SOURCE_IMPORT: - return self.async_abort(reason="no_ui_configuration_allowed") + self._show_form = True + return await self.async_step_no_ui_configuration_allowed() if user_input is None: return self.async_show_form( @@ -46,7 +48,26 @@ async def async_step_init(self, user_input: Dict[str, Any] = None) -> Dict[str, self.hass.config_entries.async_update_entry(entry=self.config_entry, data=updated_data) return self.async_create_entry(title="", data={}) - return self.async_abort(reason="no_update") + self._show_form = True + return await self.async_step_no_update() + + async def async_step_no_ui_configuration_allowed( + self, user_input: Dict[str, Any] = None + ) -> Dict[str, Any]: + """Tell user no UI configuration is allowed.""" + if self._show_form: + self._show_form = False + return self.async_show_form(step_id="no_ui_configuration_allowed", data_schema=vol.Schema({})) + + return self.async_create_entry(title="", data={}) + + async def async_step_no_update(self, user_input: Dict[str, Any] = None) -> Dict[str, Any]: + """Tell user no update to process.""" + if self._show_form: + self._show_form = False + return self.async_show_form(step_id="no_update", data_schema=vol.Schema({})) + + return self.async_create_entry(title="", data={}) class PyscriptConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): diff --git a/custom_components/pyscript/strings.json b/custom_components/pyscript/strings.json index 81b8080..dfcbd73 100644 --- a/custom_components/pyscript/strings.json +++ b/custom_components/pyscript/strings.json @@ -22,11 +22,15 @@ "data": { "allow_all_imports": "Allow All Imports?" } + }, + "no_ui_configuration_allowed": { + "title": "No UI configuration allowed", + "description": "This entry was created via `configuration.yaml`, so all configuration parameters must be updated there. The [`pyscript.reload`](developer-tools/service) service will allow you to apply the changes you make to `configuration.yaml` without restarting your Home Assistant instance." + }, + "no_update": { + "title": "No update needed", + "description": "There is nothing to update." } - }, - "abort": { - "no_ui_configuration_allowed": "This entry was created via `configuration.yaml`, so all configuration parameters must be updated there. The [`pyscript.reload`](developer-tools/service) service will allow you to apply the changes you make to `configuration.yaml` without restarting your Home Assistant instance.", - "no_update": "There is nothing to update." } } } diff --git a/custom_components/pyscript/translations/en.json b/custom_components/pyscript/translations/en.json index 81b8080..dfcbd73 100644 --- a/custom_components/pyscript/translations/en.json +++ b/custom_components/pyscript/translations/en.json @@ -22,11 +22,15 @@ "data": { "allow_all_imports": "Allow All Imports?" } + }, + "no_ui_configuration_allowed": { + "title": "No UI configuration allowed", + "description": "This entry was created via `configuration.yaml`, so all configuration parameters must be updated there. The [`pyscript.reload`](developer-tools/service) service will allow you to apply the changes you make to `configuration.yaml` without restarting your Home Assistant instance." + }, + "no_update": { + "title": "No update needed", + "description": "There is nothing to update." } - }, - "abort": { - "no_ui_configuration_allowed": "This entry was created via `configuration.yaml`, so all configuration parameters must be updated there. The [`pyscript.reload`](developer-tools/service) service will allow you to apply the changes you make to `configuration.yaml` without restarting your Home Assistant instance.", - "no_update": "There is nothing to update." } } } diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index f518036..d8c4ecf 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -187,8 +187,13 @@ async def test_options_flow_import(hass): result = await hass.config_entries.options.async_init(entry.entry_id, data=None) - assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT - assert result["reason"] == "no_ui_configuration_allowed" + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "no_ui_configuration_allowed" + + result = await hass.config_entries.options.async_configure(result["flow_id"], user_input=None) + + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["title"] == "" async def test_options_flow_user_change(hass): @@ -234,5 +239,10 @@ async def test_options_flow_user_no_change(hass): result["flow_id"], user_input={CONF_ALLOW_ALL_IMPORTS: True} ) - assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT - assert result["reason"] == "no_update" + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "no_update" + + result = await hass.config_entries.options.async_configure(result["flow_id"], user_input=None) + + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["title"] == ""