Improved error notifications

This commit is contained in:
shamoon
2023-08-23 23:50:54 -07:00
parent e6ca6b6f7a
commit cca465aa00
14 changed files with 296 additions and 260 deletions

View File

@@ -5,9 +5,26 @@
(hidden)="toastService.closeToast(toast)">
<p>{{toast.content}}</p>
<details *ngIf="toast.error">
<pre class="p-2 m-0 bg-light text-dark">
{{toast.error}}
</pre>
<div class="p-3">
<dl class="row" *ngIf="isDetailedError(toast.error)">
<dt class="col-sm-3 fw-normal text-end">URL</dt>
<dd class="col-sm-9">{{ toast.error.url }}</dd>
<dt class="col-sm-3 fw-normal text-end" i18n>Status</dt>
<dd class="col-sm-9">{{ toast.error.status }} <em>{{ toast.error.statusText }}</em></dd>
<dt class="col-sm-3 fw-normal text-end" i18n>Error</dt>
<dd class="col-sm-9">{{ getErrorText(toast.error) }}</dd>
</dl>
<div class="row">
<div class="col offset-sm-3">
<button class="btn btn-sm btn-outline-dark" (click)="copyError(toast.error)">
<svg class="sidebaricon" fill="currentColor">
<use *ngIf="!copied" xlink:href="assets/bootstrap-icons.svg#clipboard" />
<use *ngIf="copied" xlink:href="assets/bootstrap-icons.svg#clipboard-check" />
</svg>&nbsp;<ng-container i18n>Copy Raw Error</ng-container>
</button>
</div>
</div>
</div>
</details>
<p class="mb-0" *ngIf="toast.action"><button class="btn btn-sm btn-outline-secondary" (click)="toastService.closeToast(toast); toast.action()">{{toast.actionName}}</button></p>
</ngb-toast>

View File

@@ -20,8 +20,3 @@
border-bottom-left-radius: inherit;
border-bottom-right-radius: inherit;
}
pre {
white-space: pre-line;
--bs-bg-opacity: .25;
}

View File

@@ -11,6 +11,32 @@ import { HttpClientTestingModule } from '@angular/common/http/testing'
import { of } from 'rxjs'
import { NgbModule } from '@ng-bootstrap/ng-bootstrap'
const toasts = [
{
title: 'Title',
content: 'content',
delay: 5000,
},
{
title: 'Error 1',
content: 'Error 1 content',
delay: 5000,
error: 'Error 1 string',
},
{
title: 'Error 2',
content: 'Error 2 content',
delay: 5000,
error: {
url: 'https://example.com',
status: 500,
statusText: 'Internal Server Error',
message: 'Internal server error 500 message',
error: { detail: 'Error 2 message details' },
},
},
]
describe('ToastsComponent', () => {
let component: ToastsComponent
let fixture: ComponentFixture<ToastsComponent>
@@ -24,20 +50,7 @@ describe('ToastsComponent', () => {
{
provide: ToastService,
useValue: {
getToasts: () =>
of([
{
title: 'Title',
content: 'content',
delay: 5000,
},
{
title: 'Error',
content: 'Error content',
delay: 5000,
error: new Error('Error message'),
},
]),
getToasts: () => of(toasts),
},
},
],
@@ -85,10 +98,41 @@ describe('ToastsComponent', () => {
fixture.detectChanges()
expect(fixture.nativeElement.querySelector('details')).not.toBeNull()
expect(fixture.nativeElement.textContent).toContain('Error message')
expect(fixture.nativeElement.textContent).toContain('Error 1 content')
component.ngOnDestroy()
flush()
discardPeriodicTasks()
}))
it('should show error details, support copy', fakeAsync(() => {
component.ngOnInit()
fixture.detectChanges()
expect(fixture.nativeElement.querySelector('details')).not.toBeNull()
expect(fixture.nativeElement.textContent).toContain(
'Error 2 message details'
)
const copySpy = jest.spyOn(navigator.clipboard, 'writeText')
component.copyError(toasts[2].error)
expect(copySpy).toHaveBeenCalled()
component.ngOnDestroy()
flush()
discardPeriodicTasks()
}))
it('should parse error text, add ellipsis', () => {
expect(component.getErrorText(toasts[2].error)).toEqual(
'Error 2 message details'
)
expect(component.getErrorText({ error: 'Error string no detail' })).toEqual(
'Error string no detail'
)
expect(component.getErrorText('Error string')).toEqual('')
expect(
component.getErrorText({ error: new Array(205).join('a') })
).toContain('...')
})
})

View File

@@ -10,17 +10,50 @@ import { Toast, ToastService } from 'src/app/services/toast.service'
export class ToastsComponent implements OnInit, OnDestroy {
constructor(private toastService: ToastService) {}
subscription: Subscription
private subscription: Subscription
toasts: Toast[] = []
public toasts: Toast[] = []
public copied: boolean = false
ngOnDestroy(): void {
this.subscription?.unsubscribe()
}
ngOnInit(): void {
this.subscription = this.toastService
.getToasts()
.subscribe((toasts) => (this.toasts = toasts))
this.subscription = this.toastService.getToasts().subscribe((toasts) => {
this.toasts = toasts
this.toasts.forEach((t) => {
if (typeof t.error === 'string') {
try {
t.error = JSON.parse(t.error)
} catch (e) {}
}
})
})
}
public isDetailedError(error: any): boolean {
return (
typeof error === 'object' &&
'status' in error &&
'statusText' in error &&
'url' in error &&
'message' in error &&
'error' in error
)
}
public copyError(error: any) {
navigator.clipboard.writeText(JSON.stringify(error))
this.copied = true
setTimeout(() => {
this.copied = false
}, 3000)
}
getErrorText(error: any) {
const text: string = error.error?.detail ?? error.error ?? ''
return `${text.slice(0, 200)}${text.length > 200 ? '...' : ''}`
}
}

View File

@@ -398,15 +398,12 @@ describe('DocumentDetailComponent', () => {
const closeSpy = jest.spyOn(component, 'close')
const updateSpy = jest.spyOn(documentService, 'update')
const toastSpy = jest.spyOn(toastService, 'showError')
updateSpy.mockImplementation(() =>
throwError(() => new Error('failed to save'))
)
const error = new Error('failed to save')
updateSpy.mockImplementation(() => throwError(() => error))
component.save()
expect(updateSpy).toHaveBeenCalled()
expect(closeSpy).not.toHaveBeenCalled()
expect(toastSpy).toHaveBeenCalledWith(
'Error saving document: failed to save'
)
expect(toastSpy).toHaveBeenCalledWith('Error saving document', error)
})
it('should show error toast on save but close if user can no longer edit', () => {
@@ -450,15 +447,12 @@ describe('DocumentDetailComponent', () => {
const closeSpy = jest.spyOn(component, 'close')
const updateSpy = jest.spyOn(documentService, 'update')
const toastSpy = jest.spyOn(toastService, 'showError')
updateSpy.mockImplementation(() =>
throwError(() => new Error('failed to save'))
)
const error = new Error('failed to save')
updateSpy.mockImplementation(() => throwError(() => error))
component.saveEditNext()
expect(updateSpy).toHaveBeenCalled()
expect(closeSpy).not.toHaveBeenCalled()
expect(toastSpy).toHaveBeenCalledWith(
'Error saving document: failed to save'
)
expect(toastSpy).toHaveBeenCalledWith('Error saving document', error)
})
it('should show save button and save & close or save & next', () => {
@@ -798,11 +792,7 @@ describe('DocumentDetailComponent', () => {
.mockReturnValue(throwError(() => error))
const toastSpy = jest.spyOn(toastService, 'showError')
initNormally()
expect(toastSpy).toHaveBeenCalledWith(
'Error retrieving metadata',
10000,
error
)
expect(toastSpy).toHaveBeenCalledWith('Error retrieving metadata', error)
})
function initNormally() {

View File

@@ -395,7 +395,6 @@ export class DocumentDetailComponent
this.metadata = null
this.toastService.showError(
$localize`Error retrieving metadata`,
10000,
error
)
},
@@ -417,7 +416,6 @@ export class DocumentDetailComponent
this.suggestions = null
this.toastService.showError(
$localize`Error retrieving suggestions.`,
10000,
error
)
},
@@ -542,11 +540,7 @@ export class DocumentDetailComponent
close && this.close()
} else {
this.error = error.error
this.toastService.showError(
$localize`Error saving document` +
': ' +
(error.error?.detail ?? error.message ?? JSON.stringify(error))
)
this.toastService.showError($localize`Error saving document`, error)
}
},
})
@@ -587,11 +581,7 @@ export class DocumentDetailComponent
error: (error) => {
this.networkActive = false
this.error = error.error
this.toastService.showError(
$localize`Error saving document` +
': ' +
(error.error?.detail ?? error.message ?? JSON.stringify(error))
)
this.toastService.showError($localize`Error saving document`, error)
},
})
}
@@ -640,11 +630,7 @@ export class DocumentDetailComponent
this.close()
},
error: (error) => {
this.toastService.showError(
$localize`Error deleting document: ${
error.error?.detail ?? error.message ?? JSON.stringify(error)
}`
)
this.toastService.showError($localize`Error deleting document`, error)
modal.componentInstance.buttonsEnabled = true
this.subscribeModalDelete(modal)
},
@@ -687,9 +673,8 @@ export class DocumentDetailComponent
modal.componentInstance.buttonsEnabled = true
}
this.toastService.showError(
$localize`Error executing operation: ${JSON.stringify(
error.error
)}`
$localize`Error executing operation`,
error
)
},
})

View File

@@ -182,9 +182,8 @@ export class BulkEditorComponent
modal.componentInstance.buttonsEnabled = true
}
this.toastService.showError(
$localize`Error executing bulk operation: ${JSON.stringify(
error.error
)}`
$localize`Error executing bulk operation`,
error
)
},
})

View File

@@ -63,11 +63,7 @@ export class DocumentNotesComponent extends ComponentWithPermissions {
},
error: (e) => {
this.networkActive = false
this.toastService.showError(
$localize`Error saving note`,
10000,
JSON.stringify(e)
)
this.toastService.showError($localize`Error saving note`, e)
},
})
}
@@ -81,9 +77,7 @@ export class DocumentNotesComponent extends ComponentWithPermissions {
},
error: (e) => {
this.networkActive = false
this.toastService.showError(
$localize`Error deleting note: ${e.toString()}`
)
this.toastService.showError($localize`Error deleting note`, e)
},
})
}

View File

@@ -148,8 +148,7 @@ export abstract class ManagementListComponent<T extends ObjectWithId>
activeModal.componentInstance.failed.subscribe((e) => {
this.toastService.showError(
$localize`Error occurred while creating ${this.typeName}.`,
10000,
JSON.stringify(e)
e
)
})
}
@@ -169,8 +168,7 @@ export abstract class ManagementListComponent<T extends ObjectWithId>
activeModal.componentInstance.failed.subscribe((e) => {
this.toastService.showError(
$localize`Error occurred while saving ${this.typeName}.`,
10000,
JSON.stringify(e)
e
)
})
}
@@ -194,20 +192,19 @@ export abstract class ManagementListComponent<T extends ObjectWithId>
activeModal.componentInstance.btnCaption = $localize`Delete`
activeModal.componentInstance.confirmClicked.subscribe(() => {
activeModal.componentInstance.buttonsEnabled = false
this.service.delete(object).subscribe(
(_) => {
this.service.delete(object).subscribe({
next: () => {
activeModal.close()
this.reloadData()
},
(error) => {
error: (error) => {
activeModal.componentInstance.buttonsEnabled = true
this.toastService.showError(
$localize`Error while deleting element: ${JSON.stringify(
error.error
)}`
$localize`Error while deleting element`,
error
)
}
)
},
})
})
}

View File

@@ -276,18 +276,13 @@ export class SettingsComponent
error: (e) => {
this.toastService.showError(
$localize`Error retrieving groups`,
10000,
JSON.stringify(e)
e
)
},
})
},
error: (e) => {
this.toastService.showError(
$localize`Error retrieving users`,
10000,
JSON.stringify(e)
)
this.toastService.showError($localize`Error retrieving users`, e)
},
})
} else if (
@@ -312,8 +307,7 @@ export class SettingsComponent
error: (e) => {
this.toastService.showError(
$localize`Error retrieving mail rules`,
10000,
JSON.stringify(e)
e
)
},
})
@@ -321,8 +315,7 @@ export class SettingsComponent
error: (e) => {
this.toastService.showError(
$localize`Error retrieving mail accounts`,
10000,
JSON.stringify(e)
e
)
},
})
@@ -646,8 +639,7 @@ export class SettingsComponent
error: (error) => {
this.toastService.showError(
$localize`An error occurred while saving settings.`,
10000,
JSON.stringify(error)
error
)
},
})
@@ -682,8 +674,7 @@ export class SettingsComponent
(error) => {
this.toastService.showError(
$localize`Error while storing settings on server.`,
10000,
JSON.stringify(error)
error
)
}
)
@@ -742,11 +733,7 @@ export class SettingsComponent
modal.componentInstance.failed
.pipe(takeUntil(this.unsubscribeNotifier))
.subscribe((e) => {
this.toastService.showError(
$localize`Error saving user.`,
10000,
JSON.stringify(e)
)
this.toastService.showError($localize`Error saving user.`, e)
})
}
@@ -771,11 +758,7 @@ export class SettingsComponent
})
},
error: (e) => {
this.toastService.showError(
$localize`Error deleting user.`,
10000,
JSON.stringify(e)
)
this.toastService.showError($localize`Error deleting user.`, e)
},
})
})
@@ -802,11 +785,7 @@ export class SettingsComponent
modal.componentInstance.failed
.pipe(takeUntil(this.unsubscribeNotifier))
.subscribe((e) => {
this.toastService.showError(
$localize`Error saving group.`,
10000,
JSON.stringify(e)
)
this.toastService.showError($localize`Error saving group.`, e)
})
}
@@ -831,11 +810,7 @@ export class SettingsComponent
})
},
error: (e) => {
this.toastService.showError(
$localize`Error deleting group.`,
10000,
JSON.stringify(e)
)
this.toastService.showError($localize`Error deleting group.`, e)
},
})
})
@@ -869,11 +844,7 @@ export class SettingsComponent
modal.componentInstance.failed
.pipe(takeUntil(this.unsubscribeNotifier))
.subscribe((e) => {
this.toastService.showError(
$localize`Error saving account.`,
10000,
JSON.stringify(e)
)
this.toastService.showError($localize`Error saving account.`, e)
})
}
@@ -901,8 +872,7 @@ export class SettingsComponent
error: (e) => {
this.toastService.showError(
$localize`Error deleting mail account.`,
10000,
JSON.stringify(e)
e
)
},
})
@@ -932,11 +902,7 @@ export class SettingsComponent
modal.componentInstance.failed
.pipe(takeUntil(this.unsubscribeNotifier))
.subscribe((e) => {
this.toastService.showError(
$localize`Error saving rule.`,
10000,
JSON.stringify(e)
)
this.toastService.showError($localize`Error saving rule.`, e)
})
}
@@ -962,11 +928,7 @@ export class SettingsComponent
})
},
error: (e) => {
this.toastService.showError(
$localize`Error deleting mail rule.`,
10000,
JSON.stringify(e)
)
this.toastService.showError($localize`Error deleting mail rule.`, e)
},
})
})

View File

@@ -32,7 +32,7 @@ export class ToastService {
this.toastsSubject.next(this.toasts)
}
showError(content: string, delay: number = 10000, error: any = null) {
showError(content: string, error: any = null, delay: number = 10000) {
this.show({
title: $localize`Error`,
content: content,

View File

@@ -249,6 +249,10 @@ $form-check-radio-checked-bg-image-dark: url("data:image/svg+xml,<svg xmlns='htt
color: var(--pngx-primary-text-contrast);
}
.toast .btn-outline-dark:hover {
color: var(--bs-body-color)
}
.dropdown-menu {
--bs-dropdown-color: var(--bs-body-color);
}