From fb42878c11d570e9668957c8f4b560e564360388 Mon Sep 17 00:00:00 2001 From: Oliver Date: Sat, 2 Dec 2023 18:52:50 +1100 Subject: [PATCH] Give the people what they want (#6021) * Add 'existing_image' field to part API serializer * Ensure that the specified directory exists * Fix serializer - Use CharField instead of FilePathField - Custom validation - Save part with existing image * Add unit test for new feature * Bump API version --- .gitignore | 1 + InvenTree/InvenTree/api_version.py | 5 ++- InvenTree/InvenTree/serializers.py | 2 +- InvenTree/part/helpers.py | 28 ++++++++++++ InvenTree/part/models.py | 3 +- InvenTree/part/serializers.py | 47 +++++++++++++++++++- InvenTree/part/test_api.py | 69 +++++++++++++++++++++++++++--- InvenTree/part/views.py | 5 ++- 8 files changed, 146 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index d54279ccd4ea..2f8554f5fb4f 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,7 @@ dummy_image.* _tmp.csv InvenTree/label.pdf InvenTree/label.png +InvenTree/part_image_123abc.png label.pdf label.png InvenTree/my_special* diff --git a/InvenTree/InvenTree/api_version.py b/InvenTree/InvenTree/api_version.py index 1f95b0a5d838..75fd2018b3e2 100644 --- a/InvenTree/InvenTree/api_version.py +++ b/InvenTree/InvenTree/api_version.py @@ -2,11 +2,14 @@ # InvenTree API version -INVENTREE_API_VERSION = 156 +INVENTREE_API_VERSION = 157 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v157 -> 2023-12-02 : https://github.com/inventree/InvenTree/pull/6021 + - Add write-only "existing_image" field to Part API serializer + v156 -> 2023-11-26 : https://github.com/inventree/InvenTree/pull/5982 - Add POST endpoint for report and label creation diff --git a/InvenTree/InvenTree/serializers.py b/InvenTree/InvenTree/serializers.py index 96956ec4273d..d0304bb31309 100644 --- a/InvenTree/InvenTree/serializers.py +++ b/InvenTree/InvenTree/serializers.py @@ -864,7 +864,7 @@ def skip_create_fields(self): required=False, allow_blank=False, write_only=True, - label=_("URL"), + label=_("Remote Image"), help_text=_("URL of remote image file"), ) diff --git a/InvenTree/part/helpers.py b/InvenTree/part/helpers.py index 08ee86d3b587..b11ac9169d64 100644 --- a/InvenTree/part/helpers.py +++ b/InvenTree/part/helpers.py @@ -1,6 +1,9 @@ """Various helper functions for the part app""" import logging +import os + +from django.conf import settings from jinja2 import Environment @@ -66,3 +69,28 @@ def render_part_full_name(part) -> str: # Fallback to the default format elements = [el for el in [part.IPN, part.name, part.revision] if el] return ' | '.join(elements) + + +# Subdirectory for storing part images +PART_IMAGE_DIR = "part_images" + + +def get_part_image_directory() -> str: + """Return the directory where part images are stored. + + Returns: + str: Directory where part images are stored + + TODO: Future work may be needed here to support other storage backends, such as S3 + """ + + part_image_directory = os.path.abspath(os.path.join( + settings.MEDIA_ROOT, + PART_IMAGE_DIR, + )) + + # Create the directory if it does not exist + if not os.path.exists(part_image_directory): + os.makedirs(part_image_directory) + + return part_image_directory diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py index 33fdd16ee186..9ff41673ce35 100644 --- a/InvenTree/part/models.py +++ b/InvenTree/part/models.py @@ -293,7 +293,8 @@ def rename_part_image(instance, filename): Returns: Cleaned filename in format part__img """ - base = 'part_images' + + base = part_helpers.PART_IMAGE_DIR fname = os.path.basename(filename) return os.path.join(base, fname) diff --git a/InvenTree/part/serializers.py b/InvenTree/part/serializers.py index 561cc6911163..db347f347a1c 100644 --- a/InvenTree/part/serializers.py +++ b/InvenTree/part/serializers.py @@ -3,6 +3,7 @@ import imghdr import io import logging +import os from decimal import Decimal from django.core.exceptions import ValidationError @@ -27,6 +28,7 @@ import InvenTree.serializers import InvenTree.status import part.filters +import part.helpers as part_helpers import part.stocktake import part.tasks import stock.models @@ -511,6 +513,8 @@ class Meta: 'description', 'full_name', 'image', + 'remote_image', + 'existing_image', 'IPN', 'is_template', 'keywords', @@ -522,7 +526,6 @@ class Meta: 'parameters', 'pk', 'purchaseable', - 'remote_image', 'revision', 'salable', 'starred', @@ -608,7 +611,8 @@ def skip_create_fields(self): 'duplicate', 'initial_stock', 'initial_supplier', - 'copy_category_parameters' + 'copy_category_parameters', + 'existing_image', ] return fields @@ -761,6 +765,33 @@ def get_starred(self, part): help_text=_('Copy parameter templates from selected part category'), ) + # Allow selection of an existing part image file + existing_image = serializers.CharField( + label=_('Existing Image'), + help_text=_('Filename of an existing part image'), + write_only=True, + required=False, + allow_blank=False, + ) + + def validate_existing_image(self, img): + """Validate the selected image file""" + if not img: + return img + + img = img.split(os.path.sep)[-1] + + # Ensure that the file actually exists + img_path = os.path.join( + part_helpers.get_part_image_directory(), + img + ) + + if not os.path.exists(img_path) or not os.path.isfile(img_path): + raise ValidationError(_('Image file does not exist')) + + return img + @transaction.atomic def create(self, validated_data): """Custom method for creating a new Part instance using this serializer""" @@ -869,6 +900,18 @@ def save(self): super().save() part = self.instance + data = self.validated_data + + existing_image = data.pop('existing_image', None) + + if existing_image: + img_path = os.path.join( + part_helpers.PART_IMAGE_DIR, + existing_image + ) + + part.image = img_path + part.save() # Check if an image was downloaded from a remote URL remote_img = getattr(self, 'remote_image_file', None) diff --git a/InvenTree/part/test_api.py b/InvenTree/part/test_api.py index 895ced7ee889..fe334f6b9a3d 100644 --- a/InvenTree/part/test_api.py +++ b/InvenTree/part/test_api.py @@ -1,5 +1,6 @@ """Unit tests for the various part API endpoints""" +import os from datetime import datetime from decimal import Decimal from enum import IntEnum @@ -1464,6 +1465,16 @@ def test_category_parameters(self): class PartDetailTests(PartAPITestBase): """Test that we can create / edit / delete Part objects via the API.""" + @classmethod + def setUpTestData(cls): + """Custom setup routine for this class""" + super().setUpTestData() + + # Create a custom APIClient for file uploads + # Ref: https://stackoverflow.com/questions/40453947/how-to-generate-a-file-upload-test-request-with-django-rest-frameworks-apireq + cls.upload_client = APIClient() + cls.upload_client.force_authenticate(user=cls.user) + def test_part_operations(self): """Test that Part instances can be adjusted via the API""" n = Part.objects.count() @@ -1643,17 +1654,12 @@ def test_image_upload(self): with self.assertRaises(ValueError): print(p.image.file) - # Create a custom APIClient for file uploads - # Ref: https://stackoverflow.com/questions/40453947/how-to-generate-a-file-upload-test-request-with-django-rest-frameworks-apireq - upload_client = APIClient() - upload_client.force_authenticate(user=self.user) - # Try to upload a non-image file with open('dummy_image.txt', 'w') as dummy_image: dummy_image.write('hello world') with open('dummy_image.txt', 'rb') as dummy_image: - response = upload_client.patch( + response = self.upload_client.patch( url, { 'image': dummy_image, @@ -1672,7 +1678,7 @@ def test_image_upload(self): img.save(fn) with open(fn, 'rb') as dummy_image: - response = upload_client.patch( + response = self.upload_client.patch( url, { 'image': dummy_image, @@ -1686,6 +1692,55 @@ def test_image_upload(self): p = Part.objects.get(pk=pk) self.assertIsNotNone(p.image) + def test_existing_image(self): + """Test that we can allocate an existing uploaded image to a new Part""" + + # First, upload an image for an existing part + p = Part.objects.first() + + fn = 'part_image_123abc.png' + + img = PIL.Image.new('RGB', (128, 128), color='blue') + img.save(fn) + + with open(fn, 'rb') as img_file: + response = self.upload_client.patch( + reverse('api-part-detail', kwargs={'pk': p.pk}), + { + 'image': img_file, + }, + ) + + self.assertEqual(response.status_code, 200) + image_name = response.data['image'] + self.assertTrue(image_name.startswith('/media/part_images/part_image')) + + # Attempt to create, but with an invalid image name + response = self.post( + reverse('api-part-list'), + { + 'name': 'New part', + 'description': 'New Part description', + 'category': 1, + 'existing_image': 'does_not_exist.png', + }, + expected_code=400 + ) + + # Now, create a new part and assign the same image + response = self.post( + reverse('api-part-list'), + { + 'name': 'New part', + 'description': 'New part description', + 'category': 1, + 'existing_image': image_name.split(os.path.sep)[-1] + }, + expected_code=201, + ) + + self.assertEqual(response.data['image'], image_name) + def test_details(self): """Test that the required details are available.""" p = Part.objects.get(pk=1) diff --git a/InvenTree/part/views.py b/InvenTree/part/views.py index f67bc35bc22f..b8518ea64f9c 100644 --- a/InvenTree/part/views.py +++ b/InvenTree/part/views.py @@ -17,6 +17,7 @@ from company.models import SupplierPart from InvenTree.helpers import str2bool, str2int from InvenTree.views import AjaxUpdateView, AjaxView, InvenTreeRoleMixin +from part.helpers import PART_IMAGE_DIR from plugin.views import InvenTreePluginViewMixin from stock.models import StockItem, StockLocation @@ -398,12 +399,12 @@ def post(self, request, *args, **kwargs): data = {} if img: - img_path = settings.MEDIA_ROOT.joinpath('part_images', img) + img_path = settings.MEDIA_ROOT.joinpath(PART_IMAGE_DIR, img) # Ensure that the image already exists if os.path.exists(img_path): - part.image = os.path.join('part_images', img) + part.image = os.path.join(PART_IMAGE_DIR, img) part.save() data['success'] = _('Updated part image')