Skip to content

Commit

Permalink
Move Approval Request to APIRevisions (#7557)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chidozieononiwu
chidozieononiwu authored Feb 6, 2024
1 parent 585e7c6 commit e536c7a
Show file tree
Hide file tree
Showing 16 changed files with 103 additions and 93 deletions.
2 changes: 1 addition & 1 deletion src/dotnet/APIView/APIViewIntegrationTests/TestsBaseFixture.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public TestsBaseFixture()

var telemetryClient = new Mock<TelemetryClient>();

var notificationManager = new NotificationManager(config, ReviewRepository, cosmosUserProfileRepository, telemetryClient.Object);
var notificationManager = new NotificationManager(config, ReviewRepository, APIRevisionRepository, cosmosUserProfileRepository, telemetryClient.Object);

var devopsArtifactRepositoryMoq = new Mock<IDevopsArtifactRepository>();
devopsArtifactRepositoryMoq.Setup(_ => _.DownloadPackageArtifact(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()))
Expand Down
13 changes: 7 additions & 6 deletions src/dotnet/APIView/APIViewWeb/LeanModels/ReviewListModels.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,16 @@ public class LegacyRevisionModel
public bool IsApproved { get; set; }
}

public class ReviewListItemModel
public class BaseListitemModel
{
[JsonProperty("id")]
public string Id { get; set; } = IdHelper.GenerateId();
public string PackageName { get; set; }
public string Language { get; set; }
}

public class ReviewListItemModel : BaseListitemModel
{
public HashSet<string> Subscribers { get; set; } = new HashSet<string>();
public List<ReviewChangeHistoryModel> ChangeHistory { get; set; } = new List<ReviewChangeHistoryModel>();
public List<ReviewAssignmentModel> AssignedReviewers { get; set; } = new List<ReviewAssignmentModel>();
Expand All @@ -106,19 +110,16 @@ public class ReviewListItemModel
public bool IsDeleted { get; set; }
}

public class APIRevisionListItemModel
public class APIRevisionListItemModel : BaseListitemModel
{
[JsonProperty("id")]
public string Id { get; set; } = IdHelper.GenerateId();
public string ReviewId { get; set; }
public string PackageName { get; set; }
public string Language { get; set; }
public List<APICodeFileModel> Files { get; set; } = new List<APICodeFileModel>();
public string Label { get; set; }
public List<APIRevisionChangeHistoryModel> ChangeHistory { get; set; } = new List<APIRevisionChangeHistoryModel>();
public APIRevisionType APIRevisionType { get; set; }
public int? PullRequestNo { get; set; }
public Dictionary<string, HashSet<int>> HeadingsOfSectionsWithDiff { get; set; } = new Dictionary<string, HashSet<int>>();
public List<ReviewAssignmentModel> AssignedReviewers { get; set; } = new List<ReviewAssignmentModel>();
public bool IsApproved { get; set; }
public HashSet<string> Approvers { get; set; } = new HashSet<string>();
public string CreatedBy { get; set; }
Expand Down
36 changes: 36 additions & 0 deletions src/dotnet/APIView/APIViewWeb/Managers/APIRevisionsManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,42 @@ public async Task<APIRevisionListItemModel> CreateAPIRevisionAsync(string userNa
return apiRevision;
}

/// <summary>
/// Assign reviewers to a review
/// </summary>
/// <param name="User"></param>
/// <param name="apiRevisionId"></param>
/// <param name="reviewers"></param>
/// <returns></returns>
public async Task AssignReviewersToAPIRevisionAsync(ClaimsPrincipal User, string apiRevisionId, HashSet<string> reviewers)
{
APIRevisionListItemModel apiRevision = await _apiRevisionsRepository.GetAPIRevisionAsync(apiRevisionId);
foreach (var reviewer in reviewers)
{
if (!apiRevision.AssignedReviewers.Where(x => x.AssingedTo == reviewer).Any())
{
var reviewAssignment = new ReviewAssignmentModel()
{
AssingedTo = reviewer,
AssignedBy = User.GetGitHubLogin(),
AssingedOn = DateTime.Now,
};
apiRevision.AssignedReviewers.Add(reviewAssignment);
}
}
await _apiRevisionsRepository.UpsertAPIRevisionAsync(apiRevision);
}

/// <summary>
/// Get Reviews that have been assigned for review to a user
/// </summary>
/// <param name="userName"></param>
/// <returns></returns>
public async Task<IEnumerable<APIRevisionListItemModel>> GetAPIRevisionsAssignedToUser(string userName)
{
return await _apiRevisionsRepository.GetAPIRevisionsAssignedToUser(userName);
}

/// <summary>
/// Generate the Revision on a DevOps Pipeline
/// </summary>
Expand Down
2 changes: 2 additions & 0 deletions src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IAPIRevisionsManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,7 @@ public Task<APIRevisionListItemModel> CreateAPIRevisionAsync(string userName, st
public Task UpdateAPIRevisionAsync(APIRevisionListItemModel revision, LanguageService languageService);
public Task UpdateAPIRevisionAsync(APIRevisionListItemModel revision);
public Task AutoArchiveAPIRevisions(int archiveAfterMonths);
public Task AssignReviewersToAPIRevisionAsync(ClaimsPrincipal User, string apiRevisionId, HashSet<string> reviewers);
public Task<IEnumerable<APIRevisionListItemModel>> GetAPIRevisionsAssignedToUser(string userName);
}
}
2 changes: 1 addition & 1 deletion src/dotnet/APIView/APIViewWeb/Managers/Interfaces/INotificationManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public interface INotificationManager
{
public Task NotifySubscribersOnComment(ClaimsPrincipal user, CommentItemModel comment);
public Task NotifyUserOnCommentTag(CommentItemModel comment);
public Task NotifyApproversOfReview(ClaimsPrincipal user, string reviewId, HashSet<string> reviewers);
public Task NotifyApproversOfReview(ClaimsPrincipal user, string apiRevisionId, HashSet<string> reviewers);
public Task NotifySubscribersOnNewRevisionAsync(ReviewListItemModel review, APIRevisionListItemModel revision, ClaimsPrincipal user);
public Task ToggleSubscribedAsync(ClaimsPrincipal user, string reviewId);
public Task SubscribeAsync(ReviewListItemModel review, ClaimsPrincipal user);
Expand Down
2 changes: 0 additions & 2 deletions src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ public interface IReviewManager
{
public Task<IEnumerable<ReviewListItemModel>> GetReviewsAsync(string language, bool? isClosed = false);
public Task<ReviewListItemModel> GetReviewAsync(string language, string packageName, bool? isClosed = false);
public Task<IEnumerable<ReviewListItemModel>> GetReviewsAssignedToUser(string userName);
public Task<(IEnumerable<ReviewListItemModel> Reviews, int TotalCount, int TotalPages, int CurrentPage, int? PreviousPage, int? NextPage)> GetPagedReviewListAsync(
IEnumerable<string> search, IEnumerable<string> languages, bool? isClosed, bool? isApproved, int offset, int limit, string orderBy);
public Task<ReviewListItemModel> GetReviewAsync(ClaimsPrincipal user, string id);
Expand All @@ -19,7 +18,6 @@ public interface IReviewManager
public Task ToggleReviewIsClosedAsync(ClaimsPrincipal user, string id);
public Task ToggleReviewApprovalAsync(ClaimsPrincipal user, string id, string revisionId, string notes="");
public Task ApproveReviewAsync(ClaimsPrincipal user, string reviewId, string notes = "");
public Task AssignReviewersToReviewAsync(ClaimsPrincipal User, string reviewId, HashSet<string> reviewers);
public Task<int> GenerateAIReview(string reviewId, string revisionId);
public Task UpdateReviewsInBackground(HashSet<string> updateDisabledLanguages, int backgroundBatchProcessCount);
}
Expand Down
22 changes: 12 additions & 10 deletions src/dotnet/APIView/APIViewWeb/Managers/NotificationManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class NotificationManager : INotificationManager
{
private readonly string _apiviewEndpoint;
private readonly ICosmosReviewRepository _reviewRepository;
private readonly ICosmosAPIRevisionsRepository _apiRevisionRepository;
private readonly ICosmosUserProfileRepository _userProfileRepository;
private readonly string _testEmailToAddress;
private readonly string _emailSenderServiceUrl;
Expand All @@ -32,12 +33,13 @@ public class NotificationManager : INotificationManager
private const string ENDPOINT_SETTING = "Endpoint";

public NotificationManager(IConfiguration configuration,
ICosmosReviewRepository reviewRepository,
ICosmosReviewRepository reviewRepository, ICosmosAPIRevisionsRepository apiRevisionRepository,
ICosmosUserProfileRepository userProfileRepository,
TelemetryClient telemetryClient)
{
_apiviewEndpoint = configuration.GetValue<string>(ENDPOINT_SETTING);
_reviewRepository = reviewRepository;
_apiRevisionRepository = apiRevisionRepository;
_userProfileRepository = userProfileRepository;
_testEmailToAddress = configuration["apiview-email-test-address"] ?? "";
_emailSenderServiceUrl = configuration["azure-sdk-emailer-url"] ?? "";
Expand All @@ -61,15 +63,15 @@ public async Task NotifyUserOnCommentTag(CommentItemModel comment)
}
}

public async Task NotifyApproversOfReview(ClaimsPrincipal user, string reviewId, HashSet<string> reviewers)
public async Task NotifyApproversOfReview(ClaimsPrincipal user, string apiRevisionId, HashSet<string> reviewers)
{
var userProfile = await _userProfileRepository.TryGetUserProfileAsync(user.GetGitHubLogin());
var review = await _reviewRepository.GetReviewAsync(reviewId);
var apiRevision = await _apiRevisionRepository.GetAPIRevisionAsync(apiRevisionId);
foreach (var reviewer in reviewers)
{
var reviewerProfile = await _userProfileRepository.TryGetUserProfileAsync(reviewer);
await SendUserEmailsAsync(review, reviewerProfile,
GetApproverReviewHtmlContent(userProfile, review));
await SendUserEmailsAsync(apiRevision, reviewerProfile,
GetApproverReviewHtmlContent(userProfile, apiRevision));
}
}

Expand Down Expand Up @@ -135,10 +137,10 @@ public async Task UnsubscribeAsync(ReviewListItemModel review, ClaimsPrincipal u
public static string GetUserEmail(ClaimsPrincipal user) =>
user.FindFirstValue(ClaimConstants.Email);

private string GetApproverReviewHtmlContent(UserProfileModel user, ReviewListItemModel review)
private string GetApproverReviewHtmlContent<T>(UserProfileModel user, T model) where T : BaseListitemModel
{
var reviewName = review.PackageName;
var reviewLink = new Uri($"{_apiviewEndpoint}/Assemblies/Review/{review.Id}");
var reviewName = model.PackageName;
var reviewLink = new Uri($"{_apiviewEndpoint}/Assemblies/Review/{model.Id}");
var poster = user.UserName;
var userLink = new Uri($"{_apiviewEndpoint}/Assemblies/Profile/{poster}");
var requestsLink = new Uri($"{_apiviewEndpoint}/Assemblies/RequestedReviews/");
Expand Down Expand Up @@ -183,7 +185,7 @@ private string GetHtmlContent(CommentItemModel comment, ReviewListItemModel revi
private static string GetContentHeading(CommentItemModel comment, bool includeHtml) =>
$"{(includeHtml ? $"<b>{comment.CreatedBy}</b>" : $"{comment.CreatedBy}")} commented on this review.";

private async Task SendUserEmailsAsync(ReviewListItemModel review, UserProfileModel user, string htmlContent)
private async Task SendUserEmailsAsync<T>(T model, UserProfileModel user, string htmlContent) where T : BaseListitemModel
{
// Always send email to a test address when test address is configured.
if (string.IsNullOrEmpty(user.Email))
Expand All @@ -192,7 +194,7 @@ private async Task SendUserEmailsAsync(ReviewListItemModel review, UserProfileMo
return;
}

await SendEmail(user.Email, $"Notification from APIView - {review.PackageName}", htmlContent);
await SendEmail(user.Email, $"Notification from APIView - {model.PackageName}", htmlContent);
}
private async Task SendEmailsAsync(ReviewListItemModel review, ClaimsPrincipal user, string htmlContent, ISet<string> notifiedUsers)
{
Expand Down
36 changes: 0 additions & 36 deletions src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,6 @@ public Task<ReviewListItemModel> GetReviewAsync(string language, string packageN
return _reviewsRepository.GetReviewAsync(language, packageName, isClosed);
}

/// <summary>
/// Get Reviews that have been assigned for review to a user
/// </summary>
/// <param name="userName"></param>
/// <returns></returns>
public async Task<IEnumerable<ReviewListItemModel>> GetReviewsAssignedToUser(string userName)
{
return await _reviewsRepository.GetReviewsAssignedToUser(userName);
}

/// <summary>
/// Get List of Reviews for the Review Page
/// </summary>
Expand Down Expand Up @@ -266,32 +256,6 @@ public async Task ApproveReviewAsync(ClaimsPrincipal user, string reviewId, stri
await ToggleReviewApproval(user, review, notes);
}

/// <summary>
/// Assign reviewers to a review
/// </summary>
/// <param name="User"></param>
/// <param name="reviewId"></param>
/// <param name="reviewers"></param>
/// <returns></returns>
public async Task AssignReviewersToReviewAsync(ClaimsPrincipal User, string reviewId, HashSet<string> reviewers)
{
ReviewListItemModel review = await _reviewsRepository.GetReviewAsync(reviewId);
foreach (var reviewer in reviewers)
{
if (!review.AssignedReviewers.Where(x => x.AssingedTo == reviewer).Any())
{
var reviewAssignment = new ReviewAssignmentModel()
{
AssingedTo = reviewer,
AssignedBy = User.GetGitHubLogin(),
AssingedOn = DateTime.Now,
};
review.AssignedReviewers.Add(reviewAssignment);
}
}
await _reviewsRepository.UpsertReviewAsync(review);
}

/// <summary>
/// Sends info to AI service for generating initial review on APIReview file
/// </summary>
Expand Down
4 changes: 2 additions & 2 deletions src/dotnet/APIView/APIViewWeb/Pages/Assemblies/RequestedReviews.cshtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
{
@foreach (var apiRevision in Model.ActiveAPIRevisions)
{
var assignment = Model.Reviews.Single(r => r.Id == apiRevision.ReviewId).AssignedReviewers.First(ar => ar.AssingedTo == User.GetGitHubLogin());
var assignment = apiRevision.AssignedReviewers.First(ar => ar.AssingedTo == User.GetGitHubLogin());
var approvalRequestedOn = assignment.AssingedOn;
var requestBy = assignment.AssignedBy;

Expand Down Expand Up @@ -98,7 +98,7 @@
{
@foreach (var apiRevision in Model.ApprovedAPIRevisions)
{
var assignment = Model.Reviews.Single(r => r.Id == apiRevision.ReviewId).AssignedReviewers.First(ar => ar.AssingedTo == User.GetGitHubLogin());
var assignment = apiRevision.AssignedReviewers.First(ar => ar.AssingedTo == User.GetGitHubLogin());
var approvalRequestedOn = assignment.AssingedOn;
var requestBy = assignment.AssignedBy;

Expand Down
22 changes: 15 additions & 7 deletions src/dotnet/APIView/APIViewWeb/Pages/Assemblies/RequestedReviews.cshtml.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class RequestedReviews: PageModel
private readonly IAPIRevisionsManager _apiRevisionsManager;
public readonly UserPreferenceCache _preferenceCache;

public IEnumerable<ReviewListItemModel> Reviews { get; set; } = new List<ReviewListItemModel>();
public IEnumerable<APIRevisionListItemModel> APIRevisions { get; set; } = new List<APIRevisionListItemModel>();
public IEnumerable<APIRevisionListItemModel> ActiveAPIRevisions { get; set; } = new List<APIRevisionListItemModel>();
public IEnumerable<APIRevisionListItemModel> ApprovedAPIRevisions { get; set; } = new List<APIRevisionListItemModel>();

Expand All @@ -30,16 +30,24 @@ public RequestedReviews(IReviewManager reviewManager, IAPIRevisionsManager apiRe

public async Task<IActionResult> OnGetAsync()
{
Reviews = await _reviewManager.GetReviewsAssignedToUser(User.GetGitHubLogin());
APIRevisions = await _apiRevisionsManager.GetAPIRevisionsAssignedToUser(User.GetGitHubLogin());

foreach (var review in Reviews.OrderByDescending(r => r.AssignedReviewers.Select(x => x.AssingedOn)))
List<APIRevisionListItemModel> activeAPIRevs = new List<APIRevisionListItemModel>();
List<APIRevisionListItemModel> approvedAPIRevs = new List<APIRevisionListItemModel>();
foreach (var apiRevison in APIRevisions.OrderByDescending(r => r.AssignedReviewers.Select(x => x.AssingedOn)))
{
var apiRevisoins = await _apiRevisionsManager.GetAPIRevisionsAsync(review.Id);
ActiveAPIRevisions = ActiveAPIRevisions.Concat(apiRevisoins.Where(r => r.IsApproved == false));
if (!apiRevison.IsApproved)
{
activeAPIRevs.Add(apiRevison);
}

// Remove all approvals over a week old
ApprovedAPIRevisions = ApprovedAPIRevisions.Concat(apiRevisoins.Where(r => r.IsApproved == true).Where(r => r.ChangeHistory.First(c => c.ChangeAction == APIRevisionChangeAction.Approved).ChangedOn >= DateTime.Now.AddDays(-7)));
if (apiRevison.IsApproved && apiRevison.ChangeHistory.First(c => c.ChangeAction == APIRevisionChangeAction.Approved).ChangedOn >= DateTime.Now.AddDays(-7))
{
approvedAPIRevs.Add(apiRevison);
}
}
ActiveAPIRevisions = activeAPIRevs;
ApprovedAPIRevisions = approvedAPIRevs;
ApprovedAPIRevisions.OrderByDescending(r => r.ChangeHistory.First(c => c.ChangeAction == APIRevisionChangeAction.Approved).ChangedOn);

return Page();
Expand Down
5 changes: 3 additions & 2 deletions src/dotnet/APIView/APIViewWeb/Pages/Assemblies/Review.cshtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,14 @@
var anyChecked = false;
}
<form asp-resource="@Model.ReviewContent.ActiveAPIRevision" asp-page-handler="RequestReviewers" method="post">
<input type="hidden" name="apiRevisionId" value="@Model.ReviewContent.ActiveAPIRevision.Id" />
<li class="list-group-item">
<ul class="list-group list-group-flush">
@foreach (var approver in Model.ReviewContent.PreferredApprovers)
{
<li class="list-group-item">
<div class="form-check">
@if (Model.ReviewContent.Review.AssignedReviewers != null && Model.ReviewContent.Review.AssignedReviewers.Where(a => a.AssingedTo == approver).Any())
@if (Model.ReviewContent.ActiveAPIRevision.AssignedReviewers != null && Model.ReviewContent.ActiveAPIRevision.AssignedReviewers.Where(a => a.AssingedTo == approver).Any())
{
<input class="form-check-input selectReviewerForRequest" type="checkbox" name="reviewers" value="@approver" id="@(approver)CheckBox" checked>
<label class="form-check-label" for="@(approver)CheckBox">
Expand All @@ -209,7 +210,7 @@
}
else
{
<input class="form-check-input selectReviewerForRequest" type="checkbox" name="reviewers" value="@approver" id="@(approver)CheckBox">
<input class="form-check-input selectReviewerForRequest" type="checkbox" name="reviewers" value="@approver" id="@(approver)CheckBox">
<label class="form-check-label" for="@(approver)CheckBox">
<img username="@approver" size="28" width="28" class="comment-icon align-self-start mr-2" />
@approver
Expand Down
Loading

0 comments on commit e536c7a

Please sign in to comment.