Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to move file from workspace results in many errors #152

Open
craig-willis opened this issue Mar 31, 2021 · 4 comments
Open

Attempt to move file from workspace results in many errors #152

craig-willis opened this issue Mar 31, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@craig-willis
Copy link
Contributor

Problem:
When I try to move a file in the workspace via the UI, I see a long list of workspaces that I don't have access to and an equal number of 400/403 errors.

Test case:

  1. Login to https://dashboard.stage.wholetale.org
  2. Create a new tale
  3. Create a folder and upload a file to the workspace
  4. Attempt to move the file into the folder

Expected results:
I can move the file into the folder. Ideally I wouldn't have to browse through all available workspaces.

Actual results:
The "Move Item" dialog displays a long list of workspaces, many of them with empty titles, generating 400 and 403 errors. I don't see my workspace (likely due to #150).

@craig-willis craig-willis added the bug Something isn't working label Mar 31, 2021
@Xarthisius
Copy link
Contributor

Xarthisius commented Mar 31, 2021

This was caused by an inconsistent state of public flag between tale and its workspace. I fixed the db by applying:

#!girder-shell
for workspace in Folder().find({"meta.taleId": {"$exists": True}}):
    tale_id = str(workspace["meta"]["taleId"])
    tale = Tale().load(tale_id, force=True)
    if not tale:
        Folder().remove(workspace)
        continue
    workspace.update(
        {
            "access": tale["access"],
            "public": tale["public"],
            "publicFlags": tale.get("publicFlags", []),
        }
    )
    Folder().save(workspace)

TBD if that's some ancient state issue or a latent bug in ACLs handling on Tales.

@craig-willis
Copy link
Contributor Author

Thanks, @Xarthisius. That fixes the 400/403 errors. I'm still unable to move to my active workspace because it doesn't appear on the list due to #150 (I have access to 81 tales, but only 50 show)

I also noticed that it's using the default sort order, which is by the name (i.e., folder UUID), which is equivalent to created date, I expect. Given that the UI is working with folder objects instead of actual tales, perhaps we should sort by updated date descending in this case. That way my most active tales would be at the top of the list?

@craig-willis
Copy link
Contributor Author

#156 Fixes part of this -- I now see only the workspaces that I have access to. Testing that PR I noticed that we are using two different approaches to get workspaces for the "Move To..." modal and the "Select Data..." modal. "Move To" calls GET /folder (default sort UUID ascending) while "Select Data" uses GET /tale (default sort tale name descending).

Can we make these consistent? Both lists use the same endpoint (probably GET /tale) with the same sort order (name ascending?)

@bodom0015
Copy link
Member

bodom0015 commented Jun 23, 2021

Looked into this again and this would likely constitute a new modal... the existing "Move To" dialog works for more than just workspaces, so we'd probably still need to keep this modal around for moving things into and out of the user's Home folder.

Also noting that the reason for the difference here is context: "Move To" only exists in the file-explorer, which operates on files and folders. In an ideal world, the file-explorer has no knowledge of Tales, since files/folders can already exist in Girder without a Tale. That said, we can compute properties for the file-explorer based on the Tale's properties without directly coupling their internals: see https://github.com/whole-tale/ngx-dashboard/blob/master/src/app/%2Brun-tale/run-tale/tale-files/tale-files.component.html#L18-L39

I'm not sure how (or if it's even a good idea) to design a modal that encompasses both of these functionalities, but there are some ways we could do that if it would consolidate or make things more clear.

Options might include:

  1. Expand "Select Data..." so that it also shows the user's Home folder, remove "Move To..." for all cases
  2. Rewrite "Move To..." entirely, and add more WT-specific assumptions about files being tied to Tales

TL;DR

  • "Move To..." will export data from Home or a Tale's workspace to another folder in Home or a Tale's workspace, so it is more general than workspaces
  • "Select Data..." will import data into the Tale's workspace from another Tale's workspace only operates on workspaces, so we can make the assumption here that we are listing Tales (not folders)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants