Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable delete option for PR API reviews #5663

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
{
}
}