From 0804aa475bec99d680e0e604aeb5f45a8d6a635c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= <l.mierzwa@gmail.com>
Date: Wed, 25 Dec 2019 15:41:26 +0000
Subject: [PATCH] feat(ui): change fetch indicator color to red when requests
 are failing

---
 ui/src/Common/Fetch.js                            |  7 +++++--
 ui/src/Common/Fetch.test.js                       | 15 +++++++++++++++
 ui/src/Components/NavBar/FetchIndicator/index.js  |  8 +++++++-
 .../NavBar/FetchIndicator/index.test.js           |  7 +++++++
 ui/src/Stores/AlertStore.js                       | 12 +++++++++++-
 5 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/ui/src/Common/Fetch.js b/ui/src/Common/Fetch.js
index 625016fce..841da1d0f 100644
--- a/ui/src/Common/Fetch.js
+++ b/ui/src/Common/Fetch.js
@@ -13,7 +13,7 @@ const FetchRetryConfig = {
   maxTimeout: 5000
 };
 
-const FetchGet = async (uri, options, retryOptions) =>
+const FetchGet = async (uri, options, beforeRetry) =>
   await promiseRetry(
     (retry, number) =>
       fetch(
@@ -30,7 +30,10 @@ const FetchGet = async (uri, options, retryOptions) =>
           CommonOptions,
           options
         )
-      ).catch(retry),
+      ).catch(err => {
+        beforeRetry && beforeRetry(number);
+        return retry(err);
+      }),
     FetchRetryConfig
   );
 
diff --git a/ui/src/Common/Fetch.test.js b/ui/src/Common/Fetch.test.js
index 0d777b6d6..a546ceb28 100644
--- a/ui/src/Common/Fetch.test.js
+++ b/ui/src/Common/Fetch.test.js
@@ -79,4 +79,19 @@ describe("Fetch", () => {
       });
     }
   });
+
+  it("FetchGet calls beforeRetry before each retry", async () => {
+    fetch.mockReject(new Error("Fetch error"));
+
+    const beforeRetrySpy = jest.fn();
+
+    const request = FetchGet("http://example.com", {}, beforeRetrySpy);
+    await expect(request).rejects.toThrow("Fetch error");
+
+    expect(beforeRetrySpy).toHaveBeenCalledTimes(11);
+
+    for (let i = 0; i < 10; i++) {
+      expect(beforeRetrySpy.mock.calls[i][0]).toBe(i + 1);
+    }
+  });
 });
diff --git a/ui/src/Components/NavBar/FetchIndicator/index.js b/ui/src/Components/NavBar/FetchIndicator/index.js
index 28e7c32f4..4a2bff801 100644
--- a/ui/src/Components/NavBar/FetchIndicator/index.js
+++ b/ui/src/Components/NavBar/FetchIndicator/index.js
@@ -44,7 +44,13 @@ const FetchIndicator = observer(
       const status = alertStore.status.value.toString();
 
       if (status === AlertStoreStatuses.Fetching.toString())
-        return <FetchIcon icon={faCircleNotch} spin />;
+        return (
+          <FetchIcon
+            icon={faCircleNotch}
+            color={alertStore.info.isRetrying ? "danger" : "muted"}
+            spin
+          />
+        );
 
       if (status === AlertStoreStatuses.Processing.toString())
         return <FetchIcon icon={faCircleNotch} color="success" spin />;
diff --git a/ui/src/Components/NavBar/FetchIndicator/index.test.js b/ui/src/Components/NavBar/FetchIndicator/index.test.js
index 0b9e09add..0dae8a4a6 100644
--- a/ui/src/Components/NavBar/FetchIndicator/index.test.js
+++ b/ui/src/Components/NavBar/FetchIndicator/index.test.js
@@ -42,6 +42,13 @@ describe("<FetchIndicator />", () => {
     expect(tree.find("FontAwesomeIcon").hasClass("text-muted")).toBe(true);
   });
 
+  it("uses text-danger when we need to retry fetch", () => {
+    alertStore.info.setIsRetrying();
+    alertStore.status.setFetching();
+    const tree = MountedFetchIndicator();
+    expect(tree.find("FontAwesomeIcon").hasClass("text-danger")).toBe(true);
+  });
+
   it("opacity is 1 when response is processed", () => {
     alertStore.status.setProcessing();
     const tree = MountedFetchIndicator();
diff --git a/ui/src/Stores/AlertStore.js b/ui/src/Stores/AlertStore.js
index 15e4d4d3e..cc2d0bd0b 100644
--- a/ui/src/Stores/AlertStore.js
+++ b/ui/src/Stores/AlertStore.js
@@ -182,12 +182,21 @@ class AlertStore {
       totalAlerts: 0,
       version: "unknown",
       upgradeNeeded: false,
+      isRetrying: false,
       reloadNeeded: false,
+      setIsRetrying() {
+        this.isRetrying = true;
+      },
+      clearIsRetrying() {
+        this.isRetrying = false;
+      },
       setReloadNeeded() {
         this.reloadNeeded = true;
       }
     },
     {
+      setIsRetrying: action.bound,
+      clearIsRetrying: action.bound,
       setReloadNeeded: action
     },
     { name: "API response info" }
@@ -281,7 +290,7 @@ class AlertStore {
         `alerts.json?sortOrder=${sortOrder}&sortLabel=${sortLabel}&sortReverse=${sortReverse}&`
       ) + FormatAPIFilterQuery(this.filters.values.map(f => f.raw));
 
-    return FetchGet(alertsURI, {})
+    return FetchGet(alertsURI, {}, this.info.setIsRetrying)
       .then(result => {
         // we're sending requests with mode=cors so the response should also be type=cors
         // after a few failures in the retry loop we will switch to no-cors
@@ -290,6 +299,7 @@ class AlertStore {
         if (result.type === "opaque") {
           this.info.setReloadNeeded();
         }
+        this.info.clearIsRetrying();
         this.status.setProcessing();
         return result.json();
       })