Unify react fetcher components (prometheus#6629)

* set useFetch loading flag to be true initially Signed-off-by: blalov <[email protected]> * make extended props optional Signed-off-by: blalov <[email protected]> * add status indicator to targets page Signed-off-by: blalov <[email protected]> * add status indicator to tsdb status page Signed-off-by: blalov <[email protected]> * spread response in Alerts Signed-off-by: blalov <[email protected]> * disable eslint func retun type rule Signed-off-by: blalov <[email protected]> * add status indicator to Service Discovery page Signed-off-by: blalov <[email protected]> * refactor PanelList Signed-off-by: blalov <[email protected]> * test fix Signed-off-by: blalov <[email protected]> * use local storage hook in PanelList Signed-off-by: blalov <[email protected]> * use 'useFetch' for fetching metrics Signed-off-by: blalov <[email protected]> * left-overs Signed-off-by: blalov <[email protected]> * remove targets page custom error message Signed-off-by: Boyko Lalov <[email protected]> * adding components displayName Signed-off-by: Boyko Lalov <[email protected]> * display more user friendly error messages Signed-off-by: Boyko Lalov <[email protected]> * update status page snapshot Signed-off-by: Boyko Lalov <[email protected]> * pr review changes Signed-off-by: Boyko Lalov <[email protected]> * fix broken tests Signed-off-by: Boyko Lalov <[email protected]> * fix typos Signed-off-by: Boyko Lalov <[email protected]>
liguozhong · Feb 3, 2020 · 8c2bc2f · 8c2bc2f
1 parent 820d777
commit 8c2bc2f
Show file tree

Hide file tree

Showing 19 changed files with 406 additions and 907 deletions.
diff --git a/web/ui/react-app/.eslintrc.json b/web/ui/react-app/.eslintrc.json
@@ -7,6 +7,7 @@
   ],
   "rules": {
     "@typescript-eslint/camelcase": "warn",
+    "@typescript-eslint/explicit-function-return-type": ["off"],
     "eol-last": [
       "error",
       "always"

diff --git a/web/ui/react-app/src/App.test.tsx b/web/ui/react-app/src/App.test.tsx
@@ -4,7 +4,7 @@ import App from './App';
 import Navigation from './Navbar';
 import { Container } from 'reactstrap';
 import { Router } from '@reach/router';
-import { Alerts, Config, Flags, Rules, Services, Status, Targets, TSDBStatus, PanelList } from './pages';
+import { Alerts, Config, Flags, Rules, ServiceDiscovery, Status, Targets, TSDBStatus, PanelList } from './pages';
 
 describe('App', () => {
   const app = shallow(<App pathPrefix="/path/prefix" />);
@@ -13,7 +13,7 @@ describe('App', () => {
     expect(app.find(Navigation)).toHaveLength(1);
   });
   it('routes', () => {
-    [Alerts, Config, Flags, Rules, Services, Status, Targets, TSDBStatus, PanelList].forEach(component => {
+    [Alerts, Config, Flags, Rules, ServiceDiscovery, Status, Targets, TSDBStatus, PanelList].forEach(component => {
       const c = app.find(component);
       expect(c).toHaveLength(1);
       expect(c.prop('pathPrefix')).toBe('/path/prefix');

diff --git a/web/ui/react-app/src/App.tsx b/web/ui/react-app/src/App.tsx
@@ -4,7 +4,7 @@ import { Container } from 'reactstrap';
 
 import './App.css';
 import { Router, Redirect } from '@reach/router';
-import { Alerts, Config, Flags, Rules, Services, Status, Targets, TSDBStatus, PanelList } from './pages';
+import { Alerts, Config, Flags, Rules, ServiceDiscovery, Status, Targets, TSDBStatus, PanelList } from './pages';
 import PathPrefixProps from './types/PathPrefixProps';
 
 const App: FC<PathPrefixProps> = ({ pathPrefix }) => {
@@ -24,7 +24,7 @@ const App: FC<PathPrefixProps> = ({ pathPrefix }) => {
           <Config path="/config" pathPrefix={pathPrefix} />
           <Flags path="/flags" pathPrefix={pathPrefix} />
           <Rules path="/rules" pathPrefix={pathPrefix} />
-          <Services path="/service-discovery" pathPrefix={pathPrefix} />
+          <ServiceDiscovery path="/service-discovery" pathPrefix={pathPrefix} />
           <Status path="/status" pathPrefix={pathPrefix} />
           <TSDBStatus path="/tsdb-status" pathPrefix={pathPrefix} />
           <Targets path="/targets" pathPrefix={pathPrefix} />

diff --git a/web/ui/react-app/src/components/withStatusIndicator.tsx b/web/ui/react-app/src/components/withStatusIndicator.tsx
@@ -7,12 +7,14 @@ interface StatusIndicatorProps {
   error?: Error;
   isLoading?: boolean;
   customErrorMsg?: JSX.Element;
+  componentTitle?: string;
 }
 
 export const withStatusIndicator = <T extends {}>(Component: ComponentType<T>): FC<StatusIndicatorProps & T> => ({
   error,
   isLoading,
   customErrorMsg,
+  componentTitle,
   ...rest
 }) => {
   if (error) {
@@ -22,7 +24,7 @@ export const withStatusIndicator = <T extends {}>(Component: ComponentType<T>):
           customErrorMsg
         ) : (
           <>
-            <strong>Error:</strong> Error fetching {Component.displayName}: {error.message}
+            <strong>Error:</strong> Error fetching {componentTitle || Component.displayName}: {error.message}
           </>
         )}
       </Alert>

diff --git a/web/ui/react-app/src/hooks/useFetch.ts b/web/ui/react-app/src/hooks/useFetch.ts
@@ -1,6 +1,6 @@
 import { useState, useEffect } from 'react';
 
-export type APIResponse<T> = { status: string; data?: T };
+export type APIResponse<T> = { status: string; data: T };
 
 export interface FetchState<T> {
   response: APIResponse<T>;
@@ -9,15 +9,15 @@ export interface FetchState<T> {
 }
 
 export const useFetch = <T extends {}>(url: string, options?: RequestInit): FetchState<T> => {
-  const [response, setResponse] = useState<APIResponse<T>>({ status: 'start fetching' });
+  const [response, setResponse] = useState<APIResponse<T>>({ status: 'start fetching' } as any);
   const [error, setError] = useState<Error>();
-  const [isLoading, setIsLoading] = useState<boolean>(false);
+  const [isLoading, setIsLoading] = useState<boolean>(true);
 
   useEffect(() => {
     const fetchData = async () => {
       setIsLoading(true);
       try {
-        const res = await fetch(url, { cache: 'no-cache', credentials: 'same-origin', ...options });
+        const res = await fetch(url, { cache: 'no-store', credentials: 'same-origin', ...options });
         if (!res.ok) {
           throw new Error(res.statusText);
         }

diff --git a/web/ui/react-app/src/hooks/useLocalStorage.tsx b/web/ui/react-app/src/hooks/useLocalStorage.tsx
@@ -1,8 +1,8 @@
 import { Dispatch, SetStateAction, useEffect, useState } from 'react';
 
 export function useLocalStorage<S>(localStorageKey: string, initialState: S): [S, Dispatch<SetStateAction<S>>] {
-  const localStorageState = JSON.parse(localStorage.getItem(localStorageKey) as string);
-  const [value, setValue] = useState(localStorageState || initialState);
+  const localStorageState = JSON.parse(localStorage.getItem(localStorageKey) || JSON.stringify(initialState));
+  const [value, setValue] = useState(localStorageState);
 
   useEffect(() => {
     const serializedState = JSON.stringify(value);

diff --git a/web/ui/react-app/src/pages/alerts/Alerts.tsx b/web/ui/react-app/src/pages/alerts/Alerts.tsx
@@ -20,14 +20,7 @@ const Alerts: FC<RouteComponentProps & PathPrefixProps> = ({ pathPrefix = '' })
     response.data.groups.forEach(el => el.rules.forEach(r => ruleStatsCount[r.state]++));
   }
 
-  return (
-    <AlertsWithStatusIndicator
-      statsCount={ruleStatsCount}
-      groups={response.data && response.data.groups}
-      error={error}
-      isLoading={isLoading}
-    />
-  );
+  return <AlertsWithStatusIndicator statsCount={ruleStatsCount} {...response.data} error={error} isLoading={isLoading} />;
 };
 
 export default Alerts;
diff --git a/web/ui/react-app/src/pages/graph/PanelList.test.tsx b/web/ui/react-app/src/pages/graph/PanelList.test.tsx
@@ -1,8 +1,8 @@
 import * as React from 'react';
-import { mount, shallow } from 'enzyme';
-import PanelList from './PanelList';
+import { shallow } from 'enzyme';
+import PanelList, { PanelListContent } from './PanelList';
 import Checkbox from '../../components/Checkbox';
-import { Alert, Button } from 'reactstrap';
+import { Button } from 'reactstrap';
 import Panel from './Panel';
 
 describe('PanelList', () => {
@@ -14,31 +14,20 @@ describe('PanelList', () => {
       const panelList = shallow(<PanelList />);
       const checkbox = panelList.find(Checkbox).at(idx);
       expect(checkbox.prop('id')).toEqual(cb.id);
-      expect(checkbox.prop('wrapperStyles')).toEqual({
-        margin: '0 0 0 15px',
-        alignSelf: 'center',
-      });
       expect(checkbox.prop('defaultChecked')).toBe(false);
       expect(checkbox.children().text()).toBe(cb.label);
     });
   });
 
-  it('renders an alert when no data is queried yet', () => {
-    const panelList = mount(<PanelList />);
-    const alert = panelList.find(Alert);
-    expect(alert.prop('color')).toEqual('light');
-    expect(alert.children().text()).toEqual('No data queried yet');
-  });
-
   it('renders panels', () => {
-    const panelList = shallow(<PanelList />);
+    const panelList = shallow(<PanelListContent {...({ panels: [{}] } as any)} />);
     const panels = panelList.find(Panel);
     expect(panels.length).toBeGreaterThan(0);
   });
 
   it('renders a button to add a panel', () => {
-    const panelList = shallow(<PanelList />);
-    const btn = panelList.find(Button).filterWhere(btn => btn.prop('className') === 'add-panel-btn');
+    const panelList = shallow(<PanelListContent {...({ panels: [] } as any)} />);
+    const btn = panelList.find(Button);
     expect(btn.prop('color')).toEqual('primary');
     expect(btn.children().text()).toEqual('Add Panel');
   });