From 16e9ee177fbbfb9220ef13e61c8101905d6fa0c5 Mon Sep 17 00:00:00 2001 From: Kristen Schau <47155823+krschau@users.noreply.github.com> Date: Tue, 7 Nov 2023 13:04:24 -0500 Subject: [PATCH 01/13] Spelling and usings cleanup (#261) --- .editorconfig | 4 ++++ exclusion.dic | 1 + src/GitHubExtension/Client/Validation.cs | 2 +- .../DataManager/DataStoreOperationParameters.cs | 1 - src/GitHubExtension/DataManager/GitHubDataManager.cs | 2 +- src/GitHubExtension/DataManager/IGitHubSearchManager.cs | 1 - src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs | 6 +++--- src/GitHubExtension/DeveloperId/LoginUIController.cs | 2 +- src/GitHubExtension/Helpers/EnumHelper.cs | 3 +-- src/GitHubExtension/Helpers/FileHelper.cs | 1 - src/GitHubExtension/Helpers/Json.cs | 1 - src/GitHubExtension/Helpers/LocalSettings.cs | 4 ---- src/GitHubExtension/Helpers/Resources.cs | 2 +- src/GitHubExtension/Helpers/TimeSpanHelper.cs | 2 +- src/GitHubExtension/Providers/DevHomeRepository.cs | 6 +----- src/GitHubExtension/Providers/RepositoryProvider.cs | 6 ++---- src/GitHubExtension/Providers/SettingsUIController.cs | 1 - src/GitHubExtension/Strings/en-US/Resources.resw | 2 +- src/GitHubExtension/Widgets/GitHubWidget.cs | 2 +- 19 files changed, 19 insertions(+), 30 deletions(-) create mode 100644 exclusion.dic diff --git a/.editorconfig b/.editorconfig index 2a341ece..ecb4b867 100644 --- a/.editorconfig +++ b/.editorconfig @@ -200,3 +200,7 @@ dotnet_style_prefer_inferred_tuple_names = true:suggestion dotnet_style_prefer_inferred_anonymous_type_member_names = true:suggestion dotnet_style_prefer_compound_assignment = true:warning dotnet_style_prefer_simplified_interpolation = true:suggestion + +# Spelling + +spelling_exclusion_path = .\exclusion.dic \ No newline at end of file diff --git a/exclusion.dic b/exclusion.dic new file mode 100644 index 00000000..d0e22042 --- /dev/null +++ b/exclusion.dic @@ -0,0 +1 @@ +devhome diff --git a/src/GitHubExtension/Client/Validation.cs b/src/GitHubExtension/Client/Validation.cs index c8c8e689..fefa5275 100644 --- a/src/GitHubExtension/Client/Validation.cs +++ b/src/GitHubExtension/Client/Validation.cs @@ -18,7 +18,7 @@ public static bool IsValidGitHubURL(Uri uri) // Valid GitHub URL has three segments. The first is '/'. if (uri.Segments.Length < 3 || (!uri.Host.Equals("github.com", StringComparison.OrdinalIgnoreCase) && !uri.Host.Equals("www.github.com", StringComparison.OrdinalIgnoreCase))) { - Log.Logger()?.ReportDebug($"{uri.OriginalString} is not a valid github uri"); + Log.Logger()?.ReportDebug($"{uri.OriginalString} is not a valid GitHub uri"); return false; } diff --git a/src/GitHubExtension/DataManager/DataStoreOperationParameters.cs b/src/GitHubExtension/DataManager/DataStoreOperationParameters.cs index e8e96613..07f6b5a1 100644 --- a/src/GitHubExtension/DataManager/DataStoreOperationParameters.cs +++ b/src/GitHubExtension/DataManager/DataStoreOperationParameters.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation and Contributors // Licensed under the MIT license. -using GitHubExtension.DeveloperId; using Microsoft.Windows.DevHome.SDK; namespace GitHubExtension; diff --git a/src/GitHubExtension/DataManager/GitHubDataManager.cs b/src/GitHubExtension/DataManager/GitHubDataManager.cs index bb10000b..5386662e 100644 --- a/src/GitHubExtension/DataManager/GitHubDataManager.cs +++ b/src/GitHubExtension/DataManager/GitHubDataManager.cs @@ -622,7 +622,7 @@ private void SetLastUpdatedInMetaData() MetaData.AddOrUpdate(DataStore, LastUpdatedKeyName, DateTime.Now.ToDataStoreString()); } - // Converts fullname -> owner, name. + // Converts fullName -> owner, name. private string[] GetOwnerAndRepositoryNameFromFullName(string fullName) { var nameSplit = fullName.Split(new[] { '/' }); diff --git a/src/GitHubExtension/DataManager/IGitHubSearchManager.cs b/src/GitHubExtension/DataManager/IGitHubSearchManager.cs index 4cb39a65..ebf345e2 100644 --- a/src/GitHubExtension/DataManager/IGitHubSearchManager.cs +++ b/src/GitHubExtension/DataManager/IGitHubSearchManager.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. using GitHubExtension.DataManager; -using GitHubExtension.DataModel; namespace GitHubExtension; diff --git a/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs b/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs index 7a2a4bd3..94c56427 100644 --- a/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs +++ b/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs @@ -146,7 +146,7 @@ public ProviderOperationResult LogoutDeveloperId(IDeveloperId developerId) } catch (Exception error) { - Log.Logger()?.ReportError($"LoggedOut event signalling failed: {error}"); + Log.Logger()?.ReportError($"LoggedOut event signaling failed: {error}"); } return new ProviderOperationResult(ProviderOperationStatus.Success, null, "The developer account has been logged out successfully", "LogoutDeveloperId succeeded"); @@ -228,7 +228,7 @@ private DeveloperId CreateOrUpdateDeveloperId(OAuthRequest oauthRequest) } catch (Exception error) { - Log.Logger()?.ReportError($"Updated event signalling failed: {error}"); + Log.Logger()?.ReportError($"Updated event signaling failed: {error}"); } } catch (InvalidOperationException) @@ -252,7 +252,7 @@ private DeveloperId CreateOrUpdateDeveloperId(OAuthRequest oauthRequest) } catch (Exception error) { - Log.Logger()?.ReportError($"LoggedIn event signalling failed: {error}"); + Log.Logger()?.ReportError($"LoggedIn event signaling failed: {error}"); } } diff --git a/src/GitHubExtension/DeveloperId/LoginUIController.cs b/src/GitHubExtension/DeveloperId/LoginUIController.cs index 9d097996..7dab5556 100644 --- a/src/GitHubExtension/DeveloperId/LoginUIController.cs +++ b/src/GitHubExtension/DeveloperId/LoginUIController.cs @@ -54,7 +54,7 @@ public IAsyncOperation OnAction(string action, string i if (devId != null) { var resourceLoader = new ResourceLoader(ResourceLoader.GetDefaultResourceFilePath(), "GitHubExtension/Resources"); - operationResult = _loginUI.Update(_loginUITemplate.GetLoginUITemplate(LoginUIState.LoginSucceededPage).Replace("${message}", $"{devId.LoginId} {resourceLoader.GetString("LoginUI_LoginSuccededPage_text")}"), null, LoginUIState.LoginSucceededPage); + operationResult = _loginUI.Update(_loginUITemplate.GetLoginUITemplate(LoginUIState.LoginSucceededPage).Replace("${message}", $"{devId.LoginId} {resourceLoader.GetString("LoginUI_LoginSucceededPage_text")}"), null, LoginUIState.LoginSucceededPage); } else { diff --git a/src/GitHubExtension/Helpers/EnumHelper.cs b/src/GitHubExtension/Helpers/EnumHelper.cs index b5490ab3..ab8d3c9f 100644 --- a/src/GitHubExtension/Helpers/EnumHelper.cs +++ b/src/GitHubExtension/Helpers/EnumHelper.cs @@ -1,8 +1,7 @@ // Copyright (c) Microsoft Corporation and Contributors // Licensed under the MIT license. -using System.Globalization; + using GitHubExtension.DataManager; -using GitHubExtension.Widgets.Enums; namespace GitHubExtension.Helpers; public class EnumHelper diff --git a/src/GitHubExtension/Helpers/FileHelper.cs b/src/GitHubExtension/Helpers/FileHelper.cs index 929ed59b..a2f16333 100644 --- a/src/GitHubExtension/Helpers/FileHelper.cs +++ b/src/GitHubExtension/Helpers/FileHelper.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation and Contributors // Licensed under the MIT license. -using System.IO; using System.Text; using Newtonsoft.Json; diff --git a/src/GitHubExtension/Helpers/Json.cs b/src/GitHubExtension/Helpers/Json.cs index a1080c20..8e2fa183 100644 --- a/src/GitHubExtension/Helpers/Json.cs +++ b/src/GitHubExtension/Helpers/Json.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation and Contributors // Licensed under the MIT license. -using System.Threading.Tasks; using Newtonsoft.Json; namespace GitHubExtension.Helpers; diff --git a/src/GitHubExtension/Helpers/LocalSettings.cs b/src/GitHubExtension/Helpers/LocalSettings.cs index c35ccc3a..9aabb7bf 100644 --- a/src/GitHubExtension/Helpers/LocalSettings.cs +++ b/src/GitHubExtension/Helpers/LocalSettings.cs @@ -1,11 +1,7 @@ // Copyright (c) Microsoft Corporation and Contributors // Licensed under the MIT license. -using System.Runtime.CompilerServices; using DevHome.Common.Services; -using DevHome.Logging; -using Microsoft.Windows.ApplicationModel.Resources; -using Newtonsoft.Json.Linq; using Windows.Storage; namespace GitHubExtension.Helpers; diff --git a/src/GitHubExtension/Helpers/Resources.cs b/src/GitHubExtension/Helpers/Resources.cs index 7cce66d2..a2e52a33 100644 --- a/src/GitHubExtension/Helpers/Resources.cs +++ b/src/GitHubExtension/Helpers/Resources.cs @@ -32,7 +32,7 @@ public static string GetResource(string identifier, Logger? log = null) // Replaces all identifiers in the provided list in the target string. Assumes all identifiers // are wrapped with '%' to prevent sub-string replacement errors. This is intended for strings // such as a JSON string with resource identifiers embedded. - public static string ReplaceIdentifers(string str, string[] resourceIdentifiers, Logger? log = null) + public static string ReplaceIdentifiers(string str, string[] resourceIdentifiers, Logger? log = null) { var start = DateTime.Now; foreach (var identifier in resourceIdentifiers) diff --git a/src/GitHubExtension/Helpers/TimeSpanHelper.cs b/src/GitHubExtension/Helpers/TimeSpanHelper.cs index 991fd4c8..fe6f8db6 100644 --- a/src/GitHubExtension/Helpers/TimeSpanHelper.cs +++ b/src/GitHubExtension/Helpers/TimeSpanHelper.cs @@ -1,6 +1,6 @@ // Copyright (c) Microsoft Corporation and Contributors // Licensed under the MIT license. -using System.Text.Json.Nodes; + using DevHome.Logging; using Jeffijoe.MessageFormat; diff --git a/src/GitHubExtension/Providers/DevHomeRepository.cs b/src/GitHubExtension/Providers/DevHomeRepository.cs index 868e9609..bc13040c 100644 --- a/src/GitHubExtension/Providers/DevHomeRepository.cs +++ b/src/GitHubExtension/Providers/DevHomeRepository.cs @@ -2,10 +2,6 @@ // Licensed under the MIT license. using GitHubExtension.Client; -using GitHubExtension.DeveloperId; -using LibGit2Sharp; -using Microsoft.Windows.DevHome.SDK; -using Windows.Foundation; namespace GitHubExtension.Providers; @@ -33,7 +29,7 @@ public class DevHomeRepository : Microsoft.Windows.DevHome.SDK.IRepository /// /// Initializes a new instance of the class. /// - /// The repository recived from ocktokit + /// The repository received from ocktokit public DevHomeRepository(Octokit.Repository ocktokitRepository) { this.name = ocktokitRepository.Name; diff --git a/src/GitHubExtension/Providers/RepositoryProvider.cs b/src/GitHubExtension/Providers/RepositoryProvider.cs index 8c929a42..b90627af 100644 --- a/src/GitHubExtension/Providers/RepositoryProvider.cs +++ b/src/GitHubExtension/Providers/RepositoryProvider.cs @@ -1,14 +1,12 @@ // Copyright (c) Microsoft Corporation and Contributors // Licensed under the MIT license. -using System.Xml.Linq; using GitHubExtension.Client; using GitHubExtension.DeveloperId; using GitHubExtension.Helpers; using Microsoft.Windows.DevHome.SDK; using Octokit; using Windows.Foundation; -using Windows.Storage; using Windows.Storage.Streams; namespace GitHubExtension.Providers; @@ -228,8 +226,8 @@ public IAsyncOperation CloneRepositoryAsync(IRepository } catch (LibGit2Sharp.UserCancelledException userCancelledException) { - Providers.Log.Logger()?.ReportError("DevHomeRepository", "The user stoped the clone operation", userCancelledException); - return new ProviderOperationResult(ProviderOperationStatus.Failure, userCancelledException, "User cancalled the operation", userCancelledException.Message); + Providers.Log.Logger()?.ReportError("DevHomeRepository", "The user stopped the clone operation", userCancelledException); + return new ProviderOperationResult(ProviderOperationStatus.Failure, userCancelledException, "User cancelled the operation", userCancelledException.Message); } catch (LibGit2Sharp.NameConflictException nameConflictException) { diff --git a/src/GitHubExtension/Providers/SettingsUIController.cs b/src/GitHubExtension/Providers/SettingsUIController.cs index 9a0f5f84..98904cec 100644 --- a/src/GitHubExtension/Providers/SettingsUIController.cs +++ b/src/GitHubExtension/Providers/SettingsUIController.cs @@ -4,7 +4,6 @@ using GitHubExtension.Helpers; using Microsoft.Windows.ApplicationModel.Resources; using Microsoft.Windows.DevHome.SDK; -using Newtonsoft.Json.Linq; using Windows.Foundation; namespace GitHubExtension.Providers; diff --git a/src/GitHubExtension/Strings/en-US/Resources.resw b/src/GitHubExtension/Strings/en-US/Resources.resw index 4a3d8bba..96a17bfc 100644 --- a/src/GitHubExtension/Strings/en-US/Resources.resw +++ b/src/GitHubExtension/Strings/en-US/Resources.resw @@ -241,7 +241,7 @@ Sign in - + has logged in successfully! You may close this window. diff --git a/src/GitHubExtension/Widgets/GitHubWidget.cs b/src/GitHubExtension/Widgets/GitHubWidget.cs index 2c18b1cd..ae66d9a0 100644 --- a/src/GitHubExtension/Widgets/GitHubWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubWidget.cs @@ -341,7 +341,7 @@ protected string GetTemplateForPage(WidgetPageState page) { var path = Path.Combine(AppContext.BaseDirectory, GetTemplatePath(page)); var template = File.ReadAllText(path, Encoding.Default) ?? throw new FileNotFoundException(path); - template = Resources.ReplaceIdentifers(template, Resources.GetWidgetResourceIdentifiers(), Log.Logger()); + template = Resources.ReplaceIdentifiers(template, Resources.GetWidgetResourceIdentifiers(), Log.Logger()); Log.Logger()?.ReportDebug(Name, ShortId, $"Caching template for {page}"); Template[page] = template; return template; From 9ead3b9e91af224bc8e77776d54f20fe82e29227 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 7 Nov 2023 11:04:42 -0800 Subject: [PATCH 02/13] Fix Stdout tearing in debug console (#260) --- src/Logging/listeners/StdoutListener.cs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/Logging/listeners/StdoutListener.cs b/src/Logging/listeners/StdoutListener.cs index ccada394..3a9ab4e9 100644 --- a/src/Logging/listeners/StdoutListener.cs +++ b/src/Logging/listeners/StdoutListener.cs @@ -16,6 +16,10 @@ public class StdoutListener : ListenerBase private static readonly ConsoleColor CElapsedColor = ConsoleColor.Green; private static readonly ConsoleColor CSourceColor = ConsoleColor.Cyan; + // Static lock object so different instances of the Stdout listener do not simultaneously write + // to stdout and have interleaved tearing of messages. + private static readonly object _stdoutLock = new (); + public StdoutListener(string name) : base(name) { @@ -65,9 +69,19 @@ private void ConsoleHandleLogEvent(LogEvent evt, bool newline, TimeSpan elapsed) line.Add(Tuple.Create(CDefaultColor, Environment.NewLine)); } - WriteColor(line); - Console.ResetColor(); - Console.Out.Flush(); + // Do this in a static lock to prevent tearing. + lock (_stdoutLock) + { + try + { + WriteColor(line); + Console.ResetColor(); + Console.Out.Flush(); + } + catch + { + } + } } private bool MeetsFilter(LogEvent evt) From f38c724615ea716c4119f2d3d47b9ecc387c4ce5 Mon Sep 17 00:00:00 2001 From: Elmar Jansen Date: Tue, 7 Nov 2023 20:05:37 +0100 Subject: [PATCH 03/13] Corrected spelling (and casing) of error messages (#259) * Corrected spelling (and casing) of error messages * Additional corrections to exception messages --------- Co-authored-by: Kristen Schau <47155823+krschau@users.noreply.github.com> --- .../Providers/RepositoryProvider.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/GitHubExtension/Providers/RepositoryProvider.cs b/src/GitHubExtension/Providers/RepositoryProvider.cs index b90627af..42fb7f89 100644 --- a/src/GitHubExtension/Providers/RepositoryProvider.cs +++ b/src/GitHubExtension/Providers/RepositoryProvider.cs @@ -221,28 +221,28 @@ public IAsyncOperation CloneRepositoryAsync(IRepository } catch (LibGit2Sharp.RecurseSubmodulesException recurseException) { - Providers.Log.Logger()?.ReportError("DevHomeRepository", "Could not clone all sub modules", recurseException); - return new ProviderOperationResult(ProviderOperationStatus.Failure, recurseException, "Could not clone all modules", recurseException.Message); + Providers.Log.Logger()?.ReportError("DevHomeRepository", "Could not clone all submodules.", recurseException); + return new ProviderOperationResult(ProviderOperationStatus.Failure, recurseException, "Could not clone all submodules.", recurseException.Message); } catch (LibGit2Sharp.UserCancelledException userCancelledException) { - Providers.Log.Logger()?.ReportError("DevHomeRepository", "The user stopped the clone operation", userCancelledException); - return new ProviderOperationResult(ProviderOperationStatus.Failure, userCancelledException, "User cancelled the operation", userCancelledException.Message); + Providers.Log.Logger()?.ReportError("DevHomeRepository", "The user stopped the clone operation.", userCancelledException); + return new ProviderOperationResult(ProviderOperationStatus.Failure, userCancelledException, "User cancelled the clone operation.", userCancelledException.Message); } catch (LibGit2Sharp.NameConflictException nameConflictException) { Providers.Log.Logger()?.ReportError("DevHomeRepository", nameConflictException); - return new ProviderOperationResult(ProviderOperationStatus.Failure, nameConflictException, "The location exists and is non-empty", nameConflictException.Message); + return new ProviderOperationResult(ProviderOperationStatus.Failure, nameConflictException, "The destination location is non-empty.", nameConflictException.Message); } catch (LibGit2Sharp.LibGit2SharpException libGitTwoException) { - Providers.Log.Logger()?.ReportError("DevHomeRepository", $"Either no logged in account has access to this repo, or the repo can't be found", libGitTwoException); - return new ProviderOperationResult(ProviderOperationStatus.Failure, libGitTwoException, "LigGit2 threw an exception", "LibGit2 Threw an exception"); + Providers.Log.Logger()?.ReportError("DevHomeRepository", $"Either no logged in account has access to this repository, or the repository can't be found.", libGitTwoException); + return new ProviderOperationResult(ProviderOperationStatus.Failure, libGitTwoException, "LibGit2 library threw an exception.", "LibGit2 library threw an exception."); } catch (Exception e) { - Providers.Log.Logger()?.ReportError("DevHomeRepository", "Could not clone the repository", e); - return new ProviderOperationResult(ProviderOperationStatus.Failure, e, "Something happened when cloning the repo", "something happened when cloning the repo"); + Providers.Log.Logger()?.ReportError("DevHomeRepository", "Could not clone the repository.", e); + return new ProviderOperationResult(ProviderOperationStatus.Failure, e, "Something happened when cloning the repository.", "Something happened when cloning the repository."); } return new ProviderOperationResult(ProviderOperationStatus.Success, new ArgumentException("Nothing wrong"), "Nothing wrong", "Nothing wrong"); From 66a43e72159ddf16122e6a0c40b5a8dd8957a203 Mon Sep 17 00:00:00 2001 From: Matt Hyman Date: Tue, 7 Nov 2023 11:07:41 -0800 Subject: [PATCH 04/13] Ignore disabled tests. (#247) --- test/GitHubExtension/TestClass.cs | 2 +- test/UITest/GitHubExtensionScenarioStandard.cs | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/test/GitHubExtension/TestClass.cs b/test/GitHubExtension/TestClass.cs index 26e2a57f..d395478f 100644 --- a/test/GitHubExtension/TestClass.cs +++ b/test/GitHubExtension/TestClass.cs @@ -3,7 +3,6 @@ using System.Diagnostics; using GitHubExtension.DeveloperId; -using GitHubExtension.Providers; using Microsoft.Windows.DevHome.SDK; namespace GitHubExtension.Test; @@ -16,6 +15,7 @@ namespace GitHubExtension.Test; */ [TestClass] +[Ignore] public class TestClass : IDisposable { #pragma warning disable SA1310 // Field names should not contain underscore diff --git a/test/UITest/GitHubExtensionScenarioStandard.cs b/test/UITest/GitHubExtensionScenarioStandard.cs index 34db4008..e45e70f9 100644 --- a/test/UITest/GitHubExtensionScenarioStandard.cs +++ b/test/UITest/GitHubExtensionScenarioStandard.cs @@ -1,17 +1,12 @@ // Copyright (c) Microsoft Corporation and Contributors // Licensed under the MIT license. -using System; -using GitHubExtension.Tests.UITest; using Microsoft.VisualStudio.TestTools.UnitTesting; -using OpenQA.Selenium.Appium; -using OpenQA.Selenium.Appium.Windows; -using OpenQA.Selenium.Remote; -using static System.Collections.Specialized.BitVector32; namespace GitHubExtension.Tests.UITest; [TestClass] +[Ignore] public class GitHubExtensionScenarioStandard : GitHubExtensionSession { [TestMethod] From 91d7f26a6f7037bbedc152a9c22d4e354529b0e4 Mon Sep 17 00:00:00 2001 From: Darren Hoehna Date: Tue, 7 Nov 2023 11:08:35 -0800 Subject: [PATCH 05/13] Using correct public repo pull and implementing the icon property. (#256) * First * Implmenting the icon property * Removing un-used constructor * Removing an icon * Removing this file --------- Co-authored-by: Darren Hoehna --- src/GitHubExtension/Assets/GitHubLogo_Dark.png | Bin 0 -> 706 bytes src/GitHubExtension/GitHubExtension.csproj | 1 + .../Providers/RepositoryProvider.cs | 7 +------ 3 files changed, 2 insertions(+), 6 deletions(-) create mode 100644 src/GitHubExtension/Assets/GitHubLogo_Dark.png diff --git a/src/GitHubExtension/Assets/GitHubLogo_Dark.png b/src/GitHubExtension/Assets/GitHubLogo_Dark.png new file mode 100644 index 0000000000000000000000000000000000000000..f861d8f5b4ca24fe14e5c769e87bd24ea1307132 GIT binary patch literal 706 zcmV;z0zLhSP)U;(N}V;<0niZKwlWn@wW$oyS-iwE!Y(9z`26?uKEUH0Lp* z{{aMMOr)|l^vE_ozZ2|oMHTnYL} zWmK0Z6mM9jR5GZfvZ41Nx=&9^anm>{12tRcRuqQ!P8nD`1?E5)RkrXUu*xK(`!O)) zL4bM^4{Otb9)~_`!vk-|hGndSwS~0fg|U1gFfo147mxVgLT1GAYeV4rYXF@ zI^g=TvAl_S04^);AtA;KvXgqqUKde>Wc;K?8L^TLNdDlO!URj2BypCZD8NYWAPu|L-w*QWi o50|&2)dSRCgbwAnl7~nXKRszOnio?X0000007*qoM6N<$g2LP^8vp + diff --git a/src/GitHubExtension/Providers/RepositoryProvider.cs b/src/GitHubExtension/Providers/RepositoryProvider.cs index 42fb7f89..bc2ad26c 100644 --- a/src/GitHubExtension/Providers/RepositoryProvider.cs +++ b/src/GitHubExtension/Providers/RepositoryProvider.cs @@ -20,14 +20,9 @@ public IRandomAccessStreamReference Icon get; private set; } - public RepositoryProvider(IRandomAccessStreamReference icon) - { - Icon = icon; - } - public RepositoryProvider() { - Icon = RandomAccessStreamReference.CreateFromUri(new Uri("https://www.GitHub.com/microsoft/devhome")); + Icon = RandomAccessStreamReference.CreateFromUri(new Uri("ms-appx:///GitHubExtension/Assets/GitHubLogo_Dark.png")); } public IAsyncOperation IsUriSupportedAsync(Uri uri) From 152a60c4ae3efde90a085d54da2b4ac8c84e0a27 Mon Sep 17 00:00:00 2001 From: Kristen Schau <47155823+krschau@users.noreply.github.com> Date: Tue, 7 Nov 2023 19:08:26 -0500 Subject: [PATCH 06/13] Enable widget customization (#262) --- src/GitHubExtension/GitHubExtension.csproj | 2 +- src/GitHubExtension/Helpers/Resources.cs | 4 ++ .../Strings/en-US/Resources.resw | 16 +++++++ .../Widgets/Enums/WidgetAction.cs | 10 +++++ .../Widgets/GitHubAssignedWidget.cs | 17 ++++++- .../Widgets/GitHubIssuesWidget.cs | 20 +++++++++ .../Widgets/GitHubMentionedInWidget.cs | 19 +++++++- .../Widgets/GitHubPullsWidget.cs | 20 +++++++++ .../Widgets/GitHubReviewWidget.cs | 6 +++ src/GitHubExtension/Widgets/GitHubWidget.cs | 24 ++++++++++ .../GitHubIssuesConfigurationTemplate.json | 44 +++++++++++++++++++ .../GitHubPullsConfigurationTemplate.json | 44 +++++++++++++++++++ src/GitHubExtension/Widgets/WidgetImpl.cs | 2 + src/GitHubExtension/Widgets/WidgetProvider.cs | 13 +++++- .../GitHubExtensionServer.csproj | 2 +- .../Package.appxmanifest | 19 +++++--- src/Logging/DevHome.Logging.csproj | 2 +- .../GitHubExtension.Telemetry.csproj | 2 +- .../GitHubExtension.TestConsole.csproj | 2 +- .../GitHubExtension.Test.csproj | 2 +- test/UITest/GitHubExtension.UITest.csproj | 2 +- 21 files changed, 256 insertions(+), 16 deletions(-) diff --git a/src/GitHubExtension/GitHubExtension.csproj b/src/GitHubExtension/GitHubExtension.csproj index d7040cb9..9a28ec6f 100644 --- a/src/GitHubExtension/GitHubExtension.csproj +++ b/src/GitHubExtension/GitHubExtension.csproj @@ -80,7 +80,7 @@ - + diff --git a/src/GitHubExtension/Helpers/Resources.cs b/src/GitHubExtension/Helpers/Resources.cs index a2e52a33..29b4df72 100644 --- a/src/GitHubExtension/Helpers/Resources.cs +++ b/src/GitHubExtension/Helpers/Resources.cs @@ -96,6 +96,10 @@ public static string[] GetWidgetResourceIdentifiers() "Widget_Template/PR_info", "Widget_Template/Updated", "Widget_Template/Query", + "Widget_Template_Button/Save", + "Widget_Template_Button/Cancel", + "Widget_Template_Tooltip/Save", + "Widget_Template_Tooltip/Cancel", }; } } diff --git a/src/GitHubExtension/Strings/en-US/Resources.resw b/src/GitHubExtension/Strings/en-US/Resources.resw index 96a17bfc..d0f59455 100644 --- a/src/GitHubExtension/Strings/en-US/Resources.resw +++ b/src/GitHubExtension/Strings/en-US/Resources.resw @@ -400,4 +400,20 @@ Notifications Disabled + + Save + Shown in Widget, Button text + + + Cancel + Shown in Widget, Button text + + + Save + Shown in Widget, Button tooltip + + + Cancel + Shown in Widget, Button tooltip + \ No newline at end of file diff --git a/src/GitHubExtension/Widgets/Enums/WidgetAction.cs b/src/GitHubExtension/Widgets/Enums/WidgetAction.cs index 94dff826..671fc664 100644 --- a/src/GitHubExtension/Widgets/Enums/WidgetAction.cs +++ b/src/GitHubExtension/Widgets/Enums/WidgetAction.cs @@ -18,4 +18,14 @@ public enum WidgetAction /// Action to initiate the user Sign-In. /// SignIn, + + /// + /// Action to save after configuration. + /// + Save, + + /// + /// Action to discard configuration changes and leave configuration flow. + /// + Cancel, } diff --git a/src/GitHubExtension/Widgets/GitHubAssignedWidget.cs b/src/GitHubExtension/Widgets/GitHubAssignedWidget.cs index bd4eb57d..452c0c0a 100644 --- a/src/GitHubExtension/Widgets/GitHubAssignedWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubAssignedWidget.cs @@ -6,6 +6,7 @@ using System.Text.Json.Serialization; using GitHubExtension.DataManager; using GitHubExtension.Helpers; +using GitHubExtension.Widgets.Enums; using Microsoft.Windows.Widgets.Providers; using Octokit; @@ -69,7 +70,9 @@ private SearchCategory ShowCategory get => EnumHelper.StringToSearchCategory(State()); set => SetState(EnumHelper.SearchCategoryToString(value)); - } + } + + private SearchCategory? savedShowCategory; private string assignedToName = string.Empty; @@ -178,6 +181,11 @@ public override void RequestContentData() if (DateTime.Now - LastUpdated < WidgetDataRequestMinTime) { Log.Logger()?.ReportDebug(Name, ShortId, "Data request too soon, skipping."); + } + + if (ActivityState == WidgetActivityState.Configure) + { + return; } try @@ -315,6 +323,7 @@ public string GetConfigurationData() var configurationData = new JsonObject { { "showCategory", EnumHelper.SearchCategoryToString(ShowCategory == SearchCategory.Unknown ? SearchCategory.IssuesAndPullRequests : ShowCategory) }, + { "savedShowCategory", savedShowCategory != null ? "savedShowCategory" : string.Empty }, { "configuring", true }, }; return configurationData.ToJsonString(); @@ -329,6 +338,12 @@ private void SearchManagerResultsAvailableHandler(IEnumerable res LoadContentData(results); UpdateActivityState(); } + } + + public override void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs) + { + savedShowCategory = ShowCategory; + SetConfigure(); } } diff --git a/src/GitHubExtension/Widgets/GitHubIssuesWidget.cs b/src/GitHubExtension/Widgets/GitHubIssuesWidget.cs index 4e2b1ad9..8df74fdb 100644 --- a/src/GitHubExtension/Widgets/GitHubIssuesWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubIssuesWidget.cs @@ -5,6 +5,8 @@ using GitHubExtension.Client; using GitHubExtension.DataManager; using GitHubExtension.Helpers; +using GitHubExtension.Widgets.Enums; +using Microsoft.Windows.Widgets.Providers; using Octokit; namespace GitHubExtension.Widgets; @@ -46,6 +48,11 @@ public override void RequestContentData() Log.Logger()?.ReportDebug(Name, ShortId, "Data request too soon, skipping."); } + if (ActivityState == WidgetActivityState.Configure) + { + return; + } + try { Log.Logger()?.ReportDebug(Name, ShortId, $"Requesting data update for {GetOwner()}/{GetRepo()}"); @@ -200,6 +207,13 @@ public override string GetData(WidgetPageState page) private void DataManagerUpdateHandler(object? source, DataManagerUpdateEventArgs e) { Log.Logger()?.ReportDebug(Name, ShortId, $"Data Update Event: Kind={e.Kind} Info={e.Description} Context={string.Join(",", e.Context)}"); + + // Don't update if we're in configuration mode. + if (ActivityState == WidgetActivityState.Configure) + { + return; + } + if (e.Kind == DataManagerUpdateKind.Repository && !string.IsNullOrEmpty(RepositoryUrl)) { var fullName = Validation.ParseFullNameFromGitHubURL(RepositoryUrl); @@ -211,4 +225,10 @@ private void DataManagerUpdateHandler(object? source, DataManagerUpdateEventArgs } } } + + public override void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs) + { + SavedRepositoryUrl = RepositoryUrl; + SetConfigure(); + } } diff --git a/src/GitHubExtension/Widgets/GitHubMentionedInWidget.cs b/src/GitHubExtension/Widgets/GitHubMentionedInWidget.cs index 3d05c6d9..a5dd1c1c 100644 --- a/src/GitHubExtension/Widgets/GitHubMentionedInWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubMentionedInWidget.cs @@ -6,6 +6,7 @@ using System.Text.Json.Serialization; using GitHubExtension.DataManager; using GitHubExtension.Helpers; +using GitHubExtension.Widgets.Enums; using Microsoft.Windows.Widgets.Providers; using Octokit; @@ -50,7 +51,9 @@ private SearchCategory ShowCategory get => EnumHelper.StringToSearchCategory(State()); set => SetState(EnumHelper.SearchCategoryToString(value)); - } + } + + private SearchCategory? savedShowCategory; private string mentionedName = string.Empty; @@ -158,7 +161,12 @@ public override void RequestContentData() if (DateTime.Now - LastUpdated < WidgetDataRequestMinTime) { Log.Logger()?.ReportDebug(Name, ShortId, "Data request too soon, skipping."); - } + } + + if (ActivityState == WidgetActivityState.Configure) + { + return; + } try { @@ -291,6 +299,7 @@ public string GetConfigurationData() var configurationData = new JsonObject { { "showCategory", EnumHelper.SearchCategoryToString(ShowCategory == SearchCategory.Unknown ? SearchCategory.IssuesAndPullRequests : ShowCategory) }, + { "savedShowCategory", savedShowCategory != null ? "savedShowCategory" : string.Empty }, { "configuring", true }, }; return configurationData.ToJsonString(); @@ -305,6 +314,12 @@ private void SearchManagerResultsAvailableHandler(IEnumerable res LoadContentData(results); UpdateActivityState(); } + } + + public override void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs) + { + savedShowCategory = ShowCategory; + SetConfigure(); } } diff --git a/src/GitHubExtension/Widgets/GitHubPullsWidget.cs b/src/GitHubExtension/Widgets/GitHubPullsWidget.cs index 2b0dbb7d..7c00502e 100644 --- a/src/GitHubExtension/Widgets/GitHubPullsWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubPullsWidget.cs @@ -5,6 +5,8 @@ using GitHubExtension.Client; using GitHubExtension.DataManager; using GitHubExtension.Helpers; +using GitHubExtension.Widgets.Enums; +using Microsoft.Windows.Widgets.Providers; using Octokit; namespace GitHubExtension.Widgets; @@ -48,6 +50,11 @@ public override void RequestContentData() Log.Logger()?.ReportDebug(Name, ShortId, "Data request too soon, skipping."); } + if (ActivityState == WidgetActivityState.Configure) + { + return; + } + try { Log.Logger()?.ReportDebug(Name, ShortId, $"Requesting data update for {GetOwner()}/{GetRepo()}"); @@ -173,6 +180,13 @@ public override string GetData(WidgetPageState page) private void DataManagerUpdateHandler(object? source, DataManagerUpdateEventArgs e) { Log.Logger()?.ReportDebug(Name, ShortId, $"Data Update Event: Kind={e.Kind} Info={e.Description} Context={string.Join(",", e.Context)}"); + + // Don't update if we're in configuration mode. + if (ActivityState == WidgetActivityState.Configure) + { + return; + } + if (e.Kind == DataManagerUpdateKind.Repository && !string.IsNullOrEmpty(RepositoryUrl)) { var fullName = Validation.ParseFullNameFromGitHubURL(RepositoryUrl); @@ -184,4 +198,10 @@ private void DataManagerUpdateHandler(object? source, DataManagerUpdateEventArgs } } } + + public override void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs) + { + SavedRepositoryUrl = RepositoryUrl; + SetConfigure(); + } } diff --git a/src/GitHubExtension/Widgets/GitHubReviewWidget.cs b/src/GitHubExtension/Widgets/GitHubReviewWidget.cs index d76756a8..f52f42e3 100644 --- a/src/GitHubExtension/Widgets/GitHubReviewWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubReviewWidget.cs @@ -6,6 +6,7 @@ using System.Text.Json.Serialization; using GitHubExtension.DataManager; using GitHubExtension.Helpers; +using GitHubExtension.Widgets.Enums; using Microsoft.Windows.Widgets.Providers; using Octokit; @@ -107,6 +108,11 @@ public override void RequestContentData() if (DateTime.Now - LastUpdated < WidgetDataRequestMinTime) { Log.Logger()?.ReportDebug(Name, ShortId, "Data request too soon, skipping."); + } + + if (ActivityState == WidgetActivityState.Configure) + { + return; } try diff --git a/src/GitHubExtension/Widgets/GitHubWidget.cs b/src/GitHubExtension/Widgets/GitHubWidget.cs index ae66d9a0..f18afe0a 100644 --- a/src/GitHubExtension/Widgets/GitHubWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubWidget.cs @@ -44,6 +44,8 @@ protected string RepositoryUrl set => SetState(value); } + protected string SavedRepositoryUrl { get; set; } = string.Empty; + protected DateTime LastUpdated { get; set; } = DateTime.MinValue; protected DataUpdater DataUpdater { get; set; } @@ -137,12 +139,24 @@ public override void OnActionInvoked(WidgetActionInvokedArgs actionInvokedArgs) _ = HandleSignIn(); break; + case WidgetAction.Save: + SavedRepositoryUrl = string.Empty; + SetActive(); + break; + + case WidgetAction.Cancel: + RepositoryUrl = SavedRepositoryUrl; + SetActive(); + break; + case WidgetAction.Unknown: Log.Logger()?.ReportError(Name, ShortId, $"Unknown verb: {actionInvokedArgs.Verb}"); break; } } + public override void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs) => throw new NotImplementedException(); + private void HandleCheckUrl(WidgetActionInvokedArgs args) { // Set loading page while we fetch data from GitHub. @@ -207,6 +221,8 @@ public string GetConfiguration(string data) }; configurationData.Add("configuration", repositoryData); + configurationData.Add("savedRepositoryUrl", SavedRepositoryUrl); + configurationData.Add("saveEnabled", false); return configurationData.ToString(); } @@ -242,6 +258,8 @@ public string GetConfiguration(string data) configurationData.Add("hasConfiguration", true); configurationData.Add("configuration", repositoryData); + configurationData.Add("savedRepositoryUrl", SavedRepositoryUrl); + configurationData.Add("saveEnabled", SavedRepositoryUrl != data); } catch (Exception ex) { @@ -256,6 +274,7 @@ public string GetConfiguration(string data) configurationData.Add("errorMessage", ex.Message); configurationData.Add("configuration", repositoryData); + configurationData.Add("saveEnabled", false); return configurationData.ToString(); } @@ -437,6 +456,11 @@ private async Task PeriodicUpdate() return; } + if (ActivityState == WidgetActivityState.Configure) + { + return; + } + try { RequestContentData(); diff --git a/src/GitHubExtension/Widgets/Templates/GitHubIssuesConfigurationTemplate.json b/src/GitHubExtension/Widgets/Templates/GitHubIssuesConfigurationTemplate.json index 84308cf2..4130d34b 100644 --- a/src/GitHubExtension/Widgets/Templates/GitHubIssuesConfigurationTemplate.json +++ b/src/GitHubExtension/Widgets/Templates/GitHubIssuesConfigurationTemplate.json @@ -111,6 +111,50 @@ "$when": "${$root.hasConfiguration}", "horizontalAlignment": "Left", "bleed": true + }, + { + "type": "ColumnSet", + "spacing": "Medium", + "$when": "${$root.savedRepositoryUrl != \"\"}", + "columns": [ + { + "type": "Column", + "width": "stretch" + }, + { + "type": "Column", + "width": "auto", + "items": [ + { + "type": "Container", + "items": [ + { + "type": "ActionSet", + "actions": [ + { + "type": "Action.Execute", + "title": "%Widget_Template_Button/Save%", + "verb": "Save", + "tooltip": "%Widget_Template_Tooltip/Save%", + "isEnabled": "${$root.saveEnabled}" + }, + { + "type": "Action.Execute", + "title": "%Widget_Template_Button/Cancel%", + "verb": "Cancel", + "tooltip": "%Widget_Template_Tooltip/Cancel%" + } + ] + } + ] + } + ] + }, + { + "type": "Column", + "width": "stretch" + } + ] } ] } \ No newline at end of file diff --git a/src/GitHubExtension/Widgets/Templates/GitHubPullsConfigurationTemplate.json b/src/GitHubExtension/Widgets/Templates/GitHubPullsConfigurationTemplate.json index f21a9e51..81ffa7ea 100644 --- a/src/GitHubExtension/Widgets/Templates/GitHubPullsConfigurationTemplate.json +++ b/src/GitHubExtension/Widgets/Templates/GitHubPullsConfigurationTemplate.json @@ -96,6 +96,50 @@ "$when": "${$root.hasConfiguration}", "horizontalAlignment": "Left", "bleed": true + }, + { + "type": "ColumnSet", + "spacing": "Medium", + "$when": "${$root.savedRepositoryUrl != \"\"}", + "columns": [ + { + "type": "Column", + "width": "stretch" + }, + { + "type": "Column", + "width": "auto", + "items": [ + { + "type": "Container", + "items": [ + { + "type": "ActionSet", + "actions": [ + { + "type": "Action.Execute", + "title": "%Widget_Template_Button/Save%", + "verb": "Save", + "tooltip": "%Widget_Template_Tooltip/Save%", + "isEnabled": "${saveEnabled}" + }, + { + "type": "Action.Execute", + "title": "%Widget_Template_Button/Cancel%", + "verb": "Cancel", + "tooltip": "%Widget_Template_Tooltip/Cancel%" + } + ] + } + ] + } + ] + }, + { + "type": "Column", + "width": "stretch" + } + ] } ] } \ No newline at end of file diff --git a/src/GitHubExtension/Widgets/WidgetImpl.cs b/src/GitHubExtension/Widgets/WidgetImpl.cs index e67ce510..f25454ce 100644 --- a/src/GitHubExtension/Widgets/WidgetImpl.cs +++ b/src/GitHubExtension/Widgets/WidgetImpl.cs @@ -46,5 +46,7 @@ public void SetState(string state) public abstract void OnActionInvoked(WidgetActionInvokedArgs actionInvokedArgs); + public abstract void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs); + public abstract void OnWidgetContextChanged(WidgetContextChangedArgs contextChangedArgs); } diff --git a/src/GitHubExtension/Widgets/WidgetProvider.cs b/src/GitHubExtension/Widgets/WidgetProvider.cs index 5f79c051..40066a57 100644 --- a/src/GitHubExtension/Widgets/WidgetProvider.cs +++ b/src/GitHubExtension/Widgets/WidgetProvider.cs @@ -9,7 +9,7 @@ namespace GitHubExtension.Widgets; [ComVisible(true)] [ClassInterface(ClassInterfaceType.None)] [Guid("F23870B0-B391-4466-84E2-42A991078613")] -public sealed class WidgetProvider : IWidgetProvider +public sealed class WidgetProvider : IWidgetProvider, IWidgetProvider2 { public WidgetProvider() { @@ -140,6 +140,17 @@ public void OnActionInvoked(WidgetActionInvokedArgs actionInvokedArgs) } } + public void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs) + { + Log.Logger()?.ReportDebug($"OnCustomizationRequested id: {customizationRequestedArgs.WidgetContext.Id} definitionId: {customizationRequestedArgs.WidgetContext.DefinitionId}"); + var widgetContext = customizationRequestedArgs.WidgetContext; + var widgetId = widgetContext.Id; + if (runningWidgets.ContainsKey(widgetId)) + { + runningWidgets[widgetId].OnCustomizationRequested(customizationRequestedArgs); + } + } + public void OnWidgetContextChanged(WidgetContextChangedArgs contextChangedArgs) { Log.Logger()?.ReportDebug($"OnWidgetContextChanged id: {contextChangedArgs.WidgetContext.Id} definitionId: {contextChangedArgs.WidgetContext.DefinitionId}"); diff --git a/src/GitHubExtensionServer/GitHubExtensionServer.csproj b/src/GitHubExtensionServer/GitHubExtensionServer.csproj index 4b2c7f7a..7f5aa525 100644 --- a/src/GitHubExtensionServer/GitHubExtensionServer.csproj +++ b/src/GitHubExtensionServer/GitHubExtensionServer.csproj @@ -47,7 +47,7 @@ - + diff --git a/src/GitHubExtensionServer/Package.appxmanifest b/src/GitHubExtensionServer/Package.appxmanifest index 1d702b87..974dae8b 100644 --- a/src/GitHubExtensionServer/Package.appxmanifest +++ b/src/GitHubExtensionServer/Package.appxmanifest @@ -1,5 +1,14 @@  + + + + Microsoft.Windows.Widgets.winmd + + + + + Dev Home GitHub Extension (Dev) @@ -76,7 +85,7 @@ - + @@ -110,7 +119,7 @@ - + @@ -144,7 +153,7 @@ - + @@ -178,7 +187,7 @@ - + @@ -212,7 +221,7 @@ - + diff --git a/src/Logging/DevHome.Logging.csproj b/src/Logging/DevHome.Logging.csproj index e525d923..d094ccdc 100644 --- a/src/Logging/DevHome.Logging.csproj +++ b/src/Logging/DevHome.Logging.csproj @@ -17,7 +17,7 @@ - + diff --git a/src/Telemetry/GitHubExtension.Telemetry.csproj b/src/Telemetry/GitHubExtension.Telemetry.csproj index 39bd7b5a..9012ef73 100644 --- a/src/Telemetry/GitHubExtension.Telemetry.csproj +++ b/src/Telemetry/GitHubExtension.Telemetry.csproj @@ -16,7 +16,7 @@ - + diff --git a/test/Console/GitHubExtension.TestConsole.csproj b/test/Console/GitHubExtension.TestConsole.csproj index 9397bea2..781b94ab 100644 --- a/test/Console/GitHubExtension.TestConsole.csproj +++ b/test/Console/GitHubExtension.TestConsole.csproj @@ -42,7 +42,7 @@ - + diff --git a/test/GitHubExtension/GitHubExtension.Test.csproj b/test/GitHubExtension/GitHubExtension.Test.csproj index 2d3f1988..a42b18ac 100644 --- a/test/GitHubExtension/GitHubExtension.Test.csproj +++ b/test/GitHubExtension/GitHubExtension.Test.csproj @@ -22,7 +22,7 @@ - + diff --git a/test/UITest/GitHubExtension.UITest.csproj b/test/UITest/GitHubExtension.UITest.csproj index 5699717e..710992a1 100644 --- a/test/UITest/GitHubExtension.UITest.csproj +++ b/test/UITest/GitHubExtension.UITest.csproj @@ -16,7 +16,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + From c9bebd42ac3aabfa56d033149841e617dc7231b1 Mon Sep 17 00:00:00 2001 From: Vineeth Thomas Alex Date: Tue, 7 Nov 2023 21:03:35 -0800 Subject: [PATCH 07/13] Generalize hostname validation and other minor fixes (#263) * Fixes for GHES Validation * Minor updates for VS warnings --- src/GitHubExtension/Client/Validation.cs | 19 ++++++++++++++++ src/GitHubExtension/Constants.cs | 1 - .../DeveloperId/CredentialVault.cs | 12 +++++----- .../DeveloperId/DeveloperId.cs | 5 +++++ .../DeveloperId/DeveloperIdProvider.cs | 5 ++--- .../DeveloperId/LoginUIController.cs | 5 +---- .../DeveloperId/OAuthRequest.cs | 6 ++--- .../Providers/RepositoryProvider.cs | 22 +++++++++++-------- 8 files changed, 49 insertions(+), 26 deletions(-) diff --git a/src/GitHubExtension/Client/Validation.cs b/src/GitHubExtension/Client/Validation.cs index fefa5275..79e1332d 100644 --- a/src/GitHubExtension/Client/Validation.cs +++ b/src/GitHubExtension/Client/Validation.cs @@ -14,6 +14,11 @@ private static bool IsValidHttpUri(string uriString, out Uri? uri) } public static bool IsValidGitHubURL(Uri uri) + { + return IsValidGitHubComURL(uri) || IsValidGitHubEnterpriseServerURL(uri); + } + + public static bool IsValidGitHubComURL(Uri uri) { // Valid GitHub URL has three segments. The first is '/'. if (uri.Segments.Length < 3 || (!uri.Host.Equals("github.com", StringComparison.OrdinalIgnoreCase) && !uri.Host.Equals("www.github.com", StringComparison.OrdinalIgnoreCase))) @@ -25,6 +30,20 @@ public static bool IsValidGitHubURL(Uri uri) return true; } + public static bool IsValidGitHubEnterpriseServerURL(Uri server) + { + // Valid GHES URL has three segments. + // There are no restrictions on the hostname, except what is covered in IsValidHttpUri() + // https://docs.github.com/en/enterprise-server@3.10/admin/configuration/configuring-network-settings/configuring-the-hostname-for-your-instance + if (server.Segments.Length < 3) + { + Log.Logger()?.ReportDebug($"{server.OriginalString} is not a valid GHES repo uri"); + return false; + } + + return true; + } + // Ensure it is a GitHub repo URL. public static bool IsValidGitHubURL(string url) { diff --git a/src/GitHubExtension/Constants.cs b/src/GitHubExtension/Constants.cs index 742dd7f1..28aa4b62 100644 --- a/src/GitHubExtension/Constants.cs +++ b/src/GitHubExtension/Constants.cs @@ -6,6 +6,5 @@ internal class Constants { #pragma warning disable SA1310 // Field names should not contain underscore public const string DEV_HOME_APPLICATION_NAME = "DevHome"; - public const string GITHUB_REPOS_API = "https://api.github.com/repos/"; #pragma warning restore SA1310 // Field names should not contain underscore } diff --git a/src/GitHubExtension/DeveloperId/CredentialVault.cs b/src/GitHubExtension/DeveloperId/CredentialVault.cs index 1e2472fd..ca24e133 100644 --- a/src/GitHubExtension/DeveloperId/CredentialVault.cs +++ b/src/GitHubExtension/DeveloperId/CredentialVault.cs @@ -20,7 +20,7 @@ private static class CredentialVaultConfiguration internal static void SaveAccessTokenToVault(string loginId, SecureString? accessToken) { // Initialize a credential object. - CREDENTIAL credential = new CREDENTIAL + var credential = new CREDENTIAL { Type = CRED_TYPE.GENERIC, TargetName = CredentialVaultConfiguration.CredResourceName + ": " + loginId, @@ -75,7 +75,7 @@ internal static void RemoveAccessTokenFromVault(string loginId) internal static PasswordCredential GetCredentialFromLocker(string loginId) { var credentialNameToRetrieve = CredentialVaultConfiguration.CredResourceName + ": " + loginId; - IntPtr ptrToCredential = IntPtr.Zero; + var ptrToCredential = IntPtr.Zero; try { @@ -103,7 +103,7 @@ internal static PasswordCredential GetCredentialFromLocker(string loginId) var accessTokenInChars = new char[credentialObject.CredentialBlobSize / 2]; Marshal.Copy(credentialObject.CredentialBlob, accessTokenInChars, 0, accessTokenInChars.Length); - SecureString accessToken = new SecureString(); + var accessToken = new SecureString(); for (var i = 0; i < accessTokenInChars.Length; i++) { accessToken.AppendChar(accessTokenInChars[i]); @@ -114,7 +114,7 @@ internal static PasswordCredential GetCredentialFromLocker(string loginId) accessToken.MakeReadOnly(); - PasswordCredential credential = new PasswordCredential(CredentialVaultConfiguration.CredResourceName, loginId, new NetworkCredential(string.Empty, accessToken).Password); + var credential = new PasswordCredential(CredentialVaultConfiguration.CredResourceName, loginId, new NetworkCredential(string.Empty, accessToken).Password); return credential; } finally @@ -128,7 +128,7 @@ internal static PasswordCredential GetCredentialFromLocker(string loginId) public static IEnumerable GetAllSavedLoginIds() { - IntPtr ptrToCredential = IntPtr.Zero; + var ptrToCredential = IntPtr.Zero; try { @@ -164,7 +164,7 @@ public static IEnumerable GetAllSavedLoginIds() for (var i = 0; i < allCredentials.Length; i++) { #pragma warning disable CS8605 // Unboxing a possibly null value. - CREDENTIAL credential = (CREDENTIAL)Marshal.PtrToStructure(allCredentials[i], typeof(CREDENTIAL)); + var credential = (CREDENTIAL)Marshal.PtrToStructure(allCredentials[i], typeof(CREDENTIAL)); #pragma warning restore CS8605 // Unboxing a possibly null value. allLoginIds.Add(credential.UserName); } diff --git a/src/GitHubExtension/DeveloperId/DeveloperId.cs b/src/GitHubExtension/DeveloperId/DeveloperId.cs index a6a536c6..8d97599a 100644 --- a/src/GitHubExtension/DeveloperId/DeveloperId.cs +++ b/src/GitHubExtension/DeveloperId/DeveloperId.cs @@ -67,4 +67,9 @@ public Windows.Security.Credentials.PasswordCredential RefreshDeveloperId() GitHubClient.Credentials = new (credential.Password); return credential; } + + public Uri GetHostAddress() + { + return GitHubClient.BaseAddress; + } } diff --git a/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs b/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs index 94c56427..db7be001 100644 --- a/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs +++ b/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs @@ -269,7 +269,7 @@ private void RestoreDeveloperIds(IEnumerable loginIds) }; var user = gitHubClient.User.Current().Result; - DeveloperId developerId = new (user.Login, user.Email, user.Url, user.Name, gitHubClient); + DeveloperId developerId = new (user.Login, user.Name, user.Email, user.Url, gitHubClient); lock (DeveloperIdsLock) { @@ -294,9 +294,8 @@ public AuthenticationExperienceKind GetAuthenticationExperienceKind() public AdaptiveCardSessionResult GetLoginAdaptiveCardSession() { - var loginEntryPoint = string.Empty; Log.Logger()?.ReportInfo($"GetAdaptiveCardController"); - return new AdaptiveCardSessionResult(new LoginUIController(loginEntryPoint)); + return new AdaptiveCardSessionResult(new LoginUIController()); } public void Dispose() diff --git a/src/GitHubExtension/DeveloperId/LoginUIController.cs b/src/GitHubExtension/DeveloperId/LoginUIController.cs index 7dab5556..da19b399 100644 --- a/src/GitHubExtension/DeveloperId/LoginUIController.cs +++ b/src/GitHubExtension/DeveloperId/LoginUIController.cs @@ -8,14 +8,11 @@ namespace GitHubExtension.DeveloperId; internal class LoginUIController : IExtensionAdaptiveCardSession { - // _loginEntryPoint stores the calling component on Dev Home (like "Settings", "SetupTool" etc). - private readonly string _loginEntryPoint; private IExtensionAdaptiveCard? _loginUI; private static readonly LoginUITemplate _loginUITemplate = new (); - public LoginUIController(string loginEntryPoint) + public LoginUIController() { - _loginEntryPoint = loginEntryPoint; } public void Dispose() diff --git a/src/GitHubExtension/DeveloperId/OAuthRequest.cs b/src/GitHubExtension/DeveloperId/OAuthRequest.cs index d9a47548..f70ea269 100644 --- a/src/GitHubExtension/DeveloperId/OAuthRequest.cs +++ b/src/GitHubExtension/DeveloperId/OAuthRequest.cs @@ -90,7 +90,7 @@ internal async Task CompleteOAuthAsync(Uri authorizationResponse) var queryString = authorizationResponse.Query; // Parse the query string variables into a NameValueCollection. - NameValueCollection queryStringCollection = HttpUtility.ParseQueryString(queryString); + var queryStringCollection = HttpUtility.ParseQueryString(queryString); if (!string.IsNullOrEmpty(queryStringCollection.Get("error"))) { @@ -133,7 +133,7 @@ internal DeveloperId RetrieveDeveloperId() } var newUser = gitHubClient.User.Current().Result; - DeveloperId developerId = new (newUser.Login, newUser.Email, newUser.Url, newUser.Name, gitHubClient); + DeveloperId developerId = new (newUser.Login, newUser.Name, newUser.Email, newUser.Url, gitHubClient); return developerId; } @@ -144,7 +144,7 @@ internal static string RetrieveState(Uri authorizationResponse) var queryString = authorizationResponse.Query; // Parse the query string variables into a NameValueCollection. - NameValueCollection queryStringCollection = HttpUtility.ParseQueryString(queryString); + var queryStringCollection = HttpUtility.ParseQueryString(queryString); var state = queryStringCollection.Get("state"); diff --git a/src/GitHubExtension/Providers/RepositoryProvider.cs b/src/GitHubExtension/Providers/RepositoryProvider.cs index bc2ad26c..a89e735f 100644 --- a/src/GitHubExtension/Providers/RepositoryProvider.cs +++ b/src/GitHubExtension/Providers/RepositoryProvider.cs @@ -64,19 +64,23 @@ IAsyncOperation IRepositoryProvider.GetRepositoriesAsync(IDe var repositoryList = new List(); try { - ApiOptions apiOptions = new (); - apiOptions.PageSize = 50; - apiOptions.PageCount = 1; + ApiOptions apiOptions = new () + { + PageSize = 50, + PageCount = 1, + }; // Authenticate as the specified developer Id. var client = DeveloperIdProvider.GetInstance().GetDeveloperIdInternal(developerId).GitHubClient; - RepositoryRequest request = new RepositoryRequest(); - request.Sort = RepositorySort.Updated; - request.Direction = SortDirection.Descending; - request.Affiliation = RepositoryAffiliation.Owner; + var request = new RepositoryRequest + { + Sort = RepositorySort.Updated, + Direction = SortDirection.Descending, + Affiliation = RepositoryAffiliation.Owner, - // Gets only public repos for the owned repos. - request.Visibility = RepositoryRequestVisibility.Public; + // Gets only public repos for the owned repos. + Visibility = RepositoryRequestVisibility.Public, + }; var getPublicReposTask = client.Repository.GetAllForCurrent(request, apiOptions); // this is getting private org and user repos. From 9dc9efe3c963e3744ea9e6bdeeda8a92a1cad312 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 7 Nov 2023 23:04:12 -0800 Subject: [PATCH 08/13] Change PullRequest time and label to match sort order. (#266) --- src/GitHubExtension/Widgets/GitHubPullsWidget.cs | 2 +- src/GitHubExtension/Widgets/Templates/GitHubPullsTemplate.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/GitHubExtension/Widgets/GitHubPullsWidget.cs b/src/GitHubExtension/Widgets/GitHubPullsWidget.cs index 7c00502e..4624cdd7 100644 --- a/src/GitHubExtension/Widgets/GitHubPullsWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubPullsWidget.cs @@ -112,7 +112,7 @@ public override void LoadContentData() { "title", pullItem.Title }, { "url", pullItem.HtmlUrl }, { "number", pullItem.Number }, - { "date", TimeSpanHelper.DateTimeOffsetToDisplayString(pullItem.CreatedAt, Log.Logger()) }, + { "date", TimeSpanHelper.DateTimeOffsetToDisplayString(pullItem.UpdatedAt, Log.Logger()) }, { "user", pullItem.Author.Login }, { "avatar", pullItem.Author.AvatarUrl }, { "icon", pullsIconData }, diff --git a/src/GitHubExtension/Widgets/Templates/GitHubPullsTemplate.json b/src/GitHubExtension/Widgets/Templates/GitHubPullsTemplate.json index 8e205d7c..a31ba584 100644 --- a/src/GitHubExtension/Widgets/Templates/GitHubPullsTemplate.json +++ b/src/GitHubExtension/Widgets/Templates/GitHubPullsTemplate.json @@ -121,7 +121,7 @@ { "type": "TextBlock", "size": "small", - "text": "#${number} %Widget_Template/Opened% ${date}", + "text": "#${number} %Widget_Template/Updated% ${date}", "isSubtle": true, "spacing": "small", "wrap": true From 865d4bc4e0f1a0adbc14ab9196d1a2c64058ca90 Mon Sep 17 00:00:00 2001 From: Kristen Schau <47155823+krschau@users.noreply.github.com> Date: Wed, 8 Nov 2023 02:05:27 -0500 Subject: [PATCH 09/13] After editing widget, show loading page instead of stale data (#264) * Show loading page instead of stale data after customization * force immedaite data update request --- .../Widgets/GitHubAssignedWidget.cs | 4 ++++ .../Widgets/GitHubMentionedInWidget.cs | 4 ++++ src/GitHubExtension/Widgets/GitHubWidget.cs | 14 ++++++++++++++ 3 files changed, 22 insertions(+) diff --git a/src/GitHubExtension/Widgets/GitHubAssignedWidget.cs b/src/GitHubExtension/Widgets/GitHubAssignedWidget.cs index 452c0c0a..ce1c6336 100644 --- a/src/GitHubExtension/Widgets/GitHubAssignedWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubAssignedWidget.cs @@ -126,6 +126,10 @@ public override void OnActionInvoked(WidgetActionInvokedArgs actionInvokedArgs) if (dataObject != null && dataObject.ShowCategory != null) { ShowCategory = EnumHelper.StringToSearchCategory(dataObject.ShowCategory); + + // If we got here during the customization flow, we need to LoadContentData again + // so we can show the loading page rather than stale data. + LoadContentData(); UpdateActivityState(); } } diff --git a/src/GitHubExtension/Widgets/GitHubMentionedInWidget.cs b/src/GitHubExtension/Widgets/GitHubMentionedInWidget.cs index a5dd1c1c..0667fac8 100644 --- a/src/GitHubExtension/Widgets/GitHubMentionedInWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubMentionedInWidget.cs @@ -146,6 +146,10 @@ public override void OnActionInvoked(WidgetActionInvokedArgs actionInvokedArgs) if (dataObject != null && dataObject.ShowCategory != null) { ShowCategory = EnumHelper.StringToSearchCategory(dataObject.ShowCategory); + + // If we got here during the customization flow, we need to LoadContentData again + // so we can show the loading page rather than stale data. + LoadContentData(); UpdateActivityState(); } } diff --git a/src/GitHubExtension/Widgets/GitHubWidget.cs b/src/GitHubExtension/Widgets/GitHubWidget.cs index f18afe0a..90865037 100644 --- a/src/GitHubExtension/Widgets/GitHubWidget.cs +++ b/src/GitHubExtension/Widgets/GitHubWidget.cs @@ -140,7 +140,21 @@ public override void OnActionInvoked(WidgetActionInvokedArgs actionInvokedArgs) break; case WidgetAction.Save: + // Set loading page while we swap out the data. + Page = WidgetPageState.Loading; + + // It might take some time to get the new data, so + // set data state to "unknown" so that loading page is shown. + DataState = WidgetDataState.Unknown; + UpdateWidget(); + SavedRepositoryUrl = string.Empty; + LoadContentData(); + + // Reset the throttle time and force an immediate data update request. + LastUpdated = DateTime.MinValue; + RequestContentData(); + SetActive(); break; From c7cd5b34ddcb17aee39e82212412da8467518e87 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 7 Nov 2023 23:38:06 -0800 Subject: [PATCH 10/13] Add LastObserved to issues and pulls and remove unobserved data (#257) * Add LastObserved to issues and pulls and remove recently unobserved data. * Change update timings --- .../DataManager/GitHubDataManager.cs | 17 ++++ .../DataManager/GitHubDataManagerUpdate.cs | 2 +- .../DataModel/DataObjects/Issue.cs | 53 ++++++++----- .../DataModel/DataObjects/PullRequest.cs | 78 ++++++++++++++----- .../DataModel/GitHubDataStoreSchema.cs | 4 +- 5 files changed, 116 insertions(+), 38 deletions(-) diff --git a/src/GitHubExtension/DataManager/GitHubDataManager.cs b/src/GitHubExtension/DataManager/GitHubDataManager.cs index 5386662e..63d10d12 100644 --- a/src/GitHubExtension/DataManager/GitHubDataManager.cs +++ b/src/GitHubExtension/DataManager/GitHubDataManager.cs @@ -16,6 +16,13 @@ public partial class GitHubDataManager : IGitHubDataManager, IDisposable private static readonly TimeSpan NotificationRetentionTime = TimeSpan.FromDays(7); private static readonly TimeSpan SearchRetentionTime = TimeSpan.FromDays(7); private static readonly TimeSpan PullRequestStaleTime = TimeSpan.FromDays(1); + + // It is possible different widgets have queries which touch the same pull requests. + // We want to keep this window large enough that we don't delete data being used by + // other clients which simply haven't been updated yet but will in the near future. + // This is a conservative time period to check for pruning and give time for other + // consumers using the data to update its freshness before we remove it. + private static readonly TimeSpan LastObservedDeleteSpan = TimeSpan.FromMinutes(6); private static readonly long CheckSuiteIdDependabot = 29110; private static readonly string Name = nameof(GitHubDataManager); @@ -417,6 +424,10 @@ private async Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(DataStoreOpera Log.Logger()?.ReportDebug(Name, $"Updated developer pull requests for {repoFullName}."); } + + // After we update for this developer, remove all pull requests for this developer that + // were not observed recently. + PullRequest.DeleteAllByDeveloperLoginAndLastObservedBefore(DataStore, devId.LoginId, DateTime.UtcNow - LastObservedDeleteSpan); } } @@ -467,6 +478,9 @@ private async Task UpdatePullRequestsAsync(Repository repository, Octokit.GitHub CreatePullRequestStatus(dsPullRequest); } + + // Remove unobserved pull requests from this repository. + PullRequest.DeleteLastObservedBefore(DataStore, repository.Id, DateTime.UtcNow - LastObservedDeleteSpan); } private void CreatePullRequestStatus(PullRequest pullRequest) @@ -602,6 +616,9 @@ private async Task UpdateIssuesAsync(Repository repository, Octokit.GitHubClient // were not recently updated (within the last minute), remove them from the search result. SearchIssue.DeleteBefore(DataStore, search, DateTime.Now - TimeSpan.FromMinutes(1)); } + + // Remove issues from this repository that were not observed recently. + Issue.DeleteLastObservedBefore(DataStore, repository.Id, DateTime.UtcNow - LastObservedDeleteSpan); } // Removes unused data from the datastore. diff --git a/src/GitHubExtension/DataManager/GitHubDataManagerUpdate.cs b/src/GitHubExtension/DataManager/GitHubDataManagerUpdate.cs index 1c9ea6b0..4c1a91f8 100644 --- a/src/GitHubExtension/DataManager/GitHubDataManagerUpdate.cs +++ b/src/GitHubExtension/DataManager/GitHubDataManagerUpdate.cs @@ -8,7 +8,7 @@ namespace GitHubExtension; public partial class GitHubDataManager { // This is how frequently the DataStore update occurs. - private static readonly TimeSpan UpdateInterval = TimeSpan.FromMinutes(2); + private static readonly TimeSpan UpdateInterval = TimeSpan.FromMinutes(5); private static DateTime lastUpdateTime = DateTime.MinValue; public static async Task Update() diff --git a/src/GitHubExtension/DataModel/DataObjects/Issue.cs b/src/GitHubExtension/DataModel/DataObjects/Issue.cs index efb8ce0e..8b58185e 100644 --- a/src/GitHubExtension/DataModel/DataObjects/Issue.cs +++ b/src/GitHubExtension/DataModel/DataObjects/Issue.cs @@ -40,6 +40,8 @@ public class Issue public long TimeClosed { get; set; } = DataStore.NoForeignKey; + public long TimeLastObserved { get; set; } = DataStore.NoForeignKey; + // Label IDs are a string concatenation of Label internalIds. // We need to duplicate this data in order to properly do inserts and // to compare two objects for changes in order to add/remove associations. @@ -66,6 +68,10 @@ private DataStore? DataStore [Computed] public DateTime ClosedAt => TimeClosed.ToDateTime(); + [Write(false)] + [Computed] + public DateTime LastObservedAt => TimeLastObserved.ToDateTime(); + // Derived Properties so consumers of these objects do not // need to do further queries of the datastore. [Write(false)] @@ -155,6 +161,7 @@ private static Issue CreateFromOctokitIssue(DataStore dataStore, Octokit.Issue o TimeCreated = okitIssue.CreatedAt.DateTime.ToDataStoreInteger(), TimeUpdated = okitIssue.UpdatedAt.HasValue ? okitIssue.UpdatedAt.Value.DateTime.ToDataStoreInteger() : 0, TimeClosed = okitIssue.ClosedAt.HasValue ? okitIssue.ClosedAt.Value.DateTime.ToDataStoreInteger() : 0, + TimeLastObserved = DateTime.UtcNow.ToDataStoreInteger(), }; // Labels are a string concat of label internal ids. @@ -207,28 +214,22 @@ private static Issue AddOrUpdateIssue(DataStore dataStore, Issue issue) var existing = GetByInternalId(dataStore, issue.InternalId); if (existing is not null) { - if (issue.TimeUpdated > existing.TimeUpdated) - { - issue.Id = existing.Id; - dataStore.Connection!.Update(issue); - issue.DataStore = dataStore; - - if (issue.LabelIds != existing.LabelIds) - { - UpdateLabelsForIssue(dataStore, issue); - } - - if (issue.AssigneeIds != existing.AssigneeIds) - { - UpdateAssigneesForIssue(dataStore, issue); - } + // Existing issues must be updated and always marked observed. + issue.Id = existing.Id; + dataStore.Connection!.Update(issue); + issue.DataStore = dataStore; - return issue; + if (issue.LabelIds != existing.LabelIds) + { + UpdateLabelsForIssue(dataStore, issue); } - else + + if (issue.AssigneeIds != existing.AssigneeIds) { - return existing; + UpdateAssigneesForIssue(dataStore, issue); } + + return issue; } // No existing issue, add it. @@ -353,4 +354,20 @@ private static void UpdateAssigneesForIssue(DataStore dataStore, Issue issue) } } } + + // Delete records in a repository not observed before the specified date. + public static void DeleteLastObservedBefore(DataStore dataStore, long repositoryId, DateTime date) + { + // Delete issues older than the time specified for the given repository. + // This is intended to be run after updating a repository's issues so that non-observed + // records will be removed. + var sql = @"DELETE FROM Issue WHERE RepositoryId = $RepositoryId AND TimeLastObserved < $Time;"; + var command = dataStore.Connection!.CreateCommand(); + command.CommandText = sql; + command.Parameters.AddWithValue("$Time", date.ToDataStoreInteger()); + command.Parameters.AddWithValue("$RepositoryId", repositoryId); + Log.Logger()?.ReportDebug(DataStore.GetCommandLogMessage(sql, command)); + var rowsDeleted = command.ExecuteNonQuery(); + Log.Logger()?.ReportDebug(DataStore.GetDeletedLogMessage(rowsDeleted)); + } } diff --git a/src/GitHubExtension/DataModel/DataObjects/PullRequest.cs b/src/GitHubExtension/DataModel/DataObjects/PullRequest.cs index d4746e78..4ac02178 100644 --- a/src/GitHubExtension/DataModel/DataObjects/PullRequest.cs +++ b/src/GitHubExtension/DataModel/DataObjects/PullRequest.cs @@ -53,6 +53,8 @@ public class PullRequest public long TimeClosed { get; set; } = DataStore.NoForeignKey; + public long TimeLastObserved { get; set; } = DataStore.NoForeignKey; + // Label IDs are a string concatenation of Label internalIds. // We need to duplicate this data in order to properly do inserts and // to compare two objects for changes in order to add/remove associations. @@ -80,6 +82,10 @@ public class PullRequest [Computed] public DateTime ClosedAt => TimeClosed.ToDateTime(); + [Write(false)] + [Computed] + public DateTime LastObservedAt => TimeLastObserved.ToDateTime(); + // Derived Properties so consumers of these objects do not // need to do further queries of the datastore. [Write(false)] @@ -336,6 +342,7 @@ private static PullRequest CreateFromOctokitPullRequest(DataStore dataStore, Oct TimeUpdated = okitPull.UpdatedAt.DateTime.ToDataStoreInteger(), TimeMerged = okitPull.MergedAt.HasValue ? okitPull.MergedAt.Value.DateTime.ToDataStoreInteger() : 0, TimeClosed = okitPull.ClosedAt.HasValue ? okitPull.ClosedAt.Value.DateTime.ToDataStoreInteger() : 0, + TimeLastObserved = DateTime.UtcNow.ToDataStoreInteger(), }; // Labels are a string concat of label internal ids. @@ -385,28 +392,22 @@ private static PullRequest AddOrUpdatePullRequest(DataStore dataStore, PullReque var existingPull = GetByInternalId(dataStore, pull.InternalId); if (existingPull is not null) { - if (pull.TimeUpdated > existingPull.TimeUpdated) - { - pull.Id = existingPull.Id; - dataStore.Connection!.Update(pull); - pull.DataStore = dataStore; - - if (pull.LabelIds != existingPull.LabelIds) - { - UpdateLabelsForPullRequest(dataStore, pull); - } - - if (pull.AssigneeIds != existingPull.AssigneeIds) - { - UpdateAssigneesForPullRequest(dataStore, pull); - } + // Existing pull requests must always be updated to update the LastObserved time. + pull.Id = existingPull.Id; + dataStore.Connection!.Update(pull); + pull.DataStore = dataStore; - return pull; + if (pull.LabelIds != existingPull.LabelIds) + { + UpdateLabelsForPullRequest(dataStore, pull); } - else + + if (pull.AssigneeIds != existingPull.AssigneeIds) { - return existingPull; + UpdateAssigneesForPullRequest(dataStore, pull); } + + return pull; } // No existing pull request, add it. @@ -526,4 +527,45 @@ private static void UpdateAssigneesForPullRequest(DataStore dataStore, PullReque } } } + + // Delete records in a repository not observed before the specified date. + public static void DeleteLastObservedBefore(DataStore dataStore, long repositoryId, DateTime date) + { + // Delete pull requests older than the time specified for the given repository. + // This is intended to be run after updating a repository's Pull Requests so that non-observed + // records will be removed. + var sql = @"DELETE FROM PullRequest WHERE RepositoryId = $RepositoryId AND TimeLastObserved < $Time;"; + var command = dataStore.Connection!.CreateCommand(); + command.CommandText = sql; + command.Parameters.AddWithValue("$Time", date.ToDataStoreInteger()); + command.Parameters.AddWithValue("$RepositoryId", repositoryId); + Log.Logger()?.ReportDebug(DataStore.GetCommandLogMessage(sql, command)); + var rowsDeleted = command.ExecuteNonQuery(); + Log.Logger()?.ReportDebug(DataStore.GetDeletedLogMessage(rowsDeleted)); + } + + // Delete all records from a particular user before the specified date. + // This is for removing developer pull requests across any repository that were not updated + // recently. This should remove non-open pull requests from the developer across all repositories. + public static void DeleteAllByDeveloperLoginAndLastObservedBefore(DataStore dataStore, string loginId, DateTime date) + { + var developerUsers = User.GetDeveloperUsers(dataStore); + foreach (var user in developerUsers) + { + if (user.Login != loginId) + { + continue; + } + + var sql = @"DELETE FROM PullRequest WHERE AuthorId = $UserId AND TimeLastObserved < $Time;"; + var command = dataStore.Connection!.CreateCommand(); + command.CommandText = sql; + command.Parameters.AddWithValue("$Time", date.ToDataStoreInteger()); + command.Parameters.AddWithValue("$UserId", user.Id); + Log.Logger()?.ReportDebug(DataStore.GetCommandLogMessage(sql, command)); + var rowsDeleted = command.ExecuteNonQuery(); + Log.Logger()?.ReportDebug(DataStore.GetDeletedLogMessage(rowsDeleted)); + break; + } + } } diff --git a/src/GitHubExtension/DataModel/GitHubDataStoreSchema.cs b/src/GitHubExtension/DataModel/GitHubDataStoreSchema.cs index 1336ec7b..9d586bde 100644 --- a/src/GitHubExtension/DataModel/GitHubDataStoreSchema.cs +++ b/src/GitHubExtension/DataModel/GitHubDataStoreSchema.cs @@ -13,7 +13,7 @@ public GitHubDataStoreSchema() } // Update this anytime incompatible changes happen with a released version. - private const long SchemaVersionValue = 0x0004; + private const long SchemaVersionValue = 0x0005; private static readonly string Metadata = @"CREATE TABLE Metadata (" + @@ -79,6 +79,7 @@ public GitHubDataStoreSchema() "TimeCreated INTEGER NOT NULL," + "TimeUpdated INTEGER NOT NULL," + "TimeClosed INTEGER NOT NULL," + + "TimeLastObserved INTEGER NOT NULL," + "HtmlUrl TEXT NULL COLLATE NOCASE," + "Locked INTEGER NOT NULL," + "AssigneeIds TEXT NULL COLLATE NOCASE," + @@ -116,6 +117,7 @@ public GitHubDataStoreSchema() "TimeUpdated INTEGER NOT NULL," + "TimeMerged INTEGER NOT NULL," + "TimeClosed INTEGER NOT NULL," + + "TimeLastObserved INTEGER NOT NULL," + "HtmlUrl TEXT NULL COLLATE NOCASE," + "Locked INTEGER NOT NULL," + "Draft INTEGER NOT NULL," + From e1bb5691c6a5caf61651950f2ad53df6c4ecf864 Mon Sep 17 00:00:00 2001 From: Eric Johnson Date: Wed, 8 Nov 2023 00:11:03 -0800 Subject: [PATCH 11/13] Change version to 0.7 --- build/azure-pipelines.yml | 2 +- build/scripts/CreateBuildInfo.ps1 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/azure-pipelines.yml b/build/azure-pipelines.yml index b6ac2741..8ca41d27 100644 --- a/build/azure-pipelines.yml +++ b/build/azure-pipelines.yml @@ -20,7 +20,7 @@ parameters: - release variables: - MSIXVersion: '0.600' + MSIXVersion: '0.700' solution: '**/GitHubExtension.sln' appxPackageDir: 'AppxPackages' testOutputArtifactDir: 'TestResults' diff --git a/build/scripts/CreateBuildInfo.ps1 b/build/scripts/CreateBuildInfo.ps1 index d4b8a4e9..7c6f4b66 100644 --- a/build/scripts/CreateBuildInfo.ps1 +++ b/build/scripts/CreateBuildInfo.ps1 @@ -5,7 +5,7 @@ Param( ) $Major = "0" -$Minor = "6" +$Minor = "7" $Patch = "99" # default to 99 for local builds $versionSplit = $Version.Split("."); From fa13e4ba91ff784cf182277ca4b3e96ea359b9e4 Mon Sep 17 00:00:00 2001 From: Kristen Schau <47155823+krschau@users.noreply.github.com> Date: Wed, 8 Nov 2023 11:46:45 -0500 Subject: [PATCH 12/13] Update winmd to dll (#268) --- src/GitHubExtensionServer/Package.appxmanifest | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GitHubExtensionServer/Package.appxmanifest b/src/GitHubExtensionServer/Package.appxmanifest index 974dae8b..5f4f5896 100644 --- a/src/GitHubExtensionServer/Package.appxmanifest +++ b/src/GitHubExtensionServer/Package.appxmanifest @@ -3,7 +3,7 @@ - Microsoft.Windows.Widgets.winmd + Microsoft.Windows.Widgets.Providers.Projection.dll From 301f2acf5071c2067fe2736bcc2c2f140ef250d2 Mon Sep 17 00:00:00 2001 From: Vineeth Thomas Alex Date: Tue, 14 Nov 2023 11:33:47 -0800 Subject: [PATCH 13/13] Explicitly disallow multiple accounts on GitHub extension (#274) --- .../DeveloperId/LoginUIController.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/GitHubExtension/DeveloperId/LoginUIController.cs b/src/GitHubExtension/DeveloperId/LoginUIController.cs index da19b399..0a5744fb 100644 --- a/src/GitHubExtension/DeveloperId/LoginUIController.cs +++ b/src/GitHubExtension/DeveloperId/LoginUIController.cs @@ -41,13 +41,22 @@ public IAsyncOperation OnAction(string action, string i { case LoginUIState.LoginPage: { - // Inputs are validated at this point. - _loginUI.Update(_loginUITemplate.GetLoginUITemplate(LoginUIState.WaitingPage), null, LoginUIState.WaitingPage); - Log.Logger()?.ReportDebug($"inputs: {inputs}"); - try { - var devId = await (DeveloperIdProvider.GetInstance() as DeveloperIdProvider).LoginNewDeveloperIdAsync(); + // If there is already a developer id, we should block another login. + if (DeveloperIdProvider.GetInstance().GetLoggedInDeveloperIdsInternal().Any()) + { + Log.Logger()?.ReportInfo($"DeveloperId {DeveloperIdProvider.GetInstance().GetLoggedInDeveloperIdsInternal().First().LoginId} already exists. Blocking login."); + _loginUI.Update(_loginUITemplate.GetLoginUITemplate(LoginUIState.LoginFailedPage), null, LoginUIState.LoginFailedPage); + operationResult = new ProviderOperationResult(ProviderOperationStatus.Failure, null, "Only one DeveloperId can be logged in at a time", "One DeveloperId already exists"); + break; + } + + // Inputs are validated at this point. + _loginUI.Update(_loginUITemplate.GetLoginUITemplate(LoginUIState.WaitingPage), null, LoginUIState.WaitingPage); + Log.Logger()?.ReportDebug($"inputs: {inputs}"); + + var devId = await DeveloperIdProvider.GetInstance().LoginNewDeveloperIdAsync(); if (devId != null) { var resourceLoader = new ResourceLoader(ResourceLoader.GetDefaultResourceFilePath(), "GitHubExtension/Resources");