fix further valiation issues (#1307, #1308)

bihealth · Oct 24, 2023 · 31357b3 · 31357b3
1 parent 349abeb
commit 31357b3
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 56 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,