From 6043dd1c81d607f2a3c98c390277e1dc5e825fb5 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 14 Nov 2026 11:40:48 -0800 Subject: [PATCH] Be more careful about how we handle errors wrt open docs --- .../services/open-documents.service.spec.ts | 58 ++++++++++++++++++- .../app/services/open-documents.service.ts | 8 ++- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src-ui/src/app/services/open-documents.service.spec.ts b/src-ui/src/app/services/open-documents.service.spec.ts index a8df98f8ca..7c21080f5d 100644 --- a/src-ui/src/app/services/open-documents.service.spec.ts +++ b/src-ui/src/app/services/open-documents.service.spec.ts @@ -240,7 +240,7 @@ describe('OpenDocumentsService', () => { expect(refreshed.tags).toEqual([4]) }) - it('should handle error on refresh documents', () => { + it('keeps documents open after a network error during refresh', () => { subscriptions.push( openDocumentsService.openDocument(documents[1]).subscribe() ) @@ -250,7 +250,63 @@ describe('OpenDocumentsService', () => { ) expect(req.request.method).toEqual('GET') req.error(new ErrorEvent('timeout')) + expect(openDocumentsService.getOpenDocument(documents[1].id)).toBe( + documents[1] + ) + }) + + it.each([500, 403])( + 'keeps documents open after HTTP %s during refresh', + (status) => { + const doc = { ...documents[0] } + openDocumentsService.openDocument(doc).subscribe() + openDocumentsService.refreshDocument(doc.id) + httpTestingController + .expectOne( + `${environment.apiBaseUrl}documents/${doc.id}/?full_perms=true` + ) + .flush({}, { status, statusText: 'Error' }) + expect(openDocumentsService.getOpenDocument(doc.id)).toBe(doc) + } + ) + + it('preserves unsaved edits when a refreshed document no longer exists', () => { + const doc = { ...documents[0], title: 'Unsaved title' } + openDocumentsService.openDocument(doc).subscribe() + openDocumentsService.setDirty(doc, true, { title: doc.title }) + openDocumentsService.refreshDocument(doc.id) + httpTestingController + .expectOne( + `${environment.apiBaseUrl}documents/${doc.id}/?full_perms=true` + ) + .flush({}, { status: 404, statusText: 'Not Found' }) + expect(openDocumentsService.getOpenDocument(doc.id)).toBe(doc) + expect(openDocumentsService.isDirty(doc)).toBe(true) + expect( + JSON.parse(sessionStorage.getItem(OPEN_DOCUMENT_SERVICE.DOCUMENTS))[0] + .title + ).toBe('Unsaved title') + }) + + it('removes only the missing document when tab positions change during refresh', () => { + const first = { ...documents[0] } + const second = { ...documents[1] } + openDocumentsService.openDocument(first).subscribe() + openDocumentsService.openDocument(second).subscribe() + openDocumentsService.refreshDocument(first.id) + openDocumentsService.refreshDocument(second.id) + const firstRequest = httpTestingController.expectOne( + `${environment.apiBaseUrl}documents/${first.id}/?full_perms=true` + ) + const secondRequest = httpTestingController.expectOne( + `${environment.apiBaseUrl}documents/${second.id}/?full_perms=true` + ) + openDocumentsService.closeDocument(second).subscribe() + firstRequest.flush({}, { status: 404, statusText: 'Not Found' }) expect(openDocumentsService.getOpenDocuments()).toHaveLength(0) + openDocumentsService.openDocument(first).subscribe() + secondRequest.flush({}, { status: 404, statusText: 'Not Found' }) + expect(openDocumentsService.getOpenDocuments()).toEqual([first]) }) it('should log error on sessionStorage save', () => { diff --git a/src-ui/src/app/services/open-documents.service.ts b/src-ui/src/app/services/open-documents.service.ts index 288343c073..e0683ef55a 100644 --- a/src-ui/src/app/services/open-documents.service.ts +++ b/src-ui/src/app/services/open-documents.service.ts @@ -1,3 +1,4 @@ +import { HttpErrorResponse } from '@angular/common/http' import { Injectable, inject, signal } from '@angular/core' import { NgbModal } from '@ng-bootstrap/ng-bootstrap' import { Observable, Subject, of } from 'rxjs' @@ -61,7 +62,12 @@ export class OpenDocumentsService { Object.assign(openDoc, doc, unsavedEdits) this.save() }, - error: () => { + error: (error: HttpErrorResponse) => { + if (error.status !== 404) return + + const index = this.openDocuments.findIndex((doc) => doc.id === id) + if (index === -1 || this.isDirty(this.openDocuments[index])) return + this.openDocuments.splice(index, 1) this.save() },