Skip to content

Commit

Permalink
Plugin setting fix (#7258)
Browse files Browse the repository at this point in the history
* Fix CUI URLs

* Fix for plugin setting API

- Fix conflict between "key" for PluginConfig and "key" for setting
- Needs to be "plugin" for plugin lookup, which accesses the "key" field in the PluginConfig model

* Fix for editing setting in PUI

* Add 'r' back in

* Remove debug code

* Update unit tests

* Bump API version

* Another unit test fix
  • Loading branch information
SchrodingersGat committed May 19, 2024
1 parent 2431fc6 commit e4dedb6
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 25 deletions.
5 changes: 4 additions & 1 deletion src/backend/InvenTree/InvenTree/api_version.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
"""InvenTree API version information."""

# InvenTree API version
INVENTREE_API_VERSION = 197
INVENTREE_API_VERSION = 198
"""Increment this API version number whenever there is a significant change to the API that any clients need to know about."""

INVENTREE_API_TEXT = """
v198 - 2024-05-19 : https://github.com/inventree/InvenTree/pull/7258
- Fixed lookup field conflicts in the plugins API
v197 - 2024-05-14 : https://github.com/inventree/InvenTree/pull/7224
- Refactor the plugin API endpoints to use the plugin "key" for lookup, rather than the PK value
Expand Down
6 changes: 3 additions & 3 deletions src/backend/InvenTree/common/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ def test_valid_plugin_slug(self):

# get data
url = reverse(
'api-plugin-setting-detail', kwargs={'key': 'sample', 'setting': 'API_KEY'}
'api-plugin-setting-detail', kwargs={'plugin': 'sample', 'key': 'API_KEY'}
)
response = self.get(url, expected_code=200)

Expand All @@ -637,15 +637,15 @@ def test_valid_plugin_slug(self):
# Non-existent plugin
url = reverse(
'api-plugin-setting-detail',
kwargs={'key': 'doesnotexist', 'setting': 'doesnotmatter'},
kwargs={'plugin': 'doesnotexist', 'key': 'doesnotmatter'},
)
response = self.get(url, expected_code=404)
self.assertIn("Plugin 'doesnotexist' not installed", str(response.data))

# Wrong key
url = reverse(
'api-plugin-setting-detail',
kwargs={'key': 'sample', 'setting': 'doesnotexist'},
kwargs={'plugin': 'sample', 'key': 'doesnotexist'},
)
response = self.get(url, expected_code=404)
self.assertIn(
Expand Down
15 changes: 9 additions & 6 deletions src/backend/InvenTree/plugin/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ class PluginDetail(RetrieveUpdateDestroyAPI):
queryset = PluginConfig.objects.all()
serializer_class = PluginSerializers.PluginConfigSerializer
lookup_field = 'key'
lookup_url_kwarg = 'plugin'

def delete(self, request, *args, **kwargs):
"""Handle DELETE request for a PluginConfig instance.
Expand Down Expand Up @@ -202,6 +203,7 @@ class PluginUninstall(UpdateAPI):
serializer_class = PluginSerializers.PluginUninstallSerializer
permission_classes = [IsSuperuser]
lookup_field = 'key'
lookup_url_kwarg = 'plugin'

def perform_update(self, serializer):
"""Uninstall the plugin."""
Expand All @@ -222,6 +224,7 @@ class PluginActivate(UpdateAPI):
serializer_class = PluginSerializers.PluginActivateSerializer
permission_classes = [IsSuperuser]
lookup_field = 'key'
lookup_url_kwarg = 'plugin'

def get_object(self):
"""Returns the object for the view."""
Expand Down Expand Up @@ -323,10 +326,10 @@ class PluginAllSettingList(APIView):
@extend_schema(
responses={200: PluginSerializers.PluginSettingSerializer(many=True)}
)
def get(self, request, key):
def get(self, request, plugin):
"""Get all settings for a plugin config."""
# look up the plugin
plugin = check_plugin(key, None)
plugin = check_plugin(plugin, None)

settings = getattr(plugin, 'settings', {})

Expand Down Expand Up @@ -355,10 +358,10 @@ def get_object(self):
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
"""
setting_key = self.kwargs['setting']
setting_key = self.kwargs['key']

# Look up plugin
plugin = check_plugin(self.kwargs.pop('key', None), None)
plugin = check_plugin(self.kwargs.get('plugin', None), None)

settings = getattr(plugin, 'settings', {})

Expand Down Expand Up @@ -433,13 +436,13 @@ def get(self, request):
),
# Lookup for individual plugins (based on 'key', not 'pk')
path(
'<str:key>/',
'<str:plugin>/',
include([
path(
'settings/',
include([
re_path(
r'^(?P<setting>\w+)/',
r'^(?P<key>\w+)/',
PluginSettingDetail.as_view(),
name='api-plugin-setting-detail',
),
Expand Down
15 changes: 7 additions & 8 deletions src/backend/InvenTree/plugin/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def assert_plugin_active(self, active):
assert plgs is not None
self.assertEqual(plgs.active, active)

url = reverse('api-plugin-detail-activate', kwargs={'key': test_plg.key})
url = reverse('api-plugin-detail-activate', kwargs={'plugin': test_plg.key})

# Should not work - not a superuser
response = self.client.post(url, {}, follow=True)
Expand Down Expand Up @@ -227,7 +227,7 @@ def test_plugin_settings(self):
cfg = PluginConfig.objects.filter(key='sample').first()
assert cfg is not None

url = reverse('api-plugin-detail-activate', kwargs={'key': cfg.key})
url = reverse('api-plugin-detail-activate', kwargs={'plugin': cfg.key})
self.client.patch(url, {}, expected_code=200)

# Valid plugin settings endpoints
Expand All @@ -236,8 +236,7 @@ def test_plugin_settings(self):
for key in valid_settings:
response = self.get(
reverse(
'api-plugin-setting-detail',
kwargs={'key': 'sample', 'setting': key},
'api-plugin-setting-detail', kwargs={'plugin': 'sample', 'key': key}
)
)

Expand All @@ -247,7 +246,7 @@ def test_plugin_settings(self):
response = self.get(
reverse(
'api-plugin-setting-detail',
kwargs={'key': 'sample', 'setting': 'INVALID_SETTING'},
kwargs={'plugin': 'sample', 'key': 'INVALID_SETTING'},
),
expected_code=404,
)
Expand All @@ -256,7 +255,7 @@ def test_plugin_settings(self):
response = self.get(
reverse(
'api-plugin-setting-detail',
kwargs={'key': 'sample', 'setting': 'PROTECTED_SETTING'},
kwargs={'plugin': 'sample', 'key': 'PROTECTED_SETTING'},
),
expected_code=200,
)
Expand All @@ -267,7 +266,7 @@ def test_plugin_settings(self):
response = self.patch(
reverse(
'api-plugin-setting-detail',
kwargs={'key': 'sample', 'setting': 'NUMERICAL_SETTING'},
kwargs={'plugin': 'sample', 'key': 'NUMERICAL_SETTING'},
),
{'value': 456},
expected_code=200,
Expand All @@ -279,7 +278,7 @@ def test_plugin_settings(self):
response = self.get(
reverse(
'api-plugin-setting-detail',
kwargs={'key': 'sample', 'setting': 'NUMERICAL_SETTING'},
kwargs={'plugin': 'sample', 'key': 'NUMERICAL_SETTING'},
),
expected_code=200,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
if (notification) {
url = `/api/settings/notification/${pk}/`;
} else if (plugin) {
url = `/api/plugins/settings/${plugin}/${setting}/`;
url = `/api/plugins/${plugin}/settings/${setting}/`;
} else if (user) {
url = `/api/settings/user/${setting}/`;
}
Expand Down
2 changes: 1 addition & 1 deletion src/backend/InvenTree/templates/js/dynamic/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function editSetting(key, options={}) {
var url = '';

if (plugin) {
url = `/api/plugins/settings/${plugin}/${key}/`;
url = `/api/plugins/${plugin}/settings/${key}/`;
} else if (notification) {
url = `/api/settings/notification/${pk}/`;
} else if (global) {
Expand Down
19 changes: 14 additions & 5 deletions src/frontend/src/components/settings/SettingList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,27 @@ export function SettingList({

const [setting, setSetting] = useState<Setting | undefined>(undefined);

// Determine the field type of the setting
const fieldType = useMemo(() => {
if (setting?.type != undefined) {
return setting.type;
}

if (setting?.choices != undefined && setting.choices.length > 0) {
return 'choice';
}

return 'string';
}, [setting]);

const editSettingModal = useEditApiFormModal({
url: settingsState.endpoint,
pk: setting?.key,
pathParams: settingsState.pathParams,
title: t`Edit Setting`,
fields: {
value: {
value: setting?.value ?? '',
field_type:
setting?.type ?? (setting?.choices?.length ?? 0) > 0
? 'choice'
: 'string',
field_type: fieldType,
label: setting?.name,
description: setting?.description,
api_url: setting?.api_url ?? '',
Expand Down

0 comments on commit e4dedb6

Please sign in to comment.