Skip to content

Commit

Permalink
Move Approval Request to APIRevisions
Browse files Browse the repository at this point in the history
  • Loading branch information
chidozieononiwu committed Jan 20, 2024
1 parent da7860b commit 21e3f95
Show file tree
Hide file tree
Showing 16 changed files with 89 additions and 84 deletions.
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
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
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
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);
}
}
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
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
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
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
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
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,9 +30,9 @@ 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)))
foreach (var review 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));
Expand Down
4 changes: 2 additions & 2 deletions src/dotnet/APIView/APIViewWeb/Pages/Assemblies/Review.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@
{
<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 +209,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
Original file line number Diff line number Diff line change
Expand Up @@ -294,16 +294,17 @@ public async Task<IActionResult> OnPostToggleAPIRevisionApprovalAsync(string id,
}

/// <summary>
/// Request Reviewers for a Review Revision
/// Request Reviewers for aa APIRevision
/// </summary>
/// <param name="id"></param>
/// <param name="reviewId"></param>
/// <param name="reviewers"></param>
/// <returns></returns>
public async Task<ActionResult> OnPostRequestReviewersAsync(string id, HashSet<string> reviewers)
public async Task<ActionResult> OnPostRequestReviewersAsync(string id, string reviewId, HashSet<string> reviewers)
{
await _reviewManager.AssignReviewersToReviewAsync(User, id, reviewers);
await _apiRevisionsManager.AssignReviewersToAPIRevisionAsync(User, id, reviewers);
await _notificationManager.NotifyApproversOfReview(User, id, reviewers);
return RedirectToPage(new { id = id });
return RedirectToPage(new { id = reviewId, revisionId = id });
}
/// <summary>
/// Get Routing Data for a Review
Expand Down
Loading

0 comments on commit 21e3f95

Please sign in to comment.