From 82a6ff777261fe3c0a19eab807996a3edbbd37df Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 23 Jun 2021 20:58:05 +1000 Subject: [PATCH] Adds unit testing for fancy new metadata class --- InvenTree/InvenTree/api_tester.py | 16 +++++ InvenTree/InvenTree/metadata.py | 38 ++++++----- InvenTree/InvenTree/test_api.py | 65 +++++++++++++++++++ .../migrations/0029_auto_20210601_1525.py | 9 ++- .../migrations/0026_auto_20201110_1011.py | 10 ++- InvenTree/users/models.py | 2 +- 6 files changed, 119 insertions(+), 21 deletions(-) diff --git a/InvenTree/InvenTree/api_tester.py b/InvenTree/InvenTree/api_tester.py index a803e6797f..2ba4f0136c 100644 --- a/InvenTree/InvenTree/api_tester.py +++ b/InvenTree/InvenTree/api_tester.py @@ -73,6 +73,22 @@ class InvenTreeAPITestCase(APITestCase): ruleset.save() break + def getActions(self, url): + """ + Return a dict of the 'actions' available at a given endpoint. + Makes use of the HTTP 'OPTIONS' method to request this. + """ + + response = self.client.options(url) + self.assertEqual(response.status_code, 200) + + actions = response.data.get('actions', None) + + if not actions: + actions = {} + + return actions + def get(self, url, data={}, expected_code=200): """ Issue a GET request diff --git a/InvenTree/InvenTree/metadata.py b/InvenTree/InvenTree/metadata.py index b9d0732acf..b78eaa9d8c 100644 --- a/InvenTree/InvenTree/metadata.py +++ b/InvenTree/InvenTree/metadata.py @@ -40,24 +40,32 @@ class InvenTreeMetadata(SimpleMetadata): table = f"{app_label}_{tbl_label}" - actions = metadata['actions'] + actions = metadata.get('actions', None) - check = users.models.RuleSet.check_table_permission + if actions is not None: - # Map the request method to a permission type - rolemap = { - 'GET': 'view', - 'OPTIONS': 'view', - 'POST': 'add', - 'PUT': 'change', - 'PATCH': 'change', - 'DELETE': 'delete', - } + check = users.models.RuleSet.check_table_permission - # Remove any HTTP methods that the user does not have permission for - for method, permission in rolemap.items(): - if method in actions and not check(user, table, permission): - del actions[method] + # Map the request method to a permission type + rolemap = { + 'POST': 'add', + 'PUT': 'change', + 'PATCH': 'change', + 'DELETE': 'delete', + } + + # Remove any HTTP methods that the user does not have permission for + for method, permission in rolemap.items(): + if method in actions and not check(user, table, permission): + del actions[method] + + # Add a 'DELETE' action if we are allowed to delete + if check(user, table, 'delete'): + actions['DELETE'] = True + + # Add a 'VIEW' action if we are allowed to view + if check(user, table, 'view'): + actions['GET'] = True except AttributeError: # We will assume that if the serializer class does *not* have a Meta diff --git a/InvenTree/InvenTree/test_api.py b/InvenTree/InvenTree/test_api.py index 8435d756fb..a877300a27 100644 --- a/InvenTree/InvenTree/test_api.py +++ b/InvenTree/InvenTree/test_api.py @@ -157,3 +157,68 @@ class APITests(InvenTreeAPITestCase): # New role permissions should have been added now self.assertIn('delete', roles['part']) self.assertIn('change', roles['build']) + + def test_list_endpoint_actions(self): + """ + Tests for the OPTIONS method for API endpoints. + """ + + self.basicAuth() + + # Without any 'part' permissions, we should not see any available actions + url = reverse('api-part-list') + + actions = self.getActions(url) + + # No actions, as there are no permissions! + self.assertEqual(len(actions), 0) + + # Assign a new role + self.assignRole('part.view') + actions = self.getActions(url) + + # As we don't have "add" permission, there should be no available API actions + self.assertEqual(len(actions), 0) + + # But let's make things interesting... + # Why don't we treat ourselves to some "add" permissions + self.assignRole('part.add') + + actions = self.getActions(url) + + self.assertIn('POST', actions) + self.assertEqual(len(actions), 1) + + def test_detail_endpoint_actions(self): + """ + Tests for detail API endpoint actions + """ + + self.basicAuth() + + url = reverse('api-part-detail', kwargs={'pk': 1}) + + actions = self.getActions(url) + + # No actions, as we do not have any permissions! + self.assertEqual(len(actions), 0) + + # Add a 'add' permission + # Note: 'add' permission automatically implies 'change' also + self.assignRole('part.add') + + actions = self.getActions(url) + + # 'add' permission does not apply here! + self.assertEqual(len(actions), 1) + self.assertIn('PUT', actions.keys()) + + # Add some other permissions + self.assignRole('part.change') + self.assignRole('part.delete') + + actions = self.getActions(url) + + self.assertEqual(len(actions), 2) + self.assertIn('PUT', actions.keys()) + self.assertIn('DELETE', actions.keys()) diff --git a/InvenTree/build/migrations/0029_auto_20210601_1525.py b/InvenTree/build/migrations/0029_auto_20210601_1525.py index e8b2d58947..fa6bab6b26 100644 --- a/InvenTree/build/migrations/0029_auto_20210601_1525.py +++ b/InvenTree/build/migrations/0029_auto_20210601_1525.py @@ -1,8 +1,13 @@ # Generated by Django 3.2 on 2021-06-01 05:25 +import logging + from django.db import migrations +logger = logging.getLogger('inventree') + + def assign_bom_items(apps, schema_editor): """ Run through existing BuildItem objects, @@ -13,7 +18,7 @@ def assign_bom_items(apps, schema_editor): BomItem = apps.get_model('part', 'bomitem') Part = apps.get_model('part', 'part') - print("Assigning BomItems to existing BuildItem objects") + logger.info("Assigning BomItems to existing BuildItem objects") count_valid = 0 count_total = 0 @@ -41,7 +46,7 @@ def assign_bom_items(apps, schema_editor): pass if count_total > 0: - print(f"Assigned BomItem for {count_valid}/{count_total} entries") + logger.info(f"Assigned BomItem for {count_valid}/{count_total} entries") def unassign_bom_items(apps, schema_editor): diff --git a/InvenTree/company/migrations/0026_auto_20201110_1011.py b/InvenTree/company/migrations/0026_auto_20201110_1011.py index 20ec7d2f6f..29a5099c3a 100644 --- a/InvenTree/company/migrations/0026_auto_20201110_1011.py +++ b/InvenTree/company/migrations/0026_auto_20201110_1011.py @@ -1,5 +1,6 @@ # Generated by Django 3.0.7 on 2020-11-10 10:11 +import logging import sys from moneyed import CURRENCIES @@ -7,6 +8,9 @@ from django.db import migrations, connection from company.models import SupplierPriceBreak +logger = logging.getLogger('inventree') + + def migrate_currencies(apps, schema_editor): """ Migrate from the 'old' method of handling currencies, @@ -19,7 +23,7 @@ def migrate_currencies(apps, schema_editor): for the SupplierPriceBreak model, to a new django-money compatible currency. """ - print("Updating currency references for SupplierPriceBreak model...") + logger.info("Updating currency references for SupplierPriceBreak model...") # A list of available currency codes currency_codes = CURRENCIES.keys() @@ -39,7 +43,7 @@ def migrate_currencies(apps, schema_editor): suffix = suffix.strip().upper() if suffix not in currency_codes: - print("Missing suffix:", suffix) + logger.warning(f"Missing suffix: '{suffix}'") while suffix not in currency_codes: # Ask the user to input a valid currency @@ -72,7 +76,7 @@ def migrate_currencies(apps, schema_editor): count += 1 if count > 0: - print(f"Updated {count} SupplierPriceBreak rows") + logger.info(f"Updated {count} SupplierPriceBreak rows") def reverse_currencies(apps, schema_editor): """ diff --git a/InvenTree/users/models.py b/InvenTree/users/models.py index 2763ba0e10..23353948b1 100644 --- a/InvenTree/users/models.py +++ b/InvenTree/users/models.py @@ -208,7 +208,7 @@ class RuleSet(models.Model): return True # Print message instead of throwing an error - print("Failed permission check for", table, permission) + logger.info(f"User '{user.name}' failed permission check for {table}.{permission}") return False @staticmethod