diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3918f471..7433bdec 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -32,10 +32,13 @@ Fixed - Request object not provided to ``perform_project_modify()`` on create (#1301) - ``validate_form_app_settings()`` not called in ``ProjectForm`` (#1305) - Unhandled exceptions in ``validate_form_app_settings()`` calls (#1306) + - ``validate_form_app_settings()`` results handling crash in ``ProjectForm`` (#1307) + - Owner ``RoleAssignment`` provided to ``validate_form_app_settings()`` in ``ProjectForm`` (#1308) - **Timeline** - ``get_timestamp()`` template tag crash from missing ``ProjectEventStatus`` (#1297) - **Userprofile** - Unhandled exceptions in ``validate_form_app_settings()`` calls (#1306) + - ``validate_form_app_settings()`` results handling crash in ``UserSettingForm`` (#1307) Removed ------- diff --git a/docs/source/major_changes.rst b/docs/source/major_changes.rst index f77fb620..81383d3d 100644 --- a/docs/source/major_changes.rst +++ b/docs/source/major_changes.rst @@ -15,6 +15,7 @@ v0.13.3 (WIP) - Add common project badge template - Fix hidden JSON project setting reset on non-superuser project update +- Fix custom app setting validation calls in forms - General bug fixes and minor updates diff --git a/example_project_app/plugins.py b/example_project_app/plugins.py index d4e3760b..9131b2aa 100644 --- a/example_project_app/plugins.py +++ b/example_project_app/plugins.py @@ -348,6 +348,7 @@ def perform_project_modify( ), ) + # TODO: Fix this and update tests def validate_form_app_settings(self, app_settings, project=None, user=None): """Example implementation for app setting validation plugin API""" errors = {} diff --git a/projectroles/forms.py b/projectroles/forms.py index 3d0f84bd..81aa728b 100644 --- a/projectroles/forms.py +++ b/projectroles/forms.py @@ -529,29 +529,22 @@ def _validate_app_settings( app_settings, p_kwargs, instance, - instance_owner_as, ): """Validate and clean app_settings form fields""" errors = [] for plugin in app_plugins + [None]: - if plugin: - p_name = plugin.name - p_settings = app_settings.get_definitions( - APP_SETTING_SCOPE_PROJECT, app_name=p_name, **p_kwargs - ) - else: - p_name = 'projectroles' - p_settings = app_settings.get_definitions( - APP_SETTING_SCOPE_PROJECT, app_name=p_name, **p_kwargs - ) + p_name = plugin.name if plugin else 'projectroles' + p_defs = app_settings.get_definitions( + APP_SETTING_SCOPE_PROJECT, app_name=p_name, **p_kwargs + ) + p_settings = {} - plugin_app_settings = {} - for s_key, s_val in p_settings.items(): - s_field = 'settings.{}.{}'.format(p_name, s_key) - plugin_app_settings[s_key] = cleaned_data.get(s_field) + for s_key, s_val in p_defs.items(): + s_field = '.'.join(['settings', p_name, s_key]) + p_settings[s_key] = cleaned_data.get(s_field) if s_val['type'] == 'JSON': - if not plugin_app_settings[s_key]: + if not p_settings[s_key]: cleaned_data[s_field] = '{}' try: cleaned_data[s_field] = json.loads( @@ -560,9 +553,7 @@ def _validate_app_settings( except json.JSONDecodeError as err: errors.append((s_field, 'Invalid JSON\n' + str(err))) elif s_val['type'] == 'INTEGER': - # When the field is a select/dropdown the information of - # the datatype gets lost. We need to convert that here, - # otherwise subsequent checks will fail. + # Convert integers from select fields cleaned_data[s_field] = int(cleaned_data[s_field]) if not app_settings.validate( @@ -573,18 +564,16 @@ def _validate_app_settings( ): errors.append((s_field, 'Invalid value')) - # Custom validation for app settings + # Custom plugin validation for app settings if plugin and hasattr(plugin, 'validate_form_app_settings'): try: - app_settings_errors = plugin.validate_form_app_settings( - plugin_app_settings, - project=instance, - user=instance_owner_as, + p_errors = plugin.validate_form_app_settings( + p_settings, project=instance ) - if app_settings_errors: - for field, error in app_settings_errors.items(): - if error: - errors.append((field, error)) + if p_errors: + for field, error in p_errors.items(): + f_name = '.'.join(['settings', p_name, field]) + errors.append((f_name, error)) except Exception as ex: err_msg = SETTING_CUSTOM_VALIDATE_ERROR.format( plugin=p_name, exception=ex @@ -784,7 +773,6 @@ def clean(self): self.app_settings, self.p_kwargs, self.instance, - self.instance_owner_as, ) for key, value in cleaned_data.items(): self.cleaned_data[key] = value diff --git a/userprofile/forms.py b/userprofile/forms.py index a5bf780e..4382f5bf 100644 --- a/userprofile/forms.py +++ b/userprofile/forms.py @@ -137,23 +137,15 @@ def __init__(self, *args, **kwargs): def clean(self): """Function for custom form validation and cleanup""" for plugin in self.app_plugins + [None]: - if plugin: - p_name = plugin.name - p_settings = app_settings.get_definitions( - APP_SETTING_SCOPE_USER, plugin=plugin, user_modifiable=True - ) - else: - p_name = 'projectroles' - p_settings = app_settings.get_definitions( - APP_SETTING_SCOPE_USER, - app_name=p_name, - user_modifiable=True, - ) - plugin_app_settings = {} + p_name = plugin.name if plugin else 'projectroles' + p_defs = app_settings.get_definitions( + APP_SETTING_SCOPE_USER, plugin=plugin, user_modifiable=True + ) + p_settings = {} - for s_key, s_val in p_settings.items(): - s_field = 'settings.{}.{}'.format(p_name, s_key) - plugin_app_settings[s_key] = self.cleaned_data.get(s_field) + for s_key, s_val in p_defs.items(): + s_field = '.'.join(['settings', p_name, s_key]) + p_settings[s_key] = self.cleaned_data.get(s_field) if s_val['type'] == 'JSON': if not self.cleaned_data.get(s_field): @@ -164,11 +156,8 @@ def clean(self): ) except json.JSONDecodeError as err: self.add_error(s_field, 'Invalid JSON\n' + str(err)) - elif s_val['type'] == 'INTEGER': - # When field is a select/dropdown, the information of the - # data type gets lost. We need to convert that here, - # otherwise subsequent checks will fail. + # Convert integers from select fields self.cleaned_data[s_field] = int(self.cleaned_data[s_field]) if not app_settings.validate( @@ -179,16 +168,16 @@ def clean(self): ): self.add_error(s_field, 'Invalid value') - # Custom validation for app settings + # Custom plugin validation for app settings if plugin and hasattr(plugin, 'validate_form_app_settings'): try: - app_settings_errors = plugin.validate_form_app_settings( - plugin_app_settings, user=self.user + p_errors = plugin.validate_form_app_settings( + p_settings, user=self.user ) - if app_settings_errors: - for field, error in app_settings_errors.items(): - if error: - self.add_error(field, error) + if p_errors: + for field, error in p_errors.items(): + f_name = '.'.join(['settings', p_name, field]) + self.add_error(f_name, error) except Exception as ex: self.add_error( None,