From acc2786e44f824389c9a81a146e0f649d4b2d1b2 Mon Sep 17 00:00:00 2001 From: Oliver Date: Mon, 18 May 2026 20:02:46 +1000 Subject: [PATCH] [feature] Rename attachments (#11920) * Implementation * Update API and CHANGELOG * Annotate response type * Simplify attachment renaming - Use the existing API endpoint * Capture the actual saved path * Tweak attachment table fields * Use built-in validation * Update docs * Unit testing * Ignore some lines from coverage * Check if file exists before deleting --- CHANGELOG.md | 1 + docs/docs/concepts/attachments.md | 20 +++ .../InvenTree/InvenTree/api_version.py | 5 +- src/backend/InvenTree/InvenTree/models.py | 4 +- src/backend/InvenTree/common/api.py | 28 ++++- src/backend/InvenTree/common/models.py | 74 +++++++++++- src/backend/InvenTree/common/serializers.py | 2 +- src/backend/InvenTree/common/test_api.py | 114 ++++++++++++++++++ .../src/tables/general/AttachmentTable.tsx | 14 +++ 9 files changed, 257 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e12568aa52..1cf482cf7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- [#11920](https://github.com/inventree/InvenTree/pull/11920) adds support for renaming attachments after they have been uploaded. This includes both backend and frontend changes, allowing users to rename attachments via the API or through the UI. - [#11914](https://github.com/inventree/InvenTree/pull/11914) adds a "maximum_stock" field to the Part model, allowing users to specify a maximum preferred stock level for each part. This is used in conjunction with the existing "minimum_stock" field to allow users to define a preferred stock range for each part. The "high_stock" filter has also been added to the Part API endpoint, allowing users to filter parts which are above their maximum stock level. - [#11631](https://github.com/inventree/InvenTree/pull/11631) adds "raw_amount" field to the BomItem model, allowing BOM quantities to account for the units of measure of the underlying part. - [#11872](https://github.com/inventree/InvenTree/pull/11872) adds a global setting to allow or disallow the deletion of serialized stock items. diff --git a/docs/docs/concepts/attachments.md b/docs/docs/concepts/attachments.md index 7fcdde8238..55f42736f0 100644 --- a/docs/docs/concepts/attachments.md +++ b/docs/docs/concepts/attachments.md @@ -16,3 +16,23 @@ Parameters can be associated with various InvenTree models. Any model which supports attachments will have an "Attachments" tab on its detail page. This tab displays all attachments associated with that object: {{ image("concepts/attachments-tab.png", "Order Attachments Example") }} + +## Attachments Types + +The following types of attachments are supported: + +### File Attachments + +File attachments allow users to upload files directly to InvenTree. These files are stored on the server and can be downloaded or viewed by users with appropriate permissions. + +### Link Attachments + +Link attachments allow users to associate external URLs with an object. This can be useful for linking to external documentation, resources, or other relevant web content. + +## Adding Attachments + +To add an attachment to an object, navigate to the object's detail page and click on the "Attachments" tab. From there, you can click the "Add attachment" button to upload a file or the "Add external link" button to add a link. + +### Renaming Attachments + +Once a file attachment has been uploaded, it can be renamed by clicking the "Edit" action associated with the attachment. This allows you to change the filename without needing to re-upload the file. The system will handle renaming the file on the server and updating the database record accordingly. diff --git a/src/backend/InvenTree/InvenTree/api_version.py b/src/backend/InvenTree/InvenTree/api_version.py index a25ba5d0b9..efed87c3a5 100644 --- a/src/backend/InvenTree/InvenTree/api_version.py +++ b/src/backend/InvenTree/InvenTree/api_version.py @@ -1,11 +1,14 @@ """InvenTree API version information.""" # InvenTree API version -INVENTREE_API_VERSION = 487 +INVENTREE_API_VERSION = 488 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v488 -> 2026-05-17 : https://github.com/inventree/InvenTree/pull/11920 + - Allow renaming of attachments after upload via the API + v487 -> 2026-05-15 : https://github.com/inventree/InvenTree/pull/11948 - Make SelectionList default nullable - Add icon to TreePath schema diff --git a/src/backend/InvenTree/InvenTree/models.py b/src/backend/InvenTree/InvenTree/models.py index b876c7c8d2..b6f5692c0a 100644 --- a/src/backend/InvenTree/InvenTree/models.py +++ b/src/backend/InvenTree/InvenTree/models.py @@ -661,7 +661,9 @@ class InvenTreeAttachmentMixin(InvenTreePermissionCheckMixin): Before deleting the model instance, delete any associated attachments. """ - self.attachments.all().delete() + for attachment in list(self.attachments.all()): + attachment.delete() + super().delete(*args, **kwargs) @property diff --git a/src/backend/InvenTree/common/api.py b/src/backend/InvenTree/common/api.py index 124961efe9..05575f73a8 100644 --- a/src/backend/InvenTree/common/api.py +++ b/src/backend/InvenTree/common/api.py @@ -781,8 +781,34 @@ class AttachmentList(AttachmentMixin, BulkDeleteMixin, ListCreateAPI): class AttachmentDetail(AttachmentMixin, RetrieveUpdateDestroyAPI): """Detail API endpoint for Attachment objects.""" + def update(self, request, *args, **kwargs): + """Update an existing attachment object.""" + attachment = self.get_object() + + if not attachment.check_permission('change', request.user): + raise PermissionDenied( + _('User does not have permission to edit this attachment') + ) + + partial = kwargs.pop('partial', False) + data = self.clean_data(request.data) + + # Extract filename first + filename = data.pop('filename', None) + + # Run other validation / updates first, before attempting to rename the file + serializer = self.get_serializer(attachment, data=data, partial=partial) + serializer.is_valid(raise_exception=True) + self.perform_update(serializer) + + # User is attempting to rename the file + if filename and attachment.basename and filename != attachment.basename: + attachment.rename(filename) + + return Response(serializer.data) + def destroy(self, request, *args, **kwargs): - """Check user permissions before deleting an attachment.""" + """Delete an existing attachment object.""" attachment = self.get_object() if not attachment.check_permission('delete', request.user): diff --git a/src/backend/InvenTree/common/models.py b/src/backend/InvenTree/common/models.py index f66a15d14f..72a7866568 100644 --- a/src/backend/InvenTree/common/models.py +++ b/src/backend/InvenTree/common/models.py @@ -15,6 +15,7 @@ from datetime import timedelta, timezone from email.utils import make_msgid from enum import Enum from io import BytesIO +from pathlib import Path from secrets import compare_digest from typing import Any, Optional @@ -25,9 +26,10 @@ from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from django.contrib.humanize.templatetags.humanize import naturaltime from django.core.cache import cache -from django.core.exceptions import ValidationError +from django.core.exceptions import SuspiciousFileOperation, ValidationError from django.core.files.base import ContentFile from django.core.files.storage import default_storage +from django.core.files.utils import validate_file_name from django.core.mail import EmailMultiAlternatives, get_connection from django.core.mail.utils import DNS_NAME from django.core.validators import MinLengthValidator, MinValueValidator @@ -1948,6 +1950,22 @@ class Attachment(InvenTree.models.MetadataMixin, InvenTree.models.InvenTreeModel choice_fnc = common.validators.attachment_model_options + def delete(self, *args, **kwargs): + """Custom delete method for the Attachment model. + + - Ensure that the attached file is deleted from storage when the database entry is removed + """ + attachment = self.attachment + + super().delete(*args, **kwargs) + + if attachment and default_storage.exists(attachment.name): + try: + # Remove the attached file from storage + default_storage.delete(attachment.name) + except Exception: # pragma: no cover + pass + def save(self, *args, **kwargs): """Custom 'save' method for the Attachment model. @@ -1993,6 +2011,60 @@ class Attachment(InvenTree.models.MetadataMixin, InvenTree.models.InvenTreeModel return os.path.basename(self.attachment.name) return str(self.link) + def validate_rename(self, filename: str): + """Validate that the provided filename is valid, for renaming an attachment.""" + filename = filename.strip() + + if not self.attachment: + raise ValidationError(_('No file attached to rename')) + + if not filename: + raise ValidationError(_('Filename cannot be empty')) + + try: + validate_file_name(filename, allow_relative_path=False) + except SuspiciousFileOperation: + raise ValidationError(_('Invalid filename')) + + current_ext = os.path.splitext(self.attachment.name)[1] + new_ext = os.path.splitext(filename)[1] + + if current_ext.lower() != new_ext.lower(): + raise ValidationError(_('Cannot change file extension')) + + def rename(self, filename: str): + """Rename the attached file.""" + self.validate_rename(filename) + + old_path = Path(self.attachment.name) + new_path = old_path.parent / filename + + if old_path == new_path: # pragma: no cover + # No change in filename + return + + if not new_path.is_relative_to(old_path.parent): # pragma: no cover + raise ValidationError(_('Invalid filename')) + + new_path = new_path.as_posix() + + if default_storage.exists(new_path): + raise ValidationError(_('A file with this name already exists')) + + # Create a new file with the new name, and delete the old file + new_path = default_storage.save(new_path, self.attachment.file) + + # Ensure that the new file exists + if not default_storage.exists(new_path): # pragma: no cover + raise ValidationError(_('Failed to save renamed file')) + + # Update the database file path + self.attachment.name = new_path + self.save() + + # Remove the old path + default_storage.delete(old_path) + model_type = models.CharField( max_length=100, validators=[common.validators.validate_attachment_model_type], diff --git a/src/backend/InvenTree/common/serializers.py b/src/backend/InvenTree/common/serializers.py index 9a4175b9b4..025c28f6d0 100644 --- a/src/backend/InvenTree/common/serializers.py +++ b/src/backend/InvenTree/common/serializers.py @@ -1,4 +1,4 @@ -"""JSON serializers for common components.""" +"""API serializers for common components.""" from django.contrib.contenttypes.models import ContentType from django.db.models import Count, OuterRef, Subquery diff --git a/src/backend/InvenTree/common/test_api.py b/src/backend/InvenTree/common/test_api.py index 1feffb0b6f..febbf5622e 100644 --- a/src/backend/InvenTree/common/test_api.py +++ b/src/backend/InvenTree/common/test_api.py @@ -1,5 +1,9 @@ """API unit tests for InvenTree common functionality.""" +import io + +from django.core.files.base import ContentFile +from django.core.files.storage import default_storage from django.urls import reverse import common.models @@ -675,3 +679,113 @@ class ParameterAPITests(InvenTreeAPITestCase): self.assertFalse( common.models.Parameter.objects.filter(template=template.pk).exists() ) + + +class AttachmentAPITests(InvenTreeAPITestCase): + """Tests for the Attachment API.""" + + def test_attachments(self): + """Test API functionality for attachments.""" + from common.models import Attachment + from part.models import Part + + self.assignRole('part.add') + + part = Part.objects.create(name='Test Part', description='A part for testing') + + N = Attachment.objects.count() + + # Upload multiple attachments against the part instance + for ii in range(5): + file_object = io.StringIO('Hello world') + file_object.seek(0) + + fn = f'test_file_{ii}.txt' + + content_file = ContentFile(file_object.read(), name=fn) + + url = reverse('api-attachment-list') + + response = self.post( + url, + data={ + 'model_type': 'part', + 'model_id': part.pk, + 'attachment': content_file, + 'comment': f'This is test file {ii}', + }, + format='multipart', + expected_code=201, + ) + + data = response.data + + # Check that the file has actually been created + self.assertEqual(data['filename'], fn) + self.assertTrue( + default_storage.exists(data['attachment'].replace('/media/', '')) + ) + + # Check that we have the expected number of attachments + self.assertEqual(Attachment.objects.count(), N + 5) + self.assertEqual(part.attachments.count(), 5) + + # Let's rename one of the attachments + att = part.attachments.first() + self.assertEqual(att.basename, 'test_file_0.txt') + + url = reverse('api-attachment-detail', kwargs={'pk': att.pk}) + + # A few failed attempts + for new_name in [ + 'different_ext.docx', + 'test_file_1.txt', + '../../test_file.txt', + ]: + print('- ATTEMPTING:', new_name) + response = self.patch(url, data={'filename': new_name}, expected_code=400) + + att.refresh_from_db() + self.assertEqual(att.basename, 'test_file_0.txt') + + # Let's try seriously this time + new_name = 'a_new_file.txt' + response = self.patch(url, data={'filename': new_name}, expected_code=200) + + att.refresh_from_db() + self.assertEqual(att.basename, new_name) + + # Check that the file has been renamed on disk + self.assertTrue( + default_storage.exists(f'attachments/part/{part.pk}/{new_name}') + ) + self.assertFalse( + default_storage.exists(f'attachments/part/{part.pk}/test_file_0.txt') + ) + + # Next, let's delete the attachment manually - via the API + response = self.delete(url, expected_code=403) + self.assignRole('part.delete') + response = self.delete(url, expected_code=204) + + # Check that the file has been deleted from disk + self.assertFalse( + default_storage.exists(f'attachments/part/{part.pk}/{new_name}') + ) + + self.assertEqual(Attachment.objects.count(), N + 4) + self.assertEqual(part.attachments.count(), 4) + + # Fetch the remaining attachments + attachments = list(part.attachments.all()) + + # Now, delete the part instance + part.active = False + part.save() + part.delete() + + self.assertEqual(Attachment.objects.count(), N) + + for att in attachments: + # Ensure that the file associated with each attachment has been removed + self.assertFalse(default_storage.exists(att.attachment.path)) diff --git a/src/frontend/src/tables/general/AttachmentTable.tsx b/src/frontend/src/tables/general/AttachmentTable.tsx index 9608b79ace..c31993ae80 100644 --- a/src/frontend/src/tables/general/AttachmentTable.tsx +++ b/src/frontend/src/tables/general/AttachmentTable.tsx @@ -232,12 +232,16 @@ export function AttachmentTable({ hidden: true }, attachment: {}, + filename: {}, link: {}, comment: {} }; if (attachmentType != 'link') { delete fields['link']; + } else { + delete fields['attachment']; + delete fields['filename']; } // Remove the 'attachment' field if we are editing an existing attachment, or uploading a link @@ -245,6 +249,11 @@ export function AttachmentTable({ delete fields['attachment']; } + if (!selectedAttachment) { + // Cannot edit the filename during creation + delete fields['filename']; + } + return fields; }, [model_type, model_id, attachmentType, selectedAttachment]); @@ -329,6 +338,11 @@ export function AttachmentTable({ RowEditAction({ hidden: !user.hasChangePermission(model_type), onClick: () => { + if (record.attachment) { + setAttachmentType('attachment'); + } else { + setAttachmentType('link'); + } setSelectedAttachment(record.pk); editAttachment.open(); }