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

Feature/uss/copy #2004

Merged
merged 44 commits into from
Dec 13, 2022
Merged

Feature/uss/copy #2004

merged 44 commits into from
Dec 13, 2022

Conversation

KutluOzel-b
Copy link
Contributor

Proposed changes

Adds multiselect copy / paste feature to uss view

Release Notes

#1549

Milestone:

Changelog:

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • [ x] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Updates to Documentation or Tests (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the reviewer

  • [ x] I have read the CONTRIBUTOR GUIDANCE wiki
  • [ x] PR title follows Conventional Commits Guidelines
  • [ x] PR Description is included
  • gif or screenshot is included if visual changes are made
  • [ x] yarn workspace vscode-extension-for-zowe vscode:prepublish has been executed
  • All checks have passed (DCO, Jenkins and Code Coverage)
  • [ x] I have added unit test and it is passing
  • I have added integration test and it is passing
  • There is coverage for the code that I have added
  • [ x] I have tested it manually and there are no regressions found
  • I have added necessary documentation (if appropriate)
  • Any PR dependencies have been merged and published (if appropriate)

Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
@codecov
Copy link

codecov bot commented Nov 9, 2022

Codecov Report

Base: 74.49% // Head: 74.63% // Increases project coverage by +0.13% 🎉

Coverage data is based on head (d85d3bc) compared to base (5c862dc).
Patch coverage: 88.31% of modified lines in pull request are covered.

❗ Current head d85d3bc differs from pull request most recent head 7117c10. Consider uploading reports for the commit 7117c10 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2004      +/-   ##
==========================================
+ Coverage   74.49%   74.63%   +0.13%     
==========================================
  Files          63       63              
  Lines        7341     7415      +74     
  Branches     1584     1601      +17     
==========================================
+ Hits         5469     5534      +65     
- Misses       1871     1880       +9     
  Partials        1        1              
Impacted Files Coverage Δ
packages/zowe-explorer/src/extension.ts 37.38% <33.33%> (-0.05%) ⬇️
packages/zowe-explorer/src/uss/actions.ts 82.27% <89.47%> (+1.39%) ⬆️
packages/zowe-explorer/src/uss/ZoweUSSNode.ts 94.64% <96.55%> (+0.16%) ⬆️
packages/zowe-explorer/src/globals.ts 91.30% <100.00%> (ø)
packages/zowe-explorer/src/uss/utils.ts 88.23% <100.00%> (+2.52%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
@KutluOzel-b KutluOzel-b linked an issue Nov 9, 2022 that may be closed by this pull request
KutluOzel-b and others added 4 commits November 9, 2022 14:34
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
@JillieBeanSim JillieBeanSim added this to the 2.5.0 milestone Nov 15, 2022
Copy link
Contributor

@JillieBeanSim JillieBeanSim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @KutluOzel-b I commented on a couple things that need to be changed

@@ -11,6 +11,10 @@ All notable changes to the "zowe-explorer-api" extension will be documented in t

## `2.4.0`

## TBD Release
Copy link
Contributor

@JillieBeanSim JillieBeanSim Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be above the latest release tag

@@ -1,6 +1,6 @@
{
"name": "@zowe/zowe-explorer-api",
"version": "2.5.0-SNAPSHOT",
"version": "2.6.0-SNAPSHOT",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be reverted back to 2.5.0-SNAPSHOT

@@ -47,7 +47,7 @@
"vscode": "^1.53.2"
},
"dependencies": {
"@zowe/zowe-explorer-api": "2.5.0-SNAPSHOT",
"@zowe/zowe-explorer-api": "2.6.0-SNAPSHOT",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same with revert to 2.5.0-SNAPSHOT

@@ -16,6 +16,10 @@ All notable changes to the "vscode-extension-for-zowe" extension will be documen

## `2.4.0`

## TBD Release
Copy link
Contributor

@JillieBeanSim JillieBeanSim Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move CHANGELOG above the latest release tag

@@ -1736,7 +1776,7 @@
"@types/promise-queue": "^2.2.0",
"@types/selenium-webdriver": "^3.0.4",
"@types/yargs": "^11.0.0",
"eslint-plugin-zowe-explorer": "2.5.0-SNAPSHOT",
"eslint-plugin-zowe-explorer": "2.6.0-SNAPSHOT",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.5.0-SNAPSHOT

@@ -1776,7 +1816,7 @@
"webpack-cli": "^3.3.11"
},
"dependencies": {
"@zowe/zowe-explorer-api": "2.5.0-SNAPSHOT",
"@zowe/zowe-explorer-api": "2.6.0-SNAPSHOT",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.5.0-SNAPSHOT

Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
@KutluOzel-b
Copy link
Contributor Author

@JillieBeanSim Thanks for reviewing and feedbacks. Adressed all in : 767a703

KutluOzel-b and others added 5 commits November 29, 2022 11:40
Signed-off-by: Kutlu <104970275+KutluOzel-b@users.noreply.github.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
KutluOzel-b and others added 3 commits December 8, 2022 15:15
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Copy link
Contributor

@JillieBeanSim JillieBeanSim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while testing, if I haven't copied anything and right-click on a directory and select paste the status bar mentions uploading but nothing is actually created. If I paste into a directory with file name already existing that I copied it automatically overwrites without notifying the user and allowing them to rename the file/folder and since the option is there even if folder is closed, this would be a good check to have.

I would still like to research if we can find a way to check the vsc clipboard and only offer paste if something is in there.

@@ -873,7 +886,7 @@ function initSubscribers(context: vscode.ExtensionContext, theProvider: IZoweTre
}
}

function getSelectedNodeList(node: IZoweTreeNode, nodeList: IZoweTreeNode[]): IZoweTreeNode[] {
export function getSelectedNodeList(node: IZoweTreeNode, nodeList: IZoweTreeNode[]): IZoweTreeNode[] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KutluOzel-b this function seems to be a duplicate of getSelectedNodeList() in shared/utils? We should only have one of these functions especially if they are exactly the same and I would suggest having it stay in the shared/utils file vs adding more to the extension.ts file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JillieBeanSim Thanks for the warning. This is caused by pulling from main branch. Keeping it in shared/utils

Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
packages/zowe-explorer/src/uss/ZoweUSSNode.ts Outdated Show resolved Hide resolved
packages/zowe-explorer/src/uss/ZoweUSSNode.ts Outdated Show resolved Hide resolved
packages/zowe-explorer/src/uss/ZoweUSSNode.ts Outdated Show resolved Hide resolved
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
Signed-off-by: KutluOzel-b <kutlu.ozel@broadcom.com>
@traeok
Copy link
Member

traeok commented Dec 12, 2022

I tested these changes and I'm able to copy files now on Windows 👍 Thanks for looking into that issue! I might have found an edge case - copying an empty directory into another directory. If a folder has files inside, I can copy it into another directory, but in the case of an empty folder nothing happens. No exceptions occur, but the directory structure remains the same.

It's unlikely that a user would want to copy an empty directory into another directory when they can just right-click the parent -> "Create Directory" - but just wanted to mention it.

@KutluOzel-b
Copy link
Contributor Author

I tested these changes and I'm able to copy files now on Windows 👍 Thanks for looking into that issue! I might have found an edge case - copying an empty directory into another directory. If a folder has files inside, I can copy it into another directory, but in the case of an empty folder nothing happens. No exceptions occur, but the directory structure remains the same.

It's unlikely that a user would want to copy an empty directory into another directory when they can just right-click the parent -> "Create Directory" - but just wanted to mention it.

Thanks @traeok for re-reviewing it and testing. I also hit this case and right now we don't have an api that checks if this directory is created on mf side, so decided to leave as it is and to be resolved when such Api is available/ or do this all operation on mf side . If copy paste is called after directory/node refresh it works. But again : this brings a needless and annoying refresh :/ since we can't decide to call refresh or not with lack of api.

traeok
traeok previously approved these changes Dec 12, 2022
t1m0thyj
t1m0thyj previously approved these changes Dec 13, 2022
Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the issues with copying on Windows 🙂

A few questions regarding the implementation:

  • When you right-click on a USS directory, should the menu item say "Copy Directory" instead of "Copy Files"?
  • If I have loaded directory /a in the USS tree that contains a subdirectory b, I don't see a way to create a copy of /a/b at the same level (/a/b (copy)). Should we add "Paste" to the right-click menu for the root USS node?

@JillieBeanSim JillieBeanSim dismissed their stale review December 13, 2022 15:54

changes made and approvals in

JillieBeanSim and others added 2 commits December 13, 2022 10:55
@KutluOzel-b KutluOzel-b dismissed stale reviews from t1m0thyj and traeok via 1cdd32c December 13, 2022 16:31
Signed-off-by: Kutlu <104970275+KutluOzel-b@users.noreply.github.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.6% 0.6% Duplication

@JillieBeanSim JillieBeanSim merged commit a5c12f1 into main Dec 13, 2022
@JillieBeanSim JillieBeanSim deleted the feature/uss/copy branch December 13, 2022 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

uss copy file/subdirectory to directory Copy/Paste and Multiple USS Files
4 participants