Skip to content

Commit

Permalink
Give the people what they want (#6021)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
SchrodingersGat authored Dec 2, 2023
1 parent a7728d3 commit fb42878
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 14 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down
5 changes: 4 additions & 1 deletion InvenTree/InvenTree/api_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion InvenTree/InvenTree/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
)

Expand Down
28 changes: 28 additions & 0 deletions InvenTree/part/helpers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
"""Various helper functions for the part app"""

import logging
import os

from django.conf import settings

from jinja2 import Environment

Expand Down Expand Up @@ -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
3 changes: 2 additions & 1 deletion InvenTree/part/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ def rename_part_image(instance, filename):
Returns:
Cleaned filename in format part_<n>_img
"""
base = 'part_images'

base = part_helpers.PART_IMAGE_DIR
fname = os.path.basename(filename)

return os.path.join(base, fname)
Expand Down
47 changes: 45 additions & 2 deletions InvenTree/part/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import imghdr
import io
import logging
import os
from decimal import Decimal

from django.core.exceptions import ValidationError
Expand All @@ -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
Expand Down Expand Up @@ -511,6 +513,8 @@ class Meta:
'description',
'full_name',
'image',
'remote_image',
'existing_image',
'IPN',
'is_template',
'keywords',
Expand All @@ -522,7 +526,6 @@ class Meta:
'parameters',
'pk',
'purchaseable',
'remote_image',
'revision',
'salable',
'starred',
Expand Down Expand Up @@ -608,7 +611,8 @@ def skip_create_fields(self):
'duplicate',
'initial_stock',
'initial_supplier',
'copy_category_parameters'
'copy_category_parameters',
'existing_image',
]

return fields
Expand Down Expand Up @@ -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"""
Expand Down Expand Up @@ -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)
Expand Down
69 changes: 62 additions & 7 deletions InvenTree/part/test_api.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions InvenTree/part/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit fb42878

Please sign in to comment.