Refactor issue actions and conditions for clarity and maintainability

The deleteIssue and undeleteIssue methods share highly similar logic
that performs the same actions in opposite directions.

This causes code duplication and makes future maintenance more 
error-prone.

Let's,
* extract the common logic into a reusable function to eliminate
duplication and improve clarity.
* merge the two separate state-tracking lists — 
issuesPendingDeletion and issuesPendingRestore into a 
single list issuesPendingAction.
* move complex conditional logic in issue-tables.component.html 
into issue-tables.component.ts.

This makes the template easier to read, improves maintainability,
and reduce duplicated code for the same actions in reverse.

Author: TobyCyan

src/app/shared/issue-tables/issue-tables.component.html

@@ -207,7 +207,7 @@
         <mat-icon>open_in_new</mat-icon>
       </button>
       <button
-        *ngIf="this.isResponseEditable() && !issue.status && this.isActionVisible(action_buttons.RESPOND_TO_ISSUE); else tryEditIssue"
+        *ngIf="shouldEnableRespondToIssue(issue); else tryEditIssue"
         [routerLink]="'issues/' + issue.id"
         mat-button
         color="accent"
@@ -219,7 +219,7 @@
       </button>
       <ng-template #tryEditIssue>
         <button
-          *ngIf="permissions.isIssueEditable() && this.isActionVisible(action_buttons.FIX_ISSUE)"
+          *ngIf="shouldEnableEditIssue()"
           mat-button
           color="accent"
           style="transform: scale(0.8)"
@@ -230,7 +230,7 @@
         </button>
       </ng-template>
       <button
-        *ngIf="this.isResponseEditable() && issue.status && this.isActionVisible(action_buttons.MARK_AS_RESPONDED)"
+        *ngIf="shouldEnableMarkAsResponded(issue)"
         mat-button
         color="primary"
         (click)="markAsResponded(issue, $event)"
@@ -246,19 +246,16 @@
         mat-button
         (click)="markAsPending(issue, $event)"
         style="transform: scale(0.8)"
-        *ngIf="
-          (userService.currentUser.role === 'Student' || userService.currentUser.role === 'Admin') &&
-          this.isActionVisible(action_buttons.MARK_AS_PENDING)
-        "
+        *ngIf="shouldEnablePendingButton()"
         data-testid="mark_pending_button"
       >
         <mat-icon>cancel</mat-icon>
       </button>
       <button
         mat-button
         color="warn"
-        *ngIf="permissions.isIssueDeletable() && !issuesPendingDeletion[issue.id] && this.isActionVisible(action_buttons.DELETE_ISSUE)"
-        (click)="deleteIssue(issue.id, $event); $event.stopPropagation()"
+        *ngIf="isActionPerformAllowed(true, issue.id)"
+        (click)="deleteOrRestoreIssue(true, issue.id, $event)"
         matTooltip="Delete this issue"
         style="transform: scale(0.8)"
         data-testid="delete_issue_button"
@@ -268,8 +265,8 @@
       <button
         mat-button
         color="primary"
-        *ngIf="permissions.isIssueRestorable() && !issuesPendingRestore[issue.id] && this.isActionVisible(action_buttons.RESTORE_ISSUE)"
-        (click)="undeleteIssue(issue.id, $event); $event.stopPropagation()"
+        *ngIf="isActionPerformAllowed(false, issue.id)"
+        (click)="deleteOrRestoreIssue(false, issue.id, $event)"
         matTooltip="Restore this issue"
         style="transform: scale(0.8)"
       >
@@ -279,10 +276,7 @@
         color="warn"
         [diameter]="25"
         style="display: inline; padding-right: 30px; margin-left: 5px"
-        *ngIf="
-          (issuesPendingDeletion[issue.id] && this.isActionVisible(action_buttons.DELETE_ISSUE)) ||
-          (issuesPendingRestore[issue.id] && this.isActionVisible(action_buttons.RESTORE_ISSUE))
-        "
+        *ngIf="shouldEnablePendingActionSpinner(issue.id)"
       ></mat-spinner>
     </mat-cell>
   </ng-container>

src/app/shared/issue-tables/issue-tables.component.ts

@@ -16,6 +16,7 @@ import { PhaseService } from '../../core/services/phase.service';
 import { UserService } from '../../core/services/user.service';
 import { UndoActionComponent } from '../../shared/action-toasters/undo-action/undo-action.component';
 import { IssuesDataTable } from './IssuesDataTable';
+import { Observable } from 'rxjs';
 
 export enum ACTION_BUTTONS {
   VIEW_IN_WEB,
@@ -44,8 +45,7 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
   @ViewChild(MatPaginator, { static: true }) paginator: MatPaginator;
 
   issues: IssuesDataTable;
-  issuesPendingDeletion: { [id: number]: boolean };
-  issuesPendingRestore: { [id: number]: boolean };
+  issuesPendingAction: { [id: number]: boolean };
 
   public tableSettings: TableSettings;
 
@@ -66,8 +66,7 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
 
   ngOnInit() {
     this.issues = new IssuesDataTable(this.issueService, this.sort, this.paginator, this.headers, this.filters);
-    this.issuesPendingDeletion = {};
-    this.issuesPendingRestore = {};
+    this.issuesPendingAction = {};
     this.tableSettings = this.issueTableSettingsService.getTableSettings(this.table_name);
   }
 
@@ -158,69 +157,85 @@ export class IssueTablesComponent implements OnInit, AfterViewInit {
     this.githubService.viewIssueInBrowser(id, event);
   }
 
-  private handleIssueDeletionSuccess(id: number, event: Event, actionUndoable: boolean) {
-    if (!actionUndoable) {
-      return;
-    }
-    let snackBarRef = null;
-    snackBarRef = this.snackBar.openFromComponent(UndoActionComponent, {
-      data: { message: `Deleted issue ${id}` },
-      duration: this.snackBarAutoCloseTime
-    });
-    snackBarRef.onAction().subscribe(() => {
-      this.undeleteIssue(id, event, false);
-    });
-  }
+  deleteOrRestoreIssue(isDeleteAction: boolean, id: number, event: Event, actionUndoable: boolean = true) {
+    const deletingKeyword = 'Deleting';
+    const undeletingKeyword = 'Undeleting';
+    this.logger.info(`IssueTablesComponent: ${isDeleteAction ? deletingKeyword : undeletingKeyword} Issue ${id}`);
 
-  deleteIssue(id: number, event: Event, actionUndoable: boolean = true) {
-    this.logger.info(`IssueTablesComponent: Deleting Issue ${id}`);
+    this.issuesPendingAction = { ...this.issuesPendingAction, [id]: true };
 
-    this.issuesPendingDeletion = { ...this.issuesPendingDeletion, [id]: true };
-    this.issueService
-      .deleteIssue(id)
+    let observableActionedIssue: Observable<Issue>;
+    if (isDeleteAction) {
+      observableActionedIssue = this.issueService.deleteIssue(id);
+    } else {
+      observableActionedIssue = this.issueService.undeleteIssue(id);
+    }
+    observableActionedIssue
       .pipe(
         finalize(() => {
-          const { [id]: issueRemoved, ...theRest } = this.issuesPendingDeletion;
-          this.issuesPendingDeletion = theRest;
+          const { [id]: issueDeletedOrRestored, ...theRest } = this.issuesPendingAction;
+          this.issuesPendingAction = theRest;
         })
       )
       .subscribe(
-        (removedIssue) => this.handleIssueDeletionSuccess(id, event, actionUndoable),
+        () => this.handleIssueActionPerformedSuccess(isDeleteAction, id, event, actionUndoable),
         (error) => this.errorHandlingService.handleError(error)
       );
     event.stopPropagation();
   }
 
-  private handleIssueRestorationSuccess(id: number, event: Event, actionUndoable: boolean) {
+  private handleIssueActionPerformedSuccess(isDeleteAction: boolean, id: number, event: Event, actionUndoable: boolean) {
+    const deletedKeyword = 'Deleted';
+    const restoredKeyword = 'Restored';
     if (!actionUndoable) {
       return;
     }
     let snackBarRef = null;
     snackBarRef = this.snackBar.openFromComponent(UndoActionComponent, {
-      data: { message: `Restored issue ${id}` },
+      data: { message: `${isDeleteAction ? deletedKeyword : restoredKeyword} issue ${id}` },
       duration: this.snackBarAutoCloseTime
     });
     snackBarRef.onAction().subscribe(() => {
-      this.deleteIssue(id, event, false);
+      this.deleteOrRestoreIssue(!isDeleteAction, id, event, false);
     });
   }
 
-  undeleteIssue(id: number, event: Event, actionUndoable: boolean = true) {
-    this.logger.info(`IssueTablesComponent: Undeleting Issue ${id}`);
+  isActionPerformAllowed(isDeleteAction: boolean, id: number) {
+    const actionButton = isDeleteAction ? this.action_buttons.DELETE_ISSUE : this.action_buttons.RESTORE_ISSUE;
+    const isPermissionGranted = this.isIssueActionPermitted(isDeleteAction);
+    return isPermissionGranted && !this.issuesPendingAction[id] && this.isActionVisible(actionButton);
+  }
 
-    this.issuesPendingRestore = { ...this.issuesPendingRestore, [id]: true };
-    this.issueService
-      .undeleteIssue(id)
-      .pipe(
-        finalize(() => {
-          const { [id]: issueRestored, ...theRest } = this.issuesPendingRestore;
-          this.issuesPendingRestore = theRest;
-        })
-      )
-      .subscribe(
-        (restoredIssue) => this.handleIssueRestorationSuccess(id, event, actionUndoable),
-        (error) => this.errorHandlingService.handleError(error)
-      );
-    event.stopPropagation();
+  private isIssueActionPermitted(isDeleteAction: boolean) {
+    if (isDeleteAction) {
+      return this.permissions.isIssueDeletable();
+    }
+    return this.permissions.isIssueRestorable();
+  }
+
+  shouldEnablePendingButton() {
+    return (
+      (this.userService.currentUser.role === 'Student' || this.userService.currentUser.role === 'Admin') &&
+      this.isActionVisible(this.action_buttons.MARK_AS_PENDING)
+    );
+  }
+
+  shouldEnablePendingActionSpinner(id: number) {
+    return (
+      this.issuesPendingAction[id] &&
+      (this.isActionVisible(this.action_buttons.DELETE_ISSUE) || this.isActionVisible(this.action_buttons.RESTORE_ISSUE))
+    );
+  }
+
+  shouldEnableRespondToIssue(issue: Issue) {
+    return this.isResponseEditable() && !issue.status && this.isActionVisible(this.action_buttons.RESPOND_TO_ISSUE);
+  }
+
+  shouldEnableMarkAsResponded(issue: Issue) {
+    return this.isResponseEditable() && issue.status && this.isActionVisible(this.action_buttons.MARK_AS_RESPONDED);
+  }
+
+  shouldEnableEditIssue() {
+    return this.permissions.isIssueEditable() && this.isActionVisible(this.action_buttons.FIX_ISSUE);
   }
 }