diff --git a/client/packages/common/src/intl/locales/en/common.json b/client/packages/common/src/intl/locales/en/common.json index 623dd398bf..2b354cdd7a 100644 --- a/client/packages/common/src/intl/locales/en/common.json +++ b/client/packages/common/src/intl/locales/en/common.json @@ -155,7 +155,7 @@ "description.fc-sell-price": "Foreign currency sell price per pack", "description.forecast-quantity": "Forecast Quantity to Reach Target", "description.initial-stock-on-hand": "Stock on hand on the first day of the program period", - "description.available-stock": "The customer's initial stock on hand + incoming stock +/- inventory adjustments - outgoing stock", + "description.available-stock": "The customer's initial stock on hand + incoming stock +/- inventory adjustments - outgoing stock", "description.invoice-number": "Shipment number", "description.last-reading-datetime": "Date and time of the last reading", "description.max-min-temperature": "Maximum or minimum temperature reading", @@ -237,6 +237,8 @@ "error.failed-to-save-item-variant": "Failed to save item variant", "error.failed-to-save-service-charges": "Failed to save service charges", "error.failed-to-save-vaccination": "Failed to save vaccination!", + "error.failed-to-save-item-variant": "Failed to save item variant", + "error.duplicate-item-variant-name": "Item variant with the same name already exists for this item", "error.failed-to-save-vaccine-course": "Failed to save vaccine course", "error.fetch-notifications": "Unable to display cold chain notifications", "error.field-must-be-specified": "{{field}} must be specified", @@ -1642,4 +1644,4 @@ "warning.caps-lock": "Warning: Caps lock is on", "warning.field-not-parsed": "{{field}} not parsed", "warning.nothing-to-supply": "Nothing left to supply!" -} +} \ No newline at end of file diff --git a/client/packages/common/src/types/schema.ts b/client/packages/common/src/types/schema.ts index d3a46b3c3d..947e9d50fc 100644 --- a/client/packages/common/src/types/schema.ts +++ b/client/packages/common/src/types/schema.ts @@ -7490,7 +7490,7 @@ export enum UniqueValueKey { Serial = 'serial' } -export type UniqueValueViolation = InsertAssetCatalogueItemErrorInterface & InsertAssetErrorInterface & InsertAssetLogErrorInterface & InsertAssetLogReasonErrorInterface & InsertDemographicIndicatorErrorInterface & InsertDemographicProjectionErrorInterface & InsertLocationErrorInterface & UpdateAssetErrorInterface & UpdateDemographicIndicatorErrorInterface & UpdateDemographicProjectionErrorInterface & UpdateLocationErrorInterface & UpdateSensorErrorInterface & { +export type UniqueValueViolation = InsertAssetCatalogueItemErrorInterface & InsertAssetErrorInterface & InsertAssetLogErrorInterface & InsertAssetLogReasonErrorInterface & InsertDemographicIndicatorErrorInterface & InsertDemographicProjectionErrorInterface & InsertLocationErrorInterface & UpdateAssetErrorInterface & UpdateDemographicIndicatorErrorInterface & UpdateDemographicProjectionErrorInterface & UpdateLocationErrorInterface & UpdateSensorErrorInterface & UpsertItemVariantErrorInterface & { __typename: 'UniqueValueViolation'; description: Scalars['String']['output']; field: UniqueValueKey; diff --git a/client/packages/common/src/ui/layout/tables/components/Cells/NumberInputCell/NumberInputCell.tsx b/client/packages/common/src/ui/layout/tables/components/Cells/NumberInputCell/NumberInputCell.tsx index 566cd165b4..840e8e7799 100644 --- a/client/packages/common/src/ui/layout/tables/components/Cells/NumberInputCell/NumberInputCell.tsx +++ b/client/packages/common/src/ui/layout/tables/components/Cells/NumberInputCell/NumberInputCell.tsx @@ -27,11 +27,13 @@ export const NumberInputCell = ({ TextInputProps, width, endAdornment, + error, }: CellProps & NumericInputProps & { id?: string; TextInputProps?: StandardTextFieldProps; endAdornment?: string; + error?: boolean; }): React.ReactElement> => { const [buffer, setBuffer] = useBufferState(column.accessor({ rowData })); const updater = useDebounceCallback(column.setter, [column.setter], 250); @@ -64,6 +66,7 @@ export const NumberInputCell = ({ value={buffer as number | undefined} width={width} endAdornment={endAdornment} + error={error} /> ); }; diff --git a/client/packages/system/src/Item/DetailView/Tabs/ItemVariants/ItemPackagingVariantsTable.tsx b/client/packages/system/src/Item/DetailView/Tabs/ItemVariants/ItemPackagingVariantsTable.tsx index 6484b88f0b..fc2c6dd122 100644 --- a/client/packages/system/src/Item/DetailView/Tabs/ItemVariants/ItemPackagingVariantsTable.tsx +++ b/client/packages/system/src/Item/DetailView/Tabs/ItemVariants/ItemPackagingVariantsTable.tsx @@ -40,13 +40,13 @@ export const ItemPackagingVariantsTable = ({ }, { key: 'packSize', - Cell: update ? PackSizeInputCell : TooltipTextCell, + Cell: update ? NonZeroInputCell : TooltipTextCell, label: 'label.pack-size', setter: updatePackaging, }, { key: 'volumePerUnit', - Cell: update ? VolumeInputCell : TooltipTextCell, + Cell: update ? NonZeroInputCell : TooltipTextCell, label: 'label.volume-per-unit', setter: updatePackaging, }, @@ -65,11 +65,11 @@ export const ItemPackagingVariantsTable = ({ }; // Input cells can't be defined inline, otherwise they lose focus on re-render -const VolumeInputCell = (props: CellProps) => ( - -); - -// Input cells can't be defined inline, otherwise they lose focus on re-render -const PackSizeInputCell = (props: CellProps) => ( - +const NonZeroInputCell = (props: CellProps) => ( + ); diff --git a/client/packages/system/src/Item/DetailView/Tabs/ItemVariants/ItemVariantModal.tsx b/client/packages/system/src/Item/DetailView/Tabs/ItemVariants/ItemVariantModal.tsx index 29bbc2e929..661b1de22d 100644 --- a/client/packages/system/src/Item/DetailView/Tabs/ItemVariants/ItemVariantModal.tsx +++ b/client/packages/system/src/Item/DetailView/Tabs/ItemVariants/ItemVariantModal.tsx @@ -35,7 +35,7 @@ export const ItemVariantModal = ({ const t = useTranslation(); const { Modal } = useDialog({ isOpen: true, onClose, disableBackdrop: true }); const height = useKeyboardHeightAdjustment(500); - const { success } = useNotification(); + const { success, error } = useNotification(); const { draft, isComplete, updateDraft, updatePackagingVariant, save } = useItemVariant({ @@ -52,9 +52,26 @@ export const ItemVariantModal = ({ disabled={!isComplete} variant="ok" onClick={() => { - save(draft); - success(t('messages.item-variant-saved'))(); - onClose(); + save(draft) + .then(() => { + success(t('messages.item-variant-saved'))(); + onClose(); + }) + .catch(e => { + // We create the same error message as we get from the default handler, but prevent duplicates + // This avoids the same error message being displayed multiple times, and the only appears once bug... + // https://github.com/msupply-foundation/open-msupply/issues/3984 + if ( + e instanceof Error && + e.message.includes(t('error.duplicate-item-variant-name')) + ) { + error(t('error.duplicate-item-variant-name'), { + preventDuplicate: true, + })(); + return; + } + error(t('error.failed-to-save-item-variant'))(); + }); }} /> } diff --git a/client/packages/system/src/Item/api/hooks/useItemVariant/useItemVariant.ts b/client/packages/system/src/Item/api/hooks/useItemVariant/useItemVariant.ts index 0c2ec3b8be..13cb520ddd 100644 --- a/client/packages/system/src/Item/api/hooks/useItemVariant/useItemVariant.ts +++ b/client/packages/system/src/Item/api/hooks/useItemVariant/useItemVariant.ts @@ -101,6 +101,11 @@ const useUpsert = ({ itemId }: { itemId: string }) => { if (result.__typename === 'ItemVariantNode') { return result; } + if (result.__typename === 'UpsertItemVariantError') { + if (result.error.__typename === 'UniqueValueViolation') { + throw new Error(t('error.duplicate-item-variant-name')); + } + } } throw new Error(t('error.failed-to-save-item-variant')); }; @@ -114,5 +119,10 @@ const useUpsert = ({ itemId }: { itemId: string }) => { }; function getIsComplete(draft: ItemVariantFragment) { - return !!draft.name; + return ( + !!draft.name && + draft.packagingVariants.every( + pv => pv.packSize !== 0 && pv.volumePerUnit !== 0 + ) + ); } diff --git a/client/packages/system/src/Item/api/operations.generated.ts b/client/packages/system/src/Item/api/operations.generated.ts index 7b531b9ff5..97ea97306f 100644 --- a/client/packages/system/src/Item/api/operations.generated.ts +++ b/client/packages/system/src/Item/api/operations.generated.ts @@ -107,7 +107,7 @@ export type UpsertItemVariantMutationVariables = Types.Exact<{ }>; -export type UpsertItemVariantMutation = { __typename: 'Mutations', centralServer: { __typename: 'CentralServerMutationNode', itemVariant: { __typename: 'ItemVariantMutations', upsertItemVariant: { __typename: 'ItemVariantNode', id: string, name: string, dosesPerUnit?: number | null, manufacturerId?: string | null, coldStorageTypeId?: string | null, manufacturer?: { __typename: 'NameNode', code: string, id: string, isCustomer: boolean, isSupplier: boolean, isOnHold: boolean, name: string, store?: { __typename: 'StoreNode', id: string, code: string } | null } | null, coldStorageType?: { __typename: 'ColdStorageTypeNode', id: string, name: string, minTemperature: number, maxTemperature: number } | null, packagingVariants: Array<{ __typename: 'PackagingVariantNode', id: string, name: string, packagingLevel: number, packSize?: number | null, volumePerUnit?: number | null }> } | { __typename: 'UpsertItemVariantError' } } } }; +export type UpsertItemVariantMutation = { __typename: 'Mutations', centralServer: { __typename: 'CentralServerMutationNode', itemVariant: { __typename: 'ItemVariantMutations', upsertItemVariant: { __typename: 'ItemVariantNode', id: string, name: string, dosesPerUnit?: number | null, manufacturerId?: string | null, coldStorageTypeId?: string | null, manufacturer?: { __typename: 'NameNode', code: string, id: string, isCustomer: boolean, isSupplier: boolean, isOnHold: boolean, name: string, store?: { __typename: 'StoreNode', id: string, code: string } | null } | null, coldStorageType?: { __typename: 'ColdStorageTypeNode', id: string, name: string, minTemperature: number, maxTemperature: number } | null, packagingVariants: Array<{ __typename: 'PackagingVariantNode', id: string, name: string, packagingLevel: number, packSize?: number | null, volumePerUnit?: number | null }> } | { __typename: 'UpsertItemVariantError', error: { __typename: 'DatabaseError', description: string } | { __typename: 'InternalError', description: string } | { __typename: 'UniqueValueViolation', description: string, field: Types.UniqueValueKey } } } } }; export type DeleteItemVariantMutationVariables = Types.Exact<{ storeId: Types.Scalars['String']['input']; @@ -436,6 +436,17 @@ export const UpsertItemVariantDocument = gql` ... on ItemVariantNode { ...ItemVariant } + ... on UpsertItemVariantError { + __typename + error { + __typename + description + ... on UniqueValueViolation { + description + field + } + } + } } } } diff --git a/client/packages/system/src/Item/api/operations.graphql b/client/packages/system/src/Item/api/operations.graphql index eb2198d8c0..c61a69efa6 100644 --- a/client/packages/system/src/Item/api/operations.graphql +++ b/client/packages/system/src/Item/api/operations.graphql @@ -326,6 +326,17 @@ mutation upsertItemVariant($storeId: String!, $input: UpsertItemVariantInput!) { ... on ItemVariantNode { ...ItemVariant } + ... on UpsertItemVariantError { + __typename + error { + __typename + description + ... on UniqueValueViolation { + description + field + } + } + } } } } diff --git a/server/graphql/item_variant/src/mutations/upsert.rs b/server/graphql/item_variant/src/mutations/upsert.rs index d332b14538..a28bf0247d 100644 --- a/server/graphql/item_variant/src/mutations/upsert.rs +++ b/server/graphql/item_variant/src/mutations/upsert.rs @@ -1,6 +1,6 @@ use async_graphql::*; use graphql_core::{ - simple_generic_errors::{DatabaseError, InternalError}, + simple_generic_errors::{DatabaseError, InternalError, UniqueValueKey, UniqueValueViolation}, standard_graphql_error::{validate_auth, StandardGraphqlError}, ContextExt, }; @@ -49,6 +49,7 @@ pub enum UpsertItemVariantResponse { #[graphql(field(name = "description", ty = "String"))] pub enum UpsertItemVariantErrorInterface { InternalError(InternalError), + DuplicateName(UniqueValueViolation), DatabaseError(DatabaseError), } @@ -137,6 +138,13 @@ fn map_error(error: ServiceError) -> Result { let formatted_error = format!("{:#?}", error); let graphql_error = match error { + // Structured errors + ServiceError::DuplicateName => { + return Ok(UpsertItemVariantErrorInterface::DuplicateName( + UniqueValueViolation(UniqueValueKey::Name), + )) + } + // Generic errors ServiceError::CreatedRecordNotFound => InternalError(formatted_error), ServiceError::ItemDoesNotExist => InternalError(formatted_error), ServiceError::PackagingVariantError(upsert_packaging_variant_error) => { @@ -144,6 +152,9 @@ fn map_error(error: ServiceError) -> Result { UpsertPackagingVariantError::ItemVariantDoesNotExist => { BadUserInput(formatted_error) } + UpsertPackagingVariantError::CantChangeItemVariant => BadUserInput(formatted_error), + UpsertPackagingVariantError::LessThanZero(_field) => BadUserInput(formatted_error), + UpsertPackagingVariantError::DatabaseError(_repository_error) => { InternalError(formatted_error) } @@ -153,6 +164,9 @@ fn map_error(error: ServiceError) -> Result { } } ServiceError::DatabaseError(_repository_error) => InternalError(formatted_error), + ServiceError::CantChangeItem => BadUserInput(formatted_error), + + ServiceError::ColdStorageTypeDoesNotExist => BadUserInput(formatted_error), }; Err(graphql_error.extend()) diff --git a/server/service/src/item/item_variant/test/mod.rs b/server/service/src/item/item_variant/test/mod.rs index 07ef170a96..b922a3772e 100644 --- a/server/service/src/item/item_variant/test/mod.rs +++ b/server/service/src/item/item_variant/test/mod.rs @@ -6,7 +6,9 @@ mod query { use repository::{EqualFilter, StringFilter}; use util::uuid::uuid; - use crate::item::item_variant::{DeleteItemVariant, UpsertItemVariantWithPackaging}; + use crate::item::item_variant::{ + DeleteItemVariant, UpsertItemVariantError, UpsertItemVariantWithPackaging, + }; use crate::service_provider::ServiceProvider; #[actix_rt::test] @@ -139,16 +141,89 @@ mod query { #[actix_rt::test] async fn validate_item_variant() { - // TODO validation tests + let (_, _, connection_manager, _) = + setup_all("validate_item_variant", MockDataInserts::none().items()).await; - // Test that the item variant name is set? + let service_provider = ServiceProvider::new(connection_manager, "app_data"); + let context = service_provider.basic_context().unwrap(); + let service = service_provider.item_service; + + let test_item_a_variant_id = "test_item_variant_id"; // Test that we can't create a record with an item_id that doesn't exist + let result = service.upsert_item_variant( + &context, + UpsertItemVariantWithPackaging { + id: test_item_a_variant_id.to_string(), + item_id: uuid(), + name: "Variant 1".to_string(), + ..Default::default() + }, + ); + + assert_eq!( + result.unwrap_err(), + UpsertItemVariantError::ItemDoesNotExist + ); // Test that we can't change the item_id on an existing record??? - // Test that we can't create/update a record with an invalid cold_storage_id + // Create a new item variant for item_a + let _item_a_variant_a = service + .upsert_item_variant( + &context, + UpsertItemVariantWithPackaging { + id: test_item_a_variant_id.to_string(), + item_id: mock_item_a().id, + name: "item_a_variant_a".to_string(), + ..Default::default() + }, + ) + .unwrap(); + + // Try to change the item_id + let result = service.upsert_item_variant( + &context, + UpsertItemVariantWithPackaging { + id: test_item_a_variant_id.to_string(), + item_id: mock_item_b().id, + name: "Variant 1".to_string(), + ..Default::default() + }, + ); - // Test name should be unique for an item? + assert_eq!(result.unwrap_err(), UpsertItemVariantError::CantChangeItem); + + // Test that we can't create/update a record with an invalid cold_storage_id + let result = service.upsert_item_variant( + &context, + UpsertItemVariantWithPackaging { + id: test_item_a_variant_id.to_string(), + item_id: mock_item_a().id, + name: "Variant 1".to_string(), + cold_storage_type_id: Some(uuid()), + ..Default::default() + }, + ); + + assert_eq!( + result.unwrap_err(), + UpsertItemVariantError::ColdStorageTypeDoesNotExist + ); + + // Test: name should be unique for an item + + // Add another item variant for item_a with the same name + let result = service.upsert_item_variant( + &context, + UpsertItemVariantWithPackaging { + id: uuid(), + item_id: mock_item_a().id, + name: "item_a_variant_a".to_string(), + ..Default::default() + }, + ); + + assert_eq!(result.unwrap_err(), UpsertItemVariantError::DuplicateName); } } diff --git a/server/service/src/item/item_variant/upsert.rs b/server/service/src/item/item_variant/upsert.rs index ba7eabee01..4fa86c9db9 100644 --- a/server/service/src/item/item_variant/upsert.rs +++ b/server/service/src/item/item_variant/upsert.rs @@ -1,10 +1,12 @@ use repository::{ item_variant::{ + item_variant::{ItemVariantFilter, ItemVariantRepository}, item_variant_row::{ItemVariantRow, ItemVariantRowRepository}, packaging_variant::{PackagingVariantFilter, PackagingVariantRepository}, packaging_variant_row::PackagingVariantRowRepository, }, - EqualFilter, RepositoryError, StorageConnection, + ColdStorageTypeRowRepository, EqualFilter, ItemLinkRowRepository, RepositoryError, + StorageConnection, StringFilter, }; use crate::{ @@ -21,6 +23,9 @@ use crate::{ pub enum UpsertItemVariantError { CreatedRecordNotFound, ItemDoesNotExist, + CantChangeItem, + DuplicateName, + ColdStorageTypeDoesNotExist, PackagingVariantError(UpsertPackagingVariantError), DatabaseError(RepositoryError), } @@ -124,5 +129,45 @@ fn validate( return Err(UpsertItemVariantError::ItemDoesNotExist); } + let old_item_variant = ItemVariantRowRepository::new(connection).find_one_by_id(&input.id)?; + + if let Some(old_item_variant) = old_item_variant { + // Query Item Link to check if the item_id is the same + // If items have been merged, the item_id could be different, but we still want to update the row so we have the latest id + let old_item_id = ItemLinkRowRepository::new(connection) + .find_one_by_id(&old_item_variant.item_link_id)? + .map(|v| v.item_id) + .unwrap_or_else(|| old_item_variant.item_link_id.clone()); + + if old_item_id != input.item_id { + return Err(UpsertItemVariantError::CantChangeItem); + } + } + + if let Some(cold_storage_type_id) = &input.cold_storage_type_id { + // Check if the cold storage type exists + let repo = ColdStorageTypeRowRepository::new(connection); + let cold_storage_type = repo.find_one_by_id(cold_storage_type_id)?; + if cold_storage_type.is_none() { + return Err(UpsertItemVariantError::ColdStorageTypeDoesNotExist); + } + } + + // Check for duplicate name under the same item + let item_variants_with_duplicate_name = ItemVariantRepository::new(connection) + .query_by_filter( + ItemVariantFilter::new() + .name(StringFilter::equal_to(&input.name.trim())) + .item_id(EqualFilter::equal_to(&input.item_id)), + )?; + + if item_variants_with_duplicate_name + .iter() + .find(|v| v.id != input.id) + .is_some() + { + return Err(UpsertItemVariantError::DuplicateName); + } + Ok(()) } diff --git a/server/service/src/item/packaging_variant/test/mod.rs b/server/service/src/item/packaging_variant/test/mod.rs index 698c30ee9e..e23292bdcc 100644 --- a/server/service/src/item/packaging_variant/test/mod.rs +++ b/server/service/src/item/packaging_variant/test/mod.rs @@ -4,9 +4,12 @@ mod query { use repository::mock::{mock_item_a, MockDataInserts}; use repository::test_db::setup_all; use repository::EqualFilter; + use util::uuid::uuid; use crate::item::item_variant::UpsertItemVariantWithPackaging; - use crate::item::packaging_variant::{DeletePackagingVariant, UpsertPackagingVariant}; + use crate::item::packaging_variant::{ + DeletePackagingVariant, UpsertPackagingVariant, UpsertPackagingVariantError, + }; use crate::service_provider::ServiceProvider; #[actix_rt::test] @@ -45,6 +48,7 @@ mod query { id: test_packaging_variant_id.to_string(), item_variant_id: item_variant.id, name: "packaging_variant_a".to_string(), + packaging_level: 1, ..Default::default() }, ) @@ -78,6 +82,7 @@ mod query { id: test_packaging_variant_id.to_string(), item_variant_id: test_item_variant_id.to_string(), name: "updated_name".to_string(), + packaging_level: 1, ..Default::default() }, ) @@ -131,19 +136,194 @@ mod query { #[actix_rt::test] async fn validate_packaging_variant() { - // TODO validation tests + let (_, _, connection_manager, _) = setup_all( + "validate_packaging_variant", + MockDataInserts::none().items(), + ) + .await; + + let service_provider = ServiceProvider::new(connection_manager, "app_data"); + let context = service_provider.basic_context().unwrap(); + let service = service_provider.item_service; + + let test_item_variant_id = "test_item_variant_id"; + let test_item_variant2_id = "test_item_variant2_id"; + let test_packaging_variant_id = "test_packaging_variant_id"; + + // Create a 2 new item variants + let _item_variant = service + .upsert_item_variant( + &context, + UpsertItemVariantWithPackaging { + id: test_item_variant_id.to_string(), + item_id: mock_item_a().id, + name: "item_variant_a".to_string(), + ..Default::default() + }, + ) + .unwrap(); - // Test that the name is set? + let _item_variant2 = service + .upsert_item_variant( + &context, + UpsertItemVariantWithPackaging { + id: test_item_variant2_id.to_string(), + item_id: mock_item_a().id, + name: "item_variant_b".to_string(), + ..Default::default() + }, + ) + .unwrap(); + + // Create a new packaging variant + service + .upsert_packaging_variant( + &context, + UpsertPackagingVariant { + id: test_packaging_variant_id.to_string(), + item_variant_id: test_item_variant_id.to_string(), + name: "packaging_variant_a".to_string(), + packaging_level: 1, + ..Default::default() + }, + ) + .unwrap(); // Test that we can't create a record with an item_variant_id that doesn't exist + let result = service.upsert_packaging_variant( + &context, + UpsertPackagingVariant { + id: uuid(), + item_variant_id: "some_id_that_doesn't_exist".to_string(), + name: "packaging_variant_a".to_string(), + packaging_level: 1, + ..Default::default() + }, + ); + + assert_eq!( + result.unwrap_err(), + UpsertPackagingVariantError::ItemVariantDoesNotExist + ); // Test that we can't change the item_variant_id on an existing record??? + let result = service.upsert_packaging_variant( + &context, + UpsertPackagingVariant { + id: test_packaging_variant_id.to_string(), + item_variant_id: test_item_variant2_id.to_string(), + name: "packaging_variant_a".to_string(), + packaging_level: 1, + ..Default::default() + }, + ); + + assert_eq!( + result.unwrap_err(), + UpsertPackagingVariantError::CantChangeItemVariant + ); + + // Test that we can't create a record with a packaging_level < 0 + let result = service.upsert_packaging_variant( + &context, + UpsertPackagingVariant { + id: test_packaging_variant_id.to_string(), + item_variant_id: test_item_variant_id.to_string(), + name: "packaging_variant_a".to_string(), + packaging_level: -1, + ..Default::default() + }, + ); + assert_eq!( + result.unwrap_err(), + UpsertPackagingVariantError::LessThanZero("packaging_level".to_string()) + ); + + // Test that we can't create a record with a pack_size < 0 + let result = service.upsert_packaging_variant( + &context, + UpsertPackagingVariant { + id: test_packaging_variant_id.to_string(), + item_variant_id: test_item_variant_id.to_string(), + name: "packaging_variant_a".to_string(), + packaging_level:1, + pack_size: Some(-1.0), + ..Default::default() + }, + ); + assert_eq!( + result.unwrap_err(), + UpsertPackagingVariantError::LessThanZero("pack_size".to_string()) + ); + + + // Test that we can't create a record with a pack_size == 0 + let result = service.upsert_packaging_variant( + &context, + UpsertPackagingVariant { + id: test_packaging_variant_id.to_string(), + item_variant_id: test_item_variant_id.to_string(), + name: "packaging_variant_a".to_string(), + packaging_level:1, + pack_size: Some(0.0), + ..Default::default() + }, + ); + assert_eq!( + result.unwrap_err(), + UpsertPackagingVariantError::LessThanZero("pack_size".to_string()) + ); - // Test that the following fields are all > 0 if supplied... - /* - packaging_level: i32, - pack_size: Option, - volume_per_unit: Option, - */ + // Test that we can't create a record with a pack_size == 0 + let result = service.upsert_packaging_variant( + &context, + UpsertPackagingVariant { + id: test_packaging_variant_id.to_string(), + item_variant_id: test_item_variant_id.to_string(), + name: "packaging_variant_a".to_string(), + packaging_level:1, + pack_size: Some(0.0), + ..Default::default() + }, + ); + assert_eq!( + result.unwrap_err(), + UpsertPackagingVariantError::LessThanZero("pack_size".to_string()) + ); + + // Test that we can't create a record with a volume_per_unit < 0 + let result = service.upsert_packaging_variant( + &context, + UpsertPackagingVariant { + id: test_packaging_variant_id.to_string(), + item_variant_id: test_item_variant_id.to_string(), + name: "packaging_variant_a".to_string(), + packaging_level:1, + volume_per_unit: Some(-1.0), + ..Default::default() + }, + ); + assert_eq!( + result.unwrap_err(), + UpsertPackagingVariantError::LessThanZero("volume_per_unit".to_string()) + ); + + + // Test that we can't create a record with a volume_per_unit == 0 + let result = service.upsert_packaging_variant( + &context, + UpsertPackagingVariant { + id: test_packaging_variant_id.to_string(), + item_variant_id: test_item_variant_id.to_string(), + name: "packaging_variant_a".to_string(), + packaging_level:1, + volume_per_unit: Some(0.0), + ..Default::default() + }, + ); + assert_eq!( + result.unwrap_err(), + UpsertPackagingVariantError::LessThanZero("volume_per_unit".to_string()) + ); } } diff --git a/server/service/src/item/packaging_variant/upsert.rs b/server/service/src/item/packaging_variant/upsert.rs index b35b9772ec..b2b13d006e 100644 --- a/server/service/src/item/packaging_variant/upsert.rs +++ b/server/service/src/item/packaging_variant/upsert.rs @@ -9,6 +9,8 @@ use crate::{check_item_variant_exists, service_provider::ServiceContext}; pub enum UpsertPackagingVariantError { CreatedRecordNotFound, ItemVariantDoesNotExist, + CantChangeItemVariant, + LessThanZero(String), DatabaseError(RepositoryError), } @@ -76,5 +78,35 @@ fn validate( return Err(UpsertPackagingVariantError::ItemVariantDoesNotExist); } + let old_packaging_variant = + PackagingVariantRowRepository::new(connection).find_one_by_id(&input.id)?; + if let Some(old_packaging_variant) = old_packaging_variant { + if old_packaging_variant.item_variant_id != input.item_variant_id { + return Err(UpsertPackagingVariantError::CantChangeItemVariant); + } + } + + if input.packaging_level <= 0 { + return Err(UpsertPackagingVariantError::LessThanZero( + "packaging_level".to_string(), + )); + } + + if let Some(pack_size) = input.pack_size { + if pack_size <= 0.0 { + return Err(UpsertPackagingVariantError::LessThanZero( + "pack_size".to_string(), + )); + } + } + + if let Some(volume_per_unit) = input.volume_per_unit { + if volume_per_unit <= 0.0 { + return Err(UpsertPackagingVariantError::LessThanZero( + "volume_per_unit".to_string(), + )); + } + } + Ok(()) }