mirror of
https://github.com/bitwarden/browser
synced 2026-01-21 11:53:34 +00:00
feat(account): [PM-29545] Update AccountInfo creationDate to use Date instead of string
* Add creationDate of account to AccountInfo * Added initialization of creationDate. * Removed extra changes. * Fixed tests to initialize creation date * Added helper method to abstract account initialization in tests. * More test updates. * Linting * Additional test fixes. * Fixed spec reference * Fixed imports * Linting. * Fixed browser test. * Modified tsconfig to reference spec file. * Fixed import. * Removed dependency on os. This is necessary so that the @bitwarden/common/spec lib package can be referenced in tests without node. * Revert "Removed dependency on os. This is necessary so that the @bitwarden/common/spec lib package can be referenced in tests without node." This reverts commit669f6557b6. * Updated stories to hard-code new field. * Removed changes to tsconfig * Revert "Removed changes to tsconfig" This reverts commitb7d916e8dc. * Updated to use Date * Updated to use Date on sync. * Changes to tests that can't use mock function * Prettier updates * Update equality to handle Date type. * Change to type comparison. * Simplified equality comparison to just use properties. * Added comment. * Updated comment to reference Date. * Added back in internal method tests.
This commit is contained in:
@@ -82,7 +82,7 @@ class MockAccountService implements Partial<AccountService> {
|
||||
name: "Test User 1",
|
||||
email: "test@email.com",
|
||||
emailVerified: true,
|
||||
creationDate: "2024-01-01T00:00:00.000Z",
|
||||
creationDate: new Date("2024-01-01T00:00:00.000Z"),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -82,7 +82,7 @@ class MockAccountService implements Partial<AccountService> {
|
||||
name: "Test User 1",
|
||||
email: "test@email.com",
|
||||
emailVerified: true,
|
||||
creationDate: "2024-01-01T00:00:00.000Z",
|
||||
creationDate: new Date("2024-01-01T00:00:00.000Z"),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -15,7 +15,7 @@ export function mockAccountInfoWith(info: Partial<AccountInfo> = {}): AccountInf
|
||||
name: "name",
|
||||
email: "email",
|
||||
emailVerified: true,
|
||||
creationDate: "2024-01-01T00:00:00.000Z",
|
||||
creationDate: new Date("2024-01-01T00:00:00.000Z"),
|
||||
...info,
|
||||
};
|
||||
}
|
||||
@@ -111,7 +111,7 @@ export class FakeAccountService implements AccountService {
|
||||
await this.mock.setAccountEmailVerified(userId, emailVerified);
|
||||
}
|
||||
|
||||
async setAccountCreationDate(userId: UserId, creationDate: string): Promise<void> {
|
||||
async setAccountCreationDate(userId: UserId, creationDate: Date): Promise<void> {
|
||||
await this.mock.setAccountCreationDate(userId, creationDate);
|
||||
}
|
||||
|
||||
|
||||
@@ -2,33 +2,25 @@ import { Observable } from "rxjs";
|
||||
|
||||
import { UserId } from "../../types/guid";
|
||||
|
||||
/**
|
||||
* Holds state that represents a user's account with Bitwarden.
|
||||
* Any additions here should be added to the equality check in the AccountService
|
||||
* to ensure that emissions are done on every change.
|
||||
*
|
||||
* @property email - User's email address.
|
||||
* @property emailVerified - Whether the email has been verified.
|
||||
* @property name - User's display name (optional).
|
||||
* @property creationDate - Date when the account was created.
|
||||
* Will be undefined immediately after login until the first sync completes.
|
||||
*/
|
||||
export type AccountInfo = {
|
||||
email: string;
|
||||
emailVerified: boolean;
|
||||
name: string | undefined;
|
||||
creationDate: string | undefined;
|
||||
creationDate: Date | undefined;
|
||||
};
|
||||
|
||||
export type Account = { id: UserId } & AccountInfo;
|
||||
|
||||
export function accountInfoEqual(a: AccountInfo, b: AccountInfo) {
|
||||
if (a == null && b == null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (a == null || b == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const keys = new Set([...Object.keys(a), ...Object.keys(b)]) as Set<keyof AccountInfo>;
|
||||
for (const key of keys) {
|
||||
if (a[key] !== b[key]) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export abstract class AccountService {
|
||||
abstract accounts$: Observable<Record<UserId, AccountInfo>>;
|
||||
|
||||
@@ -77,7 +69,7 @@ export abstract class AccountService {
|
||||
* @param userId
|
||||
* @param creationDate
|
||||
*/
|
||||
abstract setAccountCreationDate(userId: UserId, creationDate: string): Promise<void>;
|
||||
abstract setAccountCreationDate(userId: UserId, creationDate: Date): Promise<void>;
|
||||
/**
|
||||
* updates the `accounts$` observable with the new VerifyNewDeviceLogin property for the account.
|
||||
* @param userId
|
||||
|
||||
@@ -17,7 +17,7 @@ import { LogService } from "../../platform/abstractions/log.service";
|
||||
import { MessagingService } from "../../platform/abstractions/messaging.service";
|
||||
import { Utils } from "../../platform/misc/utils";
|
||||
import { UserId } from "../../types/guid";
|
||||
import { AccountInfo, accountInfoEqual } from "../abstractions/account.service";
|
||||
import { AccountInfo } from "../abstractions/account.service";
|
||||
|
||||
import {
|
||||
ACCOUNT_ACCOUNTS,
|
||||
@@ -27,63 +27,6 @@ import {
|
||||
AccountServiceImplementation,
|
||||
} from "./account.service";
|
||||
|
||||
describe("accountInfoEqual", () => {
|
||||
const accountInfo = mockAccountInfoWith();
|
||||
|
||||
it("compares nulls", () => {
|
||||
expect(accountInfoEqual(null, null)).toBe(true);
|
||||
expect(accountInfoEqual(null, accountInfo)).toBe(false);
|
||||
expect(accountInfoEqual(accountInfo, null)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares all keys, not just those defined in AccountInfo", () => {
|
||||
const different = { ...accountInfo, extra: "extra" };
|
||||
|
||||
expect(accountInfoEqual(accountInfo, different)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares name", () => {
|
||||
const same = { ...accountInfo };
|
||||
const different = { ...accountInfo, name: "name2" };
|
||||
|
||||
expect(accountInfoEqual(accountInfo, same)).toBe(true);
|
||||
expect(accountInfoEqual(accountInfo, different)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares email", () => {
|
||||
const same = { ...accountInfo };
|
||||
const different = { ...accountInfo, email: "email2" };
|
||||
|
||||
expect(accountInfoEqual(accountInfo, same)).toBe(true);
|
||||
expect(accountInfoEqual(accountInfo, different)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares emailVerified", () => {
|
||||
const same = { ...accountInfo };
|
||||
const different = { ...accountInfo, emailVerified: false };
|
||||
|
||||
expect(accountInfoEqual(accountInfo, same)).toBe(true);
|
||||
expect(accountInfoEqual(accountInfo, different)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares creationDate", () => {
|
||||
const same = { ...accountInfo };
|
||||
const different = { ...accountInfo, creationDate: "2024-12-31T00:00:00.000Z" };
|
||||
|
||||
expect(accountInfoEqual(accountInfo, same)).toBe(true);
|
||||
expect(accountInfoEqual(accountInfo, different)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares undefined creationDate", () => {
|
||||
const accountWithoutCreationDate = mockAccountInfoWith({ creationDate: undefined });
|
||||
const same = { ...accountWithoutCreationDate };
|
||||
const different = { ...accountWithoutCreationDate, creationDate: "2024-01-01T00:00:00.000Z" };
|
||||
|
||||
expect(accountInfoEqual(accountWithoutCreationDate, same)).toBe(true);
|
||||
expect(accountInfoEqual(accountWithoutCreationDate, different)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("accountService", () => {
|
||||
let messagingService: MockProxy<MessagingService>;
|
||||
let logService: MockProxy<LogService>;
|
||||
@@ -121,6 +64,60 @@ describe("accountService", () => {
|
||||
jest.resetAllMocks();
|
||||
});
|
||||
|
||||
describe("accountInfoEqual", () => {
|
||||
const accountInfo = mockAccountInfoWith();
|
||||
|
||||
it("compares nulls", () => {
|
||||
expect((sut as any).accountInfoEqual(null, null)).toBe(true);
|
||||
expect((sut as any).accountInfoEqual(null, accountInfo)).toBe(false);
|
||||
expect((sut as any).accountInfoEqual(accountInfo, null)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares name", () => {
|
||||
const same = { ...accountInfo };
|
||||
const different = { ...accountInfo, name: "name2" };
|
||||
|
||||
expect((sut as any).accountInfoEqual(accountInfo, same)).toBe(true);
|
||||
expect((sut as any).accountInfoEqual(accountInfo, different)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares email", () => {
|
||||
const same = { ...accountInfo };
|
||||
const different = { ...accountInfo, email: "email2" };
|
||||
|
||||
expect((sut as any).accountInfoEqual(accountInfo, same)).toBe(true);
|
||||
expect((sut as any).accountInfoEqual(accountInfo, different)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares emailVerified", () => {
|
||||
const same = { ...accountInfo };
|
||||
const different = { ...accountInfo, emailVerified: false };
|
||||
|
||||
expect((sut as any).accountInfoEqual(accountInfo, same)).toBe(true);
|
||||
expect((sut as any).accountInfoEqual(accountInfo, different)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares creationDate", () => {
|
||||
const same = { ...accountInfo };
|
||||
const different = { ...accountInfo, creationDate: new Date("2024-12-31T00:00:00.000Z") };
|
||||
|
||||
expect((sut as any).accountInfoEqual(accountInfo, same)).toBe(true);
|
||||
expect((sut as any).accountInfoEqual(accountInfo, different)).toBe(false);
|
||||
});
|
||||
|
||||
it("compares undefined creationDate", () => {
|
||||
const accountWithoutCreationDate = mockAccountInfoWith({ creationDate: undefined });
|
||||
const same = { ...accountWithoutCreationDate };
|
||||
const different = {
|
||||
...accountWithoutCreationDate,
|
||||
creationDate: new Date("2024-01-01T00:00:00.000Z"),
|
||||
};
|
||||
|
||||
expect((sut as any).accountInfoEqual(accountWithoutCreationDate, same)).toBe(true);
|
||||
expect((sut as any).accountInfoEqual(accountWithoutCreationDate, different)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("activeAccount$", () => {
|
||||
it("should emit null if no account is active", () => {
|
||||
const emissions = trackEmissions(sut.activeAccount$);
|
||||
@@ -281,7 +278,7 @@ describe("accountService", () => {
|
||||
});
|
||||
|
||||
it("should update the account with a new creation date", async () => {
|
||||
const newCreationDate = "2024-12-31T00:00:00.000Z";
|
||||
const newCreationDate = new Date("2024-12-31T00:00:00.000Z");
|
||||
await sut.setAccountCreationDate(userId, newCreationDate);
|
||||
const currentState = await firstValueFrom(accountsState.state$);
|
||||
|
||||
@@ -297,6 +294,24 @@ describe("accountService", () => {
|
||||
expect(currentState).toEqual(initialState);
|
||||
});
|
||||
|
||||
it("should not update if the creation date has the same timestamp but different Date object", async () => {
|
||||
const sameTimestamp = new Date(userInfo.creationDate.getTime());
|
||||
await sut.setAccountCreationDate(userId, sameTimestamp);
|
||||
const currentState = await firstValueFrom(accountsState.state$);
|
||||
|
||||
expect(currentState).toEqual(initialState);
|
||||
});
|
||||
|
||||
it("should update if the creation date has a different timestamp", async () => {
|
||||
const differentDate = new Date(userInfo.creationDate.getTime() + 1000);
|
||||
await sut.setAccountCreationDate(userId, differentDate);
|
||||
const currentState = await firstValueFrom(accountsState.state$);
|
||||
|
||||
expect(currentState).toEqual({
|
||||
[userId]: { ...userInfo, creationDate: differentDate },
|
||||
});
|
||||
});
|
||||
|
||||
it("should update from undefined to a defined creation date", async () => {
|
||||
const accountWithoutCreationDate = mockAccountInfoWith({
|
||||
...userInfo,
|
||||
@@ -304,7 +319,7 @@ describe("accountService", () => {
|
||||
});
|
||||
accountsState.stateSubject.next({ [userId]: accountWithoutCreationDate });
|
||||
|
||||
const newCreationDate = "2024-06-15T12:30:00.000Z";
|
||||
const newCreationDate = new Date("2024-06-15T12:30:00.000Z");
|
||||
await sut.setAccountCreationDate(userId, newCreationDate);
|
||||
const currentState = await firstValueFrom(accountsState.state$);
|
||||
|
||||
@@ -313,14 +328,19 @@ describe("accountService", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("should update to a different creation date string format", async () => {
|
||||
const newCreationDate = "2023-03-15T08:45:30.123Z";
|
||||
await sut.setAccountCreationDate(userId, newCreationDate);
|
||||
const currentState = await firstValueFrom(accountsState.state$);
|
||||
|
||||
expect(currentState).toEqual({
|
||||
[userId]: { ...userInfo, creationDate: newCreationDate },
|
||||
it("should not update when both creation dates are undefined", async () => {
|
||||
const accountWithoutCreationDate = mockAccountInfoWith({
|
||||
...userInfo,
|
||||
creationDate: undefined,
|
||||
});
|
||||
accountsState.stateSubject.next({ [userId]: accountWithoutCreationDate });
|
||||
|
||||
// Attempt to set to undefined (shouldn't trigger update)
|
||||
const currentStateBefore = await firstValueFrom(accountsState.state$);
|
||||
|
||||
// We can't directly call setAccountCreationDate with undefined, but we can verify
|
||||
// the behavior through setAccountInfo which accountInfoEqual uses internally
|
||||
expect(currentStateBefore[userId].creationDate).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -18,7 +18,6 @@ import {
|
||||
Account,
|
||||
AccountInfo,
|
||||
InternalAccountService,
|
||||
accountInfoEqual,
|
||||
} from "../../auth/abstractions/account.service";
|
||||
import { LogService } from "../../platform/abstractions/log.service";
|
||||
import { MessagingService } from "../../platform/abstractions/messaging.service";
|
||||
@@ -37,7 +36,10 @@ export const ACCOUNT_ACCOUNTS = KeyDefinition.record<AccountInfo, UserId>(
|
||||
ACCOUNT_DISK,
|
||||
"accounts",
|
||||
{
|
||||
deserializer: (accountInfo) => accountInfo,
|
||||
deserializer: (accountInfo) => ({
|
||||
...accountInfo,
|
||||
creationDate: accountInfo.creationDate ? new Date(accountInfo.creationDate) : undefined,
|
||||
}),
|
||||
},
|
||||
);
|
||||
|
||||
@@ -111,7 +113,7 @@ export class AccountServiceImplementation implements InternalAccountService {
|
||||
this.activeAccount$ = this.activeAccountIdState.state$.pipe(
|
||||
combineLatestWith(this.accounts$),
|
||||
map(([id, accounts]) => (id ? ({ id, ...(accounts[id] as AccountInfo) } as Account) : null)),
|
||||
distinctUntilChanged((a, b) => a?.id === b?.id && accountInfoEqual(a, b)),
|
||||
distinctUntilChanged((a, b) => a?.id === b?.id && this.accountInfoEqual(a, b)),
|
||||
shareReplay({ bufferSize: 1, refCount: false }),
|
||||
);
|
||||
this.accountActivity$ = this.globalStateProvider
|
||||
@@ -168,7 +170,7 @@ export class AccountServiceImplementation implements InternalAccountService {
|
||||
await this.setAccountInfo(userId, { emailVerified });
|
||||
}
|
||||
|
||||
async setAccountCreationDate(userId: UserId, creationDate: string): Promise<void> {
|
||||
async setAccountCreationDate(userId: UserId, creationDate: Date): Promise<void> {
|
||||
await this.setAccountInfo(userId, { creationDate });
|
||||
}
|
||||
|
||||
@@ -274,6 +276,23 @@ export class AccountServiceImplementation implements InternalAccountService {
|
||||
this._showHeader$.next(visible);
|
||||
}
|
||||
|
||||
private accountInfoEqual(a: AccountInfo, b: AccountInfo) {
|
||||
if (a == null && b == null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (a == null || b == null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return (
|
||||
a.email === b.email &&
|
||||
a.emailVerified === b.emailVerified &&
|
||||
a.name === b.name &&
|
||||
a.creationDate?.getTime() === b.creationDate?.getTime()
|
||||
);
|
||||
}
|
||||
|
||||
private async setAccountInfo(userId: UserId, update: Partial<AccountInfo>): Promise<void> {
|
||||
function newAccountInfo(oldAccountInfo: AccountInfo): AccountInfo {
|
||||
return { ...oldAccountInfo, ...update };
|
||||
@@ -291,7 +310,7 @@ export class AccountServiceImplementation implements InternalAccountService {
|
||||
throw new Error("Account does not exist");
|
||||
}
|
||||
|
||||
return !accountInfoEqual(accounts[userId], newAccountInfo(accounts[userId]));
|
||||
return !this.accountInfoEqual(accounts[userId], newAccountInfo(accounts[userId]));
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
@@ -279,8 +279,8 @@ export class DefaultSyncService extends CoreSyncService {
|
||||
await this.avatarService.setSyncAvatarColor(response.id, response.avatarColor);
|
||||
await this.tokenService.setSecurityStamp(response.securityStamp, response.id);
|
||||
await this.accountService.setAccountEmailVerified(response.id, response.emailVerified);
|
||||
await this.accountService.setAccountCreationDate(response.id, new Date(response.creationDate));
|
||||
await this.accountService.setAccountVerifyNewDeviceLogin(response.id, response.verifyDevices);
|
||||
await this.accountService.setAccountCreationDate(response.id, response.creationDate);
|
||||
|
||||
await this.billingAccountProfileStateService.setHasPremium(
|
||||
response.premiumPersonally,
|
||||
|
||||
Reference in New Issue
Block a user