chanzuckerberg / single-cell-explorer

Hosted version of cellxgene
MIT License
11 stars 2 forks source link

Redirect and toast for withdrawn datasets is regressed #648

Open MillenniumFalconMechanic opened 1 year ago

MillenniumFalconMechanic commented 1 year ago

Need

On view of a withdrawn dataset, the user is redirected to the dataset's corresponding collection detail page in the Data Portal and a toast is popped detailing the dataset's withdrawn status.

Defect Description

What I did:

What happened:

What I expected:

MillenniumFalconMechanic commented 11 months ago

Root Cause

When viewing a dataset in Explorer, Explorer requests the dataset's metadata from the Data Portal.

When viewing a tombstoned dataset, the Explorer is expecting a 200 response code and a tombstoned: true value in the response body from the Data Portal (see here).

The Data Portal is currently throwing a NotFoundHTTPException when requesting metadata for a tombstoned dataset (see here) which is causing Explorer's redirect back to the Data Portal to fail.

Code Updates

portal_api.get_dataset_identifiers

This code block is where the defect occurs. Either get_dataset_version_from_canonical needs to return tombstoned datasets or the NotFoundHTTPException needs to be swallowed such that the get_dataset_identifiers function can return a 200 plus dataset-related details to facilitate the redirect.

base_test.generate_dataset

There is currently a defect in base_test that affects tests covering the tombstoned: true case. See here:

explorer_url = f"http://base.url/{dataset_version_id.id}.cxg/"

explorer_url should be generated from the dataset ID if the dataset is published, or the dataset version ID if the dataset is private or part of a revision. The above needs to be updated to below, or similar:

cxg_id = dataset_id.id if publish else dataset_version_id.id
explorer_url = f"http://base.url/{cxg_id}.cxg/"

persistence_mock.get_collection_version_with_datasets

There is also a defect in persistence_mock that affects tests covering the tombstoned: true case. The call to _update_version_with_canonical (see here) should be updated to include get_tomstoned so that tombstoned datasets are included in the returned collection version. If tombstoned datasets are not included in the returned collection version, the code flows through to this NotFoundHTTPException, resulting in the same error flow as above.

rdev

Spinning Up the Data Portal and Explorer

To test this end-to-end in rdev, both the Portal and Explorer running "together" are required. See blocking ticket.