From febe5809e75d02f68a139493fb94d7d641e6a542 Mon Sep 17 00:00:00 2001 From: Matthias Mair Date: Wed, 6 Aug 2025 00:47:23 +0200 Subject: [PATCH] chore(backend): increase coverage (#10121) * chore(backend): increase coverage * add a full api based install / uninstall test * fix asserted code * delete unreachable code * clean up unused code * add more notification tests * fix test * order currencies --- src/backend/InvenTree/common/apps.py | 34 +------ .../common/migrations/0001_initial.py | 4 +- .../migrations/0023_auto_20240602_1332.py | 6 +- src/backend/InvenTree/common/models.py | 10 +-- src/backend/InvenTree/common/notifications.py | 88 +------------------ .../InvenTree/common/test_migrations.py | 47 ++++++++++ src/backend/InvenTree/common/tests.py | 69 +++++++++++++++ .../builtin/integration/core_notifications.py | 5 +- src/backend/InvenTree/plugin/test_api.py | 52 +++++++++++ 9 files changed, 185 insertions(+), 130 deletions(-) diff --git a/src/backend/InvenTree/common/apps.py b/src/backend/InvenTree/common/apps.py index 021e6ccad4..d795c7136c 100644 --- a/src/backend/InvenTree/common/apps.py +++ b/src/backend/InvenTree/common/apps.py @@ -5,11 +5,7 @@ from django.apps import AppConfig import structlog import InvenTree.ready -from common.settings import ( - get_global_setting, - global_setting_overrides, - set_global_setting, -) +from common.settings import get_global_setting, set_global_setting logger = structlog.get_logger('inventree') @@ -24,11 +20,10 @@ class CommonConfig(AppConfig): def ready(self): """Initialize restart flag clearance on startup.""" - if InvenTree.ready.isRunningMigrations(): + if InvenTree.ready.isRunningMigrations(): # pragma: no cover return self.clear_restart_flag() - self.override_global_settings() def clear_restart_flag(self): """Clear the SERVER_RESTART_REQUIRED setting.""" @@ -40,28 +35,5 @@ class CommonConfig(AppConfig): if not InvenTree.ready.isImportingData(): set_global_setting('SERVER_RESTART_REQUIRED', False, None) - except Exception: + except Exception: # pragma: no cover pass - - def override_global_settings(self): - """Update global settings based on environment variables.""" - overrides = global_setting_overrides() - - if not overrides: - return - - for key, value in overrides.items(): - try: - current_value = get_global_setting(key, create=False) - - if current_value != value: - logger.info( - 'INVE-I1: Overriding global setting: %s = %s', - value, - current_value, - ) - set_global_setting(key, value, None, create=True) - - except Exception: - logger.warning('Failed to override global setting %s -> %s', key, value) - continue diff --git a/src/backend/InvenTree/common/migrations/0001_initial.py b/src/backend/InvenTree/common/migrations/0001_initial.py index 089844a70d..b9ac14acf6 100644 --- a/src/backend/InvenTree/common/migrations/0001_initial.py +++ b/src/backend/InvenTree/common/migrations/0001_initial.py @@ -18,13 +18,13 @@ class CreateModelOrSkip(migrations.CreateModel): try: super().database_forwards(app_label, schema_editor, from_state, to_state) - except Exception: + except Exception: # pragma: no cover pass def state_forwards(self, app_label, state) -> None: try: super().state_forwards(app_label, state) - except Exception: + except Exception: # pragma: no cover pass diff --git a/src/backend/InvenTree/common/migrations/0023_auto_20240602_1332.py b/src/backend/InvenTree/common/migrations/0023_auto_20240602_1332.py index 66b78f6476..a3cc1fd02a 100644 --- a/src/backend/InvenTree/common/migrations/0023_auto_20240602_1332.py +++ b/src/backend/InvenTree/common/migrations/0023_auto_20240602_1332.py @@ -49,18 +49,18 @@ def set_currencies(apps, schema_editor): value = ','.join(valid_codes) - if not settings.TESTING: + if not settings.TESTING: # pragma: no cover print(f"Found existing currency codes:", value) setting = InvenTreeSetting.objects.filter(key=key).first() if setting: - if not settings.TESTING: + if not settings.TESTING: # pragma: no cover print(f"- Updating existing setting for currency codes") setting.value = value setting.save() else: - if not settings.TESTING: + if not settings.TESTING: # pragma: no cover print(f"- Creating new setting for currency codes") setting = InvenTreeSetting(key=key, value=value) setting.save() diff --git a/src/backend/InvenTree/common/models.py b/src/backend/InvenTree/common/models.py index df01282760..f172ca2026 100644 --- a/src/backend/InvenTree/common/models.py +++ b/src/backend/InvenTree/common/models.py @@ -288,7 +288,7 @@ class BaseInvenTreeSetting(models.Model): try: cache.set(key, self, timeout=3600) - except Exception: + except Exception: # pragma: no cover pass @classmethod @@ -558,7 +558,7 @@ class BaseInvenTreeSetting(models.Model): or InvenTree.ready.isRunningMigrations() or InvenTree.ready.isRebuildingData() or InvenTree.ready.isRunningBackup() - ): + ): # pragma: no cover create = False access_global_cache = False @@ -706,7 +706,7 @@ class BaseInvenTreeSetting(models.Model): or InvenTree.ready.isRunningMigrations() or InvenTree.ready.isRebuildingData() or InvenTree.ready.isRunningBackup() - ): + ): # pragma: no cover return attempts = int(kwargs.get('attempts', 3)) @@ -731,7 +731,7 @@ class BaseInvenTreeSetting(models.Model): logger.warning("Database is locked, cannot set setting '%s'", key) # Likely the DB is locked - not much we can do here return - except Exception as exc: + except Exception as exc: # pragma: no cover logger.exception( "Error setting setting '%s' for %s: %s", key, str(cls), str(type(exc)) ) @@ -766,7 +766,7 @@ class BaseInvenTreeSetting(models.Model): except (OperationalError, ProgrammingError): logger.warning("Database is locked, cannot set setting '%s'", key) # Likely the DB is locked - not much we can do here - except Exception as exc: + except Exception as exc: # pragma: no cover # Some other error logger.exception( "Error setting setting '%s' for %s: %s", key, str(cls), str(type(exc)) diff --git a/src/backend/InvenTree/common/notifications.py b/src/backend/InvenTree/common/notifications.py index 3b1309c78f..0d17f159bc 100644 --- a/src/backend/InvenTree/common/notifications.py +++ b/src/backend/InvenTree/common/notifications.py @@ -22,92 +22,6 @@ logger = structlog.get_logger('inventree') # region methods -class NotificationMethod: - """Base class for notification methods.""" - - METHOD_NAME = '' - METHOD_ICON = None - CONTEXT_BUILTIN = ['name', 'message'] - CONTEXT_EXTRA = [] - GLOBAL_SETTING = None - USER_SETTING = None - - def __init__(self, obj: Model, category: str, targets: list, context) -> None: - """Check that the method is read. - - This checks that: - - All needed functions are implemented - - The method is not disabled via plugin - - All needed context values were provided - """ - # Check if a sending fnc is defined - if (not hasattr(self, 'send')) and (not hasattr(self, 'send_bulk')): - raise NotImplementedError( - 'A NotificationMethod must either define a `send` or a `send_bulk` method' - ) - - # No method name is no good - if self.METHOD_NAME in ('', None): - raise NotImplementedError( - f'The NotificationMethod {self.__class__} did not provide a METHOD_NAME' - ) - - # Check if plugin is disabled - if so do not gather targets etc. - if self.global_setting_disable(): - self.targets = None - return - - # Define arguments - self.obj = obj - self.category = category - self.targets = targets - self.context = self.check_context(context) - - # Gather targets - self.targets = self.get_targets() - - def check_context(self, context): - """Check that all values defined in the methods CONTEXT were provided in the current context.""" - - def check(ref, obj): - # the obj is not accessible so we are on the end - if not isinstance(obj, (list, dict, tuple)): - return ref - - # check if the ref exists - if isinstance(ref, str): - if not obj.get(ref): - return ref - return False - - # nested - elif isinstance(ref, (tuple, list)): - if len(ref) == 1: - return check(ref[0], obj) - ret = check(ref[0], obj) - if ret: - return ret - return check(ref[1:], obj[ref[0]]) - - # other cases -> raise - raise NotImplementedError( - 'This type can not be used as a context reference' - ) - - missing = [] - for item in (*self.CONTEXT_BUILTIN, *self.CONTEXT_EXTRA): - ret = check(item, context) - if ret: - missing.append(ret) - - if missing: - raise NotImplementedError( - f'The `context` is missing the following items:\n{missing}' - ) - - return context - - @dataclass() class NotificationBody: """Information needed to create a notification. @@ -182,7 +96,7 @@ def trigger_notification( kwargs: Additional arguments to pass to the notification method """ # Check if data is importing currently - if isImportingData() or isRebuildingData(): + if isImportingData() or isRebuildingData(): # pragma: no cover return targets = kwargs.get('targets') diff --git a/src/backend/InvenTree/common/test_migrations.py b/src/backend/InvenTree/common/test_migrations.py index 71d9449ed6..a82c3326a5 100644 --- a/src/backend/InvenTree/common/test_migrations.py +++ b/src/backend/InvenTree/common/test_migrations.py @@ -1,6 +1,7 @@ """Data migration unit tests for the 'common' app.""" import io +import os from django.core.files.base import ContentFile @@ -208,3 +209,49 @@ class TestForwardMigrations(MigratorTestCase): 'stockitem', ]: self.assertEqual(Attachment.objects.filter(model_type=model).count(), 2) + + +def prep_currency_migration(self, vals: str): + """Prepare the environment for the currency migration tests.""" + # Set keys + os.environ['INVENTREE_CURRENCIES'] = vals + + # And setting + InvenTreeSetting = self.old_state.apps.get_model('common', 'InvenTreeSetting') + + setting = InvenTreeSetting(key='CURRENCY_CODES', value='123') + setting.save() + + +class TestCurrencyMigration(MigratorTestCase): + """Test currency migration.""" + + migrate_from = ('common', '0022_projectcode_responsible') + migrate_to = ('common', '0023_auto_20240602_1332') + + def prepare(self): + """Prepare the environment for the migration test.""" + prep_currency_migration(self, 'USD,EUR,GBP') + + def test_currency_migration(self): + """Test that the currency migration works.""" + InvenTreeSetting = self.old_state.apps.get_model('common', 'InvenTreeSetting') + setting = InvenTreeSetting.objects.filter(key='CURRENCY_CODES').first() + self.assertEqual(setting.value, 'EUR,GBP,USD') + + +class TestCurrencyMigrationNo(MigratorTestCase): + """Test currency migration.""" + + migrate_from = ('common', '0022_projectcode_responsible') + migrate_to = ('common', '0023_auto_20240602_1332') + + def prepare(self): + """Prepare the environment for the migration test.""" + prep_currency_migration(self, 'YYY,ZZZ') + + def test_currency_migration(self): + """Test that no currency migration occurs if wrong currencies are set.""" + InvenTreeSetting = self.old_state.apps.get_model('common', 'InvenTreeSetting') + setting = InvenTreeSetting.objects.filter(key='CURRENCY_CODES').first() + self.assertEqual(setting.value, '123') diff --git a/src/backend/InvenTree/common/tests.py b/src/backend/InvenTree/common/tests.py index 779eb93a87..44f1840735 100644 --- a/src/backend/InvenTree/common/tests.py +++ b/src/backend/InvenTree/common/tests.py @@ -8,6 +8,7 @@ from http import HTTPStatus from unittest import mock from django.contrib.auth import get_user_model +from django.contrib.auth.models import Group from django.contrib.contenttypes.models import ContentType from django.core.cache import cache from django.core.exceptions import ValidationError @@ -21,6 +22,7 @@ from django.urls import reverse import PIL import common.validators +from common.notifications import trigger_notification from common.settings import get_global_setting, set_global_setting from InvenTree.helpers import str2bool from InvenTree.unit_test import ( @@ -1073,6 +1075,7 @@ class NotificationTest(InvenTreeAPITestCase): """Tests for NotificationEntry.""" fixtures = ['users'] + roles = ['admin.view'] def test_check_notification_entries(self): """Test that notification entries can be created.""" @@ -1162,6 +1165,72 @@ class NotificationTest(InvenTreeAPITestCase): self.assertEqual(NotificationMessage.objects.count(), 13) self.assertEqual(NotificationMessage.objects.filter(user=self.user).count(), 3) + def test_simple(self): + """Test that a simple notification can be created.""" + trigger_notification( + Group.objects.get(name='Sales'), + user=self.user, + data={'message': 'This is a test notification'}, + ) + + def test_with_group(self): + """Test that a notification can be created with a group.""" + grp = Group.objects.get(name='Sales') + trigger_notification( + grp, + user=self.user, + data={'message': 'This is a test notification with group'}, + targets=[grp], + ) + + def test_wrong_target(self): + """Test that a notification with an invalid target raises an error.""" + with self.assertLogs() as cm: + trigger_notification( + Group.objects.get(name='Sales'), + user=self.user, + data={'message': 'This is a test notification'}, + targets=['invalid_target'], + ) + self.assertIn('Unknown target passed to t', str(cm[1])) + + def test_wrong_obj(self): + """Test that a object without a reference is raising an issue.""" + + class SampleObj: + pass + + with self.assertRaises(KeyError) as cm: + trigger_notification( + SampleObj(), + user=self.user, + data={'message': 'This is a test notification'}, + ) + self.assertIn('Could not resolve an object reference for', str(cm.exception)) + + # Without reference, it should not raise an error + trigger_notification( + Group.objects.get(name='Sales'), + user=self.user, + data={'message': 'This is a test notification'}, + ) + + def test_recent(self): + """Test that a notification is not created if it was already sent recently.""" + grp = Group.objects.get(name='Sales') + trigger_notification( # + grp, category='core', context={'name': 'test'}, targets=[self.user] + ) + self.assertEqual(NotificationMessage.objects.count(), 1) + + # Should not create a new notification + with self.assertLogs(logger='inventree') as cm: + trigger_notification( + grp, category='core', context={'name': 'test'}, targets=[self.user] + ) + self.assertEqual(NotificationMessage.objects.count(), 1) + self.assertIn('as recently been sent for', str(cm[1])) + class CommonTest(InvenTreeAPITestCase): """Tests for the common config.""" diff --git a/src/backend/InvenTree/plugin/builtin/integration/core_notifications.py b/src/backend/InvenTree/plugin/builtin/integration/core_notifications.py index 76a9e98e79..f55fe46a79 100644 --- a/src/backend/InvenTree/plugin/builtin/integration/core_notifications.py +++ b/src/backend/InvenTree/plugin/builtin/integration/core_notifications.py @@ -32,6 +32,7 @@ class InvenTreeUINotifications(NotificationMixin, InvenTreePlugin): """Create a UI notification entry for specified users.""" from common.models import NotificationMessage + ctx = context if context else {} entries = [] if not users: @@ -45,8 +46,8 @@ class InvenTreeUINotifications(NotificationMixin, InvenTreePlugin): source_object=user, user=user, category=category, - name=context['name'], - message=context['message'], + name=ctx.get('name'), + message=ctx.get('message'), ) ) diff --git a/src/backend/InvenTree/plugin/test_api.py b/src/backend/InvenTree/plugin/test_api.py index 86bad07b50..ff91bc5ae4 100644 --- a/src/backend/InvenTree/plugin/test_api.py +++ b/src/backend/InvenTree/plugin/test_api.py @@ -594,3 +594,55 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): self.assertEqual(Y_MANDATORY_2, Y_MANDATORY + 1) self.assertEqual(N_MANDATORY_2, N_MANDATORY - 1) + + +class PluginFullAPITest(PluginMixin, InvenTreeAPITestCase): + """Tests the plugin API endpoints.""" + + superuser = True + + @override_settings(PLUGIN_TESTING_SETUP=True) + def test_full_process(self): + """Test the full plugin install/uninstall process via API.""" + install_slug = 'inventree-brother-plugin' + slug = 'brother' + + # Install a plugin + data = self.post( + reverse('api-plugin-install'), + {'confirm': True, 'packagename': install_slug}, + expected_code=201, + max_query_time=30, + max_query_count=370, + ).data + self.assertEqual(data['success'], 'Installed plugin successfully') + + # Activate the plugin + data = self.patch( + reverse('api-plugin-detail-activate', kwargs={'plugin': slug}), + data={'active': True}, + max_query_count=320, + ).data + self.assertEqual(data['active'], True) + + # Check if the plugin is installed + test_plg = PluginConfig.objects.get(key=slug) + self.assertIsNotNone(test_plg, 'Test plugin not found') + self.assertTrue(test_plg.is_active()) + + # De-activate and uninstall the plugin + data = self.patch( + reverse('api-plugin-detail-activate', kwargs={'plugin': slug}), + data={'active': False}, + max_query_count=380, + ).data + self.assertEqual(data['active'], False) + response = self.patch( + reverse('api-plugin-uninstall', kwargs={'plugin': slug}), + max_query_count=305, + ) + self.assertEqual(response.status_code, 200) + + # Successful uninstallation + with self.assertRaises(PluginConfig.DoesNotExist): + PluginConfig.objects.get(key=slug)