From c074250ce6ed7ebcbdf21df45991f11b6b280ecf Mon Sep 17 00:00:00 2001 From: Oliver Date: Thu, 28 Nov 2024 07:06:58 +1100 Subject: [PATCH] Stock Transfer Improvements (#8570) * Allow transfer of items independent of status marker * Update test * Display errors in stock transsfer form * Add option to set status when transferring stock * Fix inStock check for stock actions * Allow adjustment of status when counting stock item * Allow status adjustment for other actions: - Remove stock - Add stock * Revert error behavior * Enhanced unit test * Unit test fix * Bump API version * Fix for playwright test - Added helper func * Extend playwright tests for stock actions --- .../InvenTree/InvenTree/api_version.py | 5 +- src/backend/InvenTree/stock/models.py | 109 +++++++++++++----- src/backend/InvenTree/stock/serializers.py | 40 ++++++- src/backend/InvenTree/stock/test_api.py | 6 +- src/backend/InvenTree/stock/tests.py | 25 +++- .../components/forms/fields/ApiFormField.tsx | 7 +- .../components/forms/fields/TableField.tsx | 3 + src/frontend/src/forms/StockForms.tsx | 95 +++++++++++++-- src/frontend/src/pages/stock/StockDetail.tsx | 10 +- src/frontend/tests/helpers.ts | 12 ++ .../tests/pages/pui_sales_order.spec.ts | 9 +- src/frontend/tests/pages/pui_stock.spec.ts | 12 ++ 12 files changed, 281 insertions(+), 52 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/api_version.py b/src/backend/InvenTree/InvenTree/api_version.py index f60096525e..c84d2cdb6c 100644 --- a/src/backend/InvenTree/InvenTree/api_version.py +++ b/src/backend/InvenTree/InvenTree/api_version.py @@ -1,13 +1,16 @@ """InvenTree API version information.""" # InvenTree API version -INVENTREE_API_VERSION = 288 +INVENTREE_API_VERSION = 289 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v289 - 2024-11-27 : https://github.com/inventree/InvenTree/pull/8570 + - Enable status change when transferring stock items + v288 - 2024-11-27 : https://github.com/inventree/InvenTree/pull/8574 - Adds "consumed" filter to StockItem API diff --git a/src/backend/InvenTree/stock/models.py b/src/backend/InvenTree/stock/models.py index 29aa84cc50..d1c5de1849 100644 --- a/src/backend/InvenTree/stock/models.py +++ b/src/backend/InvenTree/stock/models.py @@ -1489,12 +1489,15 @@ class StockItem( """ return self.children.count() - @property - def in_stock(self) -> bool: - """Returns True if this item is in stock. + def is_in_stock(self, check_status: bool = True): + """Return True if this StockItem is "in stock". - See also: StockItem.IN_STOCK_FILTER for the db optimized version of this check. + Args: + check_status: If True, check the status of the StockItem. Defaults to True. """ + if check_status and self.status not in StockStatusGroups.AVAILABLE_CODES: + return False + return all([ self.quantity > 0, # Quantity must be greater than zero self.sales_order is None, # Not assigned to a SalesOrder @@ -1502,9 +1505,16 @@ class StockItem( self.customer is None, # Not assigned to a customer self.consumed_by is None, # Not consumed by a build not self.is_building, # Not part of an active build - self.status in StockStatusGroups.AVAILABLE_CODES, # Status is "available" ]) + @property + def in_stock(self) -> bool: + """Returns True if this item is in stock. + + See also: StockItem.IN_STOCK_FILTER for the db optimized version of this check. + """ + return self.is_in_stock(check_status=True) + @property def can_adjust_location(self): """Returns True if the stock location can be "adjusted" for this part. @@ -2073,14 +2083,13 @@ class StockItem( 'STOCK_ALLOW_OUT_OF_STOCK_TRANSFER', backup_value=False, cache=False ) - if not allow_out_of_stock_transfer and not self.in_stock: + if not allow_out_of_stock_transfer and not self.is_in_stock(check_status=False): raise ValidationError(_('StockItem cannot be moved as it is not in stock')) if quantity <= 0: return False if location is None: - # TODO - Raise appropriate error (cannot move to blank location) return False # Test for a partial movement @@ -2161,11 +2170,16 @@ class StockItem( return True @transaction.atomic - def stocktake(self, count, user, notes=''): + def stocktake(self, count, user, **kwargs): """Perform item stocktake. - When the quantity of an item is counted, - record the date of stocktake + Arguments: + count: The new quantity of the item + user: The user performing the stocktake + + Keyword Arguments: + notes: Optional notes for the stocktake + status: Optionally adjust the stock status """ try: count = Decimal(count) @@ -2175,25 +2189,40 @@ class StockItem( if count < 0: return False - self.stocktake_date = InvenTree.helpers.current_date() - self.stocktake_user = user - if self.updateQuantity(count): + tracking_info = {'quantity': float(count)} + + self.stocktake_date = InvenTree.helpers.current_date() + self.stocktake_user = user + + # Optional fields which can be supplied in a 'stocktake' call + for field in StockItem.optional_transfer_fields(): + if field in kwargs: + setattr(self, field, kwargs[field]) + tracking_info[field] = kwargs[field] + + self.save(add_note=False) + self.add_tracking_entry( StockHistoryCode.STOCK_COUNT, user, - notes=notes, - deltas={'quantity': float(self.quantity)}, + notes=kwargs.get('notes', ''), + deltas=tracking_info, ) return True @transaction.atomic - def add_stock(self, quantity, user, notes=''): - """Add items to stock. + def add_stock(self, quantity, user, **kwargs): + """Add a specified quantity of stock to this item. - This function can be called by initiating a ProjectRun, - or by manually adding the items to the stock location + Arguments: + quantity: The quantity to add + user: The user performing the action + + Keyword Arguments: + notes: Optional notes for the stock addition + status: Optionally adjust the stock status """ # Cannot add items to a serialized part if self.serialized: @@ -2209,20 +2238,38 @@ class StockItem( return False if self.updateQuantity(self.quantity + quantity): + tracking_info = {'added': float(quantity), 'quantity': float(self.quantity)} + + # Optional fields which can be supplied in a 'stocktake' call + for field in StockItem.optional_transfer_fields(): + if field in kwargs: + setattr(self, field, kwargs[field]) + tracking_info[field] = kwargs[field] + + self.save(add_note=False) + self.add_tracking_entry( StockHistoryCode.STOCK_ADD, user, - notes=notes, - deltas={'added': float(quantity), 'quantity': float(self.quantity)}, + notes=kwargs.get('notes', ''), + deltas=tracking_info, ) return True @transaction.atomic - def take_stock( - self, quantity, user, notes='', code=StockHistoryCode.STOCK_REMOVE, **kwargs - ): - """Remove items from stock.""" + def take_stock(self, quantity, user, code=StockHistoryCode.STOCK_REMOVE, **kwargs): + """Remove the specified quantity from this StockItem. + + Arguments: + quantity: The quantity to remove + user: The user performing the action + + Keyword Arguments: + code: The stock history code to use + notes: Optional notes for the stock removal + status: Optionally adjust the stock status + """ # Cannot remove items from a serialized part if self.serialized: return False @@ -2244,7 +2291,17 @@ class StockItem( if stockitem := kwargs.get('stockitem'): deltas['stockitem'] = stockitem.pk - self.add_tracking_entry(code, user, notes=notes, deltas=deltas) + # Optional fields which can be supplied in a 'stocktake' call + for field in StockItem.optional_transfer_fields(): + if field in kwargs: + setattr(self, field, kwargs[field]) + deltas[field] = kwargs[field] + + self.save(add_note=False) + + self.add_tracking_entry( + code, user, notes=kwargs.get('notes', ''), deltas=deltas + ) return True diff --git a/src/backend/InvenTree/stock/serializers.py b/src/backend/InvenTree/stock/serializers.py index ccc14c8804..55164bca3e 100644 --- a/src/backend/InvenTree/stock/serializers.py +++ b/src/backend/InvenTree/stock/serializers.py @@ -1554,7 +1554,7 @@ class StockAdjustmentItemSerializer(serializers.Serializer): class Meta: """Metaclass options.""" - fields = ['item', 'quantity'] + fields = ['pk', 'quantity', 'batch', 'status', 'packaging'] pk = serializers.PrimaryKeyRelatedField( queryset=StockItem.objects.all(), @@ -1565,6 +1565,17 @@ class StockAdjustmentItemSerializer(serializers.Serializer): help_text=_('StockItem primary key value'), ) + def validate_pk(self, pk): + """Ensure the stock item is valid.""" + allow_out_of_stock_transfer = get_global_setting( + 'STOCK_ALLOW_OUT_OF_STOCK_TRANSFER', backup_value=False, cache=False + ) + + if not allow_out_of_stock_transfer and not pk.is_in_stock(check_status=False): + raise ValidationError(_('Stock item is not in stock')) + + return pk + quantity = serializers.DecimalField( max_digits=15, decimal_places=5, min_value=Decimal(0), required=True ) @@ -1640,7 +1651,14 @@ class StockCountSerializer(StockAdjustmentSerializer): stock_item = item['pk'] quantity = item['quantity'] - stock_item.stocktake(quantity, request.user, notes=notes) + # Optional fields + extra = {} + + for field_name in StockItem.optional_transfer_fields(): + if field_value := item.get(field_name, None): + extra[field_name] = field_value + + stock_item.stocktake(quantity, request.user, notes=notes, **extra) class StockAddSerializer(StockAdjustmentSerializer): @@ -1658,7 +1676,14 @@ class StockAddSerializer(StockAdjustmentSerializer): stock_item = item['pk'] quantity = item['quantity'] - stock_item.add_stock(quantity, request.user, notes=notes) + # Optional fields + extra = {} + + for field_name in StockItem.optional_transfer_fields(): + if field_value := item.get(field_name, None): + extra[field_name] = field_value + + stock_item.add_stock(quantity, request.user, notes=notes, **extra) class StockRemoveSerializer(StockAdjustmentSerializer): @@ -1676,7 +1701,14 @@ class StockRemoveSerializer(StockAdjustmentSerializer): stock_item = item['pk'] quantity = item['quantity'] - stock_item.take_stock(quantity, request.user, notes=notes) + # Optional fields + extra = {} + + for field_name in StockItem.optional_transfer_fields(): + if field_value := item.get(field_name, None): + extra[field_name] = field_value + + stock_item.take_stock(quantity, request.user, notes=notes, **extra) class StockTransferSerializer(StockAdjustmentSerializer): diff --git a/src/backend/InvenTree/stock/test_api.py b/src/backend/InvenTree/stock/test_api.py index 7535fcac59..9be1e753ae 100644 --- a/src/backend/InvenTree/stock/test_api.py +++ b/src/backend/InvenTree/stock/test_api.py @@ -1780,8 +1780,8 @@ class StocktakeTest(StockAPITestCase): """Test stock transfers.""" stock_item = StockItem.objects.get(pk=1234) - # Mark this stock item as "quarantined" (cannot be moved) - stock_item.status = StockStatus.QUARANTINED.value + # Mark the item as 'out of stock' by assigning a customer + stock_item.customer = company.models.Company.objects.first() stock_item.save() InvenTreeSetting.set_setting('STOCK_ALLOW_OUT_OF_STOCK_TRANSFER', False) @@ -1797,7 +1797,7 @@ class StocktakeTest(StockAPITestCase): # First attempt should *fail* - stock item is quarantined response = self.post(url, data, expected_code=400) - self.assertIn('cannot be moved as it is not in stock', str(response.data)) + self.assertIn('Stock item is not in stock', str(response.data)) # Now, allow transfer of "out of stock" items InvenTreeSetting.set_setting('STOCK_ALLOW_OUT_OF_STOCK_TRANSFER', True) diff --git a/src/backend/InvenTree/stock/tests.py b/src/backend/InvenTree/stock/tests.py index c83e6bf274..8b6dca317e 100644 --- a/src/backend/InvenTree/stock/tests.py +++ b/src/backend/InvenTree/stock/tests.py @@ -13,7 +13,7 @@ from company.models import Company from InvenTree.unit_test import AdminTestCase, InvenTreeTestCase from order.models import SalesOrder from part.models import Part, PartTestTemplate -from stock.status_codes import StockHistoryCode +from stock.status_codes import StockHistoryCode, StockStatus from .models import ( StockItem, @@ -444,11 +444,32 @@ class StockTest(StockTestBase): self.assertIn('Counted items', track.notes) n = it.tracking_info.count() - self.assertFalse(it.stocktake(-1, None, 'test negative stocktake')) + self.assertFalse( + it.stocktake( + -1, + None, + notes='test negative stocktake', + status=StockStatus.DAMAGED.value, + ) + ) # Ensure tracking info was not added self.assertEqual(it.tracking_info.count(), n) + it.refresh_from_db() + self.assertEqual(it.status, StockStatus.OK.value) + + # Next, perform a valid stocktake + self.assertTrue( + it.stocktake( + 100, None, notes='test stocktake', status=StockStatus.DAMAGED.value + ) + ) + + it.refresh_from_db() + self.assertEqual(it.quantity, 100) + self.assertEqual(it.status, StockStatus.DAMAGED.value) + def test_add_stock(self): """Test adding stock.""" it = StockItem.objects.get(pk=2) diff --git a/src/frontend/src/components/forms/fields/ApiFormField.tsx b/src/frontend/src/components/forms/fields/ApiFormField.tsx index 6cc1d9fe52..7ca2506dc0 100644 --- a/src/frontend/src/components/forms/fields/ApiFormField.tsx +++ b/src/frontend/src/components/forms/fields/ApiFormField.tsx @@ -23,6 +23,11 @@ export type ApiFormAdjustFilterType = { data: FieldValues; }; +export type ApiFormFieldChoice = { + value: any; + display_name: string; +}; + /** Definition of the ApiForm field component. * - The 'name' attribute *must* be provided * - All other attributes are optional, and may be provided by the API @@ -83,7 +88,7 @@ export type ApiFormFieldType = { child?: ApiFormFieldType; children?: { [key: string]: ApiFormFieldType }; required?: boolean; - choices?: any[]; + choices?: ApiFormFieldChoice[]; hidden?: boolean; disabled?: boolean; exclude?: boolean; diff --git a/src/frontend/src/components/forms/fields/TableField.tsx b/src/frontend/src/components/forms/fields/TableField.tsx index 6db54e8164..51dc35b37a 100644 --- a/src/frontend/src/components/forms/fields/TableField.tsx +++ b/src/frontend/src/components/forms/fields/TableField.tsx @@ -213,6 +213,7 @@ export function TableField({ */ export function TableFieldExtraRow({ visible, + fieldName, fieldDefinition, defaultValue, emptyValue, @@ -220,6 +221,7 @@ export function TableFieldExtraRow({ onValueChange }: { visible: boolean; + fieldName?: string; fieldDefinition: ApiFormFieldType; defaultValue?: any; error?: string; @@ -253,6 +255,7 @@ export function TableFieldExtraRow({ { + return ( + StatusFilterOptions(ModelType.stockitem)()?.map((choice) => { + return { + value: choice.value, + display_name: choice.label + }; + }) ?? [] + ); + }, []); + const [quantity, setQuantity] = useState( add ? 0 : (props.item?.quantity ?? 0) ); + const [status, setStatus] = useState(undefined); + const removeAndRefresh = () => { props.removeFn(props.idx); }; @@ -463,6 +480,17 @@ function StockOperationsRow({ } }); + const [statusOpen, statusHandlers] = useDisclosure(false, { + onOpen: () => { + setStatus(record?.status || undefined); + props.changeFn(props.idx, 'status', record?.status || undefined); + }, + onClose: () => { + setStatus(undefined); + props.changeFn(props.idx, 'status', undefined); + } + }); + const stockString: string = useMemo(() => { if (!record) { return '-'; @@ -481,14 +509,21 @@ function StockOperationsRow({ <> - - -
{record.part_detail?.name}
- + + + +
{record.part_detail?.name}
+ + {props.rowErrors?.pk?.message && ( + + {props.rowErrors.pk.message} + + )} + {record.location ? record.location_detail?.pathstring : '-'} @@ -531,6 +566,15 @@ function StockOperationsRow({ } /> )} + {changeStatus && ( + } + tooltip={t`Change Status`} + onClick={() => statusHandlers.toggle()} + variant={statusOpen ? 'filled' : 'transparent'} + /> + )} {transfer && ( + {changeStatus && ( + { + setStatus(value); + props.changeFn(props.idx, 'status', value || undefined); + }} + fieldName='status' + fieldDefinition={{ + field_type: 'choice', + label: t`Status`, + choices: statusOptions, + value: status + }} + defaultValue={status} + /> + )} {transfer && ( { props.changeFn(props.idx, 'packaging', value || undefined); }} + fieldName='packaging' fieldDefinition={{ field_type: 'string', label: t`Packaging` @@ -604,19 +666,19 @@ function stockTransferFields(items: any[]): ApiFormFieldSet { ); }, - headers: [t`Part`, t`Location`, t`In Stock`, t`Move`, t`Actions`] + headers: [t`Part`, t`Location`, t`Stock`, t`Move`, t`Actions`] }, location: { filters: { structural: false } - // TODO: icon }, notes: {} }; @@ -641,6 +703,7 @@ function stockRemoveFields(items: any[]): ApiFormFieldSet { + ); }, headers: [t`Part`, t`Location`, t`In Stock`, t`Add`, t`Actions`] @@ -696,6 +765,7 @@ function stockCountFields(items: any[]): ApiFormFieldSet { return ( @@ -763,6 +833,7 @@ function stockMergeFields(items: any[]): ApiFormFieldSet { props={row} key={row.item.item} merge + changeStatus record={records[row.item.item]} /> ); diff --git a/src/frontend/src/pages/stock/StockDetail.tsx b/src/frontend/src/pages/stock/StockDetail.tsx index db3eecab2c..0d9dea4bf8 100644 --- a/src/frontend/src/pages/stock/StockDetail.tsx +++ b/src/frontend/src/pages/stock/StockDetail.tsx @@ -653,7 +653,15 @@ export default function StockDetail() { }); const stockActions = useMemo(() => { - const inStock = stockitem.in_stock; + const inStock = + user.hasChangeRole(UserRoles.stock) && + stockitem.quantity > 0 && + !stockitem.sales_order && + !stockitem.belongs_to && + !stockitem.customer && + !stockitem.consumed_by && + !stockitem.is_building; + const serial = stockitem.serial; const serialized = serial != null && diff --git a/src/frontend/tests/helpers.ts b/src/frontend/tests/helpers.ts index e352881fe9..a44bb60dcc 100644 --- a/src/frontend/tests/helpers.ts +++ b/src/frontend/tests/helpers.ts @@ -37,6 +37,18 @@ export const clearTableFilters = async (page) => { await page.getByLabel('filter-drawer-close').click(); }; +export const setTableChoiceFilter = async (page, filter, value) => { + await openFilterDrawer(page); + + await page.getByRole('button', { name: 'Add Filter' }).click(); + await page.getByPlaceholder('Select filter').fill(filter); + await page.getByRole('option', { name: 'Status' }).click(); + await page.getByPlaceholder('Select filter value').click(); + await page.getByRole('option', { name: value }).click(); + + await closeFilterDrawer(page); +}; + /** * Return the parent 'row' element for a given 'cell' element * @param cell - The cell element diff --git a/src/frontend/tests/pages/pui_sales_order.spec.ts b/src/frontend/tests/pages/pui_sales_order.spec.ts index dbe71e1090..b161607d25 100644 --- a/src/frontend/tests/pages/pui_sales_order.spec.ts +++ b/src/frontend/tests/pages/pui_sales_order.spec.ts @@ -1,8 +1,9 @@ import { test } from '../baseFixtures.ts'; import { baseUrl } from '../defaults.ts'; +import { clearTableFilters, setTableChoiceFilter } from '../helpers.ts'; import { doQuickLogin } from '../login.ts'; -test('Sales Orders', async ({ page }) => { +test('Sales Orders - Basic Tests', async ({ page }) => { await doQuickLogin(page); await page.goto(`${baseUrl}/home`); @@ -11,7 +12,11 @@ test('Sales Orders', async ({ page }) => { // Check for expected text in the table await page.getByRole('tab', { name: 'Sales Orders' }).waitFor(); - await page.getByText('In Progress').first().waitFor(); + + await clearTableFilters(page); + + await setTableChoiceFilter(page, 'status', 'On Hold'); + await page.getByText('On Hold').first().waitFor(); // Navigate to a particular sales order diff --git a/src/frontend/tests/pages/pui_stock.spec.ts b/src/frontend/tests/pages/pui_stock.spec.ts index 5d23703732..33c5f551f9 100644 --- a/src/frontend/tests/pages/pui_stock.spec.ts +++ b/src/frontend/tests/pages/pui_stock.spec.ts @@ -182,10 +182,22 @@ test('Stock - Stock Actions', async ({ page }) => { await page.getByLabel('action-menu-stock-operations-count').waitFor(); await page.getByLabel('action-menu-stock-operations-add').waitFor(); await page.getByLabel('action-menu-stock-operations-remove').waitFor(); + await page.getByLabel('action-menu-stock-operations-transfer').click(); await page.getByLabel('text-field-notes').fill('test notes'); await page.getByRole('button', { name: 'Submit' }).click(); await page.getByText('This field is required.').first().waitFor(); + + // Set the status field + await page.getByLabel('action-button-change-status').click(); + await page.getByLabel('choice-field-status').click(); + await page.getByText('Attention needed').click(); + + // Set the packaging field + await page.getByLabel('action-button-adjust-packaging').click(); + await page.getByLabel('text-field-packaging').fill('test packaging'); + + // Close the dialog await page.getByRole('button', { name: 'Cancel' }).click(); // Find an item which has been sent to a customer