Fix missing filters for get settings validator (#5480)

* Fix missing filters for get settings validator

* merge default model instance filters and kwargs

* Added tests for validators

* Give it a try without the kwargs passed to clean in save function

* Added string for identification for debug statement

* Added more debug comments

* Added more debug prints

* Fix test debug

* Modiefied workflow

* trigger ci

* Fix test and remove unused kwargs

* Added debug prints

* Only run one test in ci

* Added more debug code

* Remove all debug prints and reset workflow

* Reset overlooked file

InvenTree/common/models.py

@@ -170,7 +170,7 @@ class BaseInvenTreeSetting(models.Model):
 
         do_cache = kwargs.pop('cache', True)
 
-        self.clean(**kwargs)
+        self.clean()
         self.validate_unique()
 
         # Execute before_save action
@@ -604,7 +604,7 @@ class BaseInvenTreeSetting(models.Model):
         """Return units for setting."""
         return self.__class__.get_setting_units(self.key, **self.get_filters_for_instance())
 
-    def clean(self, **kwargs):
+    def clean(self):
         """If a validator (or multiple validators) are defined for a particular setting key, run them against the 'value' field."""
         super().clean()
 
@@ -615,7 +615,7 @@ class BaseInvenTreeSetting(models.Model):
         elif self.is_bool():
             self.value = self.as_bool()
 
-        validator = self.__class__.get_setting_validator(self.key, **kwargs)
+        validator = self.__class__.get_setting_validator(self.key, **self.get_filters_for_instance())
 
         if validator is not None:
             self.run_validator(validator)

InvenTree/common/tests.py

@@ -9,6 +9,7 @@ from unittest import mock
 
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
+from django.core.exceptions import ValidationError
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import Client, TestCase
 from django.urls import reverse
@@ -142,6 +143,36 @@ class SettingsTest(InvenTreeTestCase):
         InvenTreeSetting.set_setting('CD', "world", self.user)
         self.assertEqual(InvenTreeSetting.check_all_settings(), (True, []))
 
+    @mock.patch("common.models.InvenTreeSetting.get_setting_definition")
+    def test_settings_validator(self, get_setting_definition):
+        """Make sure that the validator function gets called on set setting."""
+
+        def validator(x):
+            if x == "hello":
+                return x
+
+            raise ValidationError(f"{x} is not valid")
+
+        mock_validator = mock.Mock(side_effect=validator)
+
+        # define partial schema
+        settings_definition = {
+            "AB": {  # key that's has not already been accessed
+                "validator": mock_validator,
+            },
+        }
+
+        def mocked(key, **kwargs):
+            return settings_definition.get(key, {})
+        get_setting_definition.side_effect = mocked
+
+        InvenTreeSetting.set_setting("AB", "hello", self.user)
+        mock_validator.assert_called_with("hello")
+
+        with self.assertRaises(ValidationError):
+            InvenTreeSetting.set_setting("AB", "world", self.user)
+        mock_validator.assert_called_with("world")
+
     def run_settings_check(self, key, setting):
         """Test that all settings are valid.
 

InvenTree/plugin/samples/integration/sample.py

@@ -1,5 +1,8 @@
 """Sample implementations for IntegrationPlugin."""
 
+import json
+
+from django.core.exceptions import ValidationError
 from django.http import HttpResponse
 from django.urls import include, re_path
 from django.utils.translation import gettext_lazy as _
@@ -8,6 +11,14 @@ from plugin import InvenTreePlugin
 from plugin.mixins import AppMixin, NavigationMixin, SettingsMixin, UrlsMixin
 
 
+def validate_json(value):
+    """Example validator for json input."""
+    try:
+        json.loads(value)
+    except Exception as e:
+        raise ValidationError(str(e))
+
+
 class SampleIntegrationPlugin(AppMixin, SettingsMixin, UrlsMixin, NavigationMixin, InvenTreePlugin):
     """A full plugin example."""
 
@@ -78,6 +89,11 @@ class SampleIntegrationPlugin(AppMixin, SettingsMixin, UrlsMixin, NavigationMixi
             'description': 'A protected setting, hidden from the UI',
             'default': 'ABC-123',
             'protected': True,
+        },
+        'VALIDATOR_SETTING': {
+            'name': 'JSON validator Setting',
+            'description': 'A setting using a JSON validator',
+            'validator': validate_json,
         }
     }
 

InvenTree/plugin/samples/integration/test_sample.py

@@ -1,5 +1,7 @@
 """Unit tests for action plugins."""
 
+from django.core.exceptions import ValidationError
+
 from InvenTree.unit_test import InvenTreeTestCase
 from plugin import registry
 
@@ -22,3 +24,18 @@ class SampleIntegrationPluginTests(InvenTreeTestCase):
         self.assertEqual(plugin.check_settings(), (False, ['API_KEY']))
         plugin.set_setting('API_KEY', "dsfiodsfjsfdjsf")
         self.assertEqual(plugin.check_settings(), (True, []))
+
+        # validator
+    def test_settings_validator(self):
+        """Test settings validator for plugins."""
+
+        plugin = registry.get_plugin('sample')
+        valid_json = '{"ts": 13}'
+        not_valid_json = '{"ts""13"}'
+
+        # no error, should pass validator
+        plugin.set_setting('VALIDATOR_SETTING', valid_json)
+
+        # should throw an error
+        with self.assertRaises(ValidationError):
+            plugin.set_setting('VALIDATOR_SETTING', not_valid_json)