From b95dfd9d3090a3df02feca5536b669c26380abdf Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Tue, 19 Mar 2024 12:19:32 +1000 Subject: [PATCH] [AC-2276] Move policyService helper methods to domain object (#8254) * Move mapPolicyFromResponse and mapPoliciesFromToken to static factory methods --- .../organizations/members/people.component.ts | 3 +- .../app/auth/accept-organization.component.ts | 2 +- .../web/src/app/auth/login/login.component.ts | 2 +- .../trial-initiation.component.ts | 4 +- .../policy/policy.service.abstraction.ts | 14 ----- .../src/admin-console/models/domain/policy.ts | 10 +++ .../services/policy/policy-api.service.ts | 5 +- .../services/policy/policy.service.spec.ts | 62 ------------------- .../services/policy/policy.service.ts | 15 ----- 9 files changed, 17 insertions(+), 100 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/members/people.component.ts b/apps/web/src/app/admin-console/organizations/members/people.component.ts index 4e45f70b6d..b3142125df 100644 --- a/apps/web/src/app/admin-console/organizations/members/people.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/people.component.ts @@ -35,6 +35,7 @@ import { PolicyType, } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { OrganizationKeysRequest } from "@bitwarden/common/admin-console/models/request/organization-keys.request"; import { ProductType } from "@bitwarden/common/enums"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; @@ -155,7 +156,7 @@ export class PeopleComponent switchMap((organization) => { if (organization.isProviderUser) { return from(this.policyApiService.getPolicies(organization.id)).pipe( - map((response) => this.policyService.mapPoliciesFromToken(response)), + map((response) => Policy.fromListResponse(response)), ); } diff --git a/apps/web/src/app/auth/accept-organization.component.ts b/apps/web/src/app/auth/accept-organization.component.ts index 21f3a41fa6..52e3b64494 100644 --- a/apps/web/src/app/auth/accept-organization.component.ts +++ b/apps/web/src/app/auth/accept-organization.component.ts @@ -167,7 +167,7 @@ export class AcceptOrganizationComponent extends BaseAcceptComponent { qParams.email, qParams.organizationUserId, ); - policyList = this.policyService.mapPoliciesFromToken(policies); + policyList = Policy.fromListResponse(policies); } catch (e) { this.logService.error(e); } diff --git a/apps/web/src/app/auth/login/login.component.ts b/apps/web/src/app/auth/login/login.component.ts index 9075c3ab28..1d2d1859e9 100644 --- a/apps/web/src/app/auth/login/login.component.ts +++ b/apps/web/src/app/auth/login/login.component.ts @@ -120,7 +120,7 @@ export class LoginComponent extends BaseLoginComponent implements OnInit { invite.email, invite.organizationUserId, ); - policyList = this.policyService.mapPoliciesFromToken(this.policies); + policyList = Policy.fromListResponse(this.policies); } catch (e) { this.logService.error(e); } diff --git a/apps/web/src/app/auth/trial-initiation/trial-initiation.component.ts b/apps/web/src/app/auth/trial-initiation/trial-initiation.component.ts index df4962185d..52a4120b1a 100644 --- a/apps/web/src/app/auth/trial-initiation/trial-initiation.component.ts +++ b/apps/web/src/app/auth/trial-initiation/trial-initiation.component.ts @@ -7,7 +7,6 @@ import { Subject, takeUntil } from "rxjs"; import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; -import { PolicyData } from "@bitwarden/common/admin-console/models/data/policy.data"; import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options"; import { Policy } from "@bitwarden/common/admin-console/models/domain/policy"; import { PlanType } from "@bitwarden/common/billing/enums"; @@ -191,8 +190,7 @@ export class TrialInitiationComponent implements OnInit, OnDestroy { invite.organizationUserId, ); if (policies.data != null) { - const policiesData = policies.data.map((p) => new PolicyData(p)); - this.policies = policiesData.map((p) => new Policy(p)); + this.policies = Policy.fromListResponse(policies); } } catch (e) { this.logService.error(e); diff --git a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts index 1a85fd79fc..fb805f94cd 100644 --- a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts @@ -1,13 +1,11 @@ import { Observable } from "rxjs"; -import { ListResponse } from "../../../models/response/list.response"; import { UserId } from "../../../types/guid"; import { PolicyType } from "../../enums"; import { PolicyData } from "../../models/data/policy.data"; import { MasterPasswordPolicyOptions } from "../../models/domain/master-password-policy-options"; import { Policy } from "../../models/domain/policy"; import { ResetPasswordPolicyOptions } from "../../models/domain/reset-password-policy-options"; -import { PolicyResponse } from "../../models/response/policy.response"; export abstract class PolicyService { /** @@ -75,18 +73,6 @@ export abstract class PolicyService { policies: Policy[], orgId: string, ) => [ResetPasswordPolicyOptions, boolean]; - - // Helpers - - /** - * Instantiates {@link Policy} objects from {@link PolicyResponse} objects. - */ - mapPolicyFromResponse: (policyResponse: PolicyResponse) => Policy; - - /** - * Instantiates {@link Policy} objects from {@link ListResponse} objects. - */ - mapPoliciesFromToken: (policiesResponse: ListResponse) => Policy[]; } export abstract class InternalPolicyService extends PolicyService { diff --git a/libs/common/src/admin-console/models/domain/policy.ts b/libs/common/src/admin-console/models/domain/policy.ts index 65af09b588..c2f9c9c8df 100644 --- a/libs/common/src/admin-console/models/domain/policy.ts +++ b/libs/common/src/admin-console/models/domain/policy.ts @@ -1,7 +1,9 @@ +import { ListResponse } from "../../../models/response/list.response"; import Domain from "../../../platform/models/domain/domain-base"; import { PolicyId } from "../../../types/guid"; import { PolicyType } from "../../enums"; import { PolicyData } from "../data/policy.data"; +import { PolicyResponse } from "../response/policy.response"; export class Policy extends Domain { id: PolicyId; @@ -27,4 +29,12 @@ export class Policy extends Domain { this.data = obj.data; this.enabled = obj.enabled; } + + static fromResponse(response: PolicyResponse): Policy { + return new Policy(new PolicyData(response)); + } + + static fromListResponse(response: ListResponse): Policy[] | undefined { + return response.data?.map((d) => Policy.fromResponse(d)) ?? undefined; + } } diff --git a/libs/common/src/admin-console/services/policy/policy-api.service.ts b/libs/common/src/admin-console/services/policy/policy-api.service.ts index 0eb37f2305..c7f093286e 100644 --- a/libs/common/src/admin-console/services/policy/policy-api.service.ts +++ b/libs/common/src/admin-console/services/policy/policy-api.service.ts @@ -10,6 +10,7 @@ import { InternalPolicyService } from "../../abstractions/policy/policy.service. import { PolicyType } from "../../enums"; import { PolicyData } from "../../models/data/policy.data"; import { MasterPasswordPolicyOptions } from "../../models/domain/master-password-policy-options"; +import { Policy } from "../../models/domain/policy"; import { PolicyRequest } from "../../models/request/policy.request"; import { PolicyResponse } from "../../models/response/policy.response"; @@ -86,9 +87,7 @@ export class PolicyApiService implements PolicyApiServiceAbstraction { const masterPasswordPolicyResponse = await this.getMasterPasswordPolicyResponseForOrgUser(orgId); - const masterPasswordPolicy = this.policyService.mapPolicyFromResponse( - masterPasswordPolicyResponse, - ); + const masterPasswordPolicy = Policy.fromResponse(masterPasswordPolicyResponse); if (!masterPasswordPolicy) { return null; diff --git a/libs/common/src/admin-console/services/policy/policy.service.spec.ts b/libs/common/src/admin-console/services/policy/policy.service.spec.ts index 672a53d34e..8fa79f4d1c 100644 --- a/libs/common/src/admin-console/services/policy/policy.service.spec.ts +++ b/libs/common/src/admin-console/services/policy/policy.service.spec.ts @@ -16,9 +16,7 @@ import { MasterPasswordPolicyOptions } from "../../../admin-console/models/domai import { Organization } from "../../../admin-console/models/domain/organization"; import { Policy } from "../../../admin-console/models/domain/policy"; import { ResetPasswordPolicyOptions } from "../../../admin-console/models/domain/reset-password-policy-options"; -import { PolicyResponse } from "../../../admin-console/models/response/policy.response"; import { POLICIES, PolicyService } from "../../../admin-console/services/policy/policy.service"; -import { ListResponse } from "../../../models/response/list.response"; import { PolicyId, UserId } from "../../../types/guid"; describe("PolicyService", () => { @@ -265,66 +263,6 @@ describe("PolicyService", () => { }); }); - describe("mapPoliciesFromToken", () => { - it("null", async () => { - const result = policyService.mapPoliciesFromToken(null); - - expect(result).toEqual(null); - }); - - it("null data", async () => { - const model = new ListResponse(null, PolicyResponse); - model.data = null; - const result = policyService.mapPoliciesFromToken(model); - - expect(result).toEqual(null); - }); - - it("empty array", async () => { - const model = new ListResponse(null, PolicyResponse); - const result = policyService.mapPoliciesFromToken(model); - - expect(result).toEqual([]); - }); - - it("success", async () => { - const policyResponse: any = { - Data: [ - { - Id: "1", - OrganizationId: "organization-1", - Type: PolicyType.DisablePersonalVaultExport, - Enabled: true, - Data: { requireUpper: true }, - }, - { - Id: "2", - OrganizationId: "organization-2", - Type: PolicyType.DisableSend, - Enabled: false, - Data: { minComplexity: 5, minLength: 20 }, - }, - ], - }; - const model = new ListResponse(policyResponse, PolicyResponse); - const result = policyService.mapPoliciesFromToken(model); - - expect(result).toEqual([ - new Policy( - policyData("1", "organization-1", PolicyType.DisablePersonalVaultExport, true, { - requireUpper: true, - }), - ), - new Policy( - policyData("2", "organization-2", PolicyType.DisableSend, false, { - minComplexity: 5, - minLength: 20, - }), - ), - ]); - }); - }); - describe("get$", () => { it("returns the specified PolicyType", async () => { activeUserState.nextState( diff --git a/libs/common/src/admin-console/services/policy/policy.service.ts b/libs/common/src/admin-console/services/policy/policy.service.ts index 4672d9b810..d60d2e3293 100644 --- a/libs/common/src/admin-console/services/policy/policy.service.ts +++ b/libs/common/src/admin-console/services/policy/policy.service.ts @@ -1,6 +1,5 @@ import { combineLatest, firstValueFrom, map, Observable, of } from "rxjs"; -import { ListResponse } from "../../../models/response/list.response"; import { KeyDefinition, POLICIES_DISK, StateProvider } from "../../../platform/state"; import { PolicyId, UserId } from "../../../types/guid"; import { OrganizationService } from "../../abstractions/organization/organization.service.abstraction"; @@ -11,7 +10,6 @@ import { MasterPasswordPolicyOptions } from "../../models/domain/master-password import { Organization } from "../../models/domain/organization"; import { Policy } from "../../models/domain/policy"; import { ResetPasswordPolicyOptions } from "../../models/domain/reset-password-policy-options"; -import { PolicyResponse } from "../../models/response/policy.response"; const policyRecordToArray = (policiesMap: { [id: string]: PolicyData }) => Object.values(policiesMap || {}).map((f) => new Policy(f)); @@ -212,19 +210,6 @@ export class PolicyService implements InternalPolicyServiceAbstraction { return [resetPasswordPolicyOptions, policy?.enabled ?? false]; } - mapPolicyFromResponse(policyResponse: PolicyResponse): Policy { - const policyData = new PolicyData(policyResponse); - return new Policy(policyData); - } - - mapPoliciesFromToken(policiesResponse: ListResponse): Policy[] { - if (policiesResponse?.data == null) { - return null; - } - - return policiesResponse.data.map((response) => this.mapPolicyFromResponse(response)); - } - async upsert(policy: PolicyData): Promise { await this.activeUserPolicyState.update((policies) => { policies ??= {};