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

feat: add upload/download for sas content #550

Merged
merged 16 commits into from
Nov 1, 2023
Merged

Conversation

scottdover
Copy link
Contributor

@scottdover scottdover commented Oct 4, 2023

Summary
This makes a few changes around uploading/downloading files in the sas extension:

  • This adds an Upload context menu item to SAS content: This feature allows a user to upload one or more files to sas content
  • This adds a Download context menu item to SAS content: This allows you to download one or more files or folders, and specify where to save them
    • NOTE: This doesn't allow you to specify file name when you are specifying a single file to download. This is to make it such that we have consistency between downloading a single file or multiple files. The single file will be persisted with the same name from SAS content
  • This adds a Download context menu item to the results pane. Clicking this will allow the user to save the results as an html file
  • This introduces ResultsPanelSubscriptionProvider which is responsible for handling commands against the results panel

Testing

  • Tested uploading single and multiple files
  • Tested downloading single files, files + folder, files + folder + a subset of children in the folder
  • Tested downloading results (tested w/ tables & sgplots)

client/src/components/ResultPanel/ResultPanel.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
client/src/components/ResultPanel/index.ts Outdated Show resolved Hide resolved
client/src/components/ResultPanel/index.ts Show resolved Hide resolved
@scottdover scottdover force-pushed the feat/dlu-sas-content branch from 4e17c65 to 242da85 Compare October 10, 2023 15:56
@2TomLi 2TomLi added this to the 1.5.0 milestone Oct 12, 2023
@scottdover scottdover force-pushed the feat/dlu-sas-content branch from b5c867c to 0e50c8d Compare October 18, 2023 17:58
@Zhirong2022
Copy link
Collaborator

It failed to download files with Windows OS.
Error details:
Error running command SAS.downloadResource: Unable to resolve filesystem provider with relative file path 'c:\VSCCode\NewFile.sas'. This is likely caused by the extension that contributes SAS.downloadResource.

@Zhirong2022
Copy link
Collaborator

Local files are not shown on file selection window if trying to Upload one with Windows OS.

@Zhirong2022
Copy link
Collaborator

'Uploading files...' keeps showing in some scenarios.

  1. If trying to upload the same file folder and its subs on the second time to the same place
  2. If trying to upload a file with large size. It is more than 20M.

@scottdover
Copy link
Contributor Author

'Uploading files...' keeps showing in some scenarios.

  1. If trying to upload the same file folder and its subs on the second time to the same place
  2. If trying to upload a file with large size. It is more than 20M.

This has been resolved in 41016de. You should now see an error message and the Uploading files... message dismissed in both of these instances.

NOTE: The messaging is not ideal for a file > 20M (it just says something generic about an upload failure). That said, this is also the case w/ dragging + dropping a file. For now, I think this is probably sufficient but at some future point we may want to investigate making our failure messages a little more granular.

@scottdover
Copy link
Contributor Author

It failed to download files with Windows OS.
Error details:
Error running command SAS.downloadResource: Unable to resolve filesystem provider with relative file path 'c:\VSCCode\NewFile.sas'. This is likely caused by the extension that contributes SAS.downloadResource.

Local files are not shown on file selection window if trying to Upload one with Windows OS.

I'm currently waiting on a windows VM to be able to test/debug these.

@2TomLi 2TomLi modified the milestones: 1.5.0, 1.6.0 Oct 26, 2023
@scottdover
Copy link
Contributor Author

scottdover commented Oct 26, 2023

It failed to download files with Windows OS.
Error details:
Error running command SAS.downloadResource: Unable to resolve filesystem provider with relative file path 'c:\VSCCode\NewFile.sas'. This is likely caused by the extension that contributes SAS.downloadResource.

Local files are not shown on file selection window if trying to Upload one with Windows OS.

I'm currently waiting on a windows VM to be able to test/debug these.

These should both be resolved now (by 22dadbd and 6c39b07). The behavior of uploads has been tweaked a bit. On Mac, a user can use the Upload menu item and choose a mix of files and folders to upload.

On Windows & Linux, a user will be presented with two options: Upload File(s) and Upload Folder(s) (mixing and matching isn't allowed on those OSes). Those options will allow you to choose one type to upload at a time.

@scottdover
Copy link
Contributor Author

@Zhirong2022 This is ready to test again when you have a moment

@Zhirong2022
Copy link
Collaborator

@scottdover Thank you so much for your investigation and help. I verified the issues in Windows OS, it can upload files and folders through Upload file(s) and Upload folder(s) on the context menu. For the error prompt, it can be enhanced. I have no other concerns.

@Zhirong2022 Zhirong2022 added testing-complete Complete the pull requests testing and removed testing Test the pull requests labels Nov 1, 2023
@scottdover scottdover force-pushed the feat/dlu-sas-content branch from 6c39b07 to 0d90c24 Compare November 1, 2023 17:19
@scottdover scottdover merged commit 15d0f11 into main Nov 1, 2023
1 check passed
@scottdover scottdover deleted the feat/dlu-sas-content branch November 1, 2023 17:23
@Zhirong2022 Zhirong2022 added verified and removed testing-complete Complete the pull requests testing labels Nov 17, 2023
@Zhirong2022 Zhirong2022 added ready for release Code pushed, but not released in VS code marketplace yet and removed verified labels Jan 15, 2024
@2TomLi 2TomLi assigned scottdover and unassigned Zhirong2022 May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for release Code pushed, but not released in VS code marketplace yet
Projects
None yet
5 participants