fix(ui): rewrite History component with hooks

ui/src/Components/NavBar/FilterInput/History.js

@@ -1,8 +1,7 @@
-import React, { Component } from "react";
+import React, { useEffect, useRef } from "react";
 import PropTypes from "prop-types";
 
-import { action, observable, toJS } from "mobx";
-import { observer } from "mobx-react";
+import { useObserver, useLocalStore } from "mobx-react";
 import { localStored } from "mobx-stored";
 
 import hash from "object-hash";
@@ -22,10 +21,6 @@ import { Settings } from "Stores/Settings";
 import { DropdownSlide } from "Components/Animations/DropdownSlide";
 import { HistoryLabel } from "Components/Labels/HistoryLabel";
 
-const defaultHistory = {
-  filters: [],
-};
-
 // takes a filter object out of alertStore.history.values and creates a new
 // object with only those keys that will be stored in history
 function ReduceFilter(filter) {
@@ -148,36 +143,34 @@ HistoryMenuContent.propTypes = {
 
 const HistoryMenu = onClickOutside(HistoryMenuContent);
 
-const History = observer(
-  class History extends Component {
-    static propTypes = {
-      alertStore: PropTypes.instanceOf(AlertStore).isRequired,
-      settingsStore: PropTypes.instanceOf(Settings).isRequired,
-    };
+const History = ({ alertStore, settingsStore }) => {
+  // this will be dumped to local storage via mobx-stored
+  const history = localStored(
+    "history.filters",
+    {
+      filters: [],
+    },
+    {
+      delay: 100,
+    }
+  );
 
-    // how many filter sets do we store in local storage and render in the
-    // dropdown menu
-    maxSize = 8;
-    // this will be dumped to local storage via mobx-stored
-    history = localStored("history.filters", defaultHistory, { delay: 100 });
+  const collapse = useLocalStore(() => ({
+    isHidden: true,
+    toggle() {
+      this.isHidden = !this.isHidden;
+    },
+    hide() {
+      this.isHidden = true;
+    },
+  }));
 
-    collapse = observable(
-      {
-        value: true,
-        toggle() {
-          this.value = !this.value;
-        },
-        hide() {
-          this.value = true;
-        },
-      },
-      { toggle: action.bound, hide: action.bound },
-      { name: "History menu toggle" }
-    );
-
-    appendToHistory = action(() => {
-      const { alertStore } = this.props;
+  const mountRef = useRef(false);
 
+  // every time this component updates we will rewrite history
+  // (if there are changes)
+  useEffect(() => {
+    if (mountRef.current) {
       // we don't store unapplied (we only have raw text for those, we need
       // name & value for coloring) or invalid filters
       // also check for value, name might be missing for fuzzy filters, but
@@ -191,85 +184,70 @@ const History = observer(
       // make a JSON dump for comparing later with what's already stored
       const filtersJSON = JSON.stringify(validAppliedFilters);
 
-      // dump observable array with stored filters to JS objects, without this
-      // we'll be passing around and comparing proxy objects that might mutate
-      // while we do so
-      const storedFilters = toJS(this.history.filters);
-
       // rewrite history putting current filter set on top, this will move
       // it up if user selects a filter set that was already in history
       let newHistory = [
         ...[validAppliedFilters],
-        ...storedFilters.filter((f) => JSON.stringify(f) !== filtersJSON),
-      ].slice(0, this.maxSize);
-      this.history.filters = newHistory;
-    });
-
-    clearHistory = action(() => {
-      this.history.filters = [];
-    });
-
-    componentDidUpdate() {
-      // every time this component updates we will rewrite history
-      // (if there are changes)
-      this.appendToHistory();
+        ...history.filters.filter((f) => JSON.stringify(f) !== filtersJSON),
+      ].slice(0, 8);
+      history.filters = newHistory;
+    } else {
+      mountRef.current = true;
     }
+  });
 
-    handleClickOutside = action((event) => {
-      this.collapse.hide();
-    });
-
-    render() {
-      const { alertStore, settingsStore } = this.props;
-
-      return (
-        // data-filters is there to register filters for observation in mobx
-        // it needs to be using full filter object to notice changes to
-        // name & value but ignore hits
-        // using it this way will force re-render on every change, which is
-        // needed to keep track of all filter changes
-        <Manager
-          data-filters={alertStore.filters.values
-            .map((f) => ReduceFilter(f))
-            .join(" ")}
-        >
-          <Reference>
-            {({ ref }) => (
-              <button
-                ref={ref}
-                onClick={this.collapse.toggle}
-                className="input-group-text border-0 rounded-0 bg-transparent cursor-pointer components-navbar-history px-2 components-navbar-icon"
-                type="button"
-                data-toggle="dropdown"
-                aria-haspopup="true"
-                aria-expanded="true"
-              >
-                <FontAwesomeIcon icon={faCaretDown} />
-              </button>
-            )}
-          </Reference>
-          <DropdownSlide in={!this.collapse.value} unmountOnExit>
-            <Popper modifiers={[{ name: "arrow", enabled: false }]}>
-              {({ placement, ref, style }) => (
-                <HistoryMenu
-                  popperPlacement={placement}
-                  popperRef={ref}
-                  popperStyle={style}
-                  filters={this.history.filters}
-                  onClear={this.clearHistory}
-                  alertStore={alertStore}
-                  settingsStore={settingsStore}
-                  afterClick={this.collapse.hide}
-                  handleClickOutside={this.collapse.hide}
-                  outsideClickIgnoreClass="components-navbar-history"
-                />
-              )}
-            </Popper>
-          </DropdownSlide>
-        </Manager>
-      );
-    }
-  }
-);
+  return useObserver(() => (
+    // data-filters is there to register filters for observation in mobx
+    // it needs to be using full filter object to notice changes to
+    // name & value but ignore hits
+    // using it this way will force re-render on every change, which is
+    // needed to keep track of all filter changes
+    <Manager
+      data-filters={alertStore.filters.values
+        .map((f) => ReduceFilter(f))
+        .join(" ")}
+    >
+      <Reference>
+        {({ ref }) => (
+          <button
+            ref={ref}
+            onClick={collapse.toggle}
+            className="input-group-text border-0 rounded-0 bg-transparent cursor-pointer components-navbar-history px-2 components-navbar-icon"
+            type="button"
+            data-toggle="dropdown"
+            aria-haspopup="true"
+            aria-expanded="true"
+          >
+            <FontAwesomeIcon icon={faCaretDown} />
+          </button>
+        )}
+      </Reference>
+      <DropdownSlide in={!collapse.isHidden} unmountOnExit>
+        <Popper modifiers={[{ name: "arrow", enabled: false }]}>
+          {({ placement, ref, style }) => (
+            <HistoryMenu
+              popperPlacement={placement}
+              popperRef={ref}
+              popperStyle={style}
+              filters={history.filters}
+              onClear={() => {
+                history.filters = [];
+              }}
+              alertStore={alertStore}
+              settingsStore={settingsStore}
+              afterClick={collapse.hide}
+              handleClickOutside={collapse.hide}
+              outsideClickIgnoreClass="components-navbar-history"
+            />
+          )}
+        </Popper>
+      </DropdownSlide>
+    </Manager>
+  ));
+};
+History.propTypes = {
+  alertStore: PropTypes.instanceOf(AlertStore).isRequired,
+  settingsStore: PropTypes.instanceOf(Settings).isRequired,
+};
 
 export { History, HistoryMenu, HistoryMenuContent, ReduceFilter };

ui/src/Components/NavBar/FilterInput/History.test.js

@@ -1,106 +1,100 @@
 import React from "react";
 import { act } from "react-dom/test-utils";
 
-import { mount, shallow } from "enzyme";
+import { mount } from "enzyme";
 
 import { AlertStore, NewUnappliedFilter } from "Stores/AlertStore";
 import { Settings } from "Stores/Settings";
-import { History, HistoryMenu, ReduceFilter } from "./History";
+import { History } from "./History";
 
 let alertStore;
 let settingsStore;
 
+beforeAll(() => {
+  jest.useFakeTimers();
+});
+
 beforeEach(() => {
   alertStore = new AlertStore([]);
   settingsStore = new Settings();
 });
 
+afterEach(() => {
+  localStorage.setItem("history.filters", "");
+});
+
 const MountedHistory = () => {
   return mount(
     <History alertStore={alertStore} settingsStore={settingsStore} />
   );
 };
 
-const ShallowHistoryMenu = (historyTree) => {
-  const tree = shallow(
-    <HistoryMenu
-      popperPlacement={null}
-      popperRef={null}
-      popperStyle={null}
-      filters={historyTree.instance().history.filters}
-      onClear={historyTree.instance().clearHistory}
-      alertStore={historyTree.props().alertStore}
-      settingsStore={historyTree.props().settingsStore}
-      afterClick={historyTree.instance().collapse.hide}
-    />
-  );
-  return tree;
-};
-
-const MountedHistoryMenu = (historyTree) => {
-  const tree = mount(
-    <HistoryMenu
-      popperPlacement={null}
-      popperRef={null}
-      popperStyle={null}
-      filters={historyTree.instance().history.filters}
-      onClear={historyTree.instance().clearHistory}
-      alertStore={historyTree.props().alertStore}
-      settingsStore={historyTree.props().settingsStore}
-      afterClick={historyTree.instance().collapse.hide}
-    />
-  );
-  return tree;
-};
-
 const AppliedFilter = (name, matcher, value) => {
   const filter = NewUnappliedFilter(`${name}${matcher}${value}`);
   filter.applied = true;
+  filter.isValid = true;
   filter.name = name;
   filter.matcher = matcher;
   filter.value = value;
   return filter;
 };
 
-const PopulateHistory = (instance, count) => {
+const PopulateHistory = (tree, count) => {
   for (let i = 1; i <= count; i++) {
     alertStore.filters.values = [
       AppliedFilter("foo", "=", `bar${i}`),
       AppliedFilter("baz", "=~", `bar${i}`),
     ];
-    instance.appendToHistory();
+    tree.update();
+    jest.runOnlyPendingTimers();
   }
 };
 
 describe("<History />", () => {
-  it("renders dropdown button when menu is hidden", () => {
-    const tree = MountedHistory();
-    const dropdown = tree.find("button");
-    expect(dropdown.props().className.split(" ")).toContain(
-      "components-navbar-history"
-    );
-  });
-
-  // Due to https://github.com/FezVrasta/popper.js/issues/478 we can't test
-  // rendered dropdown content, only the fact that toggle value is updated
-  it("renders dropdown button when menu is visible", async () => {
+  it("menu content is hidden by default", async () => {
     const promise = Promise.resolve();
     const tree = MountedHistory();
-    const toggle = tree.find("button");
-
-    expect(tree.instance().collapse.value).toBe(true);
-    toggle.simulate("click");
-    expect(tree.instance().collapse.value).toBe(false);
+    expect(tree.find("div.dropdown-menu")).toHaveLength(0);
     await act(() => promise);
   });
 
-  it("hides when handleClickOutside() is called", async () => {
+  it("clicking toggle renders menu content", async () => {
     const promise = Promise.resolve();
     const tree = MountedHistory();
-    const instance = tree.instance();
-    instance.collapse.value = false;
-    instance.handleClickOutside();
-    expect(tree.instance().collapse.value).toBe(true);
+    const toggle = tree.find("button.cursor-pointer");
+    toggle.simulate("click");
+    expect(tree.find("div.dropdown-menu")).toHaveLength(1);
+    await act(() => promise);
+  });
+
+  it("clicking toggle twice hides menu content", async () => {
+    const promise = Promise.resolve();
+    const tree = MountedHistory();
+    const toggle = tree.find("button.cursor-pointer");
+
+    toggle.simulate("click");
+    jest.runOnlyPendingTimers();
+    expect(tree.find("div.dropdown-menu")).toHaveLength(1);
+
+    toggle.simulate("click");
+    jest.runOnlyPendingTimers();
+    tree.update();
+    expect(tree.find("div.dropdown-menu")).toHaveLength(0);
+    await act(() => promise);
+  });
+
+  it("clicking menu item hides menu content", async () => {
+    const promise = Promise.resolve();
+    const tree = MountedHistory();
+    const toggle = tree.find("button.cursor-pointer");
+
+    toggle.simulate("click");
+    expect(tree.find("div.dropdown-menu")).toHaveLength(1);
+
+    tree.find(".component-history-button").at(0).simulate("click");
+    jest.runOnlyPendingTimers();
+    tree.update();
+    expect(tree.find("div.dropdown-menu")).toHaveLength(0);
     await act(() => promise);
   });
 
@@ -112,73 +106,71 @@ describe("<History />", () => {
       AppliedFilter("baz", "!=", "bar"),
     ];
     const tree = MountedHistory();
-    tree.instance().appendToHistory();
-    expect(tree.instance().history.filters).toHaveLength(1);
-    expect(JSON.stringify(tree.instance().history.filters[0])).toBe(
-      JSON.stringify([
-        ReduceFilter(AppliedFilter("foo", "=", "bar")),
-        ReduceFilter(AppliedFilter("baz", "!=", "bar")),
-      ])
-    );
+    tree.find("button.cursor-pointer").simulate("click");
+    expect(tree.find("button.dropdown-item")).toHaveLength(1);
+    const labels = tree.find("HistoryLabel");
+    expect(labels).toHaveLength(2);
+    expect(labels.at(0).html()).toMatch(/>foo=bar</);
+    expect(labels.at(1).html()).toMatch(/>baz!=bar</);
     await act(() => promise);
   });
 });
 
 describe("<HistoryMenu />", () => {
-  it("renders correctly when rendered with empty history", () => {
-    const historyTree = MountedHistory();
-    const tree = ShallowHistoryMenu(historyTree);
+  it("renders correctly when rendered with empty history", async () => {
+    const promise = Promise.resolve();
+    const tree = MountedHistory();
+    tree.find("button.cursor-pointer").simulate("click");
     expect(tree.text()).toBe(
-      "<FontAwesomeIcon />Last used filtersEmpty<ActionButton /><ActionButton /><ActionButton />"
+      "Last used filtersEmptySave filtersReset filtersClear history"
     );
+    await act(() => promise);
   });
 
-  it("renders correctly when rendered with a filter in history", () => {
-    const historyTree = MountedHistory();
-    PopulateHistory(historyTree.instance(), 1);
+  it("renders correctly when rendered with a filter in history", async () => {
+    const promise = Promise.resolve();
+    const tree = MountedHistory();
+    PopulateHistory(tree, 1);
+    tree.find("button.cursor-pointer").simulate("click");
 
-    const tree = ShallowHistoryMenu(historyTree);
     expect(tree.text()).toBe(
-      "<FontAwesomeIcon />Last used filters<HistoryLabel /><HistoryLabel /><ActionButton /><ActionButton /><ActionButton />"
+      "Last used filtersfoo=bar1baz=~bar1Save filtersReset filtersClear history"
     );
 
     const labels = tree.find("HistoryLabel");
     expect(labels).toHaveLength(2);
     expect(labels.at(0).html()).toMatch(/>foo=bar1</);
     expect(labels.at(1).html()).toMatch(/>baz=~bar1</);
+    await act(() => promise);
   });
 
-  it("clicking on a filter set in history populates alertStore", () => {
-    const historyTree = MountedHistory();
-    PopulateHistory(historyTree.instance(), 1);
+  it("clicking on a filter set in history populates alertStore", async () => {
+    const promise = Promise.resolve();
+    const tree = MountedHistory();
+    PopulateHistory(tree, 1);
+    tree.find("button.cursor-pointer").simulate("click");
 
-    const tree = MountedHistoryMenu(historyTree);
-    const button = tree.find("button").at(0);
+    const button = tree.find("button.dropdown-item").at(0);
     expect(button.text()).toBe("foo=bar1baz=~bar1");
 
     alertStore.filters.values = [AppliedFilter("job", "=", "foo")];
     expect(alertStore.filters.values).toHaveLength(1);
 
     button.simulate("click");
+    jest.runOnlyPendingTimers();
     expect(alertStore.filters.values).toHaveLength(2);
     expect(alertStore.filters.values[0]).toMatchObject({ raw: "foo=bar1" });
     expect(alertStore.filters.values[1]).toMatchObject({ raw: "baz=~bar1" });
+    await act(() => promise);
   });
 
-  it("stores only up to maxSize last filter sets in history storage", () => {
-    const historyTree = MountedHistory();
-    const instance = historyTree.instance();
-    const maxSize = instance.maxSize;
-    PopulateHistory(instance, maxSize * 2);
-    expect(instance.history.filters).toHaveLength(maxSize);
-  });
+  it("renders only up to maxSize last filter sets in history", async () => {
+    const promise = Promise.resolve();
+    const tree = MountedHistory();
+    PopulateHistory(tree, 16);
+    tree.find("button.cursor-pointer").simulate("click");
+    expect(tree.find("button.dropdown-item")).toHaveLength(8);
 
-  it("renders only up to maxSize last filter sets in history", () => {
-    const historyTree = MountedHistory();
-    const instance = historyTree.instance();
-    PopulateHistory(instance, instance.maxSize * 2);
-
-    const tree = ShallowHistoryMenu(historyTree);
     const labelSets = tree.find(".components-navbar-historymenu-labels");
     expect(labelSets).toHaveLength(8);
 
@@ -193,6 +185,7 @@ describe("<HistoryMenu />", () => {
     expect(labelsFist).toHaveLength(2);
     expect(labelsFist.at(0).html()).toMatch(/>foo=bar16</);
     expect(labelsFist.at(1).html()).toMatch(/>baz=~bar16</);
+    await act(() => promise);
   });
 
   it("clicking on 'Save filters' saves current filter set to Settings", () => {
@@ -201,12 +194,13 @@ describe("<HistoryMenu />", () => {
       AppliedFilter("bar", "=~", "baz"),
     ];
 
-    const historyTree = MountedHistory();
-    const tree = MountedHistoryMenu(historyTree);
+    const tree = MountedHistory();
+    tree.find("button.cursor-pointer").simulate("click");
     const button = tree.find(".component-history-button").at(0);
     expect(button.text()).toBe("Save filters");
 
     button.simulate("click");
+    jest.runOnlyPendingTimers();
     expect(settingsStore.savedFilters.config.filters).toHaveLength(2);
     expect(settingsStore.savedFilters.config.filters).toContain("foo=bar");
     expect(settingsStore.savedFilters.config.filters).toContain("bar=~baz");
@@ -214,27 +208,27 @@ describe("<HistoryMenu />", () => {
 
   it("clicking on 'Reset filters' clears current filter set in Settings", () => {
     settingsStore.savedFilters.config.filters = ["foo=bar"];
+    const tree = MountedHistory();
+    tree.find("button.cursor-pointer").simulate("click");
 
-    const historyTree = MountedHistory();
-    const tree = MountedHistoryMenu(historyTree);
     const button = tree.find(".component-history-button").at(1);
     expect(button.text()).toBe("Reset filters");
-
     button.simulate("click");
+    jest.runOnlyPendingTimers();
     expect(settingsStore.savedFilters.config.filters).toHaveLength(0);
   });
 
   it("clicking on 'Clear history' clears the history", () => {
-    const historyTree = MountedHistory();
-    const instance = historyTree.instance();
-    PopulateHistory(instance, 5);
-    expect(instance.history.filters).toHaveLength(5);
+    const tree = MountedHistory();
+    PopulateHistory(tree, 5);
+    tree.find("button.cursor-pointer").simulate("click");
+    expect(tree.find("button.dropdown-item")).toHaveLength(5);
 
-    const tree = MountedHistoryMenu(historyTree);
     const button = tree.find(".component-history-button").at(2);
     expect(button.text()).toBe("Clear history");
-
     button.simulate("click");
-    expect(instance.history.filters).toHaveLength(0);
+    jest.runOnlyPendingTimers();
+    tree.update();
+    expect(tree.find("button.dropdown-item")).toHaveLength(0);
   });
 });