Bug 1409836 - Fix setting filters prefs when filters are reset or cleared; r=bgrins.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 19 Oct 2017 10:07:54 +0200
changeset 387396 8baf55e85dae4c8c5c9738ec117f940abcde1aef
parent 387395 40f379f9b7523c54cef00a3e14a16ea6609c15c7
child 387397 c545c4ae3e18c59f7c57e2f0f3b9f014072843a3
push id53743
push usernchevobbe@mozilla.com
push dateFri, 20 Oct 2017 13:05:45 +0000
treeherderautoland@8baf55e85dae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1409836
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1409836 - Fix setting filters prefs when filters are reset or cleared; r=bgrins. We were getting the filter state after dispatching the action, which made all the filters to have an enabled state. Getting the state before dispatching fixes the issue. This patch enhance the Service mock in order to have a better idea of what is going on with prefs. This allow us to introduce some tests to make sure prefs are updated in reaction to given actions. MozReview-Commit-ID: Byay0TwF25I
devtools/client/webconsole/new-console-output/actions/filters.js
devtools/client/webconsole/new-console-output/test/components/filter-bar.test.js
devtools/client/webconsole/new-console-output/test/fixtures/Services.js
devtools/client/webconsole/new-console-output/test/store/filters.test.js
--- a/devtools/client/webconsole/new-console-output/actions/filters.js
+++ b/devtools/client/webconsole/new-console-output/actions/filters.js
@@ -39,38 +39,40 @@ function filterToggle(filter) {
 }
 
 function filtersClear() {
   return (dispatch, getState) => {
     dispatch({
       type: FILTERS_CLEAR,
     });
 
-    const filterState = getAllFilters(getState());
+    const filterState = getAllFilters(getState()).toJS();
     for (let filter in filterState) {
       if (filter !== FILTERS.TEXT) {
         Services.prefs.clearUserPref(PREFS.FILTER[filter.toUpperCase()]);
       }
     }
   };
 }
 
 /**
  * Set the default filters to their original values.
  * This is different than filtersClear where we reset
  * all the filters to their original values. Here we want
  * to keep non-default filters the user might have set.
  */
 function defaultFiltersReset() {
   return (dispatch, getState) => {
+    // Get the state before dispatching so the action does not alter prefs reset.
+    const filterState = getAllFilters(getState());
+
     dispatch({
       type: DEFAULT_FILTERS_RESET,
     });
 
-    const filterState = getAllFilters(getState());
     DEFAULT_FILTERS.forEach(filter => {
       if (filterState[filter] === false) {
         Services.prefs.clearUserPref(PREFS.FILTER[filter.toUpperCase()]);
       }
     });
   };
 }
 
--- a/devtools/client/webconsole/new-console-output/test/components/filter-bar.test.js
+++ b/devtools/client/webconsole/new-console-output/test/components/filter-bar.test.js
@@ -12,22 +12,28 @@ const Provider = createFactory(require("
 const actions = require("devtools/client/webconsole/new-console-output/actions/index");
 const FilterButton = require("devtools/client/webconsole/new-console-output/components/FilterButton");
 const FilterBar = createFactory(require("devtools/client/webconsole/new-console-output/components/FilterBar"));
 const { getAllUi } = require("devtools/client/webconsole/new-console-output/selectors/ui");
 const { getAllFilters } = require("devtools/client/webconsole/new-console-output/selectors/filters");
 const {
   MESSAGES_CLEAR,
   FILTERS,
+  PREFS,
 } = require("devtools/client/webconsole/new-console-output/constants");
 
 const { setupStore } = require("devtools/client/webconsole/new-console-output/test/helpers");
 const serviceContainer = require("devtools/client/webconsole/new-console-output/test/fixtures/serviceContainer");
+const ServicesMock = require("Services");
 
 describe("FilterBar component:", () => {
+  afterEach(() => {
+    ServicesMock.prefs.testHelpers.clearPrefs();
+  });
+
   it("initial render", () => {
     const store = setupStore([]);
 
     const wrapper = render(Provider({store}, FilterBar({ serviceContainer })));
     const toolbar = wrapper.find(
       ".devtools-toolbar.webconsole-filterbar-primary"
     );
 
@@ -185,21 +191,23 @@ describe("FilterBar component:", () => {
     const toolbar = wrapper.find(".webconsole-filterbar-filtered-messages");
     expect(toolbar.exists()).toBeFalsy();
   });
 
   it("displays filter bar when button is clicked", () => {
     const store = setupStore([]);
 
     expect(getAllUi(store.getState()).filterBarVisible).toBe(false);
+    expect(ServicesMock.prefs.getBoolPref(PREFS.UI.FILTER_BAR), false);
 
     const wrapper = mount(Provider({store}, FilterBar({ serviceContainer })));
     wrapper.find(".devtools-filter-icon").simulate("click");
 
     expect(getAllUi(store.getState()).filterBarVisible).toBe(true);
+    expect(ServicesMock.prefs.getBoolPref(PREFS.UI.FILTER_BAR), true);
 
     const secondaryBar = wrapper.find(".webconsole-filterbar-secondary");
     expect(secondaryBar.length).toBe(1);
 
     // Buttons are displayed
     const filterBtn = props => FilterButton(
       Object.assign({}, {
         active: true,
@@ -245,15 +253,17 @@ describe("FilterBar component:", () => {
     wrapper.find(".devtools-plaininput").simulate("input", { target: { value: "a" } });
     expect(store.getState().filters.text).toBe("a");
   });
 
   it("toggles persist logs when checkbox is clicked", () => {
     const store = setupStore([]);
 
     expect(getAllUi(store.getState()).persistLogs).toBe(false);
+    expect(ServicesMock.prefs.getBoolPref(PREFS.UI.PERSIST), false);
 
     const wrapper = mount(Provider({store}, FilterBar({ serviceContainer })));
     wrapper.find(".filter-checkbox input").simulate("change");
 
     expect(getAllUi(store.getState()).persistLogs).toBe(true);
+    expect(ServicesMock.prefs.getBoolPref(PREFS.UI.PERSIST), true);
   });
 });
--- a/devtools/client/webconsole/new-console-output/test/fixtures/Services.js
+++ b/devtools/client/webconsole/new-console-output/test/fixtures/Services.js
@@ -1,30 +1,46 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
-const { PREFS } = require("devtools/client/webconsole/new-console-output/constants");
+const {
+  DEFAULT_FILTERS_VALUES,
+  FILTERS,
+  PREFS
+} = require("devtools/client/webconsole/new-console-output/constants");
+
+function getDefaultPrefs() {
+  return Object.assign({
+    "devtools.hud.loglimit": 1000,
+    [PREFS.UI.FILTER_BAR]: false,
+    [PREFS.UI.PERSIST]: false,
+  }, Object.entries(PREFS.FILTER).reduce((res, [key, pref]) => {
+    res[pref] = DEFAULT_FILTERS_VALUES[FILTERS[key]];
+    return res;
+  }, {}));
+}
+
+let prefs = Object.assign({}, getDefaultPrefs());
 
 module.exports = {
   prefs: {
-    getIntPref: pref => {
-      switch (pref) {
-        case "devtools.hud.loglimit":
-          return 1000;
-      }
-      return null;
+    getIntPref: pref => prefs[pref],
+    getBoolPref: pref => prefs[pref],
+    setBoolPref: (pref, value) => {
+      prefs[pref] = value;
+    },
+    clearUserPref: (pref) => {
+      prefs[pref] = (getDefaultPrefs())[pref];
     },
-    getBoolPref: pref => {
-      const falsey = [
-        PREFS.FILTER.CSS,
-        PREFS.FILTER.NET,
-        PREFS.FILTER.NETXHR,
-        PREFS.UI.FILTER_BAR,
-        PREFS.UI.PERSIST,
-      ];
-      return !falsey.includes(pref);
-    },
-    setBoolPref: () => {},
-    clearUserPref: () => {},
+    testHelpers: {
+      getAllPrefs: () => prefs,
+      getFiltersPrefs: () => Object.values(PREFS.FILTER).reduce((res, pref) => {
+        res[pref] = prefs[pref];
+        return res;
+      }, {}),
+      clearPrefs: () => {
+        prefs = Object.assign({}, getDefaultPrefs());
+      }
+    }
   }
 };
--- a/devtools/client/webconsole/new-console-output/test/store/filters.test.js
+++ b/devtools/client/webconsole/new-console-output/test/store/filters.test.js
@@ -6,19 +6,20 @@
 const expect = require("expect");
 
 const actions = require("devtools/client/webconsole/new-console-output/actions/index");
 const { messageAdd } = require("devtools/client/webconsole/new-console-output/actions/index");
 const { ConsoleCommand } = require("devtools/client/webconsole/new-console-output/types");
 const { getVisibleMessages } = require("devtools/client/webconsole/new-console-output/selectors/messages");
 const { getAllFilters } = require("devtools/client/webconsole/new-console-output/selectors/filters");
 const { setupStore } = require("devtools/client/webconsole/new-console-output/test/helpers");
-const { FILTERS } = require("devtools/client/webconsole/new-console-output/constants");
+const { FILTERS, PREFS } = require("devtools/client/webconsole/new-console-output/constants");
 const { stubPackets } = require("devtools/client/webconsole/new-console-output/test/fixtures/stubs/index");
 const { stubPreparedMessages } = require("devtools/client/webconsole/new-console-output/test/fixtures/stubs/index");
+const ServicesMock = require("Services");
 
 describe("Filtering", () => {
   let store;
   let numMessages;
   // Number of messages in prepareBaseStore which are not filtered out, i.e. Evaluation
   // Results, console commands and console.groups .
   const numUnfilterableMessages = 3;
 
@@ -210,41 +211,62 @@ describe("Clear filters", () => {
     store.dispatch(actions.filterToggle(FILTERS.CSS));
     store.dispatch(actions.filterToggle(FILTERS.NET));
     store.dispatch(actions.filterToggle(FILTERS.NETXHR));
     store.dispatch(actions.filterTextSet("foobar"));
 
     let filters = getAllFilters(store.getState());
     expect(filters.toJS()).toEqual({
       // default
-      "warn": true,
-      "log": true,
-      "info": true,
-      "debug": true,
-      "css": true,
+      [FILTERS.WARN]: true,
+      [FILTERS.LOG]: true,
+      [FILTERS.INFO]: true,
+      [FILTERS.DEBUG]: true,
       // changed
-      "error": false,
-      "net": true,
-      "netxhr": true,
-      "text": "foobar",
+      [FILTERS.ERROR]: false,
+      [FILTERS.CSS]: true,
+      [FILTERS.NET]: true,
+      [FILTERS.NETXHR]: true,
+      [FILTERS.TEXT]: "foobar",
+    });
+    expect(ServicesMock.prefs.testHelpers.getFiltersPrefs()).toEqual({
+      [PREFS.FILTER.WARN]: true,
+      [PREFS.FILTER.LOG]: true,
+      [PREFS.FILTER.INFO]: true,
+      [PREFS.FILTER.DEBUG]: true,
+      [PREFS.FILTER.ERROR]: false,
+      [PREFS.FILTER.CSS]: true,
+      [PREFS.FILTER.NET]: true,
+      [PREFS.FILTER.NETXHR]: true,
     });
 
     store.dispatch(actions.filtersClear());
 
     filters = getAllFilters(store.getState());
     expect(filters.toJS()).toEqual({
-      "css": false,
-      "debug": true,
-      "error": true,
-      "info": true,
-      "log": true,
-      "net": false,
-      "netxhr": false,
-      "warn": true,
-      "text": "",
+      [FILTERS.CSS]: false,
+      [FILTERS.DEBUG]: true,
+      [FILTERS.ERROR]: true,
+      [FILTERS.INFO]: true,
+      [FILTERS.LOG]: true,
+      [FILTERS.NET]: false,
+      [FILTERS.NETXHR]: false,
+      [FILTERS.WARN]: true,
+      [FILTERS.TEXT]: "",
+    });
+
+    expect(ServicesMock.prefs.testHelpers.getFiltersPrefs()).toEqual({
+      [PREFS.FILTER.CSS]: false,
+      [PREFS.FILTER.DEBUG]: true,
+      [PREFS.FILTER.ERROR]: true,
+      [PREFS.FILTER.INFO]: true,
+      [PREFS.FILTER.LOG]: true,
+      [PREFS.FILTER.NET]: false,
+      [PREFS.FILTER.NETXHR]: false,
+      [PREFS.FILTER.WARN]: true,
     });
   });
 });
 
 describe("Resets filters", () => {
   it("resets default filters value to their original one.", () => {
     const store = setupStore([]);
 
@@ -254,43 +276,65 @@ describe("Resets filters", () => {
     store.dispatch(actions.filterToggle(FILTERS.CSS));
     store.dispatch(actions.filterToggle(FILTERS.NET));
     store.dispatch(actions.filterToggle(FILTERS.NETXHR));
     store.dispatch(actions.filterTextSet("foobar"));
 
     let filters = getAllFilters(store.getState());
     expect(filters.toJS()).toEqual({
       // default
-      "warn": true,
-      "info": true,
-      "debug": true,
+      [FILTERS.WARN]: true,
+      [FILTERS.INFO]: true,
+      [FILTERS.DEBUG]: true,
       // changed
-      "error": false,
-      "log": false,
-      "css": true,
-      "net": true,
-      "netxhr": true,
-      "text": "foobar",
+      [FILTERS.ERROR]: false,
+      [FILTERS.LOG]: false,
+      [FILTERS.CSS]: true,
+      [FILTERS.NET]: true,
+      [FILTERS.NETXHR]: true,
+      [FILTERS.TEXT]: "foobar",
+    });
+
+    expect(ServicesMock.prefs.testHelpers.getFiltersPrefs()).toEqual({
+      [PREFS.FILTER.WARN]: true,
+      [PREFS.FILTER.INFO]: true,
+      [PREFS.FILTER.DEBUG]: true,
+      [PREFS.FILTER.ERROR]: false,
+      [PREFS.FILTER.LOG]: false,
+      [PREFS.FILTER.CSS]: true,
+      [PREFS.FILTER.NET]: true,
+      [PREFS.FILTER.NETXHR]: true,
     });
 
     store.dispatch(actions.defaultFiltersReset());
 
     filters = getAllFilters(store.getState());
     expect(filters.toJS()).toEqual({
       // default
-      "error": true,
-      "warn": true,
-      "log": true,
-      "info": true,
-      "debug": true,
-      "text": "",
+      [FILTERS.ERROR]: true,
+      [FILTERS.WARN]: true,
+      [FILTERS.LOG]: true,
+      [FILTERS.INFO]: true,
+      [FILTERS.DEBUG]: true,
+      [FILTERS.TEXT]: "",
       // non-default filters weren't changed
-      "css": true,
-      "net": true,
-      "netxhr": true,
+      [FILTERS.CSS]: true,
+      [FILTERS.NET]: true,
+      [FILTERS.NETXHR]: true,
+    });
+
+    expect(ServicesMock.prefs.testHelpers.getFiltersPrefs()).toEqual({
+      [PREFS.FILTER.ERROR]: true,
+      [PREFS.FILTER.WARN]: true,
+      [PREFS.FILTER.LOG]: true,
+      [PREFS.FILTER.INFO]: true,
+      [PREFS.FILTER.DEBUG]: true,
+      [PREFS.FILTER.CSS]: true,
+      [PREFS.FILTER.NET]: true,
+      [PREFS.FILTER.NETXHR]: true,
     });
   });
 });
 
 function prepareBaseStore() {
   const store = setupStore([
     // Console API
     "console.log('foobar', 'test')",