From 53d7dd9b2d445e649312ce0f5787cb54dda0046f Mon Sep 17 00:00:00 2001 From: Mikko Nieminen Date: Tue, 3 Sep 2024 13:22:20 +0200 Subject: [PATCH] update compare_value() (#1479), fix bool comparison (#1473) --- CHANGELOG.rst | 3 + projectroles/app_settings.py | 40 +++++------ projectroles/remote_projects.py | 2 +- .../templatetags/projectroles_common_tags.py | 1 + projectroles/tests/test_app_settings.py | 66 +++++++++++++++++++ 5 files changed, 92 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cc8025e3..ed099e8a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,6 +20,7 @@ Changed - **Projectroles** - Truncate app setting values in ``remoteproject_sync.html`` (#1474) - JSON app setting value rendering in ``remoteproject_sync.html`` (#1472) + - Change ``AppSettingAPI.compare_value()`` into public method (#1479) Fixed ----- @@ -28,6 +29,8 @@ Fixed - Incorrect app plugin link order in ``get_project_app_links()`` (#1468) - Remote sync crash on updating user with additional email (#1476) - User scope app setting display in ``remoteproject_sync.html`` (#1478) + - Incorrect boolean comparison in ``AppSettingAPI._compare_value()`` with string value (#1473) + - Boolean app setting update status in remote sync (#1473) v1.0.1 (2024-08-08) diff --git a/projectroles/app_settings.py b/projectroles/app_settings.py index 6fe38362..c8b27459 100644 --- a/projectroles/app_settings.py +++ b/projectroles/app_settings.py @@ -302,24 +302,6 @@ def _get_json_value(cls, value): except Exception: raise ValueError('Value is not valid JSON: {}'.format(value)) - @classmethod - def _compare_value(cls, obj, input_value): - """ - Compare input value to value in an AppSetting object. Return True if - values match, False if there is a mismatch. - - :param obj: AppSetting object - :param input_value: Input value (string, int, bool or dict) - :return: Bool - """ - if obj.type == 'JSON': - return ( - not obj.value_json and not input_value - ) or obj.value_json == cls._get_json_value(input_value) - elif obj.type == 'BOOLEAN': - return bool(int(obj.value)) == input_value - return obj.value == str(input_value) - @classmethod def _log_set_debug( cls, action, plugin_name, setting_name, value, project, user @@ -587,7 +569,7 @@ def set( else: q_kwargs['app_plugin'] = None setting = AppSetting.objects.get(**q_kwargs) - if cls._compare_value(setting, value): + if cls.compare_value(setting, value): return False if validate: cls.validate( @@ -912,6 +894,26 @@ def get_global_value(cls, setting_def): return not setting_def['local'] # Inverse value return setting_def.get('global', APP_SETTING_GLOBAL_DEFAULT) + @classmethod + def compare_value(cls, obj, input_value): + """ + Compare input value to value in an AppSetting object. Return True if + values match, False if there is a mismatch. + + :param obj: AppSetting object + :param input_value: Input value (string, int, bool or dict) + :return: Bool + """ + if obj.type == 'JSON': + return ( + not obj.value_json and not input_value + ) or obj.value_json == cls._get_json_value(input_value) + elif obj.type == 'BOOLEAN': + if isinstance(input_value, str): + input_value = bool(int(input_value)) + return bool(int(obj.value)) == input_value + return obj.value == str(input_value) + def get_example_setting_default(project=None, user=None): """ diff --git a/projectroles/remote_projects.py b/projectroles/remote_projects.py index 0dae1549..f7bb1817 100644 --- a/projectroles/remote_projects.py +++ b/projectroles/remote_projects.py @@ -1175,7 +1175,7 @@ def _sync_app_setting(self, uuid, set_data): set_data['status'] = 'skipped' return # Skip if value is identical - if app_settings._compare_value( + if app_settings.compare_value( obj, ad['value_json'] if obj.type == 'JSON' else ad['value'] ): logger.info( diff --git a/projectroles/templatetags/projectroles_common_tags.py b/projectroles/templatetags/projectroles_common_tags.py index e1f5235a..98b01642 100644 --- a/projectroles/templatetags/projectroles_common_tags.py +++ b/projectroles/templatetags/projectroles_common_tags.py @@ -65,6 +65,7 @@ def get_user_by_uuid(sodar_uuid): """Return SODARUser by sodar_uuid""" return User.objects.filter(sodar_uuid=sodar_uuid).first() + @register.simple_tag def get_user_by_username(username): """Return User by username""" diff --git a/projectroles/tests/test_app_settings.py b/projectroles/tests/test_app_settings.py index a9262872..37ccfe06 100644 --- a/projectroles/tests/test_app_settings.py +++ b/projectroles/tests/test_app_settings.py @@ -1260,3 +1260,69 @@ def test_validate_form_app_settings_user_invalid(self): app_settings, user=self.user ) self.assertEqual(errors, {'user_str_setting': INVALID_SETTING_MSG}) + + def test_compare_value_string(self): + """Test compare_value() with string values""" + n = 'project_str_setting' + v = 'value' + vf = 'valueXYZ' + app_settings.set(EXAMPLE_APP_NAME, n, v, project=self.project) + obj = AppSetting.objects.get( + app_plugin__name=EXAMPLE_APP_NAME, name=n, project=self.project + ) + self.assertEqual(app_settings.compare_value(obj, v), True) + self.assertEqual(app_settings.compare_value(obj, vf), False) + + def test_compare_value_int(self): + """Test compare_value() with int values""" + n = 'project_int_setting' + v = 0 + vf = 1 + app_settings.set(EXAMPLE_APP_NAME, n, v, project=self.project) + obj = AppSetting.objects.get( + app_plugin__name=EXAMPLE_APP_NAME, name=n, project=self.project + ) + self.assertEqual(app_settings.compare_value(obj, v), True) + self.assertEqual(app_settings.compare_value(obj, vf), False) + self.assertEqual(app_settings.compare_value(obj, str(v)), True) + self.assertEqual(app_settings.compare_value(obj, str(vf)), False) + + def test_compare_value_bool(self): + """Test compare_value() with boolean values""" + n = 'project_bool_setting' + v = True + vf = False + app_settings.set(EXAMPLE_APP_NAME, n, v, project=self.project) + obj = AppSetting.objects.get( + app_plugin__name=EXAMPLE_APP_NAME, name=n, project=self.project + ) + self.assertEqual(app_settings.compare_value(obj, v), True) + self.assertEqual(app_settings.compare_value(obj, vf), False) + self.assertEqual(app_settings.compare_value(obj, '1'), True) + self.assertEqual(app_settings.compare_value(obj, '0'), False) + + def test_compare_value_json(self): + """Test compare_value() with JSON values""" + n = 'project_json_setting' + v = {'x': 1, 'y': 2} + vf = {'a': 3, 'b': 4} + app_settings.set(EXAMPLE_APP_NAME, n, v, project=self.project) + obj = AppSetting.objects.get( + app_plugin__name=EXAMPLE_APP_NAME, name=n, project=self.project + ) + self.assertEqual(app_settings.compare_value(obj, v), True) + self.assertEqual(app_settings.compare_value(obj, vf), False) + + def test_compare_value_json_empty(self): + """Test compare_value() with empty JSON values""" + n = 'project_json_setting' + v = {} + vf = {'x': 1, 'y': 2} + app_settings.set(EXAMPLE_APP_NAME, n, v, project=self.project) + obj = AppSetting.objects.get( + app_plugin__name=EXAMPLE_APP_NAME, name=n, project=self.project + ) + self.assertEqual(app_settings.compare_value(obj, v), True) + self.assertEqual(app_settings.compare_value(obj, vf), False) + self.assertEqual(app_settings.compare_value(obj, None), True) + self.assertEqual(app_settings.compare_value(obj, ''), True)