Skip to content

Commit

Permalink
fix app setting options validation (#1564, #1565)
Browse files Browse the repository at this point in the history
  • Loading branch information
mikkonie committed Feb 13, 2025
1 parent 1357cf4 commit 3b0745d
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ Fixed
- Missing fields in ``ProjectRetrieveAPIView`` docstring (#1551)
- Role delete alert dismissal fails with nested inherited roles (#1556)
- Incorrect initial "N/A" access status for categories in project list (#1005)
- App settings option validation as tuples (#1564)

Removed
-------
Expand Down
25 changes: 14 additions & 11 deletions projectroles/app_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@
APP_SETTING_SCOPE_PROJECT_USER: {'project': True, 'user': True},
APP_SETTING_SCOPE_SITE: {'project': False, 'user': False},
}
# TODO: Add label for -1 once validation is fixed (see #1564, #1565)
PROJECT_LIST_PAGE_OPTIONS = [10, 25, 50, 100, -1]
PROJECT_LIST_PAGE_OPTIONS = [10, 25, 50, 100, (-1, 'No pagination')]
DELETE_SCOPE_ERR_MSG = 'Argument "{arg}" must be set for {scope} scope setting'
GLOBAL_PROJECT_ERR_MSG = (
'Overriding global settings for remote projects not allowed'
Expand Down Expand Up @@ -219,16 +218,20 @@ def _validate_value_in_options(
', '.join(map(str, valid_options)),
)
)
elif (
setting_options
and not callable(setting_options)
and setting_value not in setting_options
):
raise ValueError(
'Choice "{}" not found in options ({})'.format(
setting_value, ', '.join(map(str, setting_options))
else:
opts = [
o[0] if isinstance(o, tuple) else o for o in setting_options
]
if (
setting_options
and not callable(setting_options)
and setting_value not in opts
):
raise ValueError(
'Choice "{}" not found in options ({})'.format(
setting_value, ', '.join(map(str, setting_options))
)
)
)

@classmethod
def _get_app_plugin(cls, plugin_name):
Expand Down
21 changes: 12 additions & 9 deletions projectroles/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,19 @@ def set_app_setting_field(self, plugin_name, s_field, s_def):
**setting_kwargs,
)
elif s_def.options:
choices = []
for o in s_def.options:
if isinstance(o, tuple):
val = o[0]
legend = o[1]
else:
val = o
legend = o
if s_def.type == APP_SETTING_TYPE_INTEGER:
val = int(val)
choices.append((val, legend))
self.fields[s_field] = forms.ChoiceField(
choices=[
(
(int(option), int(option))
if s_def.type == APP_SETTING_TYPE_INTEGER
else (option, option)
)
for option in s_def.options
],
**setting_kwargs,
choices=choices, **setting_kwargs
)
# Other types
elif s_def.type == APP_SETTING_TYPE_STRING:
Expand Down
5 changes: 3 additions & 2 deletions projectroles/plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,17 +738,18 @@ def validate_default_in_options(cls, setting_default, setting_options):
:param setting_options: Setting options
:raise: ValueError if default is not found in options
"""
opts = [o[0] if isinstance(o, tuple) else o for o in setting_options]
if (
setting_options is not None
and not callable(setting_options)
and setting_default is not None
and not callable(setting_default)
and setting_default not in setting_options
and setting_default not in opts
):
raise ValueError(
'Default value "{}" not found in options ({})'.format(
setting_default,
', '.join([str(o) for o in setting_options]),
', '.join([str(o) for o in opts]),
)
)

Expand Down
11 changes: 11 additions & 0 deletions projectroles/tests/test_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,17 @@ def test_init_no_defaults(self):
}
self.assertEqual(s_def.__dict__, expected)

def test_init_option_default_tuple(self):
"""Test init with option label tuples and default value"""
s_def = PluginAppSettingDef(
name=DEF_NAME,
scope=APP_SETTING_SCOPE_PROJECT,
type=APP_SETTING_TYPE_STRING,
default='string2',
options=['string', ('string2', 'string2 label')],
)
self.assertIsNotNone(s_def)

def test_init_invalid_scope(self):
"""Test init with invalid scope"""
with self.assertRaises(ValueError):
Expand Down

0 comments on commit 3b0745d

Please sign in to comment.