Refactored Select Reviewers

src/dotnet/APIView/APIViewWeb/LeanControllers/APIRevisionsController.cs

@@ -6,11 +6,8 @@
 using Microsoft.Extensions.Logging;
 using System.Threading.Tasks;
 using APIViewWeb.Managers.Interfaces;
-using System;
 using APIViewWeb.Managers;
-using Microsoft.Azure.Cosmos.Serialization.HybridRow;
 using System.Collections.Generic;
-using System.Linq;
 
 namespace APIViewWeb.LeanControllers
 {
@@ -128,21 +125,9 @@ public async Task<ActionResult<APIRevisionListItemModel>> ToggleReviewApprovalAs
         [HttpPost("{reviewId}/{apiRevisionId}/reviewers", Name = "AddReviewers")]
         public async Task<ActionResult<APIRevisionListItemModel>> AddReviewersAsync(string reviewId, string apiRevisionId, [FromBody] HashSet<string> reviewers)
         {
-            var apiRevision = await _apiRevisionsManager.GetAPIRevisionAsync(apiRevisionId);
-            var existingReviewers = new HashSet<string>(apiRevision.AssignedReviewers.Select(r => r.AssingedTo));
-
-            var newReviewers = new HashSet<string>(reviewers.Except(existingReviewers));
-            var removedReviewers = new HashSet<string>(existingReviewers.Except(reviewers));
+            var apiRevision = await _apiRevisionsManager.UpdateAPIRevisionReviewersAsync(User, apiRevisionId, reviewers);
+            await _notificationManager.NotifyApproversOfReview(User, apiRevisionId, reviewers);
 
-            if (newReviewers.Any())
-            {
-                await _apiRevisionsManager.AssignReviewersToAPIRevisionAsync(User, apiRevisionId, newReviewers);
-                await _notificationManager.NotifyApproversOfReview(User, apiRevisionId, newReviewers);
-            }
-            if (removedReviewers.Any())
-            {
-                await _apiRevisionsManager.RemoveReviewersFromAPIRevisionAsync(User, apiRevisionId, removedReviewers);
-            }
             return new LeanJsonResult(apiRevision, StatusCodes.Status200OK);
         }
     }

src/dotnet/APIView/APIViewWeb/Managers/APIRevisionsManager.cs

@@ -844,24 +844,28 @@ public async Task AssignReviewersToAPIRevisionAsync(ClaimsPrincipal User, string
             await _apiRevisionsRepository.UpsertAPIRevisionAsync(apiRevision);
         }
 
-        /// <summary>
-        /// Remove reviewers from a review
-        /// </summary>
-        /// <param name="User"></param>
-        /// <param name="apiRevisionId"></param>
-        /// <param name="reviewers"></param>
-        /// <returns></returns>
-        public async Task RemoveReviewersFromAPIRevisionAsync(ClaimsPrincipal User, string apiRevisionId, HashSet<string> reviewers)
+        public async Task<APIRevisionListItemModel> UpdateAPIRevisionReviewersAsync(ClaimsPrincipal User, string apiRevisionId, HashSet<string> reviewers)
         {
             APIRevisionListItemModel apiRevision = await _apiRevisionsRepository.GetAPIRevisionAsync(apiRevisionId);
-            var reviewersToRemove = apiRevision.AssignedReviewers.Where(x => reviewers.Contains(x.AssingedTo)).ToList();
-
-            foreach (var reviewAssignment in reviewersToRemove)
+            foreach (var reviewer in reviewers)
+            {
+                if (!apiRevision.AssignedReviewers.Where(x => x.AssingedTo == reviewer).Any())
+                {
+                    var reviewAssignment = new ReviewAssignmentModel()
+                    {
+                        AssingedTo = reviewer,
+                        AssignedBy = User.GetGitHubLogin(),
+                        AssingedOn = DateTime.Now,
+                    };
+                    apiRevision.AssignedReviewers.Add(reviewAssignment);
+                }
+            }
+            foreach (var assignment in apiRevision.AssignedReviewers.FindAll(x => !reviewers.Contains(x.AssingedTo)))
             {
-                apiRevision.AssignedReviewers.Remove(reviewAssignment);
+                apiRevision.AssignedReviewers.Remove(assignment);
             }
-
             await _apiRevisionsRepository.UpsertAPIRevisionAsync(apiRevision);
+            return apiRevision;
         }
 
         /// <summary>

src/dotnet/APIView/APIViewWeb/Managers/Interfaces/IAPIRevisionsManager.cs

@@ -45,6 +45,6 @@ public Task<APIRevisionListItemModel> CreateAPIRevisionAsync(string userName, st
         public Task<IEnumerable<APIRevisionListItemModel>> GetAPIRevisionsAssignedToUser(string userName);
         public Task<APIRevisionListItemModel> UpdateRevisionMetadataAsync(APIRevisionListItemModel revision, string packageVersion, string label, bool setReleaseTag = false);
         public Task<IEnumerable<string>> GetReviewIdsOfLanguageCorrespondingReviewAsync(string crossLanguagePackageId);
-        public Task RemoveReviewersFromAPIRevisionAsync(ClaimsPrincipal User, string apiRevisionId, HashSet<string> reviewers);
+        public Task<APIRevisionListItemModel> UpdateAPIRevisionReviewersAsync(ClaimsPrincipal User, string apiRevisionId, HashSet<string> reviewers);
     }
 }

src/dotnet/APIView/ClientSPA/src/app/_components/review-page-options/review-page-options.component.html

@@ -108,8 +108,7 @@
                 placeholder="Request Reviewers"
                 [showClear]="true"
                 [style]="{'width':'100%'}"
-                (onPanelHide)="handleOnPanelHide()"
-                (onPanelShow)="handleOnPanelShow()">
+                (onPanelHide)="handleAssignedReviewersChange()">
                     <ng-template pTemplate="selectedItems">
                         <div *ngIf="selectedApprovers && selectedApprovers.length > 0" class="inline-flex align-items-center gap-2 px-1 me-1">
                         <span>{{ formatSelectedApprovers(selectedApprovers) }}</span>

src/dotnet/APIView/ClientSPA/src/app/_components/review-page-options/review-page-options.component.ts

@@ -7,8 +7,8 @@ import { UserProfile } from 'src/app/_models/auth_service_models';
 import { Review } from 'src/app/_models/review';
 import { APIRevision } from 'src/app/_models/revision';
 import { ConfigService } from 'src/app/_services/config/config.service';
-import { CookieService } from 'ngx-cookie-service';
 import { RevisionsService } from 'src/app/_services/revisions/revisions.service';
+import { pipe, take } from 'rxjs';
 
 @Component({
   selector: 'app-review-page-options',
@@ -22,17 +22,10 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
   @Input() review : Review | undefined = undefined;
   @Input() activeAPIRevision : APIRevision | undefined = undefined;
   @Input() diffAPIRevision : APIRevision | undefined = undefined;
-<<<<<<< HEAD
-  @Input() preferedApprovers: string[] = [];
-=======
   @Input() preferredApprovers: string[] = [];
-  @Input() conversiationInfo : any | undefined = undefined;
->>>>>>> 731fd433d (Select Reviewers functional and updating DB)
   @Input() hasFatalDiagnostics : boolean = false;
   @Input() hasActiveConversation : boolean = false;
   @Input() hasHiddenAPIs : boolean = false;
-  @Input() reviewId: string | undefined;
-  @Input() apiRevisionId: string | undefined;
 
   @Output() diffStyleEmitter : EventEmitter<string> = new EventEmitter<string>();
   @Output() showCommentsEmitter : EventEmitter<boolean> = new EventEmitter<boolean>();
@@ -70,7 +63,6 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
 
   //Approvers Options
   selectedApprovers: string[] = [];
-  initialSelectedApprovers: string[] = [];
 
   diffStyleOptions : any[] = [
     { label: 'Full Diff', value: "full" },
@@ -91,7 +83,6 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
     private configService: ConfigService, 
     private route: ActivatedRoute, 
     private router: Router, 
-    private cookieService: CookieService, 
     private apiRevisionsService: RevisionsService) { }
 
   ngOnInit() {
@@ -112,11 +103,7 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
       this.showLineNumbersSwitch = true;
     }
 
-    const selectedApproversCookie = this.cookieService.get('selectedApprovers');
-    if (selectedApproversCookie) {
-      this.selectedApprovers = JSON.parse(selectedApproversCookie);
-    }
-
+    this.activeAPIRevision?.assignedReviewers.map(revision => this.selectedApprovers.push(revision.assingedTo));
     this.setAPIRevisionApprovalStates();
     this.setReviewApprovalStatus();
   }
@@ -227,30 +214,21 @@ export class ReviewPageOptionsComponent implements OnInit, OnChanges{
     this.showHiddenAPIEmitter.emit(event.checked);
   }
 
-  handleOnPanelShow() {
-    this.initialSelectedApprovers = [...this.selectedApprovers];
-  }
 
-  handleOnPanelHide() {
-    if (!this.reviewId || !this.apiRevisionId) {
-      return;
-    }
+  handleAssignedReviewersChange() {
 
-    const { isSelectedApproversChanged, currentApproversSet } = this.hasSelectedApproversChanged();
+    const existingApprovers = new Set(this.activeAPIRevision!.assignedReviewers.map(reviewer => reviewer.assingedTo));
+    const currentApprovers = new Set(this.selectedApprovers);
+    const isSelectedApproversChanged = existingApprovers.size !== currentApprovers.size ||
+                      [...existingApprovers].some(approver => !currentApprovers.has(approver));
 
-    if (!isSelectedApproversChanged) {
-      return;
+    if (isSelectedApproversChanged) {
+      this.apiRevisionsService.updateSelectedReviewers(this.activeAPIRevision!.reviewId, this.activeAPIRevision!.id, currentApprovers).pipe(take(1)).subscribe({
+        next: (response: APIRevision) => {
+          this.activeAPIRevision = response;
+          }
+      });
     }
-    this.apiRevisionsService.updateSelectedReviewers(this.reviewId, this.apiRevisionId, currentApproversSet).subscribe();
-    this.cookieService.set('selectedApprovers', JSON.stringify(this.selectedApprovers));
-  }
-
-  hasSelectedApproversChanged() {
-    const currentApproversSet = new Set(this.selectedApprovers);
-    const initialApproversSet = new Set(this.initialSelectedApprovers);
-    const isSelectedApproversChanged = this.selectedApprovers.length !== this.initialSelectedApprovers.length ||
-                      [...currentApproversSet].some(approver => !initialApproversSet.has(approver));
-    return { isSelectedApproversChanged, currentApproversSet };
   }
 
   formatSelectedApprovers(approvers: string[]): string {

src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.html

@@ -47,12 +47,10 @@
                             [review]="review"
                             [activeAPIRevision]="activeAPIRevision"
                             [diffAPIRevision]="diffAPIRevision"
-                            [preferedApprovers]="preferedApprovers"
+                            [preferredApprovers]="preferredApprovers"
                             [hasFatalDiagnostics]="hasFatalDiagnostics"
                             [hasActiveConversation]="hasActiveConversation"
                             [hasHiddenAPIs]="hasHiddenAPIs"
-                            [reviewId]="reviewId ?? undefined"
-                            [apiRevisionId]="activeApiRevisionId ?? undefined"
                             (showSystemCommentsEmitter)="handleShowSystemCommentsEmitter($event)"
                             (showDocumentationEmitter)="handleShowDocumentationEmitter($event)"
                             (showCommentsEmitter)="handleShowCommentsEmitter($event)"

src/dotnet/APIView/ClientSPA/src/app/_components/review-page/review-page.component.ts

@@ -41,7 +41,6 @@ export class ReviewPageComponent implements OnInit {
   scrollToNodeId : string | undefined = undefined;
   showLineNumbers : boolean = true;
   preferredApprovers : string[] = [];
-  conversiationInfo : any | undefined = undefined;
   hasFatalDiagnostics : boolean = false;
   hasActiveConversation : boolean = false;
   hasHiddenAPIs : boolean = false;