diff --git a/CHANGELOG.md b/CHANGELOG.md index 64620d7..ec5e7cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,30 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [Unreleased] + +## [1.5.1] - 2024-01-25 + +### Changed + +* The 'Unknown' UI tab no longer displays the wells that have any + sequencing QC state associated with it. This feature was requested + by the users. + +* To increase code readability, renamed qc_state_for_product_exists + method in lang_qc::db::helper::qc to product_has_qc_state. + +* Added method products_have_qc_state to lang_qc::db::helper::qc, + used it to optimise (reduce the number of database queries) some + of the existing back-end code. + +### Fixed + +* The client side JavaScript dependency, element-plus, is pinned + to version 2.4.4. In mid-January 2024 this was the highest version + that worked with our code. The version expression we had "^2.3.7" + allowed for fetching the latest available version of this library. + ## [1.5.0] ### Added diff --git a/frontend/package.json b/frontend/package.json index dc5b459..ef1a22c 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "npg-longue-vue", - "version": "1.5.0", + "version": "1.5.1", "description": "UI for LangQC", "author": "Kieron Taylor ", "license": "GPL-3.0-or-later", @@ -19,7 +19,7 @@ }, "dependencies": { "@element-plus/icons-vue": "^2.0.10", - "element-plus": "^2.3.7", + "element-plus": "2.4.4", "lodash": "^4.17.21", "pinia": "^2.0.23", "vue": "^3.2.37", diff --git a/lang_qc/__init__.py b/lang_qc/__init__.py index 5b60188..0f228f2 100644 --- a/lang_qc/__init__.py +++ b/lang_qc/__init__.py @@ -1 +1 @@ -__version__ = "1.5.0" +__version__ = "1.5.1" diff --git a/lang_qc/db/helper/qc.py b/lang_qc/db/helper/qc.py index e62adc7..c470570 100644 --- a/lang_qc/db/helper/qc.py +++ b/lang_qc/db/helper/qc.py @@ -40,6 +40,7 @@ APPLICATION_NAME = "LangQC" DEFAULT_QC_TYPE = "sequencing" +SEQUENCING_QC_TYPE = DEFAULT_QC_TYPE CLAIMED_QC_STATE = "Claimed" DEFAULT_FINALITY = False ONLY_PRELIM_STATES = (CLAIMED_QC_STATE, "On hold") @@ -78,7 +79,7 @@ def get_qc_states_by_id_product_list( If only sequencing type QC states are required, an optional argument, sequencing_outcomes_only, should be set to True. - If this case it is guaranteed that the list of QcState objects + In this case it is guaranteed that the list of QcState objects has only one member. Product IDs for which no QC states are found are omitted @@ -100,7 +101,7 @@ def get_qc_states_by_id_product_list( ) -def qc_state_for_product_exists( +def product_has_qc_state( session: Session, id_product: ChecksumSHA256, qc_type: str = None ) -> bool: """ @@ -132,6 +133,36 @@ def qc_state_for_product_exists( return bool(session.execute(query).scalar_one()) +def products_have_qc_state( + session: Session, ids: list[ChecksumSHA256], sequencing_outcomes_only: bool = False +) -> set[ChecksumSHA256]: + """ + Given a list of product IDs, returns a potentially empty subset of this list + as a Set object. Each product, identified by the product ID in the returned + set, has some QC state associated with it. If `sequencing_outcome_only` + boolean flag is true, the product must have sequencing QC state associated + with it; `library` QC state is excluded. + + Arguments: + `session` - `sqlalchemy.orm.Session`, a connection for LangQC database. + `ids` - a list of string product IDs. + `sequencing_outcome_only` - a boolean flag indicating whether the caller + is interested in `sequencing` QC states only + """ + + query = select(SeqProduct.id_product).where(SeqProduct.id_product.in_(ids)) + if sequencing_outcomes_only is True: + query = query.join(SeqProduct.qc_state).where( + QcStateDb.qc_type == _get_qc_type_row(session, SEQUENCING_QC_TYPE) + ) + + # We asked to retrieve data for one column only. The return value for + # each row is an array with a single value from this column. + product_ids = [row[0] for row in session.execute(query).all()] + + return set(product_ids) + + def get_seq_product(session: Session, id_product: ChecksumSHA256) -> SeqProduct: """ Given a product ID, returns a SeqProduct row for a database entity with @@ -149,7 +180,7 @@ def get_seq_product(session: Session, id_product: ChecksumSHA256) -> SeqProduct: def get_qc_state_for_product( - session: Session, id_product: ChecksumSHA256, qc_type: str = "sequencing" + session: Session, id_product: ChecksumSHA256, qc_type: str = DEFAULT_QC_TYPE ) -> QcStateDb: """ Returns a QcState database row associated with the product with the diff --git a/lang_qc/db/helper/wells.py b/lang_qc/db/helper/wells.py index 9de7f0f..100f407 100644 --- a/lang_qc/db/helper/wells.py +++ b/lang_qc/db/helper/wells.py @@ -29,7 +29,7 @@ from lang_qc.db.helper.qc import ( get_qc_states_by_id_product_list, - qc_state_for_product_exists, + products_have_qc_state, ) from lang_qc.db.mlwh_schema import PacBioRunWellMetrics from lang_qc.db.qc_schema import QcState, QcStateDict, QcType @@ -365,15 +365,11 @@ def _upcoming_wells(self): ) ) - wells = [] - for w in self.session.execute(query).scalars().all(): - if ( - qc_state_for_product_exists( - session=self.qcdb_session, id_product=w.id_pac_bio_product - ) - is False - ): - wells.append(w) + wells = self.session.execute(query).scalars().all() + ids_with_qc_state = products_have_qc_state( + session=self.qcdb_session, ids=[w.id_pac_bio_product for w in wells] + ) + wells = [w for w in wells if w.id_pac_bio_product not in ids_with_qc_state] self.total_number_of_items = len(wells) # Save the number of retrieved wells. @@ -420,10 +416,21 @@ def _aborted_and_unknown_wells(self, qc_flow_status: QcFlowStatusEnum): .all() ) + qc_state_applicable = True + if qc_flow_status == QcFlowStatusEnum.UNKNOWN: + # Remove the wells that the QC team has dealt with. + ids_with_qc_state = products_have_qc_state( + session=self.qcdb_session, + ids=[w.id_pac_bio_product for w in wells], + sequencing_outcomes_only=True, + ) + wells = [w for w in wells if w.id_pac_bio_product not in ids_with_qc_state] + qc_state_applicable = False + # Save the number of retrieved rows. self.total_number_of_items = len(wells) - return self._well_models(self.slice_data(wells), True) + return self._well_models(self.slice_data(wells), qc_state_applicable) def _well_models( self, @@ -454,7 +461,7 @@ def _well_models( sequencing_outcomes_only=True, ).get(id_product) # A well can have only one or zero current sequencing outcomes. - if qced_products is not None: + if qced_products is not None and (len(qced_products) > 0): attrs["qc_state"] = qced_products[0] pb_well = PacBioWell.model_validate(attrs) pb_well.copy_run_tracking_info(db_well) diff --git a/lang_qc/endpoints/pacbio_well.py b/lang_qc/endpoints/pacbio_well.py index b6bfb08..dca152e 100644 --- a/lang_qc/endpoints/pacbio_well.py +++ b/lang_qc/endpoints/pacbio_well.py @@ -29,7 +29,7 @@ from lang_qc.db.helper.qc import ( assign_qc_state_to_product, claim_qc_for_product, - qc_state_for_product_exists, + product_has_qc_state, ) from lang_qc.db.helper.well import well_seq_product_find_or_create from lang_qc.db.helper.wells import PacBioPagedWellsFactory, WellWh @@ -216,7 +216,7 @@ def claim_qc( mlwh_well = _find_well_product_or_error(id_product, mlwhdb_session) # Checking for any type of QC state - if qc_state_for_product_exists(qcdb_session, id_product): + if product_has_qc_state(qcdb_session, id_product): raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail=f"Well for product {id_product} has QC state assigned", @@ -261,10 +261,7 @@ def assign_qc_state( mlwh_well = _find_well_product_or_error(id_product, mlwhdb_session) - if ( - qc_state_for_product_exists(qcdb_session, id_product, request_body.qc_type) - is False - ): + if product_has_qc_state(qcdb_session, id_product, request_body.qc_type) is False: raise HTTPException( status_code=status.HTTP_409_CONFLICT, detail="QC state of an unclaimed well cannot be updated", diff --git a/pyproject.toml b/pyproject.toml index cbb4c76..bc0d22b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ name = "npg_langqc" packages = [ { include = "lang_qc" }, ] -version = "1.5.0" +version = "1.5.1" description = "FastAPI application for Long Read QC" authors = ["Adam Blanchet", "Marina Gourtovaia ", "Kieron Taylor "] license = "GPL-3.0-or-later" diff --git a/tests/test_pb_wells_factory.py b/tests/test_pb_wells_factory.py index 25a73e7..b4638d3 100644 --- a/tests/test_pb_wells_factory.py +++ b/tests/test_pb_wells_factory.py @@ -1,8 +1,9 @@ import pytest from npg_id_generation.pac_bio import PacBioEntity +from sqlalchemy import select from lang_qc.db.helper.wells import PacBioPagedWellsFactory, RunNotFoundError -from lang_qc.db.qc_schema import QcState +from lang_qc.db.qc_schema import QcState, QcType, SeqProduct from lang_qc.models.pacbio.well import PacBioPagedWells, PacBioWell from lang_qc.models.qc_flow_status import QcFlowStatusEnum from lang_qc.models.qc_state import QcState as QcStateModel @@ -175,6 +176,7 @@ def test_paged_retrieval_for_statuses( QcFlowStatusEnum.ON_HOLD.name: 2, QcFlowStatusEnum.QC_COMPLETE.name: 4, QcFlowStatusEnum.UPCOMING.name: 4, + QcFlowStatusEnum.UNKNOWN.name: 2, } for status in [ @@ -182,6 +184,7 @@ def test_paged_retrieval_for_statuses( QcFlowStatusEnum.ON_HOLD, QcFlowStatusEnum.QC_COMPLETE, QcFlowStatusEnum.UPCOMING, + QcFlowStatusEnum.UNKNOWN, ]: factory = PacBioPagedWellsFactory( @@ -425,3 +428,74 @@ def test_known_run_names_input( qc_state_objs = [well.qc_state for well in wells] assert qc_state_objs[0] is None assert qc_state_objs[1] is None + + +def test_retrieval_for_unknown_status( + qcdb_test_session, mlwhdb_test_session, load_data4well_retrieval +): + # SUBTRACTION_RUN_15:A1 + # TRACTION_RUN_14:B1 + + factory = PacBioPagedWellsFactory( + qcdb_session=qcdb_test_session, + mlwh_session=mlwhdb_test_session, + page_size=10, + page_number=1, + ) + paged_wells = factory.create_for_qc_status(QcFlowStatusEnum.UNKNOWN) + assert ( + paged_wells.total_number_of_items == 2 + ), "two wells with unknown status, no qc state" + + # Create seq QC states for these wells and test that they are gone + # from the Unknown status. + + id = PacBioEntity(run_name="TRACTION_RUN_14", well_label="B1").hash_product_id() + + seq_product = SeqProduct(id_product=id, id_seq_platform=1, has_seq_data=1) + qcdb_test_session.add(seq_product) + qcdb_test_session.commit() + + qc_type_row = ( + qcdb_test_session.execute(select(QcType).where(QcType.qc_type == "library")) + .scalars() + .one() + ) + qc_state = QcState( + created_by="LangQC", + is_preliminary=1, + id_qc_state_dict=2, + qc_type=qc_type_row, + seq_product=seq_product, + id_user=1, + ) + qcdb_test_session.add(qc_state) + qcdb_test_session.commit() + + paged_wells = factory.create_for_qc_status(QcFlowStatusEnum.UNKNOWN) + assert ( + paged_wells.total_number_of_items == 2 + ), """assigning library qc state to a well does not affect + its selection for the unknown status""" + + qc_type_row = ( + qcdb_test_session.execute(select(QcType).where(QcType.qc_type == "sequencing")) + .scalars() + .one() + ) + qc_state = QcState( + created_by="LangQC", + is_preliminary=1, + id_qc_state_dict=3, + qc_type=qc_type_row, + seq_product=seq_product, + id_user=1, + ) + qcdb_test_session.add(qc_state) + qcdb_test_session.commit() + + paged_wells = factory.create_for_qc_status(QcFlowStatusEnum.UNKNOWN) + assert ( + paged_wells.total_number_of_items == 1 + ), """assigning sequencing qc state to a well removes it + from the selection for the unknown status""" diff --git a/tests/test_qc_state_retrieval.py b/tests/test_qc_state_retrieval.py index 897d325..7babe28 100644 --- a/tests/test_qc_state_retrieval.py +++ b/tests/test_qc_state_retrieval.py @@ -3,18 +3,23 @@ from lang_qc.db.helper.qc import ( get_qc_state_for_product, get_qc_states_by_id_product_list, + product_has_qc_state, + products_have_qc_state, qc_state_dict, - qc_state_for_product_exists, ) from tests.fixtures.well_data import load_data4well_retrieval, load_dicts_and_users +MISSING_CHECKSUM = "A" * 64 # "TRACTION_RUN_1", "D1", "On hold", Final FIRST_GOOD_CHECKSUM = "6657a34aa6159d7e2426f4732a84c51fa2d9186a4578e61ec21de4cb028fc800" # "TRACTION_RUN_2", "B1", "Failed, Instrument", Preliminary SECOND_GOOD_CHECKSUM = ( "e47765a207c810c2c281d5847e18c3015f3753b18bd92e8a2bea1219ba3127ea" ) -MISSING_CHECKSUM = "A" * 64 +# "TRACTION_RUN_16", plate 1 +NO_LIB_QC_CHECKSUM = "48056b888e6890a2c2d6020018349167feeb729322b1caff97a28a4a8116d98d" +# "TRACTION_RUN_16", plate 2 +NO_SEQ_QC_CHECKSUM = "dc77c4a7f34d84afbb895fcaee72fc8bead9dac20e8d3a9614091d9dd4519acd" two_good_ids_list = [FIRST_GOOD_CHECKSUM, SECOND_GOOD_CHECKSUM] @@ -63,36 +68,61 @@ def test_bulk_retrieval(qcdb_test_session, load_data4well_retrieval): def test_product_existence(qcdb_test_session, load_data4well_retrieval): - assert qc_state_for_product_exists(qcdb_test_session, MISSING_CHECKSUM) is False + assert product_has_qc_state(qcdb_test_session, MISSING_CHECKSUM) is False for id in two_good_ids_list: - assert qc_state_for_product_exists(qcdb_test_session, id) is True - assert qc_state_for_product_exists(qcdb_test_session, id, "sequencing") is True - assert qc_state_for_product_exists(qcdb_test_session, id, "library") is True + assert product_has_qc_state(qcdb_test_session, id) is True + assert product_has_qc_state(qcdb_test_session, id, "sequencing") is True + assert product_has_qc_state(qcdb_test_session, id, "library") is True - # "TRACTION_RUN_16", plate 1 - id_no_lib_qc = "48056b888e6890a2c2d6020018349167feeb729322b1caff97a28a4a8116d98d" assert ( - qc_state_for_product_exists(qcdb_test_session, id_no_lib_qc, "sequencing") + product_has_qc_state(qcdb_test_session, NO_LIB_QC_CHECKSUM, "sequencing") is True ) assert ( - qc_state_for_product_exists(qcdb_test_session, id_no_lib_qc, "library") is False + product_has_qc_state(qcdb_test_session, NO_LIB_QC_CHECKSUM, "library") is False ) - assert qc_state_for_product_exists(qcdb_test_session, id_no_lib_qc) is True + assert product_has_qc_state(qcdb_test_session, NO_LIB_QC_CHECKSUM) is True - # "TRACTION_RUN_16", plate 2 - id_no_seq_qc = "dc77c4a7f34d84afbb895fcaee72fc8bead9dac20e8d3a9614091d9dd4519acd" assert ( - qc_state_for_product_exists(qcdb_test_session, id_no_seq_qc, "sequencing") + product_has_qc_state(qcdb_test_session, NO_SEQ_QC_CHECKSUM, "sequencing") is False ) assert ( - qc_state_for_product_exists(qcdb_test_session, id_no_seq_qc, "library") is True + product_has_qc_state(qcdb_test_session, NO_SEQ_QC_CHECKSUM, "library") is True ) - assert qc_state_for_product_exists(qcdb_test_session, id_no_seq_qc) is True + assert product_has_qc_state(qcdb_test_session, NO_SEQ_QC_CHECKSUM) is True with pytest.raises(Exception, match=r"QC type 'some_type' is invalid"): - qc_state_for_product_exists(qcdb_test_session, id_no_seq_qc, "some_type") + product_has_qc_state(qcdb_test_session, NO_SEQ_QC_CHECKSUM, "some_type") + + +def test_products_existence(qcdb_test_session, load_data4well_retrieval): + + ids = [ + FIRST_GOOD_CHECKSUM, + SECOND_GOOD_CHECKSUM, + NO_LIB_QC_CHECKSUM, + NO_SEQ_QC_CHECKSUM, + ] + + assert products_have_qc_state(qcdb_test_session, [MISSING_CHECKSUM]) == set() + assert products_have_qc_state(qcdb_test_session, [FIRST_GOOD_CHECKSUM]) == set( + [FIRST_GOOD_CHECKSUM] + ) + assert products_have_qc_state( + qcdb_test_session, [MISSING_CHECKSUM, FIRST_GOOD_CHECKSUM] + ) == set([FIRST_GOOD_CHECKSUM]) + assert products_have_qc_state(qcdb_test_session, ids) == set(ids) + + assert ( + products_have_qc_state( + qcdb_test_session, [MISSING_CHECKSUM, NO_SEQ_QC_CHECKSUM], True + ) + == set() + ) + assert products_have_qc_state(qcdb_test_session, ids, True) == set( + [FIRST_GOOD_CHECKSUM, SECOND_GOOD_CHECKSUM, NO_LIB_QC_CHECKSUM] + ) def test_product_qc_state_retrieval(qcdb_test_session, load_data4well_retrieval):