From cc94827ee81bf254a7d6191823d624fc41f1ba28 Mon Sep 17 00:00:00 2001 From: Manuel Date: Wed, 15 Jan 2025 15:43:09 +0100 Subject: [PATCH] Resolve conversations --- .../business/ActionBusinessService.java | 2 +- .../business/KeyResultBusinessService.java | 29 ++++++++ .../business/ObjectiveBusinessService.java | 34 +-------- .../persistence/ActionPersistenceService.java | 4 - .../business/ActionBusinessServiceTest.java | 4 +- .../KeyResultBusinessServiceTest.java | 51 +++++++++++++ .../ObjectiveBusinessServiceTest.java | 48 ------------ .../ActionPersistenceServiceIT.java | 1 - .../cypress/e2e/duplicate-objective.cy.ts | 73 ++++++++++++++++--- .../helper/dom-helper/pages/overviewPage.ts | 2 +- 10 files changed, 150 insertions(+), 98 deletions(-) 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 3eac43a6af..380ea64873 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 @@ -77,7 +77,7 @@ public void deleteEntitiesByKeyResultId(Long keyResultId) { getActionsByKeyResultId(keyResultId).forEach(action -> deleteEntityById(action.getId())); } - public List createDuplicateOfActions(KeyResult oldKeyResult, KeyResult newKeyResult) { + public List createDuplicates(KeyResult oldKeyResult, KeyResult newKeyResult) { List actionList = actionPersistenceService .getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId()); if (actionList == null) { 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..0a34a9b5d9 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,12 @@ package ch.puzzle.okr.service.business; 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 +140,30 @@ private boolean isKeyResultTypeChangeable(Long id) { public List getKeyResultsOwnedByUser(long id) { return keyResultPersistenceService.getKeyResultsOwnedByUser(id); } + + public 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(); + } + + public 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 8d3690fd9e..1b700ec013 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 @@ -143,43 +143,17 @@ public void duplicateKeyResult(AuthorizationUser authorizationUser, KeyResult ke KeyResult newKeyResult = null; if (keyResult.getKeyResultType().equals(KEY_RESULT_TYPE_METRIC)) { - KeyResult keyResultMetric = makeCopyOfKeyResultMetric(keyResult, duplicatedObjective); + KeyResult keyResultMetric = keyResultBusinessService.makeCopyOfKeyResultMetric(keyResult, duplicatedObjective); newKeyResult = keyResultBusinessService.createEntity(keyResultMetric, authorizationUser); } else if (keyResult.getKeyResultType().equals(KEY_RESULT_TYPE_ORDINAL)) { - KeyResult keyResultOrdinal = makeCopyOfKeyResultOrdinal(keyResult, duplicatedObjective); + KeyResult keyResultOrdinal = keyResultBusinessService.makeCopyOfKeyResultOrdinal(keyResult, duplicatedObjective); newKeyResult = keyResultBusinessService.createEntity(keyResultOrdinal, authorizationUser); } else { throw new UnsupportedOperationException("Unsupported KeyResultType: " + keyResult.getKeyResultType()); } - List copiedActions = actionBusinessService.createDuplicateOfActions(keyResult, newKeyResult); - actionPersistenceService.saveDuplicatedActions(copiedActions); - } - - public 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(); - } - - public 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(); + List copiedActions = actionBusinessService.createDuplicates(keyResult, newKeyResult); + copiedActions.forEach(actionPersistenceService::save); } @Transactional diff --git a/backend/src/main/java/ch/puzzle/okr/service/persistence/ActionPersistenceService.java b/backend/src/main/java/ch/puzzle/okr/service/persistence/ActionPersistenceService.java index c4d1076ba6..2ceb3685fe 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/persistence/ActionPersistenceService.java +++ b/backend/src/main/java/ch/puzzle/okr/service/persistence/ActionPersistenceService.java @@ -22,8 +22,4 @@ public String getModelName() { public List getActionsByKeyResultIdOrderByPriorityAsc(Long keyResultId) { return getRepository().getActionsByKeyResultIdOrderByPriorityAsc(keyResultId); } - - public void saveDuplicatedActions(List actions) { - actions.forEach(this::save); - } } 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 bab1dc14dd..c5bc4fec3c 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 @@ -180,7 +180,7 @@ void shouldReturnEmptyListWhenActionsAreEmpty() { when(actionPersistenceService.getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId())) .thenReturn(Collections.emptyList()); - List result = actionBusinessService.createDuplicateOfActions(oldKeyResult, newKeyResult); + List result = actionBusinessService.createDuplicates(oldKeyResult, newKeyResult); assertNotNull(result); assertTrue(result.isEmpty()); @@ -210,7 +210,7 @@ void shouldDuplicateActionsWhenActionsExist() { when(actionPersistenceService.getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId())) .thenReturn(List.of(action1, action2)); - List result = actionBusinessService.createDuplicateOfActions(oldKeyResult, newKeyResult); + List result = actionBusinessService.createDuplicates(oldKeyResult, newKeyResult); assertNotNull(result); assertEquals(2, result.size()); 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..50fda38dd9 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 @@ -44,6 +44,8 @@ class KeyResultBusinessServiceTest { ActionBusinessService actionBusinessService; @Mock AlignmentBusinessService alignmentBusinessService; + @Mock + ObjectiveBusinessService objectiveBusinessService; @InjectMocks private KeyResultBusinessService keyResultBusinessService; List keyResults; @@ -466,4 +468,53 @@ void shouldReturnTrueForImUsedOnKeyResultWithCheckInsAfterTypeChange() { assertTrue(returnValue); } + + // To be refactored + @DisplayName("Should duplicate metric key result") + @Test + void shouldDuplicateMetricKeyResult() { + Objective objective = Objective.Builder + .builder() // + .withId(23L) // + .withTitle("Objective 1") // + .build(); + KeyResult keyResultMetric = KeyResultMetric.Builder.builder().withId(1L).withTitle("Metric").build(); + + KeyResult newKeyResult = KeyResultMetric.Builder.builder().withId(2L).withTitle("Metric Copy").build(); + + when(keyResultBusinessService.makeCopyOfKeyResultMetric(keyResultMetric, objective)) + .thenReturn(keyResultMetric); + when(keyResultBusinessService.createEntity(any(KeyResult.class), any(AuthorizationUser.class))) + .thenReturn(newKeyResult); + + objectiveBusinessService.duplicateKeyResult(authorizationUser, keyResultMetric, objective); + + verify(keyResultBusinessService, times(1)).makeCopyOfKeyResultMetric(keyResultMetric, objective); + verify(keyResultBusinessService, times(1)).createEntity(any(KeyResult.class), any(AuthorizationUser.class)); + verify(actionBusinessService, times(1)).createDuplicates(keyResultMetric, newKeyResult); + } + + @DisplayName("Should duplicate ordinal key result") + @Test + void shouldDuplicateOrdinalKeyResult() { + Objective objective = Objective.Builder + .builder() // + .withId(23L) // + .withTitle("Objective 1") // + .build(); + KeyResult keyResultOrdinal = KeyResultOrdinal.Builder.builder().withId(1L).withTitle("Ordinal").build(); + + KeyResult newKeyResult = KeyResultOrdinal.Builder.builder().withId(2L).withTitle("Ordinal Copy").build(); + + when(keyResultBusinessService.makeCopyOfKeyResultOrdinal(keyResultOrdinal, objective)) + .thenReturn(keyResultOrdinal); + when(keyResultBusinessService.createEntity(any(KeyResult.class), any(AuthorizationUser.class))) + .thenReturn(newKeyResult); + + objectiveBusinessService.duplicateKeyResult(authorizationUser, keyResultOrdinal, objective); + + verify(keyResultBusinessService, times(1)).makeCopyOfKeyResultOrdinal(keyResultOrdinal, objective); + verify(keyResultBusinessService, times(1)).createEntity(any(KeyResult.class), any(AuthorizationUser.class)); + verify(actionBusinessService, times(1)).createDuplicates(keyResultOrdinal, newKeyResult); + } } 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 e07e199e74..5a8f82d665 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 @@ -255,54 +255,6 @@ void shouldDuplicateObjective() { verify(keyResultBusinessService, times(2)).createEntity(any(), any()); } - @DisplayName("Should duplicate metric key result") - @Test - void shouldDuplicateMetricKeyResult() { - Objective objective = Objective.Builder - .builder() // - .withId(23L) // - .withTitle("Objective 1") // - .build(); - KeyResult keyResultMetric = KeyResultMetric.Builder.builder().withId(1L).withTitle("Metric").build(); - - KeyResult newKeyResult = KeyResultMetric.Builder.builder().withId(2L).withTitle("Metric Copy").build(); - - when(objectiveBusinessService.makeCopyOfKeyResultMetric(keyResultMetric, objective)) - .thenReturn(keyResultMetric); - when(keyResultBusinessService.createEntity(any(KeyResult.class), any(AuthorizationUser.class))) - .thenReturn(newKeyResult); - - objectiveBusinessService.duplicateKeyResult(authorizationUser, keyResultMetric, objective); - - verify(objectiveBusinessService, times(1)).makeCopyOfKeyResultMetric(keyResultMetric, objective); - verify(keyResultBusinessService, times(1)).createEntity(any(KeyResult.class), any(AuthorizationUser.class)); - verify(actionBusinessService, times(1)).createDuplicateOfActions(keyResultMetric, newKeyResult); - } - - @DisplayName("Should duplicate ordinal key result") - @Test - void shouldDuplicateOrdinalKeyResult() { - Objective objective = Objective.Builder - .builder() // - .withId(23L) // - .withTitle("Objective 1") // - .build(); - KeyResult keyResultOrdinal = KeyResultOrdinal.Builder.builder().withId(1L).withTitle("Ordinal").build(); - - KeyResult newKeyResult = KeyResultOrdinal.Builder.builder().withId(2L).withTitle("Ordinal Copy").build(); - - when(objectiveBusinessService.makeCopyOfKeyResultOrdinal(keyResultOrdinal, objective)) - .thenReturn(keyResultOrdinal); - when(keyResultBusinessService.createEntity(any(KeyResult.class), any(AuthorizationUser.class))) - .thenReturn(newKeyResult); - - objectiveBusinessService.duplicateKeyResult(authorizationUser, keyResultOrdinal, objective); - - verify(objectiveBusinessService, times(1)).makeCopyOfKeyResultOrdinal(keyResultOrdinal, objective); - verify(keyResultBusinessService, times(1)).createEntity(any(KeyResult.class), any(AuthorizationUser.class)); - verify(actionBusinessService, times(1)).createDuplicateOfActions(keyResultOrdinal, newKeyResult); - } - @DisplayName("Should get all key result associated with the objective on getAllKeyResultsByObjective()") @Test void shouldGetAllKeyResultsByObjective() { 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 6624e14c37..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 @@ -118,7 +118,6 @@ void shouldThrowExceptionWhenSaveIsCalledOnAlreadyUpdatedAction() { @Test void shouldReturnListOfAllActionsWhenFindAllIsCalled() { List actions = actionPersistenceService.findAll(); - System.out.println(actions.getFirst()); assertEquals(11, actions.size()); } diff --git a/frontend/cypress/e2e/duplicate-objective.cy.ts b/frontend/cypress/e2e/duplicate-objective.cy.ts index 3679042162..c0f4143007 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(); @@ -96,22 +98,71 @@ describe('functionality of duplicating objectives and their belonging key-result cy.contains(duplicatedTitle) .should('not.exist'); }); - it('should duplicate all attributes of key-result', () => { + + it.only('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 - .duplicateObjective('Build a company culture that kills the competition.') + .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(); - keyResultDetailPage.visit('New structure that rewards funny guys and innovation before the end of Q1.'); - cy.contains('Baseline: 5%'); - cy.contains('Stretch Goal: 1%'); - cy.contains('5/10'); + + 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('Paco Eggimann'); - cy.contains('GJ 24/25-Q3'); + 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('Neue Pflanzen'); - cy.contains('Ein Buch'); - cy.contains('Mehr Getränke'); + cy.contains('One'); + cy.contains('More'); + cy.contains('Element'); }); }); }); diff --git a/frontend/cypress/support/helper/dom-helper/pages/overviewPage.ts b/frontend/cypress/support/helper/dom-helper/pages/overviewPage.ts index dbb9fcc6f4..f9336c6866 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();