From ba7f9e9a0d36186ca74e4a32e2660c1a48fa165c Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 19 Jul 2024 15:11:49 -0700 Subject: [PATCH 01/16] Add PR status --- .../DataManager/AzureDataManager.cs | 172 ++++++++++++++++-- .../DataManager/AzureDataManagerCache.cs | 2 +- .../DataManager/AzureDataManagerUpdate.cs | 71 ++++++++ .../DataManager/DataManagerUpdateEventArgs.cs | 1 + .../DataManager/IAzureDataManager.cs | 6 + .../DataModel/AzureDataStoreSchema.cs | 12 +- .../DataModel/DataObjects/Identity.cs | 44 +++-- .../DataObjects/PullRequestPolicyStatus.cs | 65 +++++++ .../DataModel/DataObjects/PullRequests.cs | 33 ++-- .../DataModel/DataObjects/Repository.cs | 45 +++++ .../DataObjects/RepositoryReference.cs | 26 ++- .../DataModel/Enums/PolicyStatus.cs | 16 ++ .../Widgets/AzurePullRequestsWidget.cs | 22 ++- src/AzureExtensionServer/Program.cs | 19 +- .../DataStore/DataObjectTests.cs | 12 +- 15 files changed, 489 insertions(+), 57 deletions(-) create mode 100644 src/AzureExtension/DataManager/AzureDataManagerUpdate.cs create mode 100644 src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs create mode 100644 src/AzureExtension/DataModel/Enums/PolicyStatus.cs diff --git a/src/AzureExtension/DataManager/AzureDataManager.cs b/src/AzureExtension/DataManager/AzureDataManager.cs index c0466c4..0e83b14 100644 --- a/src/AzureExtension/DataManager/AzureDataManager.cs +++ b/src/AzureExtension/DataManager/AzureDataManager.cs @@ -12,6 +12,7 @@ using DevHomeAzureExtension.DeveloperId; using DevHomeAzureExtension.Helpers; using Microsoft.TeamFoundation.Core.WebApi; +using Microsoft.TeamFoundation.Policy.WebApi; using Microsoft.TeamFoundation.SourceControl.WebApi; using Microsoft.TeamFoundation.WorkItemTracking.WebApi; using Microsoft.VisualStudio.Services.WebApi; @@ -262,6 +263,11 @@ public async Task UpdateDataForPullRequestsAsync(AzureUri repositoryUri, string return GetPullRequests(repositoryUri.Organization, repositoryUri.Project, repositoryUri.Repository, developerId, view); } + public PullRequests? GetPullRequestsForLoggedInDeveloperIds() + { + return null; + } + private async Task UpdateDataForQueriesAsync(DataStoreOperationParameters parameters) { _log.Debug($"Inside UpdateDataForQueriesAsync with Parameters: {parameters}"); @@ -548,6 +554,18 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p project = Project.GetOrCreateByTeamProject(DataStore, teamProject, org.Id); } + var repository = Repository.Get(DataStore, project.Id, azureUri.Repository); + if (repository is null) + { + var gitRepository = await gitClient.GetRepositoryAsync(project.InternalId, azureUri.Repository); + if (gitRepository is null) + { + throw new RepositoryNotFoundException(azureUri.Repository); + } + + repository = Repository.GetOrCreate(DataStore, gitRepository, project.Id); + } + var searchCriteria = new GitPullRequestSearchCriteria { Status = PullRequestStatus.Active, @@ -570,24 +588,77 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p break; } - var pullRequests = await gitClient.GetPullRequestsAsync(project.InternalId, azureUri.Repository, searchCriteria, null, null, PullRequestResultLimit); + var pullRequests = await gitClient.GetPullRequestsAsync(project.InternalId, repository.InternalId, searchCriteria, null, null, PullRequestResultLimit); if (pullRequests == null || pullRequests.Count == 0) { // If PRs were null or empty, this is a valid result, so create an empty record. - PullRequests.GetOrCreate(DataStore, azureUri.Repository, project.Id, parameters.DeveloperId.LoginId, parameters.PullRequestView, new JsonObject().ToJsonString()); + PullRequests.GetOrCreate(DataStore, repository.Id, project.Id, parameters.DeveloperId.LoginId, parameters.PullRequestView, new JsonObject().ToJsonString()); + + // If we had any statuses they should be removed here. return; } - // Convert relevant pull request items to Json. dynamic pullRequestsObj = new ExpandoObject(); var pullRequestsObjDict = (IDictionary)pullRequestsObj; + + var policyClient = result.Connection!.GetClient(); foreach (var pullRequest in pullRequests) { + var status = PolicyStatus.Unknown; + var statusReason = string.Empty; + try + { + // ArtifactId is null in the pull request object and it is not the correct object. The ArtifactId for the + // Policy Evaluations API is this: + // vstfs:///CodeReview/CodeReviewId/{projectId}/{pullRequestId} + var artifactId = $"vstfs:///CodeReview/CodeReviewId/{project.InternalId}/{pullRequest.PullRequestId}"; + Log.Information($"ArtifactId: {artifactId}"); + + var policyEvaluations = await policyClient.GetPolicyEvaluationsAsync(project.InternalId, artifactId); + if (policyEvaluations != null) + { + var countApplicablePolicies = 0; + foreach (var policyEvaluation in policyEvaluations) + { + if (policyEvaluation.Configuration.IsEnabled && policyEvaluation.Configuration.IsBlocking) + { + ++countApplicablePolicies; + var evalStatus = PullRequestPolicyStatus.GetFromPolicyEvaluationStatus(policyEvaluation.Status); + if (evalStatus < status) + { + statusReason = policyEvaluation.Configuration.Type.DisplayName; + status = evalStatus; + } + + Log.Information($" Policy: {policyEvaluation.EvaluationId} Name: {policyEvaluation.Configuration.Type.DisplayName} Enabled: {policyEvaluation.Configuration.IsEnabled} Blocking: {policyEvaluation.Configuration.IsBlocking} {policyEvaluation.Status}"); + } + } + + if (countApplicablePolicies == 0) + { + // If there is no applicable policy, treat the policy status as Approved. + status = PolicyStatus.Approved; + } + } + } + catch (Exception ex) + { + Log.Error(ex, $"Failed getting policy evaluations for pull request: {pullRequest.PullRequestId} {pullRequest.Url}"); + } + + // Set record + Log.Information($"PullRequest Status: {status} Reason: {statusReason}"); + + //// If this was a developer pull request, track status for notifications. + dynamic pullRequestObj = new ExpandoObject(); var pullRequestObjFields = (IDictionary)pullRequestObj; pullRequestObjFields.Add("Id", pullRequest.PullRequestId); pullRequestObjFields.Add("Title", pullRequest.Title); + pullRequestObjFields.Add("Status", pullRequest.Status); + pullRequestObjFields.Add("PolicyStatus", status.ToString()); + pullRequestObjFields.Add("PolicyStatusReason", statusReason); var creator = Identity.GetOrCreateIdentity(DataStore, pullRequest.CreatedBy, result.Connection); pullRequestObjFields.Add("CreatedBy", creator); @@ -611,12 +682,86 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p }; var serializedJson = JsonSerializer.Serialize(pullRequestsObj, serializerOptions); - PullRequests.GetOrCreate(DataStore, azureUri.Repository, project.Id, parameters.DeveloperId.LoginId, parameters.PullRequestView, serializedJson); + PullRequests.GetOrCreate(DataStore, repository.Id, project.Id, parameters.DeveloperId.LoginId, parameters.PullRequestView, serializedJson); } // Foreach AzureUri return; } + public async Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(RequestOptions? options = null, Guid? requestor = null) + { + ValidateDataStore(); + var parameters = new DataStoreOperationParameters + { + RequestOptions = options ?? RequestOptions.RequestOptionsDefault(), + OperationName = "UpdatePullRequestsForLoggedInDeveloperIdsAsync", + PullRequestView = PullRequestView.Mine, + Requestor = requestor ?? Guid.NewGuid(), + }; + + dynamic context = new ExpandoObject(); + var contextDict = (IDictionary)context; + contextDict.Add("Requestor", parameters.Requestor); + + try + { + await UpdateDataStoreAsync(parameters, UpdateDataForDeveloperPullRequestsAsync); + } + catch (Exception ex) + { + contextDict.Add("ErrorMessage", ex.Message); + SendErrorUpdateEvent(_log, this, parameters.Requestor, context, ex); + return; + } + + SendPullRequestUpdateEvent(_log, this, parameters.Requestor, context); + } + + private async Task UpdateDataForDeveloperPullRequestsAsync(DataStoreOperationParameters parameters) + { + _log.Debug($"Inside UpdateDataForDeveloperPullRequestsAsync with Parameters: {parameters}"); + + dynamic context = new ExpandoObject(); + var contextDict = (IDictionary)context; + contextDict.Add("Requestor", parameters.Requestor); + + try + { + // This is a loop over a subset of repositories with a specific developer ID and pull request view specified. + var repositoryReferences = RepositoryReference.GetAll(DataStore); + foreach (var repositoryRef in repositoryReferences) + { + var uri = new AzureUri(repositoryRef.Repository.CloneUrl); + var developerId = repositoryRef.Developer.DeveloperLoginId; + + var uris = new List + { + new(repositoryRef.Repository.CloneUrl), + }; + + var suboperationParameters = new DataStoreOperationParameters + { + Uris = uris, + DeveloperId = repositoryRef.Developer.DeveloperId, + RequestOptions = parameters.RequestOptions, + OperationName = "UpdateDataForDeveloperPullRequestsAsync", + PullRequestView = PullRequestView.Mine, + Requestor = parameters.Requestor, + }; + + await UpdateDataForPullRequestsAsync(suboperationParameters); + } + } + catch (Exception ex) + { + contextDict.Add("ErrorMessage", ex.Message); + SendErrorUpdateEvent(_log, this, parameters.Requestor, context, ex); + return; + } + + SendDeveloperUpdateEvent(_log, this, parameters.Requestor, context); + } + public TeamProject GetTeamProject(string projectName, DeveloperId.DeveloperId developerId, Uri connection) { var result = GetConnection(connection, developerId); @@ -651,12 +796,6 @@ public TeamProject GetTeamProject(string projectName, DeveloperId.DeveloperId de private async Task UpdateDataStoreAsync(DataStoreOperationParameters parameters, Func asyncAction) { parameters.RequestOptions ??= RequestOptions.RequestOptionsDefault(); - if (parameters.DeveloperId == null) - { - _log.Error($"Specified DeveloperId was not found: {parameters.LoginId}"); - throw new ArgumentException($"Specified DeveloperId was not found:"); - } - using var tx = DataStore.Connection!.BeginTransaction(); try @@ -696,6 +835,11 @@ private static void SendErrorUpdateEvent(ILogger logger, object? source, Guid re SendUpdateEvent(logger, source, DataManagerUpdateKind.Error, requestor, context, ex); } + private static void SendDeveloperUpdateEvent(ILogger logger, object? source, Guid requestor, dynamic context, Exception? ex = null) + { + SendUpdateEvent(logger, source, DataManagerUpdateKind.Developer, requestor, context, ex); + } + private static void SendCancelUpdateEvent(ILogger logger, object? source, Guid requestor, dynamic context, Exception? ex = null) { SendUpdateEvent(logger, source, DataManagerUpdateKind.Cancel, requestor, context, ex); @@ -763,6 +907,12 @@ public static ConnectionResult GetConnection(Uri connectionUri, DeveloperId.Deve } public IEnumerable GetRepositories() + { + ValidateDataStore(); + return Repository.GetAll(DataStore); + } + + public IEnumerable GetDeveloperRepositories() { ValidateDataStore(); return Repository.GetAllWithReference(DataStore); @@ -792,12 +942,12 @@ private void PruneObsoleteData() Query.DeleteBefore(DataStore, DateTime.UtcNow - _dataRetentionTime); PullRequests.DeleteBefore(DataStore, DateTime.UtcNow - _dataRetentionTime); WorkItemType.DeleteBefore(DataStore, DateTime.UtcNow - _dataRetentionTime); - Identity.DeleteBefore(DataStore, DateTime.UtcNow - _dataRetentionTime); // The following are not yet pruned, need to ensure there are no current key references // before deletion. // * Projects are referenced by Pull Requests and Queries. // * Organizations are referenced by Projects. + // * Identity is referenced by ProjectReference and RepositoryReference } // Sets a last-updated in the MetaData. diff --git a/src/AzureExtension/DataManager/AzureDataManagerCache.cs b/src/AzureExtension/DataManager/AzureDataManagerCache.cs index 50a8590..13ddaa8 100644 --- a/src/AzureExtension/DataManager/AzureDataManagerCache.cs +++ b/src/AzureExtension/DataManager/AzureDataManagerCache.cs @@ -150,7 +150,7 @@ private dynamic CreateUpdateEventContext(int errors, int accountsUpdated, int ac private void UpdateOrganization(Account account, DeveloperId.DeveloperId developerId, VssConnection connection, CancellationToken cancellationToken) { // Update account identity information: - var identity = Identity.GetOrCreateIdentity(DataStore, connection.AuthorizedIdentity, connection, true); + var identity = Identity.GetOrCreateIdentity(DataStore, connection.AuthorizedIdentity, connection, developerId.LoginId); _log.Verbose($"Updating organization: {account.AccountName}"); var organization = Organization.GetOrCreate(DataStore, account.AccountUri); diff --git a/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs b/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs new file mode 100644 index 0000000..f2fde74 --- /dev/null +++ b/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs @@ -0,0 +1,71 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using DevHomeAzureExtension.DataManager; +using Microsoft.TeamFoundation.Policy.WebApi; +using Serilog; + +namespace DevHomeAzureExtension; + +public partial class AzureDataManager +{ + // This is how frequently the DataStore update occurs. + private static readonly TimeSpan _updateInterval = TimeSpan.FromMinutes(5); + private static DateTime _lastUpdateTime = DateTime.MinValue; + + public static async Task Update() + { + // Only update per the update interval. + // This is intended to be dynamic in the future. + if (DateTime.Now - _lastUpdateTime < _updateInterval) + { + return; + } + + try + { + await UpdateDeveloperPullRequests(); + } + catch (Exception ex) + { + Log.Error(ex, "Update failed unexpectedly."); + } + + _lastUpdateTime = DateTime.Now; + } + + public static async Task UpdateDeveloperPullRequests() + { + Log.Debug($"Executing UpdateDeveloperPullRequests"); + + var cacheManager = CacheManager.GetInstance(); + if (cacheManager.UpdateInProgress) + { + Log.Information("Cache is being updated, skipping Developer Pull Request Update"); + return; + } + + var identifier = Guid.NewGuid(); + using var dataManager = CreateInstance(identifier.ToString()) ?? throw new DataStoreInaccessibleException(); + await dataManager.UpdatePullRequestsForLoggedInDeveloperIdsAsync(null, identifier); + + // Show any new notifications that were created from the pull request update. + /* + var notifications = dataManager.GetNotifications(); + foreach (var notification in notifications) + { + // Show notifications for failed checkruns for Developer users. + if (notification.Type == NotificationType.CheckRunFailed && notification.User.IsDeveloper) + { + notification.ShowToast(); + } + + // Show notifications for new reviews. + if (notification.Type == NotificationType.NewReview) + { + notification.ShowToast(); + } + } + */ + } +} diff --git a/src/AzureExtension/DataManager/DataManagerUpdateEventArgs.cs b/src/AzureExtension/DataManager/DataManagerUpdateEventArgs.cs index a48f3c3..2a3873e 100644 --- a/src/AzureExtension/DataManager/DataManagerUpdateEventArgs.cs +++ b/src/AzureExtension/DataManager/DataManagerUpdateEventArgs.cs @@ -12,6 +12,7 @@ public enum DataManagerUpdateKind Error, Cache, Cancel, + Developer, } public class DataManagerUpdateEventArgs : EventArgs diff --git a/src/AzureExtension/DataManager/IAzureDataManager.cs b/src/AzureExtension/DataManager/IAzureDataManager.cs index b7bc602..ec637d5 100644 --- a/src/AzureExtension/DataManager/IAzureDataManager.cs +++ b/src/AzureExtension/DataManager/IAzureDataManager.cs @@ -26,15 +26,21 @@ public interface IAzureDataManager : IDisposable Task UpdateDataForPullRequestsAsync(AzureUri repositoryUri, string developerLogin, PullRequestView view, RequestOptions? options = null, Guid? requestor = null); + Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(RequestOptions? options = null, Guid? requestor = null); + Query? GetQuery(string queryId, string developerId); Query? GetQuery(AzureUri queryUri, string developerId); IEnumerable GetRepositories(); + IEnumerable GetDeveloperRepositories(); + // Repository name may not be unique across projects, and projects may not be unique across // organizations, so we need all three to identify the repository. PullRequests? GetPullRequests(string organization, string project, string repositoryName, string developerId, PullRequestView view); PullRequests? GetPullRequests(AzureUri repositoryUri, string developerId, PullRequestView view); + + PullRequests? GetPullRequestsForLoggedInDeveloperIds(); } diff --git a/src/AzureExtension/DataModel/AzureDataStoreSchema.cs b/src/AzureExtension/DataModel/AzureDataStoreSchema.cs index cf96ec9..27ced27 100644 --- a/src/AzureExtension/DataModel/AzureDataStoreSchema.cs +++ b/src/AzureExtension/DataModel/AzureDataStoreSchema.cs @@ -14,7 +14,7 @@ public AzureDataStoreSchema() } // Update this anytime incompatible changes happen with a released version. - private const long SchemaVersionValue = 0x0006; + private const long SchemaVersionValue = 0x0007; private const string Metadata = @"CREATE TABLE Metadata (" + @@ -30,7 +30,7 @@ public AzureDataStoreSchema() "Name TEXT NOT NULL COLLATE NOCASE," + "InternalId TEXT NOT NULL," + "Avatar TEXT NOT NULL COLLATE NOCASE," + - "IsDeveloper INTEGER NOT NULL," + + "DeveloperLoginId TEXT NOT NULL," + "TimeUpdated INTEGER NOT NULL" + ");" + @@ -89,6 +89,10 @@ public AzureDataStoreSchema() "TimeUpdated INTEGER NOT NULL" + ");" + + // While Name and ProjectId should be unique, it is possible renaming occurs and + // we might have a collision if we have cached a repository prior to rename and + // then encounter a different repository with that name. Therefore we will not + // create a unique index on ProjectId and Name. // Repository InternalId is a Guid, so by definition is unique. "CREATE UNIQUE INDEX IDX_Repository_InternalId ON Repository (InternalId);"; @@ -138,7 +142,7 @@ public AzureDataStoreSchema() private const string PullRequests = @"CREATE TABLE PullRequests (" + "Id INTEGER PRIMARY KEY NOT NULL," + - "RepositoryName TEXT NOT NULL COLLATE NOCASE," + + "RepositoryId INTEGER NOT NULL," + "DeveloperLogin TEXT NOT NULL COLLATE NOCASE," + "Results TEXT NOT NULL," + "ProjectId INTEGER NOT NULL," + @@ -148,7 +152,7 @@ public AzureDataStoreSchema() // Developer Pull requests are unique on Org / Project / Repository and // the developer login, and the view. - "CREATE UNIQUE INDEX IDX_PullRequests_ProjectIdRepositoryNameDeveloperLoginViewId ON PullRequests (ProjectId, RepositoryName, DeveloperLogin, ViewId);"; + "CREATE UNIQUE INDEX IDX_PullRequests_ProjectIdRepositoryIdDeveloperLoginViewId ON PullRequests (ProjectId, RepositoryId, DeveloperLogin, ViewId);"; // All Sqls together. private static readonly List _schemaSqlsValue = diff --git a/src/AzureExtension/DataModel/DataObjects/Identity.cs b/src/AzureExtension/DataModel/DataObjects/Identity.cs index 2bb03e5..0fcbf4e 100644 --- a/src/AzureExtension/DataModel/DataObjects/Identity.cs +++ b/src/AzureExtension/DataModel/DataObjects/Identity.cs @@ -5,6 +5,7 @@ using System.Text.Json.Serialization; using Dapper; using Dapper.Contrib.Extensions; +using DevHomeAzureExtension.DeveloperId; using DevHomeAzureExtension.Helpers; using Microsoft.VisualStudio.Services.Profile; using Microsoft.VisualStudio.Services.Profile.Client; @@ -13,6 +14,7 @@ namespace DevHomeAzureExtension.DataModel; +// This represents an Azure DevOps Identity or IdentityRef. [Table("Identity")] public class Identity { @@ -42,9 +44,9 @@ public class Identity public string Avatar { get; set; } = string.Empty; - // Represents whether this identity is associated with a DeveloperId that is logged in. - // This is the backing database column for the IsLoggedInDeveloper property. - public long IsDeveloper { get; set; } = DataStore.NoForeignKey; + // The DeveloperLoginId associated with this identity, if one exists. + // Empty means there is no associated LoggedInDeveloperId. + public string DeveloperLoginId { get; set; } = string.Empty; [JsonIgnore] public long TimeUpdated { get; set; } = DataStore.NoForeignKey; @@ -57,7 +59,23 @@ public class Identity [Write(false)] [Computed] [JsonIgnore] - public bool IsLoggedInDeveloper => IsDeveloper != 0L; + public bool IsLoggedInDeveloper => !string.IsNullOrEmpty(DeveloperLoginId); + + [Write(false)] + [Computed] + public DeveloperId.DeveloperId? DeveloperId + { + get + { + if (!IsLoggedInDeveloper) + { + return null; + } + + var devIdProvider = DeveloperIdProvider.GetInstance(); + return devIdProvider.GetDeveloperIdFromAccountIdentifier(DeveloperLoginId); + } + } public string ToJson() => JsonSerializer.Serialize(this); @@ -145,7 +163,7 @@ private static Identity CreateFromIdentity(Microsoft.VisualStudio.Services.Ident }; } - public static Identity AddOrUpdateIdentity(DataStore dataStore, Identity identity, bool isDeveloper = false) + public static Identity AddOrUpdateIdentity(DataStore dataStore, Identity identity, string developerLoginId = "") { // Check for existing Identity data. var existingIdentity = GetByInternalId(dataStore, identity.InternalId); @@ -153,11 +171,11 @@ public static Identity AddOrUpdateIdentity(DataStore dataStore, Identity identit { identity.Id = existingIdentity.Id; - // If this is a developer, set to developer, but do not set to false. + // If this is a developer, set to developer, but do not set it. // We presume not a developer unless it is explicitly set. - if (isDeveloper) + if (!string.IsNullOrEmpty(developerLoginId)) { - identity.IsDeveloper = 1; + identity.DeveloperLoginId = developerLoginId; } dataStore.Connection!.Update(identity); @@ -192,7 +210,7 @@ public static Identity Get(DataStore? dataStore, long id) } // Creation from an Azure IdentityRef object. - public static Identity GetOrCreateIdentity(DataStore dataStore, IdentityRef? identityRef, VssConnection connection, bool isDeveloper = false) + public static Identity GetOrCreateIdentity(DataStore dataStore, IdentityRef? identityRef, VssConnection connection, string developerLoginId = "") { ArgumentNullException.ThrowIfNull(identityRef); @@ -210,18 +228,19 @@ public static Identity GetOrCreateIdentity(DataStore dataStore, IdentityRef? ide // We don't want to create an identity object and download a new avatar unless it needs to // be updated. In the event of an empty avatar we will retry more frequently to update it, // but not every time. + var isDeveloper = !string.IsNullOrEmpty(developerLoginId); if (existing is null || (isDeveloper && !existing.IsLoggedInDeveloper) || ((DateTime.UtcNow - existing.UpdatedAt) > _updateThreshold) || (string.IsNullOrEmpty(existing.Avatar) && ((DateTime.UtcNow - existing.UpdatedAt) > _avatarRetryDelay))) { var newIdentity = CreateFromIdentityRef(identityRef, connection); - return AddOrUpdateIdentity(dataStore, newIdentity, isDeveloper); + return AddOrUpdateIdentity(dataStore, newIdentity, developerLoginId); } return existing; } // Creation from an Azure Identity object. - public static Identity GetOrCreateIdentity(DataStore dataStore, Microsoft.VisualStudio.Services.Identity.Identity? identity, VssConnection connection, bool isDeveloper = false) + public static Identity GetOrCreateIdentity(DataStore dataStore, Microsoft.VisualStudio.Services.Identity.Identity? identity, VssConnection connection, string developerLoginId = "") { ArgumentNullException.ThrowIfNull(identity); @@ -232,11 +251,12 @@ public static Identity GetOrCreateIdentity(DataStore dataStore, Microsoft.Visual // We don't want to create an identity object and download a new avatar unlesss it needs to // be updated. In the event of an empty avatar we will retry more frequently to update it, // but not every time. + var isDeveloper = !string.IsNullOrEmpty(developerLoginId); if (existing is null || (isDeveloper && !existing.IsLoggedInDeveloper) || ((DateTime.UtcNow - existing.UpdatedAt) > _updateThreshold) || (string.IsNullOrEmpty(existing.Avatar) && ((DateTime.UtcNow - existing.UpdatedAt) > _avatarRetryDelay))) { var newIdentity = CreateFromIdentity(identity, connection); - return AddOrUpdateIdentity(dataStore, newIdentity, isDeveloper); + return AddOrUpdateIdentity(dataStore, newIdentity, developerLoginId); } return existing; diff --git a/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs b/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs new file mode 100644 index 0000000..64ad0a7 --- /dev/null +++ b/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Dapper; +using Dapper.Contrib.Extensions; +using DevHomeAzureExtension.Helpers; +using Microsoft.TeamFoundation.Policy.WebApi; +using Serilog; + +namespace DevHomeAzureExtension.DataModel; + +[Table("PullRequestPolicyStatus")] +public class PullRequestPolicyStatus +{ + private static readonly Lazy _logger = new(() => Serilog.Log.ForContext("SourceContext", $"DataModel/{nameof(PullRequests)}")); + + private static readonly ILogger _log = _logger.Value; + + [Key] + public long Id { get; set; } = DataStore.NoForeignKey; + + // This should suffice as a unique identifier. + public string ArtifactId { get; set; } = string.Empty; + + // Key in Project table + public long ProjectId { get; set; } = DataStore.NoForeignKey; + + public long PullRequestId { get; set; } = DataStore.NoForeignKey; + + public long TimeUpdated { get; set; } = DataStore.NoForeignKey; + + public long PolicyStatusId { get; set; } = DataStore.NoForeignKey; + + public override string ToString() => ArtifactId; + + [Write(false)] + private DataStore? DataStore { get; set; } + + [Write(false)] + [Computed] + public Project Project => Project.Get(DataStore, ProjectId); + + [Write(false)] + [Computed] + public PolicyStatus PolicyStatus => (PolicyStatus)PolicyStatusId; + + [Write(false)] + [Computed] + public DateTime UpdatedAt => TimeUpdated.ToDateTime(); + + // Map PolicyEvaluationStatus to our enum. Our enum is sorted, but the PolicyEvaluationStatus is not. + public static PolicyStatus GetFromPolicyEvaluationStatus(PolicyEvaluationStatus? policyEvaluationStatus) + { + return policyEvaluationStatus switch + { + PolicyEvaluationStatus.NotApplicable => PolicyStatus.NotApplicable, + PolicyEvaluationStatus.Approved => PolicyStatus.Approved, + PolicyEvaluationStatus.Rejected => PolicyStatus.Rejected, + PolicyEvaluationStatus.Queued => PolicyStatus.Queued, + PolicyEvaluationStatus.Running => PolicyStatus.Running, + PolicyEvaluationStatus.Broken => PolicyStatus.Broken, + _ => PolicyStatus.Unknown, + }; + } +} diff --git a/src/AzureExtension/DataModel/DataObjects/PullRequests.cs b/src/AzureExtension/DataModel/DataObjects/PullRequests.cs index 9067be0..e324669 100644 --- a/src/AzureExtension/DataModel/DataObjects/PullRequests.cs +++ b/src/AzureExtension/DataModel/DataObjects/PullRequests.cs @@ -22,7 +22,8 @@ public class PullRequests [Key] public long Id { get; set; } = DataStore.NoForeignKey; - public string RepositoryName { get; set; } = string.Empty; + // Key in Repository table + public long RepositoryId { get; set; } = DataStore.NoForeignKey; // Key in Project table public long ProjectId { get; set; } = DataStore.NoForeignKey; @@ -36,7 +37,7 @@ public class PullRequests public long TimeUpdated { get; set; } = DataStore.NoForeignKey; - public override string ToString() => DeveloperLogin + "/" + RepositoryName; + public override string ToString() => DeveloperLogin + "/" + Repository.Name; [Write(false)] private DataStore? DataStore { get; set; } @@ -45,6 +46,10 @@ public class PullRequests [Computed] public Project Project => Project.Get(DataStore, ProjectId); + [Write(false)] + [Computed] + public Repository Repository => Repository.Get(DataStore, RepositoryId); + [Write(false)] [Computed] public PullRequestView View => (PullRequestView)ViewId; @@ -57,11 +62,11 @@ public class PullRequests [Computed] public DateTime UpdatedAt => TimeUpdated.ToDateTime(); - private static PullRequests Create(string repositoryName, long projectId, string developerLogin, PullRequestView view, string pullRequests) + private static PullRequests Create(long repositoryId, long projectId, string developerLogin, PullRequestView view, string pullRequests) { return new PullRequests { - RepositoryName = repositoryName, + RepositoryId = repositoryId, ProjectId = projectId, DeveloperLogin = developerLogin, Results = pullRequests, @@ -72,7 +77,7 @@ private static PullRequests Create(string repositoryName, long projectId, string private static PullRequests AddOrUpdate(DataStore dataStore, PullRequests pullRequests) { - var existing = Get(dataStore, pullRequests.ProjectId, pullRequests.RepositoryName, pullRequests.DeveloperLogin, pullRequests.View); + var existing = Get(dataStore, pullRequests.ProjectId, pullRequests.RepositoryId, pullRequests.DeveloperLogin, pullRequests.View); if (existing is not null) { // Update threshold is in case there are many requests in a short period of time. @@ -110,13 +115,13 @@ public static PullRequests Get(DataStore dataStore, long id) return pullRequests ?? new PullRequests(); } - public static PullRequests? Get(DataStore dataStore, long projectId, string repositoryName, string developerLogin, PullRequestView view) + public static PullRequests? Get(DataStore dataStore, long projectId, long repositoryId, string developerLogin, PullRequestView view) { - var sql = @"SELECT * FROM PullRequests WHERE ProjectId = @ProjectId AND RepositoryName = @RepositoryName AND DeveloperLogin = @DeveloperLogin AND ViewId = @ViewId;"; + var sql = @"SELECT * FROM PullRequests WHERE ProjectId = @ProjectId AND RepositoryId = @RepositoryId AND DeveloperLogin = @DeveloperLogin AND ViewId = @ViewId;"; var param = new { ProjectId = projectId, - RepositoryName = repositoryName, + RepositoryId = repositoryId, DeveloperLogin = developerLogin, ViewId = (long)view, }; @@ -140,12 +145,18 @@ public static PullRequests Get(DataStore dataStore, long id) return null; } - return Get(dataStore, project.Id, repositoryName, developerLogin, view); + var repository = Repository.Get(dataStore, project.Id, repositoryName); + if (repository == null) + { + return null; + } + + return Get(dataStore, project.Id, repository.Id, developerLogin, view); } - public static PullRequests GetOrCreate(DataStore dataStore, string repositoryName, long projectId, string developerId, PullRequestView view, string pullRequests) + public static PullRequests GetOrCreate(DataStore dataStore, long repositoryId, long projectId, string developerId, PullRequestView view, string pullRequests) { - var newDeveloperPullRequests = Create(repositoryName, projectId, developerId, view, pullRequests); + var newDeveloperPullRequests = Create(repositoryId, projectId, developerId, view, pullRequests); return AddOrUpdate(dataStore, newDeveloperPullRequests); } diff --git a/src/AzureExtension/DataModel/DataObjects/Repository.cs b/src/AzureExtension/DataModel/DataObjects/Repository.cs index 90b6f95..f6eb137 100644 --- a/src/AzureExtension/DataModel/DataObjects/Repository.cs +++ b/src/AzureExtension/DataModel/DataObjects/Repository.cs @@ -21,6 +21,7 @@ public class Repository [Key] public long Id { get; set; } = DataStore.NoForeignKey; + // Name should be unique within a project scope. public string Name { get; set; } = string.Empty; // Guid representation by ADO. @@ -130,6 +131,29 @@ public static Repository Get(DataStore? dataStore, long id) return repository ?? new Repository(); } + public static Repository? GetByName(DataStore? dataStore, long projectId, string name) + { + if (dataStore == null) + { + return null; + } + + var sql = @"SELECT * FROM Repository WHERE ProjectId = @ProjectId AND Name = @Name;"; + var param = new + { + ProjectId = projectId, + Name = name, + }; + + var repository = dataStore.Connection!.QueryFirstOrDefault(sql, param, null); + if (repository != null) + { + repository.DataStore = dataStore; + } + + return repository; + } + public static IEnumerable GetAll(DataStore dataStore) { var repositories = dataStore.Connection!.GetAll() ?? []; @@ -165,6 +189,27 @@ public static IEnumerable GetAll(DataStore dataStore) return GetByInternalId(dataStore, repository.Id.ToString()); } + public static Repository? Get(DataStore dataStore, long projectId, string repositoryName) + { + var sql = @"SELECT * FROM Repository WHERE ProjectId = @ProjectId AND Name = @RepositoryName;"; + var param = new + { + ProjectId = projectId, + RepositoryName = repositoryName, + }; + + // In rare cases we might have a rename situation that results in more than one record. + // This should be an extremely rare edge case that can be resolved in the next repository + // cache update so we will assume a single record is correct. + var repository = dataStore.Connection!.QueryFirstOrDefault(sql, param, null); + if (repository != null) + { + repository.DataStore = dataStore; + } + + return repository; + } + public static IEnumerable GetAllWithReference(DataStore dataStore) { var sql = @"SELECT * FROM Repository AS R WHERE R.Id IN (SELECT RepositoryId FROM RepositoryReference)"; diff --git a/src/AzureExtension/DataModel/DataObjects/RepositoryReference.cs b/src/AzureExtension/DataModel/DataObjects/RepositoryReference.cs index a40294b..3e189e5 100644 --- a/src/AzureExtension/DataModel/DataObjects/RepositoryReference.cs +++ b/src/AzureExtension/DataModel/DataObjects/RepositoryReference.cs @@ -15,11 +15,11 @@ namespace DevHomeAzureExtension.DataModel; [Table("RepositoryReference")] public class RepositoryReference { - private static readonly Lazy _log = new(() => Serilog.Log.ForContext("SourceContext", $"DataModel/{nameof(RepositoryReference)}")); + private static readonly Lazy _logger = new(() => Serilog.Log.ForContext("SourceContext", $"DataModel/{nameof(RepositoryReference)}")); - private static readonly ILogger Log = _log.Value; + private static readonly ILogger _log = _logger.Value; - private static readonly long WeightPullRequest = 1; + private static readonly long _weightPullRequest = 1; [Key] public long Id { get; set; } = DataStore.NoForeignKey; @@ -33,12 +33,16 @@ public class RepositoryReference [Write(false)] [Computed] - public long Value => PullRequestCount * WeightPullRequest; + public long Value => PullRequestCount * _weightPullRequest; [Write(false)] [Computed] public Identity Developer => Identity.Get(DataStore, DeveloperId); + [Write(false)] + [Computed] + public Repository Repository => Repository.Get(DataStore, RepositoryId); + [Write(false)] private DataStore? DataStore { get; set; } @@ -99,13 +103,25 @@ public static RepositoryReference GetOrCreate(DataStore dataStore, long reposito return repositoryReference; } + public static IEnumerable GetAll(DataStore dataStore) + { + var repositoryReferences = dataStore.Connection!.GetAll() ?? []; + foreach (var repositoryReference in repositoryReferences) + { + repositoryReference.DataStore = dataStore; + } + + _log.Verbose("Getting all repository references."); + return repositoryReferences; + } + public static void DeleteUnreferenced(DataStore dataStore) { // Delete any RepositoryReferences for repositories that do not exist. var sql = @"DELETE FROM RepositoryReference WHERE (RepositoryId NOT IN (SELECT Id FROM Repository)) OR (DeveloperId NOT IN (SELECT Id FROM Identity))"; var command = dataStore.Connection!.CreateCommand(); command.CommandText = sql; - Log.Verbose(DataStore.GetCommandLogMessage(sql, command)); + _log.Verbose(DataStore.GetCommandLogMessage(sql, command)); command.ExecuteNonQuery(); } } diff --git a/src/AzureExtension/DataModel/Enums/PolicyStatus.cs b/src/AzureExtension/DataModel/Enums/PolicyStatus.cs new file mode 100644 index 0000000..f1d355e --- /dev/null +++ b/src/AzureExtension/DataModel/Enums/PolicyStatus.cs @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace DevHomeAzureExtension.DataModel; + +public enum PolicyStatus +{ + // Sorted by severity. Most severe is lowest. + Broken = 1, + Rejected = 2, + Queued = 3, + Running = 4, + Approved = 5, + NotApplicable = 6, + Unknown = 7, +} diff --git a/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs index 894f17a..27970c6 100644 --- a/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs +++ b/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs @@ -62,13 +62,21 @@ private PullRequestView GetPullRequestView(string viewStr) private string GetIconForPullRequestStatus(string? prStatus) { - return prStatus switch + prStatus ??= string.Empty; + if (Enum.TryParse(prStatus, false, out var policyStatus)) { - "Approved" => IconLoader.GetIconAsBase64("PullRequestApproved.png"), - "Waiting" => IconLoader.GetIconAsBase64("PullRequestWaiting.png"), - "Rejected" => IconLoader.GetIconAsBase64("PullRequestRejected.png"), - _ => IconLoader.GetIconAsBase64("PullRequestReviewNotStarted.png"), - }; + return policyStatus switch + { + PolicyStatus.Approved => IconLoader.GetIconAsBase64("PullRequestApproved.png"), + PolicyStatus.Running => IconLoader.GetIconAsBase64("PullRequestWaiting.png"), + PolicyStatus.Queued => IconLoader.GetIconAsBase64("PullRequestWaiting.png"), + PolicyStatus.Rejected => IconLoader.GetIconAsBase64("PullRequestRejected.png"), + PolicyStatus.Broken => IconLoader.GetIconAsBase64("PullRequestRejected.png"), + _ => IconLoader.GetIconAsBase64("PullRequestReviewNotStarted.png"), + }; + } + + return string.Empty; } protected override bool ValidateConfiguration(WidgetActionInvokedArgs args) @@ -294,7 +302,7 @@ public override void LoadContentData() { { "title", workItem["Title"]?.GetValue() ?? string.Empty }, { "url", workItem["HtmlUrl"]?.GetValue() ?? string.Empty }, - { "status_icon", GetIconForPullRequestStatus(workItem["Status"]?.GetValue()) }, + { "status_icon", GetIconForPullRequestStatus(workItem["PolicyStatus"]?.GetValue()) }, { "number", element.Key }, { "date", TimeSpanHelper.DateTimeOffsetToDisplayString(dateTime, Log) }, { "user", workItem["CreatedBy"]?["Name"]?.GetValue() ?? string.Empty }, diff --git a/src/AzureExtensionServer/Program.cs b/src/AzureExtensionServer/Program.cs index 936db8a..e592a30 100644 --- a/src/AzureExtensionServer/Program.cs +++ b/src/AzureExtensionServer/Program.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using DevHomeAzureExtension.Contracts; +using DevHomeAzureExtension.DataManager; using DevHomeAzureExtension.DataModel; using DevHomeAzureExtension.DevBox; using DevHomeAzureExtension.DevBox.Models; @@ -157,15 +158,31 @@ private static void HandleCOMServerActivation() widgetServer.RegisterWidget(() => widgetProviderInstance); // Cache manager updates account data. - using var cacheManager = DataManager.CacheManager.GetInstance(); + using var cacheManager = CacheManager.GetInstance(); cacheManager?.Start(); + // Set up the data updater. This will schedule updating the Developer Pull Requests. + using var dataUpdater = new DataUpdater(AzureDataManager.Update); + _ = dataUpdater.Start(); + + // Add an update whenever CacheManager is updated. + CacheManager.GetInstance().OnUpdate += HandleCacheUpdate; + // This will make the main thread wait until the event is signaled by the extension class. // Since we have single instance of the extension object, we exit as soon as it is disposed. extensionDisposedEvent.WaitOne(); Log.Information($"Extension is disposed."); } + private static void HandleCacheUpdate(object? source, CacheManagerUpdateEventArgs e) + { + if (e.Kind == CacheManagerUpdateKind.Updated) + { + Log.Debug("Cache was updated, updating developer pull requests."); + _ = AzureDataManager.UpdateDeveloperPullRequests(); + } + } + private static void LogPackageInformation() { var relatedPackageFamilyNames = new string[] diff --git a/test/AzureExtension/DataStore/DataObjectTests.cs b/test/AzureExtension/DataStore/DataObjectTests.cs index a908111..ee7a44e 100644 --- a/test/AzureExtension/DataStore/DataObjectTests.cs +++ b/test/AzureExtension/DataStore/DataObjectTests.cs @@ -345,9 +345,11 @@ public void ReadAndWritePullRequests() var org = Organization.GetOrCreate(dataStore, new Uri("https://dev.azure.com/organization/")); Assert.IsNotNull(org); dataStore.Connection.Insert(new Project { Name = "project", InternalId = "11", OrganizationId = org.Id }); + dataStore.Connection.Insert(new Repository { Name = "repository1", InternalId = "21", CloneUrl = "https://organization/project/_git/repository1/", ProjectId = 1 }); + dataStore.Connection.Insert(new Repository { Name = "repository2", InternalId = "22", CloneUrl = "https://organization/project/_git/repository2/", ProjectId = 1 }); - var p1 = PullRequests.GetOrCreate(dataStore, "repository1", 1, "foo@bar", PullRequestView.Mine, "Results"); - var p2 = PullRequests.GetOrCreate(dataStore, "repository2", 1, "foo@bar", PullRequestView.Mine, "Results"); + var p1 = PullRequests.GetOrCreate(dataStore, 1, 1, "foo@bar", PullRequestView.Mine, "Results"); + var p2 = PullRequests.GetOrCreate(dataStore, 2, 1, "foo@bar", PullRequestView.Mine, "Results"); tx.Commit(); // Verify retrieval and input into data objects. @@ -358,15 +360,15 @@ public void ReadAndWritePullRequests() var pull = PullRequests.Get(dataStore, i); Assert.IsNotNull(pull); Assert.AreEqual($"foo@bar", pull.DeveloperLogin); - Assert.AreEqual($"repository{i}", pull.RepositoryName); + Assert.AreEqual($"repository{i}", pull.Repository.Name); Assert.AreEqual("organization", pull.Project.Organization.Name); Assert.AreEqual(PullRequestView.Mine, pull.View); - TestContext?.WriteLine($" Name: {pull.RepositoryName} Results: {pull.Results}"); + TestContext?.WriteLine($" Name: {pull.Repository.Name} Results: {pull.Results}"); } var findPull = PullRequests.Get(dataStore, "organization", "project", "repository2", "foo@bar", PullRequestView.Mine); Assert.IsNotNull(findPull); - Assert.AreEqual("repository2", findPull.RepositoryName); + Assert.AreEqual("repository2", findPull.Repository.Name); Assert.AreEqual(PullRequestView.Mine, findPull.View); Assert.AreEqual("project", findPull.Project.Name); } From 8adb91bfc760818a2dacd3772a7cb476e9a1d319 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 19 Jul 2024 16:56:56 -0700 Subject: [PATCH 02/16] Use workitem and identity index lookups instead of embedding --- .../DataManager/AzureDataManager.cs | 24 ++++++++++++------- .../DataManager/IAzureDataManager.cs | 4 ++++ .../Widgets/AzurePullRequestsWidget.cs | 6 ++--- .../Widgets/AzureQueryListWidget.cs | 9 +++---- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/AzureExtension/DataManager/AzureDataManager.cs b/src/AzureExtension/DataManager/AzureDataManager.cs index 0e83b14..dfa4fd7 100644 --- a/src/AzureExtension/DataManager/AzureDataManager.cs +++ b/src/AzureExtension/DataManager/AzureDataManager.cs @@ -247,6 +247,18 @@ public async Task UpdateDataForPullRequestsAsync(AzureUri repositoryUri, string return GetQuery(queryUri.Query, developerId); } + public Identity GetIdentity(long id) + { + ValidateDataStore(); + return Identity.Get(DataStore, id); + } + + public WorkItemType GetWorkItemType(long id) + { + ValidateDataStore(); + return WorkItemType.Get(DataStore, id); + } + public PullRequests? GetPullRequests(string organization, string project, string repositoryName, string developerId, PullRequestView view) { ValidateDataStore(); @@ -453,7 +465,7 @@ private async Task UpdateDataForQueriesAsync(DataStoreOperationParameters parame if (fieldValue == IdentityRefFieldValueName) { var identity = Identity.GetOrCreateIdentity(DataStore, workItem.Fields[field] as IdentityRef, result.Connection); - workItemObjFields.Add(field, identity); + workItemObjFields.Add(field, identity.Id); continue; } @@ -467,7 +479,7 @@ private async Task UpdateDataForQueriesAsync(DataStoreOperationParameters parame workItemType = WorkItemType.GetOrCreateByTeamWorkItemType(DataStore, workItemTypeInfo, project.Id); } - workItemObjFields.Add(field, workItemType); + workItemObjFields.Add(field, workItemType.Id); continue; } @@ -612,8 +624,6 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p // Policy Evaluations API is this: // vstfs:///CodeReview/CodeReviewId/{projectId}/{pullRequestId} var artifactId = $"vstfs:///CodeReview/CodeReviewId/{project.InternalId}/{pullRequest.PullRequestId}"; - Log.Information($"ArtifactId: {artifactId}"); - var policyEvaluations = await policyClient.GetPolicyEvaluationsAsync(project.InternalId, artifactId); if (policyEvaluations != null) { @@ -629,8 +639,6 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p statusReason = policyEvaluation.Configuration.Type.DisplayName; status = evalStatus; } - - Log.Information($" Policy: {policyEvaluation.EvaluationId} Name: {policyEvaluation.Configuration.Type.DisplayName} Enabled: {policyEvaluation.Configuration.IsEnabled} Blocking: {policyEvaluation.Configuration.IsBlocking} {policyEvaluation.Status}"); } } @@ -647,7 +655,7 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p } // Set record - Log.Information($"PullRequest Status: {status} Reason: {statusReason}"); + Log.Debug($"PullRequest: {pullRequest.PullRequestId} Status: {status} Reason: {statusReason}"); //// If this was a developer pull request, track status for notifications. @@ -661,7 +669,7 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p pullRequestObjFields.Add("PolicyStatusReason", statusReason); var creator = Identity.GetOrCreateIdentity(DataStore, pullRequest.CreatedBy, result.Connection); - pullRequestObjFields.Add("CreatedBy", creator); + pullRequestObjFields.Add("CreatedBy", creator.Id); pullRequestObjFields.Add("CreationDate", pullRequest.CreationDate.Ticks); pullRequestObjFields.Add("TargetBranch", pullRequest.TargetRefName); diff --git a/src/AzureExtension/DataManager/IAzureDataManager.cs b/src/AzureExtension/DataManager/IAzureDataManager.cs index ec637d5..f89f46a 100644 --- a/src/AzureExtension/DataManager/IAzureDataManager.cs +++ b/src/AzureExtension/DataManager/IAzureDataManager.cs @@ -32,6 +32,10 @@ public interface IAzureDataManager : IDisposable Query? GetQuery(AzureUri queryUri, string developerId); + Identity GetIdentity(long id); + + WorkItemType GetWorkItemType(long id); + IEnumerable GetRepositories(); IEnumerable GetDeveloperRepositories(); diff --git a/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs index 27970c6..6c39002 100644 --- a/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs +++ b/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs @@ -297,7 +297,7 @@ public override void LoadContentData() // closer-to-correct time than the zero value decades ago, so use DateTime.UtcNow. var dateTicks = workItem["CreationDate"]?.GetValue() ?? DateTime.UtcNow.Ticks; var dateTime = dateTicks.ToDateTime(); - + var creator = DataManager.GetIdentity(workItem["CreatedBy"]?.GetValue() ?? 0L); var item = new JsonObject { { "title", workItem["Title"]?.GetValue() ?? string.Empty }, @@ -305,9 +305,9 @@ public override void LoadContentData() { "status_icon", GetIconForPullRequestStatus(workItem["PolicyStatus"]?.GetValue()) }, { "number", element.Key }, { "date", TimeSpanHelper.DateTimeOffsetToDisplayString(dateTime, Log) }, - { "user", workItem["CreatedBy"]?["Name"]?.GetValue() ?? string.Empty }, + { "user", creator.Name }, { "branch", workItem["TargetBranch"]?.GetValue().Replace("refs/heads/", string.Empty) }, - { "avatar", workItem["CreatedBy"]?["Avatar"]?.GetValue() }, + { "avatar", creator.Avatar }, }; itemsArray.Add(item); diff --git a/src/AzureExtension/Widgets/AzureQueryListWidget.cs b/src/AzureExtension/Widgets/AzureQueryListWidget.cs index 28b2f71..35d4e3d 100644 --- a/src/AzureExtension/Widgets/AzureQueryListWidget.cs +++ b/src/AzureExtension/Widgets/AzureQueryListWidget.cs @@ -300,18 +300,19 @@ public override void LoadContentData() // closer-to-correct time than the zero value decades ago, so use DateTime.UtcNow. var dateTicks = workItem["System.ChangedDate"]?.GetValue() ?? DateTime.UtcNow.Ticks; var dateTime = dateTicks.ToDateTime(); - + var creator = DataManager.GetIdentity(workItem["System.CreatedBy"]?.GetValue() ?? 0L); + var workItemType = DataManager.GetWorkItemType(workItem["System.WorkItemType"]?.GetValue() ?? 0L); var item = new JsonObject { { "title", workItem["System.Title"]?.GetValue() ?? string.Empty }, { "url", workItem[AzureDataManager.WorkItemHtmlUrlFieldName]?.GetValue() ?? string.Empty }, - { "icon", GetIconForType(workItem["System.WorkItemType"]?["Name"]?.GetValue()) }, + { "icon", GetIconForType(workItemType.Name) }, { "status_icon", GetIconForStatusState(workItem["System.State"]?.GetValue()) }, { "number", element.Key }, { "date", TimeSpanHelper.DateTimeOffsetToDisplayString(dateTime, Log) }, - { "user", workItem["System.CreatedBy"]?["Name"]?.GetValue() ?? string.Empty }, + { "user", creator.Avatar }, { "status", workItem["System.State"]?.GetValue() ?? string.Empty }, - { "avatar", workItem["System.CreatedBy"]?["Avatar"]?.GetValue() ?? string.Empty }, + { "avatar", creator.Avatar }, }; itemsArray.Add(item); From 3dda2065a8624f9963e0a7d6a2ad52560cd5afa1 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 22 Jul 2024 03:30:06 -0700 Subject: [PATCH 03/16] Add notifications and pull request status tracking --- .../DataManager/AzureDataManager.cs | 340 ++++++++++++------ .../DataManager/AzureDataManagerUpdate.cs | 11 +- .../DataManager/IAzureDataManager.cs | 2 + .../DataModel/AzureDataStoreSchema.cs | 37 +- .../DataModel/DataObjects/Notification.cs | 262 ++++++++++++++ .../DataObjects/PullRequestPolicyStatus.cs | 134 ++++++- .../DataModel/Enums/NotificationType.cs | 12 + .../Strings/en-US/Resources.resw | 12 + 8 files changed, 695 insertions(+), 115 deletions(-) create mode 100644 src/AzureExtension/DataModel/DataObjects/Notification.cs create mode 100644 src/AzureExtension/DataModel/Enums/NotificationType.cs diff --git a/src/AzureExtension/DataManager/AzureDataManager.cs b/src/AzureExtension/DataManager/AzureDataManager.cs index dfa4fd7..cfa6d8a 100644 --- a/src/AzureExtension/DataManager/AzureDataManager.cs +++ b/src/AzureExtension/DataManager/AzureDataManager.cs @@ -46,6 +46,10 @@ public partial class AzureDataManager : IAzureDataManager, IDisposable // Most data that has not been updated within this time will be removed. private static readonly TimeSpan _dataRetentionTime = TimeSpan.FromDays(1); + private static readonly TimeSpan _notificationRetentionTime = TimeSpan.FromDays(7); + + private static readonly TimeSpan _pullRequestStaleTime = TimeSpan.FromDays(1); + private static readonly string _lastUpdatedKeyName = "LastUpdated"; private static readonly string _name = nameof(AzureDataManager); @@ -259,6 +263,12 @@ public WorkItemType GetWorkItemType(long id) return WorkItemType.Get(DataStore, id); } + public IEnumerable GetNotifications(DateTime? since = null, bool includeToasted = false) + { + ValidateDataStore(); + return Notification.Get(DataStore, since, includeToasted); + } + public PullRequests? GetPullRequests(string organization, string project, string repositoryName, string developerId, PullRequestView view) { ValidateDataStore(); @@ -494,7 +504,7 @@ private async Task UpdateDataForQueriesAsync(DataStoreOperationParameters parame #if DEBUG WriteIndented = true, #else - WriteIndented = false, + WriteIndented = false, #endif }; @@ -534,20 +544,20 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p // Iterate over and process each Uri in the set. foreach (var azureUri in parameters.Uris) { - var result = GetConnection(azureUri.Connection, parameters.DeveloperId); - if (result.Result != ResultType.Success) + var connectionResult = GetConnection(azureUri.Connection, parameters.DeveloperId); + if (connectionResult.Result != ResultType.Success) { - if (result.Exception != null) + if (connectionResult.Exception != null) { - throw result.Exception; + throw connectionResult.Exception; } else { - throw new AzureAuthorizationException($"Failed getting connection: {azureUri.Connection} for {parameters.DeveloperId.LoginId} with {result.Error}"); + throw new AzureAuthorizationException($"Failed getting connection: {azureUri.Connection} for {parameters.DeveloperId.LoginId} with {connectionResult.Error}"); } } - var gitClient = result.Connection!.GetClient(); + var gitClient = connectionResult.Connection!.GetClient(); if (gitClient == null) { throw new AzureClientException($"Failed getting GitHttpClient for {parameters.DeveloperId.LoginId} and {azureUri.Connection}"); @@ -590,10 +600,10 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p case PullRequestView.Unknown: throw new ArgumentException("PullRequestView is unknown"); case PullRequestView.Mine: - searchCriteria.CreatorId = result.Connection!.AuthorizedIdentity.Id; + searchCriteria.CreatorId = connectionResult.Connection!.AuthorizedIdentity.Id; break; case PullRequestView.Assigned: - searchCriteria.ReviewerId = result.Connection!.AuthorizedIdentity.Id; + searchCriteria.ReviewerId = connectionResult.Connection!.AuthorizedIdentity.Id; break; case PullRequestView.All: /* Nothing different for this */ @@ -601,96 +611,14 @@ private async Task UpdateDataForPullRequestsAsync(DataStoreOperationParameters p } var pullRequests = await gitClient.GetPullRequestsAsync(project.InternalId, repository.InternalId, searchCriteria, null, null, PullRequestResultLimit); - if (pullRequests == null || pullRequests.Count == 0) - { - // If PRs were null or empty, this is a valid result, so create an empty record. - PullRequests.GetOrCreate(DataStore, repository.Id, project.Id, parameters.DeveloperId.LoginId, parameters.PullRequestView, new JsonObject().ToJsonString()); - - // If we had any statuses they should be removed here. - return; - } - - dynamic pullRequestsObj = new ExpandoObject(); - var pullRequestsObjDict = (IDictionary)pullRequestsObj; - - var policyClient = result.Connection!.GetClient(); - foreach (var pullRequest in pullRequests) - { - var status = PolicyStatus.Unknown; - var statusReason = string.Empty; - try - { - // ArtifactId is null in the pull request object and it is not the correct object. The ArtifactId for the - // Policy Evaluations API is this: - // vstfs:///CodeReview/CodeReviewId/{projectId}/{pullRequestId} - var artifactId = $"vstfs:///CodeReview/CodeReviewId/{project.InternalId}/{pullRequest.PullRequestId}"; - var policyEvaluations = await policyClient.GetPolicyEvaluationsAsync(project.InternalId, artifactId); - if (policyEvaluations != null) - { - var countApplicablePolicies = 0; - foreach (var policyEvaluation in policyEvaluations) - { - if (policyEvaluation.Configuration.IsEnabled && policyEvaluation.Configuration.IsBlocking) - { - ++countApplicablePolicies; - var evalStatus = PullRequestPolicyStatus.GetFromPolicyEvaluationStatus(policyEvaluation.Status); - if (evalStatus < status) - { - statusReason = policyEvaluation.Configuration.Type.DisplayName; - status = evalStatus; - } - } - } - - if (countApplicablePolicies == 0) - { - // If there is no applicable policy, treat the policy status as Approved. - status = PolicyStatus.Approved; - } - } - } - catch (Exception ex) - { - Log.Error(ex, $"Failed getting policy evaluations for pull request: {pullRequest.PullRequestId} {pullRequest.Url}"); - } - - // Set record - Log.Debug($"PullRequest: {pullRequest.PullRequestId} Status: {status} Reason: {statusReason}"); - - //// If this was a developer pull request, track status for notifications. - - dynamic pullRequestObj = new ExpandoObject(); - var pullRequestObjFields = (IDictionary)pullRequestObj; - - pullRequestObjFields.Add("Id", pullRequest.PullRequestId); - pullRequestObjFields.Add("Title", pullRequest.Title); - pullRequestObjFields.Add("Status", pullRequest.Status); - pullRequestObjFields.Add("PolicyStatus", status.ToString()); - pullRequestObjFields.Add("PolicyStatusReason", statusReason); - - var creator = Identity.GetOrCreateIdentity(DataStore, pullRequest.CreatedBy, result.Connection); - pullRequestObjFields.Add("CreatedBy", creator.Id); - pullRequestObjFields.Add("CreationDate", pullRequest.CreationDate.Ticks); - pullRequestObjFields.Add("TargetBranch", pullRequest.TargetRefName); - - // The Links lack an html url for these results, construct the Url. - var htmlUrl = $"{project.ConnectionUri}_git/{azureUri.Repository}/pullrequest/{pullRequest.PullRequestId}"; - pullRequestObjFields.Add("HtmlUrl", htmlUrl); - - pullRequestsObjDict.Add(pullRequest.PullRequestId.ToString(CultureInfo.InvariantCulture), pullRequestObj); - } - - JsonSerializerOptions serializerOptions = new() - { -#if DEBUG - WriteIndented = true, -#else - WriteIndented = false, -#endif - }; - - var serializedJson = JsonSerializer.Serialize(pullRequestsObj, serializerOptions); - PullRequests.GetOrCreate(DataStore, repository.Id, project.Id, parameters.DeveloperId.LoginId, parameters.PullRequestView, serializedJson); + await ProcessPullRequests( + connectionResult.Connection!, + pullRequests, + project, + repository, + parameters.DeveloperId.LoginId, + parameters.PullRequestView, + parameters.OperationName == "UpdateDataForDeveloperPullRequestsAsync"); } // Foreach AzureUri return; @@ -770,7 +698,218 @@ private async Task UpdateDataForDeveloperPullRequestsAsync(DataStoreOperationPar SendDeveloperUpdateEvent(_log, this, parameters.Requestor, context); } - public TeamProject GetTeamProject(string projectName, DeveloperId.DeveloperId developerId, Uri connection) + private async Task ProcessPullRequests( + VssConnection connection, + List? pullRequests, + Project project, + Repository repository, + string loginId, + PullRequestView view, + bool isDeveloper) + { + if (pullRequests is null || pullRequests.Count == 0) + { + // If PRs were null or empty, this is a valid result, so create an empty record. + PullRequests.GetOrCreate(DataStore, repository.Id, project.Id, loginId, view, new JsonObject().ToJsonString()); + return; + } + + dynamic pullRequestsObj = new ExpandoObject(); + var pullRequestsObjDict = (IDictionary)pullRequestsObj; + var policyClient = connection.GetClient(); + + foreach (var pullRequest in pullRequests) + { + var status = PolicyStatus.Unknown; + var statusReason = string.Empty; + + // ArtifactId is null in the pull request object and it is not the correct object. The ArtifactId for the + // Policy Evaluations API is this: + // vstfs:///CodeReview/CodeReviewId/{projectId}/{pullRequestId} + var artifactId = $"vstfs:///CodeReview/CodeReviewId/{project.InternalId}/{pullRequest.PullRequestId}"; + + // Url in the GitPullRequest object is a REST Api Url, and the links lack an html Url, so we must build it. + var htmlUrl = $"{repository.CloneUrl}/pullrequest/{pullRequest.PullRequestId}"; + + try + { + var policyEvaluations = await policyClient.GetPolicyEvaluationsAsync(project.InternalId, artifactId); + GetPolicyStatus(policyEvaluations, out status, out statusReason); + } + catch (Exception ex) + { + _log.Error(ex, $"Failed getting policy evaluations for pull request: {pullRequest.PullRequestId} {pullRequest.Url}"); + } + + if (isDeveloper) + { + CreatePullRequestStatus(pullRequest, artifactId, project.Id, repository.Id, status, statusReason, htmlUrl); + } + + dynamic pullRequestObj = new ExpandoObject(); + var pullRequestObjFields = (IDictionary)pullRequestObj; + + pullRequestObjFields.Add("Id", pullRequest.PullRequestId); + pullRequestObjFields.Add("Title", pullRequest.Title); + pullRequestObjFields.Add("Status", pullRequest.Status); + pullRequestObjFields.Add("PolicyStatus", status.ToString()); + pullRequestObjFields.Add("PolicyStatusReason", statusReason); + + var creator = Identity.GetOrCreateIdentity(DataStore, pullRequest.CreatedBy, connection); + pullRequestObjFields.Add("CreatedBy", creator.Id); + pullRequestObjFields.Add("CreationDate", pullRequest.CreationDate.Ticks); + pullRequestObjFields.Add("TargetBranch", pullRequest.TargetRefName); + pullRequestObjFields.Add("HtmlUrl", htmlUrl); + + pullRequestsObjDict.Add(pullRequest.PullRequestId.ToString(CultureInfo.InvariantCulture), pullRequestObj); + } + + JsonSerializerOptions serializerOptions = new() + { +#if DEBUG + WriteIndented = true, +#else + WriteIndented = false, +#endif + }; + + var serializedJson = JsonSerializer.Serialize(pullRequestsObj, serializerOptions); + PullRequests.GetOrCreate(DataStore, repository.Id, project.Id, loginId, view, serializedJson); + } + + // Gets PolicyStatus and reason for a given list of PolicyEvaluationRecords + private void GetPolicyStatus(List policyEvaluations, out PolicyStatus status, out string statusReason) + { + status = PolicyStatus.Unknown; + statusReason = string.Empty; + + if (policyEvaluations != null) + { + var countApplicablePolicies = 0; + foreach (var policyEvaluation in policyEvaluations) + { + if (policyEvaluation.Configuration.IsEnabled && policyEvaluation.Configuration.IsBlocking) + { + ++countApplicablePolicies; + var evalStatus = PullRequestPolicyStatus.GetFromPolicyEvaluationStatus(policyEvaluation.Status); + if (evalStatus < status) + { + statusReason = policyEvaluation.Configuration.Type.DisplayName; + status = evalStatus; + } + } + } + + if (countApplicablePolicies == 0) + { + // If there is no applicable policy, treat the policy status as Approved. + status = PolicyStatus.Approved; + } + } + } + + private void CreatePullRequestStatus(GitPullRequest pullRequest, string artifactId, long projectId, long repositoryId, PolicyStatus status, string reason, string htmlUrl) + { + _log.Debug($"PullRequest: {pullRequest.PullRequestId} Status: {status} Reason: {reason}"); + var prevStatus = PullRequestPolicyStatus.Get(DataStore, artifactId); + var curStatus = PullRequestPolicyStatus.Add(DataStore, pullRequest, artifactId, projectId, repositoryId, status, reason, htmlUrl); + + if (ShouldCreateRejectedNotification(curStatus, prevStatus)) + { + _log.Information($"Creating Rejected Notification for {curStatus}"); + Notification.Create(DataStore, curStatus, NotificationType.PullRequestRejected); + } + + if (ShouldCreateApprovalNotification(curStatus, prevStatus)) + { + _log.Information($"Creating Approved Notification for {curStatus}"); + Notification.Create(DataStore, curStatus, NotificationType.PullRequestApproved); + } + } + + private bool ShouldCreateRejectedNotification(PullRequestPolicyStatus curStatus, PullRequestPolicyStatus? prevStatus) + { + // If the pull request is not recent, do not create a notification for it. + /* No UpdatedAt time on the pull request, hard to know + if ((DateTime.Now - curStatus.PullRequest.UpdatedAt) > PullRequestStaleTime) + { + return false; + } + */ + + // If the Pull Request is completed or abandoned then there is nothing to do. + if (curStatus.CompletedOrAbandoned) + { + return false; + } + + // Compare pull request status. + if (prevStatus is null) + { + // No previous status for this commit, assume new PR or freshly pushed commit with + // checks likely running. Any check failures here are assumed to be notification worthy. + if (curStatus.Rejected) + { + return true; + } + } + else + { + // A failure isn't necessarily notification worthy if we've already seen it. + // We do not wish to spam the user with failure notifications. + if (curStatus.Rejected) + { + // If the previous status was not failed, or the failure was for a different + // reason, then create a new notification. + if (!prevStatus.Rejected || curStatus.PolicyStatusReason != prevStatus.PolicyStatusReason) + { + return true; + } + } + } + + return false; + } + + private bool ShouldCreateApprovalNotification(PullRequestPolicyStatus curStatus, PullRequestPolicyStatus? prevStatus) + { + // If the pull request is not recent, do not create a notification for it. + /* No UpdatedAt time on the pull request, hard to know + if ((DateTime.Now - curStatus.PullRequest.UpdatedAt) > PullRequestStaleTime) + { + return false; + } + */ + + // If the Pull Request is completed or abandoned then there is nothing to do. + if (curStatus.CompletedOrAbandoned) + { + return false; + } + + // Compare pull request status. + if (prevStatus is null) + { + // No previous status for this PR, it may have been approved between updates. + if (curStatus.Approved) + { + return true; + } + } + else + { + // Only post success notifications if it wasn't previously successful. + // We do not wish to spam the user with success notifications. + if (curStatus.Approved && !prevStatus.Approved) + { + return true; + } + } + + return false; + } + + private TeamProject GetTeamProject(string projectName, DeveloperId.DeveloperId developerId, Uri connection) { var result = GetConnection(connection, developerId); if (result.Result != ResultType.Success) @@ -950,6 +1089,7 @@ private void PruneObsoleteData() Query.DeleteBefore(DataStore, DateTime.UtcNow - _dataRetentionTime); PullRequests.DeleteBefore(DataStore, DateTime.UtcNow - _dataRetentionTime); WorkItemType.DeleteBefore(DataStore, DateTime.UtcNow - _dataRetentionTime); + Notification.DeleteBefore(DataStore, DateTime.UtcNow - _notificationRetentionTime); // The following are not yet pruned, need to ensure there are no current key references // before deletion. diff --git a/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs b/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs index f2fde74..a701211 100644 --- a/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs +++ b/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using DevHomeAzureExtension.DataManager; +using DevHomeAzureExtension.DataModel; using Microsoft.TeamFoundation.Policy.WebApi; using Serilog; @@ -50,22 +51,14 @@ public static async Task UpdateDeveloperPullRequests() await dataManager.UpdatePullRequestsForLoggedInDeveloperIdsAsync(null, identifier); // Show any new notifications that were created from the pull request update. - /* var notifications = dataManager.GetNotifications(); foreach (var notification in notifications) { // Show notifications for failed checkruns for Developer users. - if (notification.Type == NotificationType.CheckRunFailed && notification.User.IsDeveloper) - { - notification.ShowToast(); - } - - // Show notifications for new reviews. - if (notification.Type == NotificationType.NewReview) + if (notification.Type == NotificationType.PullRequestRejected || notification.Type == NotificationType.PullRequestApproved) { notification.ShowToast(); } } - */ } } diff --git a/src/AzureExtension/DataManager/IAzureDataManager.cs b/src/AzureExtension/DataManager/IAzureDataManager.cs index f89f46a..89fe3cf 100644 --- a/src/AzureExtension/DataManager/IAzureDataManager.cs +++ b/src/AzureExtension/DataManager/IAzureDataManager.cs @@ -36,6 +36,8 @@ public interface IAzureDataManager : IDisposable WorkItemType GetWorkItemType(long id); + IEnumerable GetNotifications(DateTime? since = null, bool includeToasted = false); + IEnumerable GetRepositories(); IEnumerable GetDeveloperRepositories(); diff --git a/src/AzureExtension/DataModel/AzureDataStoreSchema.cs b/src/AzureExtension/DataModel/AzureDataStoreSchema.cs index 27ced27..385f7bd 100644 --- a/src/AzureExtension/DataModel/AzureDataStoreSchema.cs +++ b/src/AzureExtension/DataModel/AzureDataStoreSchema.cs @@ -14,7 +14,7 @@ public AzureDataStoreSchema() } // Update this anytime incompatible changes happen with a released version. - private const long SchemaVersionValue = 0x0007; + private const long SchemaVersionValue = 0x0008; private const string Metadata = @"CREATE TABLE Metadata (" + @@ -154,6 +154,39 @@ public AzureDataStoreSchema() // the developer login, and the view. "CREATE UNIQUE INDEX IDX_PullRequests_ProjectIdRepositoryIdDeveloperLoginViewId ON PullRequests (ProjectId, RepositoryId, DeveloperLogin, ViewId);"; + // PullRequsetPolicyStatus is a snapshot of a developer's Pull Requests. + private const string PullRequestPolicyStatus = + @"CREATE TABLE PullRequestPolicyStatus (" + + "Id INTEGER PRIMARY KEY NOT NULL," + + "ArtifactId TEXT NULL COLLATE NOCASE," + + "ProjectId INTEGER NOT NULL," + + "RepositoryId INTEGER NOT NULL," + + "PullRequestId INTEGER NOT NULL," + + "Title TEXT NULL COLLATE NOCASE," + + "PolicyStatusId INTEGER NOT NULL," + + "PolicyStatusReason TEXT NULL COLLATE NOCASE," + + "PullRequestStatusId INTEGER NOT NULL," + + "TargetBranchName TEXT NULL COLLATE NOCASE," + + "HtmlUrl TEXT NULL COLLATE NOCASE," + + "TimeUpdated INTEGER NOT NULL," + + "TimeCreated INTEGER NOT NULL" + + ");"; + + private const string Notification = + @"CREATE TABLE Notification (" + + "Id INTEGER PRIMARY KEY NOT NULL," + + "TypeId INTEGER NOT NULL," + + "ProjectId INTEGER NOT NULL," + + "RepositoryId INTEGER NOT NULL," + + "Title TEXT NOT NULL COLLATE NOCASE," + + "Description TEXT NOT NULL COLLATE NOCASE," + + "Identifier TEXT NULL COLLATE NOCASE," + + "Result TEXT NULL COLLATE NOCASE," + + "HtmlUrl TEXT NULL COLLATE NOCASE," + + "ToastState INTEGER NOT NULL," + + "TimeCreated INTEGER NOT NULL" + + ");"; + // All Sqls together. private static readonly List _schemaSqlsValue = [ @@ -167,5 +200,7 @@ public AzureDataStoreSchema() Query, WorkItemType, PullRequests, + PullRequestPolicyStatus, + Notification, ]; } diff --git a/src/AzureExtension/DataModel/DataObjects/Notification.cs b/src/AzureExtension/DataModel/DataObjects/Notification.cs new file mode 100644 index 0000000..e068da9 --- /dev/null +++ b/src/AzureExtension/DataModel/DataObjects/Notification.cs @@ -0,0 +1,262 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Dapper; +using Dapper.Contrib.Extensions; +using DevHomeAzureExtension.Helpers; +using Microsoft.TeamFoundation.SourceControl.WebApi; +using Microsoft.Windows.ApplicationModel.Resources; +using Microsoft.Windows.AppNotifications; +using Microsoft.Windows.AppNotifications.Builder; +using Serilog; + +namespace DevHomeAzureExtension.DataModel; + +/// +/// Represents data for rendering a notification to the user. +/// +/// +/// Notifications are sent to the user as Windows notifications, but may have specific filtering, +/// such as prioritizing certain repositories, or only showing a notification for a current +/// developer. The contents of the notifications table is a representation of potential +/// notifications, it does not necessarily mean the notification will be shown to the user. It is +/// the set of things we believe may be notification-worthy and ultimately user settings and context +/// will determine when and if the notification gets shown. +/// +[Table("Notification")] +public class Notification +{ + private static readonly Lazy _logger = new(() => Serilog.Log.ForContext("SourceContext", $"DataModel/{nameof(Notification)}")); + + private static ILogger Log => _logger.Value; + + [Key] + public long Id { get; set; } = DataStore.NoForeignKey; + + public long TypeId { get; set; } = DataStore.NoForeignKey; + + // Key in Project table + public long ProjectId { get; set; } = DataStore.NoForeignKey; + + // Key in Repository table + public long RepositoryId { get; set; } = DataStore.NoForeignKey; + + public string Title { get; set; } = string.Empty; + + public string Identifier { get; set; } = string.Empty; + + public string Result { get; set; } = string.Empty; + + public string Description { get; set; } = string.Empty; + + public string HtmlUrl { get; set; } = string.Empty; + + public long ToastState { get; set; } = DataStore.NoForeignKey; + + public long TimeCreated { get; set; } = DataStore.NoForeignKey; + + [Write(false)] + private DataStore? DataStore { get; set; } + + [Write(false)] + [Computed] + public DateTime CreatedAt => TimeCreated.ToDateTime(); + + [Write(false)] + [Computed] + public Project Project => Project.Get(DataStore, ProjectId); + + [Write(false)] + [Computed] + public Repository Repository => Repository.Get(DataStore, RepositoryId); + + [Write(false)] + [Computed] + public NotificationType Type => (NotificationType)TypeId; + + [Write(false)] + [Computed] + public bool Toasted + { + get => ToastState != 0; + set + { + ToastState = value ? 1 : 0; + if (DataStore is not null) + { + try + { + DataStore.Connection!.Update(this); + } + catch (Exception ex) + { + // Catch errors so we do not throw for something like this. The local ToastState + // will still be set even if the datastore update fails. This could result in a + // toast later being shown twice, however, so report it as an error. + Log.Error(ex, "Failed setting Notification ToastState for Notification Id = {Id}"); + } + } + } + } + + public override string ToString() => $"[{Type}][{Project}] {Title}"; + + /// + /// Shows a toast formatted based on this notification's NotificationType. + /// + /// True if a toast was shown. + public bool ShowToast() + { + if (Toasted) + { + return false; + } + else if (LocalSettings.ReadSettingAsync("NotificationsEnabled").Result == "false") + { + Toasted = true; + return false; + } + + return Type switch + { + NotificationType.PullRequestRejected => ShowPullRequestRejectedToast(), + NotificationType.PullRequestApproved => ShowPullRequestApprovedToast(), + _ => false, + }; + } + + private bool ShowPullRequestRejectedToast() + { + try + { + Log.Information($"Showing Notification for {this}"); + var nb = new AppNotificationBuilder(); + nb.SetDuration(AppNotificationDuration.Long); + nb.AddArgument("htmlurl", HtmlUrl); + nb.AddText($"❌ {Resources.GetResource("Notifications_Toast_PullRequestRejected/Title", Log)} - {Description}"); + nb.AddText($"#{Identifier} - {Repository.Name}", new AppNotificationTextProperties().SetMaxLines(1)); + nb.AddText(Title); + nb.AddButton(new AppNotificationButton(Resources.GetResource("Notifications_Toast_Button/Dismiss", Log)).AddArgument("action", "dismiss")); + AppNotificationManager.Default.Show(nb.BuildNotification()); + + Toasted = true; + } + catch (Exception ex) + { + Log.Error(ex, $"Failed creating the Notification for {this}"); + return false; + } + + return true; + } + + private bool ShowPullRequestApprovedToast() + { + try + { + Log.Information($"Showing Notification for {this}"); + var nb = new AppNotificationBuilder(); + nb.SetDuration(AppNotificationDuration.Long); + nb.AddArgument("htmlurl", HtmlUrl); + nb.AddText($"✅ {Resources.GetResource("Notifications_Toast_PullRequestApproved/Title", Log)}"); + nb.AddText($"#{Identifier} - {Repository.Name}", new AppNotificationTextProperties().SetMaxLines(1)); + nb.AddText(Title); + nb.AddButton(new AppNotificationButton(Resources.GetResource("Notifications_Toast_Button/Dismiss", Log)).AddArgument("action", "dismiss")); + AppNotificationManager.Default.Show(nb.BuildNotification()); + + Toasted = true; + } + catch (Exception ex) + { + Log.Error(ex, $"Failed creating the Notification for {this}"); + return false; + } + + return true; + } + + public static Notification Create(DataStore dataStore, PullRequestPolicyStatus status, NotificationType type) + { + var pullRequestNotification = new Notification + { + TypeId = (long)type, + ProjectId = status.ProjectId, + RepositoryId = status.RepositoryId, + Title = status.Title, + Description = status.PolicyStatusReason, + Identifier = status.PullRequestId.ToStringInvariant(), + Result = status.PolicyStatus.ToString(), + HtmlUrl = status.HtmlUrl, + ToastState = 0, + TimeCreated = DateTime.Now.ToDataStoreInteger(), + }; + + Add(dataStore, pullRequestNotification); + SetOlderNotificationsToasted(dataStore, pullRequestNotification); + return pullRequestNotification; + } + + public static Notification Add(DataStore dataStore, Notification notification) + { + notification.Id = dataStore.Connection!.Insert(notification); + notification.DataStore = dataStore; + return notification; + } + + public static IEnumerable Get(DataStore dataStore, DateTime? since = null, bool includeToasted = false) + { + since ??= DateTime.MinValue; + var sql = @"SELECT * FROM Notification WHERE TimeCreated > @Time AND ToastState <= @ToastedCount ORDER BY TimeCreated DESC"; + var param = new + { + // Cast to non-nullable type since we ensure it is not null above. + Time = ((DateTime)since).ToDataStoreInteger(), + ToastedCount = includeToasted ? 1 : 0, + }; + + Log.Verbose(DataStore.GetSqlLogMessage(sql, param)); + var notifications = dataStore.Connection!.Query(sql, param, null) ?? []; + foreach (var notification in notifications) + { + notification.DataStore = dataStore; + } + + return notifications; + } + + public static void SetOlderNotificationsToasted(DataStore dataStore, Notification notification) + { + // Get all untoasted notifications for the same type, project, repository, and identifier that are older + // than the specified notification. + var sql = @"SELECT * FROM Notification WHERE TypeId = @TypeId AND RepositoryId = @RepositoryId AND Identifier = @Identifier AND ProjectId = @ProjectId AND TimeCreated < @TimeCreated AND ToastState = 0"; + var param = new + { + notification.TypeId, + notification.RepositoryId, + notification.Identifier, + notification.ProjectId, + notification.TimeCreated, + }; + + Log.Verbose(DataStore.GetSqlLogMessage(sql, param)); + var outDatedNotifications = dataStore.Connection!.Query(sql, param, null) ?? []; + foreach (var olderNotification in outDatedNotifications) + { + olderNotification.DataStore = dataStore; + olderNotification.Toasted = true; + Log.Information($"Found older notification for {olderNotification.Identifier} with result {olderNotification.Result}, marking toasted."); + } + } + + public static void DeleteBefore(DataStore dataStore, DateTime date) + { + // Delete notifications older than the date listed. + var sql = @"DELETE FROM Notification WHERE TimeCreated < $Time;"; + var command = dataStore.Connection!.CreateCommand(); + command.CommandText = sql; + command.Parameters.AddWithValue("$Time", date.ToDataStoreInteger()); + Log.Verbose(DataStore.GetCommandLogMessage(sql, command)); + var rowsDeleted = command.ExecuteNonQuery(); + Log.Verbose(DataStore.GetDeletedLogMessage(rowsDeleted)); + } +} diff --git a/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs b/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs index 64ad0a7..d8dfe23 100644 --- a/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs +++ b/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs @@ -5,6 +5,7 @@ using Dapper.Contrib.Extensions; using DevHomeAzureExtension.Helpers; using Microsoft.TeamFoundation.Policy.WebApi; +using Microsoft.TeamFoundation.SourceControl.WebApi; using Serilog; namespace DevHomeAzureExtension.DataModel; @@ -12,25 +13,40 @@ namespace DevHomeAzureExtension.DataModel; [Table("PullRequestPolicyStatus")] public class PullRequestPolicyStatus { - private static readonly Lazy _logger = new(() => Serilog.Log.ForContext("SourceContext", $"DataModel/{nameof(PullRequests)}")); + private static readonly Lazy _logger = new(() => Serilog.Log.ForContext("SourceContext", $"DataModel/{nameof(PullRequestPolicyStatus)}")); - private static readonly ILogger _log = _logger.Value; + private static ILogger Log => _logger.Value; [Key] public long Id { get; set; } = DataStore.NoForeignKey; - // This should suffice as a unique identifier. + // This should suffice as a unique identifier, as it is the Project Guid + PullRequestId public string ArtifactId { get; set; } = string.Empty; // Key in Project table public long ProjectId { get; set; } = DataStore.NoForeignKey; + // Key in Repository table + public long RepositoryId { get; set; } = DataStore.NoForeignKey; + public long PullRequestId { get; set; } = DataStore.NoForeignKey; - public long TimeUpdated { get; set; } = DataStore.NoForeignKey; + public string Title { get; set; } = string.Empty; public long PolicyStatusId { get; set; } = DataStore.NoForeignKey; + public string PolicyStatusReason { get; set; } = string.Empty; + + public long PullRequestStatusId { get; set; } = DataStore.NoForeignKey; + + public string TargetBranchName { get; set; } = string.Empty; + + public string HtmlUrl { get; set; } = string.Empty; + + public long TimeUpdated { get; set; } = DataStore.NoForeignKey; + + public long TimeCreated { get; set; } = DataStore.NoForeignKey; + public override string ToString() => ArtifactId; [Write(false)] @@ -42,12 +58,44 @@ public class PullRequestPolicyStatus [Write(false)] [Computed] - public PolicyStatus PolicyStatus => (PolicyStatus)PolicyStatusId; + public Repository Repository => Repository.Get(DataStore, RepositoryId); + + [Write(false)] + [Computed] + public DateTime CreatedAt => TimeCreated.ToDateTime(); [Write(false)] [Computed] public DateTime UpdatedAt => TimeUpdated.ToDateTime(); + [Write(false)] + [Computed] + public PolicyStatus PolicyStatus => (PolicyStatus)PolicyStatusId; + + [Write(false)] + [Computed] + public PullRequestStatus Status => (PullRequestStatus)PullRequestStatusId; + + [Write(false)] + [Computed] + public bool CompletedOrAbandoned => (Status == PullRequestStatus.Completed) || (Status == PullRequestStatus.Abandoned); + + [Write(false)] + [Computed] + public bool ActiveOrNotSet => (Status == PullRequestStatus.Active) || (Status == PullRequestStatus.NotSet); + + [Write(false)] + [Computed] + public bool Waiting => (PolicyStatus == PolicyStatus.Queued) || (PolicyStatus == PolicyStatus.Running); + + [Write(false)] + [Computed] + public bool Rejected => PolicyStatus <= PolicyStatus.Rejected; + + [Write(false)] + [Computed] + public bool Approved => PolicyStatus >= PolicyStatus.Approved; + // Map PolicyEvaluationStatus to our enum. Our enum is sorted, but the PolicyEvaluationStatus is not. public static PolicyStatus GetFromPolicyEvaluationStatus(PolicyEvaluationStatus? policyEvaluationStatus) { @@ -62,4 +110,80 @@ public static PolicyStatus GetFromPolicyEvaluationStatus(PolicyEvaluationStatus? _ => PolicyStatus.Unknown, }; } + + public static PullRequestPolicyStatus Create(GitPullRequest pullRequest, string artifactId, long projectId, long repositoryId, PolicyStatus policyStatus, string reason, string htmlUrl) + { + // Get Current status of this pull request, and create a summary capture. + return new PullRequestPolicyStatus + { + ArtifactId = artifactId, + ProjectId = projectId, + RepositoryId = repositoryId, + PullRequestId = pullRequest.PullRequestId, + Title = pullRequest.Title, + PolicyStatusId = (long)policyStatus, + PolicyStatusReason = reason, + TargetBranchName = pullRequest.TargetRefName, + PullRequestStatusId = (long)pullRequest.Status, + HtmlUrl = htmlUrl, + TimeCreated = DateTime.UtcNow.ToDataStoreInteger(), + }; + } + + public static PullRequestPolicyStatus? Get(DataStore dataStore, string artifactId) + { + var sql = @"SELECT * FROM PullRequestPolicyStatus WHERE ArtifactId = @ArtifactId ORDER BY TimeCreated DESC LIMIT 1;"; + var param = new + { + ArtifactId = artifactId, + }; + + Log.Verbose(DataStore.GetSqlLogMessage(sql, param)); + var pullRequestStatus = dataStore.Connection!.QueryFirstOrDefault(sql, param, null); + if (pullRequestStatus is not null) + { + // Add Datastore so this object can make internal queries. + pullRequestStatus.DataStore = dataStore; + } + + return pullRequestStatus; + } + + public static PullRequestPolicyStatus Add(DataStore dataStore, GitPullRequest pullRequest, string artifactId, long projectId, long repositoryId, PolicyStatus policyStatus, string reason, string htmlUrl) + { + var pullRequestStatus = Create(pullRequest, artifactId, projectId, repositoryId, policyStatus, reason, htmlUrl); + pullRequestStatus.Id = dataStore.Connection!.Insert(pullRequestStatus); + Log.Debug($"Inserted PullRequestPolicyStatus, Id = {pullRequestStatus.Id}"); + + // Remove older records we no longer need. + DeleteOutdatedForPullRequest(dataStore, artifactId); + + pullRequestStatus.DataStore = dataStore; + return pullRequestStatus; + } + + public static void DeleteOutdatedForPullRequest(DataStore dataStore, string artifactId) + { + // Delete any records beyond the most recent 2. + var sql = @"DELETE FROM PullRequestPolicyStatus WHERE ArtifactId = $Id AND Id NOT IN (SELECT Id FROM PullRequestPolicyStatus WHERE ArtifactId = $Id ORDER BY TimeCreated DESC LIMIT 2)"; + var command = dataStore.Connection!.CreateCommand(); + command.CommandText = sql; + command.Parameters.AddWithValue("$Id", artifactId); + Log.Verbose(DataStore.GetCommandLogMessage(sql, command)); + var rowsDeleted = command.ExecuteNonQuery(); + Log.Verbose(DataStore.GetDeletedLogMessage(rowsDeleted)); + } + + public static void DeleteUnreferenced(DataStore dataStore) + { + // Delete any where the HeadSha has no match in pull requests, as that means either the pull request + // no longer exists, or it has pushed new changes and the HeadSha is different, triggering new checks. + // In either case it is safe to remove the associated PullRequestPolicyStatus. + var sql = @"DELETE FROM PullRequestPolicyStatus WHERE HeadSha NOT IN (SELECT HeadSha FROM PullRequest)"; + var command = dataStore.Connection!.CreateCommand(); + command.CommandText = sql; + Log.Verbose(DataStore.GetCommandLogMessage(sql, command)); + var rowsDeleted = command.ExecuteNonQuery(); + Log.Verbose(DataStore.GetDeletedLogMessage(rowsDeleted)); + } } diff --git a/src/AzureExtension/DataModel/Enums/NotificationType.cs b/src/AzureExtension/DataModel/Enums/NotificationType.cs new file mode 100644 index 0000000..235cd0c --- /dev/null +++ b/src/AzureExtension/DataModel/Enums/NotificationType.cs @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace DevHomeAzureExtension.DataModel; + +public enum NotificationType +{ + Unknown = 0, + PullRequestApproved = 1, + PullRequestRejected = 2, + NewReview = 3, +} diff --git a/src/AzureExtension/Strings/en-US/Resources.resw b/src/AzureExtension/Strings/en-US/Resources.resw index 6b5c693..c221891 100644 --- a/src/AzureExtension/Strings/en-US/Resources.resw +++ b/src/AzureExtension/Strings/en-US/Resources.resw @@ -714,4 +714,16 @@ Launch Text shown in the button to launch Dev Box. + + Dismiss + Shown in Toast Notification Dismiss Button + + + Approved + Shown in Toast Notification, title line + + + Rejected + Shown in Toast Notification, title line + \ No newline at end of file From ae3b30813bc23c299f96747f0974665dfcf1a235 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 22 Jul 2024 15:40:04 -0700 Subject: [PATCH 04/16] Add policy status and notification test --- .../DataModel/AzureDataStoreSchema.cs | 2 +- .../DeveloperId/AuthenticationHelper.cs | 2 +- .../DataStore/DataObjectTests.cs | 53 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/AzureExtension/DataModel/AzureDataStoreSchema.cs b/src/AzureExtension/DataModel/AzureDataStoreSchema.cs index 385f7bd..0c6acd3 100644 --- a/src/AzureExtension/DataModel/AzureDataStoreSchema.cs +++ b/src/AzureExtension/DataModel/AzureDataStoreSchema.cs @@ -14,7 +14,7 @@ public AzureDataStoreSchema() } // Update this anytime incompatible changes happen with a released version. - private const long SchemaVersionValue = 0x0008; + private const long SchemaVersionValue = 0x0007; private const string Metadata = @"CREATE TABLE Metadata (" + diff --git a/src/AzureExtension/DeveloperId/AuthenticationHelper.cs b/src/AzureExtension/DeveloperId/AuthenticationHelper.cs index e2895a4..f378795 100644 --- a/src/AzureExtension/DeveloperId/AuthenticationHelper.cs +++ b/src/AzureExtension/DeveloperId/AuthenticationHelper.cs @@ -286,7 +286,7 @@ public async Task> AcquireAllDeveloperAccountTokens(string[] public async Task ObtainTokenForLoggedInDeveloperAccount(string[] scopes, string loginId) { - _log.Information($"ObtainTokenForLoggedInDeveloperAccount"); + _log.Debug($"ObtainTokenForLoggedInDeveloperAccount"); AuthenticationResult = null; var existingAccount = await GetDeveloperAccountFromCache(loginId); diff --git a/test/AzureExtension/DataStore/DataObjectTests.cs b/test/AzureExtension/DataStore/DataObjectTests.cs index ee7a44e..05b62c0 100644 --- a/test/AzureExtension/DataStore/DataObjectTests.cs +++ b/test/AzureExtension/DataStore/DataObjectTests.cs @@ -372,4 +372,57 @@ public void ReadAndWritePullRequests() Assert.AreEqual(PullRequestView.Mine, findPull.View); Assert.AreEqual("project", findPull.Project.Name); } + + [TestMethod] + [TestCategory("Unit")] + public void ReadAndWriteStatusAndNotification() + { + using var dataStore = new DataStore("TestStore", TestHelpers.GetDataStoreFilePath(TestOptions), TestOptions.DataStoreOptions.DataStoreSchema!); + Assert.IsNotNull(dataStore); + dataStore.Create(); + Assert.IsNotNull(dataStore.Connection); + + using var tx = dataStore.Connection.BeginTransaction(); + var org = Organization.GetOrCreate(dataStore, new Uri("https://dev.azure.com/organization/")); + Assert.IsNotNull(org); + dataStore.Connection.Insert(new Project { Name = "project", InternalId = "11", OrganizationId = org.Id }); + dataStore.Connection.Insert(new Repository { Name = "repository", InternalId = "21", CloneUrl = "https://organization/project/_git/repository/", ProjectId = 1 }); + + var artifactId = "vstfs:///CodeReview/CodeReviewId/foo/47"; + var title = "Pull Request Test"; + var url = "https://organization/project/_git/repository/pullrequest/47"; + dataStore.Connection.Insert(new PullRequestPolicyStatus + { + ArtifactId = artifactId, + ProjectId = 1, + RepositoryId = 1, + PullRequestId = 47, + Title = title, + PolicyStatusId = 2, + PolicyStatusReason = "Build Fail", + TargetBranchName = "refs/heads/main", + PullRequestStatusId = 1, + HtmlUrl = url, + TimeCreated = DateTime.UtcNow.ToDataStoreInteger(), + }); + + var prStatus = PullRequestPolicyStatus.Get(dataStore, artifactId); + Assert.IsNotNull(prStatus); + + TestContext?.WriteLine($" PR: {prStatus.PullRequestId} Status: {prStatus.PolicyStatusId}: {prStatus.PolicyStatus} - {prStatus.PolicyStatusReason}"); + Assert.AreEqual(url, prStatus.HtmlUrl); + Assert.AreEqual(PolicyStatus.Rejected, prStatus.PolicyStatus); + Assert.AreEqual(artifactId, prStatus.ArtifactId); + + // Create notification from PR Status + var notification = Notification.Create(dataStore, prStatus, NotificationType.PullRequestRejected); + Assert.IsNotNull(notification); + Assert.AreEqual(title, notification.Title); + Assert.AreEqual(1, notification.RepositoryId); + Assert.AreEqual(1, notification.ProjectId); + Assert.AreEqual(PolicyStatus.Rejected.ToString(), notification.Result); + Assert.AreEqual(url, notification.HtmlUrl); + TestContext?.WriteLine($" {notification.Title}"); + TestContext?.WriteLine($" {notification.Description}"); + } } From fcc61e2c977006fec3e405c8e3f44989842b4080 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 23 Jul 2024 14:27:07 -0700 Subject: [PATCH 05/16] Add add stale checks to pull request data, update based on push --- .../DataManager/AzureDataManager.cs | 41 +++++++++++++++---- .../DataManager/AzureDataManagerUpdate.cs | 5 ++- .../DataObjects/PullRequestPolicyStatus.cs | 37 +++++++++++++---- 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/src/AzureExtension/DataManager/AzureDataManager.cs b/src/AzureExtension/DataManager/AzureDataManager.cs index cfa6d8a..0f170f1 100644 --- a/src/AzureExtension/DataManager/AzureDataManager.cs +++ b/src/AzureExtension/DataManager/AzureDataManager.cs @@ -46,9 +46,17 @@ public partial class AzureDataManager : IAzureDataManager, IDisposable // Most data that has not been updated within this time will be removed. private static readonly TimeSpan _dataRetentionTime = TimeSpan.FromDays(1); + // This is how long we will keep a notification in the datastore. private static readonly TimeSpan _notificationRetentionTime = TimeSpan.FromDays(7); - private static readonly TimeSpan _pullRequestStaleTime = TimeSpan.FromDays(1); + // Pull requests without a push in this amount of time are considered too old to + // generate new notifications. + private static readonly TimeSpan _pullRequestIsAncientTime = TimeSpan.FromDays(14); + + // The amount of time we will retain a pull request status record. If a very old pull + // request is resurrected and updated beyond this time it will be treated as a new + // pull request as the previous status record will be gone. + private static readonly TimeSpan _pullRequestStatusRetentionTime = TimeSpan.FromDays(30); private static readonly string _lastUpdatedKeyName = "LastUpdated"; @@ -743,6 +751,22 @@ private async Task ProcessPullRequests( if (isDeveloper) { + // Pull requests do not fully populate commit information. If this is a developer pull request, fetch the + // additional commit information about the last merged source to determine when the last time the pull request + // was pushed a commit. + if (pullRequest.LastMergeSourceCommit is not null) + { + var gitClient = connection.GetClient(); + if (gitClient is not null) + { + var commitRef = await gitClient.GetCommitAsync(pullRequest.LastMergeSourceCommit.CommitId, repository.InternalId); + if (commitRef is not null) + { + pullRequest.LastMergeSourceCommit = commitRef; + } + } + } + CreatePullRequestStatus(pullRequest, artifactId, project.Id, repository.Id, status, statusReason, htmlUrl); } @@ -829,13 +853,12 @@ private void CreatePullRequestStatus(GitPullRequest pullRequest, string artifact private bool ShouldCreateRejectedNotification(PullRequestPolicyStatus curStatus, PullRequestPolicyStatus? prevStatus) { - // If the pull request is not recent, do not create a notification for it. - /* No UpdatedAt time on the pull request, hard to know - if ((DateTime.Now - curStatus.PullRequest.UpdatedAt) > PullRequestStaleTime) + // If the pull request is not recently updated, ignore it. This is to prevent ancient pull requests + // from showing notifications. + if ((DateTime.UtcNow - curStatus.UpdatedAt) > _pullRequestIsAncientTime) { return false; } - */ // If the Pull Request is completed or abandoned then there is nothing to do. if (curStatus.CompletedOrAbandoned) @@ -873,13 +896,12 @@ private bool ShouldCreateRejectedNotification(PullRequestPolicyStatus curStatus, private bool ShouldCreateApprovalNotification(PullRequestPolicyStatus curStatus, PullRequestPolicyStatus? prevStatus) { - // If the pull request is not recent, do not create a notification for it. - /* No UpdatedAt time on the pull request, hard to know - if ((DateTime.Now - curStatus.PullRequest.UpdatedAt) > PullRequestStaleTime) + // If the pull request is not recently updated, ignore it. This is to prevent ancient pull requests + // from showing notifications. + if ((DateTime.UtcNow - curStatus.UpdatedAt) > _pullRequestIsAncientTime) { return false; } - */ // If the Pull Request is completed or abandoned then there is nothing to do. if (curStatus.CompletedOrAbandoned) @@ -1090,6 +1112,7 @@ private void PruneObsoleteData() PullRequests.DeleteBefore(DataStore, DateTime.UtcNow - _dataRetentionTime); WorkItemType.DeleteBefore(DataStore, DateTime.UtcNow - _dataRetentionTime); Notification.DeleteBefore(DataStore, DateTime.UtcNow - _notificationRetentionTime); + PullRequestPolicyStatus.DeleteBefore(DataStore, DateTime.UtcNow - _pullRequestStatusRetentionTime); // The following are not yet pruned, need to ensure there are no current key references // before deletion. diff --git a/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs b/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs index a701211..b501da2 100644 --- a/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs +++ b/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs @@ -37,12 +37,13 @@ public static async Task Update() public static async Task UpdateDeveloperPullRequests() { - Log.Debug($"Executing UpdateDeveloperPullRequests"); + var log = Log.ForContext("SourceContext", $"UpdateDeveloperPullRequests"); + log.Debug($"Executing UpdateDeveloperPullRequests"); var cacheManager = CacheManager.GetInstance(); if (cacheManager.UpdateInProgress) { - Log.Information("Cache is being updated, skipping Developer Pull Request Update"); + log.Information("Cache is being updated, skipping Developer Pull Request Update"); return; } diff --git a/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs b/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs index d8dfe23..efcf93e 100644 --- a/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs +++ b/src/AzureExtension/DataModel/DataObjects/PullRequestPolicyStatus.cs @@ -43,8 +43,10 @@ public class PullRequestPolicyStatus public string HtmlUrl { get; set; } = string.Empty; + // Time of the last pull request update (push), if it exists. public long TimeUpdated { get; set; } = DataStore.NoForeignKey; + // Time the pull request was created. public long TimeCreated { get; set; } = DataStore.NoForeignKey; public override string ToString() => ArtifactId; @@ -114,7 +116,7 @@ public static PolicyStatus GetFromPolicyEvaluationStatus(PolicyEvaluationStatus? public static PullRequestPolicyStatus Create(GitPullRequest pullRequest, string artifactId, long projectId, long repositoryId, PolicyStatus policyStatus, string reason, string htmlUrl) { // Get Current status of this pull request, and create a summary capture. - return new PullRequestPolicyStatus + var status = new PullRequestPolicyStatus { ArtifactId = artifactId, ProjectId = projectId, @@ -126,8 +128,26 @@ public static PullRequestPolicyStatus Create(GitPullRequest pullRequest, string TargetBranchName = pullRequest.TargetRefName, PullRequestStatusId = (long)pullRequest.Status, HtmlUrl = htmlUrl, - TimeCreated = DateTime.UtcNow.ToDataStoreInteger(), + TimeCreated = pullRequest.CreationDate.ToUniversalTime().ToDataStoreInteger(), }; + + // Set TimeUpdated to be the most recent commit push time if it exists. + if (pullRequest.LastMergeSourceCommit?.Push is not null) + { + status.TimeUpdated = pullRequest.LastMergeSourceCommit.Push.Date.ToUniversalTime().ToDataStoreInteger(); + if (status.TimeUpdated < status.TimeCreated) + { + // First commits will often be pushed before the pull request is created. In this case, + // we want the TimeUpdated to be at a minimum the TimeCreated. + status.TimeUpdated = status.TimeCreated; + } + } + else + { + status.TimeUpdated = status.TimeCreated; + } + + return status; } public static PullRequestPolicyStatus? Get(DataStore dataStore, string artifactId) @@ -174,16 +194,15 @@ public static void DeleteOutdatedForPullRequest(DataStore dataStore, string arti Log.Verbose(DataStore.GetDeletedLogMessage(rowsDeleted)); } - public static void DeleteUnreferenced(DataStore dataStore) + public static void DeleteBefore(DataStore dataStore, DateTime date) { - // Delete any where the HeadSha has no match in pull requests, as that means either the pull request - // no longer exists, or it has pushed new changes and the HeadSha is different, triggering new checks. - // In either case it is safe to remove the associated PullRequestPolicyStatus. - var sql = @"DELETE FROM PullRequestPolicyStatus WHERE HeadSha NOT IN (SELECT HeadSha FROM PullRequest)"; + // Delete notifications older than the date listed. + var sql = @"DELETE FROM PullRequestPolicyStatus WHERE TimeUpdated < $Time;"; var command = dataStore.Connection!.CreateCommand(); command.CommandText = sql; - Log.Verbose(DataStore.GetCommandLogMessage(sql, command)); + command.Parameters.AddWithValue("$Time", date.ToDataStoreInteger()); + Log.Debug(DataStore.GetCommandLogMessage(sql, command)); var rowsDeleted = command.ExecuteNonQuery(); - Log.Verbose(DataStore.GetDeletedLogMessage(rowsDeleted)); + Log.Debug(DataStore.GetDeletedLogMessage(rowsDeleted)); } } From e39e4ed4aec25a7b10d66b53dfe54dbe67c43724 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 23 Jul 2024 14:53:59 -0700 Subject: [PATCH 06/16] Use UtcNow --- src/AzureExtension/DataManager/AzureDataManagerUpdate.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs b/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs index b501da2..9d5fb00 100644 --- a/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs +++ b/src/AzureExtension/DataManager/AzureDataManagerUpdate.cs @@ -18,7 +18,7 @@ public static async Task Update() { // Only update per the update interval. // This is intended to be dynamic in the future. - if (DateTime.Now - _lastUpdateTime < _updateInterval) + if (DateTime.UtcNow - _lastUpdateTime < _updateInterval) { return; } @@ -32,7 +32,7 @@ public static async Task Update() Log.Error(ex, "Update failed unexpectedly."); } - _lastUpdateTime = DateTime.Now; + _lastUpdateTime = DateTime.UtcNow; } public static async Task UpdateDeveloperPullRequests() From 7bc506fbaada1c4756dedbdaa15956da9ff9fbf6 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 24 Jul 2024 12:19:01 -0700 Subject: [PATCH 07/16] Make developerLoginId nullable --- src/AzureExtension/DataManager/AzureDataManager.cs | 2 -- src/AzureExtension/DataModel/AzureDataStoreSchema.cs | 2 +- src/AzureExtension/DataModel/DataObjects/Identity.cs | 7 +++---- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/AzureExtension/DataManager/AzureDataManager.cs b/src/AzureExtension/DataManager/AzureDataManager.cs index 0f170f1..f2944d8 100644 --- a/src/AzureExtension/DataManager/AzureDataManager.cs +++ b/src/AzureExtension/DataManager/AzureDataManager.cs @@ -676,8 +676,6 @@ private async Task UpdateDataForDeveloperPullRequestsAsync(DataStoreOperationPar foreach (var repositoryRef in repositoryReferences) { var uri = new AzureUri(repositoryRef.Repository.CloneUrl); - var developerId = repositoryRef.Developer.DeveloperLoginId; - var uris = new List { new(repositoryRef.Repository.CloneUrl), diff --git a/src/AzureExtension/DataModel/AzureDataStoreSchema.cs b/src/AzureExtension/DataModel/AzureDataStoreSchema.cs index 0c6acd3..c943d23 100644 --- a/src/AzureExtension/DataModel/AzureDataStoreSchema.cs +++ b/src/AzureExtension/DataModel/AzureDataStoreSchema.cs @@ -30,7 +30,7 @@ public AzureDataStoreSchema() "Name TEXT NOT NULL COLLATE NOCASE," + "InternalId TEXT NOT NULL," + "Avatar TEXT NOT NULL COLLATE NOCASE," + - "DeveloperLoginId TEXT NOT NULL," + + "DeveloperLoginId TEXT," + "TimeUpdated INTEGER NOT NULL" + ");" + diff --git a/src/AzureExtension/DataModel/DataObjects/Identity.cs b/src/AzureExtension/DataModel/DataObjects/Identity.cs index 0fcbf4e..52497fa 100644 --- a/src/AzureExtension/DataModel/DataObjects/Identity.cs +++ b/src/AzureExtension/DataModel/DataObjects/Identity.cs @@ -46,7 +46,7 @@ public class Identity // The DeveloperLoginId associated with this identity, if one exists. // Empty means there is no associated LoggedInDeveloperId. - public string DeveloperLoginId { get; set; } = string.Empty; + public string? DeveloperLoginId { get; set; } [JsonIgnore] public long TimeUpdated { get; set; } = DataStore.NoForeignKey; @@ -73,7 +73,7 @@ public DeveloperId.DeveloperId? DeveloperId } var devIdProvider = DeveloperIdProvider.GetInstance(); - return devIdProvider.GetDeveloperIdFromAccountIdentifier(DeveloperLoginId); + return devIdProvider.GetDeveloperIdFromAccountIdentifier(DeveloperLoginId!); } } @@ -163,7 +163,7 @@ private static Identity CreateFromIdentity(Microsoft.VisualStudio.Services.Ident }; } - public static Identity AddOrUpdateIdentity(DataStore dataStore, Identity identity, string developerLoginId = "") + public static Identity AddOrUpdateIdentity(DataStore dataStore, Identity identity, string? developerLoginId = null) { // Check for existing Identity data. var existingIdentity = GetByInternalId(dataStore, identity.InternalId); @@ -171,7 +171,6 @@ public static Identity AddOrUpdateIdentity(DataStore dataStore, Identity identit { identity.Id = existingIdentity.Id; - // If this is a developer, set to developer, but do not set it. // We presume not a developer unless it is explicitly set. if (!string.IsNullOrEmpty(developerLoginId)) { From 67146a60ea2a5773bdfe465c6d2896a2ab79a079 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 24 Jul 2024 12:30:20 -0700 Subject: [PATCH 08/16] Remove dead code, fix comment --- src/AzureExtension/DataManager/AzureDataManager.cs | 5 ----- src/AzureExtension/DataManager/IAzureDataManager.cs | 2 -- src/AzureExtension/DataModel/DataObjects/Identity.cs | 1 - 3 files changed, 8 deletions(-) diff --git a/src/AzureExtension/DataManager/AzureDataManager.cs b/src/AzureExtension/DataManager/AzureDataManager.cs index f2944d8..c70dd59 100644 --- a/src/AzureExtension/DataManager/AzureDataManager.cs +++ b/src/AzureExtension/DataManager/AzureDataManager.cs @@ -293,11 +293,6 @@ public IEnumerable GetNotifications(DateTime? since = null, bool i return GetPullRequests(repositoryUri.Organization, repositoryUri.Project, repositoryUri.Repository, developerId, view); } - public PullRequests? GetPullRequestsForLoggedInDeveloperIds() - { - return null; - } - private async Task UpdateDataForQueriesAsync(DataStoreOperationParameters parameters) { _log.Debug($"Inside UpdateDataForQueriesAsync with Parameters: {parameters}"); diff --git a/src/AzureExtension/DataManager/IAzureDataManager.cs b/src/AzureExtension/DataManager/IAzureDataManager.cs index 89fe3cf..f6f0ead 100644 --- a/src/AzureExtension/DataManager/IAzureDataManager.cs +++ b/src/AzureExtension/DataManager/IAzureDataManager.cs @@ -47,6 +47,4 @@ public interface IAzureDataManager : IDisposable PullRequests? GetPullRequests(string organization, string project, string repositoryName, string developerId, PullRequestView view); PullRequests? GetPullRequests(AzureUri repositoryUri, string developerId, PullRequestView view); - - PullRequests? GetPullRequestsForLoggedInDeveloperIds(); } diff --git a/src/AzureExtension/DataModel/DataObjects/Identity.cs b/src/AzureExtension/DataModel/DataObjects/Identity.cs index 52497fa..1f12290 100644 --- a/src/AzureExtension/DataModel/DataObjects/Identity.cs +++ b/src/AzureExtension/DataModel/DataObjects/Identity.cs @@ -45,7 +45,6 @@ public class Identity public string Avatar { get; set; } = string.Empty; // The DeveloperLoginId associated with this identity, if one exists. - // Empty means there is no associated LoggedInDeveloperId. public string? DeveloperLoginId { get; set; } [JsonIgnore] From d2b018b18338292643f926b13d898bde6ca7dd2a Mon Sep 17 00:00:00 2001 From: David Bennett Date: Wed, 24 Jul 2024 12:44:39 -0700 Subject: [PATCH 09/16] Swap operation names to a constant --- src/AzureExtension/DataManager/AzureDataManager.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/AzureExtension/DataManager/AzureDataManager.cs b/src/AzureExtension/DataManager/AzureDataManager.cs index c70dd59..2d04a70 100644 --- a/src/AzureExtension/DataManager/AzureDataManager.cs +++ b/src/AzureExtension/DataManager/AzureDataManager.cs @@ -178,7 +178,7 @@ public async Task UpdateDataForQueriesAsync(IEnumerable queryUris, str LoginId = developerLogin, DeveloperId = DeveloperIdProvider.GetInstance().GetDeveloperIdFromAccountIdentifier(developerLogin), RequestOptions = options ?? RequestOptions.RequestOptionsDefault(), - OperationName = "UpdateDataForQueryAsync", + OperationName = nameof(UpdateDataForQueriesAsync), Requestor = requestor ?? Guid.NewGuid(), }; @@ -218,7 +218,7 @@ public async Task UpdateDataForPullRequestsAsync(AzureUri repositoryUri, string DeveloperId = DeveloperIdProvider.GetInstance().GetDeveloperIdFromAccountIdentifier(developerLogin), PullRequestView = view, RequestOptions = options ?? RequestOptions.RequestOptionsDefault(), - OperationName = "UpdateDataForPullRequestsAsync", + OperationName = nameof(UpdateDataForPullRequestsAsync), Requestor = requestor ?? Guid.NewGuid(), }; @@ -621,7 +621,7 @@ await ProcessPullRequests( repository, parameters.DeveloperId.LoginId, parameters.PullRequestView, - parameters.OperationName == "UpdateDataForDeveloperPullRequestsAsync"); + parameters.OperationName == nameof(UpdateDataForDeveloperPullRequestsAsync)); } // Foreach AzureUri return; @@ -633,7 +633,7 @@ public async Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(RequestOptions? var parameters = new DataStoreOperationParameters { RequestOptions = options ?? RequestOptions.RequestOptionsDefault(), - OperationName = "UpdatePullRequestsForLoggedInDeveloperIdsAsync", + OperationName = nameof(UpdatePullRequestsForLoggedInDeveloperIdsAsync), PullRequestView = PullRequestView.Mine, Requestor = requestor ?? Guid.NewGuid(), }; @@ -681,7 +681,7 @@ private async Task UpdateDataForDeveloperPullRequestsAsync(DataStoreOperationPar Uris = uris, DeveloperId = repositoryRef.Developer.DeveloperId, RequestOptions = parameters.RequestOptions, - OperationName = "UpdateDataForDeveloperPullRequestsAsync", + OperationName = nameof(UpdateDataForDeveloperPullRequestsAsync), PullRequestView = PullRequestView.Mine, Requestor = parameters.Requestor, }; From 9e7a3cfa61797d8d54dd72c646d0bd3ee05f2a51 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 26 Jul 2024 13:37:12 -0700 Subject: [PATCH 10/16] PR Widget WIP --- src/AzureExtension/AzureExtension.csproj | 1 + .../DataManager/AzureDataManager.cs | 59 +- .../DataManager/AzureDataManagerCache.cs | 19 + .../DataManager/CacheManager.cs | 11 +- .../CacheManagerUpdateEventArgs.cs | 1 + .../DataManager/DataManagerUpdateEventArgs.cs | 1 + .../DataManager/IAzureDataManager.cs | 2 + .../DataModel/DataObjects/PullRequests.cs | 17 + .../Widgets/AzurePullRequestsBaseWidget.cs | 126 ++++ .../AzurePullRequestsDeveloperWidget.cs | 203 ++++++ ...s => AzurePullRequestsRepositoryWidget.cs} | 664 ++++++++---------- .../Widgets/AzureQueryListWidget.cs | 13 +- .../Widgets/AzureQueryTilesWidget.cs | 2 +- src/AzureExtension/Widgets/AzureWidget.cs | 13 +- .../Widgets/Enums/WidgetDataState.cs | 3 +- src/AzureExtension/Widgets/WidgetImpl.cs | 2 + src/AzureExtension/Widgets/WidgetProvider.cs | 3 +- src/AzureExtensionServer/Package.appxmanifest | 37 + 18 files changed, 779 insertions(+), 398 deletions(-) create mode 100644 src/AzureExtension/Widgets/AzurePullRequestsBaseWidget.cs create mode 100644 src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs rename src/AzureExtension/Widgets/{AzurePullRequestsWidget.cs => AzurePullRequestsRepositoryWidget.cs} (68%) diff --git a/src/AzureExtension/AzureExtension.csproj b/src/AzureExtension/AzureExtension.csproj index b1eb6a1..63beba7 100644 --- a/src/AzureExtension/AzureExtension.csproj +++ b/src/AzureExtension/AzureExtension.csproj @@ -10,6 +10,7 @@ + diff --git a/src/AzureExtension/DataManager/AzureDataManager.cs b/src/AzureExtension/DataManager/AzureDataManager.cs index 2d04a70..ea4f7dc 100644 --- a/src/AzureExtension/DataManager/AzureDataManager.cs +++ b/src/AzureExtension/DataManager/AzureDataManager.cs @@ -627,6 +627,12 @@ await ProcessPullRequests( return; } + public IEnumerable GetPullRequestsForLoggedInDeveloperIds() + { + ValidateDataStore(); + return PullRequests.GetAllForDeveloper(DataStore); + } + public async Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(RequestOptions? options = null, Guid? requestor = null) { ValidateDataStore(); @@ -653,50 +659,35 @@ public async Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(RequestOptions? return; } - SendPullRequestUpdateEvent(_log, this, parameters.Requestor, context); + SendDeveloperUpdateEvent(_log, this, parameters.Requestor, context); } private async Task UpdateDataForDeveloperPullRequestsAsync(DataStoreOperationParameters parameters) { _log.Debug($"Inside UpdateDataForDeveloperPullRequestsAsync with Parameters: {parameters}"); - dynamic context = new ExpandoObject(); - var contextDict = (IDictionary)context; - contextDict.Add("Requestor", parameters.Requestor); - - try + // This is a loop over a subset of repositories with a specific developer ID and pull request view specified. + var repositoryReferences = RepositoryReference.GetAll(DataStore); + foreach (var repositoryRef in repositoryReferences) { - // This is a loop over a subset of repositories with a specific developer ID and pull request view specified. - var repositoryReferences = RepositoryReference.GetAll(DataStore); - foreach (var repositoryRef in repositoryReferences) + var uri = new AzureUri(repositoryRef.Repository.CloneUrl); + var uris = new List { - var uri = new AzureUri(repositoryRef.Repository.CloneUrl); - var uris = new List - { - new(repositoryRef.Repository.CloneUrl), - }; + new(repositoryRef.Repository.CloneUrl), + }; - var suboperationParameters = new DataStoreOperationParameters - { - Uris = uris, - DeveloperId = repositoryRef.Developer.DeveloperId, - RequestOptions = parameters.RequestOptions, - OperationName = nameof(UpdateDataForDeveloperPullRequestsAsync), - PullRequestView = PullRequestView.Mine, - Requestor = parameters.Requestor, - }; - - await UpdateDataForPullRequestsAsync(suboperationParameters); - } - } - catch (Exception ex) - { - contextDict.Add("ErrorMessage", ex.Message); - SendErrorUpdateEvent(_log, this, parameters.Requestor, context, ex); - return; - } + var suboperationParameters = new DataStoreOperationParameters + { + Uris = uris, + DeveloperId = repositoryRef.Developer.DeveloperId, + RequestOptions = parameters.RequestOptions, + OperationName = nameof(UpdateDataForDeveloperPullRequestsAsync), + PullRequestView = PullRequestView.Mine, + Requestor = parameters.Requestor, + }; - SendDeveloperUpdateEvent(_log, this, parameters.Requestor, context); + await UpdateDataForPullRequestsAsync(suboperationParameters); + } } private async Task ProcessPullRequests( diff --git a/src/AzureExtension/DataManager/AzureDataManagerCache.cs b/src/AzureExtension/DataManager/AzureDataManagerCache.cs index 13ddaa8..c9dc076 100644 --- a/src/AzureExtension/DataManager/AzureDataManagerCache.cs +++ b/src/AzureExtension/DataManager/AzureDataManagerCache.cs @@ -24,6 +24,8 @@ public partial class AzureDataManager private static readonly int _pullRequestRepositoryLimit = 25; + private static readonly TimeSpan _orgUpdateSleepTime = TimeSpan.FromSeconds(5); + public async Task UpdateDataForAccountsAsync(RequestOptions? options = null, Guid? requestor = null) { // A parameterless call will always update the data, effectively a 'force update'. @@ -104,11 +106,23 @@ public async Task UpdateDataForAccountsAsync(TimeSpan olderThan, RequestOptions? continue; } + var orgStartTime = DateTime.UtcNow; + UpdateOrganization(account, developerId, connection, cancellationToken); tx.Commit(); _log.Information($"Updated organization: {account.AccountName}"); + + // Send an update event once the transaction is completed so anyone waiting on the DB to be + // available has an opportunity to use it. + var orgUpdateContext = CreateUpdateEventContext(0, 1, 0, DateTime.UtcNow - orgStartTime); + SendAccountUpdateEvent(_log, this, requestorGuid, orgUpdateContext, firstException); ++accountsUpdated; + + // Sleep to allow widgets and other code to respond to the event and use the database. + // This is to prevent DOS'ing widgets using the datastore during large cache updates of + // many organizations. + Thread.Sleep(_orgUpdateSleepTime); } catch (Exception ex) when (IsCancelException(ex)) { @@ -313,6 +327,11 @@ private static void SendCacheUpdateEvent(ILogger logger, object? source, Guid re SendUpdateEvent(logger, source, DataManagerUpdateKind.Cache, requestor, context, ex); } + private static void SendAccountUpdateEvent(ILogger logger, object? source, Guid requestor, dynamic context, Exception? ex) + { + SendUpdateEvent(logger, source, DataManagerUpdateKind.Account, requestor, context, ex); + } + private static bool IsCancelException(Exception ex) { return (ex is OperationCanceledException) || (ex is TaskCanceledException); diff --git a/src/AzureExtension/DataManager/CacheManager.cs b/src/AzureExtension/DataManager/CacheManager.cs index feae06d..c9f3120 100644 --- a/src/AzureExtension/DataManager/CacheManager.cs +++ b/src/AzureExtension/DataManager/CacheManager.cs @@ -14,7 +14,7 @@ public class CacheManager : IDisposable private static readonly string _cacheManagerLastUpdatedMetaDataKey = "CacheManagerLastUpdated"; // Frequency the CacheManager checks for an update. - private static readonly TimeSpan _updateInterval = TimeSpan.FromHours(4); + private static readonly TimeSpan _updateInterval = TimeSpan.FromMinutes(15); private static readonly TimeSpan _defaultAccountUpdateFrequency = TimeSpan.FromDays(3); @@ -193,6 +193,15 @@ private void HandleDataManagerUpdate(object? source, DataManagerUpdateEventArgs Log.Debug("DataManager update received"); switch (e.Kind) { + case DataManagerUpdateKind.Account: + // Account is sent after each organization has been updated, but the entire cache + // is not necessarily updated. We will treat this as an update is still in progress, and + // notify others who may be waiting for an opportunity to query the database. + // Receiving this event means a transaction was just completed and the datastore is + // briefly unlocked for queries. + SendUpdateEvent(this, CacheManagerUpdateKind.Account); + break; + case DataManagerUpdateKind.Cache: lock (_stateLock) { diff --git a/src/AzureExtension/DataManager/CacheManagerUpdateEventArgs.cs b/src/AzureExtension/DataManager/CacheManagerUpdateEventArgs.cs index e822099..e847b7e 100644 --- a/src/AzureExtension/DataManager/CacheManagerUpdateEventArgs.cs +++ b/src/AzureExtension/DataManager/CacheManagerUpdateEventArgs.cs @@ -12,6 +12,7 @@ public enum CacheManagerUpdateKind Cleared, Error, Cancel, + Account, } public class CacheManagerUpdateEventArgs : EventArgs diff --git a/src/AzureExtension/DataManager/DataManagerUpdateEventArgs.cs b/src/AzureExtension/DataManager/DataManagerUpdateEventArgs.cs index 2a3873e..8830eb9 100644 --- a/src/AzureExtension/DataManager/DataManagerUpdateEventArgs.cs +++ b/src/AzureExtension/DataManager/DataManagerUpdateEventArgs.cs @@ -11,6 +11,7 @@ public enum DataManagerUpdateKind PullRequest, Error, Cache, + Account, Cancel, Developer, } diff --git a/src/AzureExtension/DataManager/IAzureDataManager.cs b/src/AzureExtension/DataManager/IAzureDataManager.cs index f6f0ead..ddf8eb1 100644 --- a/src/AzureExtension/DataManager/IAzureDataManager.cs +++ b/src/AzureExtension/DataManager/IAzureDataManager.cs @@ -42,6 +42,8 @@ public interface IAzureDataManager : IDisposable IEnumerable GetDeveloperRepositories(); + IEnumerable GetPullRequestsForLoggedInDeveloperIds(); + // Repository name may not be unique across projects, and projects may not be unique across // organizations, so we need all three to identify the repository. PullRequests? GetPullRequests(string organization, string project, string repositoryName, string developerId, PullRequestView view); diff --git a/src/AzureExtension/DataModel/DataObjects/PullRequests.cs b/src/AzureExtension/DataModel/DataObjects/PullRequests.cs index e324669..c8e9fd2 100644 --- a/src/AzureExtension/DataModel/DataObjects/PullRequests.cs +++ b/src/AzureExtension/DataModel/DataObjects/PullRequests.cs @@ -154,6 +154,23 @@ public static PullRequests Get(DataStore dataStore, long id) return Get(dataStore, project.Id, repository.Id, developerLogin, view); } + public static IEnumerable GetAllForDeveloper(DataStore dataStore) + { + var sql = @"SELECT * FROM PullRequests WHERE ViewId = @ViewId;"; + var param = new + { + ViewId = (long)PullRequestView.Mine, + }; + + var pullRequestsSet = dataStore.Connection!.Query(sql, param, null) ?? []; + foreach (var pullRequestsEntry in pullRequestsSet) + { + pullRequestsEntry.DataStore = dataStore; + } + + return pullRequestsSet; + } + public static PullRequests GetOrCreate(DataStore dataStore, long repositoryId, long projectId, string developerId, PullRequestView view, string pullRequests) { var newDeveloperPullRequests = Create(repositoryId, projectId, developerId, view, pullRequests); diff --git a/src/AzureExtension/Widgets/AzurePullRequestsBaseWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsBaseWidget.cs new file mode 100644 index 0000000..6f60d69 --- /dev/null +++ b/src/AzureExtension/Widgets/AzurePullRequestsBaseWidget.cs @@ -0,0 +1,126 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json.Nodes; +using DevHomeAzureExtension.Client; +using DevHomeAzureExtension.DataManager; +using DevHomeAzureExtension.DataModel; +using DevHomeAzureExtension.DeveloperId; +using DevHomeAzureExtension.Helpers; +using Microsoft.Windows.Widgets.Providers; +using Newtonsoft.Json; + +namespace DevHomeAzureExtension.Widgets; + +internal abstract class AzurePullRequestsBaseWidget : AzureWidget +{ + private readonly string _sampleIconData = IconLoader.GetIconAsBase64("screenshot.png"); + + // Widget Data + protected string WidgetTitle { get; set; } = string.Empty; + + protected string? Message { get; set; } + + // Creation and destruction methods + public AzurePullRequestsBaseWidget() + : base() + { + } + + public override void CreateWidget(WidgetContext widgetContext, string state) + { + if (state.Length != 0) + { + ResetDataFromState(state); + } + + base.CreateWidget(widgetContext, state); + } + + public override void DeleteWidget(string widgetId, string customState) + { + base.DeleteWidget(widgetId, customState); + } + + protected string GetIconForPullRequestStatus(string? prStatus) + { + prStatus ??= string.Empty; + if (Enum.TryParse(prStatus, false, out var policyStatus)) + { + return policyStatus switch + { + PolicyStatus.Approved => IconLoader.GetIconAsBase64("PullRequestApproved.png"), + PolicyStatus.Running => IconLoader.GetIconAsBase64("PullRequestWaiting.png"), + PolicyStatus.Queued => IconLoader.GetIconAsBase64("PullRequestWaiting.png"), + PolicyStatus.Rejected => IconLoader.GetIconAsBase64("PullRequestRejected.png"), + PolicyStatus.Broken => IconLoader.GetIconAsBase64("PullRequestRejected.png"), + _ => IconLoader.GetIconAsBase64("PullRequestReviewNotStarted.png"), + }; + } + + return string.Empty; + } + + protected override bool ValidateConfiguration(WidgetActionInvokedArgs args) + { + return true; + } + + // Increase precision of SetDefaultDeveloperLoginId by matching the selectedRepositoryUrl's org + // with the first matching DeveloperId that contains that org. + protected override void SetDefaultDeveloperLoginId() + { + base.SetDefaultDeveloperLoginId(); + } + + // Data loading methods + public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEventArgs e) + { + return; + } + + public override void RequestContentData() + { + return; + } + + protected override void ResetDataFromState(string data) + { + return; + } + + public override string GetConfiguration(string data) + { + return EmptyJson; + } + + public override void LoadContentData() + { + return; + } + + // Overriding methods from Widget base + public override string GetTemplatePath(WidgetPageState page) + { + return page switch + { + WidgetPageState.SignIn => @"Widgets\Templates\AzureSignInTemplate.json", + WidgetPageState.Configure => @"Widgets\Templates\AzurePullRequestsConfigurationTemplate.json", + WidgetPageState.Content => @"Widgets\Templates\AzurePullRequestsTemplate.json", + WidgetPageState.Loading => @"Widgets\Templates\AzureLoadingTemplate.json", + _ => throw new NotImplementedException(Page.GetType().Name), + }; + } + + public override string GetData(WidgetPageState page) + { + return page switch + { + WidgetPageState.SignIn => GetSignIn(), + WidgetPageState.Configure => GetConfiguration(string.Empty), + WidgetPageState.Content => ContentData, + WidgetPageState.Loading => EmptyJson, + _ => throw new NotImplementedException(Page.GetType().Name), + }; + } +} diff --git a/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs new file mode 100644 index 0000000..f7382f0 --- /dev/null +++ b/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs @@ -0,0 +1,203 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json.Nodes; +using DevHomeAzureExtension.Client; +using DevHomeAzureExtension.DataManager; +using DevHomeAzureExtension.DataModel; +using DevHomeAzureExtension.DeveloperId; +using DevHomeAzureExtension.Helpers; +using Microsoft.Windows.Widgets.Providers; +using Newtonsoft.Json; + +namespace DevHomeAzureExtension.Widgets; + +internal sealed class AzurePullRequestsDeveloperWidget : AzurePullRequestsBaseWidget +{ + private readonly CacheManager _cacheManager; + + public AzurePullRequestsDeveloperWidget() + : base() + { + SupportsCustomization = false; + DeveloperIdLoginRequired = false; + _cacheManager = CacheManager.GetInstance(); + _cacheManager.OnUpdate += HandleCacheManagerUpdate; + } + + ~AzurePullRequestsDeveloperWidget() + { + try + { + _cacheManager.OnUpdate -= HandleCacheManagerUpdate; + } + catch + { + // Best effort. + } + } + + private void HandleCacheManagerUpdate(object? source, CacheManagerUpdateEventArgs e) + { + if (e.Kind == CacheManagerUpdateKind.Updated) + { + //// + } + } + + // Data loading methods + public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEventArgs e) + { + if (e.Kind == DataManagerUpdateKind.Developer) + { + Log.Debug($"Received developer pull request update."); + + if (e.Kind == DataManagerUpdateKind.Error) + { + DataState = WidgetDataState.FailedUpdate; + DataErrorMessage = e.Context.ErrorMessage; + + // The DataManager log will have detailed exception info, use the short message. + Log.Error($"Developer pull request update failed. {e.Context.ErrorMessage}"); + return; + } + + DataState = WidgetDataState.Okay; + DataErrorMessage = string.Empty; + LoadContentData(); + } + + // If we failed data state, any data update might be an opportunity to retry since any + // update means a transaction has completed. + if (DataState == WidgetDataState.FailedRead) + { + Log.Debug("Retrying datastore read."); + LoadContentData(); + return; + } + } + + protected override bool ValidateConfiguration(WidgetActionInvokedArgs args) + { + return true; + } + + public override void RequestContentData() + { + // This widget does not request data, so we will use the periodic update + // as a refresh to try to recover from any transient issue where + // data was unavailable for whatever reason. + LoadContentData(); + } + + protected override void ResetDataFromState(string data) + { + // This widget has no state. + return; + } + + public override string GetConfiguration(string data) + { + // This widget has no configuration. + return string.Empty; + } + + public override void LoadContentData() + { + try + { + // This can throw if DataStore is not connected. + var pullRequestsList = DataManager!.GetPullRequestsForLoggedInDeveloperIds(); + + if (!pullRequestsList.Any() && _cacheManager.UpdateInProgress) + { + // First time load might not find any pull requests becuase we haven't updated the + // repository cache yet. + Log.Debug("Cache is being updated, displaying loading page."); + SetLoading(); + return; + } + + var itemsData = new JsonObject(); + var itemsArray = new JsonArray(); + + foreach (var pullRequests in pullRequestsList) + { + var pullRequestsResults = pullRequests is null ? [] : JsonConvert.DeserializeObject>(pullRequests.Results); + foreach (var element in pullRequestsResults!) + { + var workItem = JsonObject.Parse(element.Value.ToStringInvariant()); + + if (workItem != null) + { + // If we can't get the real date, it is better better to show a recent + // closer-to-correct time than the zero value decades ago, so use DateTime.UtcNow. + var dateTicks = workItem["CreationDate"]?.GetValue() ?? DateTime.UtcNow.Ticks; + var dateTime = dateTicks.ToDateTime(); + var creator = DataManager.GetIdentity(workItem["CreatedBy"]?.GetValue() ?? 0L); + var item = new JsonObject + { + { "title", workItem["Title"]?.GetValue() ?? string.Empty }, + { "url", workItem["HtmlUrl"]?.GetValue() ?? string.Empty }, + { "status_icon", GetIconForPullRequestStatus(workItem["PolicyStatus"]?.GetValue()) }, + { "number", element.Key }, + { "date", TimeSpanHelper.DateTimeOffsetToDisplayString(dateTime, Log) }, + { "dateTicks", dateTicks }, + { "user", creator.Name }, + { "branch", workItem["TargetBranch"]?.GetValue().Replace("refs/heads/", string.Empty) }, + { "avatar", creator.Avatar }, + }; + + itemsArray.Add(item); + } + } + } + + try + { + // Sort all pull requests by creation date, descending so newer are at the top of the list. + var sortedItems = itemsArray.OrderByDescending(x => x?["dateTicks"]?.GetValue()); + } + catch (Exception innerJsonEx) + { + Log.Error(innerJsonEx, $"Json sort failed {innerJsonEx.Message}"); + } + + itemsData.Add("maxItemsDisplayed", AzureDataManager.PullRequestResultLimit); + itemsData.Add("items", itemsArray); + itemsData.Add("widgetTitle", WidgetTitle); + itemsData.Add("is_loading_data", DataState == WidgetDataState.Unknown); + + ContentData = itemsData.ToJsonString(); + UpdateActivityState(); + } + catch (Exception e) + { + Log.Error(e, "Error retrieving data."); + DataState = WidgetDataState.FailedRead; + return; + } + } + + public override string GetTemplatePath(WidgetPageState page) + { + return page switch + { + WidgetPageState.SignIn => @"Widgets\Templates\AzureSignInTemplate.json", + WidgetPageState.Content => @"Widgets\Templates\AzurePullRequestsTemplate.json", + WidgetPageState.Loading => @"Widgets\Templates\AzureLoadingTemplate.json", + _ => throw new NotImplementedException(Page.GetType().Name), + }; + } + + public override string GetData(WidgetPageState page) + { + return page switch + { + WidgetPageState.SignIn => GetSignIn(), + WidgetPageState.Content => ContentData, + WidgetPageState.Loading => EmptyJson, + _ => throw new NotImplementedException(Page.GetType().Name), + }; + } +} diff --git a/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs similarity index 68% rename from src/AzureExtension/Widgets/AzurePullRequestsWidget.cs rename to src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs index 6c39002..f32c390 100644 --- a/src/AzureExtension/Widgets/AzurePullRequestsWidget.cs +++ b/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs @@ -1,357 +1,307 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Text.Json.Nodes; -using DevHomeAzureExtension.Client; -using DevHomeAzureExtension.DataManager; -using DevHomeAzureExtension.DataModel; -using DevHomeAzureExtension.DeveloperId; -using DevHomeAzureExtension.Helpers; -using Microsoft.Windows.Widgets.Providers; -using Newtonsoft.Json; - -namespace DevHomeAzureExtension.Widgets; - -internal sealed class AzurePullRequestsWidget : AzureWidget -{ - private readonly string _sampleIconData = IconLoader.GetIconAsBase64("screenshot.png"); - - private const string DefaultSelectedView = "Mine"; - - // Widget Data - private string _widgetTitle = string.Empty; - private string _selectedRepositoryUrl = string.Empty; - private string _selectedRepositoryName = string.Empty; - private string _selectedView = DefaultSelectedView; - private string? _message; - - // Creation and destruction methods - public AzurePullRequestsWidget() - : base() - { - } - - public override void CreateWidget(WidgetContext widgetContext, string state) - { - if (state.Length != 0) - { - ResetDataFromState(state); - } - - base.CreateWidget(widgetContext, state); - } - - public override void DeleteWidget(string widgetId, string customState) - { - base.DeleteWidget(widgetId, customState); - } - - // Helper methods - private PullRequestView GetPullRequestView(string viewStr) - { - try - { - return Enum.Parse(viewStr); - } - catch (Exception) - { - Log.Error($"Unknown Pull Request view for string: {viewStr}"); - return PullRequestView.Unknown; - } - } - - private string GetIconForPullRequestStatus(string? prStatus) - { - prStatus ??= string.Empty; - if (Enum.TryParse(prStatus, false, out var policyStatus)) - { - return policyStatus switch - { - PolicyStatus.Approved => IconLoader.GetIconAsBase64("PullRequestApproved.png"), - PolicyStatus.Running => IconLoader.GetIconAsBase64("PullRequestWaiting.png"), - PolicyStatus.Queued => IconLoader.GetIconAsBase64("PullRequestWaiting.png"), - PolicyStatus.Rejected => IconLoader.GetIconAsBase64("PullRequestRejected.png"), - PolicyStatus.Broken => IconLoader.GetIconAsBase64("PullRequestRejected.png"), - _ => IconLoader.GetIconAsBase64("PullRequestReviewNotStarted.png"), - }; - } - - return string.Empty; - } - - protected override bool ValidateConfiguration(WidgetActionInvokedArgs args) - { - // Set loading page while we fetch data from ADO. - Page = WidgetPageState.Loading; - UpdateWidget(); - - CanSave = false; - - Page = WidgetPageState.Configure; - var data = args.Data; - var dataObject = JsonObject.Parse(data); - _message = null; - if (dataObject != null && dataObject["account"] != null && dataObject["query"] != null) - { - _widgetTitle = dataObject["widgetTitle"]?.GetValue() ?? string.Empty; - DeveloperLoginId = dataObject["account"]?.GetValue() ?? string.Empty; - _selectedRepositoryUrl = dataObject["query"]?.GetValue() ?? string.Empty; - _selectedView = dataObject["view"]?.GetValue() ?? string.Empty; - SetDefaultDeveloperLoginId(); - if (DeveloperLoginId != dataObject["account"]?.GetValue()) - { - dataObject["account"] = DeveloperLoginId; - data = dataObject.ToJsonString(); - } - - ConfigurationData = data; - - var developerId = GetDevId(DeveloperLoginId); - if (developerId == null) - { - _message = Resources.GetResource(@"Widget_Template/DevIDError"); - UpdateActivityState(); - return false; - } - - var repositoryInfo = AzureClientHelpers.GetRepositoryInfo(_selectedRepositoryUrl, developerId); - if (repositoryInfo.Result != ResultType.Success) - { - _message = GetMessageForError(repositoryInfo.Error, repositoryInfo.ErrorMessage); - UpdateActivityState(); - return false; - } - - CanSave = true; - Pinned = true; - Page = WidgetPageState.Content; - UpdateActivityState(); - return true; - } - - return false; - } - - public override void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs) - { - // Set CanSave to false so user will have to Submit again before Saving. - CanSave = false; - SavedConfigurationData = ConfigurationData; - SetConfigure(); - } - - // Increase precision of SetDefaultDeveloperLoginId by matching the selectedRepositoryUrl's org - // with the first matching DeveloperId that contains that org. - protected override void SetDefaultDeveloperLoginId() - { - base.SetDefaultDeveloperLoginId(); - var azureOrg = new AzureUri(_selectedRepositoryUrl).Organization; - if (!string.IsNullOrEmpty(azureOrg)) - { - var devIds = DeveloperIdProvider.GetInstance().GetLoggedInDeveloperIds().DeveloperIds; - if (devIds is null) - { - return; - } - - DeveloperLoginId = devIds.Where(i => i.LoginId.Contains(azureOrg, StringComparison.OrdinalIgnoreCase)).FirstOrDefault()?.LoginId ?? DeveloperLoginId; - } - } - - // Data loading methods - public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEventArgs e) - { - if (e.Requestor.ToString() == Id) - { - Log.Debug($"Received matching query update"); - - if (e.Kind == DataManagerUpdateKind.Error) - { - DataState = WidgetDataState.Failed; - DataErrorMessage = e.Context.ErrorMessage; - - // The DataManager log will have detailed exception info, use the short message. - Log.Error($"Data update failed. {e.Context.QueryId} {e.Context.ErrorMessage}"); - return; - } - - DataState = WidgetDataState.Okay; - DataErrorMessage = string.Empty; - LoadContentData(); - } - } - - public override void RequestContentData() - { - var developerId = GetDevId(DeveloperLoginId); - if (developerId == null) - { - // Should not happen - Log.Error("Failed to get DeveloperId"); - return; - } - - try - { - var requestOptions = RequestOptions.RequestOptionsDefault(); - var azureUri = new AzureUri(_selectedRepositoryUrl); - DataManager!.UpdateDataForPullRequestsAsync(azureUri, developerId.LoginId, GetPullRequestView(_selectedView), requestOptions, new Guid(Id)); - } - catch (Exception ex) - { - Log.Error(ex, "Failed requesting data update."); - } - } - - protected override void ResetDataFromState(string data) - { - var dataObject = JsonObject.Parse(data); - - if (dataObject == null) - { - return; - } - - _widgetTitle = dataObject["widgetTitle"]?.GetValue() ?? string.Empty; - DeveloperLoginId = dataObject["account"]?.GetValue() ?? string.Empty; - _selectedRepositoryUrl = dataObject["query"]?.GetValue() ?? string.Empty; - _selectedView = dataObject["view"]?.GetValue() ?? string.Empty; - _message = null; - - var developerId = GetDevId(DeveloperLoginId); - if (developerId == null) - { - return; - } - - var azureUri = new AzureUri(_selectedRepositoryUrl); - _selectedRepositoryName = azureUri.Repository; - } - - public override string GetConfiguration(string data) - { - var configurationData = new JsonObject(); - - var developerIdsData = new JsonArray(); - - var devIdProvider = DeveloperIdProvider.GetInstance(); - - foreach (var developerId in devIdProvider.GetLoggedInDeveloperIds().DeveloperIds) - { - developerIdsData.Add(new JsonObject - { - { "devId", developerId.LoginId }, - }); - } - - configurationData.Add("accounts", developerIdsData); - - configurationData.Add("selectedDevId", DeveloperLoginId); - configurationData.Add("url", _selectedRepositoryUrl); - configurationData.Add("selectedView", _selectedView); - configurationData.Add("message", _message); - configurationData.Add("widgetTitle", _widgetTitle); - - configurationData.Add("pinned", Pinned); - configurationData.Add("arrow", IconLoader.GetIconAsBase64("arrow.png")); - - return configurationData.ToString(); - } - - public override void LoadContentData() - { - try - { - var developerId = GetDevId(DeveloperLoginId); - if (developerId == null) - { - // Should not happen - Log.Error("Failed to get Dev ID"); - return; - } - - var azureUri = new AzureUri(_selectedRepositoryUrl); - if (!azureUri.IsRepository) - { - Log.Error($"Invalid Uri: {_selectedRepositoryUrl}"); - return; - } - - PullRequests? pullRequests; - - // This can throw if DataStore is not connected. - pullRequests = DataManager!.GetPullRequests(azureUri, developerId.LoginId, GetPullRequestView(_selectedView)); - - var pullRequestsResults = pullRequests is null ? new Dictionary() : JsonConvert.DeserializeObject>(pullRequests.Results); - - var itemsData = new JsonObject(); - var itemsArray = new JsonArray(); - - foreach (var element in pullRequestsResults!) - { - var workItem = JsonObject.Parse(element.Value.ToStringInvariant()); - - if (workItem != null) - { - // If we can't get the real date, it is better better to show a recent - // closer-to-correct time than the zero value decades ago, so use DateTime.UtcNow. - var dateTicks = workItem["CreationDate"]?.GetValue() ?? DateTime.UtcNow.Ticks; - var dateTime = dateTicks.ToDateTime(); - var creator = DataManager.GetIdentity(workItem["CreatedBy"]?.GetValue() ?? 0L); - var item = new JsonObject - { - { "title", workItem["Title"]?.GetValue() ?? string.Empty }, - { "url", workItem["HtmlUrl"]?.GetValue() ?? string.Empty }, - { "status_icon", GetIconForPullRequestStatus(workItem["PolicyStatus"]?.GetValue()) }, - { "number", element.Key }, - { "date", TimeSpanHelper.DateTimeOffsetToDisplayString(dateTime, Log) }, - { "user", creator.Name }, - { "branch", workItem["TargetBranch"]?.GetValue().Replace("refs/heads/", string.Empty) }, - { "avatar", creator.Avatar }, - }; - - itemsArray.Add(item); - } - } - - itemsData.Add("maxItemsDisplayed", AzureDataManager.PullRequestResultLimit); - itemsData.Add("items", itemsArray); - itemsData.Add("widgetTitle", _widgetTitle); - itemsData.Add("is_loading_data", DataState == WidgetDataState.Unknown); - - ContentData = itemsData.ToJsonString(); - UpdateActivityState(); - } - catch (Exception e) - { - Log.Error(e, "Error retrieving data."); - DataState = WidgetDataState.Failed; - return; - } - } - - // Overriding methods from Widget base - public override string GetTemplatePath(WidgetPageState page) - { - return page switch - { - WidgetPageState.SignIn => @"Widgets\Templates\AzureSignInTemplate.json", - WidgetPageState.Configure => @"Widgets\Templates\AzurePullRequestsConfigurationTemplate.json", - WidgetPageState.Content => @"Widgets\Templates\AzurePullRequestsTemplate.json", - WidgetPageState.Loading => @"Widgets\Templates\AzureLoadingTemplate.json", - _ => throw new NotImplementedException(), - }; - } - - public override string GetData(WidgetPageState page) - { - return page switch - { - WidgetPageState.SignIn => GetSignIn(), - WidgetPageState.Configure => GetConfiguration(string.Empty), - WidgetPageState.Content => ContentData, - WidgetPageState.Loading => EmptyJson, - _ => throw new NotImplementedException(Page.GetType().Name), - }; - } -} +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json.Nodes; +using DevHomeAzureExtension.Client; +using DevHomeAzureExtension.DataManager; +using DevHomeAzureExtension.DataModel; +using DevHomeAzureExtension.DeveloperId; +using DevHomeAzureExtension.Helpers; +using Microsoft.Windows.Widgets.Providers; +using Newtonsoft.Json; + +namespace DevHomeAzureExtension.Widgets; + +internal sealed class AzurePullRequestsRepositoryWidget : AzurePullRequestsBaseWidget +{ + private static readonly string _defaultSelectedView = PullRequestView.Mine.ToString(); + + // Widget Data + private string _selectedRepositoryUrl = string.Empty; + private string _selectedView = _defaultSelectedView; + + // Creation and destruction methods + public AzurePullRequestsRepositoryWidget() + : base() + { + } + + // Helper methods + private PullRequestView GetPullRequestView(string viewStr) + { + try + { + return Enum.Parse(viewStr); + } + catch (Exception) + { + Log.Error($"Unknown Pull Request view for string: {viewStr}"); + return PullRequestView.Unknown; + } + } + + // Data loading methods + public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEventArgs e) + { + if (e.Requestor.ToString() == Id) + { + Log.Debug($"Received matching DataStore update"); + + if (e.Kind == DataManagerUpdateKind.Error) + { + DataState = WidgetDataState.FailedUpdate; + DataErrorMessage = e.Context.ErrorMessage; + + // The DataManager log will have detailed exception info, use the short message. + Log.Error($"Data update failed. {e.Context.ErrorMessage}"); + return; + } + + DataState = WidgetDataState.Okay; + DataErrorMessage = string.Empty; + LoadContentData(); + return; + } + + // If we failed data state, any data update might be an opportunity to retry since any + // update means a transaction has completed. + if (DataState == WidgetDataState.FailedRead) + { + Log.Debug("Retrying datastore read."); + LoadContentData(); + return; + } + + // If we failed to update, try again after a data update has been received. + if (DataState == WidgetDataState.FailedUpdate) + { + Log.Debug("Retrying datastore update."); + RequestContentData(); + return; + } + } + + protected override bool ValidateConfiguration(WidgetActionInvokedArgs args) + { + // Set loading page while we fetch data from ADO. + Page = WidgetPageState.Loading; + UpdateWidget(); + + CanSave = false; + + Page = WidgetPageState.Configure; + var data = args.Data; + var dataObject = JsonObject.Parse(data); + Message = null; + if (dataObject != null && dataObject["account"] != null && dataObject["query"] != null) + { + WidgetTitle = dataObject["widgetTitle"]?.GetValue() ?? string.Empty; + DeveloperLoginId = dataObject["account"]?.GetValue() ?? string.Empty; + _selectedRepositoryUrl = dataObject["query"]?.GetValue() ?? string.Empty; + _selectedView = dataObject["view"]?.GetValue() ?? string.Empty; + SetDefaultDeveloperLoginId(); + if (DeveloperLoginId != dataObject["account"]?.GetValue()) + { + dataObject["account"] = DeveloperLoginId; + data = dataObject.ToJsonString(); + } + + ConfigurationData = data; + + var developerId = GetDevId(DeveloperLoginId); + if (developerId == null) + { + Message = Resources.GetResource(@"Widget_Template/DevIDError"); + UpdateActivityState(); + return false; + } + + var repositoryInfo = AzureClientHelpers.GetRepositoryInfo(_selectedRepositoryUrl, developerId); + if (repositoryInfo.Result != ResultType.Success) + { + Message = GetMessageForError(repositoryInfo.Error, repositoryInfo.ErrorMessage); + UpdateActivityState(); + return false; + } + + CanSave = true; + Pinned = true; + Page = WidgetPageState.Content; + UpdateActivityState(); + return true; + } + + return false; + } + + // Increase precision of SetDefaultDeveloperLoginId by matching the selectedRepositoryUrl's org + // with the first matching DeveloperId that contains that org. + protected override void SetDefaultDeveloperLoginId() + { + var azureOrg = new AzureUri(_selectedRepositoryUrl).Organization; + if (!string.IsNullOrEmpty(azureOrg)) + { + var devIds = DeveloperIdProvider.GetInstance().GetLoggedInDeveloperIds().DeveloperIds; + if (devIds is null) + { + return; + } + + DeveloperLoginId = devIds.Where(i => i.LoginId.Contains(azureOrg, StringComparison.OrdinalIgnoreCase)).FirstOrDefault()?.LoginId ?? DeveloperLoginId; + } + } + + public override void RequestContentData() + { + if (DataState == WidgetDataState.Requested) + { + return; + } + + var developerId = GetDevId(DeveloperLoginId); + if (developerId == null) + { + // Should not happen + Log.Error("Failed to get DeveloperId"); + DataState = WidgetDataState.FailedUpdate; + return; + } + + try + { + var requestOptions = RequestOptions.RequestOptionsDefault(); + var azureUri = new AzureUri(_selectedRepositoryUrl); + DataState = WidgetDataState.Requested; + DataManager!.UpdateDataForPullRequestsAsync(azureUri, developerId.LoginId, GetPullRequestView(_selectedView), requestOptions, new Guid(Id)); + } + catch (Exception ex) + { + Log.Error(ex, "Failed requesting data update."); + DataState = WidgetDataState.FailedUpdate; + } + } + + protected override void ResetDataFromState(string data) + { + var dataObject = JsonObject.Parse(data); + + if (dataObject == null) + { + return; + } + + WidgetTitle = dataObject["widgetTitle"]?.GetValue() ?? string.Empty; + DeveloperLoginId = dataObject["account"]?.GetValue() ?? string.Empty; + _selectedRepositoryUrl = dataObject["query"]?.GetValue() ?? string.Empty; + _selectedView = dataObject["view"]?.GetValue() ?? string.Empty; + Message = null; + + var developerId = GetDevId(DeveloperLoginId); + if (developerId == null) + { + return; + } + } + + public override string GetConfiguration(string data) + { + var configurationData = new JsonObject(); + + var developerIdsData = new JsonArray(); + + var devIdProvider = DeveloperIdProvider.GetInstance(); + + foreach (var developerId in devIdProvider.GetLoggedInDeveloperIds().DeveloperIds) + { + developerIdsData.Add(new JsonObject + { + { "devId", developerId.LoginId }, + }); + } + + configurationData.Add("accounts", developerIdsData); + + configurationData.Add("selectedDevId", DeveloperLoginId); + configurationData.Add("url", _selectedRepositoryUrl); + configurationData.Add("selectedView", _selectedView); + configurationData.Add("message", Message); + configurationData.Add("widgetTitle", WidgetTitle); + + configurationData.Add("pinned", Pinned); + configurationData.Add("arrow", IconLoader.GetIconAsBase64("arrow.png")); + + return configurationData.ToString(); + } + + public override void LoadContentData() + { + try + { + var developerId = GetDevId(DeveloperLoginId); + if (developerId == null) + { + // Should not happen + Log.Error("Failed to get Dev ID"); + return; + } + + var azureUri = new AzureUri(_selectedRepositoryUrl); + if (!azureUri.IsRepository) + { + Log.Error($"Invalid Uri: {_selectedRepositoryUrl}"); + return; + } + + PullRequests? pullRequests; + + // This can throw if DataStore is not connected. + pullRequests = DataManager!.GetPullRequests(azureUri, developerId.LoginId, GetPullRequestView(_selectedView)); + + var pullRequestsResults = pullRequests is null ? new Dictionary() : JsonConvert.DeserializeObject>(pullRequests.Results); + + var itemsData = new JsonObject(); + var itemsArray = new JsonArray(); + + foreach (var element in pullRequestsResults!) + { + var workItem = JsonObject.Parse(element.Value.ToStringInvariant()); + + if (workItem != null) + { + // If we can't get the real date, it is better better to show a recent + // closer-to-correct time than the zero value decades ago, so use DateTime.UtcNow. + var dateTicks = workItem["CreationDate"]?.GetValue() ?? DateTime.UtcNow.Ticks; + var dateTime = dateTicks.ToDateTime(); + var creator = DataManager.GetIdentity(workItem["CreatedBy"]?.GetValue() ?? 0L); + var item = new JsonObject + { + { "title", workItem["Title"]?.GetValue() ?? string.Empty }, + { "url", workItem["HtmlUrl"]?.GetValue() ?? string.Empty }, + { "status_icon", GetIconForPullRequestStatus(workItem["PolicyStatus"]?.GetValue()) }, + { "number", element.Key }, + { "date", TimeSpanHelper.DateTimeOffsetToDisplayString(dateTime, Log) }, + { "user", creator.Name }, + { "branch", workItem["TargetBranch"]?.GetValue().Replace("refs/heads/", string.Empty) }, + { "avatar", creator.Avatar }, + }; + + itemsArray.Add(item); + } + } + + itemsData.Add("maxItemsDisplayed", AzureDataManager.PullRequestResultLimit); + itemsData.Add("items", itemsArray); + itemsData.Add("widgetTitle", WidgetTitle); + itemsData.Add("is_loading_data", DataState == WidgetDataState.Unknown); + + ContentData = itemsData.ToJsonString(); + UpdateActivityState(); + } + catch (Exception e) + { + Log.Error(e, "Error retrieving data."); + DataState = WidgetDataState.FailedRead; + return; + } + } +} diff --git a/src/AzureExtension/Widgets/AzureQueryListWidget.cs b/src/AzureExtension/Widgets/AzureQueryListWidget.cs index 35d4e3d..bf10f4c 100644 --- a/src/AzureExtension/Widgets/AzureQueryListWidget.cs +++ b/src/AzureExtension/Widgets/AzureQueryListWidget.cs @@ -165,7 +165,7 @@ public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEv if (e.Kind == DataManagerUpdateKind.Error) { - DataState = WidgetDataState.Failed; + DataState = WidgetDataState.FailedUpdate; DataErrorMessage = e.Context.ErrorMessage; // The DataManager log will have detailed exception info, use the short message. @@ -177,6 +177,15 @@ public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEv DataErrorMessage = string.Empty; LoadContentData(); } + + // If we failed data state, any data update might be an opportunity to retry since any + // update means a transaction has completed. + if (DataState == WidgetDataState.FailedRead) + { + Log.Debug("Retrying datastore read."); + LoadContentData(); + return; + } } public override void RequestContentData() @@ -331,7 +340,7 @@ public override void LoadContentData() catch (Exception e) { Log.Error(e, "Error retrieving data."); - DataState = WidgetDataState.Failed; + DataState = WidgetDataState.FailedRead; return; } } diff --git a/src/AzureExtension/Widgets/AzureQueryTilesWidget.cs b/src/AzureExtension/Widgets/AzureQueryTilesWidget.cs index cf677f4..bf80d79 100644 --- a/src/AzureExtension/Widgets/AzureQueryTilesWidget.cs +++ b/src/AzureExtension/Widgets/AzureQueryTilesWidget.cs @@ -183,7 +183,7 @@ public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEv if (e.Kind == DataManagerUpdateKind.Error) { - DataState = WidgetDataState.Failed; + DataState = WidgetDataState.FailedUpdate; DataErrorMessage = e.Context.ErrorMessage; // The DataManager log will have detailed exception info, use the short message. diff --git a/src/AzureExtension/Widgets/AzureWidget.cs b/src/AzureExtension/Widgets/AzureWidget.cs index b34c5f2..6cc09c8 100644 --- a/src/AzureExtension/Widgets/AzureWidget.cs +++ b/src/AzureExtension/Widgets/AzureWidget.cs @@ -36,6 +36,8 @@ public abstract class AzureWidget : WidgetImpl protected string DeveloperLoginId { get; set; } = string.Empty; + protected bool DeveloperIdLoginRequired { get; set; } = true; + protected bool CanSave { get; set; @@ -181,6 +183,8 @@ public override void OnActionInvoked(WidgetActionInvokedArgs actionInvokedArgs) public override void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs) { + // Set CanSave to false so user will have to Submit again before Saving. + CanSave = false; SavedConfigurationData = ConfigurationData; SetConfigure(); } @@ -235,6 +239,13 @@ protected virtual bool IsUserLoggedIn() return false; } + if (!DeveloperIdLoginRequired) + { + // At least one user is logged in, and this widget does not require a specific + // DeveloperId so we are in a good state. + return true; + } + if (string.IsNullOrEmpty(DeveloperLoginId)) { // User has not yet chosen a DeveloperId, but there is at least one available, so the @@ -283,7 +294,7 @@ public void UpdateActivityState() return; } - if (!Pinned || !CanSave) + if (SupportsCustomization && (!Pinned || !CanSave)) { SetConfigure(); return; diff --git a/src/AzureExtension/Widgets/Enums/WidgetDataState.cs b/src/AzureExtension/Widgets/Enums/WidgetDataState.cs index c229a3b..09cf84f 100644 --- a/src/AzureExtension/Widgets/Enums/WidgetDataState.cs +++ b/src/AzureExtension/Widgets/Enums/WidgetDataState.cs @@ -8,6 +8,7 @@ public enum WidgetDataState Unknown, Requested, // Request is out, waiting on a response. Current data is stale. Okay, // Received and updated data, stable state. - Failed, // Failed retrieving data. + FailedUpdate, // Failed updating data. + FailedRead, // Failed to read the data. Disposed, // DataManager has been disposed and should not be used. } diff --git a/src/AzureExtension/Widgets/WidgetImpl.cs b/src/AzureExtension/Widgets/WidgetImpl.cs index 8b30da3..9cc7d9e 100644 --- a/src/AzureExtension/Widgets/WidgetImpl.cs +++ b/src/AzureExtension/Widgets/WidgetImpl.cs @@ -19,6 +19,8 @@ public WidgetImpl() protected ILogger Log => _log.Value; + protected bool SupportsCustomization { get; set; } = true; + protected string Name => GetType().Name; protected string Id { get; set; } = string.Empty; diff --git a/src/AzureExtension/Widgets/WidgetProvider.cs b/src/AzureExtension/Widgets/WidgetProvider.cs index 5f13a4b..81ccfed 100644 --- a/src/AzureExtension/Widgets/WidgetProvider.cs +++ b/src/AzureExtension/Widgets/WidgetProvider.cs @@ -19,7 +19,8 @@ public WidgetProvider() _log.Debug("Provider Constructed"); _widgetDefinitionRegistry.Add("Azure_QueryList", new WidgetImplFactory()); _widgetDefinitionRegistry.Add("Azure_QueryTiles", new WidgetImplFactory()); - _widgetDefinitionRegistry.Add("Azure_PullRequests", new WidgetImplFactory()); + _widgetDefinitionRegistry.Add("Azure_PullRequests", new WidgetImplFactory()); + _widgetDefinitionRegistry.Add("Azure_MyPullRequests", new WidgetImplFactory()); RecoverRunningWidgets(); } diff --git a/src/AzureExtensionServer/Package.appxmanifest b/src/AzureExtensionServer/Package.appxmanifest index ca0776f..e67ecd0 100644 --- a/src/AzureExtensionServer/Package.appxmanifest +++ b/src/AzureExtensionServer/Package.appxmanifest @@ -184,6 +184,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 464cf260d39bff97f036c65032ac1c9cd7d47e2f Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 26 Jul 2024 17:23:59 -0700 Subject: [PATCH 11/16] Fix bad merge, data loading --- .../DataManager/IAzureDataManager.cs | 3 - .../AzurePullRequestsRepositoryWidget.cs | 378 +----------------- src/AzureExtension/Widgets/AzureWidget.cs | 3 + src/AzureExtension/Widgets/WidgetProvider.cs | 2 +- .../Package-Can.appxmanifest | 37 ++ .../Package-Dev.appxmanifest | 37 ++ src/AzureExtensionServer/Package.appxmanifest | 2 +- 7 files changed, 98 insertions(+), 364 deletions(-) diff --git a/src/AzureExtension/DataManager/IAzureDataManager.cs b/src/AzureExtension/DataManager/IAzureDataManager.cs index fa3e7af..ddf8eb1 100644 --- a/src/AzureExtension/DataManager/IAzureDataManager.cs +++ b/src/AzureExtension/DataManager/IAzureDataManager.cs @@ -42,11 +42,8 @@ public interface IAzureDataManager : IDisposable IEnumerable GetDeveloperRepositories(); -<<<<<<< HEAD IEnumerable GetPullRequestsForLoggedInDeveloperIds(); -======= ->>>>>>> main // Repository name may not be unique across projects, and projects may not be unique across // organizations, so we need all three to identify the repository. PullRequests? GetPullRequests(string organization, string project, string repositoryName, string developerId, PullRequestView view); diff --git a/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs index c2c9629..9cc055c 100644 --- a/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs +++ b/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs @@ -1,5 +1,4 @@ -<<<<<<< HEAD:src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using System.Text.Json.Nodes; @@ -55,6 +54,14 @@ public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEv // The DataManager log will have detailed exception info, use the short message. Log.Error($"Data update failed. {e.Context.ErrorMessage}"); + + if (!LoadedDataSuccessfully) + { + // We have not loaded data successfully, so show a loading page + // instead of an error page. + SetLoading(); + } + return; } @@ -238,6 +245,11 @@ public override void LoadContentData() { try { + if (!LoadedDataSuccessfully) + { + SetLoading(); + } + var developerId = GetDevId(DeveloperLoginId); if (developerId == null) { @@ -293,375 +305,23 @@ public override void LoadContentData() itemsData.Add("maxItemsDisplayed", AzureDataManager.PullRequestResultLimit); itemsData.Add("items", itemsArray); itemsData.Add("widgetTitle", WidgetTitle); - itemsData.Add("is_loading_data", DataState == WidgetDataState.Unknown); + itemsData.Add("is_loading_data", DataState != WidgetDataState.Unknown); ContentData = itemsData.ToJsonString(); + DataState = WidgetDataState.Okay; + LoadedDataSuccessfully = true; UpdateActivityState(); } catch (Exception e) { Log.Error(e, "Error retrieving data."); DataState = WidgetDataState.FailedRead; - return; - } - } -} -======= -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Text.Json.Nodes; -using DevHomeAzureExtension.Client; -using DevHomeAzureExtension.DataManager; -using DevHomeAzureExtension.DataModel; -using DevHomeAzureExtension.DeveloperId; -using DevHomeAzureExtension.Helpers; -using Microsoft.Windows.Widgets.Providers; -using Newtonsoft.Json; - -namespace DevHomeAzureExtension.Widgets; - -internal sealed class AzurePullRequestsWidget : AzureWidget -{ - private readonly string _sampleIconData = IconLoader.GetIconAsBase64("screenshot.png"); - - private const string DefaultSelectedView = "Mine"; - - // Widget Data - private string _widgetTitle = string.Empty; - private string _selectedRepositoryUrl = string.Empty; - private string _selectedRepositoryName = string.Empty; - private string _selectedView = DefaultSelectedView; - private string? _message; - - // Creation and destruction methods - public AzurePullRequestsWidget() - : base() - { - } - - public override void CreateWidget(WidgetContext widgetContext, string state) - { - if (state.Length != 0) - { - ResetDataFromState(state); - } - - base.CreateWidget(widgetContext, state); - } - - public override void DeleteWidget(string widgetId, string customState) - { - base.DeleteWidget(widgetId, customState); - } - - // Helper methods - private PullRequestView GetPullRequestView(string viewStr) - { - try - { - return Enum.Parse(viewStr); - } - catch (Exception) - { - Log.Error($"Unknown Pull Request view for string: {viewStr}"); - return PullRequestView.Unknown; - } - } - - private string GetIconForPullRequestStatus(string? prStatus) - { - prStatus ??= string.Empty; - if (Enum.TryParse(prStatus, false, out var policyStatus)) - { - return policyStatus switch + if (!LoadedDataSuccessfully) { - PolicyStatus.Approved => IconLoader.GetIconAsBase64("PullRequestApproved.png"), - PolicyStatus.Running => IconLoader.GetIconAsBase64("PullRequestWaiting.png"), - PolicyStatus.Queued => IconLoader.GetIconAsBase64("PullRequestWaiting.png"), - PolicyStatus.Rejected => IconLoader.GetIconAsBase64("PullRequestRejected.png"), - PolicyStatus.Broken => IconLoader.GetIconAsBase64("PullRequestRejected.png"), - _ => IconLoader.GetIconAsBase64("PullRequestReviewNotStarted.png"), - }; - } - - return string.Empty; - } - - protected override bool ValidateConfiguration(WidgetActionInvokedArgs args) - { - // Set loading page while we fetch data from ADO. - Page = WidgetPageState.Loading; - UpdateWidget(); - - CanSave = false; - - Page = WidgetPageState.Configure; - var data = args.Data; - var dataObject = JsonObject.Parse(data); - _message = null; - if (dataObject != null && dataObject["account"] != null && dataObject["query"] != null) - { - _widgetTitle = dataObject["widgetTitle"]?.GetValue() ?? string.Empty; - DeveloperLoginId = dataObject["account"]?.GetValue() ?? string.Empty; - _selectedRepositoryUrl = dataObject["query"]?.GetValue() ?? string.Empty; - _selectedView = dataObject["view"]?.GetValue() ?? string.Empty; - SetDefaultDeveloperLoginId(); - if (DeveloperLoginId != dataObject["account"]?.GetValue()) - { - dataObject["account"] = DeveloperLoginId; - data = dataObject.ToJsonString(); - } - - ConfigurationData = data; - - var developerId = GetDevId(DeveloperLoginId); - if (developerId == null) - { - _message = Resources.GetResource(@"Widget_Template/DevIDError"); - UpdateActivityState(); - return false; - } - - var repositoryInfo = AzureClientHelpers.GetRepositoryInfo(_selectedRepositoryUrl, developerId); - if (repositoryInfo.Result != ResultType.Success) - { - _message = GetMessageForError(repositoryInfo.Error, repositoryInfo.ErrorMessage); - UpdateActivityState(); - return false; - } - - CanSave = true; - Pinned = true; - Page = WidgetPageState.Content; - UpdateActivityState(); - return true; - } - - return false; - } - - public override void OnCustomizationRequested(WidgetCustomizationRequestedArgs customizationRequestedArgs) - { - // Set CanSave to false so user will have to Submit again before Saving. - CanSave = false; - SavedConfigurationData = ConfigurationData; - SetConfigure(); - } - - // Increase precision of SetDefaultDeveloperLoginId by matching the selectedRepositoryUrl's org - // with the first matching DeveloperId that contains that org. - protected override void SetDefaultDeveloperLoginId() - { - base.SetDefaultDeveloperLoginId(); - var azureOrg = new AzureUri(_selectedRepositoryUrl).Organization; - if (!string.IsNullOrEmpty(azureOrg)) - { - var devIds = DeveloperIdProvider.GetInstance().GetLoggedInDeveloperIds().DeveloperIds; - if (devIds is null) - { - return; + SetLoading(); } - DeveloperLoginId = devIds.Where(i => i.LoginId.Contains(azureOrg, StringComparison.OrdinalIgnoreCase)).FirstOrDefault()?.LoginId ?? DeveloperLoginId; - } - } - - // Data loading methods - public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEventArgs e) - { - if (e.Requestor.ToString() == Id) - { - Log.Debug($"Received matching query update"); - - if (e.Kind == DataManagerUpdateKind.Error) - { - DataState = WidgetDataState.Failed; - DataErrorMessage = e.Context.ErrorMessage; - - // The DataManager log will have detailed exception info, use the short message. - Log.Error($"Data update failed. {e.Context.QueryId} {e.Context.ErrorMessage}"); - return; - } - - DataState = WidgetDataState.Okay; - DataErrorMessage = string.Empty; - LoadContentData(); - } - } - - public override void RequestContentData() - { - var developerId = GetDevId(DeveloperLoginId); - if (developerId == null) - { - // Should not happen - Log.Error("Failed to get DeveloperId"); - return; - } - - try - { - var requestOptions = RequestOptions.RequestOptionsDefault(); - var azureUri = new AzureUri(_selectedRepositoryUrl); - DataManager!.UpdateDataForPullRequestsAsync(azureUri, developerId.LoginId, GetPullRequestView(_selectedView), requestOptions, new Guid(Id)); - } - catch (Exception ex) - { - Log.Error(ex, "Failed requesting data update."); - } - } - - protected override void ResetDataFromState(string data) - { - var dataObject = JsonObject.Parse(data); - - if (dataObject == null) - { - return; - } - - _widgetTitle = dataObject["widgetTitle"]?.GetValue() ?? string.Empty; - DeveloperLoginId = dataObject["account"]?.GetValue() ?? string.Empty; - _selectedRepositoryUrl = dataObject["query"]?.GetValue() ?? string.Empty; - _selectedView = dataObject["view"]?.GetValue() ?? string.Empty; - _message = null; - - var developerId = GetDevId(DeveloperLoginId); - if (developerId == null) - { - return; - } - - var azureUri = new AzureUri(_selectedRepositoryUrl); - _selectedRepositoryName = azureUri.Repository; - } - - public override string GetConfiguration(string data) - { - var configurationData = new JsonObject(); - - var developerIdsData = new JsonArray(); - - var devIdProvider = DeveloperIdProvider.GetInstance(); - - foreach (var developerId in devIdProvider.GetLoggedInDeveloperIds().DeveloperIds) - { - developerIdsData.Add(new JsonObject - { - { "devId", developerId.LoginId }, - }); - } - - configurationData.Add("accounts", developerIdsData); - - configurationData.Add("selectedDevId", DeveloperLoginId); - configurationData.Add("url", _selectedRepositoryUrl); - configurationData.Add("selectedView", _selectedView); - configurationData.Add("message", _message); - configurationData.Add("widgetTitle", _widgetTitle); - - configurationData.Add("pinned", Pinned); - configurationData.Add("arrow", IconLoader.GetIconAsBase64("arrow.png")); - - return configurationData.ToString(); - } - - public override void LoadContentData() - { - try - { - var developerId = GetDevId(DeveloperLoginId); - if (developerId == null) - { - // Should not happen - Log.Error("Failed to get Dev ID"); - return; - } - - var azureUri = new AzureUri(_selectedRepositoryUrl); - if (!azureUri.IsRepository) - { - Log.Error($"Invalid Uri: {_selectedRepositoryUrl}"); - return; - } - - PullRequests? pullRequests; - - // This can throw if DataStore is not connected. - pullRequests = DataManager!.GetPullRequests(azureUri, developerId.LoginId, GetPullRequestView(_selectedView)); - - var pullRequestsResults = pullRequests is null ? new Dictionary() : JsonConvert.DeserializeObject>(pullRequests.Results); - - var itemsData = new JsonObject(); - var itemsArray = new JsonArray(); - - foreach (var element in pullRequestsResults!) - { - var workItem = JsonObject.Parse(element.Value.ToStringInvariant()); - - if (workItem != null) - { - // If we can't get the real date, it is better better to show a recent - // closer-to-correct time than the zero value decades ago, so use DateTime.UtcNow. - var dateTicks = workItem["CreationDate"]?.GetValue() ?? DateTime.UtcNow.Ticks; - var dateTime = dateTicks.ToDateTime(); - var creator = DataManager.GetIdentity(workItem["CreatedBy"]?.GetValue() ?? 0L); - var item = new JsonObject - { - { "title", workItem["Title"]?.GetValue() ?? string.Empty }, - { "url", workItem["HtmlUrl"]?.GetValue() ?? string.Empty }, - { "status_icon", GetIconForPullRequestStatus(workItem["PolicyStatus"]?.GetValue()) }, - { "number", element.Key }, - { "date", TimeSpanHelper.DateTimeOffsetToDisplayString(dateTime, Log) }, - { "user", creator.Name }, - { "branch", workItem["TargetBranch"]?.GetValue().Replace("refs/heads/", string.Empty) }, - { "avatar", creator.Avatar }, - }; - - itemsArray.Add(item); - } - } - - itemsData.Add("maxItemsDisplayed", AzureDataManager.PullRequestResultLimit); - itemsData.Add("items", itemsArray); - itemsData.Add("widgetTitle", _widgetTitle); - itemsData.Add("is_loading_data", DataState == WidgetDataState.Unknown); - - ContentData = itemsData.ToJsonString(); - UpdateActivityState(); - } - catch (Exception e) - { - Log.Error(e, "Error retrieving data."); - DataState = WidgetDataState.Failed; return; } } - - // Overriding methods from Widget base - public override string GetTemplatePath(WidgetPageState page) - { - return page switch - { - WidgetPageState.SignIn => @"Widgets\Templates\AzureSignInTemplate.json", - WidgetPageState.Configure => @"Widgets\Templates\AzurePullRequestsConfigurationTemplate.json", - WidgetPageState.Content => @"Widgets\Templates\AzurePullRequestsTemplate.json", - WidgetPageState.Loading => @"Widgets\Templates\AzureLoadingTemplate.json", - _ => throw new NotImplementedException(), - }; - } - - public override string GetData(WidgetPageState page) - { - return page switch - { - WidgetPageState.SignIn => GetSignIn(), - WidgetPageState.Configure => GetConfiguration(string.Empty), - WidgetPageState.Content => ContentData, - WidgetPageState.Loading => EmptyJson, - _ => throw new NotImplementedException(Page.GetType().Name), - }; - } } ->>>>>>> main:src/AzureExtension/Widgets/AzurePullRequestsWidget.cs diff --git a/src/AzureExtension/Widgets/AzureWidget.cs b/src/AzureExtension/Widgets/AzureWidget.cs index 6cc09c8..49c1714 100644 --- a/src/AzureExtension/Widgets/AzureWidget.cs +++ b/src/AzureExtension/Widgets/AzureWidget.cs @@ -38,6 +38,8 @@ public abstract class AzureWidget : WidgetImpl protected bool DeveloperIdLoginRequired { get; set; } = true; + protected bool LoadedDataSuccessfully { get; set; } + protected bool CanSave { get; set; @@ -158,6 +160,7 @@ public override void OnActionInvoked(WidgetActionInvokedArgs actionInvokedArgs) SavedConfigurationData = string.Empty; ContentData = EmptyJson; DataState = WidgetDataState.Unknown; + LoadedDataSuccessfully = false; SetActive(); } diff --git a/src/AzureExtension/Widgets/WidgetProvider.cs b/src/AzureExtension/Widgets/WidgetProvider.cs index 7e6e59a..5af7b2d 100644 --- a/src/AzureExtension/Widgets/WidgetProvider.cs +++ b/src/AzureExtension/Widgets/WidgetProvider.cs @@ -26,7 +26,7 @@ public WidgetProvider() _widgetDefinitionRegistry.Add("Azure_QueryList", new WidgetImplFactory()); _widgetDefinitionRegistry.Add("Azure_QueryTiles", new WidgetImplFactory()); _widgetDefinitionRegistry.Add("Azure_PullRequests", new WidgetImplFactory()); - _widgetDefinitionRegistry.Add("Azure_MyPullRequests", new WidgetImplFactory()); + _widgetDefinitionRegistry.Add("Azure_MyPRs", new WidgetImplFactory()); RecoverRunningWidgets(); } diff --git a/src/AzureExtensionServer/Package-Can.appxmanifest b/src/AzureExtensionServer/Package-Can.appxmanifest index e517a13..6732741 100644 --- a/src/AzureExtensionServer/Package-Can.appxmanifest +++ b/src/AzureExtensionServer/Package-Can.appxmanifest @@ -184,6 +184,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/AzureExtensionServer/Package-Dev.appxmanifest b/src/AzureExtensionServer/Package-Dev.appxmanifest index 902ed1f..6d415e2 100644 --- a/src/AzureExtensionServer/Package-Dev.appxmanifest +++ b/src/AzureExtensionServer/Package-Dev.appxmanifest @@ -184,6 +184,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/AzureExtensionServer/Package.appxmanifest b/src/AzureExtensionServer/Package.appxmanifest index 494b3d1..0890533 100644 --- a/src/AzureExtensionServer/Package.appxmanifest +++ b/src/AzureExtensionServer/Package.appxmanifest @@ -184,7 +184,7 @@ - + From 69fbb9d8191eca05c0b6e6520d6b87d5114c1aec Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 26 Jul 2024 22:01:17 -0700 Subject: [PATCH 12/16] Update pr title and loading --- .../DataManager/AzureDataManager.cs | 1 + .../Widgets/AzurePullRequestsDeveloperWidget.cs | 17 +++++++++++++++++ .../AzurePullRequestsRepositoryWidget.cs | 15 ++++++++++----- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/AzureExtension/DataManager/AzureDataManager.cs b/src/AzureExtension/DataManager/AzureDataManager.cs index a36a55d..59dfb66 100644 --- a/src/AzureExtension/DataManager/AzureDataManager.cs +++ b/src/AzureExtension/DataManager/AzureDataManager.cs @@ -760,6 +760,7 @@ private async Task ProcessPullRequests( pullRequestObjFields.Add("Id", pullRequest.PullRequestId); pullRequestObjFields.Add("Title", pullRequest.Title); + pullRequestObjFields.Add("RepositoryId", repository.Id); pullRequestObjFields.Add("Status", pullRequest.Status); pullRequestObjFields.Add("PolicyStatus", status.ToString()); pullRequestObjFields.Add("PolicyStatusReason", statusReason); diff --git a/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs index f7382f0..9b02ff9 100644 --- a/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs +++ b/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs @@ -59,6 +59,12 @@ public override void HandleDataManagerUpdate(object? source, DataManagerUpdateEv // The DataManager log will have detailed exception info, use the short message. Log.Error($"Developer pull request update failed. {e.Context.ErrorMessage}"); + + if (!LoadedDataSuccessfully) + { + SetLoading(); + } + return; } @@ -104,6 +110,11 @@ public override string GetConfiguration(string data) public override void LoadContentData() { + if (!LoadedDataSuccessfully) + { + SetLoading(); + } + try { // This can throw if DataStore is not connected. @@ -169,12 +180,18 @@ public override void LoadContentData() itemsData.Add("is_loading_data", DataState == WidgetDataState.Unknown); ContentData = itemsData.ToJsonString(); + DataState = WidgetDataState.Okay; UpdateActivityState(); } catch (Exception e) { Log.Error(e, "Error retrieving data."); DataState = WidgetDataState.FailedRead; + if (!LoadedDataSuccessfully) + { + SetLoading(); + } + return; } } diff --git a/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs index 9cc055c..9dff8f9 100644 --- a/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs +++ b/src/AzureExtension/Widgets/AzurePullRequestsRepositoryWidget.cs @@ -243,13 +243,13 @@ public override string GetConfiguration(string data) public override void LoadContentData() { - try + if (!LoadedDataSuccessfully) { - if (!LoadedDataSuccessfully) - { - SetLoading(); - } + SetLoading(); + } + try + { var developerId = GetDevId(DeveloperLoginId); if (developerId == null) { @@ -302,6 +302,11 @@ public override void LoadContentData() } } + if (string.IsNullOrEmpty(WidgetTitle)) + { + WidgetTitle = azureUri.Repository; + } + itemsData.Add("maxItemsDisplayed", AzureDataManager.PullRequestResultLimit); itemsData.Add("items", itemsArray); itemsData.Add("widgetTitle", WidgetTitle); From 1f8bdd18b9ce994e4bb698ee80042555bbd1b76c Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 29 Jul 2024 14:22:59 -0700 Subject: [PATCH 13/16] Fix loading message and behavior --- src/AzureExtension/AzureExtension.csproj | 1 - .../DataManager/CacheManager.cs | 2 + .../Strings/en-US/Resources.resw | 5 ++ .../Widgets/AzurePullRequestsBaseWidget.cs | 2 +- .../AzurePullRequestsDeveloperWidget.cs | 57 +++++-------------- src/AzureExtension/Widgets/AzureWidget.cs | 15 +++++ .../Templates/AzureLoadingTemplate.json | 10 ++-- 7 files changed, 43 insertions(+), 49 deletions(-) diff --git a/src/AzureExtension/AzureExtension.csproj b/src/AzureExtension/AzureExtension.csproj index bcb41d9..a7893f7 100644 --- a/src/AzureExtension/AzureExtension.csproj +++ b/src/AzureExtension/AzureExtension.csproj @@ -10,7 +10,6 @@ - diff --git a/src/AzureExtension/DataManager/CacheManager.cs b/src/AzureExtension/DataManager/CacheManager.cs index c9f3120..4a9b47a 100644 --- a/src/AzureExtension/DataManager/CacheManager.cs +++ b/src/AzureExtension/DataManager/CacheManager.cs @@ -32,6 +32,8 @@ public class CacheManager : IDisposable public bool UpdateInProgress { get; private set; } + public bool NeverUpdated => LastUpdated == DateTime.MinValue; + public DateTime LastUpdated { get => GetLastUpdated(); diff --git a/src/AzureExtension/Strings/en-US/Resources.resw b/src/AzureExtension/Strings/en-US/Resources.resw index c221891..4cb5d08 100644 --- a/src/AzureExtension/Strings/en-US/Resources.resw +++ b/src/AzureExtension/Strings/en-US/Resources.resw @@ -726,4 +726,9 @@ Rejected Shown in Toast Notification, title line + + Finding your pull requests. +This may take several minutes. + Shown in Widget + \ No newline at end of file diff --git a/src/AzureExtension/Widgets/AzurePullRequestsBaseWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsBaseWidget.cs index 6f60d69..4b113cc 100644 --- a/src/AzureExtension/Widgets/AzurePullRequestsBaseWidget.cs +++ b/src/AzureExtension/Widgets/AzurePullRequestsBaseWidget.cs @@ -119,7 +119,7 @@ public override string GetData(WidgetPageState page) WidgetPageState.SignIn => GetSignIn(), WidgetPageState.Configure => GetConfiguration(string.Empty), WidgetPageState.Content => ContentData, - WidgetPageState.Loading => EmptyJson, + WidgetPageState.Loading => GetLoadingMessage(), _ => throw new NotImplementedException(Page.GetType().Name), }; } diff --git a/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs index 9b02ff9..51e2203 100644 --- a/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs +++ b/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Text.Json; using System.Text.Json.Nodes; using DevHomeAzureExtension.Client; using DevHomeAzureExtension.DataManager; @@ -22,27 +23,7 @@ public AzurePullRequestsDeveloperWidget() SupportsCustomization = false; DeveloperIdLoginRequired = false; _cacheManager = CacheManager.GetInstance(); - _cacheManager.OnUpdate += HandleCacheManagerUpdate; - } - - ~AzurePullRequestsDeveloperWidget() - { - try - { - _cacheManager.OnUpdate -= HandleCacheManagerUpdate; - } - catch - { - // Best effort. - } - } - - private void HandleCacheManagerUpdate(object? source, CacheManagerUpdateEventArgs e) - { - if (e.Kind == CacheManagerUpdateKind.Updated) - { - //// - } + LoadingMessage = Resources.GetResource("Widget_Template/LoadingFindingPullRequests", Log); } // Data loading methods @@ -110,25 +91,16 @@ public override string GetConfiguration(string data) public override void LoadContentData() { - if (!LoadedDataSuccessfully) + if (_cacheManager.NeverUpdated) { SetLoading(); + return; } try { // This can throw if DataStore is not connected. var pullRequestsList = DataManager!.GetPullRequestsForLoggedInDeveloperIds(); - - if (!pullRequestsList.Any() && _cacheManager.UpdateInProgress) - { - // First time load might not find any pull requests becuase we haven't updated the - // repository cache yet. - Log.Debug("Cache is being updated, displaying loading page."); - SetLoading(); - return; - } - var itemsData = new JsonObject(); var itemsArray = new JsonArray(); @@ -164,20 +136,20 @@ public override void LoadContentData() } } - try + // Sort all pull requests by creation date, descending so newer are at the top of the list. + var sortedItems = itemsArray.OrderByDescending(x => x?["dateTicks"]?.GetValue()); + var sortedItemsArray = new JsonArray(); + foreach (var item in sortedItems) { - // Sort all pull requests by creation date, descending so newer are at the top of the list. - var sortedItems = itemsArray.OrderByDescending(x => x?["dateTicks"]?.GetValue()); - } - catch (Exception innerJsonEx) - { - Log.Error(innerJsonEx, $"Json sort failed {innerJsonEx.Message}"); + // Parent is read-only and fixed, use deep clone to re-parent the node. + var itemClone = item?.DeepClone(); + sortedItemsArray.Add(itemClone); } itemsData.Add("maxItemsDisplayed", AzureDataManager.PullRequestResultLimit); - itemsData.Add("items", itemsArray); + itemsData.Add("items", sortedItemsArray); itemsData.Add("widgetTitle", WidgetTitle); - itemsData.Add("is_loading_data", DataState == WidgetDataState.Unknown); + itemsData.Add("is_loading_data", !LoadedDataSuccessfully); ContentData = itemsData.ToJsonString(); DataState = WidgetDataState.Okay; @@ -187,6 +159,7 @@ public override void LoadContentData() { Log.Error(e, "Error retrieving data."); DataState = WidgetDataState.FailedRead; + if (!LoadedDataSuccessfully) { SetLoading(); @@ -213,7 +186,7 @@ public override string GetData(WidgetPageState page) { WidgetPageState.SignIn => GetSignIn(), WidgetPageState.Content => ContentData, - WidgetPageState.Loading => EmptyJson, + WidgetPageState.Loading => GetLoadingMessage(), _ => throw new NotImplementedException(Page.GetType().Name), }; } diff --git a/src/AzureExtension/Widgets/AzureWidget.cs b/src/AzureExtension/Widgets/AzureWidget.cs index 49c1714..0c5fd94 100644 --- a/src/AzureExtension/Widgets/AzureWidget.cs +++ b/src/AzureExtension/Widgets/AzureWidget.cs @@ -34,6 +34,8 @@ public abstract class AzureWidget : WidgetImpl protected string ContentData { get; set; } = EmptyJson; + protected string LoadingMessage { get; set; } = string.Empty; + protected string DeveloperLoginId { get; set; } = string.Empty; protected bool DeveloperIdLoginRequired { get; set; } = true; @@ -265,6 +267,19 @@ protected virtual bool IsUserLoggedIn() return false; } + public virtual string GetLoadingMessage() + { + if (string.IsNullOrEmpty(LoadingMessage)) + { + return EmptyJson; + } + + return new JsonObject + { + { "loadingMessage", LoadingMessage }, + }.ToJsonString(); + } + protected DeveloperId.DeveloperId? GetDevId(string login) { var devIdProvider = DeveloperIdProvider.GetInstance(); diff --git a/src/AzureExtension/Widgets/Templates/AzureLoadingTemplate.json b/src/AzureExtension/Widgets/Templates/AzureLoadingTemplate.json index 9722dcc..6377568 100644 --- a/src/AzureExtension/Widgets/Templates/AzureLoadingTemplate.json +++ b/src/AzureExtension/Widgets/Templates/AzureLoadingTemplate.json @@ -7,11 +7,11 @@ "type": "Container", "items": [ { - "type": "TextBlock", - "text": "%Widget_Template/Loading%", - "wrap": true, - "weight": "bolder", - "horizontalAlignment": "Center" + "type": "TextBlock", + "text": "${if(loadingMessage, loadingMessage, '%Widget_Template/Loading%')}", + "wrap": true, + "weight": "bolder", + "horizontalAlignment": "Center" } ], "horizontalAlignment": "Center", From 6e75050392957e90f010d1ff264217fa65de70a6 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 30 Jul 2024 11:53:46 -0700 Subject: [PATCH 14/16] Fix spacing --- .../Widgets/Templates/AzureLoadingTemplate.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/AzureExtension/Widgets/Templates/AzureLoadingTemplate.json b/src/AzureExtension/Widgets/Templates/AzureLoadingTemplate.json index 6377568..d59fd7e 100644 --- a/src/AzureExtension/Widgets/Templates/AzureLoadingTemplate.json +++ b/src/AzureExtension/Widgets/Templates/AzureLoadingTemplate.json @@ -7,11 +7,11 @@ "type": "Container", "items": [ { - "type": "TextBlock", - "text": "${if(loadingMessage, loadingMessage, '%Widget_Template/Loading%')}", - "wrap": true, - "weight": "bolder", - "horizontalAlignment": "Center" + "type": "TextBlock", + "text": "${if(loadingMessage, loadingMessage, '%Widget_Template/Loading%')}", + "wrap": true, + "weight": "bolder", + "horizontalAlignment": "Center" } ], "horizontalAlignment": "Center", From 06775a2fad67fefadd3833a1c53d5d2262ebcd22 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 30 Jul 2024 21:47:15 -0700 Subject: [PATCH 15/16] PR feedback --- .../DataManager/AzureDataManagerCache.cs | 6 +++--- src/AzureExtension/Strings/en-US/Resources.resw | 3 +-- .../Widgets/AzurePullRequestsDeveloperWidget.cs | 13 +++++++++---- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/AzureExtension/DataManager/AzureDataManagerCache.cs b/src/AzureExtension/DataManager/AzureDataManagerCache.cs index c9dc076..9a21d21 100644 --- a/src/AzureExtension/DataManager/AzureDataManagerCache.cs +++ b/src/AzureExtension/DataManager/AzureDataManagerCache.cs @@ -24,7 +24,7 @@ public partial class AzureDataManager private static readonly int _pullRequestRepositoryLimit = 25; - private static readonly TimeSpan _orgUpdateSleepTime = TimeSpan.FromSeconds(5); + private static readonly TimeSpan _orgUpdateDelayTime = TimeSpan.FromSeconds(5); public async Task UpdateDataForAccountsAsync(RequestOptions? options = null, Guid? requestor = null) { @@ -119,10 +119,10 @@ public async Task UpdateDataForAccountsAsync(TimeSpan olderThan, RequestOptions? SendAccountUpdateEvent(_log, this, requestorGuid, orgUpdateContext, firstException); ++accountsUpdated; - // Sleep to allow widgets and other code to respond to the event and use the database. + // Delay to allow widgets and other code to respond to the event and use the database. // This is to prevent DOS'ing widgets using the datastore during large cache updates of // many organizations. - Thread.Sleep(_orgUpdateSleepTime); + await Task.Delay(_orgUpdateDelayTime); } catch (Exception ex) when (IsCancelException(ex)) { diff --git a/src/AzureExtension/Strings/en-US/Resources.resw b/src/AzureExtension/Strings/en-US/Resources.resw index 4cb5d08..3798dd4 100644 --- a/src/AzureExtension/Strings/en-US/Resources.resw +++ b/src/AzureExtension/Strings/en-US/Resources.resw @@ -727,8 +727,7 @@ Shown in Toast Notification, title line - Finding your pull requests. -This may take several minutes. + Finding your pull requests. This may take several minutes. Shown in Widget \ No newline at end of file diff --git a/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs b/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs index 51e2203..f7910ce 100644 --- a/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs +++ b/src/AzureExtension/Widgets/AzurePullRequestsDeveloperWidget.cs @@ -104,10 +104,15 @@ public override void LoadContentData() var itemsData = new JsonObject(); var itemsArray = new JsonArray(); - foreach (var pullRequests in pullRequestsList) + foreach (var pullRequests in pullRequestsList.Where(x => x is not null)) { - var pullRequestsResults = pullRequests is null ? [] : JsonConvert.DeserializeObject>(pullRequests.Results); - foreach (var element in pullRequestsResults!) + var pullRequestsResults = JsonConvert.DeserializeObject>(pullRequests.Results); + if (pullRequestsResults is null) + { + continue; + } + + foreach (var element in pullRequestsResults) { var workItem = JsonObject.Parse(element.Value.ToStringInvariant()); @@ -137,7 +142,7 @@ public override void LoadContentData() } // Sort all pull requests by creation date, descending so newer are at the top of the list. - var sortedItems = itemsArray.OrderByDescending(x => x?["dateTicks"]?.GetValue()); + var sortedItems = itemsArray.Where(x => x is not null).OrderByDescending(x => x?["dateTicks"]?.GetValue()); var sortedItemsArray = new JsonArray(); foreach (var item in sortedItems) { From bfc18b7f9588ac0a05d6fd1726d50e9372f5a5a9 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 30 Jul 2024 22:05:32 -0700 Subject: [PATCH 16/16] Add cancellation token to delay --- src/AzureExtension/DataManager/AzureDataManagerCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AzureExtension/DataManager/AzureDataManagerCache.cs b/src/AzureExtension/DataManager/AzureDataManagerCache.cs index 9a21d21..7c5ee99 100644 --- a/src/AzureExtension/DataManager/AzureDataManagerCache.cs +++ b/src/AzureExtension/DataManager/AzureDataManagerCache.cs @@ -122,7 +122,7 @@ public async Task UpdateDataForAccountsAsync(TimeSpan olderThan, RequestOptions? // Delay to allow widgets and other code to respond to the event and use the database. // This is to prevent DOS'ing widgets using the datastore during large cache updates of // many organizations. - await Task.Delay(_orgUpdateDelayTime); + await Task.Delay(_orgUpdateDelayTime, cancellationToken); } catch (Exception ex) when (IsCancelException(ex)) {