diff --git a/backend/src/main/java/ch/puzzle/okr/controller/ObjectiveController.java b/backend/src/main/java/ch/puzzle/okr/controller/ObjectiveController.java index bd8aeb6a5d..7409725f86 100644 --- a/backend/src/main/java/ch/puzzle/okr/controller/ObjectiveController.java +++ b/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 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 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 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 keyResults = duplicateObjectiveDto - .keyResults() - .stream() - .map(keyResultMapper::toKeyResult) - .toList(); + List keyResultIds = duplicateObjectiveDto.keyResultIds(); ObjectiveDto duplicatedObjectiveDto = objectiveMapper - .toDto(objectiveAuthorizationService.duplicateEntity(id, objective, keyResults)); + .toDto(objectiveAuthorizationService.duplicateEntity(objective, keyResultIds)); return ResponseEntity.status(HttpStatus.CREATED).body(duplicatedObjectiveDto); } diff --git a/backend/src/main/java/ch/puzzle/okr/dto/DuplicateObjectiveDto.java b/backend/src/main/java/ch/puzzle/okr/dto/DuplicateObjectiveDto.java index 232fd14ae2..2b1eb2d32a 100644 --- a/backend/src/main/java/ch/puzzle/okr/dto/DuplicateObjectiveDto.java +++ b/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 keyResults) { +public record DuplicateObjectiveDto(ObjectiveDto objective, List keyResultIds) { } diff --git a/backend/src/main/java/ch/puzzle/okr/models/keyresult/KeyResult.java b/backend/src/main/java/ch/puzzle/okr/models/keyresult/KeyResult.java index f365a3dec2..177a594615 100644 --- a/backend/src/main/java/ch/puzzle/okr/models/keyresult/KeyResult.java +++ b/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; diff --git a/backend/src/main/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationService.java b/backend/src/main/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationService.java index 97d8fbb38b..800a12dcc4 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationService.java +++ b/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 keyResults) { + public Objective duplicateEntity(Objective objective, List keyResultIds) { AuthorizationUser authorizationUser = getAuthorizationService().updateOrAddAuthorizationUser(); hasRoleCreateOrUpdate(objective, authorizationUser); - return getBusinessService().duplicateObjective(id, objective, authorizationUser, keyResults); + return getBusinessService().duplicateObjective(objective, authorizationUser, keyResultIds); } public List getAllKeyResultsByObjective(Long objectiveId) { diff --git a/backend/src/main/java/ch/puzzle/okr/service/business/ActionBusinessService.java b/backend/src/main/java/ch/puzzle/okr/service/business/ActionBusinessService.java index 92319dd9a8..96ec34214d 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/business/ActionBusinessService.java +++ b/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 duplicateActions(KeyResult oldKeyResult, KeyResult newKeyResult) { + List actionList = actionPersistenceService + .getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId()); + if (actionList == null) { + return Collections.emptyList(); + } + + List 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; + } } diff --git a/backend/src/main/java/ch/puzzle/okr/service/business/KeyResultBusinessService.java b/backend/src/main/java/ch/puzzle/okr/service/business/KeyResultBusinessService.java index c0c23f6c34..8f2b27ff79 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/business/KeyResultBusinessService.java +++ b/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 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(); + } } diff --git a/backend/src/main/java/ch/puzzle/okr/service/business/ObjectiveBusinessService.java b/backend/src/main/java/ch/puzzle/okr/service/business/ObjectiveBusinessService.java index 22855840f1..7668b78848 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/business/ObjectiveBusinessService.java +++ b/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 keyResults) { + public Objective duplicateObjective(Objective objective, AuthorizationUser authorizationUser, + List 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); diff --git a/backend/src/test/java/ch/puzzle/okr/controller/ObjectiveControllerIT.java b/backend/src/test/java/ch/puzzle/okr/controller/ObjectiveControllerIT.java index d2ef8c0a8c..57957f168b 100644 --- a/backend/src/test/java/ch/puzzle/okr/controller/ObjectiveControllerIT.java +++ b/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 diff --git a/backend/src/test/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationServiceTest.java index 9a0c04d1d7..409611fdb7 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationServiceTest.java @@ -25,15 +25,14 @@ @ExtendWith(MockitoExtension.class) class ObjectiveAuthorizationServiceTest { - @InjectMocks - private ObjectiveAuthorizationService objectiveAuthorizationService; + private final AuthorizationUser authorizationUser = defaultAuthorizationUser(); + private final Objective newObjective = Objective.Builder.builder().withId(5L).withTitle("Objective 1").build(); @Mock ObjectiveBusinessService objectiveBusinessService; @Mock AuthorizationService authorizationService; - private final AuthorizationUser authorizationUser = defaultAuthorizationUser(); - - private final Objective newObjective = Objective.Builder.builder().withId(5L).withTitle("Objective 1").build(); + @InjectMocks + private ObjectiveAuthorizationService objectiveAuthorizationService; @DisplayName("Should return the created objective when authorized") @Test @@ -163,13 +162,8 @@ void shouldThrowExceptionWhenNotAuthorizedToDeleteObjectiveById() { @DisplayName("Should throw an exception when not authorized to duplicate an objective") @Test void shouldThrowExceptionWhenNotAuthorizedToDuplicateObjective() { - Long idExistingObjective = 13L; String reason = "junit test reason"; Objective objective = Objective.Builder.builder().build(); - - List keyResults = new ArrayList<>(); - keyResults.add(metricKeyResult); - when(authorizationService.updateOrAddAuthorizationUser()).thenReturn(authorizationUser); doThrow(new ResponseStatusException(HttpStatus.UNAUTHORIZED, reason)) .when(authorizationService) @@ -177,9 +171,7 @@ void shouldThrowExceptionWhenNotAuthorizedToDuplicateObjective() { ResponseStatusException exception = assertThrows(ResponseStatusException.class, () -> objectiveAuthorizationService - .duplicateEntity(idExistingObjective, - objective, - keyResults)); + .duplicateEntity(objective, new ArrayList<>())); assertEquals(UNAUTHORIZED, exception.getStatusCode()); assertEquals(reason, exception.getReason()); @@ -188,8 +180,6 @@ void shouldThrowExceptionWhenNotAuthorizedToDuplicateObjective() { @DisplayName("Should return duplicated objective when authorized") @Test void shouldReturnDuplicatedObjectiveWhenAuthorized() { - Long idExistingObjective = 13L; - Objective newObjectiveWithoutKeyResults = Objective.Builder .builder() // .withTitle("Objective without KeyResults") @@ -206,11 +196,13 @@ void shouldReturnDuplicatedObjectiveWhenAuthorized() { when(authorizationService.updateOrAddAuthorizationUser()).thenReturn(authorizationUser); when(objectiveBusinessService - .duplicateObjective(idExistingObjective, newObjectiveWithoutKeyResults, authorizationUser, keyResults)) + .duplicateObjective(newObjectiveWithoutKeyResults, + authorizationUser, + keyResults.stream().map(KeyResult::getId).toList())) .thenReturn(newObjectiveWithKeyResults); Objective objective = objectiveAuthorizationService - .duplicateEntity(idExistingObjective, newObjectiveWithoutKeyResults, keyResults); + .duplicateEntity(newObjectiveWithoutKeyResults, keyResults.stream().map(KeyResult::getId).toList()); assertEquals(newObjectiveWithKeyResults, objective); } diff --git a/backend/src/test/java/ch/puzzle/okr/service/business/ActionBusinessServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/business/ActionBusinessServiceTest.java index 1b206b9b99..dc3e339f0e 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/business/ActionBusinessServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/business/ActionBusinessServiceTest.java @@ -1,7 +1,6 @@ package ch.puzzle.okr.service.business; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertIterableEquals; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -15,6 +14,8 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -170,4 +171,64 @@ void shouldDeleteAction() { actionBusinessService.deleteEntityById(action1.getId()); verify(actionPersistenceService, times(1)).deleteById(action1.getId()); } + + @ParameterizedTest(name = "Should return a empty list when actions are {0}") + @NullAndEmptySource + void shouldReturnEmptyListWhenActionsAreEmpty(List actions) { + KeyResult newKeyResult = KeyResultMetric.Builder.builder().withId(9L).build(); + KeyResult oldKeyResult = KeyResultMetric.Builder.builder().withId(6L).build(); + + when(actionPersistenceService.getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId())) + .thenReturn(actions); + + List result = actionBusinessService.duplicateActions(oldKeyResult, newKeyResult); + + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @DisplayName("Should duplicate all actions when there are actions defined") + @Test + void shouldDuplicateActionsWhenActionsExist() { + KeyResult newKeyResult = KeyResultMetric.Builder.builder().withId(9L).build(); + KeyResult oldKeyResult = KeyResultMetric.Builder.builder().withId(6L).build(); + + Action action1 = Action.Builder + .builder() + .withAction("Neuer Drucker") + .withKeyResult(newKeyResult) + .isChecked(false) + .withPriority(1) + .build(); + Action action2 = Action.Builder + .builder() + .withAction("Neues Papier") + .withKeyResult(newKeyResult) + .isChecked(true) + .withPriority(2) + .build(); + + when(actionPersistenceService.getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId())) + .thenReturn(List.of(action1, action2)); + + List result = actionBusinessService.duplicateActions(oldKeyResult, newKeyResult); + + assertNotNull(result); + assertEquals(2, result.size()); + + Action newAction1 = result.getFirst(); + assertEquals("Neuer Drucker", newAction1.getActionPoint()); + assertFalse(newAction1.isChecked()); + assertEquals(1, newAction1.getPriority()); + assertEquals(newKeyResult, newAction1.getKeyResult()); + + Action newAction2 = result.get(1); + assertEquals("Neues Papier", newAction2.getActionPoint()); + assertTrue(newAction2.isChecked()); + assertEquals(2, newAction2.getPriority()); + assertEquals(newKeyResult, newAction2.getKeyResult()); + + verify(validator, times(2)).validate(any(Action.class)); + verify(actionPersistenceService, times(2)).save(any(Action.class)); + } } diff --git a/backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceTest.java index 11062af5cf..788d68679a 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceTest.java @@ -7,6 +7,7 @@ import ch.puzzle.okr.models.Action; import ch.puzzle.okr.models.Objective; +import ch.puzzle.okr.models.Unit; import ch.puzzle.okr.models.User; import ch.puzzle.okr.models.authorization.AuthorizationUser; import ch.puzzle.okr.models.checkin.CheckIn; @@ -26,6 +27,7 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; +import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.http.HttpStatus; import org.springframework.web.server.ResponseStatusException; @@ -44,8 +46,8 @@ class KeyResultBusinessServiceTest { ActionBusinessService actionBusinessService; @Mock AlignmentBusinessService alignmentBusinessService; - @InjectMocks - private KeyResultBusinessService keyResultBusinessService; + @Mock + ObjectiveBusinessService objectiveBusinessService; List keyResults; User user; Objective objective; @@ -56,6 +58,9 @@ class KeyResultBusinessServiceTest { CheckIn checkIn3; List checkIns; List actions; + @InjectMocks + @Spy + private KeyResultBusinessService keyResultBusinessService; @BeforeEach void setup() { @@ -466,4 +471,48 @@ void shouldReturnTrueForImUsedOnKeyResultWithCheckInsAfterTypeChange() { assertTrue(returnValue); } + + @DisplayName("Should successfully duplicate a metric key result") + @Test + void shouldSuccessfullyDuplicateMetricKeyResult() { + Objective objective = Objective.Builder.builder().withId(5L).withTitle("A new Objective").build(); + + KeyResult keyResultMetric = KeyResultMetric.Builder + .builder() + .withId(1L) + .withTitle("Metric KeyResult") + .withDescription("Description of metric key result") + .withOwner(User.Builder.builder().build()) + .withUnit(Unit.NUMBER) + .withBaseline(10.0) + .withStretchGoal(50.0) + .build(); + + KeyResult test = keyResultBusinessService.duplicateKeyResult(authorizationUser, keyResultMetric, objective); + + verify(keyResultBusinessService, times(1)).createEntity(any(KeyResultMetric.class), any()); + verify(actionBusinessService, times(1)).duplicateActions(any(KeyResultMetric.class), any()); + } + + @DisplayName("Should successfully duplicate an ordinal key result") + @Test + void shouldSuccessfullyDuplicateOrdinalKeyResult() { + Objective objective = Objective.Builder.builder().withId(5L).withTitle("A new Objective").build(); + + KeyResult keyResultOrdinal = KeyResultOrdinal.Builder + .builder() + .withId(1L) + .withTitle("Ordinal KeyResult") + .withDescription("Description of metric key result") + .withOwner(User.Builder.builder().build()) + .withCommitZone("We should be commited to this") + .withTargetZone("We could reach this target here") + .withStretchZone("Reaching this would be awesome!") + .build(); + + KeyResult test = keyResultBusinessService.duplicateKeyResult(authorizationUser, keyResultOrdinal, objective); + + verify(keyResultBusinessService, times(1)).createEntity(any(KeyResultOrdinal.class), any()); + verify(actionBusinessService, times(1)).duplicateActions(any(KeyResultOrdinal.class), any()); + } } diff --git a/backend/src/test/java/ch/puzzle/okr/service/business/ObjectiveBusinessServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/business/ObjectiveBusinessServiceTest.java index d7c006e688..47e3422f67 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/business/ObjectiveBusinessServiceTest.java +++ b/backend/src/test/java/ch/puzzle/okr/service/business/ObjectiveBusinessServiceTest.java @@ -13,6 +13,7 @@ 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 java.time.LocalDateTime; @@ -32,7 +33,6 @@ @ExtendWith(MockitoExtension.class) class ObjectiveBusinessServiceTest { - private static final AuthorizationUser authorizationUser = defaultAuthorizationUser(); @InjectMocks @Spy ObjectiveBusinessService objectiveBusinessService; @@ -41,10 +41,15 @@ class ObjectiveBusinessServiceTest { @Mock KeyResultBusinessService keyResultBusinessService; @Mock + ActionBusinessService actionBusinessService; + @Mock CompletedBusinessService completedBusinessService; @Mock ObjectiveValidationService validator = Mockito.mock(ObjectiveValidationService.class); + @Mock + ActionPersistenceService actionPersistenceService; + private static final AuthorizationUser authorizationUser = defaultAuthorizationUser(); private final Team team1 = Team.Builder.builder().withId(1L).withName("Team1").build(); private final Quarter quarter = Quarter.Builder.builder().withId(1L).withLabel("GJ 22/23-Q2").build(); private final User user = User.Builder @@ -206,11 +211,13 @@ void shouldDuplicateObjective() { .build(); KeyResult keyResultOrdinal = KeyResultOrdinal.Builder .builder() // + .withId(1L) // .withTitle("Ordinal 1") // .withObjective(sourceObjective) // .build(); KeyResult keyResultMetric = KeyResultMetric.Builder .builder() // + .withId(2L) // .withTitle("Metric 1") // .withObjective(sourceObjective) // .withUnit(Unit.FTE) // @@ -228,10 +235,13 @@ void shouldDuplicateObjective() { .build(); when(objectivePersistenceService.save(any())).thenReturn(newObjective); + when(keyResultBusinessService.getEntityById(1L)).thenReturn(keyResultOrdinal); + when(keyResultBusinessService.getEntityById(2L)).thenReturn(keyResultMetric); - // act Objective duplicatedObjective = objectiveBusinessService - .duplicateObjective(sourceObjective.getId(), newObjective, authorizationUser, keyResults); + .duplicateObjective(newObjective, + authorizationUser, + keyResults.stream().map(KeyResult::getId).toList()); // assert assertNotEquals(sourceObjective.getId(), duplicatedObjective.getId()); @@ -240,7 +250,7 @@ void shouldDuplicateObjective() { // called for creating the new Objective and the new keyResults verify(objectiveBusinessService, times(1)).createEntity(any(), any()); - verify(keyResultBusinessService, times(2)).createEntity(any(), any()); + verify(keyResultBusinessService, times(2)).duplicateKeyResult(any(), any(), any()); } @DisplayName("Should get all key result associated with the objective on getAllKeyResultsByObjective()") diff --git a/backend/src/test/java/ch/puzzle/okr/service/persistence/ActionPersistenceServiceIT.java b/backend/src/test/java/ch/puzzle/okr/service/persistence/ActionPersistenceServiceIT.java index b4abf2abc4..088d8faa9e 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/persistence/ActionPersistenceServiceIT.java +++ b/backend/src/test/java/ch/puzzle/okr/service/persistence/ActionPersistenceServiceIT.java @@ -22,6 +22,7 @@ @SpringIntegrationTest class ActionPersistenceServiceIT { + private static final String UPDATED_ACTION = "Updated Action"; Action createdAction; @Autowired private ActionPersistenceService actionPersistenceService; @@ -48,8 +49,6 @@ private static Action createAction(Long id, int version) { .build(); } - private static final String UPDATED_ACTION = "Updated Action"; - @BeforeEach void setUp() { TenantContext.setCurrentTenant(TestHelper.SCHEMA_PITC); @@ -119,7 +118,6 @@ void shouldThrowExceptionWhenSaveIsCalledOnAlreadyUpdatedAction() { @Test void shouldReturnListOfAllActionsWhenFindAllIsCalled() { List actions = actionPersistenceService.findAll(); - assertEquals(11, actions.size()); } diff --git a/frontend/cypress/e2e/duplicate-objective.cy.ts b/frontend/cypress/e2e/duplicate-objective.cy.ts index 186781b45a..3bffc7f93e 100644 --- a/frontend/cypress/e2e/duplicate-objective.cy.ts +++ b/frontend/cypress/e2e/duplicate-objective.cy.ts @@ -2,6 +2,8 @@ import * as users from '../fixtures/users.json'; import CyOverviewPage from '../support/helper/dom-helper/pages/overviewPage'; import KeyResultDetailPage from '../support/helper/dom-helper/pages/keyResultDetailPage'; import ObjectiveDialog from '../support/helper/dom-helper/dialogs/objectiveDialog'; +import { uniqueSuffix } from '../support/helper/utils'; +import { Unit } from '../../src/app/shared/types/enums/unit'; let overviewPage = new CyOverviewPage(); @@ -14,6 +16,7 @@ describe('functionality of duplicating objectives and their belonging key-result const firstKeyResultName = 'New structure that rewards funny guys and innovation before the end of Q1.'; const secondKeyResultName = 'Monthly town halls between our people and leadership teams over the next four months.'; const thirdKeyResultName = 'High employee satisfaction scores (80%+) throughout the year.'; + const keyResultDetailPage = new KeyResultDetailPage(); it('should be able to duplicate a objective into this quarter, including all keyResults', () => { const duplicatedTitle = 'This is a duplicated objective with all keyResults'; @@ -41,7 +44,6 @@ describe('functionality of duplicating objectives and their belonging key-result .excludeKeyResults([secondKeyResultName, thirdKeyResultName]) .submit(); - overviewPage.getKeyResultOfObjective(duplicatedTitle, firstKeyResultName); overviewPage @@ -96,6 +98,73 @@ describe('functionality of duplicating objectives and their belonging key-result cy.contains(duplicatedTitle) .should('not.exist'); }); + + it('should duplicate all attributes of the corresponding keyResults as well', () => { + const objectiveTitle = uniqueSuffix('What an awesome objective.'); + const duplicatedTitle = uniqueSuffix('This objective has two keyResults with lots and lots of values that should be duplicated!'); + + overviewPage + .addObjective('LoremIpsum') + .fillObjectiveTitle(objectiveTitle) + .submit(); + + overviewPage + .addKeyResult('LoremIpsum', objectiveTitle) + .fillKeyResultTitle('A metric keyResult with lots of values') + .withMetricValues(Unit.CHF, '1', '5') + .fillKeyResultDescription('Its very sunny today.') + .addActionPlanElement('Action') + .addActionPlanElement('Plan') + .addActionPlanElement('Element') + .submit(); + + overviewPage + .addKeyResult('LoremIpsum', objectiveTitle) + .fillKeyResultTitle('A ordinal keyResult with lots of values') + .withOrdinalValues('CommitZ', 'TargetZ', 'StretchZ') + .fillKeyResultDescription('Its very sunny today.') + .addActionPlanElement('One') + .addActionPlanElement('More') + .addActionPlanElement('Element') + .submit(); + + overviewPage + .duplicateObjective(objectiveTitle) + .fillObjectiveTitle(duplicatedTitle) + .submit(); + + cy.contains(duplicatedTitle); + + overviewPage.getKeyResultOfObjective(duplicatedTitle, 'A metric keyResult with lots of values') + .click(); + cy.contains('Baseline: 1 CHF'); + cy.contains('Stretch Goal: 5 CHF'); + cy.contains('Metrisch'); + cy.contains('Jaya Norris'); + cy.contains('Its very sunny today.'); + cy.contains('Action Plan') + .then(() => { + cy.contains('Action'); + cy.contains('Plan'); + cy.contains('Element'); + }); + keyResultDetailPage.close(); + + overviewPage.getKeyResultOfObjective(duplicatedTitle, 'A ordinal keyResult with lots of values') + .click(); + cy.contains('CommitZ'); + cy.contains('TargetZ'); + cy.contains('StretchZ'); + cy.contains('Ordinal'); + cy.contains('Jaya Norris'); + cy.contains('Its very sunny today.'); + cy.contains('Action Plan') + .then(() => { + cy.contains('One'); + cy.contains('More'); + cy.contains('Element'); + }); + }); }); describe('verify functionality of scoring adjustment on duplicated objectives', () => { diff --git a/frontend/cypress/support/helper/dom-helper/pages/overviewPage.ts b/frontend/cypress/support/helper/dom-helper/pages/overviewPage.ts index d6b7a22354..4a48d7e3d4 100644 --- a/frontend/cypress/support/helper/dom-helper/pages/overviewPage.ts +++ b/frontend/cypress/support/helper/dom-helper/pages/overviewPage.ts @@ -34,7 +34,7 @@ export default class CyOverviewPage extends Page { addObjective(teamName?: string) { if (teamName) { this.getTeamByName(teamName) - .find('.add-objective') + .findByTestId('add-objective') .first() .click(); return new ObjectiveDialog(); diff --git a/frontend/src/app/services/objective.service.ts b/frontend/src/app/services/objective.service.ts index daa28f321f..2139906c14 100644 --- a/frontend/src/app/services/objective.service.ts +++ b/frontend/src/app/services/objective.service.ts @@ -3,15 +3,13 @@ import { HttpClient } from '@angular/common/http'; import { Objective } from '../shared/types/model/objective'; import { Observable } from 'rxjs'; import { KeyResultDto } from '../shared/types/DTOs/key-result-dto'; -import { User } from '../shared/types/model/user'; -import { CheckIn } from '../shared/types/model/check-in'; -import { Action } from '../shared/types/model/action'; @Injectable({ providedIn: 'root' }) export class ObjectiveService { - constructor(private httpClient: HttpClient) {} + constructor(private httpClient: HttpClient) { + } getFullObjective(id: number) { return this.httpClient.get('/api/v2/objectives/' + id); @@ -33,23 +31,10 @@ export class ObjectiveService { return this.httpClient.delete(`/api/v2/objectives/${objectiveId}`); } - duplicateObjective(objectiveId: number, - duplicateObjectiveDto: { - keyResults: { - owner: User; - modifiedOn: Date | null | undefined; - keyResultType: string | undefined; - description: string; - actionList: Action[] | null; - id: undefined; - lastCheckIn: CheckIn | null | undefined; - title: string; - version: number; - createdOn: Date | null | undefined; - objective: Objective; - }[]; - objective: any; - }): Observable { - return this.httpClient.post(`/api/v2/objectives/${objectiveId}`, duplicateObjectiveDto); + duplicateObjective(duplicateObjectiveDto: { + keyResultIds: (number | undefined)[]; + objective: any; + }): Observable { + return this.httpClient.post('/api/v2/objectives/duplicate', duplicateObjectiveDto); } } diff --git a/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts b/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts index de1f6738b6..cc09882551 100644 --- a/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts +++ b/frontend/src/app/shared/dialog/objective-dialog/objective-form.component.ts @@ -185,14 +185,11 @@ export class ObjectiveFormComponent implements OnInit, OnDestroy { if (this.data.action == 'duplicate' && id) { objectiveDTO.id = null; objectiveDTO.state = 'DRAFT' as State; - return this.objectiveService.duplicateObjective(id, { + return this.objectiveService.duplicateObjective({ objective: objectiveDTO, - keyResults: this.keyResults + keyResultIds: this.keyResults .filter((keyResult, index) => this.objectiveForm.value.keyResults?.[index] ?? false) - .map((result) => ({ - ...result, - id: undefined - })) + .map((keyResult) => keyResult.id) }); } else { if (this.data.action == 'releaseBacklog') {