Skip to content

Commit

Permalink
Disable delete option for PR API reviews (#5663)
Browse files Browse the repository at this point in the history
* Disable delete option for PR API reviews
  • Loading branch information
praveenkuttappan authored Mar 13, 2023
1 parent 4a95b74 commit 69d6c5f
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 4 deletions.
19 changes: 19 additions & 0 deletions src/dotnet/APIView/APIViewIntegrationTests/ReviewManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using Xunit;
using System.IO;
using System;
using APIViewWeb;
using APIViewWeb.Repositories;

namespace APIViewIntegrationTests
{
Expand Down Expand Up @@ -71,5 +73,22 @@ public async Task AddRevisionAsync_Computes_Headings_Of_Sections_With_Diff_B()
Assert.All(headingWithDiffInSections,
item => Assert.Contains(item, new int[] { 20, 275 }));
}

[Fact]
public async Task Delete_PullRequest_Review_Throws_Exception()
{
var reviewManager = testsBaseFixture.ReviewManager;
var user = testsBaseFixture.User;
var review = await reviewManager.CreateReviewAsync(user, fileNameC, "Azure.Analytics.Purview.Account", fileStreamC, false, "Swagger", false);
Assert.Equal(ReviewType.Manual, review.FilterType);

review.FilterType = ReviewType.PullRequest;

// Change review type to PullRequest
await testsBaseFixture.ReviewRepository.UpsertReviewAsync(review);
Assert.Equal(ReviewType.PullRequest, review.FilterType);

await Assert.ThrowsAsync<UnDeletableReviewException>(async () => await reviewManager.DeleteRevisionAsync(user, review.ReviewId, review.Revisions[0].RevisionId));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public class TestsBaseFixture : IDisposable

public PackageNameManager PackageNameManager { get; private set; }
public ReviewManager ReviewManager { get; private set; }
public CosmosReviewRepository ReviewRepository { get; private set; }
public ClaimsPrincipal User { get; private set; }

public TestsBaseFixture()
Expand Down Expand Up @@ -63,7 +64,7 @@ public TestsBaseFixture()
_ = dataBaseResponse.Database.CreateContainerIfNotExistsAsync("Reviews", "/id");
_ = dataBaseResponse.Database.CreateContainerIfNotExistsAsync("Comments", "/ReviewId");
_ = dataBaseResponse.Database.CreateContainerIfNotExistsAsync("Profiles", "/id");
var cosmosReviewRepository = new CosmosReviewRepository(config);
ReviewRepository = new CosmosReviewRepository(config);
var cosmosCommentsRepository = new CosmosCommentsRepository(config);
var cosmosUserProfileRepository = new CosmosUserProfileRepository(config);

Expand All @@ -79,7 +80,7 @@ public TestsBaseFixture()
authorizationServiceMoq.Setup(_ => _.AuthorizeAsync(It.IsAny<ClaimsPrincipal>(), It.IsAny<Object>(), It.IsAny<IEnumerable<IAuthorizationRequirement>>()))
.ReturnsAsync(AuthorizationResult.Success);

var notificationManager = new NotificationManager(config, cosmosReviewRepository, cosmosUserProfileRepository);
var notificationManager = new NotificationManager(config, ReviewRepository, cosmosUserProfileRepository);

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 All @@ -89,7 +90,7 @@ public TestsBaseFixture()
.Returns(Task.CompletedTask);

ReviewManager = new ReviewManager(
authorizationServiceMoq.Object, cosmosReviewRepository, blobCodeFileRepository, blobOriginalsRepository, cosmosCommentsRepository,
authorizationServiceMoq.Object, ReviewRepository, blobCodeFileRepository, blobOriginalsRepository, cosmosCommentsRepository,
languageService, notificationManager, devopsArtifactRepositoryMoq.Object, PackageNameManager);
}

Expand Down
12 changes: 12 additions & 0 deletions src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ public async Task DeleteReviewAsync(ClaimsPrincipal user, string id)
{
var reviewModel = await _reviewsRepository.GetReviewAsync(id);
await AssertReviewOwnerAsync(user, reviewModel);
AssertReviewDeletion(reviewModel);

await _reviewsRepository.DeleteReviewAsync(reviewModel);

Expand Down Expand Up @@ -261,6 +262,7 @@ public async Task<ReviewCodeFileModel> CreateReviewCodeFileModel(string revision
public async Task DeleteRevisionAsync(ClaimsPrincipal user, string id, string revisionId)
{
var review = await GetReviewAsync(user, id);
AssertReviewDeletion(review);
var revision = review.Revisions.Single(r => r.RevisionId == revisionId);
await AssertRevisionOwner(user, revision);

Expand Down Expand Up @@ -819,6 +821,16 @@ private LanguageService GetLanguageService(string language)
return _languageServices.FirstOrDefault(service => service.Name == language);
}

private void AssertReviewDeletion(ReviewModel reviewModel)
{
// We allow deletion of manual API review only.
// Server side assertion to ensure we are not processing any requests to delete automatic and PR API review
if (reviewModel.FilterType != ReviewType.Manual)
{
throw new UnDeletableReviewException();
}
}

private async Task AssertReviewOwnerAsync(ClaimsPrincipal user, ReviewModel reviewModel)
{
var result = await _authorizationService.AuthorizeAsync(user, reviewModel, new[] { ReviewOwnerRequirement.Instance });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
<span>@review.FilterType.ToString()</span>
</td>
<td>
@if (review.Author == User.GetGitHubLogin())
@if (review.Author == User.GetGitHubLogin() && review.FilterType == ReviewType.Manual)
{
<a asp-page="./Delete" asp-route-id="@review.ReviewId">
<button type="button" class="btn pt-0 pb-0 ps-1 pe-1 btn-outline-danger"><i class="fas fa-times-circle"></i> Delete</button>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;

namespace APIViewWeb.Repositories
{
public class UnDeletableReviewException : Exception
{
}
}

0 comments on commit 69d6c5f

Please sign in to comment.