1
0
mirror of https://github.com/bitwarden/browser.git synced 2025-12-05 09:14:28 +01:00

addressing comments on PR

This commit is contained in:
Isaac Ivins 2025-12-04 15:46:57 -05:00
parent a4c0f2aac7
commit d03f84c153
4 changed files with 42 additions and 31 deletions

View File

@ -1,17 +1,21 @@
import { Component } from "@angular/core";
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { RouterModule } from "@angular/router";
import { mock } from "jest-mock-extended";
import { BehaviorSubject } from "rxjs";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { SendType } from "@bitwarden/common/tools/send/enums/send-type";
import { NavigationModule } from "@bitwarden/components";
import { SendListFiltersService } from "@bitwarden/send-ui";
import { SendFiltersNavComponent } from "../tools/send-v2/send-filters-nav.component";
import { DesktopLayoutComponent } from "./desktop-layout.component";
// Mock the child component to isolate DesktopLayoutComponent testing
// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
@Component({
selector: "app-send-filters-nav",
template: "",
})
class MockSendFiltersNavComponent {}
Object.defineProperty(window, "matchMedia", {
writable: true,
value: jest.fn().mockImplementation((query) => ({
@ -31,21 +35,10 @@ describe("DesktopLayoutComponent", () => {
let fixture: ComponentFixture<DesktopLayoutComponent>;
beforeEach(async () => {
const filterFormValueSubject = new BehaviorSubject<{ sendType: SendType | null }>({
sendType: null,
});
const sendListFiltersService = mock<SendListFiltersService>();
sendListFiltersService.filterForm = {
value: { sendType: null },
valueChanges: filterFormValueSubject.asObservable(),
patchValue: jest.fn(),
} as any;
await TestBed.configureTestingModule({
imports: [
DesktopLayoutComponent,
SendFiltersNavComponent,
MockSendFiltersNavComponent,
RouterModule.forRoot([]),
NavigationModule,
],
@ -54,10 +47,6 @@ describe("DesktopLayoutComponent", () => {
provide: I18nService,
useValue: mock<I18nService>(),
},
{
provide: SendListFiltersService,
useValue: sendListFiltersService,
},
],
}).compileComponents();

View File

@ -11,7 +11,7 @@ import { SendListFiltersService } from "@bitwarden/send-ui";
import { SendFiltersNavComponent } from "./send-filters-nav.component";
// Dummy component for router testing
// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
@Component({ template: "" })
class DummyComponent {}

View File

@ -27,11 +27,11 @@ export class SendFiltersNavComponent {
protected readonly SendType = SendType;
// Inject services at class level
protected readonly filtersService = inject(SendListFiltersService);
private readonly filtersService = inject(SendListFiltersService);
private readonly router = inject(Router);
// Convert filter form to signal for reactive updates
protected readonly currentFilter = toSignal(
private readonly currentFilter = toSignal(
this.filtersService.filterForm.valueChanges.pipe(
startWith(this.filtersService.filterForm.value),
),
@ -39,7 +39,7 @@ export class SendFiltersNavComponent {
);
// Computed: Is send route currently active?
protected isSendRouteActive(): boolean {
private isSendRouteActive(): boolean {
return this.router.url.includes("/new-sends");
}

View File

@ -1,4 +1,8 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { ChangeDetectorRef } from "@angular/core";
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { FormBuilder } from "@angular/forms";
import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject, of } from "rxjs";
@ -15,6 +19,7 @@ import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.s
import { SendService } from "@bitwarden/common/tools/send/services/send.service.abstraction";
import { SearchService } from "@bitwarden/common/vault/abstractions/search.service";
import { DialogService, ToastService } from "@bitwarden/components";
import { SendListFiltersService } from "@bitwarden/send-ui";
import * as utils from "../../../utils";
import { SearchBarService } from "../../layout/search/search-bar.service";
@ -35,6 +40,8 @@ describe("SendV2Component", () => {
let broadcasterService: MockProxy<BroadcasterService>;
let accountService: MockProxy<AccountService>;
let policyService: MockProxy<PolicyService>;
let sendListFiltersService: SendListFiltersService;
let changeDetectorRef: MockProxy<ChangeDetectorRef>;
beforeEach(async () => {
sendService = mock<SendService>();
@ -42,6 +49,13 @@ describe("SendV2Component", () => {
broadcasterService = mock<BroadcasterService>();
accountService = mock<AccountService>();
policyService = mock<PolicyService>();
changeDetectorRef = mock<ChangeDetectorRef>();
// Create real SendListFiltersService with mocked dependencies
const formBuilder = new FormBuilder();
const i18nService = mock<I18nService>();
i18nService.t.mockImplementation((key: string) => key);
sendListFiltersService = new SendListFiltersService(i18nService, formBuilder);
// Mock sendViews$ observable
sendService.sendViews$ = of([]);
@ -51,6 +65,10 @@ describe("SendV2Component", () => {
accountService.activeAccount$ = of({ id: "test-user-id" } as any);
policyService.policyAppliesToUser$ = jest.fn().mockReturnValue(of(false));
// Mock SearchService methods needed by base component
const mockSearchService = mock<SearchService>();
mockSearchService.isSearchable.mockResolvedValue(false);
await TestBed.configureTestingModule({
imports: [SendV2Component],
providers: [
@ -59,7 +77,7 @@ describe("SendV2Component", () => {
{ provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },
{ provide: EnvironmentService, useValue: mock<EnvironmentService>() },
{ provide: BroadcasterService, useValue: broadcasterService },
{ provide: SearchService, useValue: mock<SearchService>() },
{ provide: SearchService, useValue: mockSearchService },
{ provide: PolicyService, useValue: policyService },
{ provide: SearchBarService, useValue: searchBarService },
{ provide: LogService, useValue: mock<LogService>() },
@ -67,6 +85,8 @@ describe("SendV2Component", () => {
{ provide: DialogService, useValue: mock<DialogService>() },
{ provide: ToastService, useValue: mock<ToastService>() },
{ provide: AccountService, useValue: accountService },
{ provide: SendListFiltersService, useValue: sendListFiltersService },
{ provide: ChangeDetectorRef, useValue: changeDetectorRef },
],
}).compileComponents();
@ -331,7 +351,6 @@ describe("SendV2Component", () => {
describe("load", () => {
it("sets loading states correctly", async () => {
jest.spyOn(component, "search").mockResolvedValue();
jest.spyOn(component, "selectAll");
expect(component.loaded).toBeFalsy();
@ -341,14 +360,17 @@ describe("SendV2Component", () => {
expect(component.loaded).toBe(true);
});
it("does not call selectAll when onSuccessfulLoad is not set", async () => {
it("sets up sendViews$ subscription", async () => {
const mockSends = [new SendView(), new SendView()];
sendService.sendViews$ = of(mockSends);
jest.spyOn(component, "search").mockResolvedValue();
jest.spyOn(component, "selectAll");
component.onSuccessfulLoad = null;
await component.load();
expect(component.selectAll).not.toHaveBeenCalled();
// Give observable time to emit
await new Promise((resolve) => setTimeout(resolve, 10));
expect(component.sends).toEqual(mockSends);
});
it("calls onSuccessfulLoad when it is set", async () => {