From 0e9ded941917151770907a78c56db50899e0c25e Mon Sep 17 00:00:00 2001
From: Marcin Siodelski <marcin@isc.org>
Date: Wed, 22 Nov 2023 11:00:16 +0100
Subject: [PATCH] [#1210] Handle settings form errors

---
 .../settings-page.component.html              | 120 ++++++++++--------
 .../settings-page.component.spec.ts           |  16 ++-
 .../settings-page.component.stories.ts        |   4 +-
 .../settings-page/settings-page.component.ts  |  35 ++++-
 4 files changed, 116 insertions(+), 59 deletions(-)

diff --git a/webui/src/app/settings-page/settings-page.component.html b/webui/src/app/settings-page/settings-page.component.html
index f8a5fc4c9..f22230474 100644
--- a/webui/src/app/settings-page/settings-page.component.html
+++ b/webui/src/app/settings-page/settings-page.component.html
@@ -28,61 +28,79 @@
     </div>
 </app-breadcrumbs>
 
-<form [formGroup]="settingsForm">
-    <div class="grid">
-        <div class="col-6">
-            <p-fieldset legend="Intervals">
-                <div *ngFor="let setting of intervalSettings" class="w-full">
-                    <div class="flex align-items-end">
-                        <div class="w-full">
-                            <label class="block mt-2">
-                                {{ setting.title }} (in seconds):
-                                <input type="number" [formControlName]="setting.formControlName" class="w-full" />
-                            </label>
+<ng-container [ngSwitch]="formState">
+    <ng-container *ngSwitchCase="'success'">
+        <form [formGroup]="settingsForm">
+            <div class="grid">
+                <div class="col-6">
+                    <p-fieldset legend="Intervals">
+                        <div *ngFor="let setting of intervalSettings" class="w-full">
+                            <div class="flex align-items-end">
+                                <div class="w-full">
+                                    <label class="block mt-2">
+                                        {{ setting.title }} (in seconds):
+                                        <input
+                                            type="number"
+                                            [formControlName]="setting.formControlName"
+                                            class="w-full"
+                                        />
+                                    </label>
+                                </div>
+                                <div>
+                                    <app-help-tip [title]="setting.title">{{ setting.help }}</app-help-tip>
+                                </div>
+                            </div>
+                            <div *ngIf="hasError(setting.formControlName, 'required')" class="p-error">
+                                It is required.
+                            </div>
+                            <div *ngIf="hasError(setting.formControlName, 'min')" class="p-error">
+                                It must not be negative.
+                            </div>
                         </div>
-                        <div>
-                            <app-help-tip [title]="setting.title">{{ setting.help }}</app-help-tip>
-                        </div>
-                    </div>
-                    <div *ngIf="hasError(setting.formControlName, 'required')" class="p-error">It is required.</div>
-                    <div *ngIf="hasError(setting.formControlName, 'min')" class="p-error">It must not be negative.</div>
+                    </p-fieldset>
                 </div>
-            </p-fieldset>
-        </div>
 
-        <div class="col-6">
-            <p-fieldset legend="Grafana & Prometheus">
-                <div *ngFor="let setting of urlSettings" class="w-full">
-                    <div class="flex align-items-end">
-                        <div class="w-full">
-                            <label class="block mt-2">
-                                {{ setting.title }}
-                                <input
-                                    type="url"
-                                    [formControlName]="setting.formControlName"
-                                    class="w-full"
-                                    id="grafanaUrl"
-                                />
-                            </label>
-                        </div>
-                        <div>
-                            <app-help-tip [title]="setting.title">{{ setting.help }}</app-help-tip>
+                <div class="col-6">
+                    <p-fieldset legend="Grafana & Prometheus">
+                        <div *ngFor="let setting of urlSettings" class="w-full">
+                            <div class="flex align-items-end">
+                                <div class="w-full">
+                                    <label class="block mt-2">
+                                        {{ setting.title }}
+                                        <input
+                                            type="url"
+                                            [formControlName]="setting.formControlName"
+                                            class="w-full"
+                                            id="grafanaUrl"
+                                        />
+                                    </label>
+                                </div>
+                                <div>
+                                    <app-help-tip [title]="setting.title">{{ setting.help }}</app-help-tip>
+                                </div>
+                            </div>
                         </div>
-                    </div>
+                    </p-fieldset>
                 </div>
-            </p-fieldset>
-        </div>
-    </div>
-</form>
+            </div>
+        </form>
 
-<button
-    pButton
-    type="button"
-    [disabled]="settingsForm.invalid"
-    label="Save Settings"
-    id="save-settings-button"
-    (click)="saveSettings()"
-    class="mt-2 ml-1"
-></button>
+        <button
+            pButton
+            type="button"
+            [disabled]="settingsForm.invalid"
+            label="Save Settings"
+            id="save-settings-button"
+            (click)="saveSettings()"
+            class="mt-2 ml-1"
+        ></button>
 
-<span *ngIf="settingsForm.invalid" class="p-error ml-3"> There are issues in the form values. </span>
+        <span *ngIf="settingsForm.invalid" class="p-error ml-3"> There are issues in the form values. </span>
+    </ng-container>
+    <ng-container *ngSwitchCase="'fail'">
+        <button pButton type="button" label="Retry" id="retry-button" (click)="retry()" class="mt-2 ml-1"></button>
+    </ng-container>
+    <ng-container *ngSwitchDefault>
+        <p-progressSpinner></p-progressSpinner>
+    </ng-container>
+</ng-container>
diff --git a/webui/src/app/settings-page/settings-page.component.spec.ts b/webui/src/app/settings-page/settings-page.component.spec.ts
index 4147b571d..aeb3703d1 100644
--- a/webui/src/app/settings-page/settings-page.component.spec.ts
+++ b/webui/src/app/settings-page/settings-page.component.spec.ts
@@ -15,10 +15,11 @@ import { BreadcrumbsComponent } from '../breadcrumbs/breadcrumbs.component'
 import { BreadcrumbModule } from 'primeng/breadcrumb'
 import { HelpTipComponent } from '../help-tip/help-tip.component'
 import { OverlayPanelModule } from 'primeng/overlaypanel'
-import { ActivatedRoute, Router } from '@angular/router'
+import { ActivatedRoute } from '@angular/router'
 import { RouterTestingModule } from '@angular/router/testing'
 import { DividerModule } from 'primeng/divider'
 import { of, throwError } from 'rxjs'
+import { ProgressSpinnerModule } from 'primeng/progressspinner'
 
 describe('SettingsPageComponent', () => {
     let component: SettingsPageComponent
@@ -39,6 +40,7 @@ describe('SettingsPageComponent', () => {
                 MessagesModule,
                 NoopAnimationsModule,
                 OverlayPanelModule,
+                ProgressSpinnerModule,
                 RouterTestingModule,
             ],
             declarations: [SettingsPageComponent, BreadcrumbsComponent, HelpTipComponent],
@@ -117,7 +119,17 @@ describe('SettingsPageComponent', () => {
         tick()
         fixture.detectChanges()
 
-        expect(messageService.add).toHaveBeenCalled()
+        // Error message should have been displayed and the retry button should be displayed.
+        expect(messageService.add).toHaveBeenCalledTimes(1)
+        const retryBtn = fixture.debugElement.query(By.css('[label=Retry]'))
+        expect(retryBtn).not.toBeNull()
+
+        // Simulate retrying.
+        component.retry()
+        tick()
+        fixture.detectChanges()
+
+        expect(messageService.add).toHaveBeenCalledTimes(2)
     }))
 
     it('should submit the form', fakeAsync(() => {
diff --git a/webui/src/app/settings-page/settings-page.component.stories.ts b/webui/src/app/settings-page/settings-page.component.stories.ts
index 5c90173b1..044df9d39 100644
--- a/webui/src/app/settings-page/settings-page.component.stories.ts
+++ b/webui/src/app/settings-page/settings-page.component.stories.ts
@@ -17,6 +17,7 @@ import { Settings } from '../backend/model/settings'
 import { HttpClientModule } from '@angular/common/http'
 import { toastDecorator } from '../utils-stories'
 import { ToastModule } from 'primeng/toast'
+import { ProgressSpinnerModule } from 'primeng/progressspinner'
 
 let mockGetSettingsResponse: Settings = {
     bind9StatsPullerInterval: 10,
@@ -50,6 +51,7 @@ export default {
                 FormsModule,
                 MessagesModule,
                 OverlayPanelModule,
+                ProgressSpinnerModule,
                 ReactiveFormsModule,
                 RouterTestingModule,
                 ToastModule,
@@ -64,7 +66,7 @@ export default {
                 url: 'http://localhost/api/settings',
                 method: 'GET',
                 status: 200,
-                delay: 0,
+                delay: 1000,
                 response: mockGetSettingsResponse,
             },
             {
diff --git a/webui/src/app/settings-page/settings-page.component.ts b/webui/src/app/settings-page/settings-page.component.ts
index 4f13bc213..5f0d110b5 100644
--- a/webui/src/app/settings-page/settings-page.component.ts
+++ b/webui/src/app/settings-page/settings-page.component.ts
@@ -107,6 +107,13 @@ export class SettingsPageComponent implements OnInit {
      */
     settingsForm: FormGroup<SettingsForm>
 
+    /**
+     * A union defining form state.
+     *
+     * It controls what is rendered.
+     */
+    formState: 'loading' | 'fail' | 'success'
+
     /**
      * Constructor.
      *
@@ -132,17 +139,18 @@ export class SettingsPageComponent implements OnInit {
     }
 
     /**
-     * A component lifecycle hook invoked upon the component initialization.
-     *
-     * It gathers the current settings from the server and initializes them
+     * Gathers the current settings from the server and initializes them
      * in the form.
      */
-    ngOnInit() {
+    private getSettings(): void {
+        this.formState = 'loading'
         this.settingsApi.getSettings().subscribe(
             (data) => {
                 this.settingsForm.patchValue(data)
+                this.formState = 'success'
             },
             (err) => {
+                this.formState = 'fail'
                 const msg = getErrorMessage(err)
                 this.msgSrv.add({
                     severity: 'error',
@@ -154,10 +162,27 @@ export class SettingsPageComponent implements OnInit {
         )
     }
 
+    /**
+     * A component lifecycle hook invoked upon the component initialization.
+     *
+     * It gathers the current settings from the server and initializes them
+     * in the form.
+     */
+    ngOnInit() {
+        this.getSettings()
+    }
+
+    /**
+     * Retries gathering the settings after failure.
+     */
+    retry(): void {
+        this.getSettings()
+    }
+
     /**
      * Saves the current values of the settings in the backend.
      */
-    saveSettings() {
+    saveSettings(): void {
         if (!this.settingsForm.valid) {
             return
         }