From 3a9bacb27ca5a99a580b33ccd31b48bfd2bff8ed Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Sun, 8 May 2022 20:08:22 +1000 Subject: [PATCH] Implement new approach for plugin settings - URL specifies plugin slug and setting key --- InvenTree/common/tests.py | 23 +++++++++++++ InvenTree/label/api.py | 15 ++++----- InvenTree/plugin/api.py | 32 ++++++++++++++++++- InvenTree/plugin/registry.py | 25 +++++++++++++-- .../templates/InvenTree/settings/setting.html | 4 +-- .../InvenTree/settings/settings.html | 8 ++--- InvenTree/templates/js/dynamic/settings.js | 8 ++--- 7 files changed, 91 insertions(+), 24 deletions(-) diff --git a/InvenTree/common/tests.py b/InvenTree/common/tests.py index f0b0dbdd05..3f32897ee1 100644 --- a/InvenTree/common/tests.py +++ b/InvenTree/common/tests.py @@ -465,12 +465,35 @@ class NotificationUserSettingsApiTest(InvenTreeAPITestCase): class PluginSettingsApiTest(InvenTreeAPITestCase): """Tests for the plugin settings API""" + def test_plugin_list(self): + """List installed plugins via API""" + url = reverse('api-plugin-list') + + response = self.get(url, expected_code=200) + def test_api_list(self): """Test list URL""" url = reverse('api-plugin-setting-list') self.get(url, expected_code=200) + def test_invalid_plugin_slug(self): + """Test that an invalid plugin slug returns a 404""" + + url = reverse('api-plugin-setting-detail', kwargs={'plugin': 'doesnotexist', 'key': 'doesnotmatter'}) + + response = self.get(url, expected_code=404) + + self.assertIn("Plugin 'doesnotexist' not installed", str(response.data)) + + def test_invalid_setting_key(self): + """Test that an invalid setting key returns a 404""" + ... + + def test_uninitialized_setting(self): + """Test that requesting an uninitialized setting creates the setting""" + ... + class WebhookMessageTests(TestCase): def setUp(self): diff --git a/InvenTree/label/api.py b/InvenTree/label/api.py index b580853b65..78e14f8cd3 100644 --- a/InvenTree/label/api.py +++ b/InvenTree/label/api.py @@ -71,16 +71,15 @@ class LabelPrintMixin: plugin_key = request.query_params.get('plugin', None) - for slug, plugin in registry.plugins.items(): + plugin = registry.get_plugin(plugin_key) - if slug == plugin_key and plugin.mixin_enabled('labels'): - - config = plugin.plugin_config() - - if config and config.active: - # Only return the plugin if it is enabled! - return plugin + if plugin: + config = plugin.plugin_config() + if config and config.active: + # Only return the plugin if it is enabled! + return plugin + # No matches found return None diff --git a/InvenTree/plugin/api.py b/InvenTree/plugin/api.py index a74f3dc677..49eec8d7a9 100644 --- a/InvenTree/plugin/api.py +++ b/InvenTree/plugin/api.py @@ -10,6 +10,7 @@ from django.urls import include, re_path from rest_framework import generics from rest_framework import status from rest_framework import permissions +from rest_framework.exceptions import NotFound from rest_framework.response import Response from django_filters.rest_framework import DjangoFilterBackend @@ -17,6 +18,7 @@ from django_filters.rest_framework import DjangoFilterBackend from common.api import GlobalSettingsPermissions from plugin.models import PluginConfig, PluginSetting import plugin.serializers as PluginSerializers +from plugin.registry import registry class PluginList(generics.ListAPIView): @@ -120,6 +122,34 @@ class PluginSettingDetail(generics.RetrieveUpdateAPIView): queryset = PluginSetting.objects.all() serializer_class = PluginSerializers.PluginSettingSerializer + def get_object(self): + """ + Lookup the plugin setting object, based on the URL. + The URL provides the 'slug' of the plugin, and the 'key' of the setting. + + Both the 'slug' and 'key' must be valid, else a 404 error is raised + """ + + plugin_slug = self.kwargs['plugin'] + key = self.kwargs['key'] + + # Check that the 'plugin' specified is valid! + if not PluginConfig.objects.filter(key=plugin_slug).exists(): + raise NotFound(detail=f"Plugin '{plugin_slug}' not installed") + + # Get the list of settings available for the specified plugin + plugin = registry.get_plugin(plugin_slug) + + if plugin is None: + raise NotFound(detail=f"Plugin '{plugin_slug}' not found") + + settings = getattr(plugin, 'SETTINGS', {}) + + if key not in settings: + raise NotFound(detail=f"Plugin '{plugin_slug}' has no setting matching '{key}'") + + return PluginSetting.get_setting_object(key, plugin=plugin) + # Staff permission required permission_classes = [ GlobalSettingsPermissions, @@ -130,7 +160,7 @@ plugin_api_urls = [ # Plugin settings URLs re_path(r'^settings/', include([ - re_path(r'^(?P\d+)/', PluginSettingDetail.as_view(), name='api-plugin-setting-detail'), + re_path(r'^(?P\w+)/(?P\w+)/', PluginSettingDetail.as_view(), name='api-plugin-setting-detail'), re_path(r'^.*$', PluginSettingList.as_view(), name='api-plugin-setting-list'), ])), diff --git a/InvenTree/plugin/registry.py b/InvenTree/plugin/registry.py index 45961d7a8b..db0dd12c48 100644 --- a/InvenTree/plugin/registry.py +++ b/InvenTree/plugin/registry.py @@ -63,6 +63,17 @@ class PluginsRegistry: # mixins self.mixins_settings = {} + def get_plugin(self, slug): + """ + Lookup plugin by slug (unique key). + """ + + if slug not in self.plugins: + logger.warning(f"Plugin registry has no record of plugin '{slug}'") + return None + + return self.plugins[slug] + def call_plugin_function(self, slug, func, *args, **kwargs): """ Call a member function (named by 'func') of the plugin named by 'slug'. @@ -73,11 +84,19 @@ class PluginsRegistry: Instead, any error messages are returned to the worker. """ - plugin = self.plugins[slug] + plugin = self.get_plugin(slug) - plugin_func = getattr(plugin, func) + if not plugin: + return - return plugin_func(*args, **kwargs) + # Check that the plugin is enabled + config = plugin.plugin_config() + + if config and config.active: + + plugin_func = getattr(plugin, func) + + return plugin_func(*args, **kwargs) # region public functions # region loading / unloading diff --git a/InvenTree/templates/InvenTree/settings/setting.html b/InvenTree/templates/InvenTree/settings/setting.html index 95865700fe..0bc099f8a2 100644 --- a/InvenTree/templates/InvenTree/settings/setting.html +++ b/InvenTree/templates/InvenTree/settings/setting.html @@ -24,7 +24,7 @@ {% if setting.is_bool %}
- +
{% else %}
@@ -41,7 +41,7 @@ {{ setting.units }}
-
diff --git a/InvenTree/templates/InvenTree/settings/settings.html b/InvenTree/templates/InvenTree/settings/settings.html index 5b73151d91..b5af49ddea 100644 --- a/InvenTree/templates/InvenTree/settings/settings.html +++ b/InvenTree/templates/InvenTree/settings/settings.html @@ -67,7 +67,6 @@ $('table').find('.boolean-setting').change(function() { var setting = $(this).attr('setting'); - var pk = $(this).attr('pk'); var plugin = $(this).attr('plugin'); var user = $(this).attr('user'); var notification = $(this).attr('notification'); @@ -78,7 +77,7 @@ $('table').find('.boolean-setting').change(function() { var url = `/api/settings/global/${setting}/`; if (plugin) { - url = `/api/plugin/settings/${pk}/`; + url = `/api/plugin/settings/${plugin}/${setting}/`; } else if (user) { url = `/api/settings/user/${setting}/`; } else if (notification) { @@ -105,7 +104,6 @@ $('table').find('.boolean-setting').change(function() { // Callback for when non-boolean settings are edited $('table').find('.btn-edit-setting').click(function() { var setting = $(this).attr('setting'); - var pk = $(this).attr('pk'); var plugin = $(this).attr('plugin'); var is_global = true; var notification = $(this).attr('notification'); @@ -122,13 +120,11 @@ $('table').find('.btn-edit-setting').click(function() { title = '{% trans "Edit Notification Setting" %}'; } else if (is_global) { title = '{% trans "Edit Global Setting" %}'; - pk = setting; } else { title = '{% trans "Edit User Setting" %}'; - pk = setting; } - editSetting(pk, { + editSetting(setting, { plugin: plugin, global: is_global, notification: notification, diff --git a/InvenTree/templates/js/dynamic/settings.js b/InvenTree/templates/js/dynamic/settings.js index 6f0c904f1a..21eb9df5e2 100644 --- a/InvenTree/templates/js/dynamic/settings.js +++ b/InvenTree/templates/js/dynamic/settings.js @@ -32,7 +32,7 @@ const plugins_enabled = false; * Interactively edit a setting value. * Launches a modal dialog form to adjut the value of the setting. */ -function editSetting(pk, options={}) { +function editSetting(key, options={}) { // Is this a global setting or a user setting? var global = options.global || false; @@ -44,13 +44,13 @@ function editSetting(pk, options={}) { var url = ''; if (plugin) { - url = `/api/plugin/settings/${pk}/`; + url = `/api/plugin/settings/${plugin}/${key}/`; } else if (notification) { url = `/api/settings/notification/${pk}/`; } else if (global) { - url = `/api/settings/global/${pk}/`; + url = `/api/settings/global/${key}/`; } else { - url = `/api/settings/user/${pk}/`; + url = `/api/settings/user/${key}/`; } var reload_required = false;