From ee274739a6e40a789fe2788faca5190c10ba318a Mon Sep 17 00:00:00 2001 From: Lukas <76838159+wolflu05@users.noreply.github.com> Date: Wed, 12 Jul 2023 00:19:19 +0200 Subject: [PATCH] Added required attribute to settings/plugins, refactor: allValues (#5224) * Added required attribute to settings/plugins, refactor: allValues - added 'required' attribute to InvenTreeBaseSetting - added 'check_all_settings' - added 'all_settings' to get a list of all defined settings - refactored 'allValues' to use new 'all_settings' function - added docs for new 'check_setting' function on plugin SettingsMixin * Fix typing to be compatible with python 3.9 * trigger: ci * Fixed **kwargs bug and added tests --- InvenTree/common/models.py | 104 +++++++++++++----- InvenTree/common/tests.py | 35 ++++++ .../plugin/base/integration/SettingsMixin.py | 13 +++ .../plugin/samples/integration/sample.py | 1 + .../plugin/samples/integration/test_sample.py | 11 ++ docs/docs/extend/plugins/settings.md | 4 +- 6 files changed, 142 insertions(+), 26 deletions(-) diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index 1b546dd0e2..2524ba7503 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -127,6 +127,7 @@ class SettingsKeyType(TypedDict, total=False): before_save: Function that gets called after save with *args, **kwargs (optional) after_save: Function that gets called after save with *args, **kwargs (optional) protected: Protected values are not returned to the client, instead "***" is returned (optional, default: False) + required: Is this setting required to work, can be used in combination with .check_all_settings(...) (optional, default: False) model: Auto create a dropdown menu to select an associated model instance (e.g. 'company.company', 'auth.user' and 'auth.group' are possible too, optional) """ @@ -140,6 +141,7 @@ class SettingsKeyType(TypedDict, total=False): before_save: Callable[..., None] after_save: Callable[..., None] protected: bool + required: bool model: str @@ -250,13 +252,15 @@ class BaseInvenTreeSetting(models.Model): return {key: getattr(self, key, None) for key in self.extra_unique_fields if hasattr(self, key)} @classmethod - def allValues(cls, exclude_hidden=False, **kwargs): - """Return a dict of "all" defined global settings. + def all_settings(cls, *, exclude_hidden=False, settings_definition: Union[Dict[str, SettingsKeyType], None] = None, **kwargs): + """Return a list of "all" defined settings. This performs a single database lookup, and then any settings which are not *in* the database are assigned their default values """ + filters = cls.get_filters(**kwargs) + results = cls.objects.all() if exclude_hidden: @@ -264,45 +268,83 @@ class BaseInvenTreeSetting(models.Model): results = results.exclude(key__startswith='_') # Optionally filter by other keys - results = results.filter(**cls.get_filters(**kwargs)) + results = results.filter(**filters) + + settings: Dict[str, BaseInvenTreeSetting] = {} # Query the database - settings = {} - for setting in results: if setting.key: - settings[setting.key.upper()] = setting.value + settings[setting.key.upper()] = setting # Specify any "default" values which are not in the database - for key in cls.SETTINGS.keys(): - + settings_definition = settings_definition or cls.SETTINGS + for key, setting in settings_definition.items(): if key.upper() not in settings: - settings[key.upper()] = cls.get_setting_default(key) + settings[key.upper()] = cls( + key=key.upper(), + value=cls.get_setting_default(key, **filters), + **filters + ) - if exclude_hidden: - hidden = cls.SETTINGS[key].get('hidden', False) + # remove any hidden settings + if exclude_hidden and setting.get("hidden", False): + del settings[key.upper()] - if hidden: - # Remove hidden items - del settings[key.upper()] + # format settings values and remove protected + for key, setting in settings.items(): + validator = cls.get_setting_validator(key, **filters) - for key, value in settings.items(): - validator = cls.get_setting_validator(key) - - if cls.is_protected(key): - value = '***' + if cls.is_protected(key, **filters) and setting.value != "": + setting.value = '***' elif cls.validator_is_bool(validator): - value = InvenTree.helpers.str2bool(value) + setting.value = InvenTree.helpers.str2bool(setting.value) elif cls.validator_is_int(validator): try: - value = int(value) + setting.value = int(setting.value) except ValueError: - value = cls.get_setting_default(key) - - settings[key] = value + setting.value = cls.get_setting_default(key, **filters) return settings + @classmethod + def allValues(cls, *, exclude_hidden=False, settings_definition: Union[Dict[str, SettingsKeyType], None] = None, **kwargs): + """Return a dict of "all" defined global settings. + + This performs a single database lookup, + and then any settings which are not *in* the database + are assigned their default values + """ + all_settings = cls.all_settings(exclude_hidden=exclude_hidden, settings_definition=settings_definition, **kwargs) + + settings: Dict[str, Any] = {} + + for key, setting in all_settings.items(): + settings[key] = setting.value + + return settings + + @classmethod + def check_all_settings(cls, *, exclude_hidden=False, settings_definition: Union[Dict[str, SettingsKeyType], None] = None, **kwargs): + """Check if all required settings are set by definition. + + Returns: + is_valid: Are all required settings defined + missing_settings: List of all settings that are missing (empty if is_valid is 'True') + """ + all_settings = cls.all_settings(exclude_hidden=exclude_hidden, settings_definition=settings_definition, **kwargs) + + missing_settings: List[str] = [] + + for setting in all_settings.values(): + if setting.required: + value = setting.value or cls.get_setting_default(setting.key, **kwargs) + + if value == "": + missing_settings.append(setting.key.upper()) + + return len(missing_settings) == 0, missing_settings + @classmethod def get_setting_definition(cls, key, **kwargs): """Return the 'definition' of a particular settings value, as a dict object. @@ -829,7 +871,7 @@ class BaseInvenTreeSetting(models.Model): @classmethod def is_protected(cls, key, **kwargs): """Check if the setting value is protected.""" - setting = cls.get_setting_definition(key, **kwargs) + setting = cls.get_setting_definition(key, **cls.get_filters(**kwargs)) return setting.get('protected', False) @@ -838,6 +880,18 @@ class BaseInvenTreeSetting(models.Model): """Returns if setting is protected from rendering.""" return self.__class__.is_protected(self.key, **self.get_filters_for_instance()) + @classmethod + def is_required(cls, key, **kwargs): + """Check if this setting value is required.""" + setting = cls.get_setting_definition(key, **cls.get_filters(**kwargs)) + + return setting.get("required", False) + + @property + def required(self): + """Returns if setting is required.""" + return self.__class__.is_required(self.key, **self.get_filters_for_instance()) + def settings_group_options(): """Build up group tuple for settings based on your choices.""" diff --git a/InvenTree/common/tests.py b/InvenTree/common/tests.py index 4402a16933..740de2e419 100644 --- a/InvenTree/common/tests.py +++ b/InvenTree/common/tests.py @@ -5,6 +5,7 @@ import json import time from datetime import timedelta from http import HTTPStatus +from unittest import mock from django.contrib.auth import get_user_model from django.core.cache import cache @@ -105,6 +106,40 @@ class SettingsTest(InvenTreeTestCase): self.assertIn('PART_COPY_TESTS', result) self.assertIn('STOCK_OWNERSHIP_CONTROL', result) self.assertIn('SIGNUP_GROUP', result) + self.assertIn('SERVER_RESTART_REQUIRED', result) + + result = InvenTreeSetting.allValues(exclude_hidden=True) + self.assertNotIn('SERVER_RESTART_REQUIRED', result) + + def test_all_settings(self): + """Make sure that the all_settings function returns correctly""" + result = InvenTreeSetting.all_settings() + self.assertIn("INVENTREE_INSTANCE", result) + self.assertIsInstance(result['INVENTREE_INSTANCE'], InvenTreeSetting) + + @mock.patch("common.models.InvenTreeSetting.get_setting_definition") + def test_check_all_settings(self, get_setting_definition): + """Make sure that the check_all_settings function returns correctly""" + # define partial schema + settings_definition = { + "AB": { # key that's has not already been accessed + "required": True, + }, + "CD": { + "required": True, + "protected": True, + }, + "EF": {} + } + + def mocked(key, **kwargs): + return settings_definition.get(key, {}) + get_setting_definition.side_effect = mocked + + self.assertEqual(InvenTreeSetting.check_all_settings(settings_definition=settings_definition), (False, ["AB", "CD"])) + InvenTreeSetting.set_setting('AB', "hello", self.user) + InvenTreeSetting.set_setting('CD', "world", self.user) + self.assertEqual(InvenTreeSetting.check_all_settings(), (True, [])) def run_settings_check(self, key, setting): """Test that all settings are valid. diff --git a/InvenTree/plugin/base/integration/SettingsMixin.py b/InvenTree/plugin/base/integration/SettingsMixin.py index 5f14dc063d..42427e7337 100644 --- a/InvenTree/plugin/base/integration/SettingsMixin.py +++ b/InvenTree/plugin/base/integration/SettingsMixin.py @@ -84,3 +84,16 @@ class SettingsMixin: return PluginSetting.set_setting(key, value, user, plugin=plugin) + + def check_settings(self): + """Check if all required settings for this machine are defined. + + Warning: This method cannot be used in the __init__ function of the plugin + + Returns: + is_valid: Are all required settings defined + missing_settings: List of all settings that are missing (empty if is_valid is 'True') + """ + from plugin.models import PluginSetting + + return PluginSetting.check_all_settings(settings_definition=self.settings, plugin=self.plugin_config()) diff --git a/InvenTree/plugin/samples/integration/sample.py b/InvenTree/plugin/samples/integration/sample.py index 402159455a..010f56f81d 100644 --- a/InvenTree/plugin/samples/integration/sample.py +++ b/InvenTree/plugin/samples/integration/sample.py @@ -44,6 +44,7 @@ class SampleIntegrationPlugin(AppMixin, SettingsMixin, UrlsMixin, NavigationMixi 'API_KEY': { 'name': _('API Key'), 'description': _('Key required for accessing external API'), + 'required': True, }, 'NUMERICAL_SETTING': { 'name': _('Numerical'), diff --git a/InvenTree/plugin/samples/integration/test_sample.py b/InvenTree/plugin/samples/integration/test_sample.py index 1aad91928e..17d5e6446c 100644 --- a/InvenTree/plugin/samples/integration/test_sample.py +++ b/InvenTree/plugin/samples/integration/test_sample.py @@ -1,6 +1,7 @@ """Unit tests for action plugins.""" from InvenTree.unit_test import InvenTreeTestCase +from plugin import registry class SampleIntegrationPluginTests(InvenTreeTestCase): @@ -11,3 +12,13 @@ class SampleIntegrationPluginTests(InvenTreeTestCase): response = self.client.get('/plugin/sample/ho/he/') self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b'Hi there testuser this works') + + def test_settings(self): + """Check the SettingsMixin.check_settings function.""" + plugin = registry.get_plugin('sample') + self.assertIsNotNone(plugin) + + # check settings + self.assertEqual(plugin.check_settings(), (False, ['API_KEY'])) + plugin.set_setting('API_KEY', "dsfiodsfjsfdjsf") + self.assertEqual(plugin.check_settings(), (True, [])) diff --git a/docs/docs/extend/plugins/settings.md b/docs/docs/extend/plugins/settings.md index 0dc43af878..178d5b1f92 100644 --- a/docs/docs/extend/plugins/settings.md +++ b/docs/docs/extend/plugins/settings.md @@ -35,6 +35,7 @@ class PluginWithSettings(SettingsMixin, InvenTreePlugin): 'name': 'API Key', 'description': 'Security key for accessing remote API', 'default': '', + 'required': True, }, 'API_URL': { 'name': _('API URL'), @@ -71,10 +72,11 @@ class PluginWithSettings(SettingsMixin, InvenTreePlugin): !!! tip "Hidden Settings" Plugin settings can be hidden from the settings page by marking them as 'hidden' -This mixin defines the helper functions `plugin.get_setting` and `plugin.set_setting` to access all plugin specific settings: +This mixin defines the helper functions `plugin.get_setting`, `plugin.set_setting` and `plugin.check_settings` to access all plugin specific settings. The `plugin.check_settings` function can be used to check if all settings marked with `'required': True` are defined and not equal to `''`. Note that these methods cannot be used in the `__init__` function of your plugin. ```python api_url = self.get_setting('API_URL', cache = False) self.set_setting('API_URL', 'some value') +is_valid, missing_settings = self.check_settings() ``` `get_setting` has an additional parameter which lets control if the value is taken directly from the database or from the cache. If it is left away `False` is the default that means the value is taken directly from the database.