diff --git a/src/backend/InvenTree/InvenTree/api_version.py b/src/backend/InvenTree/InvenTree/api_version.py index 7feb9adf48..a137a1571a 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 = 503 +INVENTREE_API_VERSION = 504 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v504 -> 2026-06-13 : https://github.com/inventree/InvenTree/pull/12139 + - Adjustments to the SelectionList and SelectionListEntry API endpoints to support more efficient queries and data retrieval + v503 -> 2026-06-11 : https://github.com/inventree/InvenTree/pull/12155 - Adds additional filtering and sorting options to the LabelTemplate API endpoint - Adds additional filtering and sorting options to the ReportTemplate API endpoint diff --git a/src/backend/InvenTree/common/api.py b/src/backend/InvenTree/common/api.py index 9785623e61..964d9e7a77 100644 --- a/src/backend/InvenTree/common/api.py +++ b/src/backend/InvenTree/common/api.py @@ -1207,29 +1207,25 @@ class IconList(ListAPI): return list(get_icon_packs().values()) -class SelectionListList(ListCreateAPI): +class SelectionListMixin(OutputOptionsMixin): + """Mixin for SelectionList views.""" + + queryset = common.models.SelectionList.objects.all() + serializer_class = common.serializers.SelectionListSerializer + permission_classes = [IsAuthenticatedOrReadScope] + + def get_queryset(self): + """Override the queryset method to include entry count.""" + return self.serializer_class.annotate_queryset(super().get_queryset()) + + +class SelectionListList(SelectionListMixin, ListCreateAPI): """List view for SelectionList objects.""" - queryset = common.models.SelectionList.objects.all() - serializer_class = common.serializers.SelectionListSerializer - permission_classes = [IsAuthenticatedOrReadScope] - def get_queryset(self): - """Override the queryset method to include entry count.""" - return self.serializer_class.annotate_queryset(super().get_queryset()) - - -class SelectionListDetail(RetrieveUpdateDestroyAPI): +class SelectionListDetail(SelectionListMixin, RetrieveUpdateDestroyAPI): """Detail view for a SelectionList object.""" - queryset = common.models.SelectionList.objects.all() - serializer_class = common.serializers.SelectionListSerializer - permission_classes = [IsAuthenticatedOrReadScope] - - def get_queryset(self): - """Override the queryset method to include entry count.""" - return self.serializer_class.annotate_queryset(super().get_queryset()) - class EntryMixin: """Mixin for SelectionEntry views.""" @@ -1246,6 +1242,12 @@ class EntryMixin: queryset = queryset.prefetch_related('list') return queryset + def perform_destroy(self, instance): + """Prevent deletion of entries belonging to a locked selection list.""" + if instance.list.locked: + raise PermissionDenied(_('Selection list is locked')) + super().perform_destroy(instance) + class SelectionEntryList(EntryMixin, ListCreateAPI): """List view for SelectionEntry objects.""" diff --git a/src/backend/InvenTree/common/serializers.py b/src/backend/InvenTree/common/serializers.py index 522ef62e36..3c0ef9e859 100644 --- a/src/backend/InvenTree/common/serializers.py +++ b/src/backend/InvenTree/common/serializers.py @@ -1004,15 +1004,17 @@ class SelectionEntrySerializer(InvenTreeModelSerializer): def validate(self, attrs): """Ensure that the selection list is not locked.""" ret = super().validate(attrs) - if self.instance and self.instance.list.locked: + list_obj = attrs.get('list') or (self.instance and self.instance.list) + if list_obj and list_obj.locked: raise serializers.ValidationError({'list': _('Selection list is locked')}) return ret -class SelectionListSerializer(InvenTreeModelSerializer): +class SelectionListSerializer(FilterableSerializerMixin, InvenTreeModelSerializer): """Serializer for a selection list.""" _choices_validated: dict = {} + _choices_provided: bool = False class Meta: """Meta options for SelectionListSerializer.""" @@ -1029,81 +1031,25 @@ class SelectionListSerializer(InvenTreeModelSerializer): 'default', 'created', 'last_updated', - 'choices', 'entry_count', + 'choices', ] default = SelectionEntrySerializer(read_only=True, allow_null=True, many=False) - choices = SelectionEntrySerializer(source='entries', many=True, required=False) entry_count = serializers.IntegerField(read_only=True) + choices = OptionalField( + serializer_class=SelectionEntrySerializer, + serializer_kwargs={'source': 'entries', 'many': True, 'read_only': True}, + prefetch_fields=['entries'], + default_include=True, + ) + @staticmethod def annotate_queryset(queryset): """Add count of entries for each selection list.""" return queryset.annotate(entry_count=Count('entries')) - def is_valid(self, *, raise_exception=False): - """Validate the selection list. Choices are validated separately.""" - choices = ( - self.initial_data.pop('choices') - if self.initial_data.get('choices') is not None - else [] - ) - - # Validate the choices - _choices_validated = [] - db_entries = ( - {a.id: a for a in self.instance.entries.all()} if self.instance else {} - ) - - for choice in choices: - current_inst = db_entries.get(choice.get('id')) - serializer = SelectionEntrySerializer( - instance=current_inst, - data={'list': current_inst.list.pk if current_inst else None, **choice}, - ) - serializer.is_valid(raise_exception=raise_exception) - _choices_validated.append({ - **serializer.validated_data, - 'id': choice.get('id'), - }) - self._choices_validated = _choices_validated - - return super().is_valid(raise_exception=raise_exception) - - def create(self, validated_data): - """Create a new selection list. Save the choices separately.""" - list_entry = common_models.SelectionList.objects.create(**validated_data) - for choice_data in self._choices_validated: - common_models.SelectionListEntry.objects.create(**{ - **choice_data, - 'list': list_entry, - }) - return list_entry - - def update(self, instance, validated_data): - """Update an existing selection list. Save the choices separately.""" - inst_mapping = {inst.id: inst for inst in instance.entries.all()} - existing_ids = {a.get('id') for a in self._choices_validated} - - # Perform creations and updates. - ret = [] - for data in self._choices_validated: - list_inst = data.get('list', None) - inst = inst_mapping.get(data.get('id')) - if inst is None: - if list_inst is None: - data['list'] = instance - ret.append(SelectionEntrySerializer().create(data)) - else: - ret.append(SelectionEntrySerializer().update(inst, data)) - - # Perform deletions. - for entry_id in inst_mapping.keys() - existing_ids: - inst_mapping[entry_id].delete() - - return super().update(instance, validated_data) - def validate(self, attrs): """Ensure that the selection list is not locked.""" ret = super().validate(attrs) diff --git a/src/backend/InvenTree/common/test_api.py b/src/backend/InvenTree/common/test_api.py index 6cfcdad4db..cf2debd8b3 100644 --- a/src/backend/InvenTree/common/test_api.py +++ b/src/backend/InvenTree/common/test_api.py @@ -11,6 +11,7 @@ from PIL import Image from taggit.models import Tag import common.models +from common.models import SelectionList, SelectionListEntry from InvenTree.unit_test import InvenTreeAPITestCase @@ -1167,3 +1168,69 @@ class TagAPITests(InvenTreeAPITestCase): self.assertIn(self.part_a.pk, pks) self.assertNotIn(self.part_b.pk, pks) + + +class SelectionListLockedTest(InvenTreeAPITestCase): + """Tests that a locked SelectionList rejects all entry mutations.""" + + def setUp(self): + """Create a locked SelectionList with one entry.""" + super().setUp() + + self.sel_list = SelectionList.objects.create(name='Locked List', locked=True) + self.entry = SelectionListEntry.objects.create( + list=self.sel_list, value='v1', label='Entry 1' + ) + + self.list_url = reverse( + 'api-selectionlist-detail', kwargs={'pk': self.sel_list.pk} + ) + self.entry_list_url = reverse( + 'api-selectionlistentry-list', kwargs={'pk': self.sel_list.pk} + ) + self.entry_detail_url = reverse( + 'api-selectionlistentry-detail', + kwargs={'pk': self.sel_list.pk, 'entrypk': self.entry.pk}, + ) + + def test_create_entry_locked(self): + """POST a new entry to a locked list should be rejected.""" + response = self.post( + self.entry_list_url, + {'list': self.sel_list.pk, 'value': 'v2', 'label': 'Entry 2'}, + expected_code=400, + ) + self.assertIn('list', response.data) + self.assertIn('locked', str(response.data['list']).lower()) + + def test_update_entry_locked(self): + """PATCH an entry on a locked list should be rejected.""" + response = self.patch( + self.entry_detail_url, {'label': 'Changed'}, expected_code=400 + ) + self.assertIn('list', response.data) + self.assertIn('locked', str(response.data['list']).lower()) + + def test_delete_entry_locked(self): + """DELETE an entry from a locked list should be rejected.""" + self.delete(self.entry_detail_url, expected_code=403) + self.assertTrue(SelectionListEntry.objects.filter(pk=self.entry.pk).exists()) + + def test_patch_list_with_choices_locked(self): + """PATCH the list with a choices payload should be rejected when locked.""" + response = self.patch( + self.list_url, + {'choices': [{'value': 'v2', 'label': 'New'}]}, + expected_code=400, + ) + self.assertIn('locked', response.data) + + def test_patch_list_without_choices_preserves_entries(self): + """PATCH the list without choices should not touch entries (even when unlocked).""" + self.sel_list.locked = False + self.sel_list.save() + + self.patch(self.list_url, {'name': 'Renamed List'}, expected_code=200) + + # Entry must still exist — omitting choices must not delete entries + self.assertTrue(SelectionListEntry.objects.filter(pk=self.entry.pk).exists()) diff --git a/src/backend/InvenTree/common/tests.py b/src/backend/InvenTree/common/tests.py index af0420e62d..2bd0c41db8 100644 --- a/src/backend/InvenTree/common/tests.py +++ b/src/backend/InvenTree/common/tests.py @@ -2103,43 +2103,58 @@ class SelectionListTest(InvenTreeAPITestCase): # Test adding a new list via the API response = self.post( reverse('api-selectionlist-list'), - { - 'name': 'New List', - 'active': True, - 'choices': [{'value': '1', 'label': 'Test Entry'}], - }, + {'name': 'New List', 'active': True}, expected_code=201, ) list_pk = response.data['pk'] self.assertEqual(response.data['name'], 'New List') self.assertTrue(response.data['active']) + + entry_list_url = reverse('api-selectionlistentry-list', kwargs={'pk': list_pk}) + + # Add an entry via the entry API + response = self.post( + entry_list_url, + {'list': list_pk, 'value': '1', 'label': 'Test Entry'}, + expected_code=201, + ) + entry_pk = response.data['id'] + self.assertEqual(response.data['value'], '1') + self.assertEqual(response.data['label'], 'Test Entry') + + # Verify the entry appears in the list's choices + response = self.get( + reverse('api-selectionlist-detail', kwargs={'pk': list_pk}), + data={'choices': True}, + expected_code=200, + ) self.assertEqual(len(response.data['choices']), 1) self.assertEqual(response.data['choices'][0]['value'], '1') - # Test editing the list choices via the API (remove and add in same call) - response = self.patch( - reverse('api-selectionlist-detail', kwargs={'pk': list_pk}), - {'choices': [{'value': '2', 'label': 'New Label'}]}, - expected_code=200, + # Edit the entry via the entry detail API + entry_url = reverse( + 'api-selectionlistentry-detail', kwargs={'pk': list_pk, 'entrypk': entry_pk} ) - self.assertEqual(response.data['name'], 'New List') - self.assertTrue(response.data['active']) - self.assertEqual(len(response.data['choices']), 1) - self.assertEqual(response.data['choices'][0]['value'], '2') - self.assertEqual(response.data['choices'][0]['label'], 'New Label') - entry_id = response.data['choices'][0]['id'] + response = self.patch(entry_url, {'label': 'Updated Label'}, expected_code=200) + self.assertEqual(response.data['value'], '1') + self.assertEqual(response.data['label'], 'Updated Label') - # Test changing an entry via list API - response = self.patch( + # Add a second entry, then delete the first via the entry detail API + self.post( + entry_list_url, + {'list': list_pk, 'value': '2', 'label': 'Second Entry'}, + expected_code=201, + ) + self.delete(entry_url, expected_code=204) + + # Verify only the second entry remains + response = self.get( reverse('api-selectionlist-detail', kwargs={'pk': list_pk}), - {'choices': [{'id': entry_id, 'value': '2', 'label': 'New Label Text'}]}, + data={'choices': True}, expected_code=200, ) - self.assertEqual(response.data['name'], 'New List') - self.assertTrue(response.data['active']) self.assertEqual(len(response.data['choices']), 1) self.assertEqual(response.data['choices'][0]['value'], '2') - self.assertEqual(response.data['choices'][0]['label'], 'New Label Text') def test_api_locked(self): """Test editing with locked/unlocked list.""" diff --git a/src/frontend/src/forms/CommonForms.tsx b/src/frontend/src/forms/CommonForms.tsx index c935b683de..6365d10cef 100644 --- a/src/frontend/src/forms/CommonForms.tsx +++ b/src/frontend/src/forms/CommonForms.tsx @@ -284,3 +284,22 @@ export function useParameterFields({ user ]); } + +export function selectionListFields(): ApiFormFieldSet { + return { + name: {}, + description: {}, + active: {}, + source_plugin: {}, + source_string: {} + }; +} + +export function selectionEntryFields(): ApiFormFieldSet { + return { + value: {}, + label: {}, + description: {}, + active: {} + }; +} diff --git a/src/frontend/src/forms/selectionListFields.tsx b/src/frontend/src/forms/selectionListFields.tsx deleted file mode 100644 index c3b864d572..0000000000 --- a/src/frontend/src/forms/selectionListFields.tsx +++ /dev/null @@ -1,118 +0,0 @@ -import type { ApiFormFieldSet, ApiFormFieldType } from '@lib/types/Forms'; -import { t } from '@lingui/core/macro'; -import { Table } from '@mantine/core'; -import { useMemo } from 'react'; -import RemoveRowButton from '../components/buttons/RemoveRowButton'; -import { StandaloneField } from '../components/forms/StandaloneField'; -import type { TableFieldRowProps } from '../components/forms/fields/TableField'; - -function BuildAllocateLineRow({ - props -}: Readonly<{ - props: TableFieldRowProps; -}>) { - const valueField: ApiFormFieldType = useMemo(() => { - return { - field_type: 'string', - name: 'value', - required: true, - value: props.item.value, - onValueChange: (value: any) => { - props.changeFn(props.idx, 'value', value); - } - }; - }, [props]); - - const labelField: ApiFormFieldType = useMemo(() => { - return { - field_type: 'string', - name: 'label', - required: true, - value: props.item.label, - onValueChange: (value: any) => { - props.changeFn(props.idx, 'label', value); - } - }; - }, [props]); - - const descriptionField: ApiFormFieldType = useMemo(() => { - return { - field_type: 'string', - name: 'description', - required: true, - value: props.item.description, - onValueChange: (value: any) => { - props.changeFn(props.idx, 'description', value); - } - }; - }, [props]); - - const activeField: ApiFormFieldType = useMemo(() => { - return { - field_type: 'boolean', - name: 'active', - required: true, - value: props.item.active, - onValueChange: (value: any) => { - props.changeFn(props.idx, 'active', value); - } - }; - }, [props]); - - return ( -