TSK-1029: Proper distinction between delete and markForFeletion for

workbaskets

lib/taskana-core/src/main/java/pro/taskana/workbasket/api/WorkbasketService.java

@@ -292,7 +292,8 @@ public interface WorkbasketService {
    * Deletes the workbasket by the given ID of it.
    *
    * @param workbasketId Id of the workbasket which should be deleted.
-   * @return true if the workbasket is marked for deletion. False in another case.
+   * @return true if the workbasket was deleted successfully. 
+   *         false if the workbasket is marked for deletion.
    * @throws NotAuthorizedException if the current user got no permissions for this interaction.
    * @throws WorkbasketNotFoundException if the workbasket does not exist.
    * @throws WorkbasketInUseException if the workbasket does contain task-content.

rest/taskana-rest-spring/src/main/java/pro/taskana/rest/WorkbasketController.java

@@ -140,14 +140,24 @@ public class WorkbasketController extends AbstractPagingController {
 
   @DeleteMapping(path = Mapping.URL_WORKBASKET_ID)
   @Transactional(rollbackFor = Exception.class, noRollbackFor = WorkbasketNotFoundException.class)
-  public ResponseEntity<?> markWorkbasketForDeletion(
+  public ResponseEntity<?> deleteWorkbasket(
       @PathVariable(value = "workbasketId") String workbasketId)
       throws NotAuthorizedException, InvalidArgumentException, WorkbasketNotFoundException,
           WorkbasketInUseException {
     LOGGER.debug("Entry to markWorkbasketForDeletion(workbasketId= {})", workbasketId);
-    // http status code accepted because workbaskets will not be deleted immediately
+    ResponseEntity<?> response;
-    ResponseEntity<?> response =
+
-        ResponseEntity.accepted().body(workbasketService.deleteWorkbasket(workbasketId));
+    boolean workbasketDeleted = workbasketService.deleteWorkbasket(workbasketId);
+
+    if (workbasketDeleted) {
+      LOGGER.debug("Workbasket successfully deleted.");
+      response = ResponseEntity.noContent().build();
+    } else {
+      LOGGER.debug(
+          "Workbasket was only marked for deletion and will be physically deleted later on.");
+      response = ResponseEntity.accepted().build();
+    }
+
     LOGGER.debug("Exit from markWorkbasketForDeletion(), returning {}", response);
     return response;
   }
@@ -203,7 +213,8 @@ public class WorkbasketController extends AbstractPagingController {
     return result;
   }
 
-  @GetMapping(path = Mapping.URL_WORKBASKET_ID_ACCESSITEMS, 
+  @GetMapping(
+      path = Mapping.URL_WORKBASKET_ID_ACCESSITEMS,
       produces = MediaTypes.HAL_JSON_UTF8_VALUE)
   @Transactional(readOnly = true, rollbackFor = Exception.class)
   public ResponseEntity<WorkbasketAccessItemListResource> getWorkbasketAccessItems(
@@ -253,7 +264,8 @@ public class WorkbasketController extends AbstractPagingController {
     return response;
   }
 
-  @GetMapping(path = Mapping.URL_WORKBASKET_ID_DISTRIBUTION, 
+  @GetMapping(
+      path = Mapping.URL_WORKBASKET_ID_DISTRIBUTION,
       produces = MediaTypes.HAL_JSON_UTF8_VALUE)
   @Transactional(readOnly = true, rollbackFor = Exception.class)
   public ResponseEntity<DistributionTargetListResource> getDistributionTargets(

rest/taskana-rest-spring/src/test/java/pro/taskana/doc/api/WorkbasketControllerRestDocumentation.java

@@ -483,15 +483,15 @@ class WorkbasketControllerRestDocumentation extends BaseRestDocumentation {
   }
 
   @Test
-  void markWorkbasketForDeletionDocTest() throws Exception {
+  void deleteWorkbasketDocTest() throws Exception {
     this.mockMvc
         .perform(
             RestDocumentationRequestBuilders.delete(
                     restHelper.toUrl(
-                        Mapping.URL_WORKBASKET_ID, "WBI:100000000000000000000000000000000005"))
+                        Mapping.URL_WORKBASKET_ID, "WBI:100000000000000000000000000000000008"))
                 .header("Authorization", "Basic YWRtaW46YWRtaW4="))
-        .andExpect(MockMvcResultMatchers.status().isAccepted())
+        .andExpect(MockMvcResultMatchers.status().isNoContent())
-        .andDo(MockMvcRestDocumentation.document("MarkWorkbasketForDeletionDocTest"));
+        .andDo(MockMvcRestDocumentation.document("DeleteWorkbasketDocTest"));
   }
 
   @Test

rest/taskana-rest-spring/src/test/java/pro/taskana/rest/WorkbasketControllerIntTest.java

@@ -90,10 +90,9 @@ class WorkbasketControllerIntTest {
 
   @Test
   void testThrowsExceptionIfInvalidFilterIsUsed() {
-    assertThatThrownBy(
+    assertThatThrownBy(() ->
-        () ->
+        template.exchange(
-            template.exchange(
+            restHelper.toUrl(Mapping.URL_WORKBASKET) + "?invalid=PERSONAL",
-                restHelper.toUrl(Mapping.URL_WORKBASKET) + "?invalid=PERSONAL",
                 HttpMethod.GET,
                 restHelper.defaultRequest(),
                 ParameterizedTypeReference.forType(WorkbasketSummaryListResource.class)))
@@ -126,14 +125,13 @@ class WorkbasketControllerIntTest {
     workbasketResource.setOwner("Joerg");
     workbasketResource.setModified(String.valueOf(Instant.now()));
 
-    assertThatThrownBy(
+    assertThatThrownBy(() ->
-        () ->
+        template.exchange(
-                template.exchange(
+            restHelper.toUrl(Mapping.URL_WORKBASKET_ID, workbasketId),
-                    restHelper.toUrl(Mapping.URL_WORKBASKET_ID, workbasketId),
+                HttpMethod.PUT,
-                    HttpMethod.PUT,
+                new HttpEntity<>(
-                    new HttpEntity<>(
+                    mapper.writeValueAsString(workbasketResource), restHelper.getHeaders()),
-                        mapper.writeValueAsString(workbasketResource), restHelper.getHeaders()),
+                ParameterizedTypeReference.forType(WorkbasketResource.class)))
-                    ParameterizedTypeReference.forType(WorkbasketResource.class)))
         .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode())
         .isEqualTo(HttpStatus.CONFLICT);
   }
@@ -143,13 +141,12 @@ class WorkbasketControllerIntTest {
 
     String workbasketId = "WBI:100004857400039500000999999999999999";
 
-    assertThatThrownBy(
+    assertThatThrownBy(() ->
-        () ->
+        template.exchange(
-                template.exchange(
+            restHelper.toUrl(Mapping.URL_WORKBASKET_ID, workbasketId),
-                    restHelper.toUrl(Mapping.URL_WORKBASKET_ID, workbasketId),
+                HttpMethod.GET,
-                    HttpMethod.GET,
+                new HttpEntity<String>(restHelper.getHeaders()),
-                    new HttpEntity<String>(restHelper.getHeaders()),
+                ParameterizedTypeReference.forType(WorkbasketResource.class)))
-                    ParameterizedTypeReference.forType(WorkbasketResource.class)))
         .isInstanceOf(HttpClientErrorException.class)
         .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode())
         .isEqualTo(HttpStatus.NOT_FOUND);
@@ -175,12 +172,8 @@ class WorkbasketControllerIntTest {
     assertThat(response.getBody().getLink(Link.REL_SELF).getHref().endsWith(parameters)).isTrue();
   }
 
-  /**
-   * Bug Ticket TSK-1029. Businessadmin is allowed to delete any workbasket ticket without user
-   * related access restrictions.
-   */
   @Test
-  void testDeleteWorkbasketAsBusinessAdminWithoutExplicitReadPermission() {
+  void testMarkWorkbasketForDeletionAsBusinessAdminWithoutExplicitReadPermission() {
 
     String workbasketID = "WBI:100000000000000000000000000000000005";
 
@@ -193,6 +186,22 @@ class WorkbasketControllerIntTest {
     assertThat(response.getStatusCode()).isEqualTo(HttpStatus.ACCEPTED);
   }
 
+  @Test
+  void statusCode423ShouldBeReturnedIfWorkbasketContainsNonCompletedTasks() {
+    String workbasketWithNonCompletedTasks = "WBI:100000000000000000000000000000000004";
+
+    assertThatThrownBy(() ->
+        template.exchange(
+            restHelper.toUrl(Mapping.URL_WORKBASKET_ID, workbasketWithNonCompletedTasks),
+                HttpMethod.DELETE,
+                new HttpEntity<>(restHelper.getHeadersBusinessAdmin()),
+                Void.class))
+            .isInstanceOf(HttpClientErrorException.class)
+            .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode())
+            .isEqualTo(HttpStatus.LOCKED);
+
+  }
+
   @Test
   void testRemoveWorkbasketAsDistributionTarget() {
     ResponseEntity<?> response =
@@ -213,9 +222,9 @@ class WorkbasketControllerIntTest {
             ParameterizedTypeReference.forType(DistributionTargetListResource.class));
     assertThat(response2.getStatusCode()).isEqualTo(HttpStatus.OK);
     assertThat(
-        response2.getBody().getContent().stream()
+            response2.getBody().getContent().stream()
-            .map(DistributionTargetResource::getWorkbasketId)
+                .map(DistributionTargetResource::getWorkbasketId)
-            .noneMatch(id -> (id.equals("WBI:100000000000000000000000000000000007"))))
+                .noneMatch("WBI:100000000000000000000000000000000007"::equals))
         .isTrue();
   }
 
@@ -248,5 +257,4 @@ class WorkbasketControllerIntTest {
         .isEqualTo(MediaTypes.HAL_JSON_UTF8_VALUE);
     assertThat(response.getBody().getContent()).hasSize(4);
   }
-
 }

rest/taskana-rest-spring/src/test/resources/asciidoc/rest-api.adoc

@@ -46,6 +46,9 @@ use of HTTP status codes.
 | `201 Created`
 | The request completed successfully und create the new resource.
 
+| `202 Accepted`
+| The request could not be processed immediatly but will be carried out later.
+
 | `204 No Content` 
 | The request completed successfully and there is no content to send in the response payload.
 
@@ -72,6 +75,9 @@ use of HTTP status codes.
 
 | `415 Unsupported Media Type`
 | The content of the request can't be understood due to being in an unsupported media-type.
+
+| `423 Locked`
+| The resource is currently locked and cannot be modified.
 |===
 
 == Common Fields
@@ -497,15 +503,22 @@ Therefore for the response fields you can refer to the structure of the <<workba
 
 === Delete workbasket
 
-A `DELETE` request to dalete a workbasket
+A `DELETE` request to delete a workbasket. 
+
+Since workbaskets can contain completed and non-completed task, the request has different response codes: +
+
+- *204 NO_CONTENT* - Workbasket has been deleted successfully
+- *202 ACCEPTED* - Workbasket still contains completed tasks. It has been marked for deletion and will be deleted automatically as soon as all completed tasks are deleted.
+- *423 LOCKED* - Worbasket contains non-completed tasks and cannot be deleted.
+
 
 ==== Example Request
 
-include::{snippets}/MarkWorkbasketForDeletionDocTest/http-request.adoc[]
+include::{snippets}/DeleteWorkbasketDocTest/http-request.adoc[]
 
 ==== Example Response
 
-include::{snippets}/MarkWorkbasketForDeletionDocTest/http-response.adoc[]
+include::{snippets}/DeleteWorkbasketDocTest/http-response.adoc[]
 
 
 === Remove a workbasket as distribution-target

web/src/app/administration/workbasket/details/information/workbasket-information.component.ts

@@ -280,16 +280,18 @@ implements OnInit, OnChanges, OnDestroy {
         response => {
           this.requestInProgressService.setRequestInProgress(false);
           this.workbasketService.triggerWorkBasketSaved();
-          if (response) {
+          if (response.status === 202) {
             // new Key ERROR_TYPES.MARK_ERR
+            // TODO: message changed
             this.generalModalService.triggerMessage(
-              new MessageModal('There was an error marking workbasket for deletion',
+              new MessageModal('Workbasket was marked for deletion.',
-                'It not possible to mark the workbasket for deletion, It has been deleted.')
+              `The Workbasket ${this.workbasket.workbasketId} still contains completed tasks and could not be deleted. Instead is was marked for deletion and will be deleted automatically as soon as the completed tasks are deleted from the database.`)
             );
           } else {
             // new Key ALERT_TYPES.SUCCESS_ALERT_12
+            // TODO: message changed
             this.alertService.triggerAlert(
-              new AlertModel(AlertType.SUCCESS, `The Workbasket ${this.workbasket.workbasketId} has been marked for deletion`)
+              new AlertModel(AlertType.SUCCESS, `The Workbasket ${this.workbasket.workbasketId} has been deleted.`)
             );
           }
           this.router.navigate(['taskana/administration/workbaskets']);

web/src/app/shared/services/workbasket/workbasket.service.ts

@@ -89,7 +89,7 @@ export class WorkbasketService {
   // delete
   markWorkbasketForDeletion(url: string): Observable<any> {
     return this.httpClient
-      .delete<any>(url);
+      .delete<any>(url, {observe: 'response'});
   }
 
   // GET