[SM-1168] Update access policy selector to disable on submit (#8519)

* Add loading and disabled on all inputs * Add proper spinner and form disable on submit
bitwarden · Apr 29, 2024 · 61d079c · 61d079c
1 parent 20de053
commit 61d079c
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 27 deletions.
diff --git a/...en_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.html b/...en_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.html
@@ -1,4 +1,4 @@
-<form [formGroup]="formGroup" [bitSubmit]="submit">
+<form [formGroup]="formGroup" [bitSubmit]="submit" *ngIf="!loading; else spinner">
   <div class="tw-w-2/5">
     <p class="tw-mt-8" *ngIf="!loading">
       {{ "projectPeopleDescription" | i18n }}
@@ -19,3 +19,9 @@
     </button>
   </div>
 </form>
+
+<ng-template #spinner>
+  <div class="tw-items-center tw-justify-center tw-pt-64 tw-text-center">
+    <i class="bwi bwi-spinner bwi-spin bwi-3x"></i>
+  </div>
+</ng-template>
diff --git a/...rden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts b/...rden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts
@@ -1,7 +1,7 @@
 import { ChangeDetectorRef, Component, OnDestroy, OnInit } from "@angular/core";
 import { FormControl, FormGroup } from "@angular/forms";
 import { ActivatedRoute, Router } from "@angular/router";
-import { combineLatest, Subject, switchMap, takeUntil, catchError, EMPTY } from "rxjs";
+import { combineLatest, Subject, switchMap, takeUntil, catchError } from "rxjs";
 
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@@ -37,11 +37,9 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy {
         return convertToAccessPolicyItemViews(policies);
       }),
     ),
-    catchError(() => {
-      // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-      // eslint-disable-next-line @typescript-eslint/no-floating-promises
-      this.router.navigate(["/sm", this.organizationId, "projects"]);
-      return EMPTY;
+    catchError(async () => {
+      await this.router.navigate(["/sm", this.organizationId, "projects"]);
+      return [];
     }),
   );
 
@@ -99,25 +97,28 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy {
     if (this.formGroup.invalid) {
       return;
     }
+    const formValues = this.formGroup.value.accessPolicies;
+    this.formGroup.disable();
 
     const showAccessRemovalWarning =
       await this.accessPolicySelectorService.showAccessRemovalWarning(
         this.organizationId,
-        this.formGroup.value.accessPolicies,
+        formValues,
       );
 
     if (showAccessRemovalWarning) {
       const confirmed = await this.showWarning();
       if (!confirmed) {
         this.setSelected(this.currentAccessPolicies);
+        this.formGroup.enable();
         return;
       }
     }
 
     try {
       const projectPeopleView = convertToProjectPeopleAccessPoliciesView(
         this.projectId,
-        this.formGroup.value.accessPolicies,
+        formValues,
       );
       const peoplePoliciesViews = await this.accessPolicyService.putProjectPeopleAccessPolicies(
         this.projectId,
@@ -126,9 +127,7 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy {
       this.currentAccessPolicies = convertToAccessPolicyItemViews(peoplePoliciesViews);
 
       if (showAccessRemovalWarning) {
-        // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-        // eslint-disable-next-line @typescript-eslint/no-floating-promises
-        this.router.navigate(["sm", this.organizationId, "projects"]);
+        await this.router.navigate(["sm", this.organizationId, "projects"]);
       }
       this.platformUtilsService.showToast(
         "success",
@@ -139,6 +138,7 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy {
       this.validationService.showError(e);
       this.setSelected(this.currentAccessPolicies);
     }
+    this.formGroup.enable();
   };
 
   private setSelected(policiesToSelect: ApItemViewType[]) {

diff --git a/...web/src/app/secrets-manager/service-accounts/people/service-account-people.component.html b/...web/src/app/secrets-manager/service-accounts/people/service-account-people.component.html
@@ -1,4 +1,4 @@
-<form [formGroup]="formGroup" [bitSubmit]="submit">
+<form [formGroup]="formGroup" [bitSubmit]="submit" *ngIf="!loading; else spinner">
   <div class="tw-w-2/5">
     <p class="tw-mt-8" *ngIf="!loading">
       {{ "machineAccountPeopleDescription" | i18n }}
@@ -20,3 +20,9 @@
     </button>
   </div>
 </form>
+
+<ng-template #spinner>
+  <div class="tw-items-center tw-justify-center tw-pt-64 tw-text-center">
+    <i class="bwi bwi-spinner bwi-spin bwi-3x"></i>
+  </div>
+</ng-template>
diff --git a/...t-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.ts b/...t-web/src/app/secrets-manager/service-accounts/people/service-account-people.component.ts
@@ -1,7 +1,7 @@
 import { ChangeDetectorRef, Component, OnDestroy, OnInit } from "@angular/core";
 import { FormControl, FormGroup } from "@angular/forms";
 import { ActivatedRoute, Router } from "@angular/router";
-import { catchError, combineLatest, EMPTY, Subject, switchMap, takeUntil } from "rxjs";
+import { combineLatest, Subject, switchMap, takeUntil } from "rxjs";
 
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@@ -40,12 +40,6 @@ export class ServiceAccountPeopleComponent implements OnInit, OnDestroy {
           return convertToAccessPolicyItemViews(policies);
         }),
     ),
-    catchError(() => {
-      // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-      // eslint-disable-next-line @typescript-eslint/no-floating-promises
-      this.router.navigate(["/sm", this.organizationId, "machine-accounts"]);
-      return EMPTY;
-    }),
   );
 
   private potentialGrantees$ = combineLatest([this.route.params]).pipe(
@@ -101,29 +95,32 @@ export class ServiceAccountPeopleComponent implements OnInit, OnDestroy {
     if (this.isFormInvalid()) {
       return;
     }
+    const formValues = this.formGroup.value.accessPolicies;
+    this.formGroup.disable();
 
     const showAccessRemovalWarning =
       await this.accessPolicySelectorService.showAccessRemovalWarning(
         this.organizationId,
-        this.formGroup.value.accessPolicies,
+        formValues,
       );
 
     if (
       await this.handleAccessRemovalWarning(showAccessRemovalWarning, this.currentAccessPolicies)
     ) {
+      this.formGroup.enable();
       return;
     }
 
     try {
       const peoplePoliciesViews = await this.updateServiceAccountPeopleAccessPolicies(
         this.serviceAccountId,
-        this.formGroup.value.accessPolicies,
+        formValues,
       );
 
       await this.handleAccessTokenAvailableWarning(
         showAccessRemovalWarning,
         this.currentAccessPolicies,
-        this.formGroup.value.accessPolicies,
+        formValues,
       );
 
       this.currentAccessPolicies = convertToAccessPolicyItemViews(peoplePoliciesViews);
@@ -137,6 +134,7 @@ export class ServiceAccountPeopleComponent implements OnInit, OnDestroy {
       this.validationService.showError(e);
       this.setSelected(this.currentAccessPolicies);
     }
+    this.formGroup.enable();
   };
 
   private setSelected(policiesToSelect: ApItemViewType[]) {
@@ -198,9 +196,7 @@ export class ServiceAccountPeopleComponent implements OnInit, OnDestroy {
     selectedPolicies: ApItemValueType[],
   ): Promise<void> {
     if (showAccessRemovalWarning) {
-      // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-      // eslint-disable-next-line @typescript-eslint/no-floating-promises
-      this.router.navigate(["sm", this.organizationId, "machine-accounts"]);
+      await this.router.navigate(["sm", this.organizationId, "machine-accounts"]);
     } else if (
       this.accessPolicySelectorService.isAccessRemoval(currentAccessPolicies, selectedPolicies)
     ) {

diff --git a/...nager/shared/access-policies/access-policy-selector/access-policy-selector.component.html b/...nager/shared/access-policies/access-policy-selector/access-policy-selector.component.html
@@ -55,6 +55,7 @@
             bitIconButton="bwi-close"
             buttonType="main"
             size="default"
+            [disabled]="disabled"
             [attr.title]="'remove' | i18n"
             [attr.aria-label]="'remove' | i18n"
             (click)="selectionList.deselectItem(item.id); handleBlur()"
@@ -84,7 +85,14 @@
   </bit-form-field>
 
   <div class="tw-ml-3 tw-mt-7 tw-shrink-0">
-    <button type="button" bitButton buttonType="secondary" (click)="addButton()">
+    <button
+      type="button"
+      bitButton
+      buttonType="secondary"
+      [loading]="loading"
+      [disabled]="disabled"
+      (click)="addButton()"
+    >
       {{ "add" | i18n }}
     </button>
   </div>