mirror of
https://github.com/bitwarden/browser
synced 2025-12-14 15:23:33 +00:00
[PM-26731] Fix feature flag check for "pm-25855-chromium-importer-abe" (#17102)
* Fix feature flag check for "pm-25855-chromium-importer-abe" The old lofgic actually removed all chromium support when the flag was disabled. It should only remove those browser if the flag is disabled and when on Windows. * Extend tests * Update comment * Remove duplicate test * Add test for when device cannot be detected and throws and error * Add descriptive comment to feature flag test case assertions * Better test assertion --------- Co-authored-by: Daniel James Smith <djsmith85@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
6b3c4f87c7
commit
9fca0b0138
@@ -61,27 +61,28 @@ export class DefaultImportMetadataService implements ImportMetadataServiceAbstra
|
|||||||
importers: ImportersMetadata,
|
importers: ImportersMetadata,
|
||||||
type: ImportType,
|
type: ImportType,
|
||||||
client: ClientType,
|
client: ClientType,
|
||||||
enabled: boolean,
|
withABESupport: boolean,
|
||||||
): DataLoader[] | undefined {
|
): DataLoader[] | undefined {
|
||||||
let loaders = availableLoaders(importers, type, client);
|
let loaders = availableLoaders(importers, type, client);
|
||||||
let includeABE = false;
|
|
||||||
|
|
||||||
if (enabled && (type === "bravecsv" || type === "chromecsv" || type === "edgecsv")) {
|
if (withABESupport) {
|
||||||
|
return loaders;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Special handling for Brave, Chrome, and Edge CSV imports on Windows Desktop
|
||||||
|
if (type === "bravecsv" || type === "chromecsv" || type === "edgecsv") {
|
||||||
try {
|
try {
|
||||||
const device = this.system.environment.getDevice();
|
const device = this.system.environment.getDevice();
|
||||||
const isWindowsDesktop = device === DeviceType.WindowsDesktop;
|
const isWindowsDesktop = device === DeviceType.WindowsDesktop;
|
||||||
if (isWindowsDesktop) {
|
if (isWindowsDesktop) {
|
||||||
includeABE = true;
|
// Exclude the Chromium loader if on Windows Desktop without ABE support
|
||||||
|
loaders = loaders?.filter((loader) => loader !== Loader.chromium);
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
includeABE = true;
|
loaders = loaders?.filter((loader) => loader !== Loader.chromium);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// If the browser is unsupported, remove the chromium loader
|
|
||||||
if (!includeABE) {
|
|
||||||
loaders = loaders?.filter((loader) => loader !== Loader.chromium);
|
|
||||||
}
|
|
||||||
return loaders;
|
return loaders;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -55,6 +55,25 @@ describe("ImportMetadataService", () => {
|
|||||||
|
|
||||||
// Recreate the service with the updated mocks for logging tests
|
// Recreate the service with the updated mocks for logging tests
|
||||||
sut = new DefaultImportMetadataService(systemServiceProvider);
|
sut = new DefaultImportMetadataService(systemServiceProvider);
|
||||||
|
|
||||||
|
// Set up importers to include bravecsv and chromecsv with chromium loader
|
||||||
|
sut["importers"] = {
|
||||||
|
chromecsv: {
|
||||||
|
type: "chromecsv",
|
||||||
|
loaders: [Loader.file, Loader.chromium],
|
||||||
|
instructions: Instructions.chromium,
|
||||||
|
},
|
||||||
|
bravecsv: {
|
||||||
|
type: "bravecsv",
|
||||||
|
loaders: [Loader.file, Loader.chromium],
|
||||||
|
instructions: Instructions.chromium,
|
||||||
|
},
|
||||||
|
edgecsv: {
|
||||||
|
type: "edgecsv",
|
||||||
|
loaders: [Loader.file, Loader.chromium],
|
||||||
|
instructions: Instructions.chromium,
|
||||||
|
},
|
||||||
|
} as ImportersMetadata;
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@@ -112,6 +131,7 @@ describe("ImportMetadataService", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("should update when feature flag changes", async () => {
|
it("should update when feature flag changes", async () => {
|
||||||
|
environment.getDevice.mockReturnValue(DeviceType.WindowsDesktop);
|
||||||
const testType: ImportType = "bravecsv"; // Use bravecsv which supports chromium loader
|
const testType: ImportType = "bravecsv"; // Use bravecsv which supports chromium loader
|
||||||
const emissions: ImporterMetadata[] = [];
|
const emissions: ImporterMetadata[] = [];
|
||||||
|
|
||||||
@@ -126,13 +146,15 @@ describe("ImportMetadataService", () => {
|
|||||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||||
|
|
||||||
expect(emissions).toHaveLength(2);
|
expect(emissions).toHaveLength(2);
|
||||||
|
// Disable ABE - chromium loader should be excluded
|
||||||
expect(emissions[0].loaders).not.toContain(Loader.chromium);
|
expect(emissions[0].loaders).not.toContain(Loader.chromium);
|
||||||
expect(emissions[1].loaders).toContain(Loader.file);
|
// Enabled ABE - chromium loader should be included
|
||||||
|
expect(emissions[1].loaders).toContain(Loader.chromium);
|
||||||
|
|
||||||
subscription.unsubscribe();
|
subscription.unsubscribe();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should exclude chromium loader when ABE is disabled but on Windows Desktop", async () => {
|
it("should exclude chromium loader when ABE is disabled and on Windows Desktop", async () => {
|
||||||
environment.getDevice.mockReturnValue(DeviceType.WindowsDesktop);
|
environment.getDevice.mockReturnValue(DeviceType.WindowsDesktop);
|
||||||
const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
|
const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
|
||||||
featureFlagSubject.next(false);
|
featureFlagSubject.next(false);
|
||||||
@@ -146,10 +168,12 @@ describe("ImportMetadataService", () => {
|
|||||||
expect(result.loaders).toContain(Loader.file);
|
expect(result.loaders).toContain(Loader.file);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should exclude chromium loader when ABE is enabled but not on Windows Desktop", async () => {
|
it("should exclude chromium loader when ABE is disabled and getDevice throws error", async () => {
|
||||||
environment.getDevice.mockReturnValue(DeviceType.MacOsDesktop);
|
environment.getDevice.mockImplementation(() => {
|
||||||
const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
|
throw new Error("Device detection failed");
|
||||||
featureFlagSubject.next(true);
|
});
|
||||||
|
const testType: ImportType = "bravecsv";
|
||||||
|
featureFlagSubject.next(false);
|
||||||
|
|
||||||
const metadataPromise = firstValueFrom(sut.metadata$(typeSubject));
|
const metadataPromise = firstValueFrom(sut.metadata$(typeSubject));
|
||||||
typeSubject.next(testType);
|
typeSubject.next(testType);
|
||||||
@@ -160,17 +184,22 @@ describe("ImportMetadataService", () => {
|
|||||||
expect(result.loaders).toContain(Loader.file);
|
expect(result.loaders).toContain(Loader.file);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should include chromium loader when ABE is enabled and on Windows Desktop", async () => {
|
it("should include chromium loader when ABE is disabled and not on Windows Desktop", async () => {
|
||||||
// Set up importers to include bravecsv with chromium loader
|
environment.getDevice.mockReturnValue(DeviceType.MacOsDesktop);
|
||||||
sut["importers"] = {
|
const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
|
||||||
bravecsv: {
|
featureFlagSubject.next(false);
|
||||||
type: "bravecsv",
|
|
||||||
loaders: [Loader.file, Loader.chromium],
|
|
||||||
instructions: Instructions.chromium,
|
|
||||||
},
|
|
||||||
} as ImportersMetadata;
|
|
||||||
|
|
||||||
environment.getDevice.mockReturnValue(DeviceType.WindowsDesktop);
|
const metadataPromise = firstValueFrom(sut.metadata$(typeSubject));
|
||||||
|
typeSubject.next(testType);
|
||||||
|
|
||||||
|
const result = await metadataPromise;
|
||||||
|
|
||||||
|
expect(result.loaders).toContain(Loader.chromium);
|
||||||
|
expect(result.loaders).toContain(Loader.file);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should include chromium loader when ABE is enabled regardless of device", async () => {
|
||||||
|
environment.getDevice.mockReturnValue(DeviceType.MacOsDesktop);
|
||||||
const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
|
const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders
|
||||||
featureFlagSubject.next(true);
|
featureFlagSubject.next(true);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user