core: make reload dialog functional (#5200) (#5206)

Cherry-picking cf438df 

From original commit:

Because of the discrepancy between store and view on the minimum reload
period, when you set the reload period to be less than 30s, it appeared
correct while it functionally was not true. This change fixes that.

This change also allows user to set 30s sharp which was previouisly
disallowed (this was a bug).

Co-authored-by: Stephan Lee <[email protected]>

tensorboard/webapp/settings/_redux/settings_reducers.ts

@@ -31,7 +31,7 @@ function settingsReady(state: SettingsState): boolean {
 }
 
 // Auto reload period cannot be lower than 30s to prevent server load.
-const MIN_RELOAD_PERIOD_IN_MS = 30000;
+export const MIN_RELOAD_PERIOD_IN_MS = 30000;
 
 const reducer = createReducer(
   initialState,
@@ -59,7 +59,7 @@ const reducer = createReducer(
       }
 
       const nextReloadPeriod =
-        periodInMs > MIN_RELOAD_PERIOD_IN_MS
+        periodInMs >= MIN_RELOAD_PERIOD_IN_MS
           ? periodInMs
           : state.settings.reloadPeriodInMs;
       return {

tensorboard/webapp/settings/_redux/settings_reducers_test.ts

@@ -76,6 +76,19 @@ describe('settings reducer', () => {
       expect(nextState.settings.reloadPeriodInMs).toBe(50000);
     });
 
+    it('takes the minimum value, 30 seconds', () => {
+      const state = createSettingsState({
+        settings: createSettings({reloadPeriodInMs: 1}),
+      });
+
+      const nextState = reducers(
+        state,
+        actions.changeReloadPeriod({periodInMs: 30000})
+      );
+
+      expect(nextState.settings.reloadPeriodInMs).toBe(30000);
+    });
+
     it('ignores the action when periodInMs is non-positive', () => {
       const baseState = createSettingsState({
         settings: createSettings({reloadPeriodInMs: 1}),
@@ -94,6 +107,19 @@ describe('settings reducer', () => {
       expect(state2.settings.reloadPeriodInMs).toBe(1);
     });
 
+    it('ignroes the action when new time is smaller than minimum value', () => {
+      const state = createSettingsState({
+        settings: createSettings({reloadPeriodInMs: 50000}),
+      });
+
+      const nextState = reducers(
+        state,
+        actions.changeReloadPeriod({periodInMs: 5000})
+      );
+
+      expect(nextState.settings.reloadPeriodInMs).toBe(50000);
+    });
+
     it('does not set the reloadPeriodInMs when settings not loaded', () => {
       const state = createSettingsState({
         state: DataLoadState.LOADING,

tensorboard/webapp/settings/_views/settings_dialog_component.ts

@@ -28,10 +28,11 @@ import {
   AbstractControl,
   ValidatorFn,
 } from '@angular/forms';
-
 import {Subject} from 'rxjs';
 import {takeUntil, debounceTime, filter} from 'rxjs/operators';
 
+import {MIN_RELOAD_PERIOD_IN_MS} from '../_redux/settings_reducers';
+
 /** @typehack */ import * as _typeHackRxjs from 'rxjs';
 
 export function createIntegerValidator(): ValidatorFn {
@@ -68,7 +69,8 @@ export function createIntegerValidator(): ValidatorFn {
             reloadPeriodControl.hasError('required')
           "
         >
-          Reload period has to be minimum of 15 seconds.
+          Reload period has to be minimum of
+          {{ MIN_RELOAD_PERIOD_IN_S }} seconds.
         </mat-error>
       </div>
     </div>
@@ -97,9 +99,10 @@ export class SettingsDialogComponent implements OnInit, OnDestroy, OnChanges {
   @Output() reloadPeriodInMsChanged = new EventEmitter<number>();
   @Output() pageSizeChanged = new EventEmitter<number>();
 
-  readonly reloadPeriodControl = new FormControl(15, [
+  readonly MIN_RELOAD_PERIOD_IN_S = MIN_RELOAD_PERIOD_IN_MS / 1000;
+  readonly reloadPeriodControl = new FormControl(this.MIN_RELOAD_PERIOD_IN_S, [
     Validators.required,
-    Validators.min(15),
+    Validators.min(this.MIN_RELOAD_PERIOD_IN_S),
   ]);
   readonly paginationControl = new FormControl(1, [
     Validators.required,

tensorboard/webapp/settings/_views/settings_test.ts

@@ -197,7 +197,7 @@ describe('settings test', () => {
       fixture.detectChanges();
 
       const reloadPeriod = fixture.debugElement.query(By.css('.reload-period'));
-      reloadPeriod.nativeElement.value = 20;
+      reloadPeriod.nativeElement.value = 35;
       reloadPeriod.nativeElement.dispatchEvent(new Event('input'));
 
       expect(dispatchSpy).not.toHaveBeenCalled();
@@ -206,7 +206,7 @@ describe('settings test', () => {
       tick(500);
 
       expect(dispatchSpy).toHaveBeenCalledWith(
-        changeReloadPeriod({periodInMs: 20000})
+        changeReloadPeriod({periodInMs: 35000})
       );
     }));