Bug 1409836 - Fix setting filters prefs when filters are reset or cleared. r=bgrins, a=ritu
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 19 Oct 2017 10:07:54 +0200
changeset 435153 4b4ff6b8f353329e0f8573a0d75b84b704ca7077
parent 435152 3a3c0ded951b68efef19d7ce6e83656b34d09011
child 435154 fd6134823ff9c90421e7cd8c6b9ff19aac43f2a0
push id1567
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 12:36:05 +0000
treeherdermozilla-release@e512c14a0406 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins, ritu
bugs1409836
milestone57.0
Bug 1409836 - Fix setting filters prefs when filters are reset or cleared. r=bgrins, a=ritu 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/filter-button");
 const FilterBar = createFactory(require("devtools/client/webconsole/new-console-output/components/filter-bar"));
 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')",