Continue refactoring of logic and fix a couple of tests

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

@@ -6,7 +6,6 @@
 import jakarta.validation.constraints.NotNull;
 import jakarta.validation.constraints.Size;
 import java.time.LocalDateTime;
-import java.util.List;
 import java.util.Objects;
 
 @Entity
@@ -204,7 +203,6 @@ public abstract static class Builder<T> {
         private LocalDateTime createdOn;
         private LocalDateTime modifiedOn;
         private final String keyResultType;
-        private List<Action> actionList;
 
         protected Builder(String keyResultType) {
             this.keyResultType = keyResultType;
@@ -255,11 +253,6 @@ public T withModifiedOn(LocalDateTime modifiedOn) {
             return (T) this;
         }
 
-        public T withActionList(List<Action> actionList) {
-            this.actionList = actionList;
-            return (T) this;
-        }
-
         public abstract KeyResult build();
     }
 }

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

@@ -77,14 +77,16 @@ public void deleteEntitiesByKeyResultId(Long keyResultId) {
         getActionsByKeyResultId(keyResultId).forEach(action -> deleteEntityById(action.getId()));
     }
 
-    public List<Action> createDuplicates(KeyResult oldKeyResult, KeyResult newKeyResult) {
+    public List<Action> duplicateActions(KeyResult oldKeyResult, KeyResult newKeyResult) {
         List<Action> actionList = actionPersistenceService
                 .getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId());
         if (actionList == null) {
             return Collections.emptyList();
         }
 
-        return actionList.stream().map(action -> {
+        List<Action> duplicatedActions = new ArrayList<>();
+
+        actionList.forEach(action -> {
             Action newAction = Action.Builder
                     .builder()
                     .withAction(action.getActionPoint())
@@ -94,7 +96,10 @@ public List<Action> createDuplicates(KeyResult oldKeyResult, KeyResult newKeyRes
                     .withKeyResult(newKeyResult)
                     .build();
             validator.validate(newAction);
-            return newAction;
-        }).toList();
+            actionPersistenceService.save(newAction);
+            duplicatedActions.add(newAction);
+        });
+
+        return duplicatedActions;
     }
 }

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

@@ -1,5 +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.Action;
 import ch.puzzle.okr.models.Objective;
 import ch.puzzle.okr.models.authorization.AuthorizationUser;
@@ -141,7 +144,26 @@ public List<KeyResult> getKeyResultsOwnedByUser(long id) {
         return keyResultPersistenceService.getKeyResultsOwnedByUser(id);
     }
 
-    public KeyResult makeCopyOfKeyResultMetric(KeyResult keyResult, Objective duplicatedObjective) {
+    @Transactional
+    public KeyResult duplicateKeyResult(AuthorizationUser authorizationUser, KeyResult keyResult,
+                                        Objective duplicatedObjective) {
+        KeyResult newKeyResult;
+
+        if (keyResult.getKeyResultType().equals(KEY_RESULT_TYPE_METRIC)) {
+            KeyResult keyResultMetric = makeCopyOfKeyResultMetric(keyResult, duplicatedObjective);
+            newKeyResult = createEntity(keyResultMetric, authorizationUser);
+        } else if (keyResult.getKeyResultType().equals(KEY_RESULT_TYPE_ORDINAL)) {
+            KeyResult keyResultOrdinal = makeCopyOfKeyResultOrdinal(keyResult, duplicatedObjective);
+            newKeyResult = createEntity(keyResultOrdinal, authorizationUser);
+        } else {
+            throw new UnsupportedOperationException("Unsupported KeyResultType: " + keyResult.getKeyResultType());
+        }
+
+        actionBusinessService.duplicateActions(keyResult, newKeyResult);
+        return newKeyResult;
+    }
+
+    private KeyResult makeCopyOfKeyResultMetric(KeyResult keyResult, Objective duplicatedObjective) {
         return KeyResultMetric.Builder
                 .builder() //
                 .withObjective(duplicatedObjective) //
@@ -154,7 +176,7 @@ public KeyResult makeCopyOfKeyResultMetric(KeyResult keyResult, Objective duplic
                 .build();
     }
 
-    public KeyResult makeCopyOfKeyResultOrdinal(KeyResult keyResult, Objective duplicatedObjective) {
+    private KeyResult makeCopyOfKeyResultOrdinal(KeyResult keyResult, Objective duplicatedObjective) {
         return KeyResultOrdinal.Builder
                 .builder() //
                 .withObjective(duplicatedObjective) //

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

@@ -1,15 +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.Action;
 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.ActionPersistenceService;
 import ch.puzzle.okr.service.persistence.ObjectivePersistenceService;
 import ch.puzzle.okr.service.validation.ObjectiveValidationService;
 import jakarta.transaction.Transactional;
@@ -28,20 +21,17 @@ public class ObjectiveBusinessService implements BusinessServiceInterface<Long,
     private final ObjectiveValidationService validator;
     private final KeyResultBusinessService keyResultBusinessService;
     private final CompletedBusinessService completedBusinessService;
-    private final ActionPersistenceService actionPersistenceService;
     private final ActionBusinessService actionBusinessService;
 
     public ObjectiveBusinessService(@Lazy KeyResultBusinessService keyResultBusinessService,
                                     ObjectiveValidationService validator,
                                     ObjectivePersistenceService objectivePersistenceService,
                                     CompletedBusinessService completedBusinessService,
-                                    ActionPersistenceService actionPersistenceService,
                                     ActionBusinessService actionBusinessService) {
         this.keyResultBusinessService = keyResultBusinessService;
         this.validator = validator;
         this.objectivePersistenceService = objectivePersistenceService;
         this.completedBusinessService = completedBusinessService;
-        this.actionPersistenceService = actionPersistenceService;
         this.actionBusinessService = actionBusinessService;
     }
 
@@ -123,39 +113,22 @@ public Objective createEntity(Objective objective, AuthorizationUser authorizati
      * @param authorizationUser
      *            AuthorizationUser
      * @param keyResultIds
-     *            KeyResultIds to get and copy
+     *            KeyResultIds to get and duplicate
      * @return New Objective with copied KeyResults form the source Objective
      */
     @Transactional
     public Objective duplicateObjective(Objective objective, AuthorizationUser authorizationUser,
                                         List<Long> keyResultIds) {
         Objective duplicatedObjective = createEntity(objective, authorizationUser);
         for (Long keyResult : keyResultIds) {
-            duplicateKeyResult(authorizationUser,
-                               keyResultBusinessService.getEntityById(keyResult),
-                               duplicatedObjective);
+            keyResultBusinessService
+                    .duplicateKeyResult(authorizationUser,
+                                        keyResultBusinessService.getEntityById(keyResult),
+                                        duplicatedObjective);
         }
         return duplicatedObjective;
     }
 
-    public void duplicateKeyResult(AuthorizationUser authorizationUser, KeyResult keyResult,
-                                   Objective duplicatedObjective) {
-        KeyResult newKeyResult = null;
-
-        if (keyResult.getKeyResultType().equals(KEY_RESULT_TYPE_METRIC)) {
-            KeyResult keyResultMetric = keyResultBusinessService.makeCopyOfKeyResultMetric(keyResult, duplicatedObjective);
-            newKeyResult = keyResultBusinessService.createEntity(keyResultMetric, authorizationUser);
-        } else if (keyResult.getKeyResultType().equals(KEY_RESULT_TYPE_ORDINAL)) {
-            KeyResult keyResultOrdinal = keyResultBusinessService.makeCopyOfKeyResultOrdinal(keyResult, duplicatedObjective);
-            newKeyResult = keyResultBusinessService.createEntity(keyResultOrdinal, authorizationUser);
-        } else {
-            throw new UnsupportedOperationException("Unsupported KeyResultType: " + keyResult.getKeyResultType());
-        }
-
-        List<Action> copiedActions = actionBusinessService.createDuplicates(keyResult, newKeyResult);
-        copiedActions.forEach(actionPersistenceService::save);
-    }
-
     @Transactional
     public void deleteEntityById(Long id) {
         validator.validateOnDelete(id);

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

@@ -1,5 +1,13 @@
 package ch.puzzle.okr.controller;
 
+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.*;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
+
 import ch.puzzle.okr.deserializer.DeserializerHelper;
 import ch.puzzle.okr.dto.ObjectiveDto;
 import ch.puzzle.okr.mapper.ObjectiveMapper;
@@ -10,6 +18,8 @@
 import ch.puzzle.okr.service.authorization.ActionAuthorizationService;
 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;
@@ -29,17 +39,6 @@
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
 import org.springframework.web.server.ResponseStatusException;
 
-import java.time.LocalDateTime;
-import java.util.List;
-
-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.*;
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
-import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
-
 @WithMockUser(value = "spring")
 @ExtendWith(MockitoExtension.class)
 @WebMvcTest(ObjectiveController.class)
@@ -113,27 +112,27 @@ class ObjectiveControllerIT {
             .withModifiedOn(LocalDateTime.MAX)
             .build();
     private static final ObjectiveDto objective1Dto = new ObjectiveDto(5L,
-            1,
-            OBJECTIVE_TITLE_1,
-            1L,
-            1L,
-            "GJ 22/23-Q2",
-            DESCRIPTION,
-            State.DRAFT,
-            LocalDateTime.MAX,
-            LocalDateTime.MAX,
-            true);
+                                                                       1,
+                                                                       OBJECTIVE_TITLE_1,
+                                                                       1L,
+                                                                       1L,
+                                                                       "GJ 22/23-Q2",
+                                                                       DESCRIPTION,
+                                                                       State.DRAFT,
+                                                                       LocalDateTime.MAX,
+                                                                       LocalDateTime.MAX,
+                                                                       true);
     private static final ObjectiveDto objective2Dto = new ObjectiveDto(7L,
-            1,
-            OBJECTIVE_TITLE_2,
-            1L,
-            1L,
-            "GJ 22/23-Q2",
-            DESCRIPTION,
-            State.DRAFT,
-            LocalDateTime.MIN,
-            LocalDateTime.MIN,
-            true);
+                                                                       1,
+                                                                       OBJECTIVE_TITLE_2,
+                                                                       1L,
+                                                                       1L,
+                                                                       "GJ 22/23-Q2",
+                                                                       DESCRIPTION,
+                                                                       State.DRAFT,
+                                                                       LocalDateTime.MIN,
+                                                                       LocalDateTime.MIN,
+                                                                       true);
 
     @Autowired
     private MockMvc mvc;
@@ -184,16 +183,16 @@ void shouldThrowNotFoundWhenGettingObjectiveWithNonExistentId() throws Exception
     @Test
     void shouldReturnCreatedObjectiveWhenCreatingNewObjective() throws Exception {
         ObjectiveDto testObjective = new ObjectiveDto(null,
-                1,
-                "Program Faster",
-                1L,
-                1L,
-                "GJ 22/23-Q2",
-                "Just be faster",
-                State.DRAFT,
-                null,
-                null,
-                true);
+                                                      1,
+                                                      "Program Faster",
+                                                      1L,
+                                                      1L,
+                                                      "GJ 22/23-Q2",
+                                                      "Just be faster",
+                                                      State.DRAFT,
+                                                      null,
+                                                      null,
+                                                      true);
 
         BDDMockito.given(objectiveMapper.toDto(any())).willReturn(testObjective);
         BDDMockito.given(objectiveAuthorizationService.createEntity(any())).willReturn(fullObjective);
@@ -214,7 +213,7 @@ void shouldThrowResponseStatusExceptionWhenCreatingObjectiveWithNullValues() thr
         BDDMockito
                 .given(objectiveAuthorizationService.createEntity(any()))
                 .willThrow(new ResponseStatusException(HttpStatus.BAD_REQUEST,
-                        "Missing attribute title when creating objective"));
+                                                       "Missing attribute title when creating objective"));
 
         mvc
                 .perform(post(URL_BASE_OBJECTIVE)
@@ -228,16 +227,16 @@ void shouldThrowResponseStatusExceptionWhenCreatingObjectiveWithNullValues() thr
     @Test
     void shouldReturnUpdatedObjectiveWhenUpdatingObjective() throws Exception {
         ObjectiveDto testObjective = new ObjectiveDto(1L,
-                1,
-                TITLE,
-                1L,
-                1L,
-                "GJ 22/23-Q2",
-                UPDATED_DESCRIPTION,
-                State.NOTSUCCESSFUL,
-                LocalDateTime.MIN,
-                LocalDateTime.MAX,
-                true);
+                                                      1,
+                                                      TITLE,
+                                                      1L,
+                                                      1L,
+                                                      "GJ 22/23-Q2",
+                                                      UPDATED_DESCRIPTION,
+                                                      State.NOTSUCCESSFUL,
+                                                      LocalDateTime.MIN,
+                                                      LocalDateTime.MAX,
+                                                      true);
         Objective objective = Objective.Builder
                 .builder()
                 .withId(1L)
@@ -264,16 +263,16 @@ void shouldReturnUpdatedObjectiveWhenUpdatingObjective() throws Exception {
     @Test
     void shouldReturnImUsed() throws Exception {
         ObjectiveDto testObjectiveDto = new ObjectiveDto(1L,
-                1,
-                TITLE,
-                1L,
-                1L,
-                "GJ 22/23-Q2",
-                UPDATED_DESCRIPTION,
-                State.SUCCESSFUL,
-                LocalDateTime.MAX,
-                LocalDateTime.MAX,
-                true);
+                                                         1,
+                                                         TITLE,
+                                                         1L,
+                                                         1L,
+                                                         "GJ 22/23-Q2",
+                                                         UPDATED_DESCRIPTION,
+                                                         State.SUCCESSFUL,
+                                                         LocalDateTime.MAX,
+                                                         LocalDateTime.MAX,
+                                                         true);
         Objective objectiveImUsed = Objective.Builder
                 .builder()
                 .withId(1L)
@@ -301,7 +300,7 @@ void shouldReturnNotFoundWhenUpdatingObjectiveByNonExistentId() throws Exception
         BDDMockito
                 .given(objectiveAuthorizationService.updateEntity(anyLong(), any()))
                 .willThrow(new ResponseStatusException(HttpStatus.NOT_FOUND,
-                        "Failed objective -> Attribut is invalid"));
+                                                       "Failed objective -> Attribut is invalid"));
 
         mvc
                 .perform(put(URL_OBJECTIVE_10)
@@ -317,7 +316,7 @@ void shouldReturnBadRequest() throws Exception {
         BDDMockito
                 .given(objectiveAuthorizationService.updateEntity(anyLong(), any()))
                 .willThrow(new ResponseStatusException(HttpStatus.BAD_REQUEST,
-                        "Failed objective -> Attribut is invalid"));
+                                                       "Failed objective -> Attribut is invalid"));
 
         mvc
                 .perform(put(URL_OBJECTIVE_10).with(SecurityMockMvcRequestPostProcessors.csrf()))
@@ -351,7 +350,9 @@ void shouldReturnIsCreatedWhenObjectiveWasDuplicated() throws Exception {
         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(objectiveAuthorizationService.duplicateEntity(objective1, List.of(1L, 2L)))
+                .willReturn(objective1);
         BDDMockito.given(objectiveMapper.toDto(objective1)).willReturn(objective1Dto);
 
         mvc