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

Address code review feebacks.

This commit is contained in:
Jimmy Vo
2024-12-11 14:24:48 -05:00
parent 932129b501
commit 49553ec14a
5 changed files with 245 additions and 230 deletions

View File

@@ -50,10 +50,8 @@ import {
} from "../../../shared/components/access-selector";
import { commaSeparatedEmails } from "./validators/comma-separated-emails.validator";
import {
orgSeatLimitReachedValidator,
inputEmailLimitValidator,
} from "./validators/org-seat-limit-reached.validator";
import { inputEmailLimitValidator } from "./validators/input-email-limit.validator";
import { orgSeatLimitReachedValidator } from "./validators/org-seat-limit-reached.validator";
export enum MemberDialogTab {
Role = 0,
@@ -273,7 +271,7 @@ export class MemberDialogComponent implements OnDestroy {
}
private setFormValidators(organization: Organization) {
const emailsControlValidators = [
const _orgSeatLimitReachedValidator = [
Validators.required,
commaSeparatedEmails,
orgSeatLimitReachedValidator(
@@ -283,8 +281,17 @@ export class MemberDialogComponent implements OnDestroy {
),
];
const _inputEmailLimitValidator = [
Validators.required,
commaSeparatedEmails,
inputEmailLimitValidator(organization, (maxEmailsCount: number) =>
this.i18nService.t("tooManyEmails", maxEmailsCount),
),
];
const emailsControl = this.formGroup.get("emails");
emailsControl.setValidators(emailsControlValidators);
emailsControl.setValidators(_orgSeatLimitReachedValidator);
emailsControl.setValidators(_inputEmailLimitValidator);
emailsControl.updateValueAndValidity();
}
@@ -478,8 +485,6 @@ export class MemberDialogComponent implements OnDestroy {
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({
@@ -490,20 +495,6 @@ export class MemberDialogComponent implements OnDestroy {
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

@@ -0,0 +1,191 @@
import { AbstractControl, FormControl } from "@angular/forms";
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 { inputEmailLimitValidator } from "./input-email-limit.validator";
const orgFactory = (props: Partial<Organization> = {}) =>
Object.assign(
new Organization(),
{
id: "myOrgId",
enabled: true,
type: OrganizationUserType.Admin,
},
props,
);
describe("inputEmailLimitValidator", () => {
const getErrorMessage = (max: number) => `You can only add up to ${max} unique emails.`;
const createUniqueEmailString = (numberOfEmails: number) =>
Array(numberOfEmails)
.fill(null)
.map((_, i) => `email${i}@example.com`)
.join(", ");
const createIdenticalEmailString = (numberOfEmails: number) =>
Array(numberOfEmails)
.fill(null)
.map(() => `email@example.com`)
.join(", ");
describe("TeamsStarter limit validation", () => {
let teamsStarterOrganization: Organization;
beforeEach(() => {
teamsStarterOrganization = orgFactory({
productTierType: ProductTierType.TeamsStarter,
seats: 10,
});
});
it("should return null if unique email count is within the limit", () => {
// Arrange
const control = new FormControl(createUniqueEmailString(3));
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(createUniqueEmailString(10));
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("Non-TeamsStarter limit validation", () => {
let nonTeamsStarterOrganization: Organization;
beforeEach(() => {
nonTeamsStarterOrganization = orgFactory({
productTierType: ProductTierType.Enterprise,
seats: 100,
});
});
it("should return null if unique email count is within the limit", () => {
// Arrange
const control = new FormControl(createUniqueEmailString(3));
const validatorFn = inputEmailLimitValidator(nonTeamsStarterOrganization, 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(createUniqueEmailString(10));
const validatorFn = inputEmailLimitValidator(nonTeamsStarterOrganization, 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(nonTeamsStarterOrganization, 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", () => {
let organization: Organization;
beforeEach(() => {
organization = orgFactory({
productTierType: ProductTierType.Enterprise,
seats: 100,
});
});
it("should ignore duplicate emails and validate only unique ones", () => {
// Arrange
const sixUniqueEmails = createUniqueEmailString(6);
const sixDuplicateEmails = createIdenticalEmailString(6);
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

@@ -0,0 +1,40 @@
import { AbstractControl, ValidationErrors, ValidatorFn } from "@angular/forms";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { ProductTierType } from "@bitwarden/common/billing/enums";
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?.trim()) {
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

@@ -4,10 +4,7 @@ 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 {
inputEmailLimitValidator,
orgSeatLimitReachedValidator,
} from "./org-seat-limit-reached.validator";
import { orgSeatLimitReachedValidator } from "./org-seat-limit-reached.validator";
const orgFactory = (props: Partial<Organization> = {}) =>
Object.assign(
@@ -134,171 +131,3 @@ 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,39 +47,3 @@ 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) } };
};
}