1
0
mirror of https://github.com/bitwarden/browser synced 2026-01-21 11:53:34 +00:00

[PM-30287] Archive deletion navigation (#18213)

* add `routeAfterDeletion` for edit screen to redirect the user to the correct location after deleting an archived cipher

* use `historyGo` to preserve the back invocations

* fix duplicate import
This commit is contained in:
Nick Krantz
2026-01-15 15:24:32 -06:00
committed by GitHub
parent 320fe1f1c9
commit 7150283d7c
5 changed files with 173 additions and 7 deletions

View File

@@ -1,5 +1,7 @@
import { Location } from "@angular/common";
import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing";
import { By } from "@angular/platform-browser";
import { provideNoopAnimations } from "@angular/platform-browser/animations";
import { ActivatedRoute, Router } from "@angular/router";
import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject, of } from "rxjs";
@@ -59,6 +61,8 @@ describe("AddEditV2Component", () => {
const back = jest.fn().mockResolvedValue(null);
const setHistory = jest.fn();
const collect = jest.fn().mockResolvedValue(null);
const history$ = jest.fn();
const historyGo = jest.fn().mockResolvedValue(null);
const openSimpleDialog = jest.fn().mockResolvedValue(true);
const cipherArchiveService = mock<CipherArchiveService>();
@@ -68,6 +72,8 @@ describe("AddEditV2Component", () => {
navigate.mockClear();
back.mockClear();
collect.mockClear();
history$.mockClear();
historyGo.mockClear();
openSimpleDialog.mockClear();
cipherArchiveService.hasArchiveFlagEnabled$ = of(true);
@@ -81,11 +87,13 @@ describe("AddEditV2Component", () => {
await TestBed.configureTestingModule({
imports: [AddEditV2Component],
providers: [
provideNoopAnimations(),
{ provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },
{ provide: ConfigService, useValue: mock<ConfigService>() },
{ provide: PopupRouterCacheService, useValue: { back, setHistory } },
{ provide: PopupRouterCacheService, useValue: { back, setHistory, history$ } },
{ provide: PopupCloseWarningService, useValue: { disable } },
{ provide: Router, useValue: { navigate } },
{ provide: Location, useValue: { historyGo } },
{ provide: ActivatedRoute, useValue: { queryParams: queryParams$ } },
{ provide: I18nService, useValue: { t: (key: string) => key } },
{ provide: CipherService, useValue: cipherServiceMock },
@@ -558,12 +566,104 @@ describe("AddEditV2Component", () => {
expect(deleteCipherSpy).toHaveBeenCalled();
});
it("navigates to vault tab after deletion", async () => {
it("navigates to vault tab after deletion by default", async () => {
jest.spyOn(component["dialogService"], "openSimpleDialog").mockResolvedValue(true);
await component.delete();
expect(navigate).toHaveBeenCalledWith(["/tabs/vault"]);
});
it("navigates to custom route when not in history", fakeAsync(() => {
buildConfigResponse.originalCipher = { edit: true, id: "123" } as Cipher;
queryParams$.next({
cipherId: "123",
routeAfterDeletion: "/archive",
});
tick();
// Mock history without the target route
history$.mockReturnValue(
of([
{ url: "/tabs/vault" },
{ url: "/view-cipher?cipherId=123" },
{ url: "/add-edit?cipherId=123" },
]),
);
jest.spyOn(component["dialogService"], "openSimpleDialog").mockResolvedValue(true);
void component.delete();
tick();
expect(history$).toHaveBeenCalled();
expect(historyGo).not.toHaveBeenCalled();
expect(navigate).toHaveBeenCalledWith(["/archive"]);
}));
it("uses historyGo when custom route exists in history", fakeAsync(() => {
buildConfigResponse.originalCipher = { edit: true, id: "123" } as Cipher;
queryParams$.next({
cipherId: "123",
routeAfterDeletion: "/archive",
});
tick();
history$.mockReturnValue(
of([
{ url: "/tabs/vault" },
{ url: "/archive" },
{ url: "/view-cipher?cipherId=123" },
{ url: "/add-edit?cipherId=123" },
]),
);
jest.spyOn(component["dialogService"], "openSimpleDialog").mockResolvedValue(true);
void component.delete();
tick();
expect(history$).toHaveBeenCalled();
expect(historyGo).toHaveBeenCalledWith(-2);
expect(navigate).not.toHaveBeenCalled();
}));
it("uses router.navigate for default /tabs/vault route", fakeAsync(() => {
buildConfigResponse.originalCipher = { edit: true, id: "456" } as Cipher;
component.routeAfterDeletion = "/tabs/vault";
queryParams$.next({
cipherId: "456",
});
tick();
jest.spyOn(component["dialogService"], "openSimpleDialog").mockResolvedValue(true);
void component.delete();
tick();
expect(history$).not.toHaveBeenCalled();
expect(historyGo).not.toHaveBeenCalled();
expect(navigate).toHaveBeenCalledWith(["/tabs/vault"]);
}));
it("ignores invalid routeAfterDeletion query param and uses default route", fakeAsync(() => {
// Reset the component's routeAfterDeletion to default before this test
component.routeAfterDeletion = "/tabs/vault";
buildConfigResponse.originalCipher = { edit: true, id: "456" } as Cipher;
queryParams$.next({
cipherId: "456",
routeAfterDeletion: "/invalid/route",
});
tick();
// The invalid route should be ignored, routeAfterDeletion should remain default
expect(component.routeAfterDeletion).toBe("/tabs/vault");
}));
});
describe("reloadAddEditCipherData", () => {

View File

@@ -1,6 +1,6 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { CommonModule } from "@angular/common";
import { CommonModule, Location } from "@angular/common";
import { Component, OnInit, OnDestroy, viewChild } from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { FormsModule } from "@angular/forms";
@@ -64,6 +64,18 @@ import {
import { VaultPopoutType } from "../../../utils/vault-popout-window";
import { OpenAttachmentsComponent } from "../attachments/open-attachments/open-attachments.component";
/**
* Available routes to navigate to after editing a cipher.
* Useful when the user could be coming from a different view other than the main vault (e.g., archive).
*/
export const ROUTES_AFTER_EDIT_DELETION = Object.freeze({
tabsVault: "/tabs/vault",
archive: "/archive",
} as const);
export type ROUTES_AFTER_EDIT_DELETION =
(typeof ROUTES_AFTER_EDIT_DELETION)[keyof typeof ROUTES_AFTER_EDIT_DELETION];
/**
* Helper class to parse query parameters for the AddEdit route.
*/
@@ -79,6 +91,7 @@ class QueryParams {
this.username = params.username;
this.name = params.name;
this.prefillNameAndURIFromTab = params.prefillNameAndURIFromTab;
this.routeAfterDeletion = params.routeAfterDeletion ?? ROUTES_AFTER_EDIT_DELETION.tabsVault;
}
/**
@@ -131,6 +144,12 @@ class QueryParams {
* NOTE: This will override the `uri` and `name` query parameters if set to true.
*/
prefillNameAndURIFromTab?: true;
/**
* The view that will be navigated to after deleting the cipher.
* @default "/tabs/vault"
*/
routeAfterDeletion?: ROUTES_AFTER_EDIT_DELETION;
}
export type AddEditQueryParams = Partial<Record<keyof QueryParams, string>>;
@@ -168,6 +187,7 @@ export class AddEditV2Component implements OnInit, OnDestroy {
headerText: string;
config: CipherFormConfig;
canDeleteCipher$: Observable<boolean>;
routeAfterDeletion: ROUTES_AFTER_EDIT_DELETION = "/tabs/vault";
get loading() {
return this.config == null;
@@ -221,6 +241,7 @@ export class AddEditV2Component implements OnInit, OnDestroy {
private dialogService: DialogService,
protected cipherAuthorizationService: CipherAuthorizationService,
private accountService: AccountService,
private location: Location,
private archiveService: CipherArchiveService,
private archiveCipherUtilsService: ArchiveCipherUtilitiesService,
) {
@@ -407,6 +428,13 @@ export class AddEditV2Component implements OnInit, OnDestroy {
);
}
if (
params.routeAfterDeletion &&
Object.values(ROUTES_AFTER_EDIT_DELETION).includes(params.routeAfterDeletion)
) {
this.routeAfterDeletion = params.routeAfterDeletion;
}
return config;
}),
)
@@ -514,7 +542,21 @@ export class AddEditV2Component implements OnInit, OnDestroy {
return false;
}
await this.router.navigate(["/tabs/vault"]);
if (this.routeAfterDeletion !== ROUTES_AFTER_EDIT_DELETION.tabsVault) {
const history = await firstValueFrom(this.popupRouterCacheService.history$());
const targetIndex = history.map((h) => h.url).lastIndexOf(this.routeAfterDeletion);
if (targetIndex !== -1) {
const stepsBack = targetIndex - (history.length - 1);
// Use historyGo to navigate back to the target route in history
// This allows downstream calls to `back()` to continue working as expected
await this.location.historyGo(stepsBack);
} else {
await this.router.navigate([this.routeAfterDeletion]);
}
} else {
await this.router.navigate([this.routeAfterDeletion]);
}
this.toastService.showToast({
variant: "success",

View File

@@ -61,6 +61,7 @@ import { BrowserPremiumUpgradePromptService } from "../../../services/browser-pr
import { BrowserViewPasswordHistoryService } from "../../../services/browser-view-password-history.service";
import { VaultPopupScrollPositionService } from "../../../services/vault-popup-scroll-position.service";
import { closeViewVaultItemPopout, VaultPopoutType } from "../../../utils/vault-popout-window";
import { ROUTES_AFTER_EDIT_DELETION } from "../add-edit/add-edit-v2.component";
import { PopupFooterComponent } from "./../../../../../platform/popup/layout/popup-footer.component";
import { PopupHeaderComponent } from "./../../../../../platform/popup/layout/popup-header.component";
@@ -116,6 +117,7 @@ export class ViewV2Component {
collections$: Observable<CollectionView[]>;
loadAction: LoadAction;
senderTabId?: number;
routeAfterDeletion?: ROUTES_AFTER_EDIT_DELETION;
protected showFooter$: Observable<boolean>;
protected userCanArchive$ = this.accountService.activeAccount$
@@ -151,6 +153,9 @@ export class ViewV2Component {
switchMap(async (params) => {
this.loadAction = params.action;
this.senderTabId = params.senderTabId ? parseInt(params.senderTabId, 10) : undefined;
this.routeAfterDeletion = params.routeAfterDeletion
? params.routeAfterDeletion
: undefined;
this.activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(getUserId),
@@ -230,7 +235,12 @@ export class ViewV2Component {
return false;
}
void this.router.navigate(["/edit-cipher"], {
queryParams: { cipherId: this.cipher.id, type: this.cipher.type, isNew: false },
queryParams: {
cipherId: this.cipher.id,
type: this.cipher.type,
isNew: false,
routeAfterDeletion: this.routeAfterDeletion,
},
});
return true;
}

View File

@@ -18,6 +18,11 @@ import { PasswordRepromptService } from "@bitwarden/vault";
import { ArchiveComponent } from "./archive.component";
// 'qrcode-parser' is used by `BrowserTotpCaptureService` but is an es6 module that jest can't compile.
// Mock the entire module here to prevent jest from throwing an error. I wasn't able to find a way to mock the
// `BrowserTotpCaptureService` where jest would not load the file in the first place.
jest.mock("qrcode-parser", () => {});
describe("ArchiveComponent", () => {
let component: ArchiveComponent;

View File

@@ -39,6 +39,7 @@ import {
import { PopOutComponent } from "../../../platform/popup/components/pop-out.component";
import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component";
import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component";
import { ROUTES_AFTER_EDIT_DELETION } from "../components/vault-v2/add-edit/add-edit-v2.component";
// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
@@ -120,7 +121,11 @@ export class ArchiveComponent {
}
await this.router.navigate(["/view-cipher"], {
queryParams: { cipherId: cipher.id, type: cipher.type },
queryParams: {
cipherId: cipher.id,
type: cipher.type,
routeAfterDeletion: ROUTES_AFTER_EDIT_DELETION.archive,
},
});
}
@@ -130,7 +135,11 @@ export class ArchiveComponent {
}
await this.router.navigate(["/edit-cipher"], {
queryParams: { cipherId: cipher.id, type: cipher.type },
queryParams: {
cipherId: cipher.id,
type: cipher.type,
routeAfterDeletion: ROUTES_AFTER_EDIT_DELETION.archive,
},
});
}