Bug/1255 Duplicating a objective does not bring all properties of the…

… KeyResults (#1294)

* Refactor duplicating methods to include values of keyResult

* Modify duplicateObjectiveDto to only include ids

* make new method to duplicate Actionlists

* write new e2e test testing if all keyresult attributes get duplicated

* remove unnecessary wait

* remove action list attribute

* Finish logic of duplicating objectives with the action plan
---------

Co-authored-by: Nevio Di Gennaro <[email protected]>
Co-authored-by: MasterEvarior <[email protected]>

backend/src/main/java/ch/puzzle/okr/controller/ObjectiveController.java

@@ -11,7 +11,6 @@
 import ch.puzzle.okr.mapper.keyresult.KeyResultMapper;
 import ch.puzzle.okr.models.Action;
 import ch.puzzle.okr.models.Objective;
-import ch.puzzle.okr.models.keyresult.KeyResult;
 import ch.puzzle.okr.service.authorization.ActionAuthorizationService;
 import ch.puzzle.okr.service.authorization.ObjectiveAuthorizationService;
 import io.swagger.v3.oas.annotations.Operation;
@@ -100,17 +99,13 @@ public ResponseEntity<ObjectiveDto> createObjective(@io.swagger.v3.oas.annotatio
     @Operation(summary = "Duplicate Objective", description = "Duplicate a given Objective")
     @ApiResponses(value = { @ApiResponse(responseCode = "201", description = "Duplicated a given Objective", content = {
             @Content(mediaType = "application/json", schema = @Schema(implementation = ObjectiveDto.class)) }) })
-    @PostMapping("/{id}")
-    public ResponseEntity<ObjectiveDto> duplicateObjective(@Parameter(description = "The ID for duplicating an Objective.", required = true)
-    @PathVariable Long id, @io.swagger.v3.oas.annotations.parameters.RequestBody(description = "The Objective which should be duplicated as JSON", required = true) @RequestBody DuplicateObjectiveDto duplicateObjectiveDto) {
+    @PostMapping("/duplicate")
+    public ResponseEntity<ObjectiveDto> duplicateObjective(@io.swagger.v3.oas.annotations.parameters.RequestBody(description = "The Objective which should be duplicated as JSON", required = true)
+    @RequestBody DuplicateObjectiveDto duplicateObjectiveDto) {
         Objective objective = objectiveMapper.toObjective(duplicateObjectiveDto.objective());
-        List<KeyResult> keyResults = duplicateObjectiveDto
-                .keyResults()
-                .stream()
-                .map(keyResultMapper::toKeyResult)
-                .toList();
+        List<Long> keyResultIds = duplicateObjectiveDto.keyResultIds();
         ObjectiveDto duplicatedObjectiveDto = objectiveMapper
-                .toDto(objectiveAuthorizationService.duplicateEntity(id, objective, keyResults));
+                .toDto(objectiveAuthorizationService.duplicateEntity(objective, keyResultIds));
         return ResponseEntity.status(HttpStatus.CREATED).body(duplicatedObjectiveDto);
     }
 

backend/src/main/java/ch/puzzle/okr/dto/DuplicateObjectiveDto.java

@@ -1,7 +1,6 @@
 package ch.puzzle.okr.dto;
 
-import ch.puzzle.okr.dto.keyresult.KeyResultDto;
 import java.util.List;
 
-public record DuplicateObjectiveDto(ObjectiveDto objective, List<KeyResultDto> keyResults) {
+public record DuplicateObjectiveDto(ObjectiveDto objective, List<Long> keyResultIds) {
 }

backend/src/main/java/ch/puzzle/okr/models/keyresult/KeyResult.java

@@ -1,9 +1,6 @@
 package ch.puzzle.okr.models.keyresult;
 
-import ch.puzzle.okr.models.MessageKey;
-import ch.puzzle.okr.models.Objective;
-import ch.puzzle.okr.models.User;
-import ch.puzzle.okr.models.WriteableInterface;
+import ch.puzzle.okr.models.*;
 import jakarta.persistence.*;
 import jakarta.validation.constraints.NotBlank;
 import jakarta.validation.constraints.NotNull;

backend/src/main/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationService.java

@@ -15,10 +15,10 @@ public ObjectiveAuthorizationService(ObjectiveBusinessService objectiveBusinessS
         super(objectiveBusinessService, authorizationService);
     }
 
-    public Objective duplicateEntity(Long id, Objective objective, List<KeyResult> keyResults) {
+    public Objective duplicateEntity(Objective objective, List<Long> keyResultIds) {
         AuthorizationUser authorizationUser = getAuthorizationService().updateOrAddAuthorizationUser();
         hasRoleCreateOrUpdate(objective, authorizationUser);
-        return getBusinessService().duplicateObjective(id, objective, authorizationUser, keyResults);
+        return getBusinessService().duplicateObjective(objective, authorizationUser, keyResultIds);
     }
 
     public List<KeyResult> getAllKeyResultsByObjective(Long objectiveId) {

backend/src/main/java/ch/puzzle/okr/service/business/ActionBusinessService.java

@@ -1,10 +1,12 @@
 package ch.puzzle.okr.service.business;
 
 import ch.puzzle.okr.models.Action;
+import ch.puzzle.okr.models.keyresult.KeyResult;
 import ch.puzzle.okr.service.persistence.ActionPersistenceService;
 import ch.puzzle.okr.service.validation.ActionValidationService;
 import jakarta.transaction.Transactional;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import org.springframework.stereotype.Service;
 
@@ -74,4 +76,30 @@ public void deleteEntityById(Long id) {
     public void deleteEntitiesByKeyResultId(Long keyResultId) {
         getActionsByKeyResultId(keyResultId).forEach(action -> deleteEntityById(action.getId()));
     }
+
+    public List<Action> duplicateActions(KeyResult oldKeyResult, KeyResult newKeyResult) {
+        List<Action> actionList = actionPersistenceService
+                .getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId());
+        if (actionList == null) {
+            return Collections.emptyList();
+        }
+
+        List<Action> duplicatedActions = new ArrayList<>();
+
+        actionList.forEach(action -> {
+            Action newAction = Action.Builder
+                    .builder()
+                    .withAction(action.getActionPoint())
+                    .isChecked(action.isChecked())
+                    .withPriority(action.getPriority())
+                    .withVersion(action.getVersion())
+                    .withKeyResult(newKeyResult)
+                    .build();
+            validator.validate(newAction);
+            actionPersistenceService.save(newAction);
+            duplicatedActions.add(newAction);
+        });
+
+        return duplicatedActions;
+    }
 }

backend/src/main/java/ch/puzzle/okr/service/business/KeyResultBusinessService.java

@@ -1,9 +1,15 @@
 package ch.puzzle.okr.service.business;
 
+import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_METRIC;
+import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_ORDINAL;
+
 import ch.puzzle.okr.models.Action;
+import ch.puzzle.okr.models.Objective;
 import ch.puzzle.okr.models.authorization.AuthorizationUser;
 import ch.puzzle.okr.models.checkin.CheckIn;
 import ch.puzzle.okr.models.keyresult.KeyResult;
+import ch.puzzle.okr.models.keyresult.KeyResultMetric;
+import ch.puzzle.okr.models.keyresult.KeyResultOrdinal;
 import ch.puzzle.okr.models.keyresult.KeyResultWithActionList;
 import ch.puzzle.okr.service.persistence.KeyResultPersistenceService;
 import ch.puzzle.okr.service.validation.KeyResultValidationService;
@@ -137,4 +143,45 @@ private boolean isKeyResultTypeChangeable(Long id) {
     public List<KeyResult> getKeyResultsOwnedByUser(long id) {
         return keyResultPersistenceService.getKeyResultsOwnedByUser(id);
     }
+
+    @Transactional
+    public KeyResult duplicateKeyResult(AuthorizationUser authorizationUser, KeyResult keyResult,
+                                        Objective duplicatedObjective) {
+        KeyResult newKeyResult = switch (keyResult.getKeyResultType()) {
+            case KEY_RESULT_TYPE_METRIC -> makeCopyOfKeyResultMetric(keyResult, duplicatedObjective);
+            case KEY_RESULT_TYPE_ORDINAL -> makeCopyOfKeyResultOrdinal(keyResult, duplicatedObjective);
+            default ->
+                throw new UnsupportedOperationException("Unsupported KeyResultType: " + keyResult.getKeyResultType());
+        };
+        newKeyResult = createEntity(newKeyResult, authorizationUser);
+
+        actionBusinessService.duplicateActions(keyResult, newKeyResult);
+        return newKeyResult;
+    }
+
+    private KeyResult makeCopyOfKeyResultMetric(KeyResult keyResult, Objective duplicatedObjective) {
+        return KeyResultMetric.Builder
+                .builder() //
+                .withObjective(duplicatedObjective) //
+                .withTitle(keyResult.getTitle()) //
+                .withDescription(keyResult.getDescription()) //
+                .withOwner(keyResult.getOwner()) //
+                .withUnit(((KeyResultMetric) keyResult).getUnit()) //
+                .withBaseline(((KeyResultMetric) keyResult).getBaseline()) //
+                .withStretchGoal(((KeyResultMetric) keyResult).getStretchGoal()) //
+                .build();
+    }
+
+    private KeyResult makeCopyOfKeyResultOrdinal(KeyResult keyResult, Objective duplicatedObjective) {
+        return KeyResultOrdinal.Builder
+                .builder() //
+                .withObjective(duplicatedObjective) //
+                .withTitle(keyResult.getTitle()) //
+                .withDescription(keyResult.getDescription()) //
+                .withOwner(keyResult.getOwner()) //
+                .withCommitZone(((KeyResultOrdinal) keyResult).getCommitZone()) //
+                .withTargetZone(((KeyResultOrdinal) keyResult).getTargetZone()) //
+                .withStretchZone(((KeyResultOrdinal) keyResult).getStretchZone()) //
+                .build();
+    }
 }

backend/src/main/java/ch/puzzle/okr/service/business/ObjectiveBusinessService.java

@@ -1,13 +1,8 @@
 package ch.puzzle.okr.service.business;
 
-import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_METRIC;
-import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_ORDINAL;
-
 import ch.puzzle.okr.models.Objective;
 import ch.puzzle.okr.models.authorization.AuthorizationUser;
 import ch.puzzle.okr.models.keyresult.KeyResult;
-import ch.puzzle.okr.models.keyresult.KeyResultMetric;
-import ch.puzzle.okr.models.keyresult.KeyResultOrdinal;
 import ch.puzzle.okr.service.persistence.ObjectivePersistenceService;
 import ch.puzzle.okr.service.validation.ObjectiveValidationService;
 import jakarta.transaction.Transactional;
@@ -26,15 +21,18 @@ public class ObjectiveBusinessService implements BusinessServiceInterface<Long,
     private final ObjectiveValidationService validator;
     private final KeyResultBusinessService keyResultBusinessService;
     private final CompletedBusinessService completedBusinessService;
+    private final ActionBusinessService actionBusinessService;
 
     public ObjectiveBusinessService(@Lazy KeyResultBusinessService keyResultBusinessService,
                                     ObjectiveValidationService validator,
                                     ObjectivePersistenceService objectivePersistenceService,
-                                    CompletedBusinessService completedBusinessService) {
+                                    CompletedBusinessService completedBusinessService,
+                                    ActionBusinessService actionBusinessService) {
         this.keyResultBusinessService = keyResultBusinessService;
         this.validator = validator;
         this.objectivePersistenceService = objectivePersistenceService;
         this.completedBusinessService = completedBusinessService;
+        this.actionBusinessService = actionBusinessService;
     }
 
     private static boolean hasQuarterChanged(Objective objective, Objective savedObjective) {
@@ -107,67 +105,31 @@ public Objective createEntity(Objective objective, AuthorizationUser authorizati
     }
 
     /**
-     * Create a new Objective (with a new ID) and copy from a source Objective
-     * (identified by id) the KeyResults. The CheckIns are not copied.
+     * Create a new Objective and copy the KeyResults from the source Objective. The
+     * CheckIns are not copied.
      *
-     * @param id
-     *            ID of the source Objective
      * @param objective
-     *            New Objective with no KeyResults
+     *            Objective to be duplicated
      * @param authorizationUser
      *            AuthorizationUser
-     * @param keyResults
-     *            KeyResults to copy
-     *
+     * @param keyResultIds
+     *            Ids of the keyresults which should be duplicated, the new
+     *            keyresults will be associated with the newly duplicated objective
      * @return New Objective with copied KeyResults form the source Objective
      */
     @Transactional
-    public Objective duplicateObjective(Long id, Objective objective, AuthorizationUser authorizationUser,
-                                        List<KeyResult> keyResults) {
+    public Objective duplicateObjective(Objective objective, AuthorizationUser authorizationUser,
+                                        List<Long> keyResultIds) {
         Objective duplicatedObjective = createEntity(objective, authorizationUser);
-        for (KeyResult keyResult : keyResults) {
-            duplicateKeyResult(authorizationUser, keyResult, duplicatedObjective);
+        for (Long keyResult : keyResultIds) {
+            keyResultBusinessService
+                    .duplicateKeyResult(authorizationUser,
+                                        keyResultBusinessService.getEntityById(keyResult),
+                                        duplicatedObjective);
         }
         return duplicatedObjective;
     }
 
-    private void duplicateKeyResult(AuthorizationUser authorizationUser, KeyResult keyResult,
-                                    Objective duplicatedObjective) {
-        if (keyResult.getKeyResultType().equals(KEY_RESULT_TYPE_METRIC)) {
-            KeyResult keyResultMetric = makeCopyOfKeyResultMetric(keyResult, duplicatedObjective);
-            keyResultBusinessService.createEntity(keyResultMetric, authorizationUser);
-        } else if (keyResult.getKeyResultType().equals(KEY_RESULT_TYPE_ORDINAL)) {
-            KeyResult keyResultOrdinal = makeCopyOfKeyResultOrdinal(keyResult, duplicatedObjective);
-            keyResultBusinessService.createEntity(keyResultOrdinal, authorizationUser);
-        }
-    }
-
-    private KeyResult makeCopyOfKeyResultMetric(KeyResult keyResult, Objective duplicatedObjective) {
-        return KeyResultMetric.Builder
-                .builder() //
-                .withObjective(duplicatedObjective) //
-                .withTitle(keyResult.getTitle()) //
-                .withDescription(keyResult.getDescription()) //
-                .withOwner(keyResult.getOwner()) //
-                .withUnit(((KeyResultMetric) keyResult).getUnit()) //
-                .withBaseline(0D) //
-                .withStretchGoal(1D) //
-                .build();
-    }
-
-    private KeyResult makeCopyOfKeyResultOrdinal(KeyResult keyResult, Objective duplicatedObjective) {
-        return KeyResultOrdinal.Builder
-                .builder() //
-                .withObjective(duplicatedObjective) //
-                .withTitle(keyResult.getTitle()) //
-                .withDescription(keyResult.getDescription()) //
-                .withOwner(keyResult.getOwner()) //
-                .withCommitZone("-") //
-                .withTargetZone("-") //
-                .withStretchZone("-") //
-                .build();
-    }
-
     @Transactional
     public void deleteEntityById(Long id) {
         validator.validateOnDelete(id);

backend/src/test/java/ch/puzzle/okr/controller/ObjectiveControllerIT.java

@@ -1,6 +1,7 @@
 package ch.puzzle.okr.controller;
 
-import static ch.puzzle.okr.test.KeyResultTestHelpers.*;
+import static ch.puzzle.okr.test.KeyResultTestHelpers.keyResultMetricDto;
+import static ch.puzzle.okr.test.KeyResultTestHelpers.keyResultOrdinalDto;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.*;
@@ -18,6 +19,7 @@
 import ch.puzzle.okr.service.authorization.AuthorizationService;
 import ch.puzzle.okr.service.authorization.ObjectiveAuthorizationService;
 import java.time.LocalDateTime;
+import java.util.List;
 import org.hamcrest.core.Is;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DisplayName;
@@ -49,7 +51,7 @@ class ObjectiveControllerIT {
     private static final String URL_BASE_OBJECTIVE = "/api/v2/objectives";
     private static final String URL_OBJECTIVE_5 = "/api/v2/objectives/5";
     private static final String URL_OBJECTIVE_10 = "/api/v2/objectives/10";
-    private static final String URL_DUPLICATE_OBJECTIVE_5 = "/api/v2/objectives/5";
+    private static final String URL_DUPLICATE_OBJECTIVE_5 = "/api/v2/objectives/duplicate";
     private static final String JSON = """
             {
                "title": "FullObjective", "ownerId": 1, "ownerFirstname": "Bob", "ownerLastname": "Kaufmann",
@@ -60,9 +62,9 @@ class ObjectiveControllerIT {
     private static final String DUPLICATE_OBJECTIVE = """
             {
                 "objective": %s,
-                "keyResults": [%s,%s]
+                "keyResultIds": [1,2]
             }
-            """.formatted(JSON, KEY_RESULT_METRIC_JSON, KEY_RESULT_ORDINAL_JSON);
+            """.formatted(JSON);
 
     private static final String CREATE_NEW_OBJECTIVE = """
             {
@@ -341,15 +343,16 @@ void shouldThrowExceptionWhenObjectiveWithIdCantBeFoundWhileDeleting() throws Ex
                 .andExpect(MockMvcResultMatchers.status().isNotFound());
     }
 
-    @DisplayName("Should return is created when an ojective was duplicated")
+    @DisplayName("Should return is created when an objective was duplicated")
     @Test
     void shouldReturnIsCreatedWhenObjectiveWasDuplicated() throws Exception {
-        BDDMockito
-                .given(objectiveAuthorizationService.duplicateEntity(anyLong(), any(), anyList()))
-                .willReturn(objective1);
+        BDDMockito.given(objectiveMapper.toObjective(any(ObjectiveDto.class))).willReturn(objective1);
         BDDMockito.given(keyResultMapper.toDto(any(KeyResultMetric.class), any())).willReturn(keyResultMetricDto);
         BDDMockito.given(keyResultMapper.toDto(any(KeyResultOrdinal.class), any())).willReturn(keyResultOrdinalDto);
         BDDMockito.given(objectiveAuthorizationService.getAuthorizationService()).willReturn(authorizationService);
+        BDDMockito
+                .given(objectiveAuthorizationService.duplicateEntity(objective1, List.of(1L, 2L)))
+                .willReturn(objective1);
         BDDMockito.given(objectiveMapper.toDto(objective1)).willReturn(objective1Dto);
 
         mvc