From 45f318e649a7f78a602fd8f97270247f14e3d13f Mon Sep 17 00:00:00 2001 From: seallard Date: Thu, 29 Aug 2024 13:29:18 +0200 Subject: [PATCH 1/7] Add balsamic validation rules --- .../workflows/balsamic/validation_rules.py | 62 ++++++++++++++++++- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/cg/services/order_validation_service/workflows/balsamic/validation_rules.py b/cg/services/order_validation_service/workflows/balsamic/validation_rules.py index 1b73529379..29d768fa92 100644 --- a/cg/services/order_validation_service/workflows/balsamic/validation_rules.py +++ b/cg/services/order_validation_service/workflows/balsamic/validation_rules.py @@ -1,2 +1,60 @@ -CASE_RULES = [] -CASE_SAMPLE_RULES = [] +from cg.services.order_validation_service.rules.case.rules import ( + validate_case_internal_ids_exist, + validate_case_names_available, + validate_case_names_not_repeated, + validate_gene_panels_unique, +) +from cg.services.order_validation_service.rules.case_sample.rules import ( + validate_application_compatibility, + validate_application_exists, + validate_application_not_archived, + validate_buffer_skip_rc_condition, + validate_concentration_interval_if_skip_rc, + validate_concentration_required_if_skip_rc, + validate_fathers_are_male, + validate_fathers_in_same_case_as_children, + validate_gene_panels_exist, + validate_mothers_are_female, + validate_mothers_in_same_case_as_children, + validate_pedigree, + 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, + validate_case_names_not_repeated, + validate_gene_panels_exist, + validate_gene_panels_unique, +] + +CASE_SAMPLE_RULES: list[callable] = [ + validate_application_compatibility, + validate_application_exists, + validate_application_not_archived, + validate_buffer_skip_rc_condition, + validate_concentration_interval_if_skip_rc, + validate_concentration_required_if_skip_rc, + validate_fathers_are_male, + validate_fathers_in_same_case_as_children, + validate_mothers_are_female, + validate_mothers_in_same_case_as_children, + validate_pedigree, + 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, + validate_wells_contain_at_most_one_sample, +] From b532abd670fc75d52d8c17e40a0e645d031a2306 Mon Sep 17 00:00:00 2001 From: seallard Date: Thu, 29 Aug 2024 14:34:18 +0200 Subject: [PATCH 2/7] Add sex validation for subject id --- .../errors/case_sample_errors.py | 5 ++++ .../balsamic/rules/case_sample/rules.py | 30 +++++++++++++++++++ .../balsamic/rules/case_sample/utils.py | 5 ++++ cg/store/crud/read.py | 13 ++++++++ 4 files changed, 53 insertions(+) create mode 100644 cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py create mode 100644 cg/services/order_validation_service/workflows/balsamic/rules/case_sample/utils.py 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..a35ba0f746 100644 --- a/cg/services/order_validation_service/errors/case_sample_errors.py +++ b/cg/services/order_validation_service/errors/case_sample_errors.py @@ -143,3 +143,8 @@ class InvalidVolumeError(CaseSampleError): class InvalidBufferError(CaseSampleError): field: str = "elution_buffer" message: str = "The chosen buffer is not allowed when skipping reception control" + + +class SubjectIdGenderError(CaseSampleError): + field: str = "subject_id" + message: str = "Another sample with the same subject id has a different gender" diff --git a/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py new file mode 100644 index 0000000000..a7fdab6dbf --- /dev/null +++ b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py @@ -0,0 +1,30 @@ +from cg.services.order_validation_service.errors.case_sample_errors import SubjectIdGenderError +from cg.services.order_validation_service.workflows.balsamic.models.order import BalsamicOrder +from cg.services.order_validation_service.workflows.balsamic.rules.case_sample.utils import ( + has_sex_and_subject, +) +from cg.store.store import Store + + +def validate_subject_id_sex_consistency( + order: BalsamicOrder, + store: Store, +) -> list[SubjectIdGenderError]: + errors: list[SubjectIdGenderError] = [] + + for case_index, case in order.enumerated_new_cases: + for sample_index, sample in case.enumerated_new_samples: + if not has_sex_and_subject(sample): + continue + + if store.sample_exists_with_different_sex( + customer_internal_id=order.customer, + subject_id=sample.subject_id, + sex=sample.sex, + ): + error = SubjectIdGenderError( + case_index=case_index, + sample_index=sample_index, + ) + errors.append(error) + return errors diff --git a/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/utils.py b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/utils.py new file mode 100644 index 0000000000..ea4613c383 --- /dev/null +++ b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/utils.py @@ -0,0 +1,5 @@ +from cg.services.order_validation_service.workflows.balsamic.models.sample import BalsamicSample + + +def has_sex_and_subject(sample: BalsamicSample) -> bool: + return sample.subject_id and sample.sex diff --git a/cg/store/crud/read.py b/cg/store/crud/read.py index ef3be2b4ac..25f5699337 100644 --- a/cg/store/crud/read.py +++ b/cg/store/crud/read.py @@ -1562,3 +1562,16 @@ def get_case_ids_for_samples(self, sample_ids: list[int]) -> list[str]: for sample_id in sample_ids: case_ids.extend(self.get_case_ids_with_sample(sample_id)) return list(set(case_ids)) + + def sample_exists_with_different_sex( + self, + customer_internal_id: str, + subject_id: str, + sex: str, + ) -> bool: + samples: list[Sample] = self.get_samples_by_customer_and_subject_id( + customer_internal_id=customer_internal_id, + subject_id=subject_id, + ) + existing_sex: set[str] = {sample.sex for sample in samples} + return bool(samples) and sex not in existing_sex From 793c585d63333a2a4264a98097711f97bf967028 Mon Sep 17 00:00:00 2001 From: seallard Date: Thu, 29 Aug 2024 14:38:22 +0200 Subject: [PATCH 3/7] Fix merge --- .../workflows/balsamic/validation_rules.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/cg/services/order_validation_service/workflows/balsamic/validation_rules.py b/cg/services/order_validation_service/workflows/balsamic/validation_rules.py index 29d768fa92..cbfb6bb994 100644 --- a/cg/services/order_validation_service/workflows/balsamic/validation_rules.py +++ b/cg/services/order_validation_service/workflows/balsamic/validation_rules.py @@ -11,12 +11,7 @@ validate_buffer_skip_rc_condition, validate_concentration_interval_if_skip_rc, validate_concentration_required_if_skip_rc, - validate_fathers_are_male, - validate_fathers_in_same_case_as_children, validate_gene_panels_exist, - validate_mothers_are_female, - validate_mothers_in_same_case_as_children, - validate_pedigree, validate_sample_names_not_repeated, validate_samples_exist, validate_sex_required_for_new_samples, @@ -32,8 +27,6 @@ validate_case_internal_ids_exist, validate_case_names_available, validate_case_names_not_repeated, - validate_gene_panels_exist, - validate_gene_panels_unique, ] CASE_SAMPLE_RULES: list[callable] = [ @@ -43,11 +36,6 @@ validate_buffer_skip_rc_condition, validate_concentration_interval_if_skip_rc, validate_concentration_required_if_skip_rc, - validate_fathers_are_male, - validate_fathers_in_same_case_as_children, - validate_mothers_are_female, - validate_mothers_in_same_case_as_children, - validate_pedigree, validate_samples_exist, validate_sample_names_not_repeated, validate_sex_required_for_new_samples, From cfb8d79760b8ee4534916e76cd6e2c19fe8801aa Mon Sep 17 00:00:00 2001 From: seallard Date: Thu, 29 Aug 2024 14:38:46 +0200 Subject: [PATCH 4/7] Fix merge --- .../workflows/balsamic/validation_rules.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/cg/services/order_validation_service/workflows/balsamic/validation_rules.py b/cg/services/order_validation_service/workflows/balsamic/validation_rules.py index cbfb6bb994..b96906073a 100644 --- a/cg/services/order_validation_service/workflows/balsamic/validation_rules.py +++ b/cg/services/order_validation_service/workflows/balsamic/validation_rules.py @@ -2,7 +2,6 @@ validate_case_internal_ids_exist, validate_case_names_available, validate_case_names_not_repeated, - validate_gene_panels_unique, ) from cg.services.order_validation_service.rules.case_sample.rules import ( validate_application_compatibility, @@ -11,7 +10,6 @@ validate_buffer_skip_rc_condition, validate_concentration_interval_if_skip_rc, validate_concentration_required_if_skip_rc, - validate_gene_panels_exist, validate_sample_names_not_repeated, validate_samples_exist, validate_sex_required_for_new_samples, From caa8200962b8512a8a57154afb998e3a34317872 Mon Sep 17 00:00:00 2001 From: seallard Date: Thu, 29 Aug 2024 14:39:37 +0200 Subject: [PATCH 5/7] Naming --- .../workflows/balsamic/rules/case_sample/rules.py | 2 +- .../workflows/balsamic/validation_rules.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py index a7fdab6dbf..b08cfa2a60 100644 --- a/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py +++ b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py @@ -6,7 +6,7 @@ from cg.store.store import Store -def validate_subject_id_sex_consistency( +def validate_subject_sex_consistency( order: BalsamicOrder, store: Store, ) -> list[SubjectIdGenderError]: diff --git a/cg/services/order_validation_service/workflows/balsamic/validation_rules.py b/cg/services/order_validation_service/workflows/balsamic/validation_rules.py index b96906073a..0894ea39fb 100644 --- a/cg/services/order_validation_service/workflows/balsamic/validation_rules.py +++ b/cg/services/order_validation_service/workflows/balsamic/validation_rules.py @@ -20,6 +20,9 @@ validate_volume_interval, validate_wells_contain_at_most_one_sample, ) +from cg.services.order_validation_service.workflows.balsamic.rules.case_sample.rules import ( + validate_subject_sex_consistency, +) CASE_RULES: list[callable] = [ validate_case_internal_ids_exist, @@ -43,4 +46,5 @@ validate_subject_ids_different_from_sample_names, validate_volume_interval, validate_wells_contain_at_most_one_sample, + validate_subject_sex_consistency, ] From 8e300e4505e04f4a521b56d869dc2b5f7e97879d Mon Sep 17 00:00:00 2001 From: islean Date: Fri, 30 Aug 2024 10:35:35 +0200 Subject: [PATCH 6/7] Address comments --- .../errors/case_sample_errors.py | 6 ++-- .../balsamic/rules/case_sample/utils.py | 7 +++-- cg/store/crud/read.py | 29 ++++++++++++------- .../workflows/__init__.py | 0 .../workflows/balsamic/__init__.py | 0 .../balsamic/test_case_sample_rules.py | 0 6 files changed, 26 insertions(+), 16 deletions(-) create mode 100644 tests/services/order_validation_service/workflows/__init__.py create mode 100644 tests/services/order_validation_service/workflows/balsamic/__init__.py create mode 100644 tests/services/order_validation_service/workflows/balsamic/test_case_sample_rules.py 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 a35ba0f746..80c9b63655 100644 --- a/cg/services/order_validation_service/errors/case_sample_errors.py +++ b/cg/services/order_validation_service/errors/case_sample_errors.py @@ -145,6 +145,6 @@ class InvalidBufferError(CaseSampleError): message: str = "The chosen buffer is not allowed when skipping reception control" -class SubjectIdGenderError(CaseSampleError): - field: str = "subject_id" - message: str = "Another sample with the same subject id has a different gender" +class SexSubjectIdError(CaseSampleError): + field: str = "sex" + message: str = "Another sample with the same subject id has a different sex" diff --git a/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/utils.py b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/utils.py index ea4613c383..b0b6f450a2 100644 --- a/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/utils.py +++ b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/utils.py @@ -1,5 +1,8 @@ -from cg.services.order_validation_service.workflows.balsamic.models.sample import BalsamicSample +from cg.models.orders.sample_base import SexEnum +from cg.services.order_validation_service.workflows.balsamic.models.sample import ( + BalsamicSample, +) def has_sex_and_subject(sample: BalsamicSample) -> bool: - return sample.subject_id and sample.sex + return bool(sample.subject_id and sample.sex != SexEnum.unknown) diff --git a/cg/store/crud/read.py b/cg/store/crud/read.py index 25f5699337..250783f387 100644 --- a/cg/store/crud/read.py +++ b/cg/store/crud/read.py @@ -10,9 +10,13 @@ from cg.constants import SequencingRunDataAvailability, Workflow from cg.constants.constants import CaseActions, CustomerId, PrepCategory, SampleType from cg.exc import CaseNotFoundError, CgError, OrderNotFoundError, SampleNotFoundError +from cg.models.orders.sample_base import SexEnum from cg.server.dto.orders.orders_request import OrdersRequest -from cg.server.dto.samples.collaborator_samples_request import CollaboratorSamplesRequest +from cg.server.dto.samples.collaborator_samples_request import ( + CollaboratorSamplesRequest, +) from cg.store.base import BaseHandler +from cg.store.exc import EntryNotFoundError from cg.store.filters.status_analysis_filters import ( AnalysisFilter, apply_analysis_filter, @@ -21,9 +25,6 @@ ApplicationFilter, apply_application_filter, ) -from cg.store.exc import EntryNotFoundError -from cg.store.filters.status_analysis_filters import AnalysisFilter, apply_analysis_filter -from cg.store.filters.status_application_filters import ApplicationFilter, apply_application_filter from cg.store.filters.status_application_limitations_filters import ( ApplicationLimitationsFilter, apply_application_limitations_filter, @@ -64,11 +65,13 @@ ) from cg.store.filters.status_invoice_filters import InvoiceFilter, apply_invoice_filter from cg.store.filters.status_order_filters import OrderFilter, apply_order_filters - -from cg.store.filters.status_organism_filters import OrganismFilter, apply_organism_filter +from cg.store.filters.status_organism_filters import ( + OrganismFilter, + apply_organism_filter, +) from cg.store.filters.status_pacbio_smrt_cell_filters import ( - apply_pac_bio_smrt_cell_filters, PacBioSMRTCellFilter, + apply_pac_bio_smrt_cell_filters, ) from cg.store.filters.status_panel_filters import PanelFilter, apply_panel_filter from cg.store.filters.status_pool_filters import PoolFilter, apply_pool_filter @@ -91,12 +94,12 @@ Invoice, Order, Organism, + PacBioSMRTCell, Panel, Pool, Sample, SampleRunMetrics, User, - PacBioSMRTCell, ) LOG = logging.getLogger(__name__) @@ -1567,11 +1570,15 @@ def sample_exists_with_different_sex( self, customer_internal_id: str, subject_id: str, - sex: str, + sex: SexEnum, ) -> bool: samples: list[Sample] = self.get_samples_by_customer_and_subject_id( customer_internal_id=customer_internal_id, subject_id=subject_id, ) - existing_sex: set[str] = {sample.sex for sample in samples} - return bool(samples) and sex not in existing_sex + for sample in samples: + if sample.sex == SexEnum.unknown: + continue + if sample.sex != sex: + return True + return False diff --git a/tests/services/order_validation_service/workflows/__init__.py b/tests/services/order_validation_service/workflows/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/services/order_validation_service/workflows/balsamic/__init__.py b/tests/services/order_validation_service/workflows/balsamic/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/services/order_validation_service/workflows/balsamic/test_case_sample_rules.py b/tests/services/order_validation_service/workflows/balsamic/test_case_sample_rules.py new file mode 100644 index 0000000000..e69de29bb2 From 47cfd555f86c299c9691b3bd8aa38abbf6f65531 Mon Sep 17 00:00:00 2001 From: seallard Date: Fri, 30 Aug 2024 11:13:59 +0200 Subject: [PATCH 7/7] Add test --- .../balsamic/rules/case_sample/rules.py | 8 +-- .../workflows/balsamic/conftest.py | 55 +++++++++++++++++++ .../balsamic/test_case_sample_rules.py | 32 +++++++++++ 3 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 tests/services/order_validation_service/workflows/balsamic/conftest.py diff --git a/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py index b08cfa2a60..58cf5f0561 100644 --- a/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py +++ b/cg/services/order_validation_service/workflows/balsamic/rules/case_sample/rules.py @@ -1,4 +1,4 @@ -from cg.services.order_validation_service.errors.case_sample_errors import SubjectIdGenderError +from cg.services.order_validation_service.errors.case_sample_errors import SexSubjectIdError from cg.services.order_validation_service.workflows.balsamic.models.order import BalsamicOrder from cg.services.order_validation_service.workflows.balsamic.rules.case_sample.utils import ( has_sex_and_subject, @@ -9,8 +9,8 @@ def validate_subject_sex_consistency( order: BalsamicOrder, store: Store, -) -> list[SubjectIdGenderError]: - errors: list[SubjectIdGenderError] = [] +) -> list[SexSubjectIdError]: + errors: list[SexSubjectIdError] = [] for case_index, case in order.enumerated_new_cases: for sample_index, sample in case.enumerated_new_samples: @@ -22,7 +22,7 @@ def validate_subject_sex_consistency( subject_id=sample.subject_id, sex=sample.sex, ): - error = SubjectIdGenderError( + error = SexSubjectIdError( case_index=case_index, sample_index=sample_index, ) diff --git a/tests/services/order_validation_service/workflows/balsamic/conftest.py b/tests/services/order_validation_service/workflows/balsamic/conftest.py new file mode 100644 index 0000000000..d771e37d07 --- /dev/null +++ b/tests/services/order_validation_service/workflows/balsamic/conftest.py @@ -0,0 +1,55 @@ +import pytest + +from cg.constants.constants import GenomeVersion, Workflow +from cg.models.orders.sample_base import ContainerEnum, ControlEnum, SexEnum, StatusEnum +from cg.services.order_validation_service.constants import MINIMUM_VOLUME +from cg.services.order_validation_service.workflows.balsamic.constants import BalsamicDeliveryType +from cg.services.order_validation_service.workflows.balsamic.models.case import BalsamicCase +from cg.services.order_validation_service.workflows.balsamic.models.order import BalsamicOrder +from cg.services.order_validation_service.workflows.balsamic.models.sample import BalsamicSample + + +def create_sample(id: int) -> BalsamicSample: + return BalsamicSample( + name=f"name{id}", + application="PANKTTR020", + container=ContainerEnum.plate, + container_name="ContainerName", + control=ControlEnum.not_control, + require_qc_ok=True, + reference_genome=GenomeVersion.HG19, + sex=SexEnum.female, + source="source", + status=StatusEnum.affected, + subject_id=f"subject{id}", + well_position=f"A:{id}", + volume=MINIMUM_VOLUME, + is_tumour=False, + ) + + +def create_case(samples: list[BalsamicSample]) -> BalsamicCase: + return BalsamicCase( + name="name", + samples=samples, + ) + + +def create_order(cases: list[BalsamicCase]) -> BalsamicOrder: + return BalsamicOrder( + connect_to_ticket=True, + delivery_type=BalsamicDeliveryType.FASTQ_ANALYSIS, + name="order_name", + ticket_number="#12345", + workflow=Workflow.BALSAMIC, + user_id=1, + customer="cust000", + cases=cases, + ) + + +@pytest.fixture +def valid_order() -> BalsamicOrder: + sample = create_sample(1) + case = create_case([sample]) + return create_order([case]) diff --git a/tests/services/order_validation_service/workflows/balsamic/test_case_sample_rules.py b/tests/services/order_validation_service/workflows/balsamic/test_case_sample_rules.py index e69de29bb2..4cb4e8e89f 100644 --- a/tests/services/order_validation_service/workflows/balsamic/test_case_sample_rules.py +++ b/tests/services/order_validation_service/workflows/balsamic/test_case_sample_rules.py @@ -0,0 +1,32 @@ +from cg.models.orders.sample_base import SexEnum +from cg.services.order_validation_service.errors.case_sample_errors import SexSubjectIdError +from cg.services.order_validation_service.workflows.balsamic.models.order import BalsamicOrder +from cg.services.order_validation_service.workflows.balsamic.rules.case_sample.rules import ( + validate_subject_sex_consistency, +) +from cg.store.models import Sample +from cg.store.store import Store + + +def test_validate_sex_subject_id_clash(valid_order: BalsamicOrder, sample_store: Store): + # GIVEN an existing sample + sample = sample_store.session.query(Sample).first() + + # GIVEN an order and sample with the same customer and subject id + valid_order.customer = sample.customer.internal_id + valid_order.cases[0].samples[0].subject_id = "subject" + sample.subject_id = "subject" + + # GIVEN a sample in the order that has a different sex + valid_order.cases[0].samples[0].sex = SexEnum.female + sample.sex = SexEnum.male + + # WHEN validating the order + errors = validate_subject_sex_consistency( + order=valid_order, + store=sample_store, + ) + + # THEN an error should be given for the clash + assert errors + assert isinstance(errors[0], SexSubjectIdError)