[PM-13620]Existing user email linking to create-organization #13030
Conversation
|
Great job, no security vulnerabilities found in this Pull Request |
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #13030 +/- ##
==========================================
- Coverage 35.10% 35.10% -0.01%
==========================================
Files 2990 2990
Lines 91078 91093 +15
Branches 17052 17059 +7
==========================================
Hits 31974 31974
- Misses 56611 56626 +15
Partials 2493 2493 ☔ View full report in Codecov by Sentry. |
| @@ -25,19 +25,54 @@ export class CreateOrganizationComponent implements OnInit { | |||
| ngOnInit() { | |||
| // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe | |||
| this.route.queryParams.pipe(first()).subscribe(async (qParams) => { | |||
| if (qParams.plan === "families") { | |||
| if (qParams.plan === "families" || qParams.productTier == ProductTierType.Families) { | |||
There was a problem hiding this comment.
Do we need to support both these queryparams? Can we just use one or the other?
| if (this.orgPlansComponent.selectedSecretsManagerPlan) { | ||
| this.orgPlansComponent.secretsManagerSubscription.patchValue({ | ||
| enabled: true, | ||
| userSeats: 1, | ||
| additionalServiceAccounts: 0, | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
I think the logic here is: "if the selected plan supports SM, enable SM by default". Is that right?
The logic here is too complicated, especially with the waitForPlansToLoad(). I suggest moving this into the OrganizationPlansComponent, e.g.
@Input()
set EnableSecretsManagerByDefault(value: boolean) {
if (value && this.selectedSecretsManagerPlan) {
this.orgPlansComponent.secretsManagerSubscription.patchValue({
// etc
}
}...or it might have to be in ngOnInit because it relies on the plan & product being set first. (that might solve your timing problems)
Then this component can just set the input property in the template, based on the query params. e.g.
this.secretsManager = qParams.Product == ProductType.SecretsManager;<app-organization-plans [providerId]="providerId" [enableSecretsManagerByDefault]="secretsManager"></app-organization-plans>| this.orgPlansComponent.plan = PlanType.EnterpriseAnnually; | ||
| this.orgPlansComponent.productTier = ProductTierType.Enterprise; |
There was a problem hiding this comment.
These are already @Input properties in OrganizationPlansComponent. It would be better to pass them in via the template rather than using @ViewChild.
| @@ -25,19 +25,54 @@ export class CreateOrganizationComponent implements OnInit { | |||
| ngOnInit() { | |||
| // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe | |||
There was a problem hiding this comment.
Please resolve these while you're here.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-13620
📔 Objective
We will update the endpoint behavior so that when the email address for an existing user is submitted, the user is sent an email.
The email content will be the same as the one received by users who do not exist, but the link will be specific to the existing user email and the flow following receipt of the email will differ.
📸 Screenshots
Uploading Screen Recording 2025-01-23 at 11.54.43.mov…
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes