1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-16 08:13:42 +00:00

BEEEP - Auth AccountService Improvements (#11779)

* BEEEP Adjacent - AccountService misc improvements - (1) prefer null over undefined and (2) add new Account type

* LockCompV2 - Fix activeAccount type per PR feedback

* AccountService - update getUserId per PR feedback.
This commit is contained in:
Jared Snider
2024-10-30 17:54:39 -04:00
committed by GitHub
parent 690e175b1d
commit 4de7cb8012
12 changed files with 56 additions and 40 deletions

View File

@@ -12,6 +12,8 @@ export type AccountInfo = {
name: string | undefined;
};
export type Account = { id: UserId } & AccountInfo;
export function accountInfoEqual(a: AccountInfo, b: AccountInfo) {
if (a == null && b == null) {
return true;
@@ -32,7 +34,8 @@ export function accountInfoEqual(a: AccountInfo, b: AccountInfo) {
export abstract class AccountService {
accounts$: Observable<Record<UserId, AccountInfo>>;
activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>;
activeAccount$: Observable<Account | null>;
/**
* Observable of the last activity time for each account.
@@ -41,7 +44,7 @@ export abstract class AccountService {
/** Account list in order of descending recency */
sortedUserIds$: Observable<UserId[]>;
/** Next account that is not the current active account */
nextUpAccount$: Observable<{ id: UserId } & AccountInfo>;
nextUpAccount$: Observable<Account>;
/**
* Updates the `accounts$` observable with the new account data.
*

View File

@@ -88,10 +88,10 @@ describe("accountService", () => {
});
describe("activeAccount$", () => {
it("should emit undefined if no account is active", () => {
it("should emit null if no account is active", () => {
const emissions = trackEmissions(sut.activeAccount$);
expect(emissions).toEqual([undefined]);
expect(emissions).toEqual([null]);
});
it("should emit the active account", async () => {
@@ -100,7 +100,7 @@ describe("accountService", () => {
activeAccountIdState.stateSubject.next(userId);
expect(emissions).toEqual([
undefined, // initial value
null, // initial value
{ id: userId, ...userInfo },
]);
});
@@ -258,10 +258,10 @@ describe("accountService", () => {
activeAccountIdState.stateSubject.next(userId);
});
it("should emit undefined if no account is provided", async () => {
it("should emit null if no account is provided", async () => {
await sut.switchAccount(null);
const currentState = await firstValueFrom(sut.activeAccount$);
expect(currentState).toBeUndefined();
expect(currentState).toBeNull();
});
it("should throw if the account does not exist", () => {

View File

@@ -8,6 +8,7 @@ import {
} from "rxjs";
import {
Account,
AccountInfo,
InternalAccountService,
accountInfoEqual,
@@ -48,9 +49,9 @@ const LOGGED_OUT_INFO: AccountInfo = {
/**
* An rxjs map operator that extracts the UserId from an account, or throws if the account or UserId are null.
*/
export const getUserId = map<{ id: UserId | undefined }, UserId>((account) => {
if (account?.id == null) {
throw new Error("Null account or account ID");
export const getUserId = map<Account | null, UserId>((account) => {
if (account == null) {
throw new Error("Null or undefined account");
}
return account.id;
@@ -59,8 +60,8 @@ export const getUserId = map<{ id: UserId | undefined }, UserId>((account) => {
/**
* An rxjs map operator that extracts the UserId from an account, or returns undefined if the account or UserId are null.
*/
export const getOptionalUserId = map<{ id: UserId | undefined }, UserId | undefined>(
(account) => account?.id ?? undefined,
export const getOptionalUserId = map<Account | null, UserId | null>(
(account) => account?.id ?? null,
);
export class AccountServiceImplementation implements InternalAccountService {
@@ -68,10 +69,10 @@ export class AccountServiceImplementation implements InternalAccountService {
private activeAccountIdState: GlobalState<UserId | undefined>;
accounts$: Observable<Record<UserId, AccountInfo>>;
activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>;
activeAccount$: Observable<Account | null>;
accountActivity$: Observable<Record<UserId, Date>>;
sortedUserIds$: Observable<UserId[]>;
nextUpAccount$: Observable<{ id: UserId } & AccountInfo>;
nextUpAccount$: Observable<Account>;
constructor(
private messagingService: MessagingService,
@@ -86,7 +87,7 @@ export class AccountServiceImplementation implements InternalAccountService {
);
this.activeAccount$ = this.activeAccountIdState.state$.pipe(
combineLatestWith(this.accounts$),
map(([id, accounts]) => (id ? { id, ...(accounts[id] as AccountInfo) } : undefined)),
map(([id, accounts]) => (id ? ({ id, ...(accounts[id] as AccountInfo) } as Account) : null)),
distinctUntilChanged((a, b) => a?.id === b?.id && accountInfoEqual(a, b)),
shareReplay({ bufferSize: 1, refCount: false }),
);

View File

@@ -9,12 +9,12 @@ import { KeyService } from "../../../../key-management/src/abstractions/key.serv
import { OrganizationApiServiceAbstraction } from "../../admin-console/abstractions/organization/organization-api.service.abstraction";
import { OrganizationAutoEnrollStatusResponse } from "../../admin-console/models/response/organization-auto-enroll-status.response";
import { I18nService } from "../../platform/abstractions/i18n.service";
import { AccountInfo, AccountService } from "../abstractions/account.service";
import { Account, AccountInfo, AccountService } from "../abstractions/account.service";
import { PasswordResetEnrollmentServiceImplementation } from "./password-reset-enrollment.service.implementation";
describe("PasswordResetEnrollmentServiceImplementation", () => {
const activeAccountSubject = new BehaviorSubject<{ id: UserId } & AccountInfo>(null);
const activeAccountSubject = new BehaviorSubject<Account | null>(null);
let organizationApiService: MockProxy<OrganizationApiServiceAbstraction>;
let accountService: MockProxy<AccountService>;