Closes kadai-io#29 - Save access items when saving a workbasket, chec…

…k if accessId is empty
CRoberto1926 · Oct 9, 2024 · 72b0850 · 72b0850
1 parent 03b16f6
commit 72b0850
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 28 deletions.
diff --git a/lib/kadai-core/src/main/java/io/kadai/workbasket/internal/WorkbasketServiceImpl.java b/lib/kadai-core/src/main/java/io/kadai/workbasket/internal/WorkbasketServiceImpl.java
@@ -1028,15 +1028,25 @@ private Set<WorkbasketAccessItemImpl> checkAccessItemsPreconditionsAndSetId(
                       + "- the WBID does not match. Target-WBID=''%s'' WorkbasketAccessItem=%s",
                   workbasketId, workbasketAccessItem));
         }
+
+        String accessId = wbAccessItemImpl.getAccessId();
+        if (accessId == null || accessId.isBlank()) {
+          throw new InvalidArgumentException(
+              String.format(
+                  "Checking the preconditions of the current WorkbasketAccessItem failed "
+                      + "- accessId is null or empty. WorkbasketAccessItem=%s, accessId=%s",
+                  workbasketAccessItem, accessId));
+        }
+
         if (wbAccessItemImpl.getId() == null || wbAccessItemImpl.getId().isEmpty()) {
           wbAccessItemImpl.setId(
               IdGenerator.generateWithPrefix(IdGenerator.ID_PREFIX_WORKBASKET_AUTHORIZATION));
         }
-        if (ids.contains(wbAccessItemImpl.getAccessId())) {
+        if (ids.contains(accessId)) {
           throw new WorkbasketAccessItemAlreadyExistException(
-              wbAccessItemImpl.getAccessId(), wbAccessItemImpl.getWorkbasketId());
+              accessId, wbAccessItemImpl.getWorkbasketId());
         }
-        ids.add(wbAccessItemImpl.getAccessId());
+        ids.add(accessId);
         accessItems.add(wbAccessItemImpl);
       }
     }

diff --git a/...re/src/test/java/acceptance/workbasket/update/UpdateWorkbasketAuthorizations2AccTest.java b/...re/src/test/java/acceptance/workbasket/update/UpdateWorkbasketAuthorizations2AccTest.java
@@ -19,12 +19,15 @@
 package acceptance.workbasket.update;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import acceptance.AbstractAccTest;
+import io.kadai.common.api.exceptions.InvalidArgumentException;
 import io.kadai.common.test.security.JaasExtension;
 import io.kadai.common.test.security.WithAccessId;
 import io.kadai.workbasket.api.WorkbasketService;
 import io.kadai.workbasket.api.models.WorkbasketAccessItem;
+import io.kadai.workbasket.internal.models.WorkbasketAccessItemImpl;
 import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -48,4 +51,36 @@ void testUpdatedAccessItemListToEmptyList() throws Exception {
         WORKBASKET_SERVICE.getWorkbasketAccessItems(wbId);
     assertThat(updatedAccessItems).isEmpty();
   }
+
+  @WithAccessId(user = "businessadmin")
+  @Test
+  void testUpdatedAccessItemList_accessId_null() throws Exception {
+    final String wbId = "WBI:100000000000000000000000000000000002";
+    List<WorkbasketAccessItem> accessItems = WORKBASKET_SERVICE.getWorkbasketAccessItems(wbId);
+    assertThat(accessItems).hasSize(1);
+
+    WorkbasketAccessItemImpl workbasketAccessItem = ((WorkbasketAccessItemImpl) accessItems.get(0));
+    workbasketAccessItem.setAccessId(null);
+
+    assertThatThrownBy(
+            () -> WORKBASKET_SERVICE.setWorkbasketAccessItems(wbId, List.of(workbasketAccessItem)))
+        .isInstanceOf(InvalidArgumentException.class)
+        .hasMessageContaining("accessId is null or empty");
+  }
+
+  @WithAccessId(user = "businessadmin")
+  @Test
+  void testUpdatedAccessItemList_accessId_blank() throws Exception {
+    final String wbId = "WBI:100000000000000000000000000000000003";
+    List<WorkbasketAccessItem> accessItems = WORKBASKET_SERVICE.getWorkbasketAccessItems(wbId);
+    assertThat(accessItems).hasSize(1);
+
+    WorkbasketAccessItemImpl workbasketAccessItem = ((WorkbasketAccessItemImpl) accessItems.get(0));
+    workbasketAccessItem.setAccessId("   ");
+
+    assertThatThrownBy(
+            () -> WORKBASKET_SERVICE.setWorkbasketAccessItems(wbId, List.of(workbasketAccessItem)))
+        .isInstanceOf(InvalidArgumentException.class)
+        .hasMessageContaining("accessId is null or empty");
+  }
 }
diff --git a/...pp/administration/components/workbasket-access-items/workbasket-access-items.component.ts b/...pp/administration/components/workbasket-access-items/workbasket-access-items.component.ts
@@ -62,14 +62,11 @@ import { ButtonAction } from '../../models/button-action';
   styleUrls: ['./workbasket-access-items.component.scss']
 })
 export class WorkbasketAccessItemsComponent implements OnInit, OnChanges, OnDestroy, AfterViewInit, AfterViewChecked {
-  @Input()
-  workbasket: Workbasket;
+  @Input() workbasket: Workbasket;
 
-  @Input()
-  expanded: boolean;
+  @Input() expanded: boolean;
 
-  @ViewChildren('htmlInputElement')
-  inputs: QueryList<ElementRef>;
+  @ViewChildren('htmlInputElement') inputs: QueryList<ElementRef>;
 
   selectedRows: number[] = [];
   workbasketClone: Workbasket;
@@ -90,20 +87,17 @@ export class WorkbasketAccessItemsComponent implements OnInit, OnChanges, OnDest
   isAccessItemsTabSelected = false;
   destroy$ = new Subject<void>();
 
-  @Select(WorkbasketSelectors.selectedWorkbasket)
-  selectedWorkbasket$: Observable<Workbasket>;
+  @Select(WorkbasketSelectors.selectedWorkbasket) selectedWorkbasket$: Observable<Workbasket>;
 
   @Select(EngineConfigurationSelectors.accessItemsCustomisation)
   accessItemsCustomization$: Observable<AccessItemsCustomisation>;
 
   @Select(WorkbasketSelectors.workbasketAccessItems)
   accessItemsRepresentation$: Observable<WorkbasketAccessItemsRepresentation>;
 
-  @Select(WorkbasketSelectors.buttonAction)
-  buttonAction$: Observable<ButtonAction>;
+  @Select(WorkbasketSelectors.buttonAction) buttonAction$: Observable<ButtonAction>;
 
-  @Select(WorkbasketSelectors.selectedComponent)
-  selectedComponent$: Observable<WorkbasketComponent>;
+  @Select(WorkbasketSelectors.selectedComponent) selectedComponent$: Observable<WorkbasketComponent>;
 
   constructor(
     private requestInProgressService: RequestInProgressService,
@@ -175,13 +169,7 @@ export class WorkbasketAccessItemsComponent implements OnInit, OnChanges, OnDest
       this.selectedWorkbasket$.pipe(take(1)).subscribe((workbasket) => {
         this.accessItemsRepresentation._links = { self: { href: workbasket._links.accessItems.href } };
         this.setWorkbasketIdForCopy(workbasket.workbasketId);
-        this.formsValidatorService
-          .validateFormAccess(this.accessItemsGroups, this.toggleValidationAccessIdMap)
-          .then((value) => {
-            if (value) {
-              this.onSave();
-            }
-          });
+        this.onSubmit();
       });
     });
 
@@ -314,13 +302,15 @@ export class WorkbasketAccessItemsComponent implements OnInit, OnChanges, OnDest
 
   onSubmit() {
     this.formsValidatorService.formSubmitAttempt = true;
-    this.formsValidatorService
+
+    const shouldSaveWorkbasket = this.formsValidatorService
       .validateFormAccess(this.accessItemsGroups, this.toggleValidationAccessIdMap)
-      .then((value) => {
-        if (value) {
-          this.onSave();
-        }
-      });
+      .then((isFormValid) => isFormValid)
+      .catch(() => false);
+
+    if (shouldSaveWorkbasket) {
+      this.onSave();
+    }
   }
 
   onSave() {