fix: various fixes for the Run > Files view #78

bodom0015 · 2020-07-02T22:13:10Z

Problem

Fixes #89 - Files upload twice
Fixes #50 - Adding External Data would return a 400
Fixes #48 - Uploading a file would result in a 400, even though the file appeared to upload successfully
Fixes #41 - Create Folder in Home actually creates the folder in the Tale Workspace
Fixes #37 - Files cannot be removed/downloaded until after refreshing
Fixes #33 - Data registration dropdown should auto-select first result
Fixes #31 - Show previously-added data on select-data-modal
Fixes #30 - Add button should be disabled for read-only Tales
Fixes #28 - External Data shows nothing even after adding a dataset

Approach

Remove files from the upload queue after they complete
Remove extra call to Girder's /chunk endpoint - logic for initUpload appears to upload correctly (Upload file results in 400 bad request #48)
Fix createFolder to use the same logic for choosing parentId as the uploadFiles function uses (Creating a folder in Home results with a new folder in workspace #41)
Allow files to be removed and/or downloaded without needing to first refresh the view (Downloading files that were uploaded to Home/Workspace works only after reload #37)
Auto-select first dropdown result when registering data ([registration modal] Dropdown should auto select first result #33)
Rework select-data-modal to show Selected Data on the right, as was previously true in the Ember.js dashboard ([View > Files > External Data] Add from Catalog modal doesn't display currrent dataSet #31)
Disable Add button on Run > Files for read-only Tales ([View > Files > External Data] (+) button is not disabled for readonly Tale #30)
Fixed display/fetch of External Data ([View > Files > External Data] nothing shows up #28)
Remove extraneous debug line from Run > Interact view

How to Test

Prerequisites: at least one Tale created, LIGO Tale

Checkout this branch locally, rebuild the dashboard
Login to the WholeTale Dashboard
Choose a Tale you own and navigate to the Run > Files view
Select Home and upload a file
- You should see that your file is uploaded successfully (Upload file results in 400 bad request #48)
- You should see that your file has a Last Modified date (this indicates that it is now displaying an Item, as expected, and not a File)
Without refreshing the view, expand the dropdown beside the new file and choose Remove
- The file should be removed from view (Downloading files that were uploaded to Home/Workspace works only after reload #37)
Select Home, create a folder, and refresh the page
- You should still see the Home nav is still selected
- Your new folder should still exist (Creating a folder in Home results with a new folder in workspace #41)
Select External Data, click (+), and choose "Web (DOI or URL)"
- The data registration modal should open
Enter 10.5065/D6862DM8 and search
- You should see the Humans & Hydrology dataset is found
- The dataset should be automatically selected in the dropdown ([registration modal] Dropdown should auto select first result #33)
Click Register to close the modal, then click (+) again, and choose "WT Data Catalog"
- The Select Data modal should open
Click the "Humans & Hydrology" dataset to select it, optionally double-click to drill down into it and select individual folders/files, and click "Add Selected. and then
- The modal should close
- You should see the new dataset(s) appear on the right after adding it
Click "Save" below to confirm
- You should see the new dataset(s) appear in the view shortly after the modal closes
- You should no longer see 400 errors appear in the console when adding the dataset (Adding external data results in 400 #50)
Click (+) again, and choose "WT Data Catalog" once more
- You should see the data you've previously added appears on the right by default ([View > Files > External Data] Add from Catalog modal doesn't display currrent dataSet #31)
Refresh the page
- You should still see the External Data nav is still selected
- Your new dataset should still exist on the Tale ([View > Files > External Data] nothing shows up #28)
Navigate to the Run > Files view for the LIGO Tale
- You should see the (+) button at the top-right is disabled ([View > Files > External Data] (+) button is not disabled for readonly Tale #30)

Xarthisius · 2020-07-06T17:29:37Z

With this pull request #41 gets reverted. Now when I upload files to Workspace they end up in Home.

bodom0015 · 2020-07-06T20:54:44Z

@Xarthisius Good catch! #41 was actually about creating folders, but was likely the same problem as it uses the same logic to determine the parentId to use for the operation. There was a typo in the nav it was searching for: tale_workspaces should have instead been tale_workspace. Thank you for pointing this out 👍

craig-willis · 2020-07-07T18:04:34Z

Looking good. Testing no. 10 from your list, I got into a very strange state.

I selected the checkbox for the top level "Humans and Hydrology" dataset and that worked fine. I went to deselect it and instead select a single item from within the folder and things got weird. I can uncheck the checkbox, but clicking "Select" results in a new entry for the top level "Humans and Hydrology" folder.

I can select a single file from within the dataset, but the top level folder remains:

Basic test case:

Select External Data > WT Catalog, check "Humans and Hydrology" folder, click "Select". You should see a single folder in external data.
Select External Data > WT Catalog, uncheck "Humans and Hydrology" folder, click "Select". You should see two folders in external data.
Select External Data > WT Catalog, select "Humans and Hydrology" folder, check "usco2005.xls", click "Select". You should see two folders and the file.

bodom0015 · 2020-07-13T17:41:16Z

@craig-willis good catch, I've noticed this before... I think that modal needs to be reworked quite a bit for #31 as well. The current implementation seems to get confused between what was previously added and what you're trying to add now. The navigation bits can still be preserved, but the additional panel showing a list of what has already been selected may help keep things straight.

…into fix-file-upload-bad-request

bodom0015 · 2020-07-16T02:07:11Z

So I have a fix for the select-data-modal, but am not sure whether to amend this PR or to make a new one.

@Xarthisius any preference here? Should I make a new PR after this one gets merged, since there's already a lot going on in this one? What do you think?

Xarthisius · 2020-07-16T17:12:50Z

So I have a fix for the select-data-modal, but am not sure whether to amend this PR or to make a new one.

@Xarthisius any preference here? Should I make a new PR after this one gets merged, since there's already a lot going on in this one? What do you think?

Please add it here and let's review it in a one go.

ThomasThelen

Re-tested with the new commits and things look great!

craig-willis · 2020-07-20T19:51:12Z

Ran into an unexpected behavior while testing the fix. Should "Remove" remove the file/dataset from My External Data (i.e., deselect) or remove from the system catalog? Currently, the "Remove" menu calls DELETE https://girder.local.wholetale.org/api/v1/folder/, effectively removing the dataset/file from Girder. Selected dataset or file still remain selected in "Selected Data"

To repeat:

Register Humans and Hydrology (10.5065/D6862DM8)
Select + > My External Data
Browse to Humans and Hydrology > Add Selected, Save
H&H appears in external data
Select "Remove"
Select "My External Data" -- H&H appears in Selected list
Go to https://girder.local.wholetale.org/ > Whole Tale Catalog > Whole Tale Catalog -- dataset has been removed from catalog

Something I can't readily repeat -- when I select an individual file in the folder, sometimes the folder is automatically selected for me. I'll try to get a repeatable test case.

bodom0015 · 2020-07-29T19:56:41Z

@craig-willis ahhh, I think this was an oversight on my part. While fixing #30, I think I accidentally made this view writeable.

Will make a new commit to hide the "Remove" option there, is it is in fact deleting the dataset from Girder.

…into fix-file-upload-bad-request

Xarthisius · 2020-08-19T17:02:18Z

@bodom0015 I went through the test plan (and some more). Very nice! I filed issues for some minor things I've found. In particular #89 looks like something you might want to address here, but I'm open to merging this PR as-is and dealing with it in a separate branch. Please let me know what would you prefer.

bodom0015 · 2020-08-20T21:01:39Z

Good catch on these! I've pushed a fix for #89 as well as another minor bug I noticed.

Apparently we could not upload the same file twice in a row due to using the onChange handler.. since the selected file technically wasn't changing, the upload would never trigger :D

Clearing out the selected file(s) after it/them to the queue seems to workaround the issue

bodom0015 · 2020-08-20T21:04:24Z

src/app/+run-tale/run-tale/tale-files/tale-files.component.ts

@@ -209,6 +209,8 @@ export class TaleFilesComponent implements OnInit, OnChanges {
          });*/
        });
      });
+
+      this.uploadQueue.delete(upload);


While it did successfully workaround the issue, I am amazed that this did not trigger a concurrent modification exception.. in other languages, deleting an item from an array that you're looping over is not allowed and likely not advisable here

bodom0015 added 3 commits July 2, 2020 15:36

fix: external data, create folder, and upload file

51fbac6

feat: auto-select first dropdown item during data registration

3bc6175

fix: remove instanceStatus integer debug output

b3fdab0

bodom0015 requested review from Xarthisius, craig-willis and ThomasThelen July 2, 2020 22:13

fix: allow files to be deleted without refresh

3a1084a

fix: "tale_workspaces" should have been "tale_workspace"

819df99

bodom0015 mentioned this pull request Jul 6, 2020

"Move to" on a file yields error #40

Closed

bodom0015 added 3 commits July 15, 2020 16:01

Merge branch 'master' of https://github.com/whole-tale/ngx-dashboard …

3981450

…into fix-file-upload-bad-request

fix: select-data-modal now shows a running list of all added files

ebb2411

fix: simplified add/remove logic in select-data-modal

ef0d7a2

bodom0015 added 2 commits July 16, 2020 14:36

fix: disable add button on Run > Files for read-only Tales

4406dfe

fix: add subheaders for 'DATA CATALOG' and 'SELECTED DATA'

8952703

ThomasThelen approved these changes Jul 17, 2020

View reviewed changes

bodom0015 added 2 commits July 29, 2020 15:03

fix: make sure external nav dropdown options do not allow writes

a43c7a2

Merge branch 'master' of https://github.com/whole-tale/ngx-dashboard …

d58df37

…into fix-file-upload-bad-request

This was referenced Aug 19, 2020

Files upload twice #89

Closed

Excessive spacing in the select-data modal's tree hierarchy #90

Closed

Rename "Copy To..." to "Copy" (or enable functionality to select target if it exists) #91

Closed

Move to... option on files doesn't work #92

Closed

bodom0015 added 2 commits August 20, 2020 15:58

fix: remove completed uploads from the queue

99cead9

fix: clear out the file upload input between uploads

5f251a4

bodom0015 commented Aug 20, 2020

View reviewed changes

Xarthisius approved these changes Aug 25, 2020

View reviewed changes

Xarthisius merged commit 6f2c7f0 into master Aug 25, 2020

Xarthisius deleted the fix-file-upload-bad-request branch September 18, 2020 16:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: various fixes for the Run > Files view #78

fix: various fixes for the Run > Files view #78

bodom0015 commented Jul 2, 2020 •

edited

Loading

Xarthisius commented Jul 6, 2020 •

edited

Loading

bodom0015 commented Jul 6, 2020

craig-willis commented Jul 7, 2020

bodom0015 commented Jul 13, 2020 •

edited

Loading

bodom0015 commented Jul 16, 2020

Xarthisius commented Jul 16, 2020

ThomasThelen left a comment

craig-willis commented Jul 20, 2020

bodom0015 commented Jul 29, 2020

Xarthisius commented Aug 19, 2020

bodom0015 commented Aug 20, 2020 •

edited

Loading

bodom0015 Aug 20, 2020 •

edited

Loading

fix: various fixes for the Run > Files view #78

fix: various fixes for the Run > Files view #78

Conversation

bodom0015 commented Jul 2, 2020 • edited Loading

Problem

Approach

How to Test

Xarthisius commented Jul 6, 2020 • edited Loading

bodom0015 commented Jul 6, 2020

craig-willis commented Jul 7, 2020

bodom0015 commented Jul 13, 2020 • edited Loading

bodom0015 commented Jul 16, 2020

Xarthisius commented Jul 16, 2020

ThomasThelen left a comment

Choose a reason for hiding this comment

craig-willis commented Jul 20, 2020

bodom0015 commented Jul 29, 2020

Xarthisius commented Aug 19, 2020

bodom0015 commented Aug 20, 2020 • edited Loading

bodom0015 Aug 20, 2020 • edited Loading

Choose a reason for hiding this comment

bodom0015 commented Jul 2, 2020 •

edited

Loading

Xarthisius commented Jul 6, 2020 •

edited

Loading

bodom0015 commented Jul 13, 2020 •

edited

Loading

bodom0015 commented Aug 20, 2020 •

edited

Loading

bodom0015 Aug 20, 2020 •

edited

Loading