-
Notifications
You must be signed in to change notification settings - Fork 98
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(extender): Expose and reuse login and logout methods #2606
Conversation
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2606 +/- ##
==========================================
- Coverage 93.32% 93.30% -0.02%
==========================================
Files 102 102
Lines 10501 10546 +45
Branches 2250 2278 +28
==========================================
+ Hits 9800 9840 +40
- Misses 700 705 +5
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @zFernand0, thanks for this new extender functionality. As regression testing with an extender that logs into it own rest api worked fine but when I tried with zosmf / apiml combo I got this error that I don't see with released ZE using same profile
further info for recreation steps of getting the error: have team config with zosmf & base profile similar to below: On this branch I get the error above logging in, but works fine using the released ZE from marketplace saving the tokenType & tokenValue to the base profile. |
This PR will fix the issue 😋 |
thanks @zFernand0, can re-request review once pulling in those dep updates. |
Note:
|
The Lint failures seem unrelated since those were working before updating from |
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic looks good! Going to test shortly. Thanks @zFernand0!
In the meantime, I think I found the source of the lint stage failure - I added comments for those lines.
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good, thanks @zFernand0 😺
When testing this PR, I noticed there is text pre-filled in the input boxes for user/password. I think we want to retain the previous behavior of placeholder values that are "grayed out".
After
(here I have not entered a password yet, but it is trying to show "Password" which renders as dots)
packages/zowe-explorer/__tests__/__unit__/Profiles.extended.unit.test.ts
Outdated
Show resolved
Hide resolved
…pport Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
Unexpected behavior was corrected 😋
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
Quality Gate failedFailed conditions 7.8% Duplication on New Code (required ≤ 3%) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! thanks @zFernand0 for this update
Thank you, @JillieBeanSim for updating the package i18n 😋 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @zFernand0
The code duplication seems to be on new tests added. |
Proposed changes
Expose the login and logout methods to extenders
Release Notes
Milestone: 2.14.0
Changelog: Exposed the login and logout methods to extenders
Types of changes
What types of changes does your code introduce to Zowe Explorer?
Put an
x
in the boxes that applyChecklist
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 revieweryarn workspace vscode-extension-for-zowe vscode:prepublish
has been executedFurther comments
There is a dependency on the PR below:
ProfileInfo.updateProperty
zowe-cli#1983