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

Fix secure credential management initialization issue #1979

Merged
merged 24 commits into from
Oct 26, 2022

Conversation

JillieBeanSim
Copy link
Contributor

@JillieBeanSim JillieBeanSim commented Oct 18, 2022

Proposed changes

Fix activation issues with Zowe Explorer seen in Theia and Eclipse Che

Milestone: 2.4.0

Changelog: Fixed activation and Refresh Extension issues in web based editors, ie. Theia.

Types of changes

What types of changes does your code introduce to Zowe Explorer?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • 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

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • PR title follows Conventional Commits Guidelines
  • PR Description is included
  • gif or screenshot is included if visual changes are made
  • yarn workspace vscode-extension-for-zowe vscode:prepublish has been executed
  • All checks have passed (DCO, Jenkins and Code Coverage)
  • 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
  • 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)

Further comments

rudyflores and others added 11 commits October 14, 2022 14:48
Signed-off-by: Rudy Flores <68666202+rudyflores@users.noreply.github.com>
Signed-off-by: Rudy Flores <68666202+rudyflores@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
@JillieBeanSim JillieBeanSim changed the base branch from fix-che-activation-issue to main October 18, 2022 19:02
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
@JillieBeanSim JillieBeanSim mentioned this pull request Oct 18, 2022
16 tasks
@codecov
Copy link

codecov bot commented Oct 18, 2022

Codecov Report

Base: 74.71% // Head: 74.50% // Decreases project coverage by -0.20% ⚠️

Coverage data is based on head (79c06f0) compared to base (f110876).
Patch coverage: 55.55% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1979      +/-   ##
==========================================
- Coverage   74.71%   74.50%   -0.21%     
==========================================
  Files          63       63              
  Lines        7070     7080      +10     
  Branches     1523     1534      +11     
==========================================
- Hits         5282     5275       -7     
- Misses       1781     1798      +17     
  Partials        7        7              
Impacted Files Coverage Δ
packages/zowe-explorer/src/ZoweExplorerExtender.ts 63.15% <0.00%> (ø)
packages/zowe-explorer/src/extension.ts 39.20% <0.00%> (-0.24%) ⬇️
packages/zowe-explorer/src/job/ZosJobsProvider.ts 86.35% <61.53%> (-2.57%) ⬇️
packages/zowe-explorer/src/Profiles.ts 36.22% <62.96%> (+0.16%) ⬆️
packages/zowe-explorer/src/dataset/DatasetTree.ts 81.66% <64.28%> (-0.13%) ⬇️
packages/zowe-explorer/src/uss/USSTree.ts 92.18% <64.28%> (-0.22%) ⬇️
packages/zowe-explorer/src/utils/ProfilesUtils.ts 62.57% <100.00%> (ø)

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: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
@JillieBeanSim JillieBeanSim added this to the 2.4.0 milestone Oct 18, 2022
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
@JillieBeanSim JillieBeanSim marked this pull request as ready for review October 18, 2022 19:58
Copy link
Member

@phaumer phaumer left a comment

Choose a reason for hiding this comment

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

@JillieBeanSim thanks a lot. The activation is now going through in Eclipse Che.

I see two remaining problems to fix:

(1) When loading a workspace for the first time I still get this warning message. This message is not useful for the end-user. They will not know what the problem is and why it was trying to read the file ''. We need to provide a meaningful message such as No global Zowe team configuration found. Use Zowe Explorer to create one.

2022-10-19_15-44-54

(2) I started Che with a Git repository that had a project-level Zowe config file. After the repo was loaded I was not able to use the Zowe Explorer Refresh Data Sets View button to get these projects listed. I would not see the project ones in the list to select them, only could create a new team-level one. I had to reload the full browser page and they would show up. Somehow the Refresh button does not load the project-level ones from the workspace.

JillieBeanSim and others added 4 commits October 20, 2022 12:05
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Rudy Flores <68666202+rudyflores@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
@phaumer
Copy link
Member

phaumer commented Oct 21, 2022

@JillieBeanSim thanks. (1) is resolved. Only problem left is (2) that project team config in my git repo is not recognized until a full browser reload.

Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Copy link
Member

@phaumer phaumer left a comment

Choose a reason for hiding this comment

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

Approving as team config work now.
Need more PRs to to fix (1) loading project-level configs and (2) dealing with invalid config files; being able to open them again in the editor

@phaumer
Copy link
Member

phaumer commented Oct 25, 2022

Testing this one more time I see that now the code runs an await vscode.commands.executeCommand("workbench.action.reloadWindow"); when clicking the dialog button when creating a global team configuration, which reloads the entire browser and all its extensions. However, it will then find the project-level profiles in addition to the global ones.

The remaining problem is that when the user does not want to create a global team config and only wants to use the project-level one then there is no indication for the user that a reload is required. Any idea how to approach that?

Otherwise, I think for now this is an acceptable workaround that can be merged as it improves the situation and fixes several issues. We should create a new issue for the next release to add supporting project configs at workspace creation as well as not doing a reloadWindow.

@JillieBeanSim
Copy link
Contributor Author

@phaumer the issue #1984 for the opening of the workspace with config problem.

@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.0% 0.0% Duplication

@zFernand0
Copy link
Member

Audit and Lint checks will be fixed in

@zFernand0 zFernand0 merged commit 438fb20 into main Oct 26, 2022
@zFernand0 zFernand0 deleted the fix/scm-initialization branch October 26, 2022 15:50
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.

4 participants