1
0
mirror of https://github.com/bitwarden/browser.git synced 2025-02-12 00:41:29 +01:00

[PM-17465] refactor PolicyService.getAll$ to make userId not optional (#13031)

* refactor PolicyService.getAll$ to make userId not optional

* add fix to browser

* fix test to read from mock singleUserState

* remove nested pipes, cleanup
This commit is contained in:
Brandon Treston 2025-01-24 09:58:38 -05:00 committed by GitHub
parent 3f05a5e5f5
commit b23a41ac86
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 50 additions and 31 deletions

View File

@ -47,7 +47,7 @@ export class FamiliesPolicyService {
map((organizations) => organizations.find((org) => org.canManageSponsorships)?.id), map((organizations) => organizations.find((org) => org.canManageSponsorships)?.id),
switchMap((enterpriseOrgId) => switchMap((enterpriseOrgId) =>
this.policyService this.policyService
.getAll$(PolicyType.FreeFamiliesSponsorshipPolicy) .getAll$(PolicyType.FreeFamiliesSponsorshipPolicy, userId)
.pipe( .pipe(
map( map(
(policies) => (policies) =>

View File

@ -6,6 +6,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
@ -110,7 +111,11 @@ export class FreeFamiliesPolicyService {
}); });
} }
return this.policyService.getAll$(PolicyType.FreeFamiliesSponsorshipPolicy).pipe( return this.accountService.activeAccount$.pipe(
getUserId,
switchMap((userId) =>
this.policyService.getAll$(PolicyType.FreeFamiliesSponsorshipPolicy, userId),
),
map((policies) => ({ map((policies) => ({
isFreeFamilyPolicyEnabled: policies.some( isFreeFamilyPolicyEnabled: policies.some(
(policy) => policy.organizationId === organizationId && policy.enabled, (policy) => policy.organizationId === organizationId && policy.enabled,

View File

@ -98,7 +98,7 @@ export class SponsoredFamiliesComponent implements OnInit, OnDestroy {
this.availableSponsorshipOrgs$ = combineLatest([ this.availableSponsorshipOrgs$ = combineLatest([
this.organizationService.organizations$(userId), this.organizationService.organizations$(userId),
this.policyService.getAll$(PolicyType.FreeFamiliesSponsorshipPolicy), this.policyService.getAll$(PolicyType.FreeFamiliesSponsorshipPolicy, userId),
]).pipe( ]).pipe(
map(([organizations, policies]) => map(([organizations, policies]) =>
organizations organizations

View File

@ -2,12 +2,14 @@
// @ts-strict-ignore // @ts-strict-ignore
import { formatDate } from "@angular/common"; import { formatDate } from "@angular/common";
import { Component, EventEmitter, Input, Output, OnInit } from "@angular/core"; import { Component, EventEmitter, Input, Output, OnInit } from "@angular/core";
import { firstValueFrom, map, Observable } from "rxjs"; import { firstValueFrom, map, Observable, switchMap } from "rxjs";
import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@ -38,6 +40,7 @@ export class SponsoringOrgRowComponent implements OnInit {
private toastService: ToastService, private toastService: ToastService,
private configService: ConfigService, private configService: ConfigService,
private policyService: PolicyService, private policyService: PolicyService,
private accountService: AccountService,
) {} ) {}
async ngOnInit() { async ngOnInit() {
@ -54,17 +57,19 @@ export class SponsoringOrgRowComponent implements OnInit {
); );
if (this.isFreeFamilyFlagEnabled) { if (this.isFreeFamilyFlagEnabled) {
this.isFreeFamilyPolicyEnabled$ = this.policyService this.isFreeFamilyPolicyEnabled$ = this.accountService.activeAccount$.pipe(
.getAll$(PolicyType.FreeFamiliesSponsorshipPolicy) getUserId,
.pipe( switchMap((userId) =>
map( this.policyService.getAll$(PolicyType.FreeFamiliesSponsorshipPolicy, userId),
(policies) => ),
Array.isArray(policies) && map(
policies.some( (policies) =>
(policy) => policy.organizationId === this.sponsoringOrg.id && policy.enabled, Array.isArray(policies) &&
), policies.some(
), (policy) => policy.organizationId === this.sponsoringOrg.id && policy.enabled,
); ),
),
);
} }
} }

View File

@ -16,6 +16,7 @@ import {
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@ -164,9 +165,10 @@ export class AddEditComponent implements OnInit, OnDestroy {
} }
}); });
this.policyService this.accountService.activeAccount$
.getAll$(PolicyType.SendOptions)
.pipe( .pipe(
getUserId,
switchMap((userId) => this.policyService.getAll$(PolicyType.SendOptions, userId)),
map((policies) => policies?.some((p) => p.data.disableHideEmail)), map((policies) => policies?.some((p) => p.data.disableHideEmail)),
takeUntil(this.destroy$), takeUntil(this.destroy$),
) )

View File

@ -30,7 +30,7 @@ export abstract class PolicyService {
* A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). * A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner).
* @param policyType the {@link PolicyType} to search for * @param policyType the {@link PolicyType} to search for
*/ */
getAll$: (policyType: PolicyType, userId?: UserId) => Observable<Policy[]>; getAll$: (policyType: PolicyType, userId: UserId) => Observable<Policy[]>;
/** /**
* All {@link Policy} objects for the specified user (from sync data). * All {@link Policy} objects for the specified user (from sync data).

View File

@ -2,7 +2,7 @@ import { mock, MockProxy } from "jest-mock-extended";
import { firstValueFrom, of } from "rxjs"; import { firstValueFrom, of } from "rxjs";
import { FakeStateProvider, mockAccountServiceWith } from "../../../../spec"; import { FakeStateProvider, mockAccountServiceWith } from "../../../../spec";
import { FakeActiveUserState } from "../../../../spec/fake-state"; import { FakeActiveUserState, FakeSingleUserState } from "../../../../spec/fake-state";
import { import {
OrganizationUserStatusType, OrganizationUserStatusType,
OrganizationUserType, OrganizationUserType,
@ -24,6 +24,7 @@ describe("PolicyService", () => {
let stateProvider: FakeStateProvider; let stateProvider: FakeStateProvider;
let organizationService: MockProxy<OrganizationService>; let organizationService: MockProxy<OrganizationService>;
let activeUserState: FakeActiveUserState<Record<PolicyId, PolicyData>>; let activeUserState: FakeActiveUserState<Record<PolicyId, PolicyData>>;
let singleUserState: FakeSingleUserState<Record<PolicyId, PolicyData>>;
let policyService: PolicyService; let policyService: PolicyService;
@ -33,6 +34,7 @@ describe("PolicyService", () => {
organizationService = mock<OrganizationService>(); organizationService = mock<OrganizationService>();
activeUserState = stateProvider.activeUser.getFake(POLICIES); activeUserState = stateProvider.activeUser.getFake(POLICIES);
singleUserState = stateProvider.singleUser.getFake(activeUserState.userId, POLICIES);
const organizations$ = of([ const organizations$ = of([
// User // User
@ -295,7 +297,7 @@ describe("PolicyService", () => {
describe("getAll$", () => { describe("getAll$", () => {
it("returns the specified PolicyTypes", async () => { it("returns the specified PolicyTypes", async () => {
activeUserState.nextState( singleUserState.nextState(
arrayToRecord([ arrayToRecord([
policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true),
policyData("policy2", "org1", PolicyType.ActivateAutofill, true), policyData("policy2", "org1", PolicyType.ActivateAutofill, true),
@ -305,7 +307,7 @@ describe("PolicyService", () => {
); );
const result = await firstValueFrom( const result = await firstValueFrom(
policyService.getAll$(PolicyType.DisablePersonalVaultExport), policyService.getAll$(PolicyType.DisablePersonalVaultExport, activeUserState.userId),
); );
expect(result).toEqual([ expect(result).toEqual([
@ -331,7 +333,7 @@ describe("PolicyService", () => {
}); });
it("does not return disabled policies", async () => { it("does not return disabled policies", async () => {
activeUserState.nextState( singleUserState.nextState(
arrayToRecord([ arrayToRecord([
policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true),
policyData("policy2", "org1", PolicyType.ActivateAutofill, true), policyData("policy2", "org1", PolicyType.ActivateAutofill, true),
@ -341,7 +343,7 @@ describe("PolicyService", () => {
); );
const result = await firstValueFrom( const result = await firstValueFrom(
policyService.getAll$(PolicyType.DisablePersonalVaultExport), policyService.getAll$(PolicyType.DisablePersonalVaultExport, activeUserState.userId),
); );
expect(result).toEqual([ expect(result).toEqual([
@ -361,7 +363,7 @@ describe("PolicyService", () => {
}); });
it("does not return policies that do not apply to the user because the user's role is exempt", async () => { it("does not return policies that do not apply to the user because the user's role is exempt", async () => {
activeUserState.nextState( singleUserState.nextState(
arrayToRecord([ arrayToRecord([
policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true),
policyData("policy2", "org1", PolicyType.ActivateAutofill, true), policyData("policy2", "org1", PolicyType.ActivateAutofill, true),
@ -371,7 +373,7 @@ describe("PolicyService", () => {
); );
const result = await firstValueFrom( const result = await firstValueFrom(
policyService.getAll$(PolicyType.DisablePersonalVaultExport), policyService.getAll$(PolicyType.DisablePersonalVaultExport, activeUserState.userId),
); );
expect(result).toEqual([ expect(result).toEqual([
@ -391,7 +393,7 @@ describe("PolicyService", () => {
}); });
it("does not return policies for organizations that do not use policies", async () => { it("does not return policies for organizations that do not use policies", async () => {
activeUserState.nextState( singleUserState.nextState(
arrayToRecord([ arrayToRecord([
policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true), policyData("policy1", "org4", PolicyType.DisablePersonalVaultExport, true),
policyData("policy2", "org1", PolicyType.ActivateAutofill, true), policyData("policy2", "org1", PolicyType.ActivateAutofill, true),
@ -401,7 +403,7 @@ describe("PolicyService", () => {
); );
const result = await firstValueFrom( const result = await firstValueFrom(
policyService.getAll$(PolicyType.DisablePersonalVaultExport), policyService.getAll$(PolicyType.DisablePersonalVaultExport, activeUserState.userId),
); );
expect(result).toEqual([ expect(result).toEqual([

View File

@ -51,7 +51,7 @@ export class PolicyService implements InternalPolicyServiceAbstraction {
); );
} }
getAll$(policyType: PolicyType, userId?: UserId) { getAll$(policyType: PolicyType, userId: UserId) {
const filteredPolicies$ = this.stateProvider.getUserState$(POLICIES, userId).pipe( const filteredPolicies$ = this.stateProvider.getUserState$(POLICIES, userId).pipe(
map((policyData) => policyRecordToArray(policyData)), map((policyData) => policyRecordToArray(policyData)),
map((policies) => policies.filter((p) => p.type === policyType)), map((policies) => policies.filter((p) => p.type === policyType)),

View File

@ -4,11 +4,13 @@ import { CommonModule } from "@angular/common";
import { Component, Input, OnInit } from "@angular/core"; import { Component, Input, OnInit } from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { FormBuilder, ReactiveFormsModule } from "@angular/forms"; import { FormBuilder, ReactiveFormsModule } from "@angular/forms";
import { firstValueFrom, map } from "rxjs"; import { firstValueFrom, map, switchMap } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module"; import { JslibModule } from "@bitwarden/angular/jslib.module";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { SendView } from "@bitwarden/common/tools/send/models/view/send.view"; import { SendView } from "@bitwarden/common/tools/send/models/view/send.view";
import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction"; import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction";
@ -89,11 +91,14 @@ export class SendOptionsComponent implements OnInit {
private i18nService: I18nService, private i18nService: I18nService,
private toastService: ToastService, private toastService: ToastService,
private generatorService: CredentialGeneratorService, private generatorService: CredentialGeneratorService,
private accountService: AccountService,
) { ) {
this.sendFormContainer.registerChildForm("sendOptionsForm", this.sendOptionsForm); this.sendFormContainer.registerChildForm("sendOptionsForm", this.sendOptionsForm);
this.policyService
.getAll$(PolicyType.SendOptions) this.accountService.activeAccount$
.pipe( .pipe(
getUserId,
switchMap((userId) => this.policyService.getAll$(PolicyType.SendOptions, userId)),
map((policies) => policies?.some((p) => p.data.disableHideEmail)), map((policies) => policies?.some((p) => p.data.disableHideEmail)),
takeUntilDestroyed(), takeUntilDestroyed(),
) )