Fix logout to give base profile precedence over parent profile by t1m0thyj · Pull Request #3076 · zowe/zowe-explorer-vscode

t1m0thyj · 2024-08-28T17:55:12Z

Proposed changes

Follow up to #3019 - fixes behavior of logout action when token is defined in both base profile and parent profile.

TODO:

Test APIML login/logout scenarios
- Token already on base profile
- Token already on parent profile
- Token already on both base and parent profile
- Token not stored yet

Release Notes

Milestone: V3

Changelog: TBD

Types of changes

Bug fix (non-breaking change which fixes an issue)
Enhancement (non-breaking change which adds or improves functionality)
Breaking change (a change that would cause existing functionality to not work as expected)
Documentation (Markdown, README updates)
Other (please specify above in "Proposed changes" section)

Checklist

General

I have read the CONTRIBUTOR GUIDANCE wiki
All PR dependencies have been merged and published (if applicable)
A GIF or screenshot is included in the PR for visual changes
The pre-publish command has been executed:
- v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
- v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

There is coverage for the code that I have added
I have added new test cases and they are passing
I have manually tested the changes

Deployment

I have added developer documentation (if applicable)
Documentation should be added to Zowe Docs
- If you're an outside contributor, please post in the #zowe-doc Slack channel to coordinate documentation.
- Otherwise, please check with the rest of the squad about any needed documentation before merging.
These changes may need ported to the appropriate branches (list here):

Further comments

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

codecov · 2024-08-28T18:12:01Z

Codecov Report

Attention: Patch coverage is 94.64286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 92.59%. Comparing base (c4c9478) to head (4fda88f).
Report is 17 commits behind head on next.

Files with missing lines	Patch %	Lines
...owe-explorer-api/src/vscode/ZoweVsCodeExtension.ts	93.75%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #3076      +/-   ##
==========================================
+ Coverage   92.57%   92.59%   +0.02%     
==========================================
  Files         113      113              
  Lines       11638    11660      +22     
  Branches     2570     2488      -82     
==========================================
+ Hits        10774    10797      +23     
+ Misses        862      861       -1     
  Partials        2        2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

traeok

These changes LGTM, thanks @t1m0thyj for the fix!

zFernand0

Changes makes sense to me 😋

I left a comment, however this is all theoretical, so it might be nice to discuss offline (or during standup)

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

…nto fix/multi-apiml-logout Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

Initial concerned was discussed and addressed in 4f24050

…nto fix/multi-apiml-logout Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

…nto fix/multi-apiml-logout Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

…ined in the IProfile Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

github-actions · 2024-09-16T21:05:16Z

📅 Suggested merge-by date: 9/30/2024

zFernand0

LGTM! 😋

Not sure if I should be approving as this is something that I was very closely watching (and at times "fixing" tests for)

zFernand0 · 2024-09-17T17:25:07Z

            // Note: It should be expected that nested profiles within this service profile will have their credentials updated.
            const profIndex = cache.allProfiles.findIndex((profile) => profile.name === opts.serviceProfile.name);
-            cache.allProfiles[profIndex] = opts.serviceProfile;
+            cache.allProfiles[profIndex] = { ...cache.allProfiles[profIndex], profile: opts.serviceProfile.profile };


Thanks for fixing.
This seems to keep the name and type of serviceProfile in the cache, while updating the contents of the profiles (say with tokenType and tokenValue).

zFernand0 · 2024-09-17T17:27:06Z

-        // it("should prefer base profile when it exists and it has tokenValue in secure array", async () => {});
-        // it("should prefer base profile when it exists, it does not have tokenValue in its secure array, and service profile is flat", async () => {});
-        // it("should prefer parent profile when base profile does not exist and service profile is nested", () => {});
-        // it("should cancel the operation if the base profile does not exist and service profile is flat", () => {});


Thanks for adding this set of "e2e" tests 🙏🏽

zFernand0 · 2024-09-17T17:28:49Z

Seems like the workaround above was not being used after the merge from next?
either way, thanks for cleaning it up 🙏🏽

zFernand0 · 2024-09-17T17:30:34Z

+     *  If there is no API registered for the profile type, this method defaults the login behavior to that of the APIML.
+     * @param {BaseProfileAuthOptions} opts Object defining options for base profile authentication
+     */
+    public static async ssoLogin(opts: BaseProfileAuthOptions): Promise<boolean> {


I completely missed that this would've been a breaking change to extenders.
Great catch! 🙏🏽

Thanks for deprecating the old login/-out function in favor of the new ssoLogin/-out 🥳

traeok

Changes LGTM, thanks @t1m0thyj and @zFernand0 😋

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

sonarqubecloud · 2024-09-19T13:44:45Z

Quality Gate failed

Failed conditions
15.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

zFernand0

LGTM! 🙏🏽

Fix logout to give base profile precedence over parent profile

5ca75e0

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

t1m0thyj requested review from JillieBeanSim, awharn, rudyflores, traeok and zFernand0 August 28, 2024 17:55

pull-request-size Bot added the size/S label Aug 28, 2024

t1m0thyj self-assigned this Aug 28, 2024

Fix ProfilesCache unit tests

a9b0ec8

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

traeok previously approved these changes Aug 29, 2024

View reviewed changes

zFernand0 previously requested changes Aug 29, 2024

View reviewed changes

Comment thread packages/zowe-explorer-api/__tests__/__unit__/profiles/ProfilesCache.unit.test.ts

Comment thread packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts Outdated

Comment thread packages/zowe-explorer-api/src/vscode/ZoweVsCodeExtension.ts Outdated

fix(fsp): Do not fetch changes during conflict; use custom context val

60ff771

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

traeok dismissed their stale review via 60ff771 August 30, 2024 16:58

pull-request-size Bot added size/M and removed size/S labels Aug 30, 2024

t1m0thyj marked this pull request as draft August 30, 2024 23:52

t1m0thyj added 2 commits August 30, 2024 20:17

WIP Refactor login/logout methods to use interface

4f24050

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

Merge branch 'next' into fix/multi-apiml-logout

9df6a85

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

pull-request-size Bot added size/L and removed size/M labels Aug 31, 2024

t1m0thyj assigned zFernand0 Aug 31, 2024

zFernand0 added 2 commits September 6, 2024 10:11

Merge branch 'next' of https://github.com/zowe/zowe-explorer-vscode i…

d3d48be

…nto fix/multi-apiml-logout Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

test: fix unit tests and start addressing TODO items 😋

1774640

Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

zFernand0 mentioned this pull request Sep 6, 2024

V3 pre-release Sept week 1 #3094

Merged

15 tasks

zFernand0 added 4 commits September 6, 2024 11:27

Merge branch 'next' of https://github.com/zowe/zowe-explorer-vscode i…

84994b8

…nto fix/multi-apiml-logout Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

chore: update changelog

14dd916

Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

Merge branch 'next' of https://github.com/zowe/zowe-explorer-vscode i…

8f1d4b5

…nto fix/multi-apiml-logout Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

tests: add more unit tests around login

bec0933

Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

fix: Issue where osLoc is not specified, and the updProfile being def…

ea82408

…ined in the IProfile Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>

pull-request-size Bot added size/XL and removed size/L labels Sep 10, 2024

t1m0thyj added 3 commits September 16, 2024 12:07

Add e2e unit tests for fetching base profiles

b82f83e

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

Merge branch 'next' into fix/multi-apiml-logout

2fb5f7a

Deprecate methods to avoid breaking change

df87926

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

t1m0thyj marked this pull request as ready for review September 16, 2024 21:05

t1m0thyj requested review from traeok and zFernand0 September 16, 2024 21:05

zFernand0 added this to the v3 pre-releases milestone Sep 17, 2024

zFernand0 previously approved these changes Sep 17, 2024

View reviewed changes

traeok previously approved these changes Sep 18, 2024

View reviewed changes

Merge branch 'next' into fix/multi-apiml-logout

4fda88f

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

t1m0thyj dismissed stale reviews from traeok and zFernand0 via 4fda88f September 19, 2024 13:43

t1m0thyj requested review from traeok and zFernand0 September 19, 2024 13:44

traeok approved these changes Sep 19, 2024

View reviewed changes

zFernand0 approved these changes Sep 19, 2024

View reviewed changes

t1m0thyj merged commit a0c779f into next Sep 19, 2024

t1m0thyj deleted the fix/multi-apiml-logout branch September 19, 2024 15:00

Conversation

t1m0thyj commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Release Notes

Types of changes

Checklist

Further comments

Uh oh!

codecov Bot commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

traeok left a comment

Choose a reason for hiding this comment

Uh oh!

zFernand0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 16, 2024

Uh oh!

zFernand0 left a comment

Choose a reason for hiding this comment

Uh oh!

zFernand0 Sep 17, 2024

Choose a reason for hiding this comment

Uh oh!

zFernand0 Sep 17, 2024

Choose a reason for hiding this comment

Uh oh!

zFernand0 Sep 17, 2024

Choose a reason for hiding this comment

Uh oh!

zFernand0 Sep 17, 2024

Choose a reason for hiding this comment

Uh oh!

traeok left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Sep 19, 2024

Quality Gate failed

Uh oh!

zFernand0 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

t1m0thyj commented Aug 28, 2024 •

edited

Loading

codecov Bot commented Aug 28, 2024 •

edited

Loading