Merge pull request #355 from VariantEffect/bugfix/bencap/329/editable-dataset-metadata

bencap · web-flow · commit e4ec609291a7 · 2024-11-18T14:33:27.000-08:00
Expand Editable Score Set Metadata
diff --git a/pyproject.toml b/pyproject.toml
@@ -99,6 +99,7 @@ mypy_path = "mypy_stubs"
 addopts = "-v -rP --import-mode=importlib --disable-socket --allow-hosts localhost,::1,127.0.0.1"
 asyncio_mode = 'strict'
 testpaths = "tests/"
+pythonpath = "."
 norecursedirs = "tests/helpers/"
 # Uncomment the following lines to include application log output in Pytest logs.
 # log_cli = true
diff --git a/src/mavedb/routers/score_sets.py b/src/mavedb/routers/score_sets.py
@@ -651,7 +651,9 @@ async def upload_score_set_variant_data(
     return item
 
 
-@router.put("/score-sets/{urn}", response_model=score_set.ScoreSet, responses={422: {}})
+@router.put(
+    "/score-sets/{urn}", response_model=score_set.ScoreSet, responses={422: {}}, response_model_exclude_none=True
+)
 async def update_score_set(
     *,
     urn: str,
@@ -673,68 +675,65 @@ async def update_score_set(
 
     assert_permission(user_data, item, Action.UPDATE)
 
-    # Editing unpublished score set
-    if item.private is True:
-        license_ = None
+    for var, value in vars(item_update).items():
+        if var not in [
+            "contributors",
+            "score_ranges",
+            "doi_identifiers",
+            "experiment_urn",
+            "license_id",
+            "secondary_publication_identifiers",
+            "primary_publication_identifiers",
+            "target_genes",
+        ]:
+            setattr(item, var, value) if value else None
+
+    if item_update.license_id is not None:
+        save_to_logging_context({"license": item_update.license_id})
+        license_ = db.query(License).filter(License.id == item_update.license_id).one_or_none()
+
+        if not license_:
+            logger.info(
+                msg="Failed to update score set; The requested license does not exist.", extra=logging_context()
+            )
+            raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Unknown license")
 
-        if item_update.license_id is not None:
-            save_to_logging_context({"license": item_update.license_id})
-            license_ = db.query(License).filter(License.id == item_update.license_id).one_or_none()
+        item.license = license_
 
-            if not license_:
-                logger.info(
-                    msg="Failed to update score set; The requested license does not exist.", extra=logging_context()
-                )
-                raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Unknown license")
+    item.doi_identifiers = [
+        await find_or_create_doi_identifier(db, identifier.identifier)
+        for identifier in item_update.doi_identifiers or []
+    ]
+    primary_publication_identifiers = [
+        await find_or_create_publication_identifier(db, identifier.identifier, identifier.db_name)
+        for identifier in item_update.primary_publication_identifiers or []
+    ]
+    publication_identifiers = [
+        await find_or_create_publication_identifier(db, identifier.identifier, identifier.db_name)
+        for identifier in item_update.secondary_publication_identifiers or []
+    ] + primary_publication_identifiers
 
-            item.license = license_
+    # create a temporary `primary` attribute on each of our publications that indicates
+    # to our association proxy whether it is a primary publication or not
+    primary_identifiers = [p.identifier for p in primary_publication_identifiers]
+    for publication in publication_identifiers:
+        setattr(publication, "primary", publication.identifier in primary_identifiers)
 
-        for var, value in vars(item_update).items():
-            if var not in [
-                "contributors",
-                "score_ranges",
-                "doi_identifiers",
-                "experiment_urn",
-                "license_id",
-                "secondary_publication_identifiers",
-                "primary_publication_identifiers",
-                "target_genes",
-            ]:
-                setattr(item, var, value) if value else None
-
-        try:
-            item.contributors = [
-                await find_or_create_contributor(db, contributor.orcid_id)
-                for contributor in item_update.contributors or []
-            ]
-        except NonexistentOrcidUserError as e:
-            logger.error(msg="Could not find ORCID user with the provided user ID.", extra=logging_context())
-            raise pydantic.ValidationError(
-                [pydantic.error_wrappers.ErrorWrapper(ValidationError(str(e)), loc="contributors")],
-                model=score_set.ScoreSetUpdate,
-            )
+    item.publication_identifiers = publication_identifiers
 
-        item.doi_identifiers = [
-            await find_or_create_doi_identifier(db, identifier.identifier)
-            for identifier in item_update.doi_identifiers or []
-        ]
-        primary_publication_identifiers = [
-            await find_or_create_publication_identifier(db, identifier.identifier, identifier.db_name)
-            for identifier in item_update.primary_publication_identifiers or []
+    try:
+        item.contributors = [
+            await find_or_create_contributor(db, contributor.orcid_id) for contributor in item_update.contributors or []
         ]
-        publication_identifiers = [
-            await find_or_create_publication_identifier(db, identifier.identifier, identifier.db_name)
-            for identifier in item_update.secondary_publication_identifiers or []
-        ] + primary_publication_identifiers
-
-        # create a temporary `primary` attribute on each of our publications that indicates
-        # to our association proxy whether it is a primary publication or not
-        primary_identifiers = [pub.identifier for pub in primary_publication_identifiers]
-        for publication in publication_identifiers:
-            setattr(publication, "primary", publication.identifier in primary_identifiers)
-
-        item.publication_identifiers = publication_identifiers
+    except NonexistentOrcidUserError as e:
+        logger.error(msg="Could not find ORCID user with the provided user ID.", extra=logging_context())
+        raise pydantic.ValidationError(
+            [pydantic.error_wrappers.ErrorWrapper(ValidationError(str(e)), loc="contributors")],
+            model=score_set.ScoreSetUpdate,
+        )
 
+    # Score set has not been published and attributes affecting scores may still be edited.
+    if item.private:
         if item_update.score_ranges:
             item.score_ranges = item_update.score_ranges.dict()
         else:
@@ -889,35 +888,8 @@ async def update_score_set(
             if job is not None:
                 save_to_logging_context({"worker_job_id": job.job_id})
             logger.info(msg="Enqueud variant creation job.", extra=logging_context())
-
-        for var, value in vars(item_update).items():
-            if var not in [
-                "score_ranges",
-                "contributors",
-                "doi_identifiers",
-                "experiment_urn",
-                "primary_publication_identifiers",
-                "secondary_publication_identifiers",
-                "target_genes",
-            ]:
-                setattr(item, var, value) if value else None
-
-    # Editing published score set
     else:
-        for var, value in vars(item_update).items():
-            if var in ["title", "method_text", "abstract_text", "short_description"]:
-                setattr(item, var, value) if value else None
-        try:
-            item.contributors = [
-                await find_or_create_contributor(db, contributor.orcid_id)
-                for contributor in item_update.contributors or []
-            ]
-        except NonexistentOrcidUserError as e:
-            logger.error(msg="Could not find ORCID user with the provided user ID.", extra=logging_context())
-            raise pydantic.ValidationError(
-                [pydantic.error_wrappers.ErrorWrapper(ValidationError(str(e)), loc="contributors")],
-                model=score_set.ScoreSetUpdate,
-            )
+        logger.debug(msg="Skipped score range and target gene update. Score set is published.", extra=logging_context())
 
     db.add(item)
     db.commit()
diff --git a/src/mavedb/view_models/score_set.py b/src/mavedb/view_models/score_set.py
@@ -254,7 +254,6 @@ def wild_type_score_in_normal_range(cls, field_value: Optional[ScoreRanges]):
             range_model.range for range_model in field_value.ranges if range_model.classification == "normal"
         ]
         for range in normal_ranges:
-            print(range)
             if field_value.wt_score >= inf_or_float(range[0], lower=True) and field_value.wt_score < inf_or_float(
                 range[1], lower=False
             ):
diff --git a/tests/helpers/constants.py b/tests/helpers/constants.py
@@ -13,6 +13,26 @@
 VALID_ACCESSION = "NM_001637.3"
 VALID_GENE = "BRCA1"
 
+SAVED_PUBMED_PUBLICATION = {
+    "identifier": "20711194",
+    "dbName": "PubMed",
+    "title": "None",
+    "authors": [],
+    "abstract": "test",
+    "doi": "test",
+    "publicationYear": 1999,
+    "publicationJournal": "test",
+    "url": "http://www.ncbi.nlm.nih.gov/pubmed/20711194",
+    "referenceHtml": ". None. test. 1999; (Unknown volume):(Unknown pages). test",
+    "id": 1,
+}
+
+SAVED_DOI_IDENTIFIER = {
+    "identifier": TEST_CROSSREF_IDENTIFIER,
+    "url": f"https://doi.org/{TEST_CROSSREF_IDENTIFIER}",
+    "id": 1,
+}
+
 TEST_USER = {
     "username": "0000-1111-2222-3333",
     "first_name": "First",
@@ -24,6 +44,18 @@
     "is_first_login": True,
 }
 
+CONTRIBUTOR = {
+    "orcid_id": TEST_USER["username"],
+    "given_name": TEST_USER["first_name"],
+    "family_name": TEST_USER["last_name"],
+}
+
+SAVED_CONTRIBUTOR = {
+    "orcidId": TEST_USER["username"],
+    "givenName": TEST_USER["first_name"],
+    "familyName": TEST_USER["last_name"],
+}
+
 TEST_USER_DECODED_JWT = {
     "sub": TEST_USER["username"],
     "given_name": TEST_USER["first_name"],
@@ -41,6 +73,18 @@
     "is_first_login": True,
 }
 
+EXTRA_CONTRIBUTOR = {
+    "orcid_id": EXTRA_USER["username"],
+    "given_name": EXTRA_USER["first_name"],
+    "family_name": EXTRA_USER["last_name"],
+}
+
+SAVED_EXTRA_CONTRIBUTOR = {
+    "orcidId": EXTRA_USER["username"],
+    "givenName": EXTRA_USER["first_name"],
+    "familyName": EXTRA_USER["last_name"],
+}
+
 EXTRA_USER_DECODED_JWT = {
     "sub": EXTRA_USER["username"],
     "given_name": EXTRA_USER["first_name"],
@@ -305,6 +349,31 @@
     "version": "1.0",
 }
 
+SAVED_SHORT_TEST_LICENSE = {
+    "id": TEST_LICENSE["id"],
+    "shortName": TEST_LICENSE["short_name"],
+    "longName": TEST_LICENSE["long_name"],
+    "link": TEST_LICENSE["link"],
+    "version": TEST_LICENSE["version"],
+}
+
+EXTRA_LICENSE = {
+    "id": 2,
+    "short_name": "Extra",
+    "long_name": "License",
+    "text": "Don't be tooooo evil.",
+    "link": "localhost",
+    "version": "1.0",
+}
+
+SAVED_SHORT_EXTRA_LICENSE = {
+    "id": EXTRA_LICENSE["id"],
+    "shortName": EXTRA_LICENSE["short_name"],
+    "longName": EXTRA_LICENSE["long_name"],
+    "link": EXTRA_LICENSE["link"],
+    "version": EXTRA_LICENSE["version"],
+}
+
 TEST_SEQ_SCORESET = {
     "title": "Test Score Set Title",
     "short_description": "Test score set",
@@ -485,6 +554,7 @@
         {
             "recordType": "TargetGene",
             "name": "TEST2",
+            "id": 2,
             "category": "protein_coding",
             "externalIdentifiers": [],
             "targetAccession": {
@@ -548,3 +618,20 @@
     "dcd_mapping_version": "pytest.0.0",
     "mapped_date_utc": datetime.isoformat(datetime.now()),
 }
+
+
+TEST_SCORESET_RANGE = {
+    "wt_score": 1.0,
+    "ranges": [
+        {"label": "test1", "classification": "normal", "range": (0, 2.0)},
+        {"label": "test2", "classification": "abnormal", "range": (-2.0, 0)},
+    ],
+}
+
+TEST_SAVED_SCORESET_RANGE = {
+    "wtScore": 1.0,
+    "ranges": [
+        {"label": "test1", "classification": "normal", "range": [0.0, 2.0]},
+        {"label": "test2", "classification": "abnormal", "range": [-2.0, 0.0]},
+    ],
+}
diff --git a/tests/helpers/util.py b/tests/helpers/util.py
@@ -5,6 +5,7 @@
 import jsonschema
 from arq import ArqRedis
 from sqlalchemy import select
+from sqlalchemy.exc import NoResultFound
 
 from mavedb.lib.score_sets import columns_for_dataset, create_variants, create_variants_data, csv_data_to_df
 from mavedb.lib.validation.dataframe import validate_and_standardize_dataframe_pair
@@ -27,8 +28,13 @@ def add_contributor(db, urn, model, orcid_id: str, given_name: str, family_name:
     """Without making an API call, add a new contributor to the record (experiment or score set) with given urn and model."""
     item = db.query(model).filter(model.urn == urn).one_or_none()
     assert item is not None
-    contributor = Contributor(orcid_id=orcid_id, given_name=given_name, family_name=family_name)
-    db.add(contributor)
+
+    try:
+        contributor = db.execute(select(Contributor).where(Contributor.orcid_id == orcid_id)).one()
+    except NoResultFound:
+        contributor = Contributor(orcid_id=orcid_id, given_name=given_name, family_name=family_name)
+        db.add(contributor)
+
     item.contributors = [contributor]
     db.add(item)
     db.commit()
@@ -214,3 +220,16 @@ def mark_user_inactive(session, username):
 
 async def awaitable_exception():
     return Exception()
+
+
+def update_expected_response_for_created_resources(expected_response, created_experiment, created_score_set):
+    expected_response.update({"urn": created_score_set["urn"]})
+    expected_response["experiment"].update(
+        {
+            "urn": created_experiment["urn"],
+            "experimentSetUrn": created_experiment["experimentSetUrn"],
+            "scoreSetUrns": [created_score_set["urn"]],
+        }
+    )
+
+    return expected_response
diff --git a/tests/routers/conftest.py b/tests/routers/conftest.py
@@ -6,6 +6,7 @@
 import pytest
 
 from mavedb.models.controlled_keyword import ControlledKeyword
+from mavedb.models.contributor import Contributor
 from mavedb.models.enums.user_role import UserRole
 from mavedb.models.license import License
 from mavedb.models.role import Role
@@ -14,9 +15,11 @@
 from tests.helpers.constants import (
     ADMIN_USER,
     EXTRA_USER,
+    EXTRA_CONTRIBUTOR,
     TEST_CDOT_TRANSCRIPT,
     TEST_DB_KEYWORDS,
     TEST_LICENSE,
+    EXTRA_LICENSE,
     TEST_TAXONOMY,
     TEST_USER,
 )
@@ -41,6 +44,8 @@ def setup_router_db(session):
     db.add(User(**ADMIN_USER, role_objs=[Role(name=UserRole.admin)]))
     db.add(Taxonomy(**TEST_TAXONOMY))
     db.add(License(**TEST_LICENSE))
+    db.add(License(**EXTRA_LICENSE))
+    db.add(Contributor(**EXTRA_CONTRIBUTOR))
     db.bulk_save_objects([ControlledKeyword(**keyword_obj) for keyword_obj in TEST_DB_KEYWORDS])
     db.commit()
 
diff --git a/tests/routers/test_score_set.py b/tests/routers/test_score_set.py

Original file line number	Diff line number	Diff line change
`@@ -254,7 +254,6 @@ def wild_type_score_in_normal_range(cls, field_value: Optional[ScoreRanges]):`
`254`	`254`	`range_model.range for range_model in field_value.ranges if range_model.classification == "normal"`
`255`	`255`	`]`
`256`	`256`	`for range in normal_ranges:`
`257`		`- print(range)`
`258`	`257`	`if field_value.wt_score >= inf_or_float(range[0], lower=True) and field_value.wt_score < inf_or_float(`
`259`	`258`	`range[1], lower=False`
`260`	`259`	`):`