From 775e32bf3b8a7dacbe2d9dd0b0fd2d4959f3edfd Mon Sep 17 00:00:00 2001
From: shamoon <4887959+shamoon@users.noreply.github.com>
Date: Tue, 10 Feb 2026 17:01:47 -0800
Subject: [PATCH] Fix(dev): history causing infinite requests (#12059)
---
.../document-detail.component.ts | 2 +-
.../document-history.component.html | 2 +-
.../document-history.component.scss | 0
.../document-history.component.spec.ts | 39 ++++--
.../document-history.component.ts | 114 ++++++++++++++++++
.../document-history.component.ts | 85 -------------
6 files changed, 147 insertions(+), 95 deletions(-)
rename src-ui/src/app/components/{ => document-detail}/document-history/document-history.component.html (97%)
rename src-ui/src/app/components/{ => document-detail}/document-history/document-history.component.scss (100%)
rename src-ui/src/app/components/{ => document-detail}/document-history/document-history.component.spec.ts (81%)
create mode 100644 src-ui/src/app/components/document-detail/document-history/document-history.component.ts
delete mode 100644 src-ui/src/app/components/document-history/document-history.component.ts
diff --git a/src-ui/src/app/components/document-detail/document-detail.component.ts b/src-ui/src/app/components/document-detail/document-detail.component.ts
index 349d3d199..d11e11bca 100644
--- a/src-ui/src/app/components/document-detail/document-detail.component.ts
+++ b/src-ui/src/app/components/document-detail/document-detail.component.ts
@@ -116,9 +116,9 @@ import {
} from '../common/pdf-viewer/pdf-viewer.types'
import { ShareLinksDialogComponent } from '../common/share-links-dialog/share-links-dialog.component'
import { SuggestionsDropdownComponent } from '../common/suggestions-dropdown/suggestions-dropdown.component'
-import { DocumentHistoryComponent } from '../document-history/document-history.component'
import { DocumentNotesComponent } from '../document-notes/document-notes.component'
import { ComponentWithPermissions } from '../with-permissions/with-permissions.component'
+import { DocumentHistoryComponent } from './document-history/document-history.component'
import { MetadataCollapseComponent } from './metadata-collapse/metadata-collapse.component'
enum DocumentDetailNavIDs {
diff --git a/src-ui/src/app/components/document-history/document-history.component.html b/src-ui/src/app/components/document-detail/document-history/document-history.component.html
similarity index 97%
rename from src-ui/src/app/components/document-history/document-history.component.html
rename to src-ui/src/app/components/document-detail/document-history/document-history.component.html
index edb045323..4defa96fd 100644
--- a/src-ui/src/app/components/document-history/document-history.component.html
+++ b/src-ui/src/app/components/document-detail/document-history/document-history.component.html
@@ -1,6 +1,6 @@
@if (loading) {
} @else {
diff --git a/src-ui/src/app/components/document-history/document-history.component.scss b/src-ui/src/app/components/document-detail/document-history/document-history.component.scss
similarity index 100%
rename from src-ui/src/app/components/document-history/document-history.component.scss
rename to src-ui/src/app/components/document-detail/document-history/document-history.component.scss
diff --git a/src-ui/src/app/components/document-history/document-history.component.spec.ts b/src-ui/src/app/components/document-detail/document-history/document-history.component.spec.ts
similarity index 81%
rename from src-ui/src/app/components/document-history/document-history.component.spec.ts
rename to src-ui/src/app/components/document-detail/document-history/document-history.component.spec.ts
index 68b037b02..c1845c8e4 100644
--- a/src-ui/src/app/components/document-history/document-history.component.spec.ts
+++ b/src-ui/src/app/components/document-detail/document-history/document-history.component.spec.ts
@@ -83,8 +83,22 @@ describe('DocumentHistoryComponent', () => {
expect(result).toBe(correspondentName)
})
expect(getCachedSpy).toHaveBeenCalledWith(parseInt(correspondentId))
- // no correspondent found
- getCachedSpy.mockReturnValue(of(null))
+ })
+
+ it('getPrettyName should memoize results to avoid resubscribe loops', () => {
+ const correspondentId = '1'
+ const getCachedSpy = jest
+ .spyOn(correspondentService, 'getCached')
+ .mockReturnValue(of({ name: 'John Doe' }))
+ const a = component.getPrettyName(DataType.Correspondent, correspondentId)
+ const b = component.getPrettyName(DataType.Correspondent, correspondentId)
+ expect(a).toBe(b)
+ expect(getCachedSpy).toHaveBeenCalledTimes(1)
+ })
+
+ it('getPrettyName should fall back to the correspondent id when missing', () => {
+ const correspondentId = '1'
+ jest.spyOn(correspondentService, 'getCached').mockReturnValue(of(null))
component
.getPrettyName(DataType.Correspondent, correspondentId)
.subscribe((result) => {
@@ -104,8 +118,11 @@ describe('DocumentHistoryComponent', () => {
expect(result).toBe(documentTypeName)
})
expect(getCachedSpy).toHaveBeenCalledWith(parseInt(documentTypeId))
- // no document type found
- getCachedSpy.mockReturnValue(of(null))
+ })
+
+ it('getPrettyName should fall back to the document type id when missing', () => {
+ const documentTypeId = '1'
+ jest.spyOn(documentTypeService, 'getCached').mockReturnValue(of(null))
component
.getPrettyName(DataType.DocumentType, documentTypeId)
.subscribe((result) => {
@@ -125,8 +142,11 @@ describe('DocumentHistoryComponent', () => {
expect(result).toBe(storagePath)
})
expect(getCachedSpy).toHaveBeenCalledWith(parseInt(storagePathId))
- // no storage path found
- getCachedSpy.mockReturnValue(of(null))
+ })
+
+ it('getPrettyName should fall back to the storage path id when missing', () => {
+ const storagePathId = '1'
+ jest.spyOn(storagePathService, 'getCached').mockReturnValue(of(null))
component
.getPrettyName(DataType.StoragePath, storagePathId)
.subscribe((result) => {
@@ -144,8 +164,11 @@ describe('DocumentHistoryComponent', () => {
expect(result).toBe(ownerUsername)
})
expect(getCachedSpy).toHaveBeenCalledWith(parseInt(ownerId))
- // no user found
- getCachedSpy.mockReturnValue(of(null))
+ })
+
+ it('getPrettyName should fall back to the owner id when missing', () => {
+ const ownerId = '1'
+ jest.spyOn(userService, 'getCached').mockReturnValue(of(null))
component.getPrettyName('owner', ownerId).subscribe((result) => {
expect(result).toBe(ownerId)
})
diff --git a/src-ui/src/app/components/document-detail/document-history/document-history.component.ts b/src-ui/src/app/components/document-detail/document-history/document-history.component.ts
new file mode 100644
index 000000000..a2bbede3c
--- /dev/null
+++ b/src-ui/src/app/components/document-detail/document-history/document-history.component.ts
@@ -0,0 +1,114 @@
+import { AsyncPipe, KeyValuePipe, TitleCasePipe } from '@angular/common'
+import { Component, Input, OnInit, inject } from '@angular/core'
+import { NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap'
+import { NgxBootstrapIconsModule } from 'ngx-bootstrap-icons'
+import { Observable, first, map, of, shareReplay } from 'rxjs'
+import { AuditLogAction, AuditLogEntry } from 'src/app/data/auditlog-entry'
+import { DataType } from 'src/app/data/datatype'
+import { CustomDatePipe } from 'src/app/pipes/custom-date.pipe'
+import { CorrespondentService } from 'src/app/services/rest/correspondent.service'
+import { DocumentTypeService } from 'src/app/services/rest/document-type.service'
+import { DocumentService } from 'src/app/services/rest/document.service'
+import { StoragePathService } from 'src/app/services/rest/storage-path.service'
+import { UserService } from 'src/app/services/rest/user.service'
+
+@Component({
+ selector: 'pngx-document-history',
+ templateUrl: './document-history.component.html',
+ styleUrl: './document-history.component.scss',
+ imports: [
+ CustomDatePipe,
+ NgbTooltipModule,
+ AsyncPipe,
+ KeyValuePipe,
+ TitleCasePipe,
+ NgxBootstrapIconsModule,
+ ],
+})
+export class DocumentHistoryComponent implements OnInit {
+ private documentService = inject(DocumentService)
+ private correspondentService = inject(CorrespondentService)
+ private storagePathService = inject(StoragePathService)
+ private documentTypeService = inject(DocumentTypeService)
+ private userService = inject(UserService)
+
+ public AuditLogAction = AuditLogAction
+
+ private _documentId: number
+ @Input()
+ set documentId(id: number) {
+ if (this._documentId !== id) {
+ this._documentId = id
+ this.prettyNameCache.clear()
+ this.loadHistory()
+ }
+ }
+
+ public loading: boolean = true
+ public entries: AuditLogEntry[] = []
+
+ private readonly prettyNameCache = new Map>()
+
+ ngOnInit(): void {
+ this.loadHistory()
+ }
+
+ private loadHistory(): void {
+ if (this._documentId) {
+ this.loading = true
+ this.documentService.getHistory(this._documentId).subscribe((entries) => {
+ this.entries = entries
+ this.loading = false
+ })
+ }
+ }
+
+ getPrettyName(type: DataType | string, id: string): Observable {
+ const cacheKey = `${type}:${id}`
+ const cached = this.prettyNameCache.get(cacheKey)
+ if (cached) {
+ return cached
+ }
+
+ const idInt = parseInt(id, 10)
+ const fallback$ = of(id)
+
+ let result$: Observable
+ if (!Number.isFinite(idInt)) {
+ result$ = fallback$
+ } else {
+ switch (type) {
+ case DataType.Correspondent:
+ result$ = this.correspondentService.getCached(idInt).pipe(
+ first(),
+ map((correspondent) => correspondent?.name ?? id)
+ )
+ break
+ case DataType.DocumentType:
+ result$ = this.documentTypeService.getCached(idInt).pipe(
+ first(),
+ map((documentType) => documentType?.name ?? id)
+ )
+ break
+ case DataType.StoragePath:
+ result$ = this.storagePathService.getCached(idInt).pipe(
+ first(),
+ map((storagePath) => storagePath?.path ?? id)
+ )
+ break
+ case 'owner':
+ result$ = this.userService.getCached(idInt).pipe(
+ first(),
+ map((user) => user?.username ?? id)
+ )
+ break
+ default:
+ result$ = fallback$
+ }
+ }
+
+ const shared$ = result$.pipe(shareReplay({ bufferSize: 1, refCount: true }))
+ this.prettyNameCache.set(cacheKey, shared$)
+ return shared$
+ }
+}
diff --git a/src-ui/src/app/components/document-history/document-history.component.ts b/src-ui/src/app/components/document-history/document-history.component.ts
deleted file mode 100644
index d57db1056..000000000
--- a/src-ui/src/app/components/document-history/document-history.component.ts
+++ /dev/null
@@ -1,85 +0,0 @@
-import { AsyncPipe, KeyValuePipe, TitleCasePipe } from '@angular/common'
-import { Component, Input, OnInit, inject } from '@angular/core'
-import { NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap'
-import { NgxBootstrapIconsModule } from 'ngx-bootstrap-icons'
-import { Observable, first, map, of } from 'rxjs'
-import { AuditLogAction, AuditLogEntry } from 'src/app/data/auditlog-entry'
-import { DataType } from 'src/app/data/datatype'
-import { CustomDatePipe } from 'src/app/pipes/custom-date.pipe'
-import { CorrespondentService } from 'src/app/services/rest/correspondent.service'
-import { DocumentTypeService } from 'src/app/services/rest/document-type.service'
-import { DocumentService } from 'src/app/services/rest/document.service'
-import { StoragePathService } from 'src/app/services/rest/storage-path.service'
-import { UserService } from 'src/app/services/rest/user.service'
-
-@Component({
- selector: 'pngx-document-history',
- templateUrl: './document-history.component.html',
- styleUrl: './document-history.component.scss',
- imports: [
- CustomDatePipe,
- NgbTooltipModule,
- AsyncPipe,
- KeyValuePipe,
- TitleCasePipe,
- NgxBootstrapIconsModule,
- ],
-})
-export class DocumentHistoryComponent implements OnInit {
- private documentService = inject(DocumentService)
- private correspondentService = inject(CorrespondentService)
- private storagePathService = inject(StoragePathService)
- private documentTypeService = inject(DocumentTypeService)
- private userService = inject(UserService)
-
- public AuditLogAction = AuditLogAction
-
- private _documentId: number
- @Input()
- set documentId(id: number) {
- this._documentId = id
- this.ngOnInit()
- }
-
- public loading: boolean = true
- public entries: AuditLogEntry[] = []
-
- ngOnInit(): void {
- if (this._documentId) {
- this.loading = true
- this.documentService
- .getHistory(this._documentId)
- .subscribe((auditLogEntries) => {
- this.entries = auditLogEntries
- this.loading = false
- })
- }
- }
-
- getPrettyName(type: DataType | string, id: string): Observable {
- switch (type) {
- case DataType.Correspondent:
- return this.correspondentService.getCached(parseInt(id, 10)).pipe(
- first(),
- map((correspondent) => correspondent?.name ?? id)
- )
- case DataType.DocumentType:
- return this.documentTypeService.getCached(parseInt(id, 10)).pipe(
- first(),
- map((documentType) => documentType?.name ?? id)
- )
- case DataType.StoragePath:
- return this.storagePathService.getCached(parseInt(id, 10)).pipe(
- first(),
- map((storagePath) => storagePath?.path ?? id)
- )
- case 'owner':
- return this.userService.getCached(parseInt(id, 10)).pipe(
- first(),
- map((user) => user?.username ?? id)
- )
- default:
- return of(id)
- }
- }
-}