From 77dbb05556d755ba2e8da267301f96803c93d7b7 Mon Sep 17 00:00:00 2001
From: Tyler Crammond <tylerchc92@gmail.com>
Date: Fri, 22 Mar 2019 12:34:19 +0000
Subject: [PATCH] fix(setting): Ensure default settings are loaded in case of
 invalid values

---
 src/scripts/lib/bugsnag.js |  2 +-
 src/scripts/lib/db.js      | 55 ++++++++++++++++++++------------------
 src/scripts/settings.js    |  4 +--
 3 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/src/scripts/lib/bugsnag.js b/src/scripts/lib/bugsnag.js
index 3c4870571..c554e7b3d 100644
--- a/src/scripts/lib/bugsnag.js
+++ b/src/scripts/lib/bugsnag.js
@@ -25,7 +25,7 @@ function getBugsnagClient () {
     collectUserIp: false,
     beforeSend: async function (report) {
       const db = browser.extension.getBackgroundPage().db;
-      const sendErrorReports = await db.get('sendErrorReports', true);
+      const sendErrorReports = await db.get('sendErrorReports');
       if (!sendErrorReports) {
         report.ignore();
         return false;
diff --git a/src/scripts/lib/db.js b/src/scripts/lib/db.js
index 4b17f93b0..056ebd6fe 100644
--- a/src/scripts/lib/db.js
+++ b/src/scripts/lib/db.js
@@ -24,18 +24,14 @@ const DEFAULT_SETTINGS = {
   dayEndTime: '17:00',
   defaultProject: 0,
   rememberProjectPer: 'false',
-  enableAutoTagging: false
-};
+  enableAutoTagging: false,
 
-// core settings: key, default value
-const CORE_SETTINGS = {
   'dont-show-permissions': false,
   'show-permissions-info': 0,
   'settings-active-tab': 0,
   sendErrorReports: true,
   sendUsageStatistics: true
 };
-
 const transformLegacyValue = (value) => {
   if (typeof value !== 'undefined') {
     // Ensure older version's settings still function if they get saved to sync storage.
@@ -153,14 +149,27 @@ export default class Db {
     this.set(this.togglButton.$user.id + '-defaultProjects', {});
   }
 
-  get (key, defaultValue) {
+  get (key) {
+    const defaultValue = DEFAULT_SETTINGS[key];
     const hasDefaultValue = typeof defaultValue !== 'undefined';
-    return browser.storage.sync.get(hasDefaultValue ? { [key]: defaultValue } : key)
+    const options = hasDefaultValue
+      ? { [key]: defaultValue }
+      : key;
+
+    return browser.storage.sync.get(options)
       .then((result) => {
+        let value = result[key];
+        if (hasDefaultValue && typeof value !== typeof defaultValue) {
+          if (process.env.DEBUG) {
+            console.info(`Retrieved setting [${key}] is incorrect type`, value);
+          }
+          value = defaultValue;
+        }
+
         if (process.env.DEBUG) {
-          console.info(`Retrieved value ${key}: `, result[key]);
+          console.info(`Retrieved value ${key}: `, value);
         }
-        return transformLegacyValue(result[key]);
+        return transformLegacyValue(value);
       });
   }
 
@@ -173,7 +182,7 @@ export default class Db {
     return browser.storage.sync.get(settings)
       .then((result) => {
         if (process.env.DEBUG) {
-          console.info(`Retrieved values ${Object.keys(settings).join(', ')}: `, Object.values(result).map(JSON.strinfiy).join(', '));
+          console.info(`Retrieved values ${Object.keys(settings).join(', ')}: `, Object.values(result).map(JSON.stringify).join(', '));
         }
         return Object.keys(result).reduce((results, key) => {
           return Object.assign(results, {
@@ -220,14 +229,14 @@ export default class Db {
     return collection;
   }
 
-  async load (setting, defaultValue) {
-    let value = await this.get(setting);
-    if (typeof value === 'undefined') {
-      value = defaultValue;
-    }
-
-    this.set(setting, value);
-    return value;
+  load (setting, defaultValue) {
+    this.get(setting)
+      .then((value) => {
+        if (typeof value === 'undefined') {
+          value = defaultValue;
+        }
+        this.set(setting, value);
+      });
   }
 
   loadAll () {
@@ -236,12 +245,6 @@ export default class Db {
         this.load(k, DEFAULT_SETTINGS[k]);
       }
     }
-
-    for (const k in CORE_SETTINGS) {
-      if (CORE_SETTINGS.hasOwnProperty(k)) {
-        this.load(k, CORE_SETTINGS[k]);
-      }
-    }
   }
 
   updateSetting (key, state, callback, condition) {
@@ -254,7 +257,7 @@ export default class Db {
   }
 
   resetAllSettings () {
-    const allSettings = { ...DEFAULT_SETTINGS, ...CORE_SETTINGS };
+    const allSettings = { ...DEFAULT_SETTINGS };
     return this.setMultiple(allSettings)
       .then(() => {
         bugsnagClient.leaveBreadcrumb('Completed reset all settings');
@@ -270,7 +273,7 @@ export default class Db {
     bugsnagClient.leaveBreadcrumb('Attempting settings migration to v2');
 
     try {
-      const allSettings = { ...DEFAULT_SETTINGS, ...CORE_SETTINGS };
+      const allSettings = { ...DEFAULT_SETTINGS };
       const oldSettings = Object.keys(allSettings)
         .reduce((accumulator, key) => {
           const defaultValue = allSettings[key];
diff --git a/src/scripts/settings.js b/src/scripts/settings.js
index 25af96192..9bbfb3c11 100644
--- a/src/scripts/settings.js
+++ b/src/scripts/settings.js
@@ -631,7 +631,7 @@ document.addEventListener('DOMContentLoaded', async function (e) {
     if (
       !dontShowPermissions
     ) {
-      const showPermissionsInfo = await db.get('show-permissions-info', 0);
+      const showPermissionsInfo = await db.get('show-permissions-info');
       document.querySelector('.guide-container').style.display = 'flex';
       document.querySelector(
         ".guide > div[data-id='" + (showPermissionsInfo || 0) + "']"
@@ -644,7 +644,7 @@ document.addEventListener('DOMContentLoaded', async function (e) {
     }
 
     // Change active tab.
-    const settingsActiveTab = await db.get('settings-active-tab');
+    const settingsActiveTab = await db.get('settings-active-tab', 0);
     const activeTab = Number.parseInt(settingsActiveTab, 10);
     changeActiveTab(activeTab);
     document.querySelector('body').style.display = 'block';