From ace5361df5e7967ac307d7be8a1e9bd059155a1c Mon Sep 17 00:00:00 2001 From: seallard Date: Mon, 29 Jul 2024 15:04:43 +0200 Subject: [PATCH 1/4] Validate unique sample names per case --- .../order_validation_service/models/errors.py | 5 +++++ .../tomte/validation/inter_field/rules.py | 14 +++++++++++++- .../tomte/validation/inter_field/utils.py | 17 ++++++++++++++++- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cg/services/order_validation_service/models/errors.py b/cg/services/order_validation_service/models/errors.py index 2bbec19056..e6297f9fa3 100644 --- a/cg/services/order_validation_service/models/errors.py +++ b/cg/services/order_validation_service/models/errors.py @@ -53,3 +53,8 @@ class OrderNameRequiredError(OrderError): class OccupiedWellError(CaseSampleError): field: str = "well_position" message: str = "Well is already occupied" + + +class ReusedSampleNameError(CaseSampleError): + field: str = "name" + message: str = "Sample name already used" diff --git a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/rules.py b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/rules.py index 8142989ed6..1f136f9414 100644 --- a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/rules.py +++ b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/rules.py @@ -1,9 +1,13 @@ -from cg.services.order_validation_service.models.errors import OccupiedWellError +from cg.services.order_validation_service.models.errors import ( + OccupiedWellError, + ReusedSampleNameError, +) from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder from cg.services.order_validation_service.workflows.tomte.validation.inter_field.utils import ( _get_errors, _get_excess_samples, _get_plate_samples, + get_duplicate_sample_name_errors, ) @@ -11,3 +15,11 @@ def validate_wells_contain_at_most_one_sample(order: TomteOrder) -> list[Occupie samples_with_cases = _get_plate_samples(order) samples = _get_excess_samples(samples_with_cases) return _get_errors(samples) + + +def validate_unique_sample_names_in_cases(order: TomteOrder) -> list[ReusedSampleNameError]: + errors: list[ReusedSampleNameError] = [] + for case in order.cases: + case_errors = get_duplicate_sample_name_errors(case) + errors.extend(case_errors) + return errors diff --git a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py index c056dc4ba3..501a20ebd3 100644 --- a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py +++ b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py @@ -1,5 +1,9 @@ +from collections import Counter from cg.models.orders.sample_base import ContainerEnum -from cg.services.order_validation_service.models.errors import OccupiedWellError +from cg.services.order_validation_service.models.errors import ( + OccupiedWellError, + ReusedSampleNameError, +) from cg.services.order_validation_service.workflows.tomte.models.case import TomteCase from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder from cg.services.order_validation_service.workflows.tomte.models.sample import ( @@ -47,3 +51,14 @@ def _get_sample_well_map(plate_samples_with_cases: list[tuple[TomteSample, Tomte sample_well_map[sample.well_position] = [] sample_well_map[sample.well_position].append((sample, case)) return sample_well_map + + +def get_duplicate_sample_names(case: TomteCase) -> list[str]: + sample_names = [sample.name for sample in case.samples] + count = Counter(sample_names) + return list({name for name, freq in count.items() if freq > 1}) + + +def get_duplicate_sample_name_errors(case: TomteCase) -> list[ReusedSampleNameError]: + sample_names = get_duplicate_sample_names(case) + return [ReusedSampleNameError(sample_name=name, case_name=case.name) for name in sample_names] From 703346f1db36fe93b565b3887ad10c2570ee13b5 Mon Sep 17 00:00:00 2001 From: seallard Date: Mon, 29 Jul 2024 15:12:47 +0200 Subject: [PATCH 2/4] Add test --- .../workflows/tomte/validation_rules.py | 6 +++++- .../order_validation_service/conftest.py | 15 +++++++++++++++ .../test_tomte_inter_field_validators.py | 19 ++++++++++++++++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/cg/services/order_validation_service/workflows/tomte/validation_rules.py b/cg/services/order_validation_service/workflows/tomte/validation_rules.py index 4a173c9d09..e87e45fb49 100644 --- a/cg/services/order_validation_service/workflows/tomte/validation_rules.py +++ b/cg/services/order_validation_service/workflows/tomte/validation_rules.py @@ -7,6 +7,7 @@ validate_ticket_number_required_if_connected, ) from cg.services.order_validation_service.workflows.tomte.validation.inter_field.rules import ( + validate_unique_sample_names_in_cases, validate_wells_contain_at_most_one_sample, ) @@ -17,4 +18,7 @@ validate_customer_can_skip_reception_control, ] -TOMTE_CASE_SAMPLE_RULES = [validate_wells_contain_at_most_one_sample] +TOMTE_CASE_SAMPLE_RULES = [ + validate_unique_sample_names_in_cases, + validate_wells_contain_at_most_one_sample, +] diff --git a/tests/services/order_validation_service/conftest.py b/tests/services/order_validation_service/conftest.py index 4f4101291b..0d417740a0 100644 --- a/tests/services/order_validation_service/conftest.py +++ b/tests/services/order_validation_service/conftest.py @@ -71,3 +71,18 @@ def valid_order(valid_case: TomteCase) -> TomteOrder: @pytest.fixture def order_with_samples_in_same_well(case_with_samples_in_same_well: TomteCase) -> TomteOrder: return create_order([case_with_samples_in_same_well]) + + +@pytest.fixture +def case_with_samples_with_duplicate_names() -> TomteCase: + sample_1: TomteSample = create_sample(1) + sample_2: TomteSample = create_sample(1) + sample_1.name = sample_2.name + return create_case([sample_1, sample_2]) + + +@pytest.fixture +def order_with_duplicate_sample_names( + case_with_samples_with_duplicate_names: TomteCase, +) -> TomteOrder: + return create_order([case_with_samples_with_duplicate_names]) diff --git a/tests/services/order_validation_service/test_tomte_inter_field_validators.py b/tests/services/order_validation_service/test_tomte_inter_field_validators.py index c3fab447bd..3ac0b894df 100644 --- a/tests/services/order_validation_service/test_tomte_inter_field_validators.py +++ b/tests/services/order_validation_service/test_tomte_inter_field_validators.py @@ -1,6 +1,10 @@ -from cg.services.order_validation_service.models.errors import OccupiedWellError +from cg.services.order_validation_service.models.errors import ( + OccupiedWellError, + ReusedSampleNameError, +) from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder from cg.services.order_validation_service.workflows.tomte.validation.inter_field.rules import ( + validate_unique_sample_names_in_cases, validate_wells_contain_at_most_one_sample, ) @@ -28,3 +32,16 @@ def test_order_without_multiple_samples_in_well(valid_order: TomteOrder): # THEN no errors should be returned assert not errors + + +def test_duplicate_sample_names_not_allowed(order_with_duplicate_sample_names: TomteOrder): + # Given an order with samples in a case with the same name + + # WHEN validating the order + errors = validate_unique_sample_names_in_cases(order_with_duplicate_sample_names) + + # THEN errors are returned + assert errors + + # THEN the errors are about the sample names + assert isinstance(errors[0], ReusedSampleNameError) From add63a6eae5b9e04dea7edfc663d065310f5d0f2 Mon Sep 17 00:00:00 2001 From: Sebastian Allard Date: Mon, 29 Jul 2024 15:28:06 +0200 Subject: [PATCH 3/4] Update cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Isak Ohlsson Ångnell <40887124+islean@users.noreply.github.com> --- .../workflows/tomte/validation/inter_field/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py index 501a20ebd3..a157e5b1ea 100644 --- a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py +++ b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py @@ -56,7 +56,7 @@ def _get_sample_well_map(plate_samples_with_cases: list[tuple[TomteSample, Tomte def get_duplicate_sample_names(case: TomteCase) -> list[str]: sample_names = [sample.name for sample in case.samples] count = Counter(sample_names) - return list({name for name, freq in count.items() if freq > 1}) + return [name for name, freq in count.items() if freq > 1] def get_duplicate_sample_name_errors(case: TomteCase) -> list[ReusedSampleNameError]: From fa171d855675cc3d4241b1acfa1e3992d0fc0b44 Mon Sep 17 00:00:00 2001 From: seallard Date: Mon, 29 Jul 2024 15:45:22 +0200 Subject: [PATCH 4/4] Rename error --- cg/services/order_validation_service/models/errors.py | 2 +- .../workflows/tomte/validation/inter_field/rules.py | 6 +++--- .../workflows/tomte/validation/inter_field/utils.py | 6 +++--- .../test_tomte_inter_field_validators.py | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cg/services/order_validation_service/models/errors.py b/cg/services/order_validation_service/models/errors.py index e6297f9fa3..2eb3f05505 100644 --- a/cg/services/order_validation_service/models/errors.py +++ b/cg/services/order_validation_service/models/errors.py @@ -55,6 +55,6 @@ class OccupiedWellError(CaseSampleError): message: str = "Well is already occupied" -class ReusedSampleNameError(CaseSampleError): +class RepeatedSampleNameError(CaseSampleError): field: str = "name" message: str = "Sample name already used" diff --git a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/rules.py b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/rules.py index 1f136f9414..926ec28048 100644 --- a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/rules.py +++ b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/rules.py @@ -1,6 +1,6 @@ from cg.services.order_validation_service.models.errors import ( OccupiedWellError, - ReusedSampleNameError, + RepeatedSampleNameError, ) from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder from cg.services.order_validation_service.workflows.tomte.validation.inter_field.utils import ( @@ -17,8 +17,8 @@ def validate_wells_contain_at_most_one_sample(order: TomteOrder) -> list[Occupie return _get_errors(samples) -def validate_unique_sample_names_in_cases(order: TomteOrder) -> list[ReusedSampleNameError]: - errors: list[ReusedSampleNameError] = [] +def validate_unique_sample_names_in_cases(order: TomteOrder) -> list[RepeatedSampleNameError]: + errors: list[RepeatedSampleNameError] = [] for case in order.cases: case_errors = get_duplicate_sample_name_errors(case) errors.extend(case_errors) diff --git a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py index a157e5b1ea..7aa76251e9 100644 --- a/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py +++ b/cg/services/order_validation_service/workflows/tomte/validation/inter_field/utils.py @@ -2,7 +2,7 @@ from cg.models.orders.sample_base import ContainerEnum from cg.services.order_validation_service.models.errors import ( OccupiedWellError, - ReusedSampleNameError, + RepeatedSampleNameError, ) from cg.services.order_validation_service.workflows.tomte.models.case import TomteCase from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder @@ -59,6 +59,6 @@ def get_duplicate_sample_names(case: TomteCase) -> list[str]: return [name for name, freq in count.items() if freq > 1] -def get_duplicate_sample_name_errors(case: TomteCase) -> list[ReusedSampleNameError]: +def get_duplicate_sample_name_errors(case: TomteCase) -> list[RepeatedSampleNameError]: sample_names = get_duplicate_sample_names(case) - return [ReusedSampleNameError(sample_name=name, case_name=case.name) for name in sample_names] + return [RepeatedSampleNameError(sample_name=name, case_name=case.name) for name in sample_names] diff --git a/tests/services/order_validation_service/test_tomte_inter_field_validators.py b/tests/services/order_validation_service/test_tomte_inter_field_validators.py index 3ac0b894df..9fef1a06b1 100644 --- a/tests/services/order_validation_service/test_tomte_inter_field_validators.py +++ b/tests/services/order_validation_service/test_tomte_inter_field_validators.py @@ -1,6 +1,6 @@ from cg.services.order_validation_service.models.errors import ( OccupiedWellError, - ReusedSampleNameError, + RepeatedSampleNameError, ) from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder from cg.services.order_validation_service.workflows.tomte.validation.inter_field.rules import ( @@ -44,4 +44,4 @@ def test_duplicate_sample_names_not_allowed(order_with_duplicate_sample_names: T assert errors # THEN the errors are about the sample names - assert isinstance(errors[0], ReusedSampleNameError) + assert isinstance(errors[0], RepeatedSampleNameError)