chore: curator API fix tests

backend/layers/business/business.py

@@ -190,7 +190,8 @@ def predicate(version: CollectionVersion):
                 or (filter.is_published is False and version.published_at is None)
             )
             owner = filter.owner is None or filter.owner == version.owner
-            return published and owner
+            curator = filter.curator_name is None or filter.curator_name == version.curator_name
+            return published and owner and curator
 
         for collection_version in iterable:
             if predicate(collection_version):
@@ -404,40 +405,40 @@ def get_dataset_status(self, dataset_version_id: DatasetVersionId) -> DatasetSta
     def update_dataset_version_status(
         self,
         dataset_version_id: DatasetVersionId,
-        status_key: Optional[DatasetStatusKey] = None,
-        new_dataset_status: Optional[DatasetStatusGeneric] = None,
+        status_key: DatasetStatusKey,
+        new_dataset_status: DatasetStatusGeneric,
         validation_message: Optional[str] = None,
     ) -> None:
         """
         TODO: split into two method, one for updating validation_message, and the other statuses.
         Updates the status of a dataset version.
         status_key can be one of: [upload, validation, cxg, rds, h5ad, processing]
         """
-        if all([status_key, new_dataset_status]):
-            if status_key == DatasetStatusKey.UPLOAD and isinstance(new_dataset_status, DatasetUploadStatus):
-                self.database_provider.update_dataset_upload_status(dataset_version_id, new_dataset_status)
-            elif status_key == DatasetStatusKey.PROCESSING and isinstance(new_dataset_status, DatasetProcessingStatus):
-                self.database_provider.update_dataset_processing_status(dataset_version_id, new_dataset_status)
-            elif status_key == DatasetStatusKey.VALIDATION and isinstance(new_dataset_status, DatasetValidationStatus):
-                self.database_provider.update_dataset_validation_status(dataset_version_id, new_dataset_status)
-            elif status_key == DatasetStatusKey.CXG and isinstance(new_dataset_status, DatasetConversionStatus):
-                self.database_provider.update_dataset_conversion_status(
-                    dataset_version_id, "cxg_status", new_dataset_status
-                )
-            elif status_key == DatasetStatusKey.RDS and isinstance(new_dataset_status, DatasetConversionStatus):
-                self.database_provider.update_dataset_conversion_status(
-                    dataset_version_id, "rds_status", new_dataset_status
-                )
-            elif status_key == DatasetStatusKey.H5AD and isinstance(new_dataset_status, DatasetConversionStatus):
-                self.database_provider.update_dataset_conversion_status(
-                    dataset_version_id, "h5ad_status", new_dataset_status
-                )
-            else:
-                raise DatasetUpdateException(
-                    f"Invalid status update for dataset {dataset_version_id}: cannot set {status_key} to "
-                    f"{new_dataset_status}"
-                )
-        elif validation_message is not None:
+        if status_key == DatasetStatusKey.UPLOAD and isinstance(new_dataset_status, DatasetUploadStatus):
+            self.database_provider.update_dataset_upload_status(dataset_version_id, new_dataset_status)
+        elif status_key == DatasetStatusKey.PROCESSING and isinstance(new_dataset_status, DatasetProcessingStatus):
+            self.database_provider.update_dataset_processing_status(dataset_version_id, new_dataset_status)
+        elif status_key == DatasetStatusKey.VALIDATION and isinstance(new_dataset_status, DatasetValidationStatus):
+            self.database_provider.update_dataset_validation_status(dataset_version_id, new_dataset_status)
+        elif status_key == DatasetStatusKey.CXG and isinstance(new_dataset_status, DatasetConversionStatus):
+            self.database_provider.update_dataset_conversion_status(
+                dataset_version_id, "cxg_status", new_dataset_status
+            )
+        elif status_key == DatasetStatusKey.RDS and isinstance(new_dataset_status, DatasetConversionStatus):
+            self.database_provider.update_dataset_conversion_status(
+                dataset_version_id, "rds_status", new_dataset_status
+            )
+        elif status_key == DatasetStatusKey.H5AD and isinstance(new_dataset_status, DatasetConversionStatus):
+            self.database_provider.update_dataset_conversion_status(
+                dataset_version_id, "h5ad_status", new_dataset_status
+            )
+        else:
+            raise DatasetUpdateException(
+                f"Invalid status update for dataset {dataset_version_id}: cannot set {status_key} to "
+                f"{new_dataset_status}"
+            )
+
+        if validation_message is not None:
             self.database_provider.update_dataset_validation_message(dataset_version_id, validation_message)
 
     def add_dataset_artifact(

backend/layers/common/entities.py

@@ -1,5 +1,6 @@
-from pydantic import Field
-from pydantic.dataclasses import dataclass
+# from pydantic import Field
+# from pydantic.dataclasses import dataclass
+from dataclasses import dataclass
 from datetime import datetime
 from enum import Enum
 from typing import List, Optional
@@ -244,14 +245,13 @@ class CollectionVersionBase:
     published_at: Optional[datetime]
     created_at: datetime
     canonical_collection: CanonicalCollection
-    curator_name: Optional[str] = ""
 
 
 @dataclass
 class CollectionVersion(CollectionVersionBase):
-    datasets: List[DatasetVersionId] = Field(default_factory=list)
+    datasets: List[DatasetVersionId]
 
 
 @dataclass
 class CollectionVersionWithDatasets(CollectionVersionBase):
-    datasets: List[DatasetVersion] = Field(default_factory=list)
+    datasets: List[DatasetVersion]

backend/layers/persistence/persistence_mock.py

@@ -68,9 +68,7 @@ def create_canonical_collection(
     ) -> CollectionVersion:
         collection_id = CollectionId(self._generate_id())
         version_id = CollectionVersionId(self._generate_id())
-        canonical = CanonicalCollection(
-            id=collection_id, version_id=version_id, originally_published_at=None, tombstoned=False
-        )
+        canonical = CanonicalCollection(collection_id, None, None, False)
         version = CollectionVersion(
             collection_id=collection_id,
             version_id=version_id,
@@ -84,7 +82,6 @@ def create_canonical_collection(
             datasets=[],
         )
         self.collections_versions[version_id.id] = version
-        self.collections[collection_id.id] = canonical
         # Don't set mappings here - those will be set when publishing the collection!
         return copy.deepcopy(version)
 

backend/portal/api/curation/v1/curation/collections/actions.py

@@ -47,7 +47,6 @@ def get(visibility: str, token_info: dict, curator: str = None):
 
 
 def post(body: dict, user: str):
-
     # Extract DOI into link
     errors = []
     if doi_url := body.get("doi"):
@@ -61,7 +60,7 @@ def post(body: dict, user: str):
     metadata = CollectionMetadata(body["name"], body["description"], body["contact_name"], body["contact_email"], links)
 
     try:
-        version = get_business_logic().create_collection(user, metadata)
+        version = get_business_logic().create_collection(user, body.get("curator_name", ""), metadata)
     except CollectionCreationException as ex:
         errors.extend(ex.errors)
     if errors:

backend/portal/api/curation/v1/curation/collections/collection_id/s3_upload_credentials.py

@@ -20,6 +20,7 @@
 def get(collection_id: str, token_info: dict):
     config = CorporaConfig()
     user_info = UserInfo(token_info)
+    # TODO: Since this method only works on private collections, I think we should just accept the version_id here
     collection_version = get_infered_collection_version_else_forbidden(collection_id)
     is_owner_or_allowed_else_forbidden(collection_version, user_info)
     if collection_version.published_at:

backend/portal/api/curation/v1/curation/collections/common.py

@@ -84,15 +84,17 @@ def reshape_for_curation_api(
 
     # build response
     doi, links = extract_doi_from_links(collection_version.metadata.links)
-    revised_at = business_logic.get_published_collection_version(
-        collection_version.canonical_collection.id
-    ).published_at
+    published_version = business_logic.get_published_collection_version(collection_version.canonical_collection.id)
+    if published_version is not None:
+        revised_at = published_version.published_at
+    else:
+        revised_at = None
     response = dict(
         collection_url=f"{CorporaConfig().collections_base_url}/collections/{collection_id.id}",
         contact_email=collection_version.metadata.contact_email,
         contact_name=collection_version.metadata.contact_name,
         created_at=collection_version.created_at,
-        curator_name=collection_version.owner,
+        curator_name=collection_version.curator_name,
         datasets=response_datasets,
         description=collection_version.metadata.description,
         doi=doi,
@@ -278,7 +280,7 @@ def get_collection_level_processing_status(datasets: List[DatasetVersion]) -> st
     return return_status
 
 
-def get_infered_collection_version_else_forbidden(collection_id: str) -> Optional[CollectionVersionWithDatasets]:
+def get_infered_collection_version_else_forbidden(collection_id: str) -> CollectionVersionWithDatasets:
     """
     Infer the collection version from either a CollectionId or a CollectionVersionId and return the CollectionVersion.
     :param collection_id: identifies the collection version

tests/unit/backend/layers/api/test_curation_api.py

@@ -17,7 +17,7 @@
 from backend.portal.api.curation.v1.curation.collections.common import EntityColumns
 from tests.unit.backend.fixtures.mock_aws_test_case import CorporaTestCaseUsingMockAWS
 from tests.unit.backend.layers.common.base_test import DatasetArtifactUpdate, DatasetStatusUpdate
-from unit.backend.layers.common.base_api_test import BaseAPIPortalTest
+from tests.unit.backend.layers.common.base_api_test import BaseAPIPortalTest
 
 
 class TestAsset(CorporaTestCaseUsingMockAWS):
@@ -138,8 +138,18 @@ def test__delete_tombstone_collection(self):
 
 
 class TestS3Credentials(BaseAPIPortalTest):
+    @patch("backend.common.corpora_config.CorporaConfig.__getattr__")
     @patch("backend.portal.api.curation.v1.curation.collections.collection_id.s3_upload_credentials.sts_client")
-    def test__generate_s3_credentials__OK(self, sts_client: Mock):
+    def test__generate_s3_credentials__OK(self, sts_client: Mock,  mock_config: Mock):
+
+        def mock_config_fn(name):
+            if name == "curator_role_arn":
+                return "test_role_arn"
+            if name == "submission_bucket":
+                return "cellxgene-dataset-submissions-test"
+
+        mock_config.side_effect = mock_config_fn
+
         def _test(token, is_super_curator: bool = False):
             sts_client.assume_role_with_web_identity = Mock(
                 return_value={
@@ -150,17 +160,17 @@ def _test(token, is_super_curator: bool = False):
                     }
                 }
             )
-            collection_id = self.generate_unpublished_collection().collection_id
+            version_id = self.generate_unpublished_collection().version_id
             headers = {"Authorization": f"Bearer {token}"}
 
-            response = self.app.get(f"/curation/v1/collections/{collection_id}/s3-upload-credentials", headers=headers)
+            response = self.app.get(f"/curation/v1/collections/{version_id}/s3-upload-credentials", headers=headers)
             self.assertEqual(200, response.status_code)
             token_sub = self._mock_assert_authorized_token(token)["sub"]
             self.assertEqual(response.json["Bucket"], "cellxgene-dataset-submissions-test")
             if is_super_curator:
-                self.assertEqual(response.json["UploadKeyPrefix"], f"super/{collection_id}/")
+                self.assertEqual(response.json["UploadKeyPrefix"], f"super/{version_id}/")
             else:
-                self.assertEqual(response.json["UploadKeyPrefix"], f"{token_sub}/{collection_id}/")
+                self.assertEqual(response.json["UploadKeyPrefix"], f"{token_sub}/{version_id}/")
 
         with self.subTest("collection owner"):
             _test("owner")

tests/unit/backend/layers/api/test_portal_api.py

@@ -149,7 +149,7 @@ def test__get_collection_id__ok(self):
             "contact_email": "[email protected]",
             "contact_name": "john doe",
             "created_at": mock.ANY,
-            "curator_name": "Test User",
+            "curator_name": "Jane Smith",
             "data_submission_policy_version": "1.0",
             "datasets": [
                 {

tests/unit/backend/layers/common/base_api_test.py

@@ -46,6 +46,10 @@ class BaseAPIPortalTest(BaseAuthAPITest, BaseTest):
     def setUp(self):
         super().setUp()
 
+        # TODO: this can be improved, but the current authorization method requires it
+        self.mock = patch("backend.common.corpora_config.CorporaAuthConfig.__getattr__", return_value="mock_audience")
+        self.mock.start()
+
         self.cloudfront_provider = CDNProviderInterface()
         pa = PortalApi(self.business_logic, self.cloudfront_provider)
 

tests/unit/backend/layers/common/base_test.py

@@ -152,17 +152,18 @@ def generate_unpublished_collection(
 
         return self.business_logic.get_collection_version(collection.version_id)
 
+    # Public collections need to have at least one dataset!
     # Public collections need to have at least one dataset!
     def generate_published_collection(
         self,
         owner="test_user_id",
         links: List[Link] = [],
         add_datasets: int = 1,
-        curator_name: str = "Test User",
+        curator_name: str = "Jane Smith",
         metadata=None,
     ) -> CollectionVersion:
         unpublished_collection = self.generate_unpublished_collection(
-            owner, links, curator_name=curator_name, add_datasets=add_datasets, metadata=metadata
+            owner, curator_name, links, add_datasets=add_datasets, metadata=metadata
         )
         self.business_logic.publish_collection_version(unpublished_collection.version_id)
         return self.business_logic.get_collection_version(unpublished_collection.version_id)
@@ -192,7 +193,7 @@ def generate_dataset(
         )
         if not metadata:
             metadata = copy.deepcopy(self.sample_dataset_metadata)
-        if name:
+        if name is not None:
             metadata.name = name
         self.business_logic.set_dataset_metadata(dataset_version_id, metadata)
         for status in statuses: