From 3b62bc5f42f0240c605ebccfc888a3f65111beb8 Mon Sep 17 00:00:00 2001 From: Jonathan Pevarnek Date: Fri, 18 Dec 2015 11:24:26 -0500 Subject: [PATCH 1/2] Use merge when processing setting updates This removes the chance for human error when we add new fields to settings, also looks cleaner. No change in functionality. --- src/generic_core/uproxy_core.ts | 38 ++++++++++++++------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/src/generic_core/uproxy_core.ts b/src/generic_core/uproxy_core.ts index b682703f37..817b03f83c 100644 --- a/src/generic_core/uproxy_core.ts +++ b/src/generic_core/uproxy_core.ts @@ -197,22 +197,28 @@ export class uProxyCore implements uproxy_core_api.CoreApi { if (newSettings.stunServers.length === 0) { newSettings.stunServers = globals.DEFAULT_STUN_SERVERS; } + var oldDescription = globals.settings.description; globals.storage.save('globalSettings', newSettings) .catch((e) => { log.error('Could not save globalSettings to storage', e.stack); }); - // Clear the existing servers and add in each new server. - // Trying globalSettings = newSettings does not correctly update - // pre-existing references to stunServers (e.g. from RemoteInstances). - globals.settings.stunServers - .splice(0, globals.settings.stunServers.length); - for (var i = 0; i < newSettings.stunServers.length; ++i) { - globals.settings.stunServers.push(newSettings.stunServers[i]); - } + _.merge(globals.settings, newSettings, (a :Object, b :Object) => { + // ensure we do not merge the arrays and that the reference remains intact + if (_.isArray(a) && _.isArray(b)) { + var arrayA = a; + arrayA.splice(0, arrayA.length); + for (var i in b) { + arrayA.push((b)[i]); + } + return a; + } + + // this causes us to fall back to the default merge behaviour + return undefined; + }); - if (newSettings.description != globals.settings.description) { - globals.settings.description = newSettings.description; + if (globals.settings.description !== oldDescription) { // Resend instance info to update description for logged in networks. for (var networkName in social_network.networks) { for (var userId in social_network.networks[networkName]) { @@ -221,21 +227,9 @@ export class uProxyCore implements uproxy_core_api.CoreApi { } } - globals.settings.hasSeenSharingEnabledScreen = - newSettings.hasSeenSharingEnabledScreen; - globals.settings.hasSeenWelcome = newSettings.hasSeenWelcome; - globals.settings.allowNonUnicast = newSettings.allowNonUnicast; - globals.settings.mode = newSettings.mode; - globals.settings.statsReportingEnabled = newSettings.statsReportingEnabled; - globals.settings.splashState = newSettings.splashState; - globals.settings.consoleFilter = newSettings.consoleFilter; loggingController.setDefaultFilter( loggingTypes.Destination.console, globals.settings.consoleFilter); - globals.settings.language = newSettings.language; - globals.settings.force_message_version = newSettings.force_message_version; - globals.settings.quiverUserName = newSettings.quiverUserName; - globals.settings.showCloud = newSettings.showCloud; } public getFullState = () :Promise => { From 76922c34360c5a7f94108164d23b0ac6f8e6d37e Mon Sep 17 00:00:00 2001 From: Jonathan Pevarnek Date: Fri, 18 Dec 2015 11:26:33 -0500 Subject: [PATCH 2/2] Add a new field, hasSeenMetrics, use it to keep track of that Right now we were using hasSeenWelcome to determine whether the user had seen the welcome screen and also refer to it when deciding whether to show the screen for opting-in to metrics. This caused you to see it multiple times when hitting back during the first run. This fixes that. --- src/generic_core/globals.ts | 1 + src/generic_ui/polymer/splash.ts | 16 +++++++++------- src/generic_ui/scripts/ui.ts | 1 + src/integration/core.spec.ts | 1 + src/interfaces/uproxy_core_api.ts | 1 + 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/generic_core/globals.ts b/src/generic_core/globals.ts index bad86b8b78..b41495d4a3 100644 --- a/src/generic_core/globals.ts +++ b/src/generic_core/globals.ts @@ -35,6 +35,7 @@ export var settings :uproxy_core_api.GlobalSettings = { stunServers: DEFAULT_STUN_SERVERS.slice(0), hasSeenSharingEnabledScreen: false, hasSeenWelcome: false, + hasSeenMetrics: false, allowNonUnicast: false, mode: user_interface.Mode.GET, version: STORAGE_VERSION, diff --git a/src/generic_ui/polymer/splash.ts b/src/generic_ui/polymer/splash.ts index 6aee62af3f..e34db2a794 100644 --- a/src/generic_ui/polymer/splash.ts +++ b/src/generic_ui/polymer/splash.ts @@ -40,7 +40,7 @@ Polymer({ if (this.supportsQuiver && model.globalSettings.splashState == this.SPLASH_STATES.METRICS_OPT_IN) { ui.view = ui_constants.View.ROSTER; - } else if (model.globalSettings.hasSeenWelcome && + } else if (model.globalSettings.hasSeenMetrics && model.globalSettings.splashState == this.SPLASH_STATES.INTRO) { // Skip metrics opt-in if we've seen it before. this.setState(this.SPLASH_STATES.NETWORKS); @@ -49,7 +49,7 @@ Polymer({ } }, prev: function() { - if (model.globalSettings.hasSeenWelcome && + if (model.globalSettings.hasSeenMetrics && model.globalSettings.splashState == this.SPLASH_STATES.NETWORKS) { // Skip metrics opt-in if we've seen it before. this.setState(this.SPLASH_STATES.INTRO); @@ -100,15 +100,17 @@ Polymer({ }); this.supportsQuiver = model.hasQuiverSupport(); }, - enableStats: function() { - model.globalSettings.statsReportingEnabled = true; + updateSeenMetrics: function(val :Boolean) { + model.globalSettings.hasSeenMetrics = true; + model.globalSettings.statsReportingEnabled = val; core.updateGlobalSettings(model.globalSettings); this.next(); }, + enableStats: function() { + return this.updateSeenMetrics(true); + }, disableStats: function() { - model.globalSettings.statsReportingEnabled = false; - core.updateGlobalSettings(model.globalSettings); - this.next(); + return this.updateSeenMetrics(false); }, observe: { 'model.networkNames': 'updateNetworkButtonNames' diff --git a/src/generic_ui/scripts/ui.ts b/src/generic_ui/scripts/ui.ts index e4bc650899..aac803ca3f 100644 --- a/src/generic_ui/scripts/ui.ts +++ b/src/generic_ui/scripts/ui.ts @@ -56,6 +56,7 @@ export class Model { description: '', stunServers: [], hasSeenSharingEnabledScreen: false, + hasSeenMetrics: false, hasSeenWelcome: false, splashState : 0, mode : ui_constants.Mode.GET, diff --git a/src/integration/core.spec.ts b/src/integration/core.spec.ts index 291e13df12..c571bf2748 100644 --- a/src/integration/core.spec.ts +++ b/src/integration/core.spec.ts @@ -98,6 +98,7 @@ describe('uproxy core', function() { stunServers: [], hasSeenSharingEnabledScreen: true, hasSeenWelcome: false, + hasSeenMetrics: false, allowNonUnicast: true, statsReportingEnabled: false }; diff --git a/src/interfaces/uproxy_core_api.ts b/src/interfaces/uproxy_core_api.ts index 1a482dd0a0..1d5a6eb98f 100644 --- a/src/interfaces/uproxy_core_api.ts +++ b/src/interfaces/uproxy_core_api.ts @@ -28,6 +28,7 @@ export interface GlobalSettings { stunServers :freedom.RTCPeerConnection.RTCIceServer[]; hasSeenSharingEnabledScreen :boolean; hasSeenWelcome :boolean; + hasSeenMetrics :boolean; allowNonUnicast :boolean; mode :ui.Mode; statsReportingEnabled :boolean;