From 37760655ae9f2a7f353a45ab35a9f03b10ffce9f Mon Sep 17 00:00:00 2001 From: islean Date: Thu, 29 Aug 2024 14:26:44 +0200 Subject: [PATCH 1/3] Remove unnecessary rules --- .../order_validation_service/models/case.py | 13 ++++--- .../rules/case_sample/rules.py | 22 ------------ .../rules/sample/rules.py | 25 ------------- .../workflows/microsalt/models/sample.py | 10 +++--- .../workflows/microsalt/validation_rules.py | 2 -- .../workflows/mip_dna/validation_rules.py | 4 --- .../workflows/tomte/validation_rules.py | 5 --- .../sample_rules/conftest.py | 5 ++- .../sample_rules/test_data_validators.py | 34 ------------------ .../test_data_validators.py | 15 -------- .../test_tomte_inter_field_validators.py | 35 +++++-------------- 11 files changed, 26 insertions(+), 144 deletions(-) diff --git a/cg/services/order_validation_service/models/case.py b/cg/services/order_validation_service/models/case.py index eb1b8e1955..26250cc007 100644 --- a/cg/services/order_validation_service/models/case.py +++ b/cg/services/order_validation_service/models/case.py @@ -7,25 +7,28 @@ from cg.services.order_validation_service.models.existing_sample import ExistingSample from cg.services.order_validation_service.models.sample import Sample +NewSample = Annotated[Sample, Tag("new")] +ExistingSampleType = Annotated[ExistingSample, Tag("existing")] + class Case(BaseModel): name: str = Field(pattern=NAME_PATTERN, min_length=2, max_length=128) priority: PriorityTerms = PriorityTerms.STANDARD samples: list[ Annotated[ - Annotated[Sample, Tag("new")] | Annotated[ExistingSample, Tag("existing")], + NewSample | ExistingSampleType, Discriminator(has_internal_id), ] ] - @property - def enumerated_samples(self): - return enumerate(self.samples) - @property def is_new(self) -> bool: return True + @property + def enumerated_samples(self): + return enumerate(self.samples) + @property def enumerated_new_samples(self): samples: list[tuple[int, Sample]] = [] diff --git a/cg/services/order_validation_service/rules/case_sample/rules.py b/cg/services/order_validation_service/rules/case_sample/rules.py index 309d3d6e4a..aa87d090c3 100644 --- a/cg/services/order_validation_service/rules/case_sample/rules.py +++ b/cg/services/order_validation_service/rules/case_sample/rules.py @@ -24,8 +24,6 @@ SampleDoesNotExistError, SampleNameRepeatedError, SexMissingError, - SourceMissingError, - StatusMissingError, SubjectIdSameAsCaseNameError, SubjectIdSameAsSampleNameError, WellPositionMissingError, @@ -232,16 +230,6 @@ def validate_sex_required_for_new_samples(order: OrderWithCases, **kwargs) -> li return errors -def validate_source_required(order: OrderWithCases, **kwargs) -> list[SourceMissingError]: - errors: list[SourceMissingError] = [] - for case_index, case in order.enumerated_new_cases: - for sample_index, sample in case.enumerated_new_samples: - if not sample.source: - error = SourceMissingError(case_index=case_index, sample_index=sample_index) - errors.append(error) - return errors - - def validate_wells_contain_at_most_one_sample( order: OrderWithCases, **kwargs ) -> list[OccupiedWellError]: @@ -350,13 +338,3 @@ def validate_concentration_interval_if_skip_rc( ) errors.extend(case_errors) return errors - - -def validate_status_required_if_new(order: OrderWithCases, **kwargs) -> list[StatusMissingError]: - errors: list[StatusMissingError] = [] - for case_index, case in order.enumerated_new_cases: - for sample_index, sample in case.enumerated_new_samples: - if not sample.status: - error = StatusMissingError(case_index=case_index, sample_index=sample_index) - errors.append(error) - return errors diff --git a/cg/services/order_validation_service/rules/sample/rules.py b/cg/services/order_validation_service/rules/sample/rules.py index e748ed5ede..d84f2e186d 100644 --- a/cg/services/order_validation_service/rules/sample/rules.py +++ b/cg/services/order_validation_service/rules/sample/rules.py @@ -4,8 +4,6 @@ ApplicationArchivedError, ApplicationNotCompatibleError, ApplicationNotValidError, - ElutionBufferMissingError, - ExtractionMethodMissingError, InvalidVolumeError, OccupiedWellError, OrganismDoesNotExistError, @@ -68,29 +66,6 @@ def validate_organism_exists( return errors -def validate_buffer_required( - order: OrderWithNonHumanSamples, - **kwargs, -) -> list[ElutionBufferMissingError]: - errors: list[ElutionBufferMissingError] = [] - for sample_index, sample in order.enumerated_samples: - if not sample.elution_buffer: - error = ElutionBufferMissingError(sample_index=sample_index) - errors.append(error) - return errors - - -def validate_extraction_method_required( - order: OrderWithNonHumanSamples, **kwargs -) -> list[ExtractionMethodMissingError]: - errors: list[ExtractionMethodMissingError] = [] - for sample_index, sample in order.enumerated_samples: - if not sample.extraction_method: - error = ExtractionMethodMissingError(sample_index=sample_index) - errors.append(error) - return errors - - def validate_application_compatibility( order: OrderWithNonHumanSamples, store: Store, diff --git a/cg/services/order_validation_service/workflows/microsalt/models/sample.py b/cg/services/order_validation_service/workflows/microsalt/models/sample.py index 0e00c12281..e0a5d89f75 100644 --- a/cg/services/order_validation_service/workflows/microsalt/models/sample.py +++ b/cg/services/order_validation_service/workflows/microsalt/models/sample.py @@ -7,10 +7,10 @@ class MicrosaltSample(Sample): control: ControlEnum | None = None - elution_buffer: str | None = None - extraction_method: ExtractionMethod | None = None - organism: str | None = None - priority: PriorityEnum | None = None + elution_buffer: str + extraction_method: ExtractionMethod + organism: str + priority: PriorityEnum quantity: int | None = None - reference_genome: str | None = Field(default=None, max_length=255) + reference_genome: str = Field(max_length=255) sample_concentration: float | None = None diff --git a/cg/services/order_validation_service/workflows/microsalt/validation_rules.py b/cg/services/order_validation_service/workflows/microsalt/validation_rules.py index 55a4ae2a6b..2138560605 100644 --- a/cg/services/order_validation_service/workflows/microsalt/validation_rules.py +++ b/cg/services/order_validation_service/workflows/microsalt/validation_rules.py @@ -2,7 +2,6 @@ validate_application_compatibility, validate_application_exists, validate_applications_not_archived, - validate_buffer_required, validate_organism_exists, validate_sample_names_available, validate_sample_names_unique, @@ -15,7 +14,6 @@ validate_application_compatibility, validate_application_exists, validate_applications_not_archived, - validate_buffer_required, validate_organism_exists, validate_sample_names_available, validate_sample_names_unique, diff --git a/cg/services/order_validation_service/workflows/mip_dna/validation_rules.py b/cg/services/order_validation_service/workflows/mip_dna/validation_rules.py index 29d768fa92..8dd187f48b 100644 --- a/cg/services/order_validation_service/workflows/mip_dna/validation_rules.py +++ b/cg/services/order_validation_service/workflows/mip_dna/validation_rules.py @@ -20,8 +20,6 @@ validate_sample_names_not_repeated, validate_samples_exist, validate_sex_required_for_new_samples, - validate_source_required, - validate_status_required_if_new, validate_subject_ids_different_from_case_names, validate_subject_ids_different_from_sample_names, validate_volume_interval, @@ -51,8 +49,6 @@ validate_samples_exist, validate_sample_names_not_repeated, validate_sex_required_for_new_samples, - validate_source_required, - validate_status_required_if_new, validate_subject_ids_different_from_case_names, validate_subject_ids_different_from_sample_names, validate_volume_interval, 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 f23c691db0..8dd187f48b 100644 --- a/cg/services/order_validation_service/workflows/tomte/validation_rules.py +++ b/cg/services/order_validation_service/workflows/tomte/validation_rules.py @@ -20,15 +20,12 @@ validate_sample_names_not_repeated, validate_samples_exist, validate_sex_required_for_new_samples, - validate_source_required, - validate_status_required_if_new, validate_subject_ids_different_from_case_names, validate_subject_ids_different_from_sample_names, validate_volume_interval, validate_wells_contain_at_most_one_sample, ) - CASE_RULES: list[callable] = [ validate_case_internal_ids_exist, validate_case_names_available, @@ -52,8 +49,6 @@ validate_samples_exist, validate_sample_names_not_repeated, validate_sex_required_for_new_samples, - validate_source_required, - validate_status_required_if_new, validate_subject_ids_different_from_case_names, validate_subject_ids_different_from_sample_names, validate_volume_interval, diff --git a/tests/services/order_validation_service/sample_rules/conftest.py b/tests/services/order_validation_service/sample_rules/conftest.py index 8ca8d01e07..e5cd9c0e15 100644 --- a/tests/services/order_validation_service/sample_rules/conftest.py +++ b/tests/services/order_validation_service/sample_rules/conftest.py @@ -1,9 +1,10 @@ import pytest from cg.constants.constants import Workflow -from cg.models.orders.sample_base import ContainerEnum +from cg.models.orders.sample_base import ContainerEnum, PriorityEnum from cg.services.order_validation_service.constants import ( MINIMUM_VOLUME, + ElutionBuffer, ExtractionMethod, ) from cg.services.order_validation_service.workflows.microsalt.constants import ( @@ -25,8 +26,10 @@ def create_microsalt_sample(id: int) -> MicrosaltSample: application="MWRNXTR003", container=ContainerEnum.plate, container_name="ContainerName", + elution_buffer=ElutionBuffer.WATER, extraction_method=ExtractionMethod.MAELSTROM, organism="C. jejuni", + priority=PriorityEnum.standard, require_qc_ok=True, reference_genome="NC_00001", well_position=f"A:{id}", diff --git a/tests/services/order_validation_service/sample_rules/test_data_validators.py b/tests/services/order_validation_service/sample_rules/test_data_validators.py index 3f72a2009a..c7fb7611bf 100644 --- a/tests/services/order_validation_service/sample_rules/test_data_validators.py +++ b/tests/services/order_validation_service/sample_rules/test_data_validators.py @@ -3,8 +3,6 @@ ApplicationArchivedError, ApplicationNotCompatibleError, ApplicationNotValidError, - ElutionBufferMissingError, - ExtractionMethodMissingError, InvalidVolumeError, OrganismDoesNotExistError, ) @@ -12,8 +10,6 @@ validate_application_compatibility, validate_application_exists, validate_applications_not_archived, - validate_buffer_required, - validate_extraction_method_required, validate_organism_exists, validate_volume_interval, ) @@ -119,33 +115,3 @@ def test_valid_organisms(valid_order: MicrosaltOrder, base_store: Store): # THEN no error should be returned assert not errors - - -def test_buffer_required(valid_order: MicrosaltOrder): - - # GIVEN an order containing a sample with missing buffer - valid_order.samples[0].elution_buffer = None - - # WHEN validating that buffers are set for new samples - errors = validate_buffer_required(order=valid_order) - - # THEN an error should be returned - assert errors - - # THEN the error should concern the missing buffer - assert isinstance(errors[0], ElutionBufferMissingError) - - -def test_extraction_method_missing(valid_order: MicrosaltOrder): - - # GIVEN an order containing a sample with missing extraction method - valid_order.samples[0].extraction_method = None - - # WHEN validating that the extraction method is set for all new samples - errors = validate_extraction_method_required(order=valid_order) - - # THEN an error should be raised - assert errors - - # THEN the error should concern the missing extraction method - assert isinstance(errors[0], ExtractionMethodMissingError) diff --git a/tests/services/order_validation_service/test_data_validators.py b/tests/services/order_validation_service/test_data_validators.py index 49e065ffca..d462c39bb3 100644 --- a/tests/services/order_validation_service/test_data_validators.py +++ b/tests/services/order_validation_service/test_data_validators.py @@ -11,7 +11,6 @@ ApplicationNotValidError, SampleDoesNotExistError, SexMissingError, - SourceMissingError, ) from cg.services.order_validation_service.models.existing_case import ExistingCase from cg.services.order_validation_service.models.existing_sample import ExistingSample @@ -26,7 +25,6 @@ validate_gene_panels_exist, validate_samples_exist, validate_sex_required_for_new_samples, - validate_source_required, ) from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder from cg.store.models import Application @@ -172,16 +170,3 @@ def test_sex_missing_for_new_sample(valid_order: TomteOrder): # THEN the error should concern the missing sex assert isinstance(errors[0], SexMissingError) - - -def test_source_missing_for_new_sample(valid_order: TomteOrder): - - # GIVEN an order with a new sample without 'source' set - valid_order.cases[0].samples[0].source = None - - # WHEN validating that all new samples have 'source' set - errors = validate_source_required(order=valid_order) - - assert errors - - assert isinstance(errors[0], SourceMissingError) 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 381f17e93d..4482394d0b 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 @@ -11,28 +11,26 @@ OccupiedWellError, SampleIsOwnFatherError, SampleNameRepeatedError, - StatusMissingError, SubjectIdSameAsSampleNameError, ) -from cg.services.order_validation_service.rules.case.rules import validate_case_names_not_repeated -from cg.services.order_validation_service.rules.case_sample.rules import ( - validate_buffers_are_allowed, - validate_concentration_required_if_skip_rc, - validate_subject_ids_different_from_sample_names, -) -from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder -from cg.services.order_validation_service.workflows.tomte.models.sample import ( - TomteSample, +from cg.services.order_validation_service.rules.case.rules import ( + validate_case_names_not_repeated, ) from cg.services.order_validation_service.rules.case_sample.rules import ( + validate_buffers_are_allowed, validate_concentration_interval_if_skip_rc, + validate_concentration_required_if_skip_rc, validate_fathers_are_male, validate_fathers_in_same_case_as_children, validate_pedigree, validate_sample_names_not_repeated, - validate_status_required_if_new, + validate_subject_ids_different_from_sample_names, validate_wells_contain_at_most_one_sample, ) +from cg.services.order_validation_service.workflows.tomte.models.order import TomteOrder +from cg.services.order_validation_service.workflows.tomte.models.sample import ( + TomteSample, +) from cg.store.models import Application from cg.store.store import Store @@ -240,18 +238,3 @@ def test_concentration_not_within_interval_if_skip_rc( # THEN the error should concern the application interval assert isinstance(errors[0], InvalidConcentrationIfSkipRCError) - - -def test_missing_status_on_new_sample(valid_order: TomteOrder): - - # GIVEN an order with a new sample with 'status' not set - valid_order.cases[0].samples[0].status = None - - # WHEN validating that all new samples have a provided status - errors = validate_status_required_if_new(valid_order) - - # THEN an error should be returned - assert errors - - # THEN the error should concern the missing status - assert isinstance(errors[0], StatusMissingError) From 1aa8498155b009270e45850cf178ddd213a6e285 Mon Sep 17 00:00:00 2001 From: islean Date: Thu, 29 Aug 2024 14:28:31 +0200 Subject: [PATCH 2/3] Remove unnecessary rules --- .../errors/case_sample_errors.py | 10 ---------- .../order_validation_service/errors/sample_errors.py | 10 ---------- 2 files changed, 20 deletions(-) diff --git a/cg/services/order_validation_service/errors/case_sample_errors.py b/cg/services/order_validation_service/errors/case_sample_errors.py index 037f380d6b..7d04d8857d 100644 --- a/cg/services/order_validation_service/errors/case_sample_errors.py +++ b/cg/services/order_validation_service/errors/case_sample_errors.py @@ -79,11 +79,6 @@ class MotherNotInCaseError(CaseSampleError): message: str = "Mother must be in the same case" -class StatusMissingError(CaseSampleError): - field: str = "status" - message: str = "Carrier status is required" - - class SampleDoesNotExistError(CaseSampleError): field: str = "internal_id" message: str = "The sample does not exist" @@ -94,11 +89,6 @@ class SexMissingError(CaseSampleError): message: str = "Sex is required" -class SourceMissingError(CaseSampleError): - field: str = "source" - message: str = "Source is required" - - class SubjectIdSameAsCaseNameError(CaseSampleError): field: str = "subject_id" message: str = "Subject id must be different from the case name" diff --git a/cg/services/order_validation_service/errors/sample_errors.py b/cg/services/order_validation_service/errors/sample_errors.py index f0203d9034..2733f7397e 100644 --- a/cg/services/order_validation_service/errors/sample_errors.py +++ b/cg/services/order_validation_service/errors/sample_errors.py @@ -49,16 +49,6 @@ class OrganismDoesNotExistError(SampleError): message: str = "Organism does not exist" -class ElutionBufferMissingError(SampleError): - field: str = "elution_buffer" - message: str = "Buffer is required" - - -class ExtractionMethodMissingError(SampleError): - field: str = "extraction_method" - message: str = "Extraction method is required" - - class SampleNameNotAvailableError(SampleError): field: str = "name" message: str = "Sample name already used in previous order" From 7984b286b2be8c1807522549366377a2ae4cd9f6 Mon Sep 17 00:00:00 2001 From: islean Date: Thu, 29 Aug 2024 14:32:52 +0200 Subject: [PATCH 3/3] Remove unnecessary rules --- .../workflows/tomte/models/case.py | 10 ++++------ .../workflows/tomte/models/order.py | 10 ++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/cg/services/order_validation_service/workflows/tomte/models/case.py b/cg/services/order_validation_service/workflows/tomte/models/case.py index 75dad382f3..a6a3e28288 100644 --- a/cg/services/order_validation_service/workflows/tomte/models/case.py +++ b/cg/services/order_validation_service/workflows/tomte/models/case.py @@ -8,17 +8,15 @@ TomteSample, ) +NewSample = Annotated[TomteSample, Tag("new")] +OldSample = Annotated[ExistingSample, Tag("existing")] + class TomteCase(Case): cohorts: list[str] | None = None panels: list[str] synopsis: str | None = None - samples: list[ - Annotated[ - Annotated[TomteSample, Tag("new")] | Annotated[ExistingSample, Tag("existing")], - Discriminator(has_internal_id), - ] - ] + samples: list[Annotated[NewSample | OldSample, Discriminator(has_internal_id)]] def get_sample(self, sample_name: str) -> TomteSample | None: for sample in self.samples: diff --git a/cg/services/order_validation_service/workflows/tomte/models/order.py b/cg/services/order_validation_service/workflows/tomte/models/order.py index 720ca29ec5..68ffd53c55 100644 --- a/cg/services/order_validation_service/workflows/tomte/models/order.py +++ b/cg/services/order_validation_service/workflows/tomte/models/order.py @@ -9,12 +9,10 @@ ) from cg.services.order_validation_service.workflows.tomte.models.case import TomteCase +NewCase = Annotated[TomteCase, Tag("new")] +OldCase = Annotated[ExistingCase, Tag("existing")] + class TomteOrder(OrderWithCases): - cases: list[ - Annotated[ - Annotated[TomteCase, Tag("new")] | Annotated[ExistingCase, Tag("existing")], - Discriminator(has_internal_id), - ] - ] + cases: list[Annotated[NewCase | OldCase, Discriminator(has_internal_id)]] delivery_type: TomteDeliveryType