This repository was archived by the owner on Jul 9, 2025. It is now read-only.
fix: do not require multiple sign ins for tenant selection #6628
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
a-b-r-o-w-n
requested review from
GeoffCoxMSFT,
beyackle,
boydc2014,
cwhitten,
hatpick,
srinaath and
tonyanziano
as code owners
beyackle
suggested changes
Mar 31, 2021
Contributor
beyackle
left a comment
beyackle
left a comment
There was a problem hiding this comment.
One tiny thing, but otherwise this looks great.
extensions/azurePublish/src/components/azureProvisionDialog.tsx
Outdated
Show resolved
Hide resolved
extensions/azurePublish/src/components/azureProvisionDialog.tsx
Outdated
Show resolved
Hide resolved
beyackle
previously approved these changes
Mar 31, 2021
GeoffCoxMSFT
reviewed
Apr 1, 2021
GeoffCoxMSFT
reviewed
Apr 1, 2021
GeoffCoxMSFT
reviewed
Apr 1, 2021
GeoffCoxMSFT
reviewed
Apr 1, 2021
| const getDefaultFormData = (currentProfile, defaults) => { | ||
| return { | ||
| choice: defaults.choice ?? 'create', | ||
| tenantId: currentProfile?.tenantId ?? defaults.tenantId, |
Member
There was a problem hiding this comment.
This is probably overthinking this, but I noticed that you clear the other fields when the tenantId changes. Here, this chooses between the profile tenant and the default. If we choose one or the other, would we want to use all the tenant-bound values from that choice? (i.e. I choose the profile tenantId and so I want to choose the subscriptionId from the profile and not the default?)
Contributor
Author
There was a problem hiding this comment.
I think this could actually just be tenantId: currentProfile?.tenantId because we won't ever set the tenant id with the defaults.
GeoffCoxMSFT
reviewed
Apr 1, 2021
GeoffCoxMSFT
reviewed
Apr 1, 2021
GeoffCoxMSFT
reviewed
Apr 1, 2021
extensions/azurePublish/src/components/azureProvisionDialog.tsx
Outdated
Show resolved
Hide resolved
GeoffCoxMSFT
reviewed
Apr 1, 2021
GeoffCoxMSFT
previously approved these changes
Apr 1, 2021
a-b-r-o-w-n
commented
Apr 2, 2021
| @@ -1,4 +1,5 @@ | |||
| process.env.DEBUG = 'composer*'; | |||
Contributor
Author
There was a problem hiding this comment.
@tonyanziano I removed this. Was it there for a good reason? Just trying to cut down on the noise in the test output!
GeoffCoxMSFT
approved these changes
Apr 2, 2021
cwhitten
approved these changes
Apr 2, 2021
Merged
lei9444
pushed a commit
to lei9444/BotFramework-Composer-1
that referenced
this pull request
Jun 15, 2021
…#6628) * make dropdown width same as dropdown element * do not require sign in every time fetching tenants * open dev tools after the app is ready * do not populate dropdowns if token is null * hide tenant selection when tokens are user provided * rename method for clarity * refactor how form data is used * make seeding form data more resiliant * fix issue where page was not set correctly * allow publishing from web * store tenant id in publish profiles for token based users * add test for using cached tenant token * disable resource group downdop for existing profiles * remove useless if statement * Fix function name * update name for clarity * remove default tenant id * add isGetTokensFromUser for compatibility with 3p extensions * revert lint-staged config Co-authored-by: Ben Yackley <61990921+beyackle@users.noreply.github.com> Co-authored-by: Chris Whitten <christopherwhitten@gmail.com> Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
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.
Description
Task Item
fixes #6595
fixes #6614