From f33b666a199a6f10cdfb542ddd04ee3dfc58ff99 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 23 Jun 2016 10:25:06 -0700 Subject: [PATCH 1/4] Fix Never Check News Option Bug Not honoring never setting for Survey news options Fix Make sure we do honor this option and never automatically display survey news when enabled. Testing Manual testing by clearing out cache and hitting this same code path multiple times. --- Nodejs/Product/Nodejs/NodejsPackage.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Nodejs/Product/Nodejs/NodejsPackage.cs b/Nodejs/Product/Nodejs/NodejsPackage.cs index 332df7a94..a5fb7e1ef 100644 --- a/Nodejs/Product/Nodejs/NodejsPackage.cs +++ b/Nodejs/Product/Nodejs/NodejsPackage.cs @@ -599,6 +599,7 @@ internal void CheckSurveyNews(bool forceCheckAndWarnIfNoneAvailable) { var elapsedTime = DateTime.Now - options.SurveyNewsLastCheck; switch (options.SurveyNewsCheck) { case SurveyNewsPolicy.Disabled: + shouldQueryServer = false; break; case SurveyNewsPolicy.CheckOnceDay: shouldQueryServer = elapsedTime.TotalDays >= 1; From 47bbafb922cb5009ccce74bf4ea4ce40b5b7ed60 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 23 Jun 2016 10:57:01 -0700 Subject: [PATCH 2/4] Dont show on unknown --- Nodejs/Product/Nodejs/NodejsPackage.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Nodejs/Product/Nodejs/NodejsPackage.cs b/Nodejs/Product/Nodejs/NodejsPackage.cs index a5fb7e1ef..52c97d6ad 100644 --- a/Nodejs/Product/Nodejs/NodejsPackage.cs +++ b/Nodejs/Product/Nodejs/NodejsPackage.cs @@ -586,7 +586,6 @@ internal void CheckSurveyNews(bool forceCheckAndWarnIfNoneAvailable) { if (forceCheckAndWarnIfNoneAvailable) { shouldQueryServer = true; } else { - shouldQueryServer = true; var options = GeneralOptionsPage; // Ensure that we don't prompt the user on their very first project creation. // Delay by 3 days by pretending we checked 4 days ago (the default of check @@ -612,6 +611,7 @@ internal void CheckSurveyNews(bool forceCheckAndWarnIfNoneAvailable) { break; default: Debug.Assert(false, String.Format("Unexpected SurveyNewsPolicy: {0}.", options.SurveyNewsCheck)); + shouldQueryServer = false; break; } } From 7505637cb2845231ab59b86e8dfb90088af20c88 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 23 Jun 2016 11:05:41 -0700 Subject: [PATCH 3/4] Extract method to make code more readable --- Nodejs/Product/Nodejs/NodejsPackage.cs | 64 ++++++++++++-------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/Nodejs/Product/Nodejs/NodejsPackage.cs b/Nodejs/Product/Nodejs/NodejsPackage.cs index 52c97d6ad..7dda36a78 100644 --- a/Nodejs/Product/Nodejs/NodejsPackage.cs +++ b/Nodejs/Product/Nodejs/NodejsPackage.cs @@ -582,45 +582,41 @@ private void OnSurveyNewsDocumentCompleted(object sender, WebBrowserDocumentComp } internal void CheckSurveyNews(bool forceCheckAndWarnIfNoneAvailable) { - bool shouldQueryServer = false; - if (forceCheckAndWarnIfNoneAvailable) { - shouldQueryServer = true; - } else { + if (ShouldQuerySurveryNewsServer(forceCheckAndWarnIfNoneAvailable)) { var options = GeneralOptionsPage; - // Ensure that we don't prompt the user on their very first project creation. - // Delay by 3 days by pretending we checked 4 days ago (the default of check - // once a week ensures we'll check again in 3 days). - if (options.SurveyNewsLastCheck == DateTime.MinValue) { - options.SurveyNewsLastCheck = DateTime.Now - TimeSpan.FromDays(4); - options.SaveSettingsToStorage(); - } + options.SurveyNewsLastCheck = DateTime.Now; + options.SaveSettingsToStorage(); + CheckSurveyNewsThread(new Uri(options.SurveyNewsFeedUrl), forceCheckAndWarnIfNoneAvailable); + } + } - var elapsedTime = DateTime.Now - options.SurveyNewsLastCheck; - switch (options.SurveyNewsCheck) { - case SurveyNewsPolicy.Disabled: - shouldQueryServer = false; - break; - case SurveyNewsPolicy.CheckOnceDay: - shouldQueryServer = elapsedTime.TotalDays >= 1; - break; - case SurveyNewsPolicy.CheckOnceWeek: - shouldQueryServer = elapsedTime.TotalDays >= 7; - break; - case SurveyNewsPolicy.CheckOnceMonth: - shouldQueryServer = elapsedTime.TotalDays >= 30; - break; - default: - Debug.Assert(false, String.Format("Unexpected SurveyNewsPolicy: {0}.", options.SurveyNewsCheck)); - shouldQueryServer = false; - break; - } + private bool ShouldQuerySurveryNewsServer(bool forceCheckAndWarnIfNoneAvailable) { + if (forceCheckAndWarnIfNoneAvailable) { + return true; } - if (shouldQueryServer) { - var options = GeneralOptionsPage; - options.SurveyNewsLastCheck = DateTime.Now; + var options = GeneralOptionsPage; + // Ensure that we don't prompt the user on their very first project creation. + // Delay by 3 days by pretending we checked 4 days ago (the default of check + // once a week ensures we'll check again in 3 days). + if (options.SurveyNewsLastCheck == DateTime.MinValue) { + options.SurveyNewsLastCheck = DateTime.Now - TimeSpan.FromDays(4); options.SaveSettingsToStorage(); - CheckSurveyNewsThread(new Uri(options.SurveyNewsFeedUrl), forceCheckAndWarnIfNoneAvailable); + } + + var elapsedTime = DateTime.Now - options.SurveyNewsLastCheck; + switch (options.SurveyNewsCheck) { + case SurveyNewsPolicy.Disabled: + return false; + case SurveyNewsPolicy.CheckOnceDay: + return elapsedTime.TotalDays >= 1; + case SurveyNewsPolicy.CheckOnceWeek: + return elapsedTime.TotalDays >= 7; + case SurveyNewsPolicy.CheckOnceMonth: + return elapsedTime.TotalDays >= 30; + default: + Debug.Assert(false, String.Format("Unexpected SurveyNewsPolicy: {0}.", options.SurveyNewsCheck)); + return false; } } From 987e9e9d8792603f1dd62c76c60102cd40eb0b96 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 23 Jun 2016 11:07:29 -0700 Subject: [PATCH 4/4] Small cleanup to remove parameter --- Nodejs/Product/Nodejs/NodejsPackage.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Nodejs/Product/Nodejs/NodejsPackage.cs b/Nodejs/Product/Nodejs/NodejsPackage.cs index 7dda36a78..1ecfee526 100644 --- a/Nodejs/Product/Nodejs/NodejsPackage.cs +++ b/Nodejs/Product/Nodejs/NodejsPackage.cs @@ -582,7 +582,7 @@ private void OnSurveyNewsDocumentCompleted(object sender, WebBrowserDocumentComp } internal void CheckSurveyNews(bool forceCheckAndWarnIfNoneAvailable) { - if (ShouldQuerySurveryNewsServer(forceCheckAndWarnIfNoneAvailable)) { + if (forceCheckAndWarnIfNoneAvailable || ShouldQuerySurveryNewsServer()) { var options = GeneralOptionsPage; options.SurveyNewsLastCheck = DateTime.Now; options.SaveSettingsToStorage(); @@ -590,11 +590,7 @@ internal void CheckSurveyNews(bool forceCheckAndWarnIfNoneAvailable) { } } - private bool ShouldQuerySurveryNewsServer(bool forceCheckAndWarnIfNoneAvailable) { - if (forceCheckAndWarnIfNoneAvailable) { - return true; - } - + private bool ShouldQuerySurveryNewsServer() { var options = GeneralOptionsPage; // Ensure that we don't prompt the user on their very first project creation. // Delay by 3 days by pretending we checked 4 days ago (the default of check