From 69e3a02d42142a3e3f92d25db7270b2b18f74a21 Mon Sep 17 00:00:00 2001 From: helen229 Date: Tue, 7 Oct 2025 12:17:36 -0700 Subject: [PATCH 01/19] update controller --- .../Controllers/AutoReviewController.cs | 30 +++++++++++++++---- .../APIViewWeb/Helpers/CommonUtilities.cs | 26 +++++++++++++++- .../LeanControllers/PullRequestsController.cs | 5 ++-- .../LeanControllers/ReviewsController.cs | 2 ++ .../APIViewWeb/LeanModels/ReviewListModels.cs | 1 + .../Interfaces/IPullRequestManager.cs | 2 +- .../Managers/Interfaces/IReviewManager.cs | 3 +- .../APIViewWeb/Managers/PullRequestManager.cs | 15 +++++++--- .../APIViewWeb/Managers/ReviewManager.cs | 20 ++++++++++++- .../review-page/review-page.component.ts | 19 ++++++++---- .../ClientSPA/src/app/_models/review.ts | 7 +++++ 11 files changed, 108 insertions(+), 22 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs index f7c68cf3fe0..b009b321f75 100644 --- a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs +++ b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs @@ -44,7 +44,7 @@ public AutoReviewController(IAuthorizationService authorizationService, ICodeFil // regular CI pipeline will not send this flag in request [TypeFilter(typeof(ApiKeyAuthorizeAsyncFilter))] [HttpPost] - public async Task UploadAutoReview([FromForm] IFormFile file, string label, bool compareAllRevisions = false, string packageVersion = null, bool setReleaseTag = false) + public async Task UploadAutoReview([FromForm] IFormFile file, string label, bool compareAllRevisions = false, string packageVersion = null, bool setReleaseTag = false, string packageType = null) { if (file != null) { @@ -54,7 +54,7 @@ public async Task UploadAutoReview([FromForm] IFormFile file, stri var codeFile = await _codeFileManager.CreateCodeFileAsync(originalName: file.FileName, fileStream: openReadStream, runAnalysis: false, memoryStream: memoryStream); - (var review, var apiRevision) = await CreateAutomaticRevisionAsync(codeFile: codeFile, label: label, originalName: file.FileName, memoryStream: memoryStream, compareAllRevisions); + (var review, var apiRevision) = await CreateAutomaticRevisionAsync(codeFile: codeFile, label: label, originalName: file.FileName, memoryStream: memoryStream, compareAllRevisions, packageType); if (apiRevision != null) { apiRevision = await _apiRevisionsManager.UpdateRevisionMetadataAsync(apiRevision, packageVersion ?? codeFile.PackageVersion, label, setReleaseTag); @@ -142,7 +142,8 @@ public async Task CreateApiReview( bool compareAllRevisions, string project, string packageVersion = null, - bool setReleaseTag = false + bool setReleaseTag = false, + string packageType = null ) { using var memoryStream = new MemoryStream(); @@ -154,7 +155,7 @@ public async Task CreateApiReview( { return StatusCode(statusCode: StatusCodes.Status204NoContent, $"API review code file for package {packageName} is not found in DevOps pipeline artifacts."); } - (var review, var apiRevision) = await CreateAutomaticRevisionAsync(codeFile: codeFile, label: label, originalName: originalFilePath, memoryStream: memoryStream, compareAllRevisions); + (var review, var apiRevision) = await CreateAutomaticRevisionAsync(codeFile: codeFile, label: label, originalName: originalFilePath, memoryStream: memoryStream, compareAllRevisions, packageType); if (apiRevision != null) { apiRevision = await _apiRevisionsManager.UpdateRevisionMetadataAsync(apiRevision, packageVersion ?? codeFile.PackageVersion, label, setReleaseTag); @@ -177,7 +178,7 @@ public async Task CreateApiReview( return StatusCode(statusCode: StatusCodes.Status500InternalServerError); } - private async Task<(ReviewListItemModel review, APIRevisionListItemModel apiRevision)> CreateAutomaticRevisionAsync(CodeFile codeFile, string label, string originalName, MemoryStream memoryStream, bool compareAllRevisions = false) + private async Task<(ReviewListItemModel review, APIRevisionListItemModel apiRevision)> CreateAutomaticRevisionAsync(CodeFile codeFile, string label, string originalName, MemoryStream memoryStream, bool compareAllRevisions = false, string packageType = null) { var createNewRevision = true; var review = await _reviewManager.GetReviewAsync(packageName: codeFile.PackageName, language: codeFile.Language, isClosed: null); @@ -187,6 +188,16 @@ public async Task CreateApiReview( if (review != null) { + // Update package type if provided from controller parameter + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var parsedPackageType)) + { + if (review.PackageType != parsedPackageType) + { + review.PackageType = parsedPackageType; + review = await _reviewManager.UpdateReviewAsync(review); + } + } + apiRevisions = await _apiRevisionsManager.GetAPIRevisionsAsync(review.Id); if (apiRevisions.Any()) { @@ -239,7 +250,14 @@ public async Task CreateApiReview( } else { - review = await _reviewManager.CreateReviewAsync(packageName: codeFile.PackageName, language: codeFile.Language, isClosed: false); + // Parse package type if provided, otherwise pass null for automatic classification + PackageType? parsedPackageType = null; + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) + { + parsedPackageType = packageTypeEnum; + } + + review = await _reviewManager.CreateReviewAsync(packageName: codeFile.PackageName, language: codeFile.Language, isClosed: false, packageType: parsedPackageType); } if (createNewRevision) diff --git a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs index cf081fdc18e..e7d9ffb5bab 100644 --- a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs +++ b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs @@ -3,7 +3,8 @@ using System; using System.Linq; -using System.Text.RegularExpressions; +using Newtonsoft.Json; +using Newtonsoft.Json.Converters; namespace APIViewWeb.Helpers { @@ -69,6 +70,29 @@ public static bool IsSDKLanguageOrTypeSpec(string language) } } + /// + /// Represents the plane classification of a package + /// + [JsonConverter(typeof(StringEnumConverter))] + public enum PackageType + { + /// + /// Data plane package (client libraries for Azure services) + /// + Data, + + /// + /// Management plane package (resource management libraries) + /// + Management, + + /// + /// Cannot determine plane type from package name + /// + Unknown + } +} + /* /// /// Backward compatibility alias for existing code diff --git a/src/dotnet/APIView/APIViewWeb/LeanControllers/PullRequestsController.cs b/src/dotnet/APIView/APIViewWeb/LeanControllers/PullRequestsController.cs index 577e1218024..f72cadaf5f5 100644 --- a/src/dotnet/APIView/APIViewWeb/LeanControllers/PullRequestsController.cs +++ b/src/dotnet/APIView/APIViewWeb/LeanControllers/PullRequestsController.cs @@ -113,13 +113,14 @@ public async Task>> GetPullRequestRev /// /// /// + /// /// [AllowAnonymous] [HttpGet("CreateAPIRevisionIfAPIHasChanges", Name = "CreateAPIRevisionIfAPIHasChanges")] public async Task>> CreateAPIRevisionIfAPIHasChanges( string buildId, string artifactName, string filePath, string commitSha,string repoName, string packageName, int pullRequestNumber = 0, string codeFile = null, string baselineCodeFile = null, string language = null, - string project = "internal") + string project = "internal", string packageType = null) { var responseContent = new CreateAPIRevisionAPIResponse(); if (!ValidateInputParams()) @@ -135,7 +136,7 @@ public async Task>> Creat artifactName: artifactName, originalFileName: filePath, commitSha: commitSha, repoName: repoName, packageName: packageName, prNumber: pullRequestNumber, hostName: this.Request.Host.ToUriComponent(), responseContent: responseContent, codeFileName: codeFile, baselineCodeFileName: baselineCodeFile, - language: language, project: project); + language: language, project: project, packageType: packageType); responseContent.APIRevisionUrl = apiRevisionUrl; diff --git a/src/dotnet/APIView/APIViewWeb/LeanControllers/ReviewsController.cs b/src/dotnet/APIView/APIViewWeb/LeanControllers/ReviewsController.cs index a216d7f0aa3..5cc566382cc 100644 --- a/src/dotnet/APIView/APIViewWeb/LeanControllers/ReviewsController.cs +++ b/src/dotnet/APIView/APIViewWeb/LeanControllers/ReviewsController.cs @@ -187,6 +187,8 @@ public async Task> ToggleSubscribeAsync(s return Ok(); } + + /// ///Retrieve the Content (codeLines and Navigation) of a review /// diff --git a/src/dotnet/APIView/APIViewWeb/LeanModels/ReviewListModels.cs b/src/dotnet/APIView/APIViewWeb/LeanModels/ReviewListModels.cs index 3de0a59824c..d5e4318a683 100644 --- a/src/dotnet/APIView/APIViewWeb/LeanModels/ReviewListModels.cs +++ b/src/dotnet/APIView/APIViewWeb/LeanModels/ReviewListModels.cs @@ -113,6 +113,7 @@ public class ReviewListItemModel : BaseListitemModel public List AssignedReviewers { get; set; } = new List(); public bool IsClosed { get; set; } public bool IsApproved { get; set; } // TODO: Deprecate in the future - redundant with NamespaceReviewStatus + public PackageType? PackageType { get; set; } // Nullable - null means not yet classified public NamespaceReviewStatus NamespaceReviewStatus { get; set; } = NamespaceReviewStatus.NotStarted; public string CreatedBy { get; set; } public DateTime CreatedOn { get; set; } diff --git a/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IPullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IPullRequestManager.cs index ea0e374cbea..ba62c91ca24 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IPullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IPullRequestManager.cs @@ -15,6 +15,6 @@ public interface IPullRequestManager public Task CreateAPIRevisionIfAPIHasChanges( string buildId, string artifactName, string originalFileName, string commitSha, string repoName, string packageName, int prNumber, string hostName, CreateAPIRevisionAPIResponse responseContent, - string codeFileName = null, string baselineCodeFileName = null, string language = null, string project = "internal"); + string codeFileName = null, string baselineCodeFileName = null, string language = null, string project = "internal", string packageType = null); } } diff --git a/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs index 7e1e18729c8..64e9d63c48b 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs @@ -19,7 +19,8 @@ public interface IReviewManager public Task> GetReviewsAsync(IEnumerable reviewIds, bool? isClosed = null); public Task GetLegacyReviewAsync(ClaimsPrincipal user, string id); public Task GetOrCreateReview(IFormFile file, string filePath, string language, bool runAnalysis = false); - public Task CreateReviewAsync(string packageName, string language, bool isClosed = true); + public Task CreateReviewAsync(string packageName, string language, bool isClosed = true, PackageType? packageType = null); + public Task UpdateReviewAsync(ReviewListItemModel review); public Task SoftDeleteReviewAsync(ClaimsPrincipal user, string id); public Task ToggleReviewIsClosedAsync(ClaimsPrincipal user, string id); public Task ToggleReviewApprovalAsync(ClaimsPrincipal user, string id, string revisionId, string notes=""); diff --git a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs index e9a55cc9bf0..ef85e78e211 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs @@ -134,7 +134,7 @@ public async Task CreateAPIRevisionIfAPIHasChanges( string buildId, string artifactName, string originalFileName, string commitSha, string repoName, string packageName, int prNumber, string hostName, CreateAPIRevisionAPIResponse responseContent, string codeFileName = null, string baselineCodeFileName = null, string language = null, - string project = "internal") + string project = "internal", string packageType = null) { language = LanguageServiceHelpers.MapLanguageAlias(language: language); originalFileName = originalFileName ?? codeFileName; @@ -182,7 +182,7 @@ public async Task CreateAPIRevisionIfAPIHasChanges( if (codeFile != null) { await CreateAPIRevisionIfRequired(codeFile: codeFile, originalFileName: originalFileName, memoryStream: memoryStream, pullRequestModel: pullRequestModel, - baselineCodeFile: baseLineCodeFile, baseLineStream: baselineStream, baselineFileName: baselineCodeFileName, responseContent: responseContent); + baselineCodeFile: baseLineCodeFile, baseLineStream: baselineStream, baselineFileName: baselineCodeFileName, responseContent: responseContent, packageType: packageType); } else { @@ -248,13 +248,20 @@ private async Task ClosePullRequestAPIRevision(PullRequestModel pullRequestModel } private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string originalFileName, MemoryStream memoryStream, - PullRequestModel pullRequestModel, CodeFile baselineCodeFile, MemoryStream baseLineStream, string baselineFileName, CreateAPIRevisionAPIResponse responseContent) + PullRequestModel pullRequestModel, CodeFile baselineCodeFile, MemoryStream baseLineStream, string baselineFileName, CreateAPIRevisionAPIResponse responseContent, string packageType = null) { // fetch review for the package or create brand new review var review = await _reviewManager.GetReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName); if (review == null) { - review = await _reviewManager.CreateReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName, isClosed: false); + // Parse package type if provided, otherwise pass null for automatic classification + APIViewWeb.Helpers.PackageType? parsedPackageType = null; + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) + { + parsedPackageType = packageTypeEnum; + } + + review = await _reviewManager.CreateReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName, isClosed: false, packageType: parsedPackageType); responseContent.ActionsTaken.Add($"No existing review with packageName: '{codeFile.PackageName}' and language: '{codeFile.Language}'."); responseContent.ActionsTaken.Add($"Created a new Review with Id: '{review.Id}'."); } diff --git a/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs index dc1bcfff8a4..252b8840b9f 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs @@ -238,8 +238,9 @@ public async Task GetOrCreateReview(IFormFile file, string /// /// /// + /// Optional package type. If not provided, will be automatically classified. /// - public async Task CreateReviewAsync(string packageName, string language, bool isClosed=true) + public async Task CreateReviewAsync(string packageName, string language, bool isClosed = true, PackageType? packageType = null) { if (string.IsNullOrEmpty(packageName) || string.IsNullOrEmpty(language)) { @@ -250,6 +251,7 @@ public async Task CreateReviewAsync(string packageName, str { PackageName = packageName, Language = language, + PackageType = packageType, CreatedOn = DateTime.UtcNow, CreatedBy = ApiViewConstants.AzureSdkBotName, IsClosed = isClosed, @@ -268,6 +270,22 @@ public async Task CreateReviewAsync(string packageName, str return review; } + /// + /// Update an existing review + /// + /// The review to update + /// + public async Task UpdateReviewAsync(ReviewListItemModel review) + { + if (review == null) + { + throw new ArgumentNullException(nameof(review)); + } + + await _reviewsRepository.UpsertReviewAsync(review); + return review; + } + /// /// SoftDeleteReviewAsync /// diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts b/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts index 5c5cc8e53f5..b6c1a91e567 100644 --- a/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts +++ b/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts @@ -5,7 +5,7 @@ import { MenuItem, TreeNode } from 'primeng/api'; import { concatMap, EMPTY, from, Observable, Subject, take, takeUntil, tap } from 'rxjs'; import { CodeLineRowNavigationDirection, getLanguageCssSafeName } from 'src/app/_helpers/common-helpers'; import { getQueryParams } from 'src/app/_helpers/router-helpers'; -import { Review } from 'src/app/_models/review'; +import { Review, PackageType } from 'src/app/_models/review'; import { APIRevision, APIRevisionGroupedByLanguage, ApiTreeBuilderData } from 'src/app/_models/revision'; import { ReviewsService } from 'src/app/_services/reviews/reviews.service'; import { APIRevisionsService } from 'src/app/_services/revisions/revisions.service'; @@ -277,13 +277,21 @@ export class ReviewPageComponent implements OnInit { this.reviewsService.getReview(reviewId) .pipe(takeUntil(this.destroy$)).subscribe({ next: (review: Review) => { - this.review = review; + this.updateReview(review); this.updateLoadingStateBasedOnReviewDeletionStatus(); this.updatePageTitle(); + }, + error: (error) => { + this.loadFailed = true; + this.loadFailedMessage = "Failed to load review. Please refresh the page or try again later."; } }); } + updateReview(review: Review) { + this.review = review; + } + loadPreferredApprovers(reviewId: string) { this.reviewsService.getPreferredApprovers(reviewId) .pipe(takeUntil(this.destroy$)).subscribe({ @@ -371,7 +379,6 @@ export class ReviewPageComponent implements OnInit { private processEmbeddedComments() { if (!this.codePanelData || !this.comments) return; - Object.values(this.codePanelData.nodeMetaData).forEach(nodeData => { if (nodeData.commentThread) { Object.values(nodeData.commentThread).forEach(commentThreadRow => { @@ -569,7 +576,7 @@ export class ReviewPageComponent implements OnInit { if (value) { this.reviewsService.toggleReviewApproval(this.reviewId!, this.activeApiRevisionId!, true).pipe(take(1)).subscribe({ next: (review: Review) => { - this.review = review; + this.updateReview(review); } }); } @@ -579,7 +586,7 @@ export class ReviewPageComponent implements OnInit { if (value) { this.reviewsService.requestNamespaceReview(this.reviewId!, this.activeApiRevisionId!).pipe(take(1)).subscribe({ next: (review: Review) => { - this.review = review; + this.updateReview(review); // Reset loading state in the options component on success if (this.reviewPageOptionsComponent) { this.reviewPageOptionsComponent.resetNamespaceReviewLoadingState(); @@ -671,7 +678,7 @@ export class ReviewPageComponent implements OnInit { this.signalRService.onReviewUpdates().pipe(takeUntil(this.destroy$)).subscribe({ next: (updatedReview: Review) => { if (updatedReview.id === this.reviewId) { - this.review = updatedReview; + this.updateReview(updatedReview); } } }); diff --git a/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts b/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts index 6174bd18ba7..5cd91be8783 100644 --- a/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts +++ b/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts @@ -6,6 +6,12 @@ export enum FirstReleaseApproval { All } +export enum PackageType { + Data = 'Data', + Management = 'Management', + Unknown = 'Unknown' +} + export class Review { id: string packageName: string @@ -16,6 +22,7 @@ export class Review { changeHistory: ChangeHistory[] subscribers: string[] namespaceReviewStatus: string + packageType?: PackageType | null // Optional - undefined or null if not yet classified constructor() { this.id = '' From d9d84b1ac5a99acbdb76b8accd9190508ba2679c Mon Sep 17 00:00:00 2001 From: Helen Gao Date: Tue, 7 Oct 2025 13:34:22 -0700 Subject: [PATCH 02/19] Update src/dotnet/APIView/APIViewWeb/LeanControllers/ReviewsController.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../APIView/APIViewWeb/LeanControllers/ReviewsController.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/LeanControllers/ReviewsController.cs b/src/dotnet/APIView/APIViewWeb/LeanControllers/ReviewsController.cs index 5cc566382cc..a216d7f0aa3 100644 --- a/src/dotnet/APIView/APIViewWeb/LeanControllers/ReviewsController.cs +++ b/src/dotnet/APIView/APIViewWeb/LeanControllers/ReviewsController.cs @@ -187,8 +187,6 @@ public async Task> ToggleSubscribeAsync(s return Ok(); } - - /// ///Retrieve the Content (codeLines and Navigation) of a review /// From 5898f7ef8cc1032d63742252f3041a3aaed3a3ba Mon Sep 17 00:00:00 2001 From: helen229 Date: Tue, 7 Oct 2025 14:32:01 -0700 Subject: [PATCH 03/19] fix comments --- .../APIViewWeb/Controllers/AutoReviewController.cs | 8 +++++++- .../APIView/APIViewWeb/Helpers/CommonUtilities.cs | 5 ----- .../APIView/APIViewWeb/Managers/PullRequestManager.cs | 10 +++++----- .../_components/review-page/review-page.component.ts | 6 +----- src/dotnet/APIView/ClientSPA/src/app/_models/review.ts | 3 +-- 5 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs index b009b321f75..7b1ae696e89 100644 --- a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs +++ b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs @@ -191,11 +191,17 @@ public async Task CreateApiReview( // Update package type if provided from controller parameter if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var parsedPackageType)) { - if (review.PackageType != parsedPackageType) + if (!review.PackageType.HasValue) { + // If the current review has no packageType and packageType is provided, update it review.PackageType = parsedPackageType; review = await _reviewManager.UpdateReviewAsync(review); } + else if (review.PackageType.Value != parsedPackageType) + { + // If review has a packageType that doesn't match the provided packageType + throw new InvalidOperationException($"Package type conflict: Review has PackageType '{review.PackageType.Value}' but supplied PackageType is '{parsedPackageType}'. Package types cannot be changed once set."); + } } apiRevisions = await _apiRevisionsManager.GetAPIRevisionsAsync(review.Id); diff --git a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs index e7d9ffb5bab..99dfbf83871 100644 --- a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs +++ b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs @@ -85,11 +85,6 @@ public enum PackageType /// Management plane package (resource management libraries) /// Management, - - /// - /// Cannot determine plane type from package name - /// - Unknown } } diff --git a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs index ef85e78e211..f5d5909b890 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs @@ -255,13 +255,13 @@ private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string origina if (review == null) { // Parse package type if provided, otherwise pass null for automatic classification - APIViewWeb.Helpers.PackageType? parsedPackageType = null; - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) + Helpers.PackageType? validPackageType = null; + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) { - parsedPackageType = packageTypeEnum; + validPackageType = packageTypeEnum; } - - review = await _reviewManager.CreateReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName, isClosed: false, packageType: parsedPackageType); + + review = await _reviewManager.CreateReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName, isClosed: false, packageType: validPackageType); responseContent.ActionsTaken.Add($"No existing review with packageName: '{codeFile.PackageName}' and language: '{codeFile.Language}'."); responseContent.ActionsTaken.Add($"Created a new Review with Id: '{review.Id}'."); } diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts b/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts index b6c1a91e567..459c32f8533 100644 --- a/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts +++ b/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts @@ -277,7 +277,7 @@ export class ReviewPageComponent implements OnInit { this.reviewsService.getReview(reviewId) .pipe(takeUntil(this.destroy$)).subscribe({ next: (review: Review) => { - this.updateReview(review); + this.review = review; this.updateLoadingStateBasedOnReviewDeletionStatus(); this.updatePageTitle(); }, @@ -288,10 +288,6 @@ export class ReviewPageComponent implements OnInit { }); } - updateReview(review: Review) { - this.review = review; - } - loadPreferredApprovers(reviewId: string) { this.reviewsService.getPreferredApprovers(reviewId) .pipe(takeUntil(this.destroy$)).subscribe({ diff --git a/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts b/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts index 5cd91be8783..6185bd804ae 100644 --- a/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts +++ b/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts @@ -8,8 +8,7 @@ export enum FirstReleaseApproval { export enum PackageType { Data = 'Data', - Management = 'Management', - Unknown = 'Unknown' + Management = 'Management' } export class Review { From 1f04c538ef749478685f4e385e2c0d1ec470f94b Mon Sep 17 00:00:00 2001 From: helen229 Date: Tue, 7 Oct 2025 14:42:32 -0700 Subject: [PATCH 04/19] comment fix --- .../Managers/Interfaces/IReviewManager.cs | 1 - .../APIView/APIViewWeb/Managers/ReviewManager.cs | 16 ---------------- 2 files changed, 17 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs index 64e9d63c48b..a95921870a5 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs @@ -20,7 +20,6 @@ public interface IReviewManager public Task GetLegacyReviewAsync(ClaimsPrincipal user, string id); public Task GetOrCreateReview(IFormFile file, string filePath, string language, bool runAnalysis = false); public Task CreateReviewAsync(string packageName, string language, bool isClosed = true, PackageType? packageType = null); - public Task UpdateReviewAsync(ReviewListItemModel review); public Task SoftDeleteReviewAsync(ClaimsPrincipal user, string id); public Task ToggleReviewIsClosedAsync(ClaimsPrincipal user, string id); public Task ToggleReviewApprovalAsync(ClaimsPrincipal user, string id, string revisionId, string notes=""); diff --git a/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs index 252b8840b9f..c88ef8049b5 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs @@ -270,22 +270,6 @@ public async Task CreateReviewAsync(string packageName, str return review; } - /// - /// Update an existing review - /// - /// The review to update - /// - public async Task UpdateReviewAsync(ReviewListItemModel review) - { - if (review == null) - { - throw new ArgumentNullException(nameof(review)); - } - - await _reviewsRepository.UpsertReviewAsync(review); - return review; - } - /// /// SoftDeleteReviewAsync /// From a9061097a7449b1c442c811cc6ac66964778fca0 Mon Sep 17 00:00:00 2001 From: helen229 Date: Tue, 7 Oct 2025 15:33:28 -0700 Subject: [PATCH 05/19] move enum to model file --- .../APIViewWeb/Helpers/CommonUtilities.cs | 40 ------------------- .../APIViewWeb/Managers/PullRequestManager.cs | 4 +- .../APIView/APIViewWeb/Models/PackageModel.cs | 23 ++++++++++- 3 files changed, 23 insertions(+), 44 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs index 99dfbf83871..cbce960a0aa 100644 --- a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs +++ b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs @@ -69,44 +69,4 @@ public static bool IsSDKLanguageOrTypeSpec(string language) return ApiViewConstants.AllSupportedLanguages.Contains(language); } } - - /// - /// Represents the plane classification of a package - /// - [JsonConverter(typeof(StringEnumConverter))] - public enum PackageType - { - /// - /// Data plane package (client libraries for Azure services) - /// - Data, - - /// - /// Management plane package (resource management libraries) - /// - Management, - } -} - - /* - /// - /// Backward compatibility alias for existing code - /// TODO: Auto-approval feature is currently disabled - commenting out for future use - /// - [Obsolete("Use DateTimeHelper instead for better organization")] - public static class BusinessDayCalculator - { - /// - /// Calculate business days from a start date, excluding weekends - /// TODO: Auto-approval feature is currently disabled - commenting out for future use - /// - /// The starting date - /// Number of business days to add - /// The calculated date after adding the specified business days - public static DateTime CalculateBusinessDays(DateTime startDate, int businessDays) - { - return DateTimeHelper.CalculateBusinessDays(startDate, businessDays); - } - } - */ } diff --git a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs index f5d5909b890..7a6f7563a06 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs @@ -255,8 +255,8 @@ private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string origina if (review == null) { // Parse package type if provided, otherwise pass null for automatic classification - Helpers.PackageType? validPackageType = null; - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) + APIViewWeb.Models.PackageType? validPackageType = null; + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) { validPackageType = packageTypeEnum; } diff --git a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs index d3a7f584352..a8ad278a362 100644 --- a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs +++ b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs @@ -1,6 +1,8 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. using CsvHelper.Configuration.Attributes; +using Newtonsoft.Json; +using Newtonsoft.Json.Converters; namespace APIViewWeb.Models { @@ -21,4 +23,21 @@ public class PackageModel [Name("GroupId")] public string GroupId { get; set; } } + + /// + /// Represents the plane classification of a package + /// + [JsonConverter(typeof(StringEnumConverter))] + public enum PackageType + { + /// + /// Data plane package (client libraries for Azure services) + /// + Data, + + /// + /// Management plane package (resource management libraries) + /// + Management, + } } From 46d85e357871c05433852b4b2848840e958d8614 Mon Sep 17 00:00:00 2001 From: helen229 Date: Tue, 7 Oct 2025 15:42:36 -0700 Subject: [PATCH 06/19] revert change back due to deployment issue --- .../Managers/Interfaces/IReviewManager.cs | 2 + .../APIViewWeb/Managers/ReviewManager.cs | 16 ++++ .../APIView/APIViewWeb/Models/PackageModel.cs | 86 +++++++++---------- 3 files changed, 61 insertions(+), 43 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs index a95921870a5..b08c710ae0a 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IReviewManager.cs @@ -3,6 +3,7 @@ using System.Threading.Tasks; using APIViewWeb.Helpers; using APIViewWeb.LeanModels; +using APIViewWeb.Models; using Microsoft.AspNetCore.Http; @@ -20,6 +21,7 @@ public interface IReviewManager public Task GetLegacyReviewAsync(ClaimsPrincipal user, string id); public Task GetOrCreateReview(IFormFile file, string filePath, string language, bool runAnalysis = false); public Task CreateReviewAsync(string packageName, string language, bool isClosed = true, PackageType? packageType = null); + public Task UpdateReviewAsync(ReviewListItemModel review); public Task SoftDeleteReviewAsync(ClaimsPrincipal user, string id); public Task ToggleReviewIsClosedAsync(ClaimsPrincipal user, string id); public Task ToggleReviewApprovalAsync(ClaimsPrincipal user, string id, string revisionId, string notes=""); diff --git a/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs index c88ef8049b5..252b8840b9f 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs @@ -270,6 +270,22 @@ public async Task CreateReviewAsync(string packageName, str return review; } + /// + /// Update an existing review + /// + /// The review to update + /// + public async Task UpdateReviewAsync(ReviewListItemModel review) + { + if (review == null) + { + throw new ArgumentNullException(nameof(review)); + } + + await _reviewsRepository.UpsertReviewAsync(review); + return review; + } + /// /// SoftDeleteReviewAsync /// diff --git a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs index a8ad278a362..0f575584e6a 100644 --- a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs +++ b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs @@ -1,43 +1,43 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. -using CsvHelper.Configuration.Attributes; -using Newtonsoft.Json; -using Newtonsoft.Json.Converters; - -namespace APIViewWeb.Models -{ - public class PackageModel - { - [Name("Package")] - public string Name { get; set; } - - [Name("DisplayName")] - public string DisplayName { get; set; } - - [Name("ServiceName")] - public string ServiceName { get; set; } - - [Name("New")] - public bool IsNew { get; set; } - - [Name("GroupId")] - public string GroupId { get; set; } - } - - /// - /// Represents the plane classification of a package - /// - [JsonConverter(typeof(StringEnumConverter))] - public enum PackageType - { - /// - /// Data plane package (client libraries for Azure services) - /// - Data, - - /// - /// Management plane package (resource management libraries) - /// - Management, - } -} +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +using CsvHelper.Configuration.Attributes; +using Newtonsoft.Json; +using Newtonsoft.Json.Converters; + +namespace APIViewWeb.Models +{ + public class PackageModel + { + [Name("Package")] + public string Name { get; set; } + + [Name("DisplayName")] + public string DisplayName { get; set; } + + [Name("ServiceName")] + public string ServiceName { get; set; } + + [Name("New")] + public bool IsNew { get; set; } + + [Name("GroupId")] + public string GroupId { get; set; } + } + + /// + /// Represents the plane classification of a package + /// + [JsonConverter(typeof(StringEnumConverter))] + public enum PackageType + { + /// + /// Data plane package + /// + Data, + + /// + /// Management plane package + /// + Management, + } +} From 1ceba9fe72dc2cb8b269a2d22d568adab699ca50 Mon Sep 17 00:00:00 2001 From: helen229 Date: Thu, 9 Oct 2025 07:53:11 -0700 Subject: [PATCH 07/19] update enum value to match SdkType --- src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs index 0f575584e6a..c84bf0ca03b 100644 --- a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs +++ b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs @@ -3,6 +3,7 @@ using CsvHelper.Configuration.Attributes; using Newtonsoft.Json; using Newtonsoft.Json.Converters; +using System.Runtime.Serialization; namespace APIViewWeb.Models { @@ -31,13 +32,15 @@ public class PackageModel public enum PackageType { /// - /// Data plane package + /// Data plane package (client libraries for Azure services) /// + [EnumMember(Value = "client")] Data, /// - /// Management plane package + /// Management plane package (resource management libraries) /// + [EnumMember(Value = "mgmt")] Management, } } From 16206121f0a5e5ad1b79b5df2161efce3dde24d6 Mon Sep 17 00:00:00 2001 From: helen229 Date: Thu, 9 Oct 2025 12:53:42 -0700 Subject: [PATCH 08/19] update existing review with PackageType --- .../APIViewWeb/Managers/PullRequestManager.cs | 21 +++++++++++++++---- .../APIView/APIViewWeb/Models/PackageModel.cs | 6 ++---- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs index 7a6f7563a06..abce13d20ee 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs @@ -254,16 +254,29 @@ private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string origina var review = await _reviewManager.GetReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName); if (review == null) { - // Parse package type if provided, otherwise pass null for automatic classification - APIViewWeb.Models.PackageType? validPackageType = null; - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) + // Parse package type if provided, otherwise pass null + Models.PackageType? validPackageType = null; + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) { validPackageType = packageTypeEnum; } - review = await _reviewManager.CreateReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName, isClosed: false, packageType: validPackageType); responseContent.ActionsTaken.Add($"No existing review with packageName: '{codeFile.PackageName}' and language: '{codeFile.Language}'."); responseContent.ActionsTaken.Add($"Created a new Review with Id: '{review.Id}'."); + responseContent.ActionsTaken.Add($"Review created with packageType: '{validPackageType?.ToString() ?? "null"}'."); + } + else + { + // Update existing review with packageType if provided and different from current value + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) + { + if (!review.PackageType.HasValue || review.PackageType.Value != packageTypeEnum) + { + review.PackageType = packageTypeEnum; + review = await _reviewManager.UpdateReviewAsync(review); + responseContent.ActionsTaken.Add($"Updated existing review '{review.Id}' with PackageType: '{packageTypeEnum}'."); + } + } } pullRequestModel.ReviewId = review.Id; responseContent.ActionsTaken.Add($"Pull Request data ReviewId set to '{pullRequestModel.ReviewId}' in memory - not yet saved to database."); diff --git a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs index c84bf0ca03b..451ce364d30 100644 --- a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs +++ b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs @@ -34,13 +34,11 @@ public enum PackageType /// /// Data plane package (client libraries for Azure services) /// - [EnumMember(Value = "client")] - Data, + client, /// /// Management plane package (resource management libraries) /// - [EnumMember(Value = "mgmt")] - Management, + mgmt, } } From 482772f72264b78a0e91f42c5f62692908bc0bef Mon Sep 17 00:00:00 2001 From: helen229 Date: Mon, 13 Oct 2025 10:29:11 -0700 Subject: [PATCH 09/19] fix comments --- .../Controllers/AutoReviewController.cs | 42 +++++++++---------- .../APIViewWeb/Managers/PullRequestManager.cs | 29 +++++++------ .../review-page/review-page.component.ts | 6 +-- 3 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs index 7b1ae696e89..58f35f22fe5 100644 --- a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs +++ b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs @@ -54,7 +54,7 @@ public async Task UploadAutoReview([FromForm] IFormFile file, stri var codeFile = await _codeFileManager.CreateCodeFileAsync(originalName: file.FileName, fileStream: openReadStream, runAnalysis: false, memoryStream: memoryStream); - (var review, var apiRevision) = await CreateAutomaticRevisionAsync(codeFile: codeFile, label: label, originalName: file.FileName, memoryStream: memoryStream, compareAllRevisions, packageType); + (var review, var apiRevision) = await CreateAutomaticRevisionAsync(codeFile: codeFile, label: label, originalName: file.FileName, memoryStream: memoryStream, packageType: packageType, compareAllRevisions: compareAllRevisions); if (apiRevision != null) { apiRevision = await _apiRevisionsManager.UpdateRevisionMetadataAsync(apiRevision, packageVersion ?? codeFile.PackageVersion, label, setReleaseTag); @@ -155,7 +155,7 @@ public async Task CreateApiReview( { return StatusCode(statusCode: StatusCodes.Status204NoContent, $"API review code file for package {packageName} is not found in DevOps pipeline artifacts."); } - (var review, var apiRevision) = await CreateAutomaticRevisionAsync(codeFile: codeFile, label: label, originalName: originalFilePath, memoryStream: memoryStream, compareAllRevisions, packageType); + (var review, var apiRevision) = await CreateAutomaticRevisionAsync(codeFile: codeFile, label: label, originalName: originalFilePath, memoryStream: memoryStream, packageType: packageType, compareAllRevisions: compareAllRevisions); if (apiRevision != null) { apiRevision = await _apiRevisionsManager.UpdateRevisionMetadataAsync(apiRevision, packageVersion ?? codeFile.PackageVersion, label, setReleaseTag); @@ -178,8 +178,20 @@ public async Task CreateApiReview( return StatusCode(statusCode: StatusCodes.Status500InternalServerError); } - private async Task<(ReviewListItemModel review, APIRevisionListItemModel apiRevision)> CreateAutomaticRevisionAsync(CodeFile codeFile, string label, string originalName, MemoryStream memoryStream, bool compareAllRevisions = false, string packageType = null) + private static PackageType? ParsePackageType(string packageType) { + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var parsedPackageType)) + { + return parsedPackageType; + } + return null; + } + + private async Task<(ReviewListItemModel review, APIRevisionListItemModel apiRevision)> CreateAutomaticRevisionAsync(CodeFile codeFile, string label, string originalName, MemoryStream memoryStream, string packageType, bool compareAllRevisions = false) + { + // Parse package type once at the beginning + var parsedPackageType = ParsePackageType(packageType); + var createNewRevision = true; var review = await _reviewManager.GetReviewAsync(packageName: codeFile.PackageName, language: codeFile.Language, isClosed: null); var apiRevision = default(APIRevisionListItemModel); @@ -188,20 +200,11 @@ public async Task CreateApiReview( if (review != null) { - // Update package type if provided from controller parameter - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var parsedPackageType)) + // Update package type if provided from controller parameter and not already set + if (parsedPackageType.HasValue && !review.PackageType.HasValue) { - if (!review.PackageType.HasValue) - { - // If the current review has no packageType and packageType is provided, update it - review.PackageType = parsedPackageType; - review = await _reviewManager.UpdateReviewAsync(review); - } - else if (review.PackageType.Value != parsedPackageType) - { - // If review has a packageType that doesn't match the provided packageType - throw new InvalidOperationException($"Package type conflict: Review has PackageType '{review.PackageType.Value}' but supplied PackageType is '{parsedPackageType}'. Package types cannot be changed once set."); - } + review.PackageType = parsedPackageType; + review = await _reviewManager.UpdateReviewAsync(review); } apiRevisions = await _apiRevisionsManager.GetAPIRevisionsAsync(review.Id); @@ -256,13 +259,6 @@ public async Task CreateApiReview( } else { - // Parse package type if provided, otherwise pass null for automatic classification - PackageType? parsedPackageType = null; - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) - { - parsedPackageType = packageTypeEnum; - } - review = await _reviewManager.CreateReviewAsync(packageName: codeFile.PackageName, language: codeFile.Language, isClosed: false, packageType: parsedPackageType); } diff --git a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs index abce13d20ee..52f88521a3b 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs @@ -247,19 +247,25 @@ private async Task ClosePullRequestAPIRevision(PullRequestModel pullRequestModel await _pullRequestsRepository.UpsertPullRequestAsync(pullRequestModel); } + private static Models.PackageType? ParsePackageType(string packageType) + { + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var parsedPackageType)) + { + return parsedPackageType; + } + return null; + } + private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string originalFileName, MemoryStream memoryStream, PullRequestModel pullRequestModel, CodeFile baselineCodeFile, MemoryStream baseLineStream, string baselineFileName, CreateAPIRevisionAPIResponse responseContent, string packageType = null) { + // Parse package type once at the beginning + var validPackageType = ParsePackageType(packageType); + // fetch review for the package or create brand new review var review = await _reviewManager.GetReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName); if (review == null) { - // Parse package type if provided, otherwise pass null - Models.PackageType? validPackageType = null; - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) - { - validPackageType = packageTypeEnum; - } review = await _reviewManager.CreateReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName, isClosed: false, packageType: validPackageType); responseContent.ActionsTaken.Add($"No existing review with packageName: '{codeFile.PackageName}' and language: '{codeFile.Language}'."); responseContent.ActionsTaken.Add($"Created a new Review with Id: '{review.Id}'."); @@ -268,14 +274,11 @@ private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string origina else { // Update existing review with packageType if provided and different from current value - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var packageTypeEnum)) + if (validPackageType.HasValue && (!review.PackageType.HasValue || review.PackageType.Value != validPackageType.Value)) { - if (!review.PackageType.HasValue || review.PackageType.Value != packageTypeEnum) - { - review.PackageType = packageTypeEnum; - review = await _reviewManager.UpdateReviewAsync(review); - responseContent.ActionsTaken.Add($"Updated existing review '{review.Id}' with PackageType: '{packageTypeEnum}'."); - } + review.PackageType = validPackageType; + review = await _reviewManager.UpdateReviewAsync(review); + responseContent.ActionsTaken.Add($"Updated existing review '{review.Id}' with PackageType: '{validPackageType}'."); } } pullRequestModel.ReviewId = review.Id; diff --git a/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts b/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts index 459c32f8533..a404c1d4dd1 100644 --- a/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts +++ b/src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts @@ -572,7 +572,7 @@ export class ReviewPageComponent implements OnInit { if (value) { this.reviewsService.toggleReviewApproval(this.reviewId!, this.activeApiRevisionId!, true).pipe(take(1)).subscribe({ next: (review: Review) => { - this.updateReview(review); + this.review = review; } }); } @@ -582,7 +582,7 @@ export class ReviewPageComponent implements OnInit { if (value) { this.reviewsService.requestNamespaceReview(this.reviewId!, this.activeApiRevisionId!).pipe(take(1)).subscribe({ next: (review: Review) => { - this.updateReview(review); + this.review = review; // Reset loading state in the options component on success if (this.reviewPageOptionsComponent) { this.reviewPageOptionsComponent.resetNamespaceReviewLoadingState(); @@ -674,7 +674,7 @@ export class ReviewPageComponent implements OnInit { this.signalRService.onReviewUpdates().pipe(takeUntil(this.destroy$)).subscribe({ next: (updatedReview: Review) => { if (updatedReview.id === this.reviewId) { - this.updateReview(updatedReview); + this.review = updatedReview; } } }); From c01b5b3b15a4905bdf15790e7880cd3bea4f95d9 Mon Sep 17 00:00:00 2001 From: helen229 Date: Mon, 13 Oct 2025 10:35:49 -0700 Subject: [PATCH 10/19] update enum type --- src/dotnet/APIView/ClientSPA/src/app/_models/review.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts b/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts index 6185bd804ae..142270fc06e 100644 --- a/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts +++ b/src/dotnet/APIView/ClientSPA/src/app/_models/review.ts @@ -7,8 +7,8 @@ export enum FirstReleaseApproval { } export enum PackageType { - Data = 'Data', - Management = 'Management' + client = 'client', + mgmt = 'mgmt' } export class Review { From cd44a31f4fc242e27a2f7d6abead9457f4929f48 Mon Sep 17 00:00:00 2001 From: helen229 Date: Mon, 13 Oct 2025 21:24:53 -0700 Subject: [PATCH 11/19] update to JsonStringEnumConver --- src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs index 451ce364d30..144112f1044 100644 --- a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs +++ b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs @@ -1,9 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. using CsvHelper.Configuration.Attributes; -using Newtonsoft.Json; -using Newtonsoft.Json.Converters; -using System.Runtime.Serialization; +using System.Text.Json.Serialization; namespace APIViewWeb.Models { @@ -28,7 +26,7 @@ public class PackageModel /// /// Represents the plane classification of a package /// - [JsonConverter(typeof(StringEnumConverter))] + [JsonConverter(typeof(JsonStringEnumConverter))] public enum PackageType { /// From 907356040380066bbc2f71f961211d63c4970bf5 Mon Sep 17 00:00:00 2001 From: helen229 Date: Tue, 14 Oct 2025 09:27:10 -0700 Subject: [PATCH 12/19] Add PackageType enum with centralized parsing and Unknown default --- .../Controllers/AutoReviewController.cs | 13 ++---------- .../APIViewWeb/Helpers/CommonUtilities.cs | 20 +++++++++++++++++++ .../APIViewWeb/Managers/PullRequestManager.cs | 16 +++------------ .../APIView/APIViewWeb/Models/PackageModel.cs | 5 +++++ 4 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs index 58f35f22fe5..a2190417cd8 100644 --- a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs +++ b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs @@ -178,19 +178,10 @@ public async Task CreateApiReview( return StatusCode(statusCode: StatusCodes.Status500InternalServerError); } - private static PackageType? ParsePackageType(string packageType) - { - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var parsedPackageType)) - { - return parsedPackageType; - } - return null; - } - private async Task<(ReviewListItemModel review, APIRevisionListItemModel apiRevision)> CreateAutomaticRevisionAsync(CodeFile codeFile, string label, string originalName, MemoryStream memoryStream, string packageType, bool compareAllRevisions = false) { // Parse package type once at the beginning - var parsedPackageType = ParsePackageType(packageType); + var parsedPackageType = PackageTypeHelper.ParsePackageType(packageType); var createNewRevision = true; var review = await _reviewManager.GetReviewAsync(packageName: codeFile.PackageName, language: codeFile.Language, isClosed: null); @@ -201,7 +192,7 @@ public async Task CreateApiReview( if (review != null) { // Update package type if provided from controller parameter and not already set - if (parsedPackageType.HasValue && !review.PackageType.HasValue) + if (parsedPackageType != PackageType.Unknown && !review.PackageType.HasValue) { review.PackageType = parsedPackageType; review = await _reviewManager.UpdateReviewAsync(review); diff --git a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs index cbce960a0aa..1af1f88fb79 100644 --- a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs +++ b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs @@ -69,4 +69,24 @@ public static bool IsSDKLanguageOrTypeSpec(string language) return ApiViewConstants.AllSupportedLanguages.Contains(language); } } + + /// + /// Package type parsing and validation utilities for APIView + /// + public static class PackageTypeHelper + { + /// + /// Parse a package type string into a PackageType enum value + /// + /// The package type string to parse (e.g., "client", "mgmt") + /// The parsed PackageType enum value, or PackageType.Unknown if invalid or null + public static Models.PackageType ParsePackageType(string packageType) + { + if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var parsedPackageType)) + { + return parsedPackageType; + } + return Models.PackageType.Unknown; + } + } } diff --git a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs index 52f88521a3b..09464a74c2d 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs @@ -247,20 +247,10 @@ private async Task ClosePullRequestAPIRevision(PullRequestModel pullRequestModel await _pullRequestsRepository.UpsertPullRequestAsync(pullRequestModel); } - private static Models.PackageType? ParsePackageType(string packageType) - { - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var parsedPackageType)) - { - return parsedPackageType; - } - return null; - } - private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string originalFileName, MemoryStream memoryStream, PullRequestModel pullRequestModel, CodeFile baselineCodeFile, MemoryStream baseLineStream, string baselineFileName, CreateAPIRevisionAPIResponse responseContent, string packageType = null) { - // Parse package type once at the beginning - var validPackageType = ParsePackageType(packageType); + var validPackageType = PackageTypeHelper.ParsePackageType(packageType); // fetch review for the package or create brand new review var review = await _reviewManager.GetReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName); @@ -269,12 +259,12 @@ private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string origina review = await _reviewManager.CreateReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName, isClosed: false, packageType: validPackageType); responseContent.ActionsTaken.Add($"No existing review with packageName: '{codeFile.PackageName}' and language: '{codeFile.Language}'."); responseContent.ActionsTaken.Add($"Created a new Review with Id: '{review.Id}'."); - responseContent.ActionsTaken.Add($"Review created with packageType: '{validPackageType?.ToString() ?? "null"}'."); + responseContent.ActionsTaken.Add($"Review created with packageType: '{validPackageType}'."); } else { // Update existing review with packageType if provided and different from current value - if (validPackageType.HasValue && (!review.PackageType.HasValue || review.PackageType.Value != validPackageType.Value)) + if (validPackageType != Models.PackageType.Unknown && (!review.PackageType.HasValue || review.PackageType.Value != validPackageType)) { review.PackageType = validPackageType; review = await _reviewManager.UpdateReviewAsync(review); diff --git a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs index 144112f1044..1f08ed8dcbb 100644 --- a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs +++ b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs @@ -29,6 +29,11 @@ public class PackageModel [JsonConverter(typeof(JsonStringEnumConverter))] public enum PackageType { + /// + /// Unknown or unspecified package type + /// + Unknown, + /// /// Data plane package (client libraries for Azure services) /// From 7dd92b6c7b8dcbd5f5aa3f7072d35248c7e5ba21 Mon Sep 17 00:00:00 2001 From: helen229 Date: Tue, 14 Oct 2025 13:30:18 -0700 Subject: [PATCH 13/19] change it back to null and one line --- .../Controllers/AutoReviewController.cs | 4 ++-- .../APIViewWeb/Helpers/CommonUtilities.cs | 20 ------------------- .../APIViewWeb/Managers/PullRequestManager.cs | 4 ++-- .../APIView/APIViewWeb/Models/PackageModel.cs | 5 ----- 4 files changed, 4 insertions(+), 29 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs index a2190417cd8..6dcd4446ae6 100644 --- a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs +++ b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs @@ -181,7 +181,7 @@ public async Task CreateApiReview( private async Task<(ReviewListItemModel review, APIRevisionListItemModel apiRevision)> CreateAutomaticRevisionAsync(CodeFile codeFile, string label, string originalName, MemoryStream memoryStream, string packageType, bool compareAllRevisions = false) { // Parse package type once at the beginning - var parsedPackageType = PackageTypeHelper.ParsePackageType(packageType); + var parsedPackageType = !string.IsNullOrEmpty(packageType) ? Enum.TryParse(packageType, true, out var result) ? (PackageType?)result : null : null; var createNewRevision = true; var review = await _reviewManager.GetReviewAsync(packageName: codeFile.PackageName, language: codeFile.Language, isClosed: null); @@ -192,7 +192,7 @@ public async Task CreateApiReview( if (review != null) { // Update package type if provided from controller parameter and not already set - if (parsedPackageType != PackageType.Unknown && !review.PackageType.HasValue) + if (parsedPackageType.HasValue && !review.PackageType.HasValue) { review.PackageType = parsedPackageType; review = await _reviewManager.UpdateReviewAsync(review); diff --git a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs index 1af1f88fb79..cbce960a0aa 100644 --- a/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs +++ b/src/dotnet/APIView/APIViewWeb/Helpers/CommonUtilities.cs @@ -69,24 +69,4 @@ public static bool IsSDKLanguageOrTypeSpec(string language) return ApiViewConstants.AllSupportedLanguages.Contains(language); } } - - /// - /// Package type parsing and validation utilities for APIView - /// - public static class PackageTypeHelper - { - /// - /// Parse a package type string into a PackageType enum value - /// - /// The package type string to parse (e.g., "client", "mgmt") - /// The parsed PackageType enum value, or PackageType.Unknown if invalid or null - public static Models.PackageType ParsePackageType(string packageType) - { - if (!string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var parsedPackageType)) - { - return parsedPackageType; - } - return Models.PackageType.Unknown; - } - } } diff --git a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs index 09464a74c2d..790b1ca150a 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs @@ -250,7 +250,7 @@ private async Task ClosePullRequestAPIRevision(PullRequestModel pullRequestModel private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string originalFileName, MemoryStream memoryStream, PullRequestModel pullRequestModel, CodeFile baselineCodeFile, MemoryStream baseLineStream, string baselineFileName, CreateAPIRevisionAPIResponse responseContent, string packageType = null) { - var validPackageType = PackageTypeHelper.ParsePackageType(packageType); + var validPackageType = !string.IsNullOrEmpty(packageType) ? Enum.TryParse(packageType, true, out var result) ? (Models.PackageType?)result : null : null; // fetch review for the package or create brand new review var review = await _reviewManager.GetReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName); @@ -264,7 +264,7 @@ private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string origina else { // Update existing review with packageType if provided and different from current value - if (validPackageType != Models.PackageType.Unknown && (!review.PackageType.HasValue || review.PackageType.Value != validPackageType)) + if (validPackageType.HasValue && (!review.PackageType.HasValue || review.PackageType.Value != validPackageType.Value)) { review.PackageType = validPackageType; review = await _reviewManager.UpdateReviewAsync(review); diff --git a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs index 1f08ed8dcbb..144112f1044 100644 --- a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs +++ b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs @@ -29,11 +29,6 @@ public class PackageModel [JsonConverter(typeof(JsonStringEnumConverter))] public enum PackageType { - /// - /// Unknown or unspecified package type - /// - Unknown, - /// /// Data plane package (client libraries for Azure services) /// From b3a9399f1967d725490be464ec3e0120e9847ced Mon Sep 17 00:00:00 2001 From: helen229 Date: Tue, 14 Oct 2025 13:38:48 -0700 Subject: [PATCH 14/19] update expression --- .../APIView/APIViewWeb/Controllers/AutoReviewController.cs | 2 +- src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs index 6dcd4446ae6..cf7fa2f188f 100644 --- a/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs +++ b/src/dotnet/APIView/APIViewWeb/Controllers/AutoReviewController.cs @@ -181,7 +181,7 @@ public async Task CreateApiReview( private async Task<(ReviewListItemModel review, APIRevisionListItemModel apiRevision)> CreateAutomaticRevisionAsync(CodeFile codeFile, string label, string originalName, MemoryStream memoryStream, string packageType, bool compareAllRevisions = false) { // Parse package type once at the beginning - var parsedPackageType = !string.IsNullOrEmpty(packageType) ? Enum.TryParse(packageType, true, out var result) ? (PackageType?)result : null : null; + var parsedPackageType = !string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var result) ? (PackageType?)result : null; var createNewRevision = true; var review = await _reviewManager.GetReviewAsync(packageName: codeFile.PackageName, language: codeFile.Language, isClosed: null); diff --git a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs index 790b1ca150a..930ed829d1f 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs @@ -250,7 +250,7 @@ private async Task ClosePullRequestAPIRevision(PullRequestModel pullRequestModel private async Task CreateAPIRevisionIfRequired(CodeFile codeFile, string originalFileName, MemoryStream memoryStream, PullRequestModel pullRequestModel, CodeFile baselineCodeFile, MemoryStream baseLineStream, string baselineFileName, CreateAPIRevisionAPIResponse responseContent, string packageType = null) { - var validPackageType = !string.IsNullOrEmpty(packageType) ? Enum.TryParse(packageType, true, out var result) ? (Models.PackageType?)result : null : null; + var validPackageType = !string.IsNullOrEmpty(packageType) && Enum.TryParse(packageType, true, out var result) ? (Models.PackageType?)result : null; // fetch review for the package or create brand new review var review = await _reviewManager.GetReviewAsync(language: codeFile.Language, packageName: codeFile.PackageName); From 1c97d1955f2fc7c27e22fd4c643997901c87c0aa Mon Sep 17 00:00:00 2001 From: helen229 Date: Thu, 16 Oct 2025 11:53:39 -0700 Subject: [PATCH 15/19] revert to StringEnumConverter --- src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs index 144112f1044..ebc237887f3 100644 --- a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs +++ b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs @@ -1,7 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. using CsvHelper.Configuration.Attributes; -using System.Text.Json.Serialization; +using Newtonsoft.Json; +using Newtonsoft.Json.Converters; namespace APIViewWeb.Models { @@ -26,17 +27,17 @@ public class PackageModel /// /// Represents the plane classification of a package /// - [JsonConverter(typeof(JsonStringEnumConverter))] + [JsonConverter(typeof(StringEnumConverter))] public enum PackageType { /// /// Data plane package (client libraries for Azure services) /// - client, + client = 0, /// /// Management plane package (resource management libraries) /// - mgmt, + Management = 1, } } From 2dd4b97addb0b68dd7d739aaad5c533999c02127 Mon Sep 17 00:00:00 2001 From: helen229 Date: Thu, 16 Oct 2025 15:10:27 -0700 Subject: [PATCH 16/19] modifcations after testing with mgmt --- .../APIView/APIViewWeb/Managers/PullRequestManager.cs | 6 ++++++ src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs index 930ed829d1f..f164b65e891 100644 --- a/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs +++ b/src/dotnet/APIView/APIViewWeb/Managers/PullRequestManager.cs @@ -150,6 +150,12 @@ public async Task CreateAPIRevisionIfAPIHasChanges( baselineCodeFileName: baselineCodeFileName, baselineStream: baselineStream, project: project, language: language); + if (codeFile == null) + { + responseContent.ActionsTaken.Add($"Failed to process code file. Language processor for '{language}' may not be available or file format is unsupported."); + return ""; + } + if (codeFile.PackageName != null && (packageName == null || packageName != codeFile.PackageName)) { packageName = codeFile.PackageName; diff --git a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs index ebc237887f3..ccecdb9ad1d 100644 --- a/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs +++ b/src/dotnet/APIView/APIViewWeb/Models/PackageModel.cs @@ -38,6 +38,6 @@ public enum PackageType /// /// Management plane package (resource management libraries) /// - Management = 1, + mgmt = 1, } } From 09d2eda46fd7d1fa1404f486a1c898c549a6515c Mon Sep 17 00:00:00 2001 From: helen229 Date: Fri, 17 Oct 2025 09:35:43 -0700 Subject: [PATCH 17/19] add tests --- .../AutoReviewControllerTests.cs | 82 +++++++++++++++++++ .../PullRequestsControllerTests.cs | 77 +++++++++++++++++ 2 files changed, 159 insertions(+) create mode 100644 src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs create mode 100644 src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs diff --git a/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs b/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs new file mode 100644 index 00000000000..46c4c2e187c --- /dev/null +++ b/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs @@ -0,0 +1,82 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Security.Claims; +using System.Threading.Tasks; +using ApiView; +using APIViewWeb.Controllers; +using APIViewWeb.Helpers; +using APIViewWeb.LeanModels; +using APIViewWeb.Managers.Interfaces; +using APIViewWeb.Models; +using FluentAssertions; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Configuration; +using Moq; +using Xunit; + +namespace APIViewUnitTests +{ + public class AutoReviewControllerTests + { + [Theory] + [InlineData("client", PackageType.client)] + [InlineData("mgmt", PackageType.mgmt)] + [InlineData("CLIENT", PackageType.client)] // Test case insensitivity + [InlineData("MGMT", PackageType.mgmt)] + public void PackageTypeEnumParsing_WithValidValues_ParsesCorrectly(string packageTypeString, PackageType expectedPackageType) + { + // Act + var result = Enum.TryParse(packageTypeString, true, out var parsedPackageType); + + // Assert + result.Should().BeTrue(); + parsedPackageType.Should().Be(expectedPackageType); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("invalid")] + [InlineData("unknown")] + public void PackageTypeEnumParsing_WithInvalidValues_ReturnsFalse(string packageTypeString) + { + // Act + var result = Enum.TryParse(packageTypeString, true, out var parsedPackageType); + + // Assert + result.Should().BeFalse(); + parsedPackageType.Should().Be(default(PackageType)); + } + + [Theory] + [InlineData("client", PackageType.client)] + [InlineData("mgmt", PackageType.mgmt)] + public void PackageTypeEnumParsing_WithStringValues_ProducesCorrectNullableResult(string packageTypeString, PackageType expectedPackageType) + { + // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync + var parsedPackageType = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var result) ? (PackageType?)result : null; + + // Assert + parsedPackageType.Should().NotBeNull(); + parsedPackageType.Value.Should().Be(expectedPackageType); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("invalid")] + public void PackageTypeEnumParsing_WithInvalidStringValues_ProducesNullResult(string packageTypeString) + { + // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync + var parsedPackageType = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var result) ? (PackageType?)result : null; + + // Assert + parsedPackageType.Should().BeNull(); + } + } +} diff --git a/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs b/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs new file mode 100644 index 00000000000..20e67fb1490 --- /dev/null +++ b/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs @@ -0,0 +1,77 @@ +using System; +using APIViewWeb.Models; +using FluentAssertions; +using Xunit; + +namespace APIViewUnitTests +{ + public class PullRequestsControllerTests + { + [Theory] + [InlineData("client")] + [InlineData("mgmt")] + [InlineData("CLIENT")] // Test case insensitivity + [InlineData("MGMT")] + public void PackageTypeParameterHandling_WithValidValues_ShouldPassThroughToManager(string packageTypeValue) + { + // This test verifies that valid packageType values are passed through correctly + // The actual enum parsing happens in the manager layer, not the controller + // Controllers should pass through the string value as-is + + // Act - simulate controller receiving packageType parameter + var receivedPackageType = packageTypeValue; + + // Assert - controller should pass through the value unchanged + receivedPackageType.Should().Be(packageTypeValue); + receivedPackageType.Should().NotBeNullOrEmpty(); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("invalid")] + [InlineData("unknown")] + public void PackageTypeParameterHandling_WithInvalidValues_ShouldPassThroughToManager(string packageTypeValue) + { + // This test verifies that invalid/null packageType values are also passed through + // The validation happens in the manager layer, not the controller + + // Act - simulate controller receiving packageType parameter + var receivedPackageType = packageTypeValue; + + // Assert - controller should pass through the value unchanged, even if invalid + receivedPackageType.Should().Be(packageTypeValue); + } + + [Fact] + public void PackageTypeDefaultValue_ShouldBeNull() + { + // Test verifies that the default value for packageType parameter is null + // when not provided in the request + + // Act - simulate controller method with default parameter value + string packageType = null; // Default value for optional parameter + + // Assert + packageType.Should().BeNull(); + } + + [Theory] + [InlineData("client", "Client package type should be supported")] + [InlineData("mgmt", "Management package type should be supported")] + public void PackageTypeController_DocumentedBehavior_ForValidValues(string packageTypeValue, string expectedBehavior) + { + // This test documents the expected behavior for different packageType values + // in the context of the PullRequestsController + + // Act + var isValidValue = !string.IsNullOrWhiteSpace(packageTypeValue) && + (packageTypeValue.Equals("client", StringComparison.OrdinalIgnoreCase) || + packageTypeValue.Equals("mgmt", StringComparison.OrdinalIgnoreCase)); + + // Assert + isValidValue.Should().BeTrue(expectedBehavior); + } + } +} From 71b1555c10b3009598aaba4bb8c90018732e8ea0 Mon Sep 17 00:00:00 2001 From: helen229 Date: Fri, 17 Oct 2025 09:59:21 -0700 Subject: [PATCH 18/19] update unit tests --- .../AutoReviewControllerTests.cs | 99 ++++++++++++++----- .../PullRequestsControllerTests.cs | 26 ++--- 2 files changed, 83 insertions(+), 42 deletions(-) diff --git a/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs b/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs index 46c4c2e187c..e997ebfc3e0 100644 --- a/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs +++ b/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs @@ -28,12 +28,15 @@ public class AutoReviewControllerTests [InlineData("MGMT", PackageType.mgmt)] public void PackageTypeEnumParsing_WithValidValues_ParsesCorrectly(string packageTypeString, PackageType expectedPackageType) { - // Act - var result = Enum.TryParse(packageTypeString, true, out var parsedPackageType); + // Act - test both direct enum parsing and controller logic + var directParseResult = Enum.TryParse(packageTypeString, true, out var directParsedPackageType); + var controllerLogicResult = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var controllerParsedType) ? (PackageType?)controllerParsedType : null; // Assert - result.Should().BeTrue(); - parsedPackageType.Should().Be(expectedPackageType); + directParseResult.Should().BeTrue(); + directParsedPackageType.Should().Be(expectedPackageType); + controllerLogicResult.Should().NotBeNull(); + controllerLogicResult.Value.Should().Be(expectedPackageType); } [Theory] @@ -42,41 +45,93 @@ public void PackageTypeEnumParsing_WithValidValues_ParsesCorrectly(string packag [InlineData(" ")] [InlineData("invalid")] [InlineData("unknown")] - public void PackageTypeEnumParsing_WithInvalidValues_ReturnsFalse(string packageTypeString) + public void PackageTypeEnumParsing_WithInvalidValues_ReturnsExpectedResults(string packageTypeString) { - // Act - var result = Enum.TryParse(packageTypeString, true, out var parsedPackageType); + // Act - test both direct enum parsing and controller logic + var directParseResult = Enum.TryParse(packageTypeString, true, out var directParsedPackageType); + var controllerLogicResult = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var controllerParsedType) ? (PackageType?)controllerParsedType : null; // Assert - result.Should().BeFalse(); - parsedPackageType.Should().Be(default(PackageType)); + directParseResult.Should().BeFalse(); + directParsedPackageType.Should().Be(default(PackageType)); + controllerLogicResult.Should().BeNull(); } - [Theory] - [InlineData("client", PackageType.client)] - [InlineData("mgmt", PackageType.mgmt)] - public void PackageTypeEnumParsing_WithStringValues_ProducesCorrectNullableResult(string packageTypeString, PackageType expectedPackageType) + [Fact] + public void ReviewUpdate_WithExistingReviewWithoutPackageType_ShouldSetPackageType() { - // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync + // Arrange - simulate existing review without PackageType + var existingReview = new ReviewListItemModel() + { + Id = "existing-review-id", + PackageName = "TestPackage", + Language = "C#", + PackageType = null // No package type set initially + }; + + var packageTypeString = "mgmt"; var parsedPackageType = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var result) ? (PackageType?)result : null; + // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync + if (parsedPackageType.HasValue && !existingReview.PackageType.HasValue) + { + existingReview.PackageType = parsedPackageType; + } + // Assert - parsedPackageType.Should().NotBeNull(); - parsedPackageType.Value.Should().Be(expectedPackageType); + existingReview.PackageType.Should().Be(PackageType.mgmt); + } + + [Fact] + public void ReviewUpdate_WithExistingReviewWithPackageType_ShouldNotOverridePackageType() + { + // Arrange - simulate existing review with PackageType already set + var existingReview = new ReviewListItemModel() + { + Id = "existing-review-id", + PackageName = "TestPackage", + Language = "C#", + PackageType = PackageType.client // Already has package type set + }; + + var packageTypeString = "mgmt"; + var parsedPackageType = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var result) ? (PackageType?)result : null; + + // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync + if (parsedPackageType.HasValue && !existingReview.PackageType.HasValue) + { + existingReview.PackageType = parsedPackageType; + } + + // Assert - PackageType should remain unchanged + existingReview.PackageType.Should().Be(PackageType.client); } [Theory] + [InlineData("invalid-package-type")] [InlineData(null)] [InlineData("")] - [InlineData(" ")] - [InlineData("invalid")] - public void PackageTypeEnumParsing_WithInvalidStringValues_ProducesNullResult(string packageTypeString) + public void ReviewUpdate_WithInvalidOrNullPackageType_ShouldNotSetPackageType(string packageTypeString) { - // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync + // Arrange - simulate existing review without PackageType + var existingReview = new ReviewListItemModel() + { + Id = "existing-review-id", + PackageName = "TestPackage", + Language = "C#", + PackageType = null // No package type set initially + }; + var parsedPackageType = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var result) ? (PackageType?)result : null; - // Assert - parsedPackageType.Should().BeNull(); + // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync + if (parsedPackageType.HasValue && !existingReview.PackageType.HasValue) + { + existingReview.PackageType = parsedPackageType; + } + + // Assert - PackageType should remain null due to invalid/null input + existingReview.PackageType.Should().BeNull(); } } } diff --git a/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs b/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs index 20e67fb1490..7139f6d3589 100644 --- a/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs +++ b/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs @@ -12,40 +12,26 @@ public class PullRequestsControllerTests [InlineData("mgmt")] [InlineData("CLIENT")] // Test case insensitivity [InlineData("MGMT")] - public void PackageTypeParameterHandling_WithValidValues_ShouldPassThroughToManager(string packageTypeValue) - { - // This test verifies that valid packageType values are passed through correctly - // The actual enum parsing happens in the manager layer, not the controller - // Controllers should pass through the string value as-is - - // Act - simulate controller receiving packageType parameter - var receivedPackageType = packageTypeValue; - - // Assert - controller should pass through the value unchanged - receivedPackageType.Should().Be(packageTypeValue); - receivedPackageType.Should().NotBeNullOrEmpty(); - } - - [Theory] [InlineData(null)] [InlineData("")] [InlineData(" ")] [InlineData("invalid")] [InlineData("unknown")] - public void PackageTypeParameterHandling_WithInvalidValues_ShouldPassThroughToManager(string packageTypeValue) + public void PackageTypeParameterHandling_ShouldPassThroughToManager(string packageTypeValue) { - // This test verifies that invalid/null packageType values are also passed through - // The validation happens in the manager layer, not the controller + // This test verifies that packageType values (valid or invalid) are passed through correctly + // The actual enum parsing and validation happens in the manager layer, not the controller + // Controllers should pass through the string value as-is // Act - simulate controller receiving packageType parameter var receivedPackageType = packageTypeValue; - // Assert - controller should pass through the value unchanged, even if invalid + // Assert - controller should pass through the value unchanged receivedPackageType.Should().Be(packageTypeValue); } [Fact] - public void PackageTypeDefaultValue_ShouldBeNull() + public void PackageTypeParameterHandling_WhenOmitted_ShouldDefaultToNull() { // Test verifies that the default value for packageType parameter is null // when not provided in the request From a303298c1a7420887c5f444f749e3c04b80b4add Mon Sep 17 00:00:00 2001 From: helen229 Date: Fri, 17 Oct 2025 10:57:09 -0700 Subject: [PATCH 19/19] update tests with more mocks --- .../AutoReviewControllerTests.cs | 391 +++++++++++++++--- .../PullRequestsControllerTests.cs | 256 ++++++++++-- 2 files changed, 560 insertions(+), 87 deletions(-) diff --git a/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs b/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs index e997ebfc3e0..a57f66d6a74 100644 --- a/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs +++ b/src/dotnet/APIView/APIViewUnitTests/AutoReviewControllerTests.cs @@ -4,9 +4,11 @@ using System.Security.Claims; using System.Threading.Tasks; using ApiView; +using APIViewWeb; using APIViewWeb.Controllers; using APIViewWeb.Helpers; using APIViewWeb.LeanModels; +using APIViewWeb.Managers; using APIViewWeb.Managers.Interfaces; using APIViewWeb.Models; using FluentAssertions; @@ -21,22 +23,112 @@ namespace APIViewUnitTests { public class AutoReviewControllerTests { + private readonly Mock _mockAuthorizationService; + private readonly Mock _mockCodeFileManager; + private readonly Mock _mockReviewManager; + private readonly Mock _mockApiRevisionsManager; + private readonly Mock _mockCommentsManager; + private readonly Mock _mockConfiguration; + private readonly List _languageServices; + private readonly AutoReviewController _controller; + + public AutoReviewControllerTests() + { + _mockAuthorizationService = new Mock(); + _mockCodeFileManager = new Mock(); + _mockReviewManager = new Mock(); + _mockApiRevisionsManager = new Mock(); + _mockCommentsManager = new Mock(); + _mockConfiguration = new Mock(); + _languageServices = new List + { + new MockLanguageService("C#", false) + }; + + _controller = new AutoReviewController( + _mockAuthorizationService.Object, + _mockCodeFileManager.Object, + _mockReviewManager.Object, + _mockApiRevisionsManager.Object, + _mockCommentsManager.Object, + _mockConfiguration.Object, + _languageServices); + + // Set up the HTTP context with a mock user principal + SetupControllerContext(); + } + [Theory] - [InlineData("client", PackageType.client)] - [InlineData("mgmt", PackageType.mgmt)] - [InlineData("CLIENT", PackageType.client)] // Test case insensitivity - [InlineData("MGMT", PackageType.mgmt)] - public void PackageTypeEnumParsing_WithValidValues_ParsesCorrectly(string packageTypeString, PackageType expectedPackageType) + [InlineData("client")] + [InlineData("mgmt")] + [InlineData("CLIENT")] + [InlineData("MGMT")] + public async Task UploadAutoReview_WithValidPackageType_CreatesNewReviewWithCorrectPackageType(string packageTypeValue) { - // Act - test both direct enum parsing and controller logic - var directParseResult = Enum.TryParse(packageTypeString, true, out var directParsedPackageType); - var controllerLogicResult = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var controllerParsedType) ? (PackageType?)controllerParsedType : null; + // Arrange + var mockFile = CreateMockFormFile("test.json", "dummy content"); + var mockCodeFile = new CodeFile() + { + Name = "test", + Language = "C#", + PackageName = "TestPackage", + PackageVersion = "1.0.0" + }; + + var mockApiRevision = new APIRevisionListItemModel() + { + Id = "test-revision-id", + ReviewId = "test-review-id", + Language = "C#", + IsApproved = false + }; + + var expectedPackageType = Enum.Parse(packageTypeValue, true); + + // Setup mocks for new review creation scenario + _mockCodeFileManager.Setup(m => m.CreateCodeFileAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockCodeFile); + + _mockReviewManager.Setup(m => m.GetReviewAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((ReviewListItemModel)null); // No existing review + + _mockReviewManager.Setup(m => m.CreateReviewAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new ReviewListItemModel() + { + Id = "test-review-id", + PackageName = "TestPackage", + Language = "C#", + IsApproved = false, + PackageType = expectedPackageType + }); + + _mockApiRevisionsManager.Setup(m => m.CreateAPIRevisionAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockApiRevision); + + _mockApiRevisionsManager.Setup(m => m.UpdateRevisionMetadataAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockApiRevision); + + _mockConfiguration.Setup(c => c["ReviewUrl"]).Returns("https://test.com"); + + // Act + var result = await _controller.UploadAutoReview(mockFile.Object, "test-label", packageType: packageTypeValue); // Assert - directParseResult.Should().BeTrue(); - directParsedPackageType.Should().Be(expectedPackageType); - controllerLogicResult.Should().NotBeNull(); - controllerLogicResult.Value.Should().Be(expectedPackageType); + result.Should().NotBeNull(); + + // UploadAutoReview returns ObjectResult with 202 status and review URL + var objectResult = result.Should().BeOfType().Which; + objectResult.StatusCode.Should().Be(StatusCodes.Status202Accepted); + objectResult.Value.Should().NotBeNull(); + objectResult.Value.Should().BeOfType().Which.Should().Contain("/Assemblies/Review/"); + + // Verify that CreateReviewAsync was called with the correct parsed PackageType + _mockReviewManager.Verify(m => m.CreateReviewAsync( + "TestPackage", + "C#", + false, + It.Is(pt => pt.HasValue && pt.Value == expectedPackageType)), + Times.Once); } [Theory] @@ -44,94 +136,277 @@ public void PackageTypeEnumParsing_WithValidValues_ParsesCorrectly(string packag [InlineData("")] [InlineData(" ")] [InlineData("invalid")] - [InlineData("unknown")] - public void PackageTypeEnumParsing_WithInvalidValues_ReturnsExpectedResults(string packageTypeString) + public async Task UploadAutoReview_WithInvalidPackageType_CreatesReviewWithNullPackageType(string packageTypeValue) { - // Act - test both direct enum parsing and controller logic - var directParseResult = Enum.TryParse(packageTypeString, true, out var directParsedPackageType); - var controllerLogicResult = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var controllerParsedType) ? (PackageType?)controllerParsedType : null; + // Arrange + var mockFile = CreateMockFormFile("test.json", "dummy content"); + var mockCodeFile = new CodeFile() + { + Name = "test", + Language = "C#", + PackageName = "TestPackage", + PackageVersion = "1.0.0" + }; + + var mockApiRevision = new APIRevisionListItemModel() + { + Id = "test-revision-id", + ReviewId = "test-review-id", + Language = "C#", + IsApproved = false + }; + + // Setup mocks for new review creation scenario + _mockCodeFileManager.Setup(m => m.CreateCodeFileAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockCodeFile); + + _mockReviewManager.Setup(m => m.GetReviewAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((ReviewListItemModel)null); // No existing review + + _mockReviewManager.Setup(m => m.CreateReviewAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new ReviewListItemModel() + { + Id = "test-review-id", + PackageName = "TestPackage", + Language = "C#", + IsApproved = false, + PackageType = null + }); + + _mockApiRevisionsManager.Setup(m => m.CreateAPIRevisionAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockApiRevision); + + _mockApiRevisionsManager.Setup(m => m.UpdateRevisionMetadataAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockApiRevision); + + _mockConfiguration.Setup(c => c["ReviewUrl"]).Returns("https://test.com"); + + // Act + var result = await _controller.UploadAutoReview(mockFile.Object, "test-label", packageType: packageTypeValue); // Assert - directParseResult.Should().BeFalse(); - directParsedPackageType.Should().Be(default(PackageType)); - controllerLogicResult.Should().BeNull(); + result.Should().NotBeNull(); + + // UploadAutoReview returns ObjectResult with 202 status and review URL + var objectResult = result.Should().BeOfType().Which; + objectResult.StatusCode.Should().Be(StatusCodes.Status202Accepted); + objectResult.Value.Should().NotBeNull(); + objectResult.Value.Should().BeOfType(); + + // Verify that CreateReviewAsync was called with null PackageType for invalid values + _mockReviewManager.Verify(m => m.CreateReviewAsync( + "TestPackage", + "C#", + false, + It.Is(pt => !pt.HasValue)), + Times.Once); } [Fact] - public void ReviewUpdate_WithExistingReviewWithoutPackageType_ShouldSetPackageType() + public async Task UploadAutoReview_WithExistingReviewWithoutPackageType_UpdatesReviewPackageType() { - // Arrange - simulate existing review without PackageType + // Arrange + var mockFile = CreateMockFormFile("test.json", "dummy content"); + var mockCodeFile = new CodeFile() + { + Name = "test", + Language = "C#", + PackageName = "TestPackage", + PackageVersion = "1.0.0" + }; + var existingReview = new ReviewListItemModel() { Id = "existing-review-id", PackageName = "TestPackage", Language = "C#", + IsApproved = false, PackageType = null // No package type set initially }; - var packageTypeString = "mgmt"; - var parsedPackageType = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var result) ? (PackageType?)result : null; - - // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync - if (parsedPackageType.HasValue && !existingReview.PackageType.HasValue) + var mockApiRevision = new APIRevisionListItemModel() { - existingReview.PackageType = parsedPackageType; - } + Id = "test-revision-id", + ReviewId = "existing-review-id", + Language = "C#", + IsApproved = false + }; + + // Setup mocks for existing review scenario + _mockCodeFileManager.Setup(m => m.CreateCodeFileAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockCodeFile); + + _mockReviewManager.Setup(m => m.GetReviewAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(existingReview); + + _mockReviewManager.Setup(m => m.UpdateReviewAsync(It.IsAny())) + .ReturnsAsync(existingReview); + + _mockApiRevisionsManager.Setup(m => m.GetAPIRevisionsAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new List()); + + _mockApiRevisionsManager.Setup(m => m.CreateAPIRevisionAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockApiRevision); + + _mockApiRevisionsManager.Setup(m => m.UpdateRevisionMetadataAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockApiRevision); + + _mockConfiguration.Setup(c => c["ReviewUrl"]).Returns("https://test.com"); + + // Act + var result = await _controller.UploadAutoReview(mockFile.Object, "test-label", packageType: "mgmt"); // Assert - existingReview.PackageType.Should().Be(PackageType.mgmt); + result.Should().NotBeNull(); + + // UploadAutoReview returns ObjectResult with 202 status and review URL + var objectResult = result.Should().BeOfType().Which; + objectResult.StatusCode.Should().Be(StatusCodes.Status202Accepted); + objectResult.Value.Should().NotBeNull(); + objectResult.Value.Should().BeOfType(); + + // Verify that UpdateReviewAsync was called (indicating the review was updated) + _mockReviewManager.Verify(m => m.UpdateReviewAsync( + It.Is(r => r.PackageType == PackageType.mgmt)), + Times.Once); } [Fact] - public void ReviewUpdate_WithExistingReviewWithPackageType_ShouldNotOverridePackageType() + public async Task UploadAutoReview_WithExistingReviewWithPackageType_DoesNotOverridePackageType() { - // Arrange - simulate existing review with PackageType already set + // Arrange + var mockFile = CreateMockFormFile("test.json", "dummy content"); + var mockCodeFile = new CodeFile() + { + Name = "test", + Language = "C#", + PackageName = "TestPackage", + PackageVersion = "1.0.0" + }; + var existingReview = new ReviewListItemModel() { Id = "existing-review-id", PackageName = "TestPackage", Language = "C#", + IsApproved = false, PackageType = PackageType.client // Already has package type set }; - var packageTypeString = "mgmt"; - var parsedPackageType = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var result) ? (PackageType?)result : null; - - // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync - if (parsedPackageType.HasValue && !existingReview.PackageType.HasValue) + var mockApiRevision = new APIRevisionListItemModel() { - existingReview.PackageType = parsedPackageType; - } + Id = "test-revision-id", + ReviewId = "existing-review-id", + Language = "C#", + IsApproved = false + }; + + // Setup mocks for existing review scenario + _mockCodeFileManager.Setup(m => m.CreateCodeFileAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockCodeFile); + + _mockReviewManager.Setup(m => m.GetReviewAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(existingReview); + + _mockApiRevisionsManager.Setup(m => m.GetAPIRevisionsAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new List()); + + _mockApiRevisionsManager.Setup(m => m.CreateAPIRevisionAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockApiRevision); - // Assert - PackageType should remain unchanged - existingReview.PackageType.Should().Be(PackageType.client); + _mockApiRevisionsManager.Setup(m => m.UpdateRevisionMetadataAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(mockApiRevision); + + _mockConfiguration.Setup(c => c["ReviewUrl"]).Returns("https://test.com"); + + // Act + var result = await _controller.UploadAutoReview(mockFile.Object, "test-label", packageType: "mgmt"); + + // Assert + result.Should().NotBeNull(); + + // UploadAutoReview returns ObjectResult with 202 status and review URL + var objectResult = result.Should().BeOfType().Which; + objectResult.StatusCode.Should().Be(StatusCodes.Status202Accepted); + objectResult.Value.Should().NotBeNull(); + objectResult.Value.Should().BeOfType(); + + // Verify that UpdateReviewAsync was NOT called since PackageType was already set + _mockReviewManager.Verify(m => m.UpdateReviewAsync(It.IsAny()), Times.Never); } - [Theory] - [InlineData("invalid-package-type")] - [InlineData(null)] - [InlineData("")] - public void ReviewUpdate_WithInvalidOrNullPackageType_ShouldNotSetPackageType(string packageTypeString) + [Fact] + public async Task UploadAutoReview_WithNullFile_ReturnsInternalServerError() { - // Arrange - simulate existing review without PackageType - var existingReview = new ReviewListItemModel() + // Act + var result = await _controller.UploadAutoReview(null, "test-label", packageType: "client"); + + // Assert + result.Should().NotBeNull(); + result.Should().BeOfType().Which.StatusCode.Should().Be(StatusCodes.Status500InternalServerError); + + // Verify no manager methods were called + _mockCodeFileManager.Verify(m => m.CreateCodeFileAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _mockReviewManager.Verify(m => m.CreateReviewAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + + private Mock CreateMockFormFile(string fileName, string content) + { + var mockFile = new Mock(); + var stream = new MemoryStream(); + var writer = new StreamWriter(stream); + writer.Write(content); + writer.Flush(); + stream.Position = 0; + + mockFile.Setup(f => f.OpenReadStream()).Returns(stream); + mockFile.Setup(f => f.FileName).Returns(fileName); + mockFile.Setup(f => f.Length).Returns(stream.Length); + mockFile.Setup(f => f.ContentType).Returns("application/json"); + + return mockFile; + } + + private void SetupControllerContext() + { + // Create a claims principal with the GitHub login claim + var claims = new List { - Id = "existing-review-id", - PackageName = "TestPackage", - Language = "C#", - PackageType = null // No package type set initially + new Claim("urn:github:login", "testuser") }; + var identity = new ClaimsIdentity(claims, "TestAuthType"); + var claimsPrincipal = new ClaimsPrincipal(identity); - var parsedPackageType = !string.IsNullOrEmpty(packageTypeString) && Enum.TryParse(packageTypeString, true, out var result) ? (PackageType?)result : null; + // Set up the HTTP context + var httpContext = new DefaultHttpContext(); + httpContext.User = claimsPrincipal; + + _controller.ControllerContext = new ControllerContext() + { + HttpContext = httpContext + }; + } + + private class MockLanguageService : LanguageService + { + private readonly string _name; + private readonly bool _usesTreeStyleParser; - // Act - simulate the logic from AutoReviewController.CreateAutomaticRevisionAsync - if (parsedPackageType.HasValue && !existingReview.PackageType.HasValue) + public MockLanguageService(string name, bool usesTreeStyleParser) { - existingReview.PackageType = parsedPackageType; + _name = name; + _usesTreeStyleParser = usesTreeStyleParser; } - // Assert - PackageType should remain null due to invalid/null input - existingReview.PackageType.Should().BeNull(); + public override string Name => _name; + public override string[] Extensions => new[] { ".json" }; + public override string VersionString => "1.0"; + public override bool CanUpdate(string versionString) => false; + public override Task GetCodeFileAsync(string originalName, Stream stream, bool runAnalysis) => Task.FromResult(null); + public override bool UsesTreeStyleParser => _usesTreeStyleParser; + public override CodeFile GetReviewGenPendingCodeFile(string fileName) => null; + public override bool GeneratePipelineRunParams(APIRevisionGenerationPipelineParamModel param) => false; + public override bool CanConvert(string versionString) => false; } } } diff --git a/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs b/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs index 7139f6d3589..6888a2ffba2 100644 --- a/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs +++ b/src/dotnet/APIView/APIViewUnitTests/PullRequestsControllerTests.cs @@ -1,63 +1,261 @@ using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using APIViewWeb; +using APIViewWeb.DTOs; +using APIViewWeb.Helpers; +using APIViewWeb.LeanControllers; +using APIViewWeb.Managers; using APIViewWeb.Models; using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; +using Moq; using Xunit; namespace APIViewUnitTests { public class PullRequestsControllerTests { + private readonly Mock> _mockLogger; + private readonly Mock _mockPullRequestManager; + private readonly Mock _mockConfiguration; + private readonly List _languageServices; + private readonly PullRequestsController _controller; + + public PullRequestsControllerTests() + { + _mockLogger = new Mock>(); + _mockPullRequestManager = new Mock(); + _mockConfiguration = new Mock(); + _languageServices = new List(); + + _controller = new PullRequestsController( + _mockLogger.Object, + _mockPullRequestManager.Object, + _mockConfiguration.Object, + _languageServices); + } + [Theory] [InlineData("client")] [InlineData("mgmt")] - [InlineData("CLIENT")] // Test case insensitivity + [InlineData("CLIENT")] [InlineData("MGMT")] + public async Task CreateAPIRevisionIfAPIHasChanges_WithValidPackageType_PassesCorrectValueToManager(string packageTypeValue) + { + // Arrange + _mockPullRequestManager.Setup(m => m.CreateAPIRevisionIfAPIHasChanges( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync("https://test.com/review/test-id"); + + // Setup HTTP context for Request.Host + var httpContext = new DefaultHttpContext(); + httpContext.Request.Host = new HostString("test.com"); + _controller.ControllerContext = new ControllerContext() + { + HttpContext = httpContext + }; + + // Act + var result = await _controller.CreateAPIRevisionIfAPIHasChanges( + buildId: "test-build-id", + artifactName: "test-artifact", + filePath: "test/path", + commitSha: "abc123", + repoName: "test-repo", + packageName: "test-package", + pullRequestNumber: 123, + packageType: packageTypeValue); + + // Assert + result.Should().NotBeNull(); + + // Verify that the manager was called with the exact packageType value passed from controller + _mockPullRequestManager.Verify(m => m.CreateAPIRevisionIfAPIHasChanges( + "test-build-id", + "test-artifact", + "test/path", + "abc123", + "test-repo", + "test-package", + 123, + "test.com", + It.IsAny(), + null, // codeFile + null, // baselineCodeFile + null, // language - actual value from controller + "internal", // default project + packageTypeValue), // packageType should be passed exactly as received + Times.Once); + } + + [Theory] [InlineData(null)] [InlineData("")] [InlineData(" ")] [InlineData("invalid")] [InlineData("unknown")] - public void PackageTypeParameterHandling_ShouldPassThroughToManager(string packageTypeValue) + public async Task CreateAPIRevisionIfAPIHasChanges_WithInvalidPackageType_PassesValueToManager(string packageTypeValue) + { + // Arrange + _mockPullRequestManager.Setup(m => m.CreateAPIRevisionIfAPIHasChanges( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync("https://test.com/review/test-id"); + + // Setup HTTP context for Request.Host + var httpContext = new DefaultHttpContext(); + httpContext.Request.Host = new HostString("test.com"); + _controller.ControllerContext = new ControllerContext() + { + HttpContext = httpContext + }; + + // Act + var result = await _controller.CreateAPIRevisionIfAPIHasChanges( + buildId: "test-build-id", + artifactName: "test-artifact", + filePath: "test/path", + commitSha: "abc123", + repoName: "test-repo", + packageName: "test-package", + pullRequestNumber: 123, + packageType: packageTypeValue); + + // Assert + result.Should().NotBeNull(); + + // Verify that the manager was called with the exact packageType value (even if invalid) + _mockPullRequestManager.Verify(m => m.CreateAPIRevisionIfAPIHasChanges( + "test-build-id", + "test-artifact", + "test/path", + "abc123", + "test-repo", + "test-package", + 123, + "test.com", + It.IsAny(), + null, // codeFile + null, // baselineCodeFile + null, // language - actual value from controller + "internal", // default project + packageTypeValue), // packageType should be passed exactly as received (even invalid values) + Times.Once); + } + + [Fact] + public async Task CreateAPIRevisionIfAPIHasChanges_WhenPackageTypeOmitted_PassesNullToManager() { - // This test verifies that packageType values (valid or invalid) are passed through correctly - // The actual enum parsing and validation happens in the manager layer, not the controller - // Controllers should pass through the string value as-is - - // Act - simulate controller receiving packageType parameter - var receivedPackageType = packageTypeValue; - - // Assert - controller should pass through the value unchanged - receivedPackageType.Should().Be(packageTypeValue); + // Arrange + _mockPullRequestManager.Setup(m => m.CreateAPIRevisionIfAPIHasChanges( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync("https://test.com/review/test-id"); + + // Setup HTTP context for Request.Host + var httpContext = new DefaultHttpContext(); + httpContext.Request.Host = new HostString("test.com"); + _controller.ControllerContext = new ControllerContext() + { + HttpContext = httpContext + }; + + // Act - Not providing packageType parameter to test default behavior + var result = await _controller.CreateAPIRevisionIfAPIHasChanges( + buildId: "test-build-id", + artifactName: "test-artifact", + filePath: "test/path", + commitSha: "abc123", + repoName: "test-repo", + packageName: "test-package", + pullRequestNumber: 123); + // packageType parameter omitted + + // Assert + result.Should().NotBeNull(); + + // Verify that the manager was called with null packageType when omitted + _mockPullRequestManager.Verify(m => m.CreateAPIRevisionIfAPIHasChanges( + "test-build-id", + "test-artifact", + "test/path", + "abc123", + "test-repo", + "test-package", + 123, + "test.com", + It.IsAny(), + null, // codeFile + null, // baselineCodeFile + null, // language - actual value from controller + "internal", // default project + null), // packageType should be null when omitted + Times.Once); } [Fact] - public void PackageTypeParameterHandling_WhenOmitted_ShouldDefaultToNull() + public async Task CreateAPIRevisionIfAPIHasChanges_WhenNoAPIRevisionUrlReturned_ReturnsAlreadyReported() { - // Test verifies that the default value for packageType parameter is null - // when not provided in the request - - // Act - simulate controller method with default parameter value - string packageType = null; // Default value for optional parameter + // Arrange - Manager returns null/empty URL indicating no changes + _mockPullRequestManager.Setup(m => m.CreateAPIRevisionIfAPIHasChanges( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((string)null); // No API revision URL returned + + // Setup HTTP context for Request.Host + var httpContext = new DefaultHttpContext(); + httpContext.Request.Host = new HostString("test.com"); + _controller.ControllerContext = new ControllerContext() + { + HttpContext = httpContext + }; + + // Act + var result = await _controller.CreateAPIRevisionIfAPIHasChanges( + buildId: "test-build-id", + artifactName: "test-artifact", + filePath: "test/path", + commitSha: "abc123", + repoName: "test-repo", + packageName: "test-package", + pullRequestNumber: 123, + packageType: "client"); // Assert - packageType.Should().BeNull(); + result.Should().NotBeNull(); } - [Theory] - [InlineData("client", "Client package type should be supported")] - [InlineData("mgmt", "Management package type should be supported")] - public void PackageTypeController_DocumentedBehavior_ForValidValues(string packageTypeValue, string expectedBehavior) + [Fact] + public async Task GetAssociatedPullRequestsAsync_ReturnsExpectedResult() { - // This test documents the expected behavior for different packageType values - // in the context of the PullRequestsController - + // Arrange + var reviewId = "test-review-id"; + var apiRevisionId = "test-revision-id"; + var expectedPullRequests = new List + { + new PullRequestModel { ReviewId = reviewId, PullRequestNumber = 123 }, + new PullRequestModel { ReviewId = reviewId, PullRequestNumber = 456 } + }; + + _mockPullRequestManager.Setup(m => m.GetPullRequestsModelAsync(reviewId, apiRevisionId)) + .ReturnsAsync(expectedPullRequests); + // Act - var isValidValue = !string.IsNullOrWhiteSpace(packageTypeValue) && - (packageTypeValue.Equals("client", StringComparison.OrdinalIgnoreCase) || - packageTypeValue.Equals("mgmt", StringComparison.OrdinalIgnoreCase)); + var result = await _controller.GetAssociatedPullRequestsAsync(reviewId, apiRevisionId); // Assert - isValidValue.Should().BeTrue(expectedBehavior); + result.Should().NotBeNull(); + + _mockPullRequestManager.Verify(m => m.GetPullRequestsModelAsync(reviewId, apiRevisionId), Times.Once); } } }