Skip to content

Commit

Permalink
Add Windows toast notifications for new reviews for the logged in dev…
Browse files Browse the repository at this point in the history
…eloper's pull requests. (#337)

* Add review to datastore and notifications
  • Loading branch information
dkbennett authored Feb 5, 2024
1 parent b3579fd commit 73156d2
Show file tree
Hide file tree
Showing 10 changed files with 532 additions and 9 deletions.
59 changes: 55 additions & 4 deletions src/GitHubExtension/DataManager/GitHubDataManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public partial class GitHubDataManager : IGitHubDataManager, IDisposable
private static readonly TimeSpan NotificationRetentionTime = TimeSpan.FromDays(7);
private static readonly TimeSpan SearchRetentionTime = TimeSpan.FromDays(7);
private static readonly TimeSpan PullRequestStaleTime = TimeSpan.FromDays(1);
private static readonly TimeSpan ReviewStaleTime = TimeSpan.FromDays(7);

// It is possible different widgets have queries which touch the same pull requests.
// We want to keep this window large enough that we don't delete data being used by
Expand Down Expand Up @@ -438,6 +439,27 @@ private async Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(DataStoreOpera
CommitCombinedStatus.GetOrCreate(DataStore, commitCombinedStatus);

CreatePullRequestStatus(dsPullRequest);

// Review information for this pull request.
// We will only get review data for the logged-in Developer's pull requests.
try
{
var octoReviews = await devId.GitHubClient.PullRequest.Review.GetAll(repoName[0], repoName[1], octoPull.Number);
foreach (var octoReview in octoReviews)
{
ProcessReview(dsPullRequest, octoReview);
}
}
catch (Exception e)
{
// Octokit can sometimes fail unexpectedly or have bugs. Should that occur here, we
// will not stop processing all pull requests and instead skip over getting the PR
// review information for this particular pull request.
Log.Logger()?.ReportError($"Error updating Reviews for Pull Request #{octoPull.Number}: {e.Message}");

// Put the full stack trace in debug if this occurs to reduce log spam.
Log.Logger()?.ReportDebug($"Error updating Reviews for Pull Request #{octoPull.Number}.", e);
}
}

Log.Logger()?.ReportDebug(Name, $"Updated developer pull requests for {repoFullName}.");
Expand Down Expand Up @@ -514,6 +536,36 @@ private async Task UpdatePullRequestsAsync(Repository repository, Octokit.GitHub
PullRequest.DeleteLastObservedBefore(DataStore, repository.Id, DateTime.UtcNow - LastObservedDeleteSpan);
}

private void ProcessReview(PullRequest pullRequest, Octokit.PullRequestReview octoReview)
{
// Skip reviews that are stale.
if ((DateTime.Now - octoReview.SubmittedAt) > ReviewStaleTime)
{
return;
}

// For creating review notifications, must first determine if the review has changed.
var existingReview = Review.GetByInternalId(DataStore, octoReview.Id);

// Add/update the review record.
var newReview = Review.GetOrCreateByOctokitReview(DataStore, octoReview, pullRequest.Id);

// Ignore comments or pending state.
if (string.IsNullOrEmpty(newReview.State) || newReview.State == "Commented")
{
Log.Logger()?.ReportDebug(Name, "Notifications", $"Ignoring review for {pullRequest}. State: {newReview.State}");
return;
}

// Create a new notification if the state is different or the review did not exist.
if (existingReview == null || (existingReview.State != newReview.State))
{
// We assume that the logged in developer created this pull request.
Log.Logger()?.ReportInfo(Name, "Notifications", $"Creating NewReview Notification for {pullRequest}. State: {newReview.State}");
Notification.Create(DataStore, newReview, NotificationType.NewReview);
}
}

private void CreatePullRequestStatus(PullRequest pullRequest)
{
// Get the previous status for comparison.
Expand All @@ -525,15 +577,13 @@ private void CreatePullRequestStatus(PullRequest pullRequest)
if (ShouldCreateCheckFailureNotification(curStatus, prevStatus))
{
Log.Logger()?.ReportInfo(Name, "Notifications", $"Creating CheckRunFailure Notification for {curStatus}");
var notification = Notification.Create(curStatus, NotificationType.CheckRunFailed);
Notification.Add(DataStore, notification);
Notification.Create(DataStore, curStatus, NotificationType.CheckRunFailed);
}

if (ShouldCreateCheckSucceededNotification(curStatus, prevStatus))
{
Log.Logger()?.ReportDebug(Name, "Notifications", $"Creating CheckRunSuccess Notification for {curStatus}");
var notification = Notification.Create(curStatus, NotificationType.CheckRunSucceeded);
Notification.Add(DataStore, notification);
Notification.Create(DataStore, curStatus, NotificationType.CheckRunSucceeded);
}
}

Expand Down Expand Up @@ -662,6 +712,7 @@ private void PruneObsoleteData()
Notification.DeleteBefore(DataStore, DateTime.Now - NotificationRetentionTime);
Search.DeleteBefore(DataStore, DateTime.Now - SearchRetentionTime);
SearchIssue.DeleteUnreferenced(DataStore);
Review.DeleteUnreferenced(DataStore);
}

// Sets a last-updated in the MetaData.
Expand Down
6 changes: 6 additions & 0 deletions src/GitHubExtension/DataManager/GitHubDataManagerUpdate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ private static async Task UpdateDeveloperPullRequests()
{
notification.ShowToast();
}

// Show notifications for new reviews.
if (notification.Type == NotificationType.NewReview)
{
notification.ShowToast();
}
}
}

Expand Down
99 changes: 97 additions & 2 deletions src/GitHubExtension/DataModel/DataObjects/Notification.cs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ public bool ShowToast()
{
NotificationType.CheckRunFailed => ShowFailedCheckRunToast(),
NotificationType.CheckRunSucceeded => ShowSucceededCheckRunToast(),
NotificationType.NewReview => ShowNewReviewToast(),
_ => false,
};
}
Expand Down Expand Up @@ -225,9 +226,52 @@ private bool ShowSucceededCheckRunToast()
return true;
}

public static Notification Create(PullRequestStatus status, NotificationType type)
private bool ShowNewReviewToast()
{
return new Notification
try
{
Notifications.Log.Logger()?.ReportInfo($"Showing Notification for {this}");
var resLoader = new ResourceLoader(ResourceLoader.GetDefaultResourceFilePath(), "GitHubExtension/Resources");
var nb = new AppNotificationBuilder();
nb.SetDuration(AppNotificationDuration.Long);
nb.AddArgument("htmlurl", HtmlUrl);

switch (Result)
{
case "Approved":
nb.AddText($"✅ {resLoader.GetString("Notifications_Toast_NewReview/Approved")}");
break;

case "ChangesRequested":
nb.AddText($"⚠️ {resLoader.GetString("Notifications_Toast_NewReview/ChangesRequested")}");
break;

default:
throw new ArgumentException($"Unknown Review Result: {Result}");
}

nb.AddText($"#{Identifier} - {Repository.FullName}", new AppNotificationTextProperties().SetMaxLines(1));

// We want to show Author login but the AppNotification has a max 3 AddText calls, see:
// https://learn.microsoft.com/windows/windows-app-sdk/api/winrt/microsoft.windows.appnotifications.builder.appnotificationbuilder.addtext?view=windows-app-sdk-1.2
// The newline is a workaround to the 3 line restriction to show the Author line.
nb.AddText(Title + Environment.NewLine + "@" + User.Login);
nb.AddButton(new AppNotificationButton(resLoader.GetString("Notifications_Toast_Button/Dismiss")).AddArgument("action", "dismiss"));
AppNotificationManager.Default.Show(nb.BuildNotification());
Toasted = true;
}
catch (Exception ex)
{
Notifications.Log.Logger()?.ReportError($"Failed creating the Notification for {this}", ex);
return false;
}

return true;
}

public static Notification Create(DataStore dataStore, PullRequestStatus status, NotificationType type)
{
var pullRequestNotification = new Notification
{
TypeId = (long)type,
UserId = status.PullRequest.AuthorId,
Expand All @@ -242,6 +286,33 @@ public static Notification Create(PullRequestStatus status, NotificationType typ
TimeOccurred = status.TimeOccurred,
TimeCreated = DateTime.Now.ToDataStoreInteger(),
};

Add(dataStore, pullRequestNotification);
SetOlderNotificationsToasted(dataStore, pullRequestNotification);
return pullRequestNotification;
}

public static Notification Create(DataStore dataStore, Review review, NotificationType type)
{
var reviewNotification = new Notification
{
TypeId = (long)type,
UserId = review.AuthorId,
RepositoryId = review.PullRequest.RepositoryId,
Title = review.PullRequest.Title,
Description = review.Body,
Identifier = review.PullRequest.Number.ToStringInvariant(),
Result = review.State,
HtmlUrl = review.HtmlUrl,
DetailsUrl = review.HtmlUrl,
ToastState = 0,
TimeOccurred = review.TimeSubmitted,
TimeCreated = DateTime.Now.ToDataStoreInteger(),
};

Add(dataStore, reviewNotification);
SetOlderNotificationsToasted(dataStore, reviewNotification);
return reviewNotification;
}

public static Notification Add(DataStore dataStore, Notification notification)
Expand Down Expand Up @@ -272,6 +343,30 @@ public static IEnumerable<Notification> Get(DataStore dataStore, DateTime? since
return notifications;
}

public static void SetOlderNotificationsToasted(DataStore dataStore, Notification notification)
{
// Get all untoasted notifications for the same type, identifier, and author that are older
// than the specified notification.
var sql = @"SELECT * FROM Notification WHERE TypeId = @TypeId AND RepositoryId = @RepositoryId AND Identifier = @Identifier AND UserId = @UserId AND TimeOccurred < @TimeOccurred AND ToastState = 0";
var param = new
{
notification.TypeId,
notification.RepositoryId,
notification.Identifier,
notification.UserId,
notification.TimeOccurred,
};

Log.Logger()?.ReportDebug(DataStore.GetSqlLogMessage(sql, param));
var outDatedNotifications = dataStore.Connection!.Query<Notification>(sql, param, null) ?? Enumerable.Empty<Notification>();
foreach (var olderNotification in outDatedNotifications)
{
olderNotification.DataStore = dataStore;
olderNotification.Toasted = true;
Notifications.Log.Logger()?.ReportInfo($"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.
Expand Down
20 changes: 20 additions & 0 deletions src/GitHubExtension/DataModel/DataObjects/PullRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,26 @@ public PullRequestStatus? PullRequestStatus
}
}

/// <summary>
/// Gets all reviews associated with this pull request.
/// </summary>
[Write(false)]
[Computed]
public IEnumerable<Review> Reviews
{
get
{
if (DataStore == null)
{
return Enumerable.Empty<Review>();
}
else
{
return Review.GetAllForPullRequest(DataStore, this) ?? Enumerable.Empty<Review>();
}
}
}

public override string ToString() => $"{Number}: {Title}";

// Create pull request from OctoKit pull request data
Expand Down
Loading

0 comments on commit 73156d2

Please sign in to comment.