From 01ff562dcdfd4514462a614a3254a6da8ba49738 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Fri, 13 Nov 2020 11:50:58 +1100 Subject: [PATCH 1/7] Extra unit testing for settings forms / views --- InvenTree/InvenTree/urls.py | 2 +- InvenTree/common/models.py | 29 ++++--- InvenTree/common/test_views.py | 143 +++++++++++++++++++++++++++++++++ InvenTree/common/views.py | 29 +++++++ 4 files changed, 189 insertions(+), 14 deletions(-) create mode 100644 InvenTree/common/test_views.py diff --git a/InvenTree/InvenTree/urls.py b/InvenTree/InvenTree/urls.py index b7e0d64d18..944c6432b5 100644 --- a/InvenTree/InvenTree/urls.py +++ b/InvenTree/InvenTree/urls.py @@ -82,7 +82,7 @@ settings_urls = [ url(r'^purchase-order/?', SettingsView.as_view(template_name='InvenTree/settings/po.html'), name='settings-po'), url(r'^sales-order/?', SettingsView.as_view(template_name='InvenTree/settings/so.html'), name='settings-so'), - url(r'^(?P\d+)/edit/?', SettingEdit.as_view(), name='setting-edit'), + url(r'^(?P\d+)/edit/', SettingEdit.as_view(), name='setting-edit'), # Catch any other urls url(r'^.*$', SettingsView.as_view(template_name='InvenTree/settings/user.html'), name='settings'), diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index 53803dd94e..966a46c7a5 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -165,6 +165,11 @@ class InvenTreeSetting(models.Model): verbose_name = "InvenTree Setting" verbose_name_plural = "InvenTree Settings" + def save(self, *args, **kwargs): + + self.clean() + super().save(*args, **kwargs) + @classmethod def get_setting_name(cls, key): """ @@ -281,20 +286,15 @@ class InvenTreeSetting(models.Model): try: setting = InvenTreeSetting.objects.filter(key__iexact=key).first() - except OperationalError: - # Settings table has not been created yet! - return None except (ValueError, InvenTreeSetting.DoesNotExist): - - try: - # Attempt Create the setting if it does not exist - setting = InvenTreeSetting.create( - key=key, - value=InvenTreeSetting.get_default_value(key) - ) - except OperationalError: - # Settings table has not been created yet - setting = None + setting = None + + if not setting: + # Attempt Create the setting if it does not exist + setting = InvenTreeSetting.objects.create( + key=key, + value=InvenTreeSetting.get_default_value(key) + ) return setting @@ -403,6 +403,9 @@ class InvenTreeSetting(models.Model): if validator is not None: self.run_validator(validator) + if self.is_bool(): + self.value = InvenTree.helpers.str2bool(self.value) + def run_validator(self, validator): """ Run a validator against the 'value' field for this InvenTreeSetting object. diff --git a/InvenTree/common/test_views.py b/InvenTree/common/test_views.py new file mode 100644 index 0000000000..d948f93b02 --- /dev/null +++ b/InvenTree/common/test_views.py @@ -0,0 +1,143 @@ +""" +Unit tests for the views associated with the 'common' app +""" + +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +import json + +from django.test import TestCase +from django.urls import reverse +from django.contrib.auth import get_user_model + +from common.models import InvenTreeSetting + + +class SettingsViewTest(TestCase): + """ + Tests for the settings management views + """ + + fixtures = [ + 'settings', + ] + + def setUp(self): + super().setUp() + + # Create a user (required to access the views / forms) + self.user = get_user_model().objects.create_user( + username='username', + email='me@email.com', + password='password', + ) + + self.client.login(username='username', password='password') + + def get_url(self, pk): + return reverse('setting-edit', args=(pk,)) + + def get_setting(self, title): + + return InvenTreeSetting.get_setting_object(title) + + def get(self, url, status=200): + + response = self.client.get(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') + + self.assertEqual(response.status_code, status) + + data = json.loads(response.content) + + return response, data + + def post(self, url, data, valid=None): + + response = self.client.post(url, data, HTTP_X_REQUESTED_WITH='XMLHttpRequest') + + json_data = json.loads(response.content) + + # If a particular status code is required + if valid is not None: + if valid: + self.assertEqual(json_data['form_valid'], True) + else: + self.assertEqual(json_data['form_valid'], False) + + form_errors = json.loads(json_data['form_errors']) + + return json_data, form_errors + + def test_instance_name(self): + """ + Test that we can get the settings view for particular setting objects. + """ + + # Start with something basic - load the settings view for INVENTREE_INSTANCE + setting = self.get_setting('INVENTREE_INSTANCE') + + self.assertIsNotNone(setting) + self.assertEqual(setting.value, 'My very first InvenTree Instance') + + url = self.get_url(setting.pk) + + response = self.get(url) + + new_name = 'A new instance name!' + + # Change the instance name via the form + data, errors = self.post(url, {'value': new_name}, valid=True) + + name = InvenTreeSetting.get_setting('INVENTREE_INSTANCE') + + self.assertEqual(name, new_name) + + def test_choices(self): + """ + Tests for a setting which has choices + """ + + setting = InvenTreeSetting.get_setting_object('INVENTREE_DEFAULT_CURRENCY') + + # Default value! + self.assertEqual(setting.value, 'USD') + + url = self.get_url(setting.pk) + + # Try posting an invalid currency option + data, errors = self.post(url, {'value': 'XPQaaa'}, valid=False) + + self.assertIsNotNone(errors.get('value'), None) + + # Try posting a valid currency option + data, errors = self.post(url, {'value': 'AUD'}, valid=True) + + def test_binary_values(self): + """ + Test for binary value + """ + + setting = InvenTreeSetting.get_setting_object('PART_COMPONENT') + + self.assertTrue(setting.as_bool()) + + url = self.get_url(setting.pk) + + setting.value = True + setting.save() + + # Try posting some invalid values + # The value should be "cleaned" and stay the same + for value in ['', 'abc', 'cat', 'TRUETRUETRUE']: + self.post(url, {'value': value}, valid=True) + + # Try posting some valid (True) values + for value in [True, 'True', '1', 'yes']: + self.post(url, {'value': value}, valid=True) + self.assertTrue(InvenTreeSetting.get_setting('PART_COMPONENT')) + + # Try posting some valid (False) values + for value in [False, 'False']: + self.post(url, {'value': value}, valid=True) + self.assertFalse(InvenTreeSetting.get_setting('PART_COMPONENT')) diff --git a/InvenTree/common/views.py b/InvenTree/common/views.py index 3bf3769231..1f3c827532 100644 --- a/InvenTree/common/views.py +++ b/InvenTree/common/views.py @@ -72,3 +72,32 @@ class SettingEdit(AjaxUpdateView): form.fields['value'].help_text = description return form + + def validate(self, setting, form): + """ + Perform custom validation checks on the form data. + """ + + data = form.cleaned_data + + value = data.get('value', None) + + if setting.choices(): + """ + If a set of choices are provided for a given setting, + the provided value must be one of those choices. + """ + + choices = [choice[0] for choice in setting.choices()] + + if value not in choices: + form.add_error('value', _('Supplied value is not allowed')) + + if setting.is_bool(): + """ + If a setting is defined as a boolean setting, + the provided value must look somewhat like a boolean value! + """ + + if not str2bool(value, test=True) and not str2bool(value, test=False): + form.add_error('value', _('Supplied value must be a boolean')) From ee70e27f7d0d2d2022cfa297e3fd336c8ed043b8 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Fri, 13 Nov 2020 13:21:43 +1100 Subject: [PATCH 2/7] Change function name --- InvenTree/common/models.py | 8 ++++---- InvenTree/common/tests.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index 966a46c7a5..a70aa609a0 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -235,7 +235,7 @@ class InvenTreeSetting(models.Model): return None @classmethod - def get_default_value(cls, key): + def get_setting_default(cls, key): """ Return the default value for a particular setting. @@ -293,7 +293,7 @@ class InvenTreeSetting(models.Model): # Attempt Create the setting if it does not exist setting = InvenTreeSetting.objects.create( key=key, - value=InvenTreeSetting.get_default_value(key) + value=InvenTreeSetting.get_setting_default(key) ) return setting @@ -322,7 +322,7 @@ class InvenTreeSetting(models.Model): # If no backup value is specified, atttempt to retrieve a "default" value if backup_value is None: - backup_value = cls.get_default_value(key) + backup_value = cls.get_setting_default(key) setting = InvenTreeSetting.get_setting_object(key) @@ -380,7 +380,7 @@ class InvenTreeSetting(models.Model): @property def default_value(self): - return InvenTreeSetting.get_default_value(self.key) + return InvenTreeSetting.get_setting_default(self.key) @property def description(self): diff --git a/InvenTree/common/tests.py b/InvenTree/common/tests.py index 4666a0a5a6..6d6e517ef5 100644 --- a/InvenTree/common/tests.py +++ b/InvenTree/common/tests.py @@ -70,7 +70,7 @@ class SettingsTest(TestCase): for key in InvenTreeSetting.GLOBAL_SETTINGS.keys(): - value = InvenTreeSetting.get_default_value(key) + value = InvenTreeSetting.get_setting_default(key) InvenTreeSetting.set_setting(key, value, self.user) From 03e852f95726898fb9300280869d56c6fb6854df Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Fri, 13 Nov 2020 20:22:28 +1100 Subject: [PATCH 3/7] Remove custom save method --- InvenTree/common/models.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index a70aa609a0..e189db33f4 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -165,11 +165,6 @@ class InvenTreeSetting(models.Model): verbose_name = "InvenTree Setting" verbose_name_plural = "InvenTree Settings" - def save(self, *args, **kwargs): - - self.clean() - super().save(*args, **kwargs) - @classmethod def get_setting_name(cls, key): """ From 5f9758e85fcf32148f6e12672a434768ded221ba Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Fri, 13 Nov 2020 21:01:30 +1100 Subject: [PATCH 4/7] More fixes --- InvenTree/common/models.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index e189db33f4..2563ee456b 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -9,6 +9,7 @@ from __future__ import unicode_literals import os from django.db import models +from django.db.utils import IntegrityError from django.conf import settings import djmoney.settings @@ -283,10 +284,13 @@ class InvenTreeSetting(models.Model): setting = InvenTreeSetting.objects.filter(key__iexact=key).first() except (ValueError, InvenTreeSetting.DoesNotExist): setting = None + except (IntegrityError): + setting = None if not setting: - # Attempt Create the setting if it does not exist - setting = InvenTreeSetting.objects.create( + # Return a new setting object if it does not already exist + # Do not save it to the database, though + setting = InvenTreeSetting( key=key, value=InvenTreeSetting.get_setting_default(key) ) From aae14009291e6782b0b10a9830959d4792043c26 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Fri, 13 Nov 2020 21:37:39 +1100 Subject: [PATCH 5/7] Mayyyyyyyyyyyyybe? --- InvenTree/common/models.py | 19 ++++++++++--------- InvenTree/common/test_views.py | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index 2563ee456b..97a0b024c4 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -9,7 +9,7 @@ from __future__ import unicode_literals import os from django.db import models -from django.db.utils import IntegrityError +from django.db.utils import IntegrityError, OperationalError from django.conf import settings import djmoney.settings @@ -17,7 +17,6 @@ from djmoney.models.fields import MoneyField from djmoney.contrib.exchange.models import convert_money from djmoney.contrib.exchange.exceptions import MissingRate -from django.db.utils import OperationalError from django.utils.translation import ugettext as _ from django.core.validators import MinValueValidator from django.core.exceptions import ValidationError @@ -284,16 +283,18 @@ class InvenTreeSetting(models.Model): setting = InvenTreeSetting.objects.filter(key__iexact=key).first() except (ValueError, InvenTreeSetting.DoesNotExist): setting = None - except (IntegrityError): + except (IntegrityError, OperationalError): setting = None + # Setting does not exist! (Try to create it) if not setting: - # Return a new setting object if it does not already exist - # Do not save it to the database, though - setting = InvenTreeSetting( - key=key, - value=InvenTreeSetting.get_setting_default(key) - ) + try: + # Attempt to create a new settin object + setting = InvenTreeSetting.objects.create(key=key, value=InvenTreeSetting.get_setting_default(key)) + except (IntegrityError, OperationalError): + # It might be the case that the database isn't created yet + # In such a case, return the object (but do not save it!) + setting = InvenTreeSetting(key=key, value=InvenTreeSetting.get_setting_default(key)) return setting diff --git a/InvenTree/common/test_views.py b/InvenTree/common/test_views.py index d948f93b02..0cd902d083 100644 --- a/InvenTree/common/test_views.py +++ b/InvenTree/common/test_views.py @@ -82,7 +82,7 @@ class SettingsViewTest(TestCase): url = self.get_url(setting.pk) - response = self.get(url) + self.get(url) new_name = 'A new instance name!' From b738f8b1432d1862b85b23936ce5d83de88fb84b Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Fri, 13 Nov 2020 22:22:02 +1100 Subject: [PATCH 6/7] Try transaction.atomic --- InvenTree/common/models.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index 97a0b024c4..234b64124f 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -8,7 +8,7 @@ from __future__ import unicode_literals import os -from django.db import models +from django.db import models, transaction from django.db.utils import IntegrityError, OperationalError from django.conf import settings @@ -288,13 +288,14 @@ class InvenTreeSetting(models.Model): # Setting does not exist! (Try to create it) if not setting: - try: - # Attempt to create a new settin object - setting = InvenTreeSetting.objects.create(key=key, value=InvenTreeSetting.get_setting_default(key)) + + setting = InvenTreeSetting(key=key, value=InvenTreeSetting.get_setting_default(key)) + + with transaction.atomic(): + setting.save() except (IntegrityError, OperationalError): # It might be the case that the database isn't created yet - # In such a case, return the object (but do not save it!) - setting = InvenTreeSetting(key=key, value=InvenTreeSetting.get_setting_default(key)) + pass return setting From 2e842503e626d287ef3467c12deeb09ae8c004a2 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Sat, 14 Nov 2020 07:39:51 +1100 Subject: [PATCH 7/7] Fix try statement --- InvenTree/common/models.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index 234b64124f..2f27dd602d 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -291,8 +291,10 @@ class InvenTreeSetting(models.Model): setting = InvenTreeSetting(key=key, value=InvenTreeSetting.get_setting_default(key)) - with transaction.atomic(): - setting.save() + try: + # Wrap this statement in "atomic", so it can be rolled back if it fails + with transaction.atomic(): + setting.save() except (IntegrityError, OperationalError): # It might be the case that the database isn't created yet pass