This repository was archived by the owner on Jun 5, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 20
Fix multi user extension login for setup environment and create environment flows #3907
Merged
bbonaby
merged 2 commits into
main
from
user/bbonaby/allow-multiple-environments-from-same-provider
Oct 3, 2024
Merged
Fix multi user extension login for setup environment and create environment flows #3907
bbonaby
merged 2 commits into
main
from
user/bbonaby/allow-multiple-environments-from-same-provider
Oct 3, 2024
+304
−128
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sshilov7
reviewed
Oct 1, 2024
sshilov7
approved these changes
Oct 1, 2024
dhoehna
reviewed
Oct 1, 2024
tools/SetupFlow/DevHome.SetupFlow/Converters/CreationStateKindToVisibilityConverter.cs
Outdated
Show resolved
Hide resolved
dhoehna
reviewed
Oct 1, 2024
...s/SetupFlow/DevHome.SetupFlow/ViewModels/Environments/EnvironmentCreationOptionsViewModel.cs
Show resolved
Hide resolved
dhoehna
reviewed
Oct 1, 2024
...s/SetupFlow/DevHome.SetupFlow/ViewModels/Environments/EnvironmentCreationOptionsViewModel.cs
Show resolved
Hide resolved
dhoehna
reviewed
Oct 1, 2024
krschau
reviewed
Oct 2, 2024
|
|
||
| public ComputeSystemProvider Provider { get; set; } | ||
|
|
||
| public DeveloperIdWrapper CurrentDeveloperId { get; set; } |
Contributor
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.
Do we handle logout already? If we're reloading the environments on page load, we're probably fine, since the user will have to move to a different page to logout. If not, we might need to listen to DevId logout event
Contributor
Author
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.
Thats a good call out we'd probably need listen for the event and refresh the page accordingly. I'll create an issue for that since we probably don't want this PR getting too big
huzaifa-d
reviewed
Oct 2, 2024
tools/SetupFlow/DevHome.SetupFlow/ViewModels/Environments/ComputeSystemsListViewModel.cs
Show resolved
Hide resolved
…ad used the weak event listener
huzaifa-d
approved these changes
Oct 3, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the pull request
Update setup target and create environment flows to allow an extension with multiple logged in developerIds to show their environments. Before this change both places use to use the first DeveloperId it find from the extension to query for environments. This caused an issue for extensions like the azure extension where the user might be logged in using a work or school account as well as a personal MSA account.
Changes:
DevHomeChoiceSetWithDynamicRefresh.cswhen the combo boxes unload. But we don't add them back on load which is probably how this issue [Environments reliability] Dev Box operations: When I switch between different Projects for Dev Box creation, the Pool selection is not updated #3638 occurred. So I updated it to prevent that from happening.ComputeSystemsListViewModelwe now filter out compute systems that don't support environments in its constructor.SetupTargetViewModel.cs'sUpdateListViewModelListmethod to create aComputeSystemsListViewModelfor each developerId that returns a list of compute systems.IsAdaptiveCardSessionLoadedto be enums, now that there are multiple states the initial creation page that shows the adaptive card can be in. A converter was created to allow controls to be visible/collapsed depending on the state of the initial creation page.ExtensionAdaptiveCardSession2object. TheEnvironmentCreationOptionsViewModelthat holds it sends the adaptive card to different views depending on the state of the flow.Video of me showing how the create environment flow works with multiple users logged into the Azure extension:
Video.showing.multi.user.flow.when.an.extension.has.multiple.developerIds.mp4
Video of me showing that users can now see all their dev boxes when multiple users are logged in. In this scenario I don't have access to DevBoxes on my secondary msa, but you can see that doesn't prevent us from showing others
video.showing.multi.account.behavior.for.setup.environment.flow.mp4
References and relevant issues
Detailed description of the pull request / Additional comments
Validation steps performed
PR checklist