1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-12 06:23:38 +00:00

Address code review feedback.

This commit is contained in:
Jimmy Vo
2024-12-10 11:18:34 -05:00
parent 5b3c1fcd01
commit 4df84ff3d2
4 changed files with 290 additions and 77 deletions

View File

@@ -50,7 +50,10 @@ import {
} from "../../../shared/components/access-selector";
import { commaSeparatedEmails } from "./validators/comma-separated-emails.validator";
import { orgSeatLimitReachedValidator } from "./validators/org-seat-limit-reached.validator";
import {
orgSeatLimitReachedValidator,
inputEmailLimitValidator,
} from "./validators/org-seat-limit-reached.validator";
export enum MemberDialogTab {
Role = 0,
@@ -285,72 +288,6 @@ export class MemberDialogComponent implements OnDestroy {
emailsControl.updateValueAndValidity();
}
private async handleInviteUsers(userView: OrganizationUserAdminView, organization: Organization) {
const emails = [...new Set(this.formGroup.value.emails.trim().split(/\s*,\s*/))];
if (this.enforceEmailCountLimit(emails, organization)) {
return;
}
await this.userService.invite(emails, userView);
this.toastService.showToast({
variant: "success",
title: null,
message: this.i18nService.t("invitedUsers"),
});
this.close(MemberDialogResult.Saved);
}
private enforceEmailCountLimit(emails: string[], organization: Organization): boolean {
const maxEmailsCount = organization.productTierType === ProductTierType.TeamsStarter ? 10 : 20;
if (emails.length > maxEmailsCount) {
this.formGroup.controls.emails.setErrors({
tooManyEmails: { message: this.i18nService.t("tooManyEmails", maxEmailsCount) },
});
return true;
}
return false;
}
private async handleEditUser(userView: OrganizationUserAdminView) {
userView.id = this.params.organizationUserId;
await this.userService.save(userView);
this.toastService.showToast({
variant: "success",
title: null,
message: this.i18nService.t("editedUserId", this.params.name),
});
this.close(MemberDialogResult.Saved);
}
private async getUserView(): Promise<OrganizationUserAdminView> {
const userView = new OrganizationUserAdminView();
userView.organizationId = this.params.organizationId;
userView.type = this.formGroup.value.type;
userView.permissions = this.setRequestPermissions(
userView.permissions ?? new PermissionsApi(),
userView.type !== OrganizationUserType.Custom,
);
userView.collections = this.formGroup.value.access
.filter((v) => v.type === AccessItemType.Collection)
.map(convertToSelectionView);
userView.groups = (await firstValueFrom(this.restrictEditingSelf$))
? null
: this.formGroup.value.groups.map((m) => m.id);
userView.accessSecretsManager = this.formGroup.value.accessSecretsManager;
return userView;
}
private loadOrganizationUser(
userDetails: OrganizationUserAdminView,
groups: GroupDetailsView[],
@@ -502,6 +439,71 @@ export class MemberDialogComponent implements OnDestroy {
}
};
private async getUserView(): Promise<OrganizationUserAdminView> {
const userView = new OrganizationUserAdminView();
userView.organizationId = this.params.organizationId;
userView.type = this.formGroup.value.type;
userView.permissions = this.setRequestPermissions(
userView.permissions ?? new PermissionsApi(),
userView.type !== OrganizationUserType.Custom,
);
userView.collections = this.formGroup.value.access
.filter((v) => v.type === AccessItemType.Collection)
.map(convertToSelectionView);
userView.groups = (await firstValueFrom(this.restrictEditingSelf$))
? null
: this.formGroup.value.groups.map((m) => m.id);
userView.accessSecretsManager = this.formGroup.value.accessSecretsManager;
return userView;
}
private async handleEditUser(userView: OrganizationUserAdminView) {
userView.id = this.params.organizationUserId;
await this.userService.save(userView);
this.toastService.showToast({
variant: "success",
title: null,
message: this.i18nService.t("editedUserId", this.params.name),
});
this.close(MemberDialogResult.Saved);
}
private async handleInviteUsers(userView: OrganizationUserAdminView, organization: Organization) {
const emails = [...new Set(this.formGroup.value.emails.trim().split(/\s*,\s*/))];
this.setInputEmailCountValidator(organization, emails.length);
await this.userService.invite(emails, userView);
this.toastService.showToast({
variant: "success",
title: null,
message: this.i18nService.t("invitedUsers"),
});
this.close(MemberDialogResult.Saved);
}
private setInputEmailCountValidator(organization: Organization, emailCount: number) {
const emailsControlValidators = [
Validators.required,
commaSeparatedEmails,
inputEmailLimitValidator(organization, (maxEmailsCount: number) =>
this.i18nService.t("tooManyEmails", maxEmailsCount),
),
];
const emailsControl = this.formGroup.get("emails");
emailsControl.setValidators(emailsControlValidators);
emailsControl.updateValueAndValidity();
}
remove = async () => {
if (!this.editMode) {
return;

View File

@@ -4,7 +4,10 @@ import { OrganizationUserType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { ProductTierType } from "@bitwarden/common/billing/enums";
import { orgSeatLimitReachedValidator } from "./org-seat-limit-reached.validator";
import {
inputEmailLimitValidator,
orgSeatLimitReachedValidator,
} from "./org-seat-limit-reached.validator";
const orgFactory = (props: Partial<Organization> = {}) =>
Object.assign(
@@ -131,3 +134,171 @@ describe("orgSeatLimitReachedValidator", () => {
expect(result).toBeNull();
});
});
describe("inputEmailLimitValidator", () => {
const getErrorMessage = (max: number) => `You can only add up to ${max} unique emails.`;
const createUniqueEmailString = (numberOfEmails: number) => {
let result = "";
for (let i = 1; i <= numberOfEmails; i++) {
result += `test${i}@test.com,`;
}
// Remove the last comma and space
result = result.slice(0, -1);
return result;
};
describe("TeamsStarter limit validation", () => {
const teamsStarterOrganization = orgFactory({
productTierType: ProductTierType.TeamsStarter,
seats: 10,
});
it("should return null if unique email count is within the limit", () => {
// Arrange
const control = new FormControl("test1@test.com, test2@test.com, test3@test.com");
const validatorFn = inputEmailLimitValidator(teamsStarterOrganization, getErrorMessage);
// Act
const result = validatorFn(control);
// Assert
expect(result).toBeNull();
});
it("should return null if unique email count is equal the limit", () => {
// Arrange
const control = new FormControl(
"test1@test.com, test2@test.com, test3@test.com, test4@test.com, test5@test.com, test6@test.com, test7@test.com, test8@test.com, test9@test.com, test10@test.com",
);
const validatorFn = inputEmailLimitValidator(teamsStarterOrganization, getErrorMessage);
// Act
const result = validatorFn(control);
// Assert
expect(result).toBeNull();
});
it("should return an error if unique email count exceeds the limit", () => {
// Arrange
const control = new FormControl(createUniqueEmailString(11));
const validatorFn = inputEmailLimitValidator(teamsStarterOrganization, getErrorMessage);
// Act
const result = validatorFn(control);
// Assert
expect(result).toEqual({
tooManyEmails: { message: "You can only add up to 10 unique emails." },
});
});
});
describe("none TeamsStarter limit validation", () => {
const noneTeamsStarterOrganization = orgFactory({
productTierType: ProductTierType.Enterprise,
seats: 100,
});
it("should return null if unique email count is within the limit", () => {
// Arrange
const control = new FormControl("test1@test.com, test2@test.com, test3@test.com");
const validatorFn = inputEmailLimitValidator(noneTeamsStarterOrganization, getErrorMessage);
// Act
const result = validatorFn(control);
// Assert
expect(result).toBeNull();
});
it("should return null if unique email count is equal the limit", () => {
// Arrange
const control = new FormControl(
"test1@test.com, test2@test.com, test3@test.com, test4@test.com, test5@test.com, test6@test.com, test7@test.com, test8@test.com, test9@test.com, test10@test.com",
);
const validatorFn = inputEmailLimitValidator(noneTeamsStarterOrganization, getErrorMessage);
// Act
const result = validatorFn(control);
// Assert
expect(result).toBeNull();
});
it("should return an error if unique email count exceeds the limit", () => {
// Arrange
const control = new FormControl(createUniqueEmailString(21));
const validatorFn = inputEmailLimitValidator(noneTeamsStarterOrganization, getErrorMessage);
// Act
const result = validatorFn(control);
// Assert
expect(result).toEqual({
tooManyEmails: { message: "You can only add up to 20 unique emails." },
});
});
});
describe("input email validation", () => {
const organization = orgFactory({
productTierType: ProductTierType.Enterprise,
seats: 100,
});
it("should ignore duplicate emails and validate only unique ones", () => {
// Arrange
const sixUniqueEmails = createUniqueEmailString(6);
const sixDuplicateEmails =
"test1@test.com,test1@test.com,test1@test.com,test1@test.com,test1@test.com,test1@test.com";
const control = new FormControl(sixUniqueEmails + sixDuplicateEmails);
const validatorFn = inputEmailLimitValidator(organization, getErrorMessage);
// Act
const result = validatorFn(control);
// Assert
expect(result).toBeNull();
});
it("should return null if input is null", () => {
// Arrange
const control: AbstractControl = new FormControl(null);
const validatorFn = inputEmailLimitValidator(organization, getErrorMessage);
// Act
const result = validatorFn(control);
// Assert
expect(result).toBeNull();
});
it("should return null if input is empty", () => {
// Arrange
const control: AbstractControl = new FormControl("");
const validatorFn = inputEmailLimitValidator(organization, getErrorMessage);
// Act
const result = validatorFn(control);
// Assert
expect(result).toBeNull();
});
});
});

View File

@@ -47,3 +47,39 @@ export function orgSeatLimitReachedValidator(
: null;
};
}
function getUniqueInputEmails(control: AbstractControl): string[] {
const emails: string[] = control.value
.split(",")
.filter((email: string) => email && email.trim() !== "");
const uniqueEmails: string[] = Array.from(new Set(emails));
return uniqueEmails;
}
/**
* Ensure the number of unique emails in an input does not exceed the allowed maximum.
* @param organization An object representing the organization
* @param getErrorMessage A callback function that generates the error message. It takes the `maxEmailsCount` as a parameter.
* @returns A function that validates an `AbstractControl` and returns `ValidationErrors` or `null`
*/
export function inputEmailLimitValidator(
organization: Organization,
getErrorMessage: (maxEmailsCount: number) => string,
): ValidatorFn {
return (control: AbstractControl): ValidationErrors | null => {
if (control.value === "" || !control.value) {
return null;
}
const maxEmailsCount = organization.productTierType === ProductTierType.TeamsStarter ? 10 : 20;
const uniqueEmails = getUniqueInputEmails(control);
if (uniqueEmails.length <= maxEmailsCount) {
return null;
}
return { tooManyEmails: { message: getErrorMessage(maxEmailsCount) } };
};
}

View File

@@ -462,7 +462,7 @@ export class MembersComponent extends BaseMembersComponent<OrganizationUserView>
firstValueFrom(simpleDialog.closed).then(this.handleDialogClose.bind(this));
}
private async handleInviteDialog(initialTab: MemberDialogTab) {
private async handleInviteDialog() {
const dialog = openUserAddEditDialog(this.dialogService, {
data: {
name: null,
@@ -471,7 +471,7 @@ export class MembersComponent extends BaseMembersComponent<OrganizationUserView>
allOrganizationUserEmails: this.dataSource.data?.map((user) => user.email) ?? [],
usesKeyConnector: null,
isOnSecretsManagerStandalone: this.orgIsOnSecretsManagerStandalone,
initialTab: initialTab,
initialTab: MemberDialogTab.Role,
numConfirmedMembers: this.dataSource.confirmedUserCount,
managedByOrganization: null,
},
@@ -505,7 +505,7 @@ export class MembersComponent extends BaseMembersComponent<OrganizationUserView>
}
}
async invite(initialTab: MemberDialogTab = MemberDialogTab.Role) {
async invite() {
if (
this.organization.hasReseller &&
this.organization.seats === this.dataSource.confirmedUserCount
@@ -515,16 +515,22 @@ export class MembersComponent extends BaseMembersComponent<OrganizationUserView>
title: this.i18nService.t("seatLimitReached"),
message: this.i18nService.t("contactYourProvider"),
});
} else if (
return;
}
if (
this.dataSource.data.length === this.organization.seats &&
(this.organization.productTierType === ProductTierType.Free ||
this.organization.productTierType === ProductTierType.TeamsStarter ||
this.organization.productTierType === ProductTierType.Families)
) {
await this.handleSeatLimitForFixedTiers();
} else {
await this.handleInviteDialog(initialTab);
return;
}
await this.handleInviteDialog();
}
async edit(user: OrganizationUserView, initialTab: MemberDialogTab = MemberDialogTab.Role) {
@@ -550,9 +556,7 @@ export class MembersComponent extends BaseMembersComponent<OrganizationUserView>
case MemberDialogResult.Saved:
case MemberDialogResult.Revoked:
case MemberDialogResult.Restored:
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.load();
await this.load();
break;
}
}