[PM-6382] Migrate disableContextMenuItem to Autofill Settings state (#8246)

* add enableContextMenu to autofillSettings and migrate disableContextMenuItem

* replace usages of disableContextMenuItem with autofill settings service global enableContextMenu
This commit is contained in:
Jonathan Prusik 2024-03-08 10:34:07 -05:00 committed by GitHub
parent 4cd8b07ea3
commit f90de62adc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 182 additions and 33 deletions

View File

@ -1,6 +1,8 @@
import { mock, MockProxy } from "jest-mock-extended";
import { of } from "rxjs";
import { NOOP_COMMAND_SUFFIX } from "@bitwarden/common/autofill/constants";
import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { CipherType } from "@bitwarden/common/vault/enums";
@ -13,6 +15,7 @@ import { MainContextMenuHandler } from "./main-context-menu-handler";
describe("context-menu", () => {
let stateService: MockProxy<BrowserStateService>;
let autofillSettingsService: MockProxy<AutofillSettingsServiceAbstraction>;
let i18nService: MockProxy<I18nService>;
let logService: MockProxy<LogService>;
@ -26,6 +29,7 @@ describe("context-menu", () => {
beforeEach(() => {
stateService = mock();
autofillSettingsService = mock();
i18nService = mock();
logService = mock();
@ -41,14 +45,20 @@ describe("context-menu", () => {
});
i18nService.t.mockImplementation((key) => key);
sut = new MainContextMenuHandler(stateService, i18nService, logService);
sut = new MainContextMenuHandler(
stateService,
autofillSettingsService,
i18nService,
logService,
);
autofillSettingsService.enableContextMenu$ = of(true);
});
afterEach(() => jest.resetAllMocks());
describe("init", () => {
it("has menu disabled", async () => {
stateService.getDisableContextMenuItem.mockResolvedValue(true);
autofillSettingsService.enableContextMenu$ = of(false);
const createdMenu = await sut.init();
expect(createdMenu).toBeFalsy();
@ -56,8 +66,6 @@ describe("context-menu", () => {
});
it("has menu enabled, but does not have premium", async () => {
stateService.getDisableContextMenuItem.mockResolvedValue(false);
stateService.getCanAccessPremium.mockResolvedValue(false);
const createdMenu = await sut.init();
@ -66,8 +74,6 @@ describe("context-menu", () => {
});
it("has menu enabled and has premium", async () => {
stateService.getDisableContextMenuItem.mockResolvedValue(false);
stateService.getCanAccessPremium.mockResolvedValue(true);
const createdMenu = await sut.init();

View File

@ -1,3 +1,5 @@
import { firstValueFrom } from "rxjs";
import {
AUTOFILL_CARD_ID,
AUTOFILL_ID,
@ -14,6 +16,7 @@ import {
ROOT_ID,
SEPARATOR_ID,
} from "@bitwarden/common/autofill/constants";
import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
@ -22,6 +25,7 @@ import { GlobalState } from "@bitwarden/common/platform/models/domain/global-sta
import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { autofillSettingsServiceFactory } from "../../autofill/background/service_factories/autofill-settings-service.factory";
import { Account } from "../../models/account";
import { CachedServices } from "../../platform/background/service-factories/factory-options";
import {
@ -156,6 +160,7 @@ export class MainContextMenuHandler {
constructor(
private stateService: BrowserStateService,
private autofillSettingsService: AutofillSettingsServiceAbstraction,
private i18nService: I18nService,
private logService: LogService,
) {}
@ -183,6 +188,7 @@ export class MainContextMenuHandler {
return new MainContextMenuHandler(
await stateServiceFactory(cachedServices, serviceOptions),
await autofillSettingsServiceFactory(cachedServices, serviceOptions),
await i18nServiceFactory(cachedServices, serviceOptions),
await logServiceFactory(cachedServices, serviceOptions),
);
@ -193,8 +199,8 @@ export class MainContextMenuHandler {
* @returns a boolean showing whether or not items were created
*/
async init(): Promise<boolean> {
const menuDisabled = await this.stateService.getDisableContextMenuItem();
if (menuDisabled) {
const menuEnabled = await firstValueFrom(this.autofillSettingsService.enableContextMenu$);
if (!menuEnabled) {
await MainContextMenuHandler.removeAll();
return false;
}

View File

@ -950,6 +950,7 @@ export default class MainBackground {
if (!this.popupOnlyContext) {
this.mainContextMenuHandler = new MainContextMenuHandler(
this.stateService,
this.autofillSettingsService,
this.i18nService,
this.logService,
);

View File

@ -105,7 +105,9 @@ export class OptionsComponent implements OnInit {
this.userNotificationSettingsService.enableChangedPasswordPrompt$,
);
this.enableContextMenuItem = !(await this.stateService.getDisableContextMenuItem());
this.enableContextMenuItem = await firstValueFrom(
this.autofillSettingsService.enableContextMenu$,
);
this.showCardsCurrentTab = !(await this.stateService.getDontShowCardsCurrentTab());
this.showIdentitiesCurrentTab = !(await this.stateService.getDontShowIdentitiesCurrentTab());
@ -143,7 +145,7 @@ export class OptionsComponent implements OnInit {
}
async updateContextMenuItem() {
await this.stateService.setDisableContextMenuItem(!this.enableContextMenuItem);
await this.autofillSettingsService.setEnableContextMenu(this.enableContextMenuItem);
this.messagingService.send("bgUpdateContextMenu");
}

View File

@ -53,6 +53,10 @@ const INLINE_MENU_VISIBILITY = new KeyDefinition(
},
);
const ENABLE_CONTEXT_MENU = new KeyDefinition(AUTOFILL_SETTINGS_DISK, "enableContextMenu", {
deserializer: (value: boolean) => value ?? true,
});
const CLEAR_CLIPBOARD_DELAY = new KeyDefinition(
AUTOFILL_SETTINGS_DISK_LOCAL,
"clearClipboardDelay",
@ -75,6 +79,8 @@ export abstract class AutofillSettingsServiceAbstraction {
setAutoCopyTotp: (newValue: boolean) => Promise<void>;
inlineMenuVisibility$: Observable<InlineMenuVisibilitySetting>;
setInlineMenuVisibility: (newValue: InlineMenuVisibilitySetting) => Promise<void>;
enableContextMenu$: Observable<boolean>;
setEnableContextMenu: (newValue: boolean) => Promise<void>;
clearClipboardDelay$: Observable<ClearClipboardDelaySetting>;
setClearClipboardDelay: (newValue: ClearClipboardDelaySetting) => Promise<void>;
}
@ -100,6 +106,9 @@ export class AutofillSettingsService implements AutofillSettingsServiceAbstracti
private inlineMenuVisibilityState: GlobalState<InlineMenuVisibilitySetting>;
readonly inlineMenuVisibility$: Observable<InlineMenuVisibilitySetting>;
private enableContextMenuState: GlobalState<boolean>;
readonly enableContextMenu$: Observable<boolean>;
private clearClipboardDelayState: ActiveUserState<ClearClipboardDelaySetting>;
readonly clearClipboardDelay$: Observable<ClearClipboardDelaySetting>;
@ -142,6 +151,9 @@ export class AutofillSettingsService implements AutofillSettingsServiceAbstracti
map((x) => x ?? AutofillOverlayVisibility.Off),
);
this.enableContextMenuState = this.stateProvider.getGlobal(ENABLE_CONTEXT_MENU);
this.enableContextMenu$ = this.enableContextMenuState.state$.pipe(map((x) => x ?? true));
this.clearClipboardDelayState = this.stateProvider.getActive(CLEAR_CLIPBOARD_DELAY);
this.clearClipboardDelay$ = this.clearClipboardDelayState.state$.pipe(
map((x) => x ?? ClearClipboardDelay.Never),
@ -172,6 +184,10 @@ export class AutofillSettingsService implements AutofillSettingsServiceAbstracti
await this.inlineMenuVisibilityState.update(() => newValue);
}
async setEnableContextMenu(newValue: boolean): Promise<void> {
await this.enableContextMenuState.update(() => newValue);
}
async setClearClipboardDelay(newValue: ClearClipboardDelaySetting): Promise<void> {
await this.clearClipboardDelayState.update(() => newValue);
}

View File

@ -189,8 +189,6 @@ export abstract class StateService<T extends Account = Account> {
setDecryptedSends: (value: SendView[], options?: StorageOptions) => Promise<void>;
getDefaultUriMatch: (options?: StorageOptions) => Promise<UriMatchType>;
setDefaultUriMatch: (value: UriMatchType, options?: StorageOptions) => Promise<void>;
getDisableContextMenuItem: (options?: StorageOptions) => Promise<boolean>;
setDisableContextMenuItem: (value: boolean, options?: StorageOptions) => Promise<void>;
/**
* @deprecated Do not call this, use SettingsService
*/

View File

@ -26,6 +26,5 @@ export class GlobalState {
enableBrowserIntegrationFingerprint?: boolean;
enableDuckDuckGoBrowserIntegration?: boolean;
neverDomains?: { [id: string]: unknown };
disableContextMenuItem?: boolean;
deepLinkRedirectUrl?: string;
}

View File

@ -832,24 +832,6 @@ export class StateService<
);
}
async getDisableContextMenuItem(options?: StorageOptions): Promise<boolean> {
return (
(await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions())))
?.disableContextMenuItem ?? false
);
}
async setDisableContextMenuItem(value: boolean, options?: StorageOptions): Promise<void> {
const globals = await this.getGlobals(
this.reconcileOptions(options, await this.defaultOnDiskOptions()),
);
globals.disableContextMenuItem = value;
await this.saveGlobals(
globals,
this.reconcileOptions(options, await this.defaultOnDiskOptions()),
);
}
async getDisableFavicon(options?: StorageOptions): Promise<boolean> {
return (
(

View File

@ -26,6 +26,7 @@ import { MoveBiometricUnlockToStateProviders } from "./migrations/28-move-biomet
import { UserNotificationSettingsKeyMigrator } from "./migrations/29-move-user-notification-settings-to-state-provider";
import { FixPremiumMigrator } from "./migrations/3-fix-premium";
import { PolicyMigrator } from "./migrations/30-move-policy-state-to-state-provider";
import { EnableContextMenuMigrator } from "./migrations/31-move-enable-context-menu-to-autofill-settings-state-provider";
import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked";
import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys";
import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key";
@ -35,7 +36,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting
import { MinVersionMigrator } from "./migrations/min-version";
export const MIN_VERSION = 2;
export const CURRENT_VERSION = 30;
export const CURRENT_VERSION = 31;
export type MinVersion = typeof MIN_VERSION;
export function createMigrationBuilder() {
@ -68,7 +69,8 @@ export function createMigrationBuilder() {
.with(BadgeSettingsMigrator, 26, 27)
.with(MoveBiometricUnlockToStateProviders, 27, 28)
.with(UserNotificationSettingsKeyMigrator, 28, 29)
.with(PolicyMigrator, 29, CURRENT_VERSION);
.with(PolicyMigrator, 29, 30)
.with(EnableContextMenuMigrator, 30, CURRENT_VERSION);
}
export async function currentVersion(

View File

@ -0,0 +1,91 @@
import { any, MockProxy } from "jest-mock-extended";
import { KeyDefinitionLike, MigrationHelper } from "../migration-helper";
import { mockMigrationHelper } from "../migration-helper.spec";
import { EnableContextMenuMigrator } from "./31-move-enable-context-menu-to-autofill-settings-state-provider";
function exampleJSON() {
return {
global: {
disableContextMenuItem: true,
otherStuff: "otherStuff1",
},
otherStuff: "otherStuff2",
};
}
function rollbackJSON() {
return {
global_autofillSettings_enableContextMenu: false,
global: {
otherStuff: "otherStuff1",
},
otherStuff: "otherStuff2",
};
}
const enableContextMenuKeyDefinition: KeyDefinitionLike = {
stateDefinition: {
name: "autofillSettings",
},
key: "enableContextMenu",
};
describe("EnableContextMenuMigrator", () => {
let helper: MockProxy<MigrationHelper>;
let sut: EnableContextMenuMigrator;
describe("migrate", () => {
beforeEach(() => {
helper = mockMigrationHelper(exampleJSON(), 30);
sut = new EnableContextMenuMigrator(30, 31);
});
it("should remove global disableContextMenuItem setting", async () => {
await sut.migrate(helper);
expect(helper.set).toHaveBeenCalledTimes(1);
expect(helper.set).toHaveBeenCalledWith("global", {
otherStuff: "otherStuff1",
});
});
it("should set enableContextMenu globally", async () => {
await sut.migrate(helper);
expect(helper.setToGlobal).toHaveBeenCalledTimes(1);
expect(helper.setToGlobal).toHaveBeenCalledWith(enableContextMenuKeyDefinition, false);
});
});
describe("rollback", () => {
beforeEach(() => {
helper = mockMigrationHelper(rollbackJSON(), 31);
sut = new EnableContextMenuMigrator(30, 31);
});
it("should null out new enableContextMenu global value", async () => {
await sut.rollback(helper);
expect(helper.setToGlobal).toHaveBeenCalledTimes(1);
expect(helper.setToGlobal).toHaveBeenCalledWith(enableContextMenuKeyDefinition, null);
});
it("should add disableContextMenuItem global value back", async () => {
await sut.rollback(helper);
expect(helper.set).toHaveBeenCalledTimes(1);
expect(helper.set).toHaveBeenCalledWith("global", {
disableContextMenuItem: true,
otherStuff: "otherStuff1",
});
});
it("should not try to restore values to missing accounts", async () => {
await sut.rollback(helper);
expect(helper.set).not.toHaveBeenCalledWith("user-3", any());
});
});
});

View File

@ -0,0 +1,46 @@
import { KeyDefinitionLike, MigrationHelper } from "../migration-helper";
import { Migrator } from "../migrator";
type ExpectedGlobalState = {
disableContextMenuItem?: boolean;
};
const enableContextMenuKeyDefinition: KeyDefinitionLike = {
stateDefinition: {
name: "autofillSettings",
},
key: "enableContextMenu",
};
export class EnableContextMenuMigrator extends Migrator<30, 31> {
async migrate(helper: MigrationHelper): Promise<void> {
const globalState = await helper.get<ExpectedGlobalState>("global");
// disableContextMenuItem -> enableContextMenu
if (globalState?.disableContextMenuItem != null) {
await helper.setToGlobal(enableContextMenuKeyDefinition, !globalState.disableContextMenuItem);
// delete `disableContextMenuItem` from state global
delete globalState.disableContextMenuItem;
await helper.set<ExpectedGlobalState>("global", globalState);
}
}
async rollback(helper: MigrationHelper): Promise<void> {
const globalState = (await helper.get<ExpectedGlobalState>("global")) || {};
const enableContextMenu: boolean = await helper.getFromGlobal(enableContextMenuKeyDefinition);
// enableContextMenu -> disableContextMenuItem
if (enableContextMenu != null) {
await helper.set<ExpectedGlobalState>("global", {
...globalState,
disableContextMenuItem: !enableContextMenu,
});
// remove the global state provider framework key for `enableContextMenu`
await helper.setToGlobal(enableContextMenuKeyDefinition, null);
}
}
}