diff --git a/backend/config/corpora-api.yml b/backend/config/corpora-api.yml index 174f1e1b2882e..23710c74909ae 100644 --- a/backend/config/corpora-api.yml +++ b/backend/config/corpora-api.yml @@ -19,12 +19,12 @@ paths: - wmg summary: Get WMG cell types (WMG FEATURE PROTOTYPE) security: - - cxguserCookieLenient: [ ] - - { } + - cxguserCookieLenient: [] + - {} description: >- JSON of all genes operationId: backend.corpora.lambdas.api.v1.wmg.get_cell_types - parameters: [ ] + parameters: [] responses: "200": description: OK @@ -50,12 +50,12 @@ paths: - wmg summary: Get WMG genes (WMG FEATURE PROTOTYPE) security: - - cxguserCookieLenient: [ ] - - { } + - cxguserCookieLenient: [] + - {} description: >- JSON of all cell types operationId: backend.corpora.lambdas.api.v1.wmg.get_genes - parameters: [ ] + parameters: [] responses: "200": description: OK @@ -81,18 +81,18 @@ paths: - wmg summary: Get WMG gene expression for a given gene name (WMG FEATURE PROTOTYPE) security: - - cxguserCookieLenient: [ ] - - { } + - cxguserCookieLenient: [] + - {} description: >- JSON of gene expression for a given gene operationId: backend.corpora.lambdas.api.v1.wmg.get_gene_expression parameters: - - name: gene_name - in: path - required: true - schema: - type: string - description: The name of the gene + - name: gene_name + in: path + required: true + schema: + type: string + description: The name of the gene responses: "200": description: OK @@ -235,7 +235,6 @@ paths: operationId: backend.corpora.lambdas.api.v1.collection.get_collection_details parameters: - $ref: "#/components/parameters/path_collection_uuid" - - $ref: "#/components/parameters/query_visibility" responses: "200": description: OK @@ -338,7 +337,6 @@ paths: operationId: backend.corpora.lambdas.api.v1.collection.delete_collection parameters: - $ref: "#/components/parameters/path_collection_uuid" - - $ref: "#/components/parameters/query_visibility" responses: "204": $ref: "#/components/responses/204" @@ -396,7 +394,7 @@ paths: description: >- An authenticated user can upload a file from a shared link to replace an existing dataset in their collection. - The existing dataset must be in an error start or completely uploaded before a replacement can be attempted. + The existing dataset must be in an error state or completely uploaded before a replacement can be attempted. Once the upload of the replacement dataset is complete, the previous version of the dataset is removed and is no longer available for downloading through this collection. The replacement datasets are not publicly available until the collection revision is published. The old datasets are available in the public collection until the @@ -1000,7 +998,7 @@ paths: tags: - wmg operationId: backend.wmg.api.v1.primary_filter_dimensions - parameters: [ ] + parameters: [] responses: "200": description: OK @@ -1026,7 +1024,7 @@ paths: tags: - wmg operationId: backend.wmg.api.v1.query - parameters: [ ] + parameters: [] requestBody: content: application/json: @@ -1205,7 +1203,6 @@ paths: ethnicity_terms: $ref: "#/components/schemas/wmg_ontology_term_id_label_list" - components: schemas: user_uuid: @@ -1362,7 +1359,6 @@ components: - name - revision - collection_id - - collection_visibility properties: id: $ref: "#/components/schemas/dataset_uuid" @@ -1401,8 +1397,6 @@ components: $ref: "#/components/schemas/dataset_asset" collection_id: $ref: "#/components/schemas/collection_uuid" - collection_visibility: - $ref: "#/components/schemas/visibility" is_primary_data: $ref: "#/components/schemas/is_primary_data" cell_type: @@ -1513,7 +1507,6 @@ components: type: string format: uuid - parameters: path_collection_uuid: name: collection_uuid diff --git a/backend/corpora/common/corpora_orm.py b/backend/corpora/common/corpora_orm.py index 37164255b5ffd..0597d374e08de 100644 --- a/backend/corpora/common/corpora_orm.py +++ b/backend/corpora/common/corpora_orm.py @@ -3,12 +3,12 @@ from datetime import datetime from sqlalchemy import ( Boolean, + CheckConstraint, Column, DateTime, Enum, Float, ForeignKey, - ForeignKeyConstraint, Integer, String, UniqueConstraint, @@ -216,7 +216,7 @@ class DbCollection(Base, AuditMixin, TimestampMixin): # the tablename is "project" instead of "collection" to avoid migrating the database __tablename__ = "project" - visibility = Column(Enum(CollectionVisibility), primary_key=True, nullable=False) + visibility = Column(Enum(CollectionVisibility), nullable=False) owner = Column(StrippedString, nullable=False) name = Column(StrippedString) description = Column(StrippedString) @@ -226,12 +226,16 @@ class DbCollection(Base, AuditMixin, TimestampMixin): data_submission_policy_version = Column(StrippedString, nullable=True) tombstone = Column(Boolean, default=False, nullable=False) publisher_metadata = Column(JSON, nullable=True) + revision_of = Column(String, ForeignKey("project.id"), nullable=True, unique=True) # Relationships + revision = relationship("DbCollection", cascade="all, delete-orphan", uselist=False) links = relationship("DbProjectLink", back_populates="collection", cascade="all, delete-orphan") datasets = relationship("DbDataset", back_populates="collection", cascade="all, delete-orphan") genesets = relationship("DbGeneset", back_populates="collection", cascade="all, delete-orphan") + __table_args__ = (CheckConstraint("revision_of IS NULL OR visibility = 'PRIVATE'", "ck_project_private_revision"),) + class DbProjectLink(Base, AuditMixin): """ @@ -241,8 +245,7 @@ class DbProjectLink(Base, AuditMixin): # the tablename is "project_link" instead of "collection_link" to avoid migrating the database __tablename__ = "project_link" - collection_id = Column(String, nullable=False) - collection_visibility = Column(Enum(CollectionVisibility), nullable=False) + collection_id = Column(String, ForeignKey("project.id"), nullable=False) link_name = Column(StrippedString) link_url = Column(StrippedString) link_type = Column(Enum(CollectionLinkType)) @@ -250,12 +253,6 @@ class DbProjectLink(Base, AuditMixin): # Relationships collection = relationship("DbCollection", uselist=False, back_populates="links") - # Composite FK - __table_args__ = ( - ForeignKeyConstraint([collection_id, collection_visibility], [DbCollection.id, DbCollection.visibility]), - {}, - ) - # provide a consistent name DbCollectionLink = DbProjectLink @@ -283,8 +280,7 @@ class DbDataset(Base, AuditMixin, TimestampMixin): cell_count = Column(Integer) is_valid = Column(Boolean, default=False) is_primary_data = Column(Enum(IsPrimaryData)) - collection_id = Column(String, nullable=False) - collection_visibility = Column(Enum(CollectionVisibility), nullable=False) + collection_id = Column(String, ForeignKey("project.id"), nullable=False) tombstone = Column(Boolean, default=False, nullable=False) original_id = Column(String) published = Column(Boolean, default=False) @@ -304,11 +300,7 @@ class DbDataset(Base, AuditMixin, TimestampMixin): genesets = relationship("DbGeneset", secondary="geneset_dataset_link", back_populates="datasets") # Composite FK - __table_args__ = ( - ForeignKeyConstraint([collection_id, collection_visibility], [DbCollection.id, DbCollection.visibility]), - UniqueConstraint("collection_id", "curator_tag", name="_dataset__collection_id_curator_tag"), - {}, - ) + __table_args__ = (UniqueConstraint("collection_id", "curator_tag", name="_dataset__collection_id_curator_tag"),) def to_dict(self, *args, **kwargs): kwargs["remove_attr"] = kwargs.get("remove_attr", []) + ["genesets"] @@ -326,7 +318,7 @@ class DbDatasetArtifact(Base, AuditMixin): __tablename__ = "dataset_artifact" __mapper_args__ = {"confirm_deleted_rows": False} - dataset_id = Column(ForeignKey("dataset.id"), nullable=False) + dataset_id = Column(String, ForeignKey("dataset.id"), nullable=False) filename = Column(String) filetype = Column(Enum(DatasetArtifactFileType)) user_submitted = Column(Boolean) @@ -418,7 +410,7 @@ class DbDatasetProcessingStatus(Base, AuditMixin): __tablename__ = "dataset_processing_status" - dataset_id = Column(ForeignKey("dataset.id"), nullable=False) + dataset_id = Column(String, ForeignKey("dataset.id"), nullable=False) upload_status = Column(Enum(UploadStatus)) upload_progress = Column(Float) upload_message = Column(String) @@ -443,15 +435,11 @@ class DbGeneset(Base, AuditMixin): name = Column(String, nullable=False) description = Column(String) genes = Column(JSONB) - collection_id = Column(String, nullable=False) - collection_visibility = Column(Enum(CollectionVisibility), nullable=False) + collection_id = Column(String, ForeignKey("project.id"), nullable=False) collection = relationship("DbCollection", uselist=False, back_populates="genesets") datasets = relationship("DbDataset", secondary="geneset_dataset_link", back_populates="genesets") - __table_args__ = ( - ForeignKeyConstraint([collection_id, collection_visibility], [DbCollection.id, DbCollection.visibility]), - UniqueConstraint("name", "collection_id", "collection_visibility", name="_geneset_name__collection_uc"), - ) + __table_args__ = (UniqueConstraint("name", "collection_id", name="_geneset_name__collection_uc"),) def to_dict(self, *args, **kwargs): kwargs["remove_attr"] = kwargs.get("remove_attr", []) + ["datasets"] @@ -467,5 +455,5 @@ class DbGenesetDatasetLink(Base, AuditMixin): __tablename__ = "geneset_dataset_link" - geneset_id = Column(String, ForeignKey("geneset.id"), index=True) - dataset_id = Column(String, ForeignKey("dataset.id"), index=True) + geneset_id = Column(String, ForeignKey("geneset.id"), index=True, nullable=False) + dataset_id = Column(String, ForeignKey("dataset.id"), index=True, nullable=False) diff --git a/backend/corpora/common/entities/collection.py b/backend/corpora/common/entities/collection.py index 71fb0650a66d3..c1236e8400757 100644 --- a/backend/corpora/common/entities/collection.py +++ b/backend/corpora/common/entities/collection.py @@ -1,11 +1,19 @@ import typing from datetime import datetime from sqlalchemy import and_ +from sqlalchemy.orm import Session from . import Dataset from .entity import Entity from .geneset import Geneset -from ..corpora_orm import CollectionLinkType, DbCollection, DbCollectionLink, CollectionVisibility +from .collection_link import CollectionLink +from ..corpora_orm import ( + CollectionLinkType, + DbCollection, + DbCollectionLink, + CollectionVisibility, + generate_uuid, +) from ..utils.db_helpers import clone @@ -19,7 +27,7 @@ def __init__(self, db_object: DbCollection): @classmethod def create( cls, - session, + session: Session, visibility: CollectionVisibility, name: str = "", description: str = "", @@ -50,10 +58,7 @@ def create( publisher_metadata=publisher_metadata, **kwargs, ) - - new_db_object.links = [ - DbCollectionLink(collection_id=new_db_object.id, collection_visibility=visibility, **link) for link in links - ] + new_db_object.links = [DbCollectionLink(collection_id=new_db_object.id, **link) for link in links] session.add(new_db_object) session.commit() return cls(new_db_object) @@ -61,9 +66,10 @@ def create( @classmethod def get_collection( cls, - session, - collection_uuid: str, - visibility: str = CollectionVisibility.PUBLIC.name, + session: Session, + collection_uuid: str = None, + visibility: CollectionVisibility = None, + revision_of: str = None, include_tombstones: bool = False, owner: typing.Optional[str] = None, ) -> typing.Union["Collection", None]: @@ -72,33 +78,47 @@ def get_collection( :param session: the database session object. :param collection_uuid: :param visibility: the visibility of the collection + :param revision_of: the id of the associated public collection iff this collection is a revision :param include_tombstones: If true, the collection is returned even if it has been tombstoned. :param owner: A user id use to check if the user is the owner of the collection. If the user id matches the owner then the collection is returned. If this parameters is not included then owner is not used as a filter. :return: the collection if it matches the filter. """ - filters = [cls.table.id == collection_uuid, cls.table.visibility == visibility] + filters = [] + if visibility: + filters.append(cls.table.visibility == visibility) + if collection_uuid: + filters.append(cls.table.id == collection_uuid) + if revision_of: + filters.append(cls.table.revision_of == revision_of) if owner: filters.append(cls.table.owner == owner) if not include_tombstones: filters.append(cls.table.tombstone == False) # noqa collection = session.query(cls.table).filter(*filters).one_or_none() + return cls(collection) if collection else None @classmethod - def list_collections_in_time_range(cls, session, *args, **kwargs): + def list_collections_in_time_range(cls, session: Session, *args, **kwargs): return cls.list_attributes_in_time_range( session, *args, filters=[DbCollection.visibility == CollectionVisibility.PUBLIC.name], **kwargs ) @classmethod def list_attributes_in_time_range( - cls, session, to_date: int = None, from_date: int = None, filters: list = None, list_attributes: list = None + cls, + session: Session, + to_date: int = None, + from_date: int = None, + filters: list = None, + list_attributes: list = None, ) -> typing.List[typing.Dict]: """ Queries the database for Entities that have been created within the specified time range. Return only the entity attributes in `list_attributes`. + :param session: The SQLAlchemy database Session :param to_date: If provided, only lists collections that were created before this date. Format of param is Unix timestamp since the epoch in UTC timezone. :param from_date: If provided, only lists collections that were created after this date. Format of param is Unix @@ -131,6 +151,57 @@ def to_dict(db_object): return results + @classmethod + def list_public_datasets_for_index(cls, session: Session) -> typing.List[typing.Dict]: + """ + Return a list of all the datasets and associated metadata. For efficiency reasons, this only returns the fields + inside the `dataset` table and doesn't include relationships. + """ + + attrs = [ + Dataset.table.id, + Dataset.table.name, + Dataset.table.collection_id, + Dataset.table.tissue, + Dataset.table.disease, + Dataset.table.assay, + Dataset.table.organism, + Dataset.table.cell_count, + Dataset.table.cell_type, + Dataset.table.sex, + Dataset.table.ethnicity, + Dataset.table.development_stage, + Dataset.table.is_primary_data, + Dataset.table.mean_genes_per_cell, + Dataset.table.schema_version, # Required for schema manipulation + Dataset.table.explorer_url, + Dataset.table.published_at, + Dataset.table.revised_at, + ] + + def to_dict(db_object): + _result = {} + for _field in db_object._fields: + _value = getattr(db_object, _field) + if _value is None: + continue + _result[_field] = getattr(db_object, _field) + return _result + + filters = [~Dataset.table.tombstone, cls.table.visibility == CollectionVisibility.PUBLIC.name] + + results = [ + to_dict(result) + for result in session.query(Dataset.table).join(cls.table).filter(*filters).with_entities(*attrs).all() + ] + + for result in results: + Dataset.transform_organism_for_schema_2_0_0(result) + Dataset.transform_sex_for_schema_2_0_0(result) + Dataset.enrich_development_stage_with_ancestors(result) + + return results + def reshape_for_api(self, tombstoned_datasets=False) -> dict: """ Reshape the collection to match the expected api output. @@ -174,74 +245,76 @@ def publish(self, data_submission_policy_version): # Timestamp for published_at and revised_at now = datetime.utcnow() # Create a public collection with the same uuid and same fields - public_collection = Collection.get_collection(self.session, self.id, CollectionVisibility.PUBLIC) is_existing_collection = False - - if public_collection: + if self.revision_of: + # This is a revision of a published Collection + public_collection = Collection.get_collection(self.session, collection_uuid=self.revision_of) revision = self.to_dict( - remove_attr=("updated_at", "created_at", "visibility", "id"), remove_relationships=True + remove_attr=("updated_at", "created_at", "visibility", "id", "revision_of", "published_at"), + remove_relationships=True, ) revision["data_submission_policy_version"] = data_submission_policy_version public_collection.update( commit=False, **revision, ) + for link in self.links: + CollectionLink(link).update(collection_id=self.revision_of, commit=False) + public_collection.links = self.links is_existing_collection = True - # A published collection with the same uuid does not already exist. - # This is a new collection. + else: - public_collection = Collection( - clone( - self.db_object, - primary_key=dict(id=self.id, visibility=CollectionVisibility.PUBLIC), - # We want to update published_at only when the collection is first published. - published_at=now, - data_submission_policy_version=data_submission_policy_version, - ) + # This is the first publishing of this Collection + self.update( + commit=False, + visibility=CollectionVisibility.PUBLIC, + published_at=now, + data_submission_policy_version=data_submission_policy_version, + keep_links=True, ) - self.session.add(public_collection) - - # Copy over relationships - for link in self.links: - link.collection_visibility = CollectionVisibility.PUBLIC has_dataset_changes = False for dataset in self.datasets: - revision = Dataset(dataset) - original = Dataset.get(self.session, revision.original_id) if revision.original_id else None + revised_dataset = Dataset(dataset) + original = Dataset.get(self.session, revised_dataset.original_id) if revised_dataset.original_id else None if original and public_collection.check_has_dataset(original): - dataset_is_changed = original.publish_revision(revision, now) + dataset_is_changed = original.publish_revision(revised_dataset, now) if dataset_is_changed: has_dataset_changes = True else: # The dataset is new - revision.publish_new(now) + revised_dataset.publish_new(now) has_dataset_changes = True - if is_existing_collection and has_dataset_changes: - public_collection.update(commit=False, remove_attr="revised_at", revised_at=now) + self.session.flush() + self.session.expire_all() + + if is_existing_collection: + if has_dataset_changes: + public_collection.update(commit=False, remove_attr="revised_at", revised_at=now) + self.delete() + self.db_object = public_collection.db_object self.session.commit() - # commit expires the session, need to retrieve the original private collection to delete it - private_collection = Collection.get_collection(self.session, self.id, CollectionVisibility.PRIVATE) - private_collection.delete() - self.db_object = public_collection.db_object - def revision(self) -> "Collection": + def create_revision(self) -> "Collection": """ Generate a collection revision from a public collection :return: collection revision. """ revision_collection = clone( - self.db_object, primary_key=dict(id=self.id, visibility=CollectionVisibility.PRIVATE) + self.db_object, + primary_key=dict(id=generate_uuid()), + visibility=CollectionVisibility.PRIVATE, + revision_of=self.id, ) self.session.add(revision_collection) for link in self.links: - self.session.add(clone(link, collection_id=self.id, collection_visibility=CollectionVisibility.PRIVATE)) - self.session.commit() + self.session.add(clone(link, collection_id=revision_collection.id)) for dataset in self.datasets: - Dataset(dataset).create_revision() + Dataset(dataset).create_revision(revision_collection.id) + self.session.commit() return Collection(revision_collection) def tombstone_collection(self): @@ -254,19 +327,20 @@ def tombstone_collection(self): ds.tombstone_dataset_and_delete_child_objects() self.session.commit() - def update(self, links: list = None, **kwargs) -> None: + def update(self, links: list = None, keep_links=False, **kwargs) -> None: """ Update an existing collection to match provided the parameters. The specified columns are replaced. :param links: links to create and connect to the collection. If present, the existing attached entries will be removed and replaced with new entries. + :param keep_links: boolean - whether or not links need to be preserved. Links are preserved if True. :param kwargs: Any other fields in the dataset that will be replaced. """ links = links if links else [] - for link in self.links: - self.session.delete(link) - new_objs = [ - DbCollectionLink(collection_id=self.id, collection_visibility=self.visibility, **link) for link in links - ] + if not keep_links: + for link in self.links: + self.session.delete(link) + + new_objs = [DbCollectionLink(collection_id=self.id, **link) for link in links] self.session.add_all(new_objs) super().update(**kwargs) @@ -281,7 +355,7 @@ def delete(self): def check_has_dataset(self, dataset: Dataset) -> bool: """Check that a dataset is part of the collection""" - return all([self.id == dataset.collection_id, self.visibility == dataset.collection_visibility]) + return self.id == dataset.collection_id def get_doi(self) -> str: doi = [link for link in self.links if link.link_type == CollectionLinkType.DOI] diff --git a/backend/corpora/common/entities/collection_link.py b/backend/corpora/common/entities/collection_link.py new file mode 100644 index 0000000000000..c334edc85eb1b --- /dev/null +++ b/backend/corpora/common/entities/collection_link.py @@ -0,0 +1,21 @@ +import typing + +from sqlalchemy.orm import Session + +from backend.corpora.common.corpora_orm import DbCollectionLink +from backend.corpora.common.entities.entity import Entity + + +class CollectionLink(Entity): + table = DbCollectionLink + + @classmethod + def create(cls, session: Session, collection_id: str, **kwargs) -> "CollectionLink": + link = DbCollectionLink(collection_id=collection_id, **kwargs) + session.add(link) + session.commit() + return cls(link) + + @classmethod + def retrieve_all_links_for_a_collection(cls, session: Session, collection_id: str) -> typing.List["CollectionLink"]: + return session.query(cls.table).filter(cls.table.collection_id == collection_id).all() diff --git a/backend/corpora/common/entities/dataset.py b/backend/corpora/common/entities/dataset.py index 70acb46590da6..6689dfd798a14 100644 --- a/backend/corpora/common/entities/dataset.py +++ b/backend/corpora/common/entities/dataset.py @@ -5,6 +5,8 @@ from collections import OrderedDict from datetime import datetime from pathlib import PurePosixPath +from sqlalchemy.orm import Session + from urllib.parse import urlparse from .dataset_asset import DatasetAsset @@ -17,7 +19,6 @@ UploadStatus, ProcessingStatus, DbGenesetDatasetLink, - CollectionVisibility, generate_uuid, DatasetArtifactFileType, ConversionStatus, @@ -38,7 +39,7 @@ def __init__(self, db_object: DbDataset): @classmethod def create( cls, - session, + session: Session, revision: int = 0, name: str = "", artifacts: list = None, @@ -85,11 +86,10 @@ def update(self, artifacts: list = None, processing_status: dict = None, commit= if processing_status: new_obj = DbDatasetProcessingStatus(dataset_id=self.id, **processing_status) self.session.add(new_obj) - super().update(commit=commit, **kwargs) @classmethod - def get(cls, session, dataset_uuid, include_tombstones=False) -> "Dataset": + def get(cls, session: Session, dataset_uuid, include_tombstones=False) -> "Dataset": dataset = super().get(session, dataset_uuid) if not include_tombstones: if dataset and dataset.tombstone is True: @@ -97,7 +97,7 @@ def get(cls, session, dataset_uuid, include_tombstones=False) -> "Dataset": return dataset @classmethod - def get_by_explorer_url(cls, session, explorer_url): + def get_by_explorer_url(cls, session: Session, explorer_url): """ Return the most recently created dataset with the given explorer_url or None """ @@ -160,55 +160,6 @@ def enrich_development_stage_with_ancestors(dataset): if unique_ancestors: dataset["development_stage_ancestors"] = unique_ancestors - @classmethod - def list_for_index(cls, session) -> typing.List[typing.Dict]: - """ - Return a list of all the datasets and associated metadata. For efficiency reasons, this only returns the fields - inside the `dataset` table and doesn't include relationships. - """ - - attrs = [ - DbDataset.id, - DbDataset.name, - DbDataset.collection_id, - DbDataset.tissue, - DbDataset.disease, - DbDataset.assay, - DbDataset.organism, - DbDataset.cell_count, - DbDataset.cell_type, - DbDataset.sex, - DbDataset.ethnicity, - DbDataset.development_stage, - DbDataset.is_primary_data, - DbDataset.mean_genes_per_cell, - DbDataset.schema_version, # Required for schema manipulation - DbDataset.explorer_url, - DbDataset.published_at, - DbDataset.revised_at, - ] - table = cls.table - - def to_dict(db_object): - _result = {} - for _field in db_object._fields: - _value = getattr(db_object, _field) - if _value is None: - continue - _result[_field] = getattr(db_object, _field) - return _result - - filters = [~DbDataset.tombstone, DbDataset.collection_visibility == CollectionVisibility.PUBLIC] - - results = [to_dict(result) for result in session.query(table).filter(*filters).with_entities(*attrs).all()] - - for result in results: - Dataset.transform_organism_for_schema_2_0_0(result) - Dataset.transform_sex_for_schema_2_0_0(result) - Dataset.enrich_development_stage_with_ancestors(result) - - return results - def _create_new_explorer_url(self, new_uuid: str) -> str: if self.explorer_url is None: return None @@ -219,10 +170,10 @@ def _create_new_explorer_url(self, new_uuid: str) -> str: # Note: the final slash is mandatory, otherwise the explorer won't load this link return f"{new_url}/" - def create_revision(self) -> "Dataset": + def create_revision(self, revision_collection_id: str) -> "Dataset": """ Generate a dataset revision from a dataset in a public collection - :param dataset_id: specify the uuid if the revision, otherwise one is generated. + :param revision_collection_id: specify the collection revision to which this dataset revision belongs :return: dataset revision. """ @@ -231,8 +182,7 @@ def create_revision(self) -> "Dataset": revision_dataset = clone( self.db_object, id=revision_dataset_uuid, - collection_id=self.collection_id, - collection_visibility=CollectionVisibility.PRIVATE, + collection_id=revision_collection_id, original_id=self.id, explorer_url=revision_explorer_url, ) @@ -329,7 +279,8 @@ def publish_new(self, now: datetime): with the provided datetime. :param now: Datetime to populate dataset's published_at. """ - self.update(collection_visibility=CollectionVisibility.PUBLIC, published=True, published_at=now, commit=False) + collection_id = self.collection.revision_of if self.collection.revision_of else self.collection.id + self.update(collection_id=collection_id, published=True, published_at=now, commit=False) def publish_revision(self, revision: "Dataset", now: datetime) -> bool: """ @@ -360,8 +311,8 @@ def publish_revision(self, revision: "Dataset", now: datetime) -> bool: remove_attr=[ "updated_at", "created_at", - "collection_visibility", "id", + "collection_id", "original_id", "published", "revised_at", @@ -377,7 +328,6 @@ def publish_revision(self, revision: "Dataset", now: datetime) -> bool: self.update(commit=False, **updates, revised_at=now) return True - return False diff --git a/backend/corpora/common/entities/entity.py b/backend/corpora/common/entities/entity.py index bde4147398558..402ba14c25546 100644 --- a/backend/corpora/common/entities/entity.py +++ b/backend/corpora/common/entities/entity.py @@ -2,6 +2,7 @@ import typing from sqlalchemy import inspect +from sqlalchemy.orm import Session logger = logging.getLogger(__name__) @@ -29,9 +30,10 @@ def __init__(self, db_object: Base): self.session = inspect(db_object).session @classmethod - def get(cls, session, key: typing.Union[str, typing.Tuple[str, str]]) -> typing.Union["Entity", None]: + def get(cls, session: Session, key: typing.Union[str, typing.Tuple[str, str]]) -> typing.Union["Entity", None]: """ Retrieves an entity from the database given its primary key if found. + :param session: The SQLAlchemy database Session :param key: Simple or composite primary key :return: Entity or None """ @@ -43,7 +45,7 @@ def get(cls, session, key: typing.Union[str, typing.Tuple[str, str]]) -> typing. return None @classmethod - def list(cls, session) -> "Entity": + def list(cls, session: Session) -> "Entity": """ Retrieves a list of entities from the database :return: list of Entity diff --git a/backend/corpora/common/entities/geneset.py b/backend/corpora/common/entities/geneset.py index a1814ee8bc161..471e00f34baa5 100644 --- a/backend/corpora/common/entities/geneset.py +++ b/backend/corpora/common/entities/geneset.py @@ -1,5 +1,6 @@ import typing from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.orm import Session from ..corpora_orm import DbGeneset, DbGenesetDatasetLink from .entity import Entity @@ -10,7 +11,7 @@ class Geneset(Entity): table = DbGeneset @classmethod - def create(cls, session, name: str, description: str, genes: list, dataset_ids: list = [], **kwargs): + def create(cls, session: Session, name: str, description: str, genes: list, dataset_ids: list = [], **kwargs): gene_set = DbGeneset(name=name, description=description, genes=genes, **kwargs) session.add(gene_set) session.commit() @@ -20,7 +21,7 @@ def create(cls, session, name: str, description: str, genes: list, dataset_ids: return cls(gene_set) @classmethod - def retrieve_all_genesets_for_a_collection(cls, session, collection_id): + def retrieve_all_genesets_for_a_collection(cls, session: Session, collection_id): genesets = session.query(cls.table).filter(cls.table.collection_id == collection_id).all() reshaped_genesets = [] for geneset in genesets: @@ -32,7 +33,6 @@ def retrieve_all_genesets_for_a_collection(cls, session, collection_id): "updated_at", "collection", "collection_id", - "collection_visibility", ] ) ) @@ -62,14 +62,14 @@ class GenesetDatasetLink(Entity): table = DbGenesetDatasetLink @classmethod - def create(cls, session, geneset_id: str, dataset_id: str): + def create(cls, session: Session, geneset_id: str, dataset_id: str): link = DbGenesetDatasetLink(geneset_id=geneset_id, dataset_id=dataset_id) session.add(link) session.commit() return cls(link) @classmethod - def get(cls, session, geneset_id: str, dataset_id: str): + def get(cls, session: Session, geneset_id: str, dataset_id: str): link = ( session.query(cls.table) .filter(cls.table.geneset_id == geneset_id, cls.table.dataset_id == dataset_id) @@ -82,7 +82,7 @@ def get(cls, session, geneset_id: str, dataset_id: str): @classmethod def update_links_for_a_dataset( - cls, session, dataset_id: str, add: typing.Optional[list] = None, remove: typing.Optional[list] = None + cls, session: Session, dataset_id: str, add: typing.Optional[list] = None, remove: typing.Optional[list] = None ): for gene_set_id in remove: link = cls.get(session=session, dataset_id=dataset_id, geneset_id=gene_set_id) diff --git a/backend/corpora/common/utils/db_helpers.py b/backend/corpora/common/utils/db_helpers.py index eeb0c99affe49..ad10614b5d40e 100644 --- a/backend/corpora/common/utils/db_helpers.py +++ b/backend/corpora/common/utils/db_helpers.py @@ -1,6 +1,7 @@ import logging from backend.corpora.common.corpora_orm import Base, DbDatasetProcessingStatus +from sqlalchemy.orm import Session logger = logging.getLogger(__name__) @@ -24,7 +25,7 @@ def clone(model: Base, primary_key: dict = None, **kwargs) -> Base: return model.__class__(**data) -def processing_status_updater(session, uuid: str, updates: dict): +def processing_status_updater(session: Session, uuid: str, updates: dict): """ Updates the DbDatasetProcessingStatus object with the specified statuses Note: The uuid parameter is the ID of the dataset_processing_status row, _NOT_ the dataset! diff --git a/backend/corpora/lambdas/api/v1/collection.py b/backend/corpora/lambdas/api/v1/collection.py index f57f3abfb056b..4b89bed774695 100644 --- a/backend/corpora/lambdas/api/v1/collection.py +++ b/backend/corpora/lambdas/api/v1/collection.py @@ -10,7 +10,11 @@ from ....common.corpora_orm import DbCollection, CollectionVisibility from ....common.entities import Collection -from ....common.utils.exceptions import InvalidParametersHTTPException, ForbiddenHTTPException, ConflictException +from ....common.utils.exceptions import ( + InvalidParametersHTTPException, + ForbiddenHTTPException, + ConflictException, +) from ....api_server.db import dbconnect from backend.corpora.lambdas.api.v1.authorization import has_scope @@ -36,15 +40,30 @@ def get_collections_list(from_date: int = None, to_date: int = None, user: Optio db_session, from_date=from_date, to_date=to_date, - list_attributes=[DbCollection.id, DbCollection.visibility, DbCollection.owner, DbCollection.created_at], + list_attributes=[ + DbCollection.id, + DbCollection.visibility, + DbCollection.owner, + DbCollection.created_at, + DbCollection.revision_of, + ], ) collections = [] for coll_dict in all_collections: visibility = coll_dict["visibility"] owner = coll_dict["owner"] - if visibility == CollectionVisibility.PUBLIC or _is_user_owner_or_allowed(user, owner): + if visibility == CollectionVisibility.PUBLIC: collections.append(dict(id=coll_dict["id"], created_at=coll_dict["created_at"], visibility=visibility.name)) + elif _is_user_owner_or_allowed(user, owner): + collections.append( + dict( + id=coll_dict["id"], + created_at=coll_dict["created_at"], + visibility=visibility.name, + revision_of=coll_dict["revision_of"], + ) + ) result = {"collections": collections} if from_date: @@ -63,10 +82,12 @@ def get_collection(db_session, collection_uuid, visibility, **kwargs): @dbconnect -def get_collection_details(collection_uuid: str, visibility: str, user: str): +def get_collection_details(collection_uuid: str, user: str): db_session = g.db_session - collection = get_collection(db_session, collection_uuid, visibility, include_tombstones=True) - if collection.tombstone and visibility == CollectionVisibility.PUBLIC.name: + collection = Collection.get_collection(db_session, collection_uuid, include_tombstones=True) + if not collection: + raise ForbiddenHTTPException() + if collection.tombstone: result = "" response = 410 else: @@ -110,11 +131,11 @@ def post_collection_revision(collection_uuid: str, user: str): collection = get_collection( db_session, collection_uuid, - CollectionVisibility.PUBLIC.name, + CollectionVisibility.PUBLIC, owner=_owner_or_allowed(user), ) try: - collection_revision = collection.revision() + collection_revision = collection.create_revision() except sqlalchemy.exc.IntegrityError as ex: db_session.rollback() raise ConflictException() from ex @@ -197,43 +218,27 @@ def get_collection_dataset(dataset_uuid: str): @dbconnect -def delete_collection(collection_uuid: str, visibility: str, user: str): +def delete_collection(collection_uuid: str, user: str): db_session = g.db_session - if visibility == CollectionVisibility.PUBLIC.name: - pub_collection = Collection.get_collection( - db_session, - collection_uuid, - visibility, - owner=_owner_or_allowed(user), - include_tombstones=True, - ) - priv_collection = Collection.get_collection( - db_session, - collection_uuid, - CollectionVisibility.PRIVATE.name, - owner=_owner_or_allowed(user), - include_tombstones=True, - ) - - if pub_collection: - if not pub_collection.tombstone: - pub_collection.tombstone_collection() - if priv_collection: - if not priv_collection.tombstone: - priv_collection.delete() - return "", 204 - else: - priv_collection = Collection.get_collection( - db_session, - collection_uuid, - CollectionVisibility.PRIVATE.name, - owner=_owner_or_allowed(user), - include_tombstones=True, - ) - if priv_collection: - if not priv_collection.tombstone: - priv_collection.delete() - return "", 204 + collection = Collection.get_collection( + db_session, + collection_uuid, + owner=_owner_or_allowed(user), + include_tombstones=True, + ) + if collection: + if collection.visibility == CollectionVisibility.PUBLIC: + revision = Collection.get_collection( + db_session, + revision_of=collection_uuid, + owner=_owner_or_allowed(user), + ) + if revision: + revision.delete() + collection.tombstone_collection() + else: + collection.delete() + return "", 204 return "", 403 @@ -243,7 +248,7 @@ def update_collection(collection_uuid: str, body: dict, user: str): collection = get_collection( db_session, collection_uuid, - CollectionVisibility.PRIVATE.name, + visibility=CollectionVisibility.PRIVATE.name, owner=_owner_or_allowed(user), ) diff --git a/backend/corpora/lambdas/api/v1/collection_uuid/gene_set.py b/backend/corpora/lambdas/api/v1/collection_uuid/gene_set.py index ff8620d75d50a..23602f54578ec 100644 --- a/backend/corpora/lambdas/api/v1/collection_uuid/gene_set.py +++ b/backend/corpora/lambdas/api/v1/collection_uuid/gene_set.py @@ -29,7 +29,6 @@ def post(collection_uuid: str, body: dict, user: str): description=gene_set["gene_set_description"], genes=gene_set["genes"], collection_id=collection.id, - collection_visibility=collection.visibility.name, ) except IntegrityError: db_session.rollback() diff --git a/backend/corpora/lambdas/api/v1/collection_uuid/publish.py b/backend/corpora/lambdas/api/v1/collection_uuid/publish.py index 4eaae4ef681fe..e958f1b2fd4c8 100644 --- a/backend/corpora/lambdas/api/v1/collection_uuid/publish.py +++ b/backend/corpora/lambdas/api/v1/collection_uuid/publish.py @@ -24,8 +24,8 @@ def post(collection_uuid: str, body: object, user: str): raise ForbiddenHTTPException() if all([dataset.tombstone for dataset in collection.datasets]): raise ConflictException(detail="The collection must have a least one dataset.") - + collection_id = collection.revision_of if collection.revision_of else collection.id data_submission_policy_version = body["data_submission_policy_version"] collection.publish(data_submission_policy_version=data_submission_policy_version) cloudfront.create_invalidation_for_index_paths() - return make_response({"collection_uuid": collection.id, "visibility": collection.visibility}, 202) + return make_response({"collection_uuid": collection_id, "visibility": CollectionVisibility.PUBLIC}, 202) diff --git a/backend/corpora/lambdas/api/v1/collection_uuid/upload.py b/backend/corpora/lambdas/api/v1/collection_uuid/upload.py index 79955584abc1a..fb815481dee5c 100644 --- a/backend/corpora/lambdas/api/v1/collection_uuid/upload.py +++ b/backend/corpora/lambdas/api/v1/collection_uuid/upload.py @@ -49,18 +49,20 @@ def upload_from_link(collection_uuid: str, user: str, url: str, dataset_id: str if resp["name"].rsplit(".")[-1].lower() not in CorporaConfig().upload_file_formats: raise InvalidParametersHTTPException("The file referred to by the link is not a support file format.") - # Create dataset + # Get the Collection collection = Collection.get_collection( db_session, collection_uuid, - CollectionVisibility.PRIVATE, + visibility=CollectionVisibility.PRIVATE, # Do not allow changes to public Collections owner=_owner_or_allowed(user), ) if not collection: raise ForbiddenHTTPException + if dataset_id: + # Update dataset dataset = Dataset.get(db_session, dataset_id) - if collection_uuid == dataset.collection_id and CollectionVisibility.PRIVATE == dataset.collection_visibility: + if collection_uuid == dataset.collection_id: if dataset.processing_status.processing_status in [ProcessingStatus.SUCCESS, ProcessingStatus.FAILURE]: dataset.reprocess() else: @@ -69,6 +71,7 @@ def upload_from_link(collection_uuid: str, user: str, url: str, dataset_id: str raise NotFoundHTTPException else: + # Add new dataset dataset = Dataset.create(db_session, collection=collection) dataset.update(processing_status=dataset.new_processing_status()) # Start processing link diff --git a/backend/corpora/lambdas/api/v1/dataset.py b/backend/corpora/lambdas/api/v1/dataset.py index 1c2f02b3852fa..bbdcf2c02fee6 100644 --- a/backend/corpora/lambdas/api/v1/dataset.py +++ b/backend/corpora/lambdas/api/v1/dataset.py @@ -79,7 +79,7 @@ def get_status(dataset_uuid: str, user: str): @dbconnect def get_datasets_index(): db_session = g.db_session - datasets = Dataset.list_for_index(db_session) + datasets = Collection.list_public_datasets_for_index(db_session) return make_response(jsonify(datasets), 200) @@ -95,12 +95,11 @@ def delete_dataset(dataset_uuid: str, user: str): collection = Collection.get_collection( db_session, dataset.collection.id, - dataset.collection.visibility, owner=_owner_or_allowed(user), ) if not collection: raise ForbiddenHTTPException() - if dataset.collection_visibility == CollectionVisibility.PUBLIC: + if dataset.collection.visibility == CollectionVisibility.PUBLIC: return make_response(jsonify("Can not delete a public dataset"), 405) if dataset.tombstone is False: if dataset.published: @@ -108,7 +107,7 @@ def delete_dataset(dataset_uuid: str, user: str): else: if dataset.original_id: original = Dataset.get(db_session, dataset.original_id) - original.create_revision() + original.create_revision(dataset.collection.id) dataset.asset_deletion() dataset.delete() return "", 202 @@ -127,7 +126,7 @@ def get_dataset_identifiers(url: str): "s3_uri": s3_uri, "dataset_id": dataset.id, "collection_id": dataset.collection_id, - "collection_visibility": dataset.collection_visibility, + "collection_visibility": dataset.collection.visibility, "tombstoned": dataset.tombstone, } return make_response(jsonify(dataset_identifiers), 200) @@ -153,7 +152,6 @@ def post_dataset_gene_sets(dataset_uuid: str, body: object, user: str): x.to_dict( remove_attr=[ "collection", - "collection_visibility", "collection_id", "created_at", "updated_at", diff --git a/backend/corpora/lambdas/api/v1/gene_sets.py b/backend/corpora/lambdas/api/v1/gene_sets.py index 0d74b520cb493..eef02b7ff9f22 100644 --- a/backend/corpora/lambdas/api/v1/gene_sets.py +++ b/backend/corpora/lambdas/api/v1/gene_sets.py @@ -21,7 +21,7 @@ def delete(geneset_uuid: str, user: str): return accepted_response if not _is_user_owner_or_allowed(user, geneset.collection.owner): raise ForbiddenHTTPException() - if geneset.collection_visibility == CollectionVisibility.PUBLIC: + if geneset.collection.visibility == CollectionVisibility.PUBLIC: return make_response(jsonify("Cannot delete a public geneset"), 405) geneset.delete() return accepted_response diff --git a/backend/database/versions/29_e08b6b67f076_make_collection_id_pkey.py b/backend/database/versions/29_e08b6b67f076_make_collection_id_pkey.py new file mode 100644 index 0000000000000..6f07691abff80 --- /dev/null +++ b/backend/database/versions/29_e08b6b67f076_make_collection_id_pkey.py @@ -0,0 +1,118 @@ +"""make_collection_id_pkey + +Revision ID: 28_e08b6b67f076 +Revises: 27_4d70e8c321e3 +Create Date: 2022-03-04 21:54:51.795657 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "29_e08b6b67f076" +down_revision = "28_d7976d87cc2f" +branch_labels = None +depends_on = None + + +def upgrade(): + # Drop foreign keys that reference composite key [project.id, project.visibility] + op.drop_constraint("dataset_collection_id_fkey", "dataset", type_="foreignkey") + op.drop_column("dataset", "collection_visibility") + op.drop_constraint("project_link_collection_id_fkey", "project_link", type_="foreignkey") + op.drop_column("project_link", "collection_visibility") + + # Drop foreign key that references composite key [project.id, project.visibility] + op.drop_constraint("geneset_collection_id_fkey", "geneset", type_="foreignkey") + # Drop unique key that uses collection_visibility + op.drop_constraint("_geneset_name__collection_uc", "geneset", type_="unique") + op.drop_column("geneset", "collection_visibility") + # Recreate unique key without collection_visibility + op.create_unique_constraint(op.f("_geneset_name__collection_uc"), "geneset", ["name", "collection_id"]) + + # Add project.revision_of + op.add_column("project", sa.Column("revision_of", sa.String(), nullable=True, unique=True)) + + # Drop project composite primary key; add project.id as primary key + op.drop_constraint("project_pkey", "project") + op.create_primary_key(op.f("pk_project"), "project", ["id"]) + + # Add foreign keys for project.id + op.create_foreign_key(op.f("fk_dataset_collection_id_project"), "dataset", "project", ["collection_id"], ["id"]) + op.create_foreign_key(op.f("fk_geneset_collection_id_project"), "geneset", "project", ["collection_id"], ["id"]) + op.create_foreign_key(op.f("fk_project_revision_of_project"), "project", "project", ["revision_of"], ["id"]) + op.create_foreign_key( + op.f("fk_project_link_collection_id_project"), "project_link", "project", ["collection_id"], ["id"] + ) + + # Check constraint: published Collections must not be a revision_of another Collection + op.create_check_constraint( + op.f("ck_project_private_revision"), "project", "revision_of IS NULL OR visibility = 'PRIVATE'" + ) + + +def downgrade(): + def remake_collection_id_and_collection_visibility_composite_foreign_key(table: str): + op.add_column( + table, + sa.Column( + "collection_visibility", + postgresql.ENUM("PRIVATE", "PUBLIC", name="collectionvisibility"), + autoincrement=False, + ), + ) + op.execute( + f""" + UPDATE {table} + SET collection_visibility = ( + SELECT visibility + FROM project + WHERE project.id = {table}.collection_id + ) + """ + ) + op.alter_column(table, "collection_visibility", nullable=False) + op.create_foreign_key( + f"{table}_collection_id_fkey", + table, + "project", + ["collection_id", "collection_visibility"], + ["id", "visibility"], + ) + + # Drop foreign keys for project.id + op.drop_constraint(op.f("fk_project_link_collection_id_project"), "project_link", type_="foreignkey") + op.drop_constraint(op.f("fk_project_revision_of_project"), "project", type_="foreignkey") + op.drop_constraint(op.f("fk_geneset_collection_id_project"), "geneset", type_="foreignkey") + op.drop_constraint(op.f("fk_dataset_collection_id_project"), "dataset", type_="foreignkey") + + # Drop project.id as primary key + op.drop_constraint(op.f("pk_project"), "project") + + # Reassign id for revisions + op.execute( + """ + UPDATE project + SET id = revision_of, visibility = 'PRIVATE' + WHERE revision_of IS NOT NULL + """ + ) + + # Remove project.revision_of + op.drop_constraint(op.f("ck_project_private_revision"), "project", type_="check") + op.drop_column("project", "revision_of") + + # Add project composite primary key + op.create_primary_key("project_pkey", "project", ["id", "visibility"]) + + # Create foreign keys that reference composite key [project.id, project.visibility] + remake_collection_id_and_collection_visibility_composite_foreign_key("project_link") + remake_collection_id_and_collection_visibility_composite_foreign_key("geneset") + remake_collection_id_and_collection_visibility_composite_foreign_key("dataset") + + # Reset unique constraint + op.drop_constraint("_geneset_name__collection_uc", "geneset", type_="unique") + op.create_unique_constraint( + "_geneset_name__collection_uc", "geneset", ["name", "collection_id", "collection_visibility"] + ) diff --git a/backend/wmg/data/extract.py b/backend/wmg/data/extract.py index d9ec4ecbcf8fc..003a9495be1b3 100644 --- a/backend/wmg/data/extract.py +++ b/backend/wmg/data/extract.py @@ -1,7 +1,7 @@ import subprocess from backend.corpora.common.corpora_orm import DatasetArtifactFileType -from backend.corpora.common.entities import Dataset, DatasetAsset +from backend.corpora.common.entities import Collection, Dataset, DatasetAsset from backend.corpora.common.utils.db_session import db_session_manager @@ -28,11 +28,12 @@ def get_dataset_s3_uris(): dataset_ids = [] published_dataset_non_null_assays = ( session.query(Dataset.table.id, Dataset.table.assay) + .join(Dataset.table.collection) .filter( Dataset.table.assay != "null", Dataset.table.published == "TRUE", Dataset.table.is_primary_data == "PRIMARY", - Dataset.table.collection_visibility == "PUBLIC", + Collection.table.visibility == "PUBLIC", Dataset.table.tombstone == "FALSE", ) .all() diff --git a/frontend/src/common/constants/routes.ts b/frontend/src/common/constants/routes.ts index ae79d30ed6e12..42ec952a8f8a2 100644 --- a/frontend/src/common/constants/routes.ts +++ b/frontend/src/common/constants/routes.ts @@ -4,7 +4,6 @@ export enum ROUTES { COLLECTION = "/collections/:id", COLLECTIONS = "/collections", DATASETS = "/datasets", - PRIVATE_COLLECTION = "/collections/:id/private", TOS = "/tos/", PRIVACY = "/privacy/", PREVIEW_POLICIES = "/previewpolicies/", diff --git a/frontend/src/common/entities.ts b/frontend/src/common/entities.ts index 45107c19ab3e5..9984c7bb7232d 100644 --- a/frontend/src/common/entities.ts +++ b/frontend/src/common/entities.ts @@ -97,11 +97,12 @@ export interface Collection { data_submission_policy_version: string; created_at: number; updated_at: number; - has_revision: boolean; publisher_metadata: PublisherMetadata; revision_diff: boolean; summaryCitation?: string; tombstone?: boolean; + revision_of?: Collection["id"]; + revisioning_in?: Collection["id"]; } /** diff --git a/frontend/src/common/queries/collections.ts b/frontend/src/common/queries/collections.ts index f4e306c86334c..53575cbfd5986 100644 --- a/frontend/src/common/queries/collections.ts +++ b/frontend/src/common/queries/collections.ts @@ -1,5 +1,6 @@ import { useMutation, + UseMutationResult, useQuery, useQueryClient, UseQueryResult, @@ -62,6 +63,7 @@ export interface CollectionResponse { id: string; created_at: number; visibility: VISIBILITY_TYPE; + revision_of?: string; } export interface RevisionResponse extends CollectionResponse { @@ -82,9 +84,10 @@ async function fetchCollections(): Promise { const collectionsMap: CollectionResponsesMap = new Map(); for (const collection of json.collections as CollectionResponse[]) { - const collectionsWithId = collectionsMap.get(collection.id) || new Map(); - collectionsWithId.set(collection.visibility, collection); - collectionsMap.set(collection.id, collectionsWithId); + const publicID = collection.revision_of || collection.id; + const collectionsWithID = collectionsMap.get(publicID) || new Map(); + collectionsWithID.set(collection.visibility, collection); + collectionsMap.set(publicID, collectionsWithID); } return collectionsMap; @@ -120,21 +123,15 @@ export interface TombstonedCollection { function fetchCollection(allCollections: CollectionResponsesMap | undefined) { return async function ( - id: string, - visibility: VISIBILITY_TYPE + id: string ): Promise { if (!id) { return null; } - const baseUrl = apiTemplateToUrl(API_URL + API.COLLECTION, { id }); + const url = apiTemplateToUrl(API_URL + API.COLLECTION, { id }); - const finalUrl = - visibility === VISIBILITY_TYPE.PRIVATE - ? baseUrl + `?visibility=${VISIBILITY_TYPE.PRIVATE}` - : baseUrl; - - let response = await fetch(finalUrl, DEFAULT_FETCH_OPTIONS); + let response = await fetch(url, DEFAULT_FETCH_OPTIONS); let json = await response.json(); if (response.status === HTTP_STATUS_CODE.GONE) { @@ -144,18 +141,26 @@ function fetchCollection(allCollections: CollectionResponsesMap | undefined) { throw json; } - const collection = { ...json, datasets: generateDatasetMap(json) }; + const collection: Collection = { + ...json, + datasets: generateDatasetMap(json), + }; let publishedCounterpart; - if (allCollections) { + if (allCollections && collection.visibility === VISIBILITY_TYPE.PUBLIC) { const collectionsWithID = allCollections.get(id); - collection.has_revision = collectionsWithID && collectionsWithID.size > 1; + collection.revisioning_in = collectionsWithID?.get( + VISIBILITY_TYPE.PRIVATE + )?.id; } - if (visibility === VISIBILITY_TYPE.PRIVATE && collection.has_revision) { - response = await fetch(baseUrl, DEFAULT_FETCH_OPTIONS); + if (collection.revision_of) { + const publicCollectionURL = apiTemplateToUrl(API_URL + API.COLLECTION, { + id: collection.revision_of, + }); + response = await fetch(publicCollectionURL, DEFAULT_FETCH_OPTIONS); json = await response.json(); if (response.ok) { @@ -167,7 +172,7 @@ function fetchCollection(allCollections: CollectionResponsesMap | undefined) { } // check for diffs between revision and published collection - if (collection.has_revision && visibility === VISIBILITY_TYPE.PRIVATE) { + if (collection.revision_of) { collection.revision_diff = checkForRevisionChange( collection, publishedCounterpart @@ -185,17 +190,15 @@ function fetchCollection(allCollections: CollectionResponsesMap | undefined) { export function useCollection({ id = "", - visibility = VISIBILITY_TYPE.PUBLIC, }: { id?: string; - visibility: VISIBILITY_TYPE; }): UseQueryResult { const { data: collections } = useCollections(); const queryFn = fetchCollection(collections); return useQuery( - [USE_COLLECTION, id, visibility, collections], - () => queryFn(id, visibility), + [USE_COLLECTION, id, collections], + () => queryFn(id), { enabled: !!collections, // TODO review use of refetch flag below on remove of filter flag (#1718). This is set to false as a fix for #2204 @@ -291,32 +294,26 @@ async function collectionUploadLinks({ return json.dataset_uuid; } -export function useCollectionUploadLinks( - id: string, - visibility: VISIBILITY_TYPE -) { +export function useCollectionUploadLinks(id: string) { const queryCache = useQueryClient(); return useMutation(collectionUploadLinks, { onSuccess: () => { - queryCache.invalidateQueries([USE_COLLECTION, id, visibility]); + queryCache.invalidateQueries([USE_COLLECTION, id]); }, }); } async function deleteCollection({ collectionID, - visibility = VISIBILITY_TYPE.PRIVATE, }: { collectionID: Collection["id"]; - visibility?: VISIBILITY_TYPE; }) { - const baseUrl = apiTemplateToUrl(API_URL + API.COLLECTION, { + const finalURL = apiTemplateToUrl(API_URL + API.COLLECTION, { id: collectionID, }); - const finalUrl = baseUrl + `?visibility=${visibility}`; - const response = await fetch(finalUrl, DELETE_FETCH_OPTIONS); + const response = await fetch(finalURL, DELETE_FETCH_OPTIONS); if (!response.ok) { throw await response.json(); @@ -325,10 +322,14 @@ async function deleteCollection({ export function useDeleteCollection( id = "", - visibility = VISIBILITY_TYPE.PRIVATE -) { + visibility = "" +): UseMutationResult< + void, + unknown, + { collectionID: string }, + { previousCollections: CollectionResponsesMap } +> { const queryClient = useQueryClient(); - return useMutation(deleteCollection, { onError: ( _, @@ -364,7 +365,7 @@ export function useDeleteCollection( onSuccess: () => { return Promise.all([ queryClient.invalidateQueries([USE_COLLECTIONS]), - queryClient.removeQueries([USE_COLLECTION, id, visibility], { + queryClient.removeQueries([USE_COLLECTION, id], { exact: false, }), ]); @@ -379,7 +380,7 @@ export type PublishCollection = { async function publishCollection({ id, payload }: PublishCollection) { const url = apiTemplateToUrl(API_URL + API.COLLECTION_PUBLISH, { id }); - + console.log("attempting to publish via API call"); const response = await fetch(url, { ...DEFAULT_FETCH_OPTIONS, ...JSON_BODY_FETCH_OPTIONS, @@ -390,16 +391,17 @@ async function publishCollection({ id, payload }: PublishCollection) { if (!response.ok) { throw await response.json(); } + return id; } export function usePublishCollection() { const queryClient = useQueryClient(); return useMutation(publishCollection, { - onSuccess: () => { - // (thuang): We don't need to invalidate `[USE_COLLECTION, id, visibility]` - // because `visibility` has changed from PRIVATE to PUBLIC + onSuccess: (id) => { + console.log("Completed publish mutation"); queryClient.invalidateQueries([USE_COLLECTIONS]); + queryClient.invalidateQueries([USE_COLLECTION, id]); }, }); } @@ -443,22 +445,31 @@ const editCollection = async function ({ }; }; -export function useEditCollection(collectionID?: Collection["id"]) { +export function useEditCollection( + collectionID?: Collection["id"], + publicID?: Collection["id"] +): UseMutationResult< + CollectionEditResponse, + unknown, + { id: string; payload: string }, + unknown +> { const queryClient = useQueryClient(); - const { data: collections } = useCollections(); + const { data: collections } = useCollections(); //all collections const { data: collection } = useCollection({ + //revision id: collectionID, - visibility: VISIBILITY_TYPE.PRIVATE, }); const { data: publishedCollection } = useCollection({ - id: collectionID, - visibility: VISIBILITY_TYPE.PUBLIC, + //published collection + id: publicID, }); return useMutation(editCollection, { + // newCollection is the result of the PUT on the revision onSuccess: ({ collection: newCollection }) => { // Check for updated collection: it's possible server-side validation errors have occurred where the error has // been swallowed (allowing error messages to be displayed on the edit form) and success flow is executed even @@ -467,7 +478,7 @@ export function useEditCollection(collectionID?: Collection["id"]) { return; } queryClient.setQueryData( - [USE_COLLECTION, collectionID, VISIBILITY_TYPE.PRIVATE, collections], + [USE_COLLECTION, collectionID, collections], () => { let revision_diff; if (isTombstonedCollection(newCollection)) { @@ -475,7 +486,7 @@ export function useEditCollection(collectionID?: Collection["id"]) { } else if ( !isTombstonedCollection(collection) && !isTombstonedCollection(publishedCollection) && - collection?.has_revision && + collection?.revision_of && publishedCollection ) { revision_diff = checkForRevisionChange( @@ -485,6 +496,7 @@ export function useEditCollection(collectionID?: Collection["id"]) { return { ...collection, ...newCollection, revision_diff }; } + return { ...collection, ...newCollection }; } ); }, @@ -504,14 +516,14 @@ const createRevision = async function (id: string) { return response.json(); }; -export function useCreateRevision(callback: () => void) { +export function useCreateRevision(callback: (id: Collection["id"]) => void) { const queryClient = useQueryClient(); return useMutation(createRevision, { onSuccess: async (collection: Collection) => { await queryClient.invalidateQueries([USE_COLLECTIONS]); await queryClient.invalidateQueries([USE_COLLECTION, collection.id]); - callback(); + callback(collection.id); }, }); } @@ -547,16 +559,14 @@ const reuploadDataset = async function ({ return result.dataset_uuid; }; -export function useReuploadDataset(collectionId: string) { +export function useReuploadDataset( + collectionId: string +): UseMutationResult { const queryClient = useQueryClient(); return useMutation(reuploadDataset, { onSuccess: () => { - queryClient.invalidateQueries([ - USE_COLLECTION, - collectionId, - VISIBILITY_TYPE.PRIVATE, - ]); + queryClient.invalidateQueries([USE_COLLECTION, collectionId]); }, }); } diff --git a/frontend/src/common/utils/checkForRevisionChange.ts b/frontend/src/common/utils/checkForRevisionChange.ts index e68fab62ab4a2..7288484da7209 100644 --- a/frontend/src/common/utils/checkForRevisionChange.ts +++ b/frontend/src/common/utils/checkForRevisionChange.ts @@ -4,10 +4,13 @@ import xorWith from "lodash/xorWith"; import { Collection, Dataset } from "../entities"; const IGNORED_COLLECTION_FIELDS = [ + "id", "visibility", "created_at", "updated_at", - "has_revision", + "revisioning_in", + "revision_of", + "id", "revision_diff", "datasets", "genesets", @@ -19,6 +22,7 @@ const IGNORED_DATASET_FIELDS = [ "collection_visibility", "original_uuid", "id", + "collection_id", "processing_status", "dataset_assets", "dataset_deployments", diff --git a/frontend/src/components/Collection/components/CollectionDatasetsGrid/components/DatasetsGrid/index.tsx b/frontend/src/components/Collection/components/CollectionDatasetsGrid/components/DatasetsGrid/index.tsx index 4047d5dc1f759..e6ad7d13845a3 100644 --- a/frontend/src/components/Collection/components/CollectionDatasetsGrid/components/DatasetsGrid/index.tsx +++ b/frontend/src/components/Collection/components/CollectionDatasetsGrid/components/DatasetsGrid/index.tsx @@ -1,10 +1,10 @@ -/* Copied from src/components/Collections/components/Grid/components/DatasetsGrid and modified as an intermediate +/* Copied from src/components/Collections/components/Grid/components/DatasetsGrid and modified as an intermediate upgrade to the collection datasets table while keeping the existing core datasets grid outside of the filter feature flag untouched. Once filter feature flag is removed, the existing core datasets grid can be deleted and replaced with this version. Ideally collection datasets table should be moved to react-table but requires changes to how dataset information (statuses etc) is calculated and rolled out. */ import { FC } from "react"; -import { MutateFunction } from "react-query"; +import { UseMutateAsyncFunction } from "react-query"; import { Collection, Dataset, VISIBILITY_TYPE } from "src/common/entities"; import { ReuploadLink } from "src/common/queries/collections"; import DatasetRow from "src/components/Collection/components/CollectionDatasetsGrid/components/Row/DatsetRow"; @@ -22,10 +22,20 @@ interface Props { accessType?: Collection["access_type"]; isRevision: boolean; onUploadFile: ( - reuploadDataset?: MutateFunction, + reuploadDataset?: UseMutateAsyncFunction< + unknown, + unknown, + ReuploadLink, + unknown + >, datasetId?: string ) => ChooserProps["onUploadFile"]; - reuploadDataset: MutateFunction; + reuploadDataset: UseMutateAsyncFunction< + unknown, + unknown, + ReuploadLink, + unknown + >; } const DatasetsGrid: FC = ({ diff --git a/frontend/src/components/Collection/components/CollectionDatasetsGrid/components/Row/DatsetRow/index.tsx b/frontend/src/components/Collection/components/CollectionDatasetsGrid/components/Row/DatsetRow/index.tsx index 1a709597dd45b..3f64960ae812a 100644 --- a/frontend/src/components/Collection/components/CollectionDatasetsGrid/components/Row/DatsetRow/index.tsx +++ b/frontend/src/components/Collection/components/CollectionDatasetsGrid/components/Row/DatsetRow/index.tsx @@ -6,13 +6,14 @@ import { Intent, Tooltip } from "@blueprintjs/core"; import loadable from "@loadable/component"; import { FC } from "react"; -import { CancelledError, useQueryClient } from "react-query"; +import { CancelledError, useQueryClient, UseQueryResult } from "react-query"; import { PLURALIZED_METADATA_LABEL } from "src/common/constants/metadata"; import { ACCESS_TYPE, Collection, CONVERSION_STATUS, Dataset, + DatasetUploadStatus, UPLOAD_STATUS, VALIDATION_STATUS, VISIBILITY_TYPE, @@ -69,6 +70,17 @@ const ErrorTooltip = ({ isFailed, error, type }: FailReturn) => { return ; }; +const handlePossibleError = ( + datasetStatusResult: UseQueryResult +) => { + if ( + datasetStatusResult.isError && + !(datasetStatusResult.error instanceof CancelledError) + ) { + console.error(datasetStatusResult.error); + } +}; + interface Props { dataset: Dataset; file?: UploadingFile; @@ -79,7 +91,6 @@ interface Props { onUploadFile: ChooserProps["onUploadFile"]; } -/* eslint-disable sonarjs/cognitive-complexity */ const DatasetRow: FC = ({ dataset, file, @@ -102,12 +113,7 @@ const DatasetRow: FC = ({ const { upload_progress } = datasetStatus; - if ( - datasetStatusResult.isError && - !(datasetStatusResult.error instanceof CancelledError) - ) { - console.error(datasetStatusResult.error); - } + handlePossibleError(datasetStatusResult); const isNamePopulated = Boolean(dataset.name); diff --git a/frontend/src/components/Collections/components/DeleteCollection/index.tsx b/frontend/src/components/Collections/components/DeleteCollection/index.tsx index 941205e1b64c6..92b2670b404e3 100644 --- a/frontend/src/components/Collections/components/DeleteCollection/index.tsx +++ b/frontend/src/components/Collections/components/DeleteCollection/index.tsx @@ -15,14 +15,19 @@ interface Props { id: Collection["id"]; Button?: React.ElementType; isRevision: boolean; + visibility: Collection["visibility"]; } const DeleteCollection: FC = ({ id, Button = RawButton, isRevision, + visibility, }) => { - const { mutateAsync: deleteMutation, isLoading } = useDeleteCollection(id); + const { mutateAsync: deleteMutation, isLoading } = useDeleteCollection( + id, + visibility + ); const router = useRouter(); const handleDelete = async () => { diff --git a/frontend/src/components/Collections/components/Grid/components/DatasetsGrid/index.tsx b/frontend/src/components/Collections/components/Grid/components/DatasetsGrid/index.tsx index 8fde2da2d29b2..296ecbc3b8daa 100644 --- a/frontend/src/components/Collections/components/Grid/components/DatasetsGrid/index.tsx +++ b/frontend/src/components/Collections/components/Grid/components/DatasetsGrid/index.tsx @@ -1,5 +1,5 @@ import { FC } from "react"; -import { MutateFunction } from "react-query"; +import { UseMutateAsyncFunction } from "react-query"; import { Collection, Dataset, VISIBILITY_TYPE } from "src/common/entities"; import { ReuploadLink } from "src/common/queries/collections"; import { @@ -20,10 +20,20 @@ interface Props { accessType?: Collection["access_type"]; isRevision: boolean; onUploadFile: ( - reuploadDataset?: MutateFunction, + reuploadDataset?: UseMutateAsyncFunction< + unknown, + unknown, + ReuploadLink, + unknown + >, datasetId?: string ) => ChooserProps["onUploadFile"]; - reuploadDataset: MutateFunction; + reuploadDataset: UseMutateAsyncFunction< + unknown, + unknown, + ReuploadLink, + unknown + >; } const DatasetsGrid: FC = ({ diff --git a/frontend/src/components/Collections/components/Grid/components/Row/CollectionRow/index.tsx b/frontend/src/components/Collections/components/Grid/components/Row/CollectionRow/index.tsx index 8f1082f28511d..1a11075ced525 100644 --- a/frontend/src/components/Collections/components/Grid/components/Row/CollectionRow/index.tsx +++ b/frontend/src/components/Collections/components/Grid/components/Row/CollectionRow/index.tsx @@ -5,7 +5,7 @@ import { useRouter } from "next/router"; import { FC } from "react"; import { PLURALIZED_METADATA_LABEL } from "src/common/constants/metadata"; import { ROUTES } from "src/common/constants/routes"; -import { ACCESS_TYPE, VISIBILITY_TYPE } from "src/common/entities"; +import { ACCESS_TYPE, Collection, VISIBILITY_TYPE } from "src/common/entities"; import { useCollection, useCreateRevision, @@ -49,13 +49,12 @@ const conditionalPopover = ( const CollectionRow: FC = (props) => { const { data: collection } = useCollection({ id: props.id, - visibility: props.visibility, }); const router = useRouter(); - const navigateToRevision = () => { - router.push(ROUTES.PRIVATE_COLLECTION.replace(":id", id)); + const navigateToRevision = (id: Collection["id"]) => { + router.push(ROUTES.COLLECTION.replace(":id", id)); }; const { mutate, isLoading } = useCreateRevision(navigateToRevision); @@ -63,10 +62,10 @@ const CollectionRow: FC = (props) => { if (!collection || isTombstonedCollection(collection)) return null; const handleRevisionClick = () => { - if (collection?.has_revision === false) { + if (!collection?.revisioning_in) { mutate(id); } else { - navigateToRevision(); + navigateToRevision(collection?.revisioning_in || collection.id); } }; @@ -85,10 +84,7 @@ const CollectionRow: FC = (props) => { return ( - + {name} @@ -103,7 +99,7 @@ const CollectionRow: FC = (props) => { > {isPrivate ? "Private" : "Published"} - {props.revisionsEnabled && collection.has_revision && ( + {props.revisionsEnabled && collection.revisioning_in && ( Revision Pending @@ -118,7 +114,7 @@ const CollectionRow: FC = (props) => { {cell_count || "-"} {props.revisionsEnabled && visibility === VISIBILITY_TYPE.PUBLIC ? ( @@ -130,13 +126,13 @@ const CollectionRow: FC = (props) => { }; const RevisionCell = ({ - isRevision, handleRevisionClick, isLoading, + revisionId, }: { - isRevision?: boolean; handleRevisionClick: () => void; isLoading: boolean; + revisionId: Collection["revisioning_in"]; }) => { return ( @@ -147,7 +143,7 @@ const RevisionCell = ({ onClick={handleRevisionClick} data-test-id="revision-action-button" > - {isRevision ? "Continue" : "Start Revision"} + {revisionId ? "Continue" : "Start Revision"} ); diff --git a/frontend/src/components/Collections/components/PublishCollection/index.tsx b/frontend/src/components/Collections/components/PublishCollection/index.tsx index d8be56fa4333b..23dced310d9b8 100644 --- a/frontend/src/components/Collections/components/PublishCollection/index.tsx +++ b/frontend/src/components/Collections/components/PublishCollection/index.tsx @@ -20,13 +20,13 @@ const AsyncAlert = loadable( interface Props { id: Collection["id"]; isPublishable: boolean; - isRevision: boolean; + revisionOf: Collection["revision_of"]; } const PublishCollection: FC = ({ id = "", isPublishable, - isRevision, + revisionOf, }) => { const [isOpen, setIsOpen] = useState(false); const { mutateAsync: publish, isSuccess, isLoading } = usePublishCollection(); @@ -35,7 +35,11 @@ const PublishCollection: FC = ({ const PublishButton = isFilterEnabled ? StyledPrimaryButton : Button; if (isSuccess) { - router.push(ROUTES.COLLECTION.replace(":id", id)); + console.log( + "IS SUCCESS, used to be private collection redirect, now just refreshes data?" + ); + if (revisionOf) router.push(ROUTES.COLLECTION.replace(":id", revisionOf)); + else router.reload(); } const toggleAlert = () => setIsOpen(!isOpen); @@ -48,8 +52,9 @@ const PublishCollection: FC = ({ { id, payload }, { onSuccess: () => { - //if revision show revision toast - if (isRevision) { + //if revision show revision toast + if (revisionOf) { + console.log("Published a revision"); Toast.show({ icon: IconNames.TICK, intent: Intent.SUCCESS, @@ -85,7 +90,7 @@ const PublishCollection: FC = ({ = ({ onConfirm={handleConfirm} loading={isLoading} > - {isRevision ? ( + {revisionOf ? ( <>
Are you sure you want to publish a revision to this collection? diff --git a/frontend/src/components/CreateCollectionModal/components/Content/index.tsx b/frontend/src/components/CreateCollectionModal/components/Content/index.tsx index e729d9b91233a..b4ae8c463884c 100644 --- a/frontend/src/components/CreateCollectionModal/components/Content/index.tsx +++ b/frontend/src/components/CreateCollectionModal/components/Content/index.tsx @@ -2,11 +2,7 @@ import { Button, Classes, Intent } from "@blueprintjs/core"; import { useRouter } from "next/router"; import { FC, Fragment, useEffect, useRef, useState } from "react"; import { ROUTES } from "src/common/constants/routes"; -import { - Collection, - COLLECTION_LINK_TYPE, - VISIBILITY_TYPE, -} from "src/common/entities"; +import { Collection, COLLECTION_LINK_TYPE } from "src/common/entities"; import { FEATURES } from "src/common/featureFlags/features"; import { useFeatureFlag } from "src/common/hooks/useFeatureFlag"; import { useUserInfo } from "src/common/queries/auth"; @@ -95,6 +91,7 @@ const emailValidation = (value: string) => { return emailRegex.test(value) || "Invalid email"; }; +// eslint-disable-next-line sonarjs/cognitive-complexity const Content: FC = (props) => { const isEditCollection = !!props.id; const initialBooleanState = isEditCollection; @@ -127,11 +124,16 @@ const Content: FC = (props) => { let { data } = useCollection({ id: props.id, - visibility: VISIBILITY_TYPE.PRIVATE, }); + const publishedID = + data && "revision_of" in data ? data.revision_of : undefined; + const { mutateAsync: mutateCreateCollection } = useCreateCollection(); - const { mutateAsync: mutateEditCollection } = useEditCollection(props.id); + const { mutateAsync: mutateEditCollection } = useEditCollection( + props.id, + publishedID + ); // Null / tombstone checking is type safety netting. We shouldn't be getting to these lines/cases since we can't open the modal if the collection is tombstoned/doesn't exist. if (isTombstonedCollection(data)) data = null; @@ -362,7 +364,7 @@ const Content: FC = (props) => { } if (collectionId) { - router.push(ROUTES.PRIVATE_COLLECTION.replace(":id", collectionId)); + router.push(ROUTES.COLLECTION.replace(":id", collectionId)); } } diff --git a/frontend/src/views/Collection/components/ActionButtons/components/MoreDropdown/components/Menu/index.tsx b/frontend/src/views/Collection/components/ActionButtons/components/MoreDropdown/components/Menu/index.tsx index 77edc91c28632..66ac9267bdcdf 100644 --- a/frontend/src/views/Collection/components/ActionButtons/components/MoreDropdown/components/Menu/index.tsx +++ b/frontend/src/views/Collection/components/ActionButtons/components/MoreDropdown/components/Menu/index.tsx @@ -5,6 +5,7 @@ import { MenuItem, } from "@blueprintjs/core"; import { IconNames } from "@blueprintjs/icons"; +import { Collection } from "src/common/entities"; import DeleteCollection from "src/components/Collections/components/DeleteCollection"; import CreateCollection from "src/components/CreateCollectionModal"; import styled from "styled-components"; @@ -43,6 +44,7 @@ const EditButton = (props: Partial) => { interface Props { id?: string; isRevision: boolean; + visibility: Collection["visibility"]; } const StyledMenu = styled(RawMenu)` @@ -53,11 +55,16 @@ const StyledMenu = styled(RawMenu)` } `; -const Menu = ({ id = "", isRevision }: Props) => { +const Menu = ({ id = "", isRevision, visibility }: Props) => { return ( - + ); }; diff --git a/frontend/src/views/Collection/components/ActionButtons/components/MoreDropdown/index.tsx b/frontend/src/views/Collection/components/ActionButtons/components/MoreDropdown/index.tsx index ba22b26325445..b6cf8f01a6f93 100644 --- a/frontend/src/views/Collection/components/ActionButtons/components/MoreDropdown/index.tsx +++ b/frontend/src/views/Collection/components/ActionButtons/components/MoreDropdown/index.tsx @@ -1,17 +1,19 @@ import { Position } from "@blueprintjs/core"; import { useMemo } from "react"; +import { Collection } from "src/common/entities"; import RawMoreDropdown from "src/components/common/MoreDropdown"; import Menu from "./components/Menu"; interface Props { id: string; isRevision: boolean; + visibility: Collection["visibility"]; } -const MoreDropdown = ({ id = "", isRevision }: Props) => { +const MoreDropdown = ({ id = "", isRevision, visibility }: Props) => { const popoverProps = useMemo(() => { return { - content: , + content: , position: Position.BOTTOM, }; }, [id, isRevision]); diff --git a/frontend/src/views/Collection/components/ActionButtons/index.tsx b/frontend/src/views/Collection/components/ActionButtons/index.tsx index 2e68c77d2fc68..e4ad44568ebcd 100644 --- a/frontend/src/views/Collection/components/ActionButtons/index.tsx +++ b/frontend/src/views/Collection/components/ActionButtons/index.tsx @@ -17,25 +17,27 @@ interface Props { addNewFile: DropboxChooserProps["onUploadFile"]; id: Collection["id"]; isPublishable: boolean; - isRevision: boolean; + revisionOf: Collection["revision_of"]; + visibility: Collection["visibility"]; } const ActionButtons = ({ addNewFile, id, isPublishable, - isRevision, + revisionOf, + visibility, }: Props): JSX.Element => { const isFilterEnabled = useFeatureFlag(FEATURES.FILTER); const Actions = isFilterEnabled ? CollectionActions : Wrapper; return ( - + ); diff --git a/frontend/src/views/Collection/components/DatasetTab/index.tsx b/frontend/src/views/Collection/components/DatasetTab/index.tsx index 7cc1ef7d0d1ca..5d11003af540e 100644 --- a/frontend/src/views/Collection/components/DatasetTab/index.tsx +++ b/frontend/src/views/Collection/components/DatasetTab/index.tsx @@ -2,12 +2,11 @@ import { Button, Intent, UL } from "@blueprintjs/core"; import { IconNames } from "@blueprintjs/icons"; import memoize from "lodash/memoize"; import { FC, useState } from "react"; -import { MutateFunction, useQueryClient } from "react-query"; +import { useQueryClient } from "react-query"; import { Collection, Dataset } from "src/common/entities"; import { FEATURES } from "src/common/featureFlags/features"; import { useFeatureFlag } from "src/common/hooks/useFeatureFlag"; import { - ReuploadLink, useCollection, useCollectionUploadLinks, useReuploadDataset, @@ -38,13 +37,10 @@ const DatasetTab: FC = ({ const CLI_README_LINK = "https://github.com/chanzuckerberg/single-cell-curation/blob/main/readme.md"; - const { mutateAsync: uploadLink } = useCollectionUploadLinks( - collectionId, - visibility - ); + const { mutateAsync: uploadLink } = useCollectionUploadLinks(collectionId); const { mutateAsync: reuploadDataset } = useReuploadDataset(collectionId); const [uploadedFiles, setUploadedFiles] = useState({} as UploadedFiles); - const { data: collection } = useCollection({ id: collectionId, visibility }); + const { data: collection } = useCollection({ id: collectionId }); const isFilterEnabled = useFeatureFlag(FEATURES.FILTER); const queryClient = useQueryClient(); @@ -61,14 +57,7 @@ const DatasetTab: FC = ({ () => collectionId + visibility ); - const addNewFile = ( - mutationFunction = uploadLink as MutateFunction< - string, - unknown, - ReuploadLink - >, - originalId?: string - ) => { + const addNewFile = (mutationFunction = uploadLink, originalId?: string) => { return (newFile: UploadingFile) => { if (!newFile.link) return; diff --git a/frontend/src/views/Collection/index.tsx b/frontend/src/views/Collection/index.tsx index a52d47e37f7d8..ac3b1af03acae 100644 --- a/frontend/src/views/Collection/index.tsx +++ b/frontend/src/views/Collection/index.tsx @@ -60,26 +60,19 @@ const Collection: FC = () => { const [userWithdrawn, setUserWithdrawn] = useState(false); let id = ""; - let isPrivate = false; if (Array.isArray(params)) { id = params[0]; - isPrivate = params[1] === "private"; } else if (params) { id = params; } - const visibility = isPrivate - ? VISIBILITY_TYPE.PRIVATE - : VISIBILITY_TYPE.PUBLIC; - - const { mutateAsync: uploadLink } = useCollectionUploadLinks(id, visibility); + const { mutateAsync: uploadLink } = useCollectionUploadLinks(id); const [isUploadingLink, setIsUploadingLink] = useState(false); const collectionState = useCollection({ id, - visibility, }); const [hasShownWithdrawToast, setHasShownWithdrawToast] = useState(false); @@ -88,7 +81,9 @@ const Collection: FC = () => { const { mutateAsync: deleteMutation, isLoading } = useDeleteCollection( id, - visibility + collection && "visibility" in collection + ? collection.visibility + : VISIBILITY_TYPE.PRIVATE ); const isCurator = get(FEATURES.CURATOR) === BOOLEAN.TRUE; @@ -132,7 +127,9 @@ const Collection: FC = () => { return null; } - const isRevision = isCurator && !!collection?.has_revision; + const isPrivate = collection.visibility === VISIBILITY_TYPE.PRIVATE; + + const isRevision = isCurator && !!collection?.revision_of; const addNewFile = (newFile: UploadingFile) => { if (!newFile.link) return; @@ -178,7 +175,7 @@ const Collection: FC = () => { const shouldShowPrivateWriteAction = hasWriteAccess && isPrivate; const shouldShowPublicWriteAction = hasWriteAccess && !isPrivate; const shouldShowCollectionRevisionCallout = - collection.has_revision && visibility === VISIBILITY_TYPE.PRIVATE; + collection.revision_of && isPrivate; // TODO update to use buildCollectionMetadataLinks once filter feature flag is removed (#1718). const collectionMetadataLinksFn = isFilterEnabled ? buildCollectionMetadataLinks @@ -195,7 +192,6 @@ const Collection: FC = () => { await deleteMutation({ collectionID: id, - visibility: VISIBILITY_TYPE.PUBLIC, }); router.push(ROUTES.MY_COLLECTIONS); @@ -226,7 +222,8 @@ const Collection: FC = () => { id={id} addNewFile={addNewFile} isPublishable={isPublishable} - isRevision={isRevision} + revisionOf={collection.revision_of} + visibility={collection.visibility} /> )} {shouldShowPublicWriteAction && ( @@ -254,12 +251,12 @@ const Collection: FC = () => { collectionID={id} datasets={datasets} isRevision={isRevision} - visibility={visibility} + visibility={collection.visibility} /> ) : ( - {collection.has_revision && visibility === VISIBILITY_TYPE.PRIVATE && ( + {collection.revision_of && isPrivate && ( {collection.revision_diff @@ -293,7 +290,8 @@ const Collection: FC = () => { id={id} addNewFile={addNewFile} isPublishable={isPublishable} - isRevision={isRevision} + revisionOf={collection.revision_of} + visibility={collection.visibility} /> )} {hasWriteAccess && !isPrivate && ( @@ -318,7 +316,7 @@ const Collection: FC = () => { } diff --git a/frontend/src/views/Collection/utils.tsx b/frontend/src/views/Collection/utils.tsx index e001843744cc7..2e9e5459a23d5 100644 --- a/frontend/src/views/Collection/utils.tsx +++ b/frontend/src/views/Collection/utils.tsx @@ -323,7 +323,7 @@ function buildCollectionMetadataLink( function isPrivateRevision(collection: Collection) { return ( - collection.visibility === VISIBILITY_TYPE.PRIVATE && collection.has_revision + collection.visibility === VISIBILITY_TYPE.PRIVATE && collection.revision_of ); } diff --git a/tests/unit/backend/corpora/api_server/collection_uuid/test_publish.py b/tests/unit/backend/corpora/api_server/collection_uuid/test_publish.py index d2bd65c3fed39..43509dce60f58 100644 --- a/tests/unit/backend/corpora/api_server/collection_uuid/test_publish.py +++ b/tests/unit/backend/corpora/api_server/collection_uuid/test_publish.py @@ -2,7 +2,8 @@ from datetime import datetime from unittest.mock import Mock, patch -from backend.corpora.common.corpora_orm import CollectionVisibility, CollectionLinkType +from backend.corpora.common.corpora_orm import CollectionLinkType +from backend.corpora.common.entities import Collection from tests.unit.backend.corpora.api_server.base_api_test import BaseAuthAPITest, BasicAuthAPITestCurator from tests.unit.backend.corpora.api_server.mock_auth import get_auth_token @@ -21,7 +22,9 @@ def setUp(self): self.headers_unauthed = {"host": "localhost", "Content-Type": "application/json"} self.mock_published_at = datetime(2000, 12, 25, 0, 0) - def verify_publish_collection(self, collection_id: str, mock_timestamp: datetime = None) -> dict: + def verify_publish_collection( + self, pub_collection_id: str, id_to_publish: str = None, mock_timestamp: datetime = None + ) -> dict: """ Verify publish collection. :return: Jsonified response of GET collection/. @@ -29,16 +32,18 @@ def verify_publish_collection(self, collection_id: str, mock_timestamp: datetime if not mock_timestamp: mock_timestamp = self.mock_published_at + id_to_publish = id_to_publish if id_to_publish else pub_collection_id + # Publish collection body = {"data_submission_policy_version": "1.0"} - path = f"{self.base_path}/{collection_id}/publish" + path = f"{self.base_path}/{id_to_publish}/publish" with patch("backend.corpora.common.entities.collection.datetime") as mock_dt: mock_dt.utcnow = Mock(return_value=mock_timestamp) response = self.app.post(path, headers=self.headers_authed, data=json.dumps(body)) self.assertEqual(202, response.status_code) - self.assertDictEqual({"collection_uuid": collection_id, "visibility": "PUBLIC"}, json.loads(response.data)) - self.addCleanup(self.delete_collection, collection_id, "PUBLIC") + self.assertDictEqual({"collection_uuid": pub_collection_id, "visibility": "PUBLIC"}, json.loads(response.data)) + self.addCleanup(self.delete_collection, pub_collection_id) # Cannot call publish for an already published collection response = self.app.post(path, headers=self.headers_authed, data=json.dumps(body)) @@ -49,26 +54,26 @@ def verify_publish_collection(self, collection_id: str, mock_timestamp: datetime self.assertEqual(200, response.status_code) ids = [col["id"] for col in json.loads(response.data)["collections"]] - self.assertIn(collection_id, ids) + self.assertIn(pub_collection_id, ids) # Check GET collection/ - path = f"{self.base_path}/{collection_id}" + path = f"{self.base_path}/{pub_collection_id}" response = self.app.get(path, headers=self.headers_unauthed) self.assertEqual(200, response.status_code) response_json = json.loads(response.data) self.assertEqual("PUBLIC", response_json["visibility"]) - self.assertEqual(collection_id, response_json["id"]) + self.assertEqual(pub_collection_id, response_json["id"]) return response_json def test__publish_a_new_collection__OK(self): """Publish a new collection with a single dataset.""" collection = self.generate_collection(self.session) - self.generate_dataset(self.session, collection_id=collection.id, collection_visibility=collection.visibility) + self.generate_dataset(self.session, collection_id=collection.id) self.assertIsNone(collection.published_at) - response_json = self.verify_publish_collection(collection.id, self.mock_published_at) + response_json = self.verify_publish_collection(collection.id, mock_timestamp=self.mock_published_at) # Check collection published_at self.assertEqual(self.mock_published_at, datetime.utcfromtimestamp(response_json["published_at"])) @@ -80,10 +85,10 @@ def test__published_at_not_updated_for_publish_a_collection_revision(self): """Publish a collection revision should not update published_at timestamp.""" # Publish a new collection collection = self.generate_collection(self.session) - self.generate_dataset(self.session, collection_id=collection.id, collection_visibility=collection.visibility) + self.generate_dataset(self.session, collection_id=collection.id) self.assertIsNone(collection.published_at) - response_json = self.verify_publish_collection(collection.id, self.mock_published_at) + response_json = self.verify_publish_collection(collection.id, mock_timestamp=self.mock_published_at) self.assertEqual(self.mock_published_at, datetime.utcfromtimestamp(response_json["published_at"])) # Start a revision of the collection @@ -91,11 +96,14 @@ def test__published_at_not_updated_for_publish_a_collection_revision(self): self.assertEqual(201, response.status_code) response_json = json.loads(response.data) + revision_id = response_json["id"] self.assertEqual("PRIVATE", response_json["visibility"]) # Publish revision mock_revision_published_dt = datetime(2001, 1, 25, 0, 0) - response_json = self.verify_publish_collection(collection.id, mock_revision_published_dt) + response_json = self.verify_publish_collection( + collection.id, id_to_publish=revision_id, mock_timestamp=mock_revision_published_dt + ) # published_at should not be updated - only updates on initial publish self.assertEqual(self.mock_published_at, datetime.utcfromtimestamp(response_json["published_at"])) @@ -107,12 +115,8 @@ def test__publish_collection_w_multiple_datasets__OK(self): """Publish collection with multiple datasets.""" collection_id = self.generate_collection(self.session).id dataset_ids = [ - self.generate_dataset( - self.session, collection_id=collection_id, collection_visibility=CollectionVisibility.PRIVATE.name - ).id, - self.generate_dataset( - self.session, collection_id=collection_id, collection_visibility=CollectionVisibility.PRIVATE.name - ).id, + self.generate_dataset(self.session, collection_id=collection_id).id, + self.generate_dataset(self.session, collection_id=collection_id).id, ] response_json = self.verify_publish_collection(collection_id) @@ -127,6 +131,17 @@ def test__publish_collection_w_multiple_datasets__OK(self): self.assertTrue(dataset["published"]) self.assertEqual(self.mock_published_at, datetime.utcfromtimestamp(dataset["published_at"])) + def verify_publish_collection_with_links(self, collection: Collection, id_to_publish: str = None): + link_names = [link.link_name for link in collection.links] + self.generate_dataset(self.session, collection_id=collection.id, published_at=self.mock_published_at).id + + response_json = self.verify_publish_collection(collection.id, id_to_publish=id_to_publish) + self.assertEqual(self.mock_published_at, datetime.utcfromtimestamp(response_json["published_at"])) + + # Check links + res_links = [link["link_name"] for link in response_json["links"]] + self.assertListEqual(sorted(link_names), sorted(res_links)) + def test__publish_collection_with_links__OK(self): """Publish collection with a link.""" collection = self.generate_collection( @@ -134,31 +149,25 @@ def test__publish_collection_with_links__OK(self): links=[ {"link_name": "test_link", "link_type": CollectionLinkType.PROTOCOL, "link_url": "https://link.link"} ], + published_at=self.mock_published_at, ) - link_names = [link.link_name for link in collection.links] - dataset_id = self.generate_dataset( - self.session, collection_id=collection.id, collection_visibility=CollectionVisibility.PRIVATE.name - ).id + self.verify_publish_collection_with_links(collection) - response_json = self.verify_publish_collection(collection.id) - self.assertEqual(self.mock_published_at, datetime.utcfromtimestamp(response_json["published_at"])) + def test__publish_collection_revision_with_links__OK(self): + revision = Collection.get_collection(self.session, revision_of="test_collection_id") + collection = Collection.get_collection(self.session, collection_uuid="test_collection_id") + collection.update(published_at=self.mock_published_at) - dataset = response_json["datasets"][0] - self.assertEqual(dataset_id, dataset["id"]) - self.assertEqual(self.mock_published_at, datetime.utcfromtimestamp(dataset["published_at"])) - - # Check links - res_links = [link["link_name"] for link in response_json["links"]] - self.assertListEqual(sorted(link_names), sorted(res_links)) + self.verify_publish_collection_with_links(collection, revision.id) @patch("backend.corpora.common.utils.cloudfront.create_invalidation_for_index_paths") def test_publish_collection_does_cloudfront_invalidation(self, mock_cloudfront): """Publish a new collection with a single dataset.""" collection = self.generate_collection(self.session) - self.generate_dataset(self.session, collection_id=collection.id, collection_visibility=collection.visibility) + self.generate_dataset(self.session, collection_id=collection.id) self.assertIsNone(collection.published_at) - self.verify_publish_collection(collection.id, self.mock_published_at) + self.verify_publish_collection(collection.id, mock_timestamp=self.mock_published_at) mock_cloudfront.assert_called_once() @@ -180,20 +189,18 @@ def test__bad_uuid__403(self): class TestPublishCurators(BasicAuthAPITestCurator): - def test__can_publish_owned_collection(self): - collection_id = self.generate_collection(self.session).id - self.generate_dataset(self.session, collection_id=collection_id, collection_visibility="PRIVATE") - path = f"/dp/v1/collections/{collection_id}/publish" + def verify_can_publish(self, collection): + self.generate_dataset(self.session, collection_id=collection.id) + path = f"/dp/v1/collections/{collection.id}/publish" body = {"data_submission_policy_version": "1.0"} headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} response = self.app.post(path, headers=headers, data=json.dumps(body)) self.assertEqual(202, response.status_code) + def test__can_publish_owned_collection(self): + collection = self.generate_collection(self.session) + self.verify_can_publish(collection) + def test__can_publish_non_owned_collection(self): - collection_id = self.generate_collection(self.session, owner="someone_else").id - self.generate_dataset(self.session, collection_id=collection_id, collection_visibility="PRIVATE") - path = f"/dp/v1/collections/{collection_id}/publish" - body = {"data_submission_policy_version": "1.0"} - headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} - response = self.app.post(path, headers=headers, data=json.dumps(body)) - self.assertEqual(202, response.status_code) + collection = self.generate_collection(self.session, owner="someone_else") + self.verify_can_publish(collection) diff --git a/tests/unit/backend/corpora/api_server/test_revisions.py b/tests/unit/backend/corpora/api_server/test_revisions.py index 082ff04127f7f..3cb7a71390d2b 100644 --- a/tests/unit/backend/corpora/api_server/test_revisions.py +++ b/tests/unit/backend/corpora/api_server/test_revisions.py @@ -6,7 +6,11 @@ from sqlalchemy.exc import SQLAlchemyError -from backend.corpora.common.corpora_orm import CollectionVisibility, ConversionStatus, DatasetArtifactFileType +from backend.corpora.common.corpora_orm import ( + CollectionVisibility, + ConversionStatus, + DatasetArtifactFileType, +) from backend.corpora.common.entities import Dataset, Collection from backend.corpora.common.utils.db_session import db_session_manager from backend.corpora.common.utils.exceptions import CorporaException @@ -21,11 +25,9 @@ def setUp(self): super().setUp() pub_collection = self.generate_collection(self.session, visibility="PUBLIC") for i in range(2): - self.generate_dataset_with_s3_resources( - self.session, collection_visibility="PUBLIC", collection_id=pub_collection.id, published=True - ) + self.generate_dataset_with_s3_resources(self.session, collection_id=pub_collection.id, published=True) self.pub_collection = pub_collection - self.rev_collection = pub_collection.revision() + self.rev_collection = pub_collection.create_revision() self.headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} def assertPublishedCollectionOK(self, expected_body, s3_objects): @@ -49,7 +51,6 @@ def refresh_datasets(self) -> typing.List[Dataset]: for dataset in self.pub_collection.datasets: ds = self.generate_dataset_with_s3_resources( self.session, - collection_visibility="PRIVATE", collection_id=self.rev_collection.id, original_id=dataset.id, revision=dataset.revision + 1, @@ -76,7 +77,7 @@ class TestRevision(BaseRevisionTest): def verify_start_revision(self, collection_id: str) -> dict: """ Verify start of a collection's revision. - :return: Jsonified response of POST collection/. + :return: Jsonified response of POST collection/ """ path = f"/dp/v1/collections/{collection_id}" response = self.app.post(path, self.headers) @@ -90,9 +91,9 @@ def verify_start_revision(self, collection_id: str) -> dict: def verify_get_revision(self, collection_id: str, dataset_ids: typing.List[str] = None) -> dict: """ Verify the contents of a collection under revision. - :return: Jsonified response of GET collection/?visibility=PRIVATE. + :return: Jsonified response of GET collection/ """ - path = f"/dp/v1/collections/{collection_id}?visibility=PRIVATE" + path = f"/dp/v1/collections/{collection_id}" response = self.app.get(path, headers=self.headers) response_json = json.loads(response.data) @@ -125,7 +126,7 @@ def verify_get_revision(self, collection_id: str, dataset_ids: typing.List[str] def verify_unauthed_get_revision(self, collection_id: str, expected_body: dict) -> None: """Verify unauthorized view of a collection under revision.""" - path = f"/dp/v1/collections/{collection_id}?visibility=PRIVATE" + path = f"/dp/v1/collections/{collection_id}" headers = {"host": "localhost", "Content-Type": "application/json"} response = self.app.get(path, headers=headers) self.assertEqual(200, response.status_code) @@ -143,11 +144,12 @@ def test__start_revision_of_a_collection__201(self): ] res_post_json = self.verify_start_revision(collection.id) - res_get_json = self.verify_get_revision(collection.id, dataset_ids) + revision_id = res_post_json["id"] + res_get_json = self.verify_get_revision(revision_id, dataset_ids) self.assertEqual(res_post_json, res_get_json) self.assertEqual("WRITE", res_get_json.pop("access_type")) - self.verify_unauthed_get_revision(collection.id, res_get_json) + self.verify_unauthed_get_revision(revision_id, res_get_json) def test__start_revision_of_a_collection_w_links__201(self): """Start a revision of a collection with links.""" @@ -166,16 +168,17 @@ def test__start_revision_of_a_collection_w_links__201(self): link_names = [link.link_name for link in collection.links] res_post_json = self.verify_start_revision(collection.id) + revision_id = res_post_json["id"] # Check link names res_links = [link["link_name"] for link in res_post_json["links"]] self.assertListEqual(sorted(link_names), sorted(res_links)) - res_get_json = self.verify_get_revision(collection.id, dataset_ids) + res_get_json = self.verify_get_revision(revision_id, dataset_ids) self.assertEqual(res_post_json, res_get_json) self.assertEqual("WRITE", res_get_json.pop("access_type")) - self.verify_unauthed_get_revision(collection.id, res_get_json) + self.verify_unauthed_get_revision(revision_id, res_get_json) def test__revision__409(self): """Starting a revision on a revision.""" @@ -211,18 +214,17 @@ class TestDeleteRevision(BaseRevisionTest): def setUp(self): super().setUp() - url = f"/dp/v1/collections/{self.pub_collection.id}" - self.test_url_collect_private = f"{url}?visibility=PRIVATE" - self.test_url_collection_public = f"{url}?visibility=PUBLIC" + self.test_url_collection_private = f"/dp/v1/collections/{self.rev_collection.id}" + self.test_url_collection_public = f"/dp/v1/collections/{self.pub_collection.id}" def test__revision_deleted__204(self): """Delete a collection under revision.""" # Delete the revision - resp = self.app.delete(self.test_url_collect_private, headers=self.headers) + resp = self.app.delete(self.test_url_collection_private, headers=self.headers) self.assertEqual(204, resp.status_code) # Cannot get the revision - resp = self.app.get(self.test_url_collect_private, headers=self.headers) + resp = self.app.get(self.test_url_collection_private, headers=self.headers) self.assertEqual(403, resp.status_code) def test__revision_deleted_with_published_datasets(self): @@ -237,7 +239,7 @@ def test__revision_deleted_with_published_datasets(self): self.assertEqual(pub_s3_objects, rev_s3_objects) # Delete the revision - resp = self.app.delete(self.test_url_collect_private, headers=self.headers) + resp = self.app.delete(self.test_url_collection_private, headers=self.headers) self.assertEqual(204, resp.status_code) self.assertPublishedCollectionOK(expected_body, pub_s3_objects) @@ -245,7 +247,7 @@ def test__revision_deleted_with_new_datasets(self): """The new datasets should be deleted when the revison is deleted.""" # Generate revision dataset rev_dataset = self.generate_dataset_with_s3_resources( - self.session, collection_visibility="PRIVATE", collection_id=self.rev_collection.id, published=False + self.session, collection_id=self.rev_collection.id, published=False ) s3_objects = self.get_s3_object_paths_from_dataset(rev_dataset) @@ -255,7 +257,7 @@ def test__revision_deleted_with_new_datasets(self): self.assertS3FileExists(bucket, file_name) # Delete Revision - resp = self.app.delete(self.test_url_collect_private, headers=self.headers) + resp = self.app.delete(self.test_url_collection_private, headers=self.headers) self.assertEqual(204, resp.status_code) with self.subTest("new artifacts and explorer s3 objects deleted"): @@ -277,7 +279,7 @@ def test__revision_deleted_with_refreshed_datasets(self): self.assertNotIn(s3_object, pub_s3_objects) # Delete the revision - resp = self.app.delete(self.test_url_collect_private, headers=self.headers) + resp = self.app.delete(self.test_url_collection_private, headers=self.headers) self.assertEqual(204, resp.status_code) with self.subTest("refreshed artifacts and explorer s3 objects deleted"): @@ -336,12 +338,12 @@ def test__delete_published_dataset_during_revision(self): self.assertEqual(202, resp.status_code) # Get the revision authenticated - resp = self.app.get(self.test_url_collect_private, headers=self.headers) + resp = self.app.get(self.test_url_collection_private, headers=self.headers) self.assertEqual(200, resp.status_code) self.assertEqual(rev_dataset_count, len(json.loads(resp.data)["datasets"])) # Get the revision unauthenticated - resp = self.app.get(self.test_url_collect_private) + resp = self.app.get(self.test_url_collection_private) self.assertEqual(200, resp.status_code) # The dataset is a tombstone in the revision self.assertEqual(rev_dataset_count - 1, len(json.loads(resp.data)["datasets"])) @@ -362,7 +364,7 @@ def setUp(self): self.base_path = "/dp/v1/collections" self.mock_timestamp = datetime(2000, 12, 25, 0, 0) - def publish_collection(self, collection_id: str) -> dict: + def publish_collection(self, collection: Collection) -> dict: """ Verify publish a collection under revision. :return: Jsonified response of GET collection/. @@ -373,20 +375,23 @@ def publish_collection(self, collection_id: str) -> dict: self.assertIsNone(self.pub_collection.published_at) self.assertIsNone(self.pub_collection.revised_at) + revision_id: str = collection.id + collection_id: str = collection.revision_of + for dataset in self.pub_collection.datasets: self.assertIsNone(dataset.published_at) self.assertIsNone(dataset.revised_at) self.session.expire_all() body = {"data_submission_policy_version": "1.0"} - path = f"{self.base_path}/{collection_id}/publish" + path = f"{self.base_path}/{revision_id}/publish" with patch("backend.corpora.common.entities.collection.datetime") as mock_dt: mock_dt.utcnow = Mock(return_value=self.mock_timestamp) response = self.app.post(path, headers=self.headers, data=json.dumps(body)) self.assertEqual(202, response.status_code) self.assertDictEqual({"collection_uuid": collection_id, "visibility": "PUBLIC"}, json.loads(response.data)) - self.addCleanup(self.delete_collection, collection_id, "PUBLIC") + self.addCleanup(self.delete_collection, collection_id) # Cannot call publish for an already published collection response = self.app.post(path, headers=self.headers, data=json.dumps(body)) @@ -428,14 +433,12 @@ def verify_datasets(self, actual_body: dict, expected_dataset_ids: typing.List[s def test__with_revision_with_new_dataset__OK(self): """Publish a revision with new datasets.""" - new_dataset_id = self.generate_dataset_with_s3_resources( - self.session, collection_id=self.rev_collection.id, collection_visibility=CollectionVisibility.PRIVATE - ).id + new_dataset_id = self.generate_dataset_with_s3_resources(self.session, collection_id=self.rev_collection.id).id dataset_ids = [ds.id for ds in self.pub_collection.datasets] dataset_ids.append(new_dataset_id) # Publish revision - response_json = self.publish_collection(self.rev_collection.id) + response_json = self.publish_collection(self.rev_collection) self.verify_datasets(response_json, dataset_ids) # Check published_at and revised_at @@ -455,18 +458,17 @@ def test__with_revision_with_tombstoned_datasets__OK(self): """Publish a revision with delete datasets.""" rev_dataset_id = self.rev_collection.datasets[0].id pub_dataset = self.pub_collection.datasets[0] + pub_dataset_id = pub_dataset.id published_s3_objects = self.get_s3_object_paths_from_dataset(pub_dataset) # Delete a dataset under revision self.app.delete(f"/dp/v1/datasets/{rev_dataset_id}", headers=self.headers) # Publish the revision with the deleted dataset - response_json = self.publish_collection(self.rev_collection.id) + response_json = self.publish_collection(self.rev_collection) self.session.expire_all() - - dataset = Dataset.get(self.session, pub_dataset.id, include_tombstones=True) - - self.assertIn(pub_dataset.id, dataset.explorer_url) + dataset = Dataset.get(self.session, pub_dataset_id, include_tombstones=True) + self.assertIn(pub_dataset_id, dataset.explorer_url) self.assertTrue(dataset.tombstone) for s3_object in published_s3_objects: self.assertS3FileDoesNotExist(*s3_object) @@ -502,7 +504,7 @@ def test__with_revision_with_tombstoned_datasets_rollback__OK(self): expect_datasets.sort(key=lambda x: x["id"]) with self.assertRaises(CorporaException): with db_session_manager() as session: - rev_collection = Collection.get(session, (self.rev_collection.id, self.rev_collection.visibility)) + rev_collection = Collection.get(session, self.rev_collection.id) with mock.patch.object(rev_collection.session, "commit", side_effect=SQLAlchemyError): rev_collection.publish(data_submission_policy_version="v1") @@ -544,7 +546,7 @@ def test__with_revision_with_refreshed_datasets__OK(self): """ "Publish a revision with refreshed datasets.""" self.refresh_datasets() - response_json = self.publish_collection(self.rev_collection.id) + response_json = self.publish_collection(self.rev_collection) self.verify_datasets(response_json, [ds.id for ds in self.pub_collection.datasets]) # Check published_at and revised_at @@ -574,7 +576,7 @@ def test__publish_revision_with_collection_info_updated__201(self): pub_s3_objects, _ = self.get_s3_objects_from_collections() # Published revision with collection details updated - response_json = self.publish_collection(self.rev_collection.id) + response_json = self.publish_collection(self.rev_collection) self.assertPublishedCollectionOK(expected_body, pub_s3_objects) self.verify_datasets(response_json, [ds.id for ds in self.pub_collection.datasets]) @@ -591,7 +593,7 @@ def test__publish_revision_with_collection_info_updated__201(self): def test__with_revision_and_existing_datasets(self): """Publish a revision with the same, existing datasets.""" - response_json = self.publish_collection(self.rev_collection.id) + response_json = self.publish_collection(self.rev_collection) self.verify_datasets(response_json, [ds.id for ds in self.pub_collection.datasets]) # Check published_at and revised_at @@ -623,7 +625,7 @@ def test__delete_old_seurat_artifact_when_skipped(self): id_with_skipped_seurat = dataset_with_skipped_seurat.original_id # Publishing collection should result in rds artifact no longer existing for dataset_with_skipped_seurat - self.publish_collection(self.rev_collection.id) + self.publish_collection(self.rev_collection) self.session.expire_all() for dataset in self.pub_collection.db_object.datasets: diff --git a/tests/unit/backend/corpora/api_server/test_v1_collection.py b/tests/unit/backend/corpora/api_server/test_v1_collection.py index 1009e5bf084f4..4e64917b78506 100644 --- a/tests/unit/backend/corpora/api_server/test_v1_collection.py +++ b/tests/unit/backend/corpora/api_server/test_v1_collection.py @@ -81,7 +81,6 @@ def validate_collection_uuid_response_structure(self, body): "created_at", "updated_at", "collection_id", - "collection_visibility", "is_valid", "cell_count", ] @@ -182,7 +181,6 @@ def test__get_collection_uuid__ok(self): "name": "test_dataset_name", "organism": [{"label": "test_organism", "ontology_term_id": "test_obo"}], "collection_id": "test_collection_id", - "collection_visibility": "PUBLIC", "cell_type": [{"label": "test_cell_type", "ontology_term_id": "test_opo"}], "x_normalization": "test_x_normalization", "x_approximate_distribution": "NORMAL", @@ -214,7 +212,6 @@ def test__get_collection_uuid__ok(self): "genesets": [ { "collection_id": "test_collection_id", - "collection_visibility": "PUBLIC", "linked_datasets": [], "description": "this is a geneset", "id": "test_geneset", @@ -222,7 +219,6 @@ def test__get_collection_uuid__ok(self): }, { "collection_id": "test_collection_id", - "collection_visibility": "PUBLIC", "linked_datasets": ["test_dataset_id"], "description": "this is a geneset with a dataset", "id": "test_geneset_with_dataset", @@ -326,7 +322,6 @@ def test_get_collection_minimal__ok(self): "datasets": [ { "collection_id": collection.id, - "collection_visibility": "PUBLIC", "dataset_assets": [], "dataset_deployments": [], "linked_genesets": [], @@ -358,10 +353,6 @@ def test_get_collection_minimal__ok(self): # should keep stricter data types, but JSON doesn't support Enums and therefore have to be converted # as strings. self.assertEqual(expected_body.pop("visibility").name, actual_body.pop("visibility")) - self.assertEqual( - expected_body["datasets"][0].pop("collection_visibility").name, - actual_body["datasets"][0].pop("collection_visibility"), - ) self.assertEqual( expected_body["datasets"][0].pop("x_approximate_distribution").name, actual_body["datasets"][0].pop("x_approximate_distribution"), @@ -737,6 +728,15 @@ def test__list_collection__check_owner(self): private_not_owned = self.generate_collection( self.session, visibility=CollectionVisibility.PRIVATE.name, owner="someone else" ).id + revision_not_owned = self.generate_collection( + self.session, + visibility=CollectionVisibility.PRIVATE.name, + owner="someone else", + revision_of=public_not_owned, + ).id + revision_owned = self.generate_collection( + self.session, visibility=CollectionVisibility.PRIVATE.name, owner="test_user_id", revision_of=public_owned + ).id path = "/dp/v1/collections" with self.subTest("no auth"): @@ -751,6 +751,7 @@ def test__list_collection__check_owner(self): self.assertIn(public_not_owned, ids) self.assertNotIn(private_owned, ids) self.assertNotIn(private_not_owned, ids) + self.assertNotIn(revision_owned, ids) with self.subTest("auth"): headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} @@ -764,6 +765,11 @@ def test__list_collection__check_owner(self): self.assertIn(public_not_owned, ids) self.assertIn(private_owned, ids) self.assertNotIn(private_not_owned, ids) + self.assertNotIn(revision_not_owned, ids) + self.assertIn(revision_owned, ids) + self.assertTrue( + [collection for collection in collections if collection.get("revision_of") == public_owned][0] + ) @unittest.skip("Flaky") # TODO @mdunitz ticket 2223 def test__get_all_collections_for_index(self): @@ -844,24 +850,24 @@ def test_delete_private_collection__ok(self): for bucket, key in s3_objects: self.assertS3FileDoesNotExist(bucket, key) - def test_tombstone_collection_revision__ok(self): + def test_delete_collection_revision__ok(self): # Generate test collection collection = self.generate_collection( - self.session, visibility=CollectionVisibility.PRIVATE.name, owner="test_user_id" + self.session, visibility=CollectionVisibility.PUBLIC.name, owner="test_user_id" ) # Generate the public collection with the same id as the private so a tombstone is created - self.generate_collection( - self.session, id=collection.id, visibility=CollectionVisibility.PUBLIC.name, owner="test_user_id" + revision = self.generate_collection( + self.session, visibility=CollectionVisibility.PRIVATE.name, owner="test_user_id", revision_of=collection.id ) processing_status_1 = {"upload_status": UploadStatus.WAITING, "upload_progress": 0.0} processing_status_2 = {"upload_status": UploadStatus.UPLOADED, "upload_progress": 100.0} - dataset_1 = self.generate_dataset(self.session, collection=collection, processing_status=processing_status_1) - dataset_2 = self.generate_dataset(self.session, collection=collection, processing_status=processing_status_2) + dataset_1 = self.generate_dataset(self.session, collection=revision, processing_status=processing_status_1) + dataset_2 = self.generate_dataset(self.session, collection=revision, processing_status=processing_status_2) headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} - test_private_url = furl(path=f"/dp/v1/collections/{collection.id}", query_params=dict(visibility="PRIVATE")) - test_public_url = furl(path=f"/dp/v1/collections/{collection.id}", query_params=dict(visibility="PUBLIC")) + test_private_url = furl(path=f"/dp/v1/collections/{revision.id}") + test_public_url = furl(path=f"/dp/v1/collections/{collection.id}") response = self.app.get(test_private_url.url, headers=headers) self.assertEqual(200, response.status_code) @@ -885,27 +891,24 @@ def test_tombstone_collection_revision__ok(self): def test_tombstone_published_collection_with_revision__ok(self): """Both the published and revised collections are tombstoned.""" - # Generate test collection - collection_rev = self.generate_collection( - self.session, visibility=CollectionVisibility.PRIVATE.name, owner="test_user_id" + # Generate the public collection + collection = self.generate_collection( + self.session, visibility=CollectionVisibility.PUBLIC.name, owner="test_user_id" ) - # Generate the public collection with the same id as the private so a tombstone is created - collection_pub = self.generate_collection( - self.session, id=collection_rev.id, visibility=CollectionVisibility.PUBLIC.name, owner="test_user_id" + # Generate test collection + revision = self.generate_collection( + self.session, visibility=CollectionVisibility.PRIVATE.name, owner="test_user_id", revision_of=collection.id ) + revision_id = revision.id processing_status = {"upload_status": UploadStatus.UPLOADED, "upload_progress": 100.0} - dataset_rev = self.generate_dataset( - self.session, collection=collection_rev, processing_status=processing_status - ) - dataset_pub = self.generate_dataset( - self.session, collection=collection_pub, processing_status=processing_status - ) + dataset_rev = self.generate_dataset(self.session, collection=revision, processing_status=processing_status) + dataset_pub = self.generate_dataset(self.session, collection=collection, processing_status=processing_status) headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} # Verify private collections exist - test_private_url = furl(path=f"/dp/v1/collections/{collection_rev.id}", query_params=dict(visibility="PRIVATE")) + test_private_url = furl(path=f"/dp/v1/collections/{revision.id}", query_params=dict(visibility="PRIVATE")) response = self.app.get(test_private_url.url, headers=headers) self.assertEqual(200, response.status_code) body = json.loads(response.data) @@ -913,7 +916,7 @@ def test_tombstone_published_collection_with_revision__ok(self): self.assertIn(dataset_rev.id, dataset_ids) # Verify public collections exist - test_public_url = furl(path=f"/dp/v1/collections/{collection_pub.id}", query_params=dict(visibility="PUBLIC")) + test_public_url = furl(path=f"/dp/v1/collections/{collection.id}", query_params=dict(visibility="PUBLIC")) response = self.app.get(test_public_url.url, headers=headers) self.assertEqual(200, response.status_code) body = json.loads(response.data) @@ -924,7 +927,7 @@ def test_tombstone_published_collection_with_revision__ok(self): response = self.app.delete(test_public_url.url, headers=headers) self.assertEqual(response.status_code, 204) - # check collection revision and datasets tombstoned + # check collection revision and datasets are gone response = self.app.get(test_private_url.url, headers=headers) self.assertEqual(response.status_code, 403) @@ -934,29 +937,31 @@ def test_tombstone_published_collection_with_revision__ok(self): self.session.expire_all() collection = Collection.get_collection( - self.session, collection_pub.id, CollectionVisibility.PUBLIC.name, include_tombstones=True + self.session, collection.id, CollectionVisibility.PUBLIC.name, include_tombstones=True ) self.assertTrue(collection.tombstone) self.assertTrue(dataset_pub.tombstone) + rev_collection = Collection.get_collection(self.session, revision_id, include_tombstones=True) + self.assertIsNone(rev_collection) # Revision should be deleted, not tombstoned def test_delete_collection__dataset_not_available(self): - # Generate test collection + # Generate the public collection collection = self.generate_collection( - self.session, visibility=CollectionVisibility.PRIVATE.name, owner="test_user_id" + self.session, visibility=CollectionVisibility.PUBLIC.name, owner="test_user_id", tombstone=True ) - # Generate the public collection with the same id as the private so a tombstone is created - self.generate_collection( - self.session, id=collection.id, visibility=CollectionVisibility.PUBLIC.name, owner="test_user_id" + # Generate test collection + revision = self.generate_collection( + self.session, visibility=CollectionVisibility.PRIVATE.name, owner="test_user_id", revision_of=collection.id ) processing_status = {"upload_status": UploadStatus.UPLOADED, "upload_progress": 100.0} - dataset = self.generate_dataset(self.session, collection=collection, processing_status=processing_status) + dataset = self.generate_dataset(self.session, collection=revision, processing_status=processing_status) headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} dataset_url = furl(path=f"/dp/v1/datasets/{dataset.id}/status") response = self.app.get(dataset_url.url, headers=headers) self.assertEqual(response.status_code, 200) - test_url = furl(path=f"/dp/v1/collections/{collection.id}", query_params=dict(visibility="PRIVATE")) + test_url = furl(path=f"/dp/v1/collections/{revision.id}", query_params=dict(visibility="PRIVATE")) response = self.app.delete(test_url.url, headers=headers) self.assertEqual(response.status_code, 204) @@ -965,13 +970,15 @@ def test_delete_collection__dataset_not_available(self): self.assertEqual(response.status_code, 403) def test_delete_collection__already_tombstoned__ok(self): + # Generate the public collection collection = self.generate_collection( - self.session, visibility=CollectionVisibility.PRIVATE.name, owner="test_user_id", tombstone=True + self.session, visibility=CollectionVisibility.PUBLIC.name, owner="test_user_id", tombstone=True ) - # Generate the public collection with the same id as the private so a tombstone is created + # Generate test collection self.generate_collection( - self.session, id=collection.id, visibility=CollectionVisibility.PUBLIC.name, owner="test_user_id" + self.session, visibility=CollectionVisibility.PRIVATE.name, owner="test_user_id", revision_of=collection.id ) + test_url = furl(path=f"/dp/v1/collections/{collection.id}", query_params=dict(visibility="PRIVATE")) headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} response = self.app.delete(test_url.url, headers=headers) @@ -983,7 +990,6 @@ def test_delete_collection__public__ok(self): ) test_urls = [ - furl(path=f"/dp/v1/collections/{collection.id}", query_params=dict(visibility="PUBLIC")), furl(path=f"/dp/v1/collections/{collection.id}"), ] headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} @@ -1001,7 +1007,7 @@ def test_delete_collection__not_owner(self): collection = self.generate_collection( self.session, visibility=CollectionVisibility.PRIVATE.name, owner="someone_else" ) - test_url = furl(path=f"/dp/v1/collections/{collection.id}", query_params=dict(visibility="PRIVATE")) + test_url = furl(path=f"/dp/v1/collections/{collection.id}") headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} response = self.app.delete(test_url.url, headers=headers) self.assertEqual(response.status_code, 403) diff --git a/tests/unit/backend/corpora/api_server/test_v1_collection_upload_link.py b/tests/unit/backend/corpora/api_server/test_v1_collection_upload_link.py index 2f0bb26c0ac82..1d0e1c3e75f71 100644 --- a/tests/unit/backend/corpora/api_server/test_v1_collection_upload_link.py +++ b/tests/unit/backend/corpora/api_server/test_v1_collection_upload_link.py @@ -20,7 +20,7 @@ def setUp(self): @patch("backend.corpora.common.upload_sfn.start_upload_sfn") def test__link__202(self, mocked): with EnvironmentSetup({"CORPORA_CONFIG": fixture_file_path("bogo_config.js")}): - path = "/dp/v1/collections/test_collection_id/upload-links" + path = "/dp/v1/collections/test_collection_id_revision/upload-links" headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} body = {"url": self.good_link} @@ -127,13 +127,12 @@ def test__reupload_published_dataset_during_revision__202(self, mocked): pub_dataset = self.generate_dataset_with_s3_resources( self.session, collection_id=pub_collection.id, - collection_visibility=pub_collection.visibility, published=True, processing_status={"processing_status": ProcessingStatus.SUCCESS}, ) pub_s3_objects = self.get_s3_object_paths_from_dataset(pub_dataset) - rev_collection = pub_collection.revision() - path = f"/dp/v1/collections/{pub_collection.id}/upload-links" + rev_collection = pub_collection.create_revision() + path = f"/dp/v1/collections/{rev_collection.id}/upload-links" body = {"url": self.good_link, "id": rev_collection.datasets[0].id} with EnvironmentSetup({"CORPORA_CONFIG": fixture_file_path("bogo_config.js")}): @@ -151,7 +150,6 @@ def test__reupload_unpublished_dataset__202(self, mocked): dataset = self.generate_dataset_with_s3_resources( self.session, collection_id=collection.id, - collection_visibility=collection.visibility, published=False, processing_status={"processing_status": ProcessingStatus.SUCCESS}, ) @@ -175,7 +173,6 @@ def test__reupload_unpublished_dataset_during_revision_202(self, mock): dataset = self.generate_dataset_with_s3_resources( self.session, collection_id=collection.id, - collection_visibility=collection.visibility, published=True, processing_status={"processing_status": ProcessingStatus.SUCCESS}, ) @@ -195,7 +192,6 @@ def test__reupload_public_dataset__403(self): pub_dataset = self.generate_dataset_with_s3_resources( self.session, collection_id=collection.id, - collection_visibility=collection.visibility, published=True, processing_status={"processing_status": ProcessingStatus.SUCCESS}, ) @@ -217,7 +213,6 @@ def test__reupload_while_processing_dataset__405(self): dataset = self.generate_dataset_with_s3_resources( self.session, collection_id=collection.id, - collection_visibility=collection.visibility, processing_status={"processing_status": ProcessingStatus.PENDING}, ) s3_objects = self.get_s3_object_paths_from_dataset(dataset) @@ -236,7 +231,6 @@ def test__reupload_dataset_not_owner__403(self): dataset = self.generate_dataset_with_s3_resources( self.session, collection_id=collection.id, - collection_visibility=collection.visibility, published=False, processing_status={"processing_status": ProcessingStatus.SUCCESS}, ) @@ -291,7 +285,6 @@ def test__can_reupload_dataset_not_owner(self, mocked): dataset = self.generate_dataset_with_s3_resources( self.session, collection_id=collection.id, - collection_visibility=collection.visibility, published=False, processing_status={"processing_status": ProcessingStatus.SUCCESS}, ) diff --git a/tests/unit/backend/corpora/api_server/test_v1_dataset.py b/tests/unit/backend/corpora/api_server/test_v1_dataset.py index 8973a8ef245c8..2376735e3624b 100644 --- a/tests/unit/backend/corpora/api_server/test_v1_dataset.py +++ b/tests/unit/backend/corpora/api_server/test_v1_dataset.py @@ -115,28 +115,38 @@ def test__minimal_status__ok(self): self.assertEqual(json.loads(response.data)["upload_status"], status.name) def test__get_all_datasets_for_index(self): + test_dataset_id = "test_dataset_id_for_index" dataset = self.generate_dataset( self.session, - id="test_dataset_id_for_index", + id=test_dataset_id, cell_count=42, mean_genes_per_cell=0.05, published_at=datetime.now(), revised_at=datetime.now(), ) self.generate_dataset(self.session, id="test_dataset_id_for_index_tombstone", tombstone=True) - self.generate_dataset(self.session, id="test_dataset_id_for_index_private", collection_visibility="PRIVATE") + self.generate_dataset( + self.session, + id="test_dataset_id_for_index_private", + collection_id="test_collection_id_revision", + ) test_url = furl(path="/dp/v1/datasets/index") headers = {"host": "localhost", "Content-Type": "application/json", "Cookie": get_auth_token(self.app)} response = self.app.get(test_url.url, headers=headers) self.assertEqual(200, response.status_code) body = json.loads(response.data) - ids = [dataset["id"] for dataset in body] + ids = [d["id"] for d in body] self.assertIn("test_dataset_id_for_index", ids) self.assertNotIn("test_dataset_id_for_index_tombstone", ids) self.assertNotIn("test_dataset_id_for_index_private", ids) - actual_dataset = body[-1] # last added dataset + actual_dataset = None + for d in body: + if d["id"] == test_dataset_id: + actual_dataset = d + self.assertIsNotNone(actual_dataset) + self.assertEqual(actual_dataset["id"], dataset.id) self.assertEqual(actual_dataset["name"], dataset.name) self.assertNotIn("description", actual_dataset) @@ -176,9 +186,10 @@ def test__enrich_development_stage_with_ancestors_missing_key_ok(self): self.assertNotIn("development_stage_ancestors", dataset) def test__get_all_datasets_for_index_with_ontology_expansion(self): + test_dataset_id = "test_dataset_id_for_index_2" dataset = self.generate_dataset( self.session, - id="test_dataset_id_for_index_2", + id=test_dataset_id, cell_count=42, development_stage=[{"ontology_term_id": "HsapDv:0000008", "label": "Test"}], published_at=datetime.now(), @@ -192,7 +203,11 @@ def test__get_all_datasets_for_index_with_ontology_expansion(self): self.assertEqual(200, response.status_code) body = json.loads(response.data) - actual_dataset = body[-1] # last added dataset + actual_dataset = None + for d in body: + if d["id"] == test_dataset_id: + actual_dataset = d + self.assertIsNotNone(actual_dataset) self.assertEqual(actual_dataset["development_stage"], dataset.development_stage) self.assertEqual( diff --git a/tests/unit/backend/corpora/api_server/test_v1_genesets.py b/tests/unit/backend/corpora/api_server/test_v1_genesets.py index 4f18e0612ba16..9f74829000536 100644 --- a/tests/unit/backend/corpora/api_server/test_v1_genesets.py +++ b/tests/unit/backend/corpora/api_server/test_v1_genesets.py @@ -55,9 +55,7 @@ def test__delete_geneset_ACCEPTED(self): with self.subTest("With collection"): # create a geneset - geneset = self.generate_geneset( - self.session, collection_id=collection.id, collection_visibility=collection.visibility - ) + geneset = self.generate_geneset(self.session, collection_id=collection.id) # get geneset actual_geneset_ids = self._get_geneset_ids(collection.id, headers) @@ -67,14 +65,10 @@ def test__delete_geneset_ACCEPTED(self): with self.subTest("With dataset"): # create dataset - dataset = self.generate_dataset( - self.session, collection_id=collection.id, collection_visibility=collection.visibility - ) + dataset = self.generate_dataset(self.session, collection_id=collection.id) # create a geneset - geneset = self.generate_geneset( - self.session, collection_id=collection.id, collection_visibility=collection.visibility - ) + geneset = self.generate_geneset(self.session, collection_id=collection.id) GenesetDatasetLink.create(self.session, geneset.id, dataset.id) # get geneset @@ -96,9 +90,7 @@ def test__delete_gene_set__UNAUTHORIZED(self): ) # create a geneset - geneset = self.generate_geneset( - self.session, collection_id=collection.id, collection_visibility=collection.visibility - ) + geneset = self.generate_geneset(self.session, collection_id=collection.id) # get geneset actual_geneset_ids = self._get_geneset_ids(collection.id, headers) @@ -121,9 +113,7 @@ def test__delete_gene_set__NOT_ALLOWED(self): ) # create a geneset - geneset = self.generate_geneset( - self.session, collection_id=collection.id, collection_visibility=collection.visibility - ) + geneset = self.generate_geneset(self.session, collection_id=collection.id) # get geneset actual_geneset_ids = self._get_geneset_ids(collection.id, headers, True) @@ -157,9 +147,7 @@ def test__delete_gene_set__200(self): ) # create a geneset - geneset = self.generate_geneset( - self.session, collection_id=collection.id, collection_visibility=collection.visibility - ) + geneset = self.generate_geneset(self.session, collection_id=collection.id) # get geneset actual_geneset_ids = self._get_geneset_ids(collection.id, headers) diff --git a/tests/unit/backend/corpora/common/entities/datasets/__init__.py b/tests/unit/backend/corpora/common/entities/datasets/__init__.py index 3c301aee0c273..0ee6f12a98882 100644 --- a/tests/unit/backend/corpora/common/entities/datasets/__init__.py +++ b/tests/unit/backend/corpora/common/entities/datasets/__init__.py @@ -48,13 +48,7 @@ def assertRowsDeleted(self, tests: typing.List[typing.Tuple[str, Base]]): """ self.session.expire_all() for p_key, table in tests: - if len(p_key) == 2: - # handle the special case for collections with a composite primary key - actual = [ - i for i in self.session.query(table).filter(table.id == p_key[0], table.visibility == p_key[1]) - ] - else: - actual = [i for i in self.session.query(table).filter(table.id == p_key)] + actual = [i for i in self.session.query(table).filter(table.id == p_key)] self.assertFalse(actual, f"Row not deleted {table.__name__}:{p_key}") def assertRowsExist(self, tests): @@ -64,11 +58,5 @@ def assertRowsExist(self, tests): """ self.session.expire_all() for p_key, table in tests: - if len(p_key) == 2: - # handle the special case for collections with a composite primary key - actual = [ - i for i in self.session.query(table).filter(table.id == p_key[0], table.visibility == p_key[1]) - ] - else: - actual = [i for i in self.session.query(table).filter(table.id == p_key)] + actual = [i for i in self.session.query(table).filter(table.id == p_key)] self.assertTrue(actual, f"Row does not exist {table.__name__}:{p_key}") diff --git a/tests/unit/backend/corpora/common/entities/datasets/test_delete.py b/tests/unit/backend/corpora/common/entities/datasets/test_delete.py index bb437df34a46a..c3326addbbbf6 100644 --- a/tests/unit/backend/corpora/common/entities/datasets/test_delete.py +++ b/tests/unit/backend/corpora/common/entities/datasets/test_delete.py @@ -1,5 +1,4 @@ from backend.corpora.common.corpora_orm import ( - CollectionVisibility, DbDataset, DbDatasetArtifact, DbCollection, @@ -19,13 +18,12 @@ def test__cascade_delete_dataset__ok(self): self.session, **BogusDatasetParams.get( collection_id="test_collection_id", - collection_visibility=CollectionVisibility.PUBLIC.name, artifacts=[{}], ), ) test_dataset_ids = [(test_dataset.id, DbDataset)] test_artifact_ids = [(art.id, DbDatasetArtifact) for art in test_dataset.artifacts] - test_collection_ids = [(("test_collection_id", CollectionVisibility.PUBLIC.name), DbCollection)] + test_collection_ids = [("test_collection_id", DbCollection)] with self.subTest("verify everything exists"): expected_exists = test_collection_ids + test_dataset_ids + test_artifact_ids diff --git a/tests/unit/backend/corpora/common/entities/datasets/test_reprocess.py b/tests/unit/backend/corpora/common/entities/datasets/test_reprocess.py index 595210be9c4b0..651cf009b56f8 100644 --- a/tests/unit/backend/corpora/common/entities/datasets/test_reprocess.py +++ b/tests/unit/backend/corpora/common/entities/datasets/test_reprocess.py @@ -11,18 +11,14 @@ def _verify(_dataset): self.assertLess(previous_revision, dataset.revision) with self.subTest("published_dataset"): - dataset = self.generate_dataset_with_s3_resources( - self.session, collection_visibility="PRIVATE", published=True - ) + dataset = self.generate_dataset_with_s3_resources(self.session, published=True) s3_objects = self.get_s3_object_paths_from_dataset(dataset) _verify(dataset) for s3_objects in s3_objects: self.assertS3FileExists(*s3_objects) with self.subTest("unpublished_dataset"): - dataset = self.generate_dataset_with_s3_resources( - self.session, collection_visibility="PRIVATE", published=False - ) + dataset = self.generate_dataset_with_s3_resources(self.session, published=False) s3_objects = self.get_s3_object_paths_from_dataset(dataset) _verify(dataset) for s3_objects in s3_objects: diff --git a/tests/unit/backend/corpora/common/entities/datasets/test_revision.py b/tests/unit/backend/corpora/common/entities/datasets/test_revision.py index 00336bd9d9f06..5849a619f304a 100644 --- a/tests/unit/backend/corpora/common/entities/datasets/test_revision.py +++ b/tests/unit/backend/corpora/common/entities/datasets/test_revision.py @@ -4,7 +4,7 @@ class TestDatasetRevision(TestDataset): def test__create_dataset_revision(self): dataset = self.generate_dataset_with_s3_resources(self.session, published=True) - rev_dataset = dataset.create_revision().to_dict() + rev_dataset = dataset.create_revision("test_collection_id_revision").to_dict() dataset = dataset.to_dict() with self.subTest("artifacts are correctly created and point to correct s3 uri"): @@ -36,9 +36,7 @@ def test__create_dataset_revision(self): self.assertNotEqual(revision_collection, dataset_1_collection) with self.subTest("metadata of revised matches original"): for key in rev_dataset.keys(): - self.compare_original_and_revision( - dataset, rev_dataset, key, ("original_id", "id", "collection_visibility") - ) + self.compare_original_and_revision(dataset, rev_dataset, key, ("original_id", "id", "collection_id")) def compare_original_and_revision(self, original, revision, key, unique_fields): if key in unique_fields: diff --git a/tests/unit/backend/corpora/common/entities/test_collection.py b/tests/unit/backend/corpora/common/entities/test_collection.py index 96072a74968bb..e28def1440dff 100644 --- a/tests/unit/backend/corpora/common/entities/test_collection.py +++ b/tests/unit/backend/corpora/common/entities/test_collection.py @@ -1,7 +1,5 @@ from datetime import datetime -from sqlalchemy.exc import SQLAlchemyError - from backend.corpora.common.corpora_orm import CollectionLinkType, DbCollectionLink, CollectionVisibility, DbDataset from backend.corpora.common.entities import Dataset from backend.corpora.common.entities.collection import Collection @@ -17,9 +15,8 @@ def setUp(self): super().setUp() def test__get__ok(self): - key = (self.uuid, self.visibility) - collection = Collection.get(self.session, key) + collection = Collection.get(self.session, self.uuid) # Verify Columns self.assertEqual(collection.name, "test_collection_name") @@ -37,14 +34,14 @@ def test__get__ok(self): self.assertEqual(collection.links[0].id, "test_collection_doi_link_id") def test__get__does_not_exist(self): - non_existent_key = ("non_existent_id", self.visibility) + non_existent_key = "non_existent_id" self.assertEqual(Collection.get(self.session, non_existent_key), None) - def test__get__invalid_visibility(self): - invalid_visibility_key = (self.uuid, "invalid_visibility") - with self.assertRaises(SQLAlchemyError): - Collection.get(self.session, invalid_visibility_key) + # def test__get__invalid_visibility(self): + # invalid_visibility_key = (self.uuid, "invalid_visibility") + # with self.assertRaises(SQLAlchemyError): + # Collection.get(self.session, invalid_visibility_key) def test__create_collection_creates_associated_links(self): """ @@ -58,14 +55,13 @@ def test__create_collection_creates_associated_links(self): with self.subTest(i): collection = Collection.create(self.session, links=[link_params] * i, **collection_params) - collection_key = (collection.id, collection.visibility) expected_links = collection.links # Expire all local object and retrieve them from the DB to make sure the transactions went through. self.session.expire_all() - actual_collection = Collection.get(self.session, collection_key) - self.assertEqual(collection_key, (actual_collection.id, actual_collection.visibility)) + actual_collection = Collection.get(self.session, collection.id) + self.assertEqual(collection.id, actual_collection.id) self.assertCountEqual(expected_links, actual_collection.links) def test__list_in_time_range__ok(self): @@ -201,9 +197,7 @@ def test__cascade_delete_collection_with_dataset__ok(self): expected_collection_id = test_collection.id test_dataset = Dataset.create( self.session, - **BogusDatasetParams.get( - collection_id=test_collection.id, collection_visibility=test_collection.visibility - ), + **BogusDatasetParams.get(collection_id=test_collection.id), ) expected_dataset_id = test_dataset.id diff --git a/tests/unit/backend/corpora/dataset_processing/test_process.py b/tests/unit/backend/corpora/dataset_processing/test_process.py index ba3be88b81e75..90526440a515c 100644 --- a/tests/unit/backend/corpora/dataset_processing/test_process.py +++ b/tests/unit/backend/corpora/dataset_processing/test_process.py @@ -302,7 +302,8 @@ def test_update_db(self): collection = Collection.create(self.session, visibility=CollectionVisibility.PRIVATE) dataset = Dataset.create( - self.session, collection_id=collection.id, collection_visibility=CollectionVisibility.PRIVATE + self.session, + collection_id=collection.id, ) dataset_id = dataset.id diff --git a/tests/unit/backend/fixtures/data_portal_test_case.py b/tests/unit/backend/fixtures/data_portal_test_case.py index 24415f08f261c..c3a7d128344dc 100644 --- a/tests/unit/backend/fixtures/data_portal_test_case.py +++ b/tests/unit/backend/fixtures/data_portal_test_case.py @@ -9,6 +9,7 @@ class DataPortalTestCase(GenerateDataMixin, unittest.TestCase): @classmethod def setUpClass(cls): super().setUpClass() + cls.reinitialize_database() # danieljhegeman -- use clean db for each test case; negligible time cost TestDatabaseManager.initialize_db() def setUp(self): diff --git a/tests/unit/backend/fixtures/generate_data_mixin.py b/tests/unit/backend/fixtures/generate_data_mixin.py index c806fe38d880c..3c963d97baa17 100644 --- a/tests/unit/backend/fixtures/generate_data_mixin.py +++ b/tests/unit/backend/fixtures/generate_data_mixin.py @@ -1,7 +1,15 @@ +from sqlalchemy.orm import Session + from backend.corpora.common.entities import Collection, Dataset from backend.corpora.common.entities.geneset import Geneset +from backend.corpora.common.entities.collection_link import CollectionLink from backend.corpora.common.utils.db_session import db_session_manager -from tests.unit.backend.utils import BogusCollectionParams, BogusDatasetParams, BogusGenesetParams +from tests.unit.backend.utils import ( + BogusCollectionParams, + BogusDatasetParams, + BogusGenesetParams, + BogusDbCollectionLinkParams, +) class GenerateDataMixin: @@ -10,15 +18,15 @@ class GenerateDataMixin: """ @staticmethod - def delete_collection(uuid, visibility): + def delete_collection(uuid): with db_session_manager() as session: - col = Collection.get(session, (uuid, visibility)) + col = Collection.get(session, uuid) if col: col.delete() - def generate_collection(self, session, **params) -> Collection: + def generate_collection(self, session: Session, **params) -> Collection: _collection = Collection.create(session, **BogusCollectionParams.get(**params)) - self.addCleanup(self.delete_collection, _collection.id, _collection.visibility) + self.addCleanup(self.delete_collection, _collection.id) return _collection @staticmethod @@ -28,7 +36,7 @@ def delete_dataset(uuid): if dat: dat.delete() - def generate_dataset(self, session, **params) -> Dataset: + def generate_dataset(self, session: Session, **params) -> Dataset: _dataset = Dataset.create(session, **BogusDatasetParams.get(**params)) self.addCleanup(self.delete_dataset, _dataset.id) return _dataset @@ -40,7 +48,19 @@ def delete_geneset(uuid): if geneset: geneset.delete() - def generate_geneset(self, session, **params) -> Geneset: + def generate_geneset(self, session: Session, **params) -> Geneset: _geneset = Geneset.create(session, **BogusGenesetParams.get(**params)) self.addCleanup(self.delete_geneset, _geneset.id) return _geneset + + @staticmethod + def delete_link(uuid): + with db_session_manager() as session: + link = CollectionLink.get(session, uuid) + if link: + link.delete() + + def generate_link(self, session: Session, **params) -> CollectionLink: + _link = CollectionLink.create(session, **BogusDbCollectionLinkParams.get(**params)) + self.addCleanup(self.delete_link, _link.id) + return _link diff --git a/tests/unit/backend/fixtures/mock_aws_test_case.py b/tests/unit/backend/fixtures/mock_aws_test_case.py index 9a6bb54beceb7..1648f8321d233 100644 --- a/tests/unit/backend/fixtures/mock_aws_test_case.py +++ b/tests/unit/backend/fixtures/mock_aws_test_case.py @@ -5,6 +5,7 @@ import botocore import boto3 from moto import mock_s3 +from sqlalchemy.orm import Session from backend.corpora.common.corpora_config import CorporaConfig from backend.corpora.common.corpora_orm import DatasetArtifactFileType @@ -71,7 +72,7 @@ def create_s3_object( return s3object def generate_artifact( - self, session, dataset_id, artifact_type=DatasetArtifactFileType.H5AD, file_name="data", upload=False + self, session: Session, dataset_id, artifact_type=DatasetArtifactFileType.H5AD, file_name="data", upload=False ) -> DatasetAsset: file_name = f"{file_name}.{artifact_type.name}" if upload: @@ -85,7 +86,7 @@ def generate_artifact( s3_uri = DatasetAsset.make_s3_uri(self.bucket.name, dataset_id, file_name) return DatasetAsset.create(session, dataset_id, file_name, artifact_type, False, s3_uri) - def create_explorer_s3_object(self, session, dataset_id, upload=False): + def create_explorer_s3_object(self, session: Session, dataset_id, upload=False): file_name = f"{dataset_id}.cxg/" if upload: with tempfile.TemporaryDirectory() as temp_path: @@ -101,7 +102,9 @@ def create_explorer_s3_object(self, session, dataset_id, upload=False): s3_uri = f"s3://{self.cellxgene_bucket.name}/{file_name}" return DatasetAsset.create(session, dataset_id, file_name, DatasetArtifactFileType.CXG, False, s3_uri) - def generate_dataset_with_s3_resources(self, session, artifacts=True, explorer_s3_object=True, **params) -> Dataset: + def generate_dataset_with_s3_resources( + self, session: Session, artifacts=True, explorer_s3_object=True, **params + ) -> Dataset: dataset = self.generate_dataset(session, **params) if artifacts: for ext in DatasetArtifactFileType: diff --git a/tests/unit/backend/fixtures/test_db.py b/tests/unit/backend/fixtures/test_db.py index 88c6f8fd3f8bd..179d2454f68da 100644 --- a/tests/unit/backend/fixtures/test_db.py +++ b/tests/unit/backend/fixtures/test_db.py @@ -70,8 +70,9 @@ def _create_test_collections(self): ) self.session.add(collection) collection = DbCollection( - id="test_collection_id", + id="test_collection_id_revision", visibility=CollectionVisibility.PRIVATE.name, + revision_of="test_collection_id", owner="test_user_id", name="test_collection_name", description="test_description", @@ -118,6 +119,25 @@ def _create_test_collections(self): data_submission_policy_version="0", ) self.session.add(collection) + collection = DbCollection( + id="test_collection_with_link", + visibility=CollectionVisibility.PUBLIC.name, + owner="Someone_else", + name="test_collection_name", + description="test_description", + data_submission_policy_version="0", + ) + self.session.add(collection) + collection = DbCollection( + id="test_collection_with_link_revision", + revision_of="test_collection_with_link", + visibility=CollectionVisibility.PRIVATE.name, + owner="Someone_else", + name="test_collection_name", + description="test_description", + data_submission_policy_version="0", + ) + self.session.add(collection) self.session.commit() def _create_test_geneset(self): @@ -126,7 +146,6 @@ def _create_test_geneset(self): name="test_geneset", description="this is a geneset", collection_id="test_collection_id", - collection_visibility=CollectionVisibility.PUBLIC.name, ) self.session.add(geneset) @@ -136,7 +155,6 @@ def _create_test_geneset(self): name="test_geneset_with_dataset", description="this is a geneset with a dataset", collection_id="test_collection_id", - collection_visibility=CollectionVisibility.PUBLIC.name, datasets=[dataset], ) self.session.add(geneset) @@ -148,7 +166,6 @@ def _create_test_collection_links(self): DbCollectionLink( id=f"test_collection_{link_type.value}_link_id", collection_id="test_collection_id", - collection_visibility=CollectionVisibility.PUBLIC.name, link_name=f"test_{link_type.value}_link_name", link_url=f"http://test_{link_type.value}_url.place", link_type=link_type.name, @@ -158,11 +175,26 @@ def _create_test_collection_links(self): DbCollectionLink( id=f"test_collection_no_name_{link_type.value}_link_id", collection_id="test_collection_id", - collection_visibility=CollectionVisibility.PUBLIC.name, link_url=f"http://test_no_link_name_{link_type.value}_url.place", link_type=link_type.name, ) ) + self.session.add( + DbCollectionLink( + id=f"test_publish_revision_with_links__{link_type.value}_link", + collection_id="test_collection_with_link", + link_url=f"http://test_link_{link_type.value}_url.place", + link_type=link_type.name, + ) + ) + self.session.add( + DbCollectionLink( + id=f"test_publish_revision_with_links__revision_{link_type.value}_link", + collection_id="test_collection_with_link_revision", + link_url=f"http://test_link_{link_type.value}_url.place_REVISION", + link_type=link_type.name, + ) + ) self.session.commit() def _create_test_datasets(self): @@ -187,7 +219,6 @@ def _create_test_datasets(self): development_stage=[{"ontology_term_id": "test_obo", "label": "test_development_stage"}], collection_id="test_collection_id", explorer_url="test_url", - collection_visibility=CollectionVisibility.PUBLIC.name, cell_type=[{"label": "test_cell_type", "ontology_term_id": "test_opo"}], is_primary_data=IsPrimaryData.PRIMARY.name, x_normalization="test_x_normalization", @@ -215,7 +246,6 @@ def _create_test_datasets(self): development_stage=[{"ontology_term_id": "test_obo", "label": "test_development_stage"}], collection_id="test_collection_id_public_for_revision_one", explorer_url="test_url", - collection_visibility=CollectionVisibility.PUBLIC.name, schema_version="2.0.0", ) self.session.add(dataset) @@ -239,7 +269,6 @@ def _create_test_datasets(self): development_stage=[{"ontology_term_id": "test_obo", "label": "test_development_stage"}], collection_id="test_collection_id_public_for_revision_two", explorer_url="test_url", - collection_visibility=CollectionVisibility.PUBLIC.name, schema_version="2.0.0", ) self.session.add(dataset) @@ -262,7 +291,6 @@ def _create_test_datasets(self): ethnicity=[{"ontology_term_id": "test_obo", "label": "test_ethnicity"}], development_stage=[{"ontology_term_id": "test_obo", "label": "test_development_stage"}], collection_id="test_collection_id_not_owner", - collection_visibility=CollectionVisibility.PRIVATE.name, explorer_url="test_url", cell_type=[{"label": "test_cell_type", "ontology_term_id": "test_opo"}], is_primary_data=IsPrimaryData.PRIMARY.name, @@ -271,6 +299,33 @@ def _create_test_datasets(self): schema_version="2.0.0", ) self.session.add(dataset) + dataset = DbDataset( + id="test_publish_revision_with_links__revision_dataset", + revision=0, + name="test_dataset_name_revised", + organism=[{"ontology_term_id": "test_obo", "label": "test_organism"}], + tissue=[{"ontology_term_id": "test_obo", "label": "test_tissue"}], + assay=[{"ontology_term_id": "test_obo", "label": "test_assay"}], + disease=[ + {"ontology_term_id": "test_obo", "label": "test_disease"}, + {"ontology_term_id": "test_obp", "label": "test_disease2"}, + {"ontology_term_id": "test_obq", "label": "test_disease3"}, + ], + sex=[ + {"label": "test_sex", "ontology_term_id": "test_obo"}, + {"label": "test_sex2", "ontology_term_id": "test_obp"}, + ], + ethnicity=[{"ontology_term_id": "test_obo", "label": "test_ethnicity"}], + development_stage=[{"ontology_term_id": "test_obo", "label": "test_development_stage"}], + collection_id="test_collection_id_revision", + explorer_url="test_url_revised", + cell_type=[{"label": "test_cell_type", "ontology_term_id": "test_opo"}], + is_primary_data=IsPrimaryData.PRIMARY.name, + x_normalization="test_x_normalization", + x_approximate_distribution=XApproximateDistribution.NORMAL.name, + schema_version="2.0.0", + ) + self.session.add(dataset) self.session.commit() def _create_test_dataset_artifacts(self): @@ -356,8 +411,6 @@ def _create_test_dataset_artifacts(self): self.session.commit() - self.session.commit() - self.session.add( DbDatasetArtifact( id="test_dataset_artifact_for_revision_two_RDS_id", @@ -371,6 +424,19 @@ def _create_test_dataset_artifacts(self): self.session.commit() + self.session.add( + DbDatasetArtifact( + id="test_publish_revision_with_links__revision_artifact", + dataset_id="test_publish_revision_with_links__revision_dataset", + filename="test_filename", + filetype=DatasetArtifactFileType.H5AD.name, + user_submitted=True, + s3_uri=self.real_s3_file if self.real_data else self.fake_s3_file, + ) + ) + + self.session.commit() + def _create_test_dataset_processing_status(self): dataset_processing_status = DbDatasetProcessingStatus( id="test_dataset_processing_status_id", diff --git a/tests/unit/backend/utils.py b/tests/unit/backend/utils.py index 5ea0fe12a9a73..437bbd1333fa0 100644 --- a/tests/unit/backend/utils.py +++ b/tests/unit/backend/utils.py @@ -8,6 +8,7 @@ UploadStatus, ValidationStatus, XApproximateDistribution, + CollectionLinkType, ) @@ -63,7 +64,6 @@ def get(cls, **kwargs): cell_type=[{"ontology_term_id": "Hepatic-1A", "label": "liver"}], is_primary_data=IsPrimaryData.PRIMARY.name, collection_id="test_collection_id", - collection_visibility=CollectionVisibility.PUBLIC.name, explorer_url="test_url", x_normalization="normal", x_approximate_distribution=XApproximateDistribution.NORMAL.name, @@ -74,6 +74,19 @@ def get(cls, **kwargs): return bogus_data +class BogusDbCollectionLinkParams: + @classmethod + def get(cls, **kwargs): + bogus_data = dict( + collection_id="test_collection_id", + link_name="link_name", + link_url="link_url", + link_type=CollectionLinkType.DOI, + ) + bogus_data.update(**kwargs) + return bogus_data + + class BogusGenesetParams: @classmethod def get(cls, gene_count=6, **kwargs): @@ -95,7 +108,6 @@ def get(cls, gene_count=6, **kwargs): name=cls.generate_random_string(7), genes=genes, collection_id="test_collection_id", - collection_visibility=CollectionVisibility.PUBLIC.name, ) bogus_data.update(**kwargs) diff --git a/tests/unit/backend/wmg/data/test_extract.py b/tests/unit/backend/wmg/data/test_extract.py index 0ee4a9a9ddb33..f934f4005b5e4 100644 --- a/tests/unit/backend/wmg/data/test_extract.py +++ b/tests/unit/backend/wmg/data/test_extract.py @@ -21,7 +21,6 @@ def setUp(self): self.session, artifacts=True, explorer_s3_object=False, - collection_visibility="PUBLIC", collection_id=pub_collection.id, published=True, is_primary_data="PRIMARY", @@ -32,7 +31,6 @@ def setUp(self): self.session, artifacts=True, explorer_s3_object=False, - collection_visibility="PUBLIC", collection_id=pub_collection.id, published=True, is_primary_data="PRIMARY", @@ -45,7 +43,6 @@ def setUp(self): self.session, artifacts=True, explorer_s3_object=False, - collection_visibility="PUBLIC", collection_id=pub_collection.id, published=True, is_primary_data="PRIMARY", @@ -56,7 +53,6 @@ def setUp(self): self.session, artifacts=True, explorer_s3_object=False, - collection_visibility="PUBLIC", collection_id=pub_collection.id, published=True, is_primary_data="SECONDARY", @@ -68,7 +64,6 @@ def setUp(self): self.session, artifacts=True, explorer_s3_object=False, - collection_visibility="PUBLIC", collection_id=pub_collection.id, published=False, is_primary_data="PRIMARY", @@ -79,7 +74,6 @@ def setUp(self): self.session, artifacts=True, explorer_s3_object=False, - collection_visibility="PUBLIC", collection_id=pub_collection.id, published=True, is_primary_data="PRIMARY", @@ -92,7 +86,6 @@ def setUp(self): self.session, artifacts=True, explorer_s3_object=False, - collection_visibility="PRIVATE", collection_id=private_collection.id, published=True, is_primary_data="PRIMARY", diff --git a/tests/unit/backend/wmg/fixtures/test_snapshot.py b/tests/unit/backend/wmg/fixtures/test_snapshot.py index c0a2f4c031fbd..44b9747fe890c 100644 --- a/tests/unit/backend/wmg/fixtures/test_snapshot.py +++ b/tests/unit/backend/wmg/fixtures/test_snapshot.py @@ -172,7 +172,7 @@ def build_cell_orderings(cell_counts_cube_dir_, cell_ordering_generator_fn) -> D def create_dataset(dataset_id_ordinal: int) -> str: coll_id = f"dataset_id_{dataset_id_ordinal}_coll_id" with db_session_manager() as session: - if coll := Collection.get(session, (coll_id, CollectionVisibility.PUBLIC)): + if coll := Collection.get(session, coll_id): Collection.delete(coll) collection = DbCollection( @@ -186,7 +186,6 @@ def create_dataset(dataset_id_ordinal: int) -> str: id=f"dataset_id_{dataset_id_ordinal}", name=f"dataset_name_{dataset_id_ordinal}", collection_id=coll_id, - collection_visibility=CollectionVisibility.PUBLIC, ) session.add(dataset) return dataset.id diff --git a/tests/unit/processing_container/test_process.py b/tests/unit/processing_container/test_process.py index 9530374a176af..80f600c61e811 100644 --- a/tests/unit/processing_container/test_process.py +++ b/tests/unit/processing_container/test_process.py @@ -4,9 +4,6 @@ import requests -from backend.corpora.common.corpora_orm import ( - CollectionVisibility, -) from backend.corpora.dataset_processing import process from tests.unit.backend.fixtures.mock_aws_test_case import CorporaTestCaseUsingMockAWS @@ -63,9 +60,7 @@ def test_main(self, mock_download_from_dropbox): """ mock_download_from_dropbox.return_value = self.h5ad_raw - dataset = self.generate_dataset( - self.session, collection_id="test_collection_id", collection_visibility=CollectionVisibility.PUBLIC.name - ) + dataset = self.generate_dataset(self.session, collection_id="test_collection_id") process.process( dataset.id,