search

broadinstitute / single_cell_portal_core

Rails/Docker application for the Broad Institute's single cell RNA-seq data portal
https://singlecell.broadinstitute.org
BSD 3-Clause "New" or "Revised" License
62 stars 26 forks source link

Clear out existing cached filename on validation failure (SCP-5771) #2135

Closed bistline closed 4 weeks ago

bistline commented 1 month ago

BACKGROUND & CHANGES

This addresses a corner case where a previous valid file is cached after a CSFV failure, leading to an inconsistent state where it appears that you can save an obviously bad file. This can happen if a user uploads a valid file then uses the Replace button and supplies a file that fails validation. Now, if an uploaded file fails validation, the filename/upload button is cleared out, which also disables the save button.

MANUAL TESTING

  1. Boot as normal and sign in
  2. Go to the upload wizard for any study (non-AnnData is easier in this case)
  3. In any main tab (expression, metadata, or clustering) that will accept a file upload, add a known good file
  4. Once CSFV passes, use the Replace link and add a file that fails validation
  5. Confirm the previous file is cleared out, the upload button reverts back to Choose file and the Save button is disabled
codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 69.82%. Comparing base (6fd4ff1) to head (4e14c43). Report is 13 commits behind head on development.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2135/graphs/tree.svg?width=650&height=150&src=pr&token=HMWE5BO2a4&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute)](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2135?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute) ```diff @@ Coverage Diff @@ ## development #2135 +/- ## =============================================== + Coverage 69.80% 69.82% +0.02% =============================================== Files 328 328 Lines 27649 27651 +2 Branches 2395 2396 +1 =============================================== + Hits 19300 19307 +7 + Misses 8215 8210 -5 Partials 134 134 ``` | [Files with missing lines](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2135?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute) | Coverage Δ | | |---|---|---| | [...javascript/components/upload/FileUploadControl.jsx](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2135?src=pr&el=tree&filepath=app%2Fjavascript%2Fcomponents%2Fupload%2FFileUploadControl.jsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute#diff-YXBwL2phdmFzY3JpcHQvY29tcG9uZW50cy91cGxvYWQvRmlsZVVwbG9hZENvbnRyb2wuanN4) | `69.86% <100.00%> (+0.84%)` | :arrow_up: | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/broadinstitute/single_cell_portal_core/pull/2135/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=broadinstitute)
bistline commented 4 weeks ago

Functional test: works as described.

One papercut (probably pre-existing): Deleted an uploaded AnnData file but explore tab still populates even though file was deleted. I'm assuming not in scope, for this PR but, just in case it fits... (while making the screencapture, realized that the metadata has not been deleted - maybe this should be a separate ticket...)

Screen.Recording.2024-09-24.at.10.18.31.AM.mov

If you didn't have DelayedJob running, this is expected. All the deletion logic happens in the background.