Skip to content

Commit

Permalink
remove action list attribute
Browse files Browse the repository at this point in the history
  • Loading branch information
nevio18324 committed Jan 14, 2025
1 parent 8bfb604 commit 70d3ebd
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 97 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ public abstract class KeyResult implements WriteableInterface {
@Column(name = "key_result_type", insertable = false, updatable = false)
private String keyResultType;

@OneToMany(fetch = FetchType.LAZY, mappedBy = "keyResult", cascade = CascadeType.ALL)
private List<Action> actionList;

@Transient
private boolean writeable;

Expand Down Expand Up @@ -131,14 +128,6 @@ private void setKeyResultType(String keyResultType) {
this.keyResultType = keyResultType;
}

public List<Action> getActionList() {
return actionList;
}

public void setActionList(List<Action> actionList) {
this.actionList = actionList;
}

@Override
public boolean isWriteable() {
return writeable;
Expand All @@ -154,7 +143,7 @@ public String toString() {
return "KeyResult{" + "id=" + id + ", version=" + version + ", objective=" + objective + ", title='" + title
+ '\'' + ", description='" + description + '\'' + ", owner=" + owner + ", createdBy=" + createdBy
+ ", createdOn=" + createdOn + ", modifiedOn=" + modifiedOn + ", keyResultType='" + keyResultType
+ ", actionList=" + actionList + ", writeable=" + writeable + '\'' + '}';
+ ", writeable=" + writeable + '\'' + '}';
}

@Override
Expand All @@ -169,8 +158,7 @@ public boolean equals(Object o) {
&& Objects.equals(description, keyResult.description) && Objects.equals(owner, keyResult.owner)
&& Objects.equals(createdBy, keyResult.createdBy) && Objects.equals(createdOn, keyResult.createdOn)
&& Objects.equals(modifiedOn, keyResult.modifiedOn)
&& Objects.equals(keyResultType, keyResult.keyResultType)
&& Objects.equals(actionList, keyResult.actionList);
&& Objects.equals(keyResultType, keyResult.keyResultType);
}

@Override
Expand All @@ -185,8 +173,7 @@ public int hashCode() {
createdBy,
createdOn,
modifiedOn,
keyResultType,
actionList);
keyResultType);
}

protected KeyResult() {
Expand All @@ -203,7 +190,6 @@ protected KeyResult(Builder<?> builder) {
setCreatedOn(builder.createdOn);
setModifiedOn(builder.modifiedOn);
setKeyResultType(builder.keyResultType);
setActionList(builder.actionList);
}

@SuppressWarnings(value = "unchecked")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,41 +1,45 @@
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;
import java.time.LocalDateTime;
import java.util.List;
import java.util.Objects;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.annotation.Lazy;
import org.springframework.stereotype.Service;

import java.time.LocalDateTime;
import java.util.List;
import java.util.Objects;

import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_METRIC;
import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_ORDINAL;

@Service
public class ObjectiveBusinessService implements BusinessServiceInterface<Long, Objective> {
private static final Logger logger = LoggerFactory.getLogger(ObjectiveBusinessService.class);
private final ObjectivePersistenceService objectivePersistenceService;
private final ObjectiveValidationService validator;
private final KeyResultBusinessService keyResultBusinessService;
private final CompletedBusinessService completedBusinessService;
private final ActionPersistenceService actionPersistenceService;

public ObjectiveBusinessService(@Lazy KeyResultBusinessService keyResultBusinessService,
ObjectiveValidationService validator,
ObjectivePersistenceService objectivePersistenceService,
CompletedBusinessService completedBusinessService) {
CompletedBusinessService completedBusinessService, ActionPersistenceService actionPersistenceService) {
this.keyResultBusinessService = keyResultBusinessService;
this.validator = validator;
this.objectivePersistenceService = objectivePersistenceService;
this.completedBusinessService = completedBusinessService;
this.actionPersistenceService = actionPersistenceService;
}

private static boolean hasQuarterChanged(Objective objective, Objective savedObjective) {
Expand Down Expand Up @@ -110,12 +114,10 @@ public Objective createEntity(Objective objective, AuthorizationUser authorizati
/**
* Create a new Objective (with a new ID) and copy from a source Objective
* the KeyResults. The CheckIns are not copied.
* @param objective
* New Objective with no KeyResults
* @param authorizationUser
* AuthorizationUser
* @param keyResultIds
* KeyResultIds to copy
*
* @param objective New Objective with no KeyResults
* @param authorizationUser AuthorizationUser
* @param keyResultIds KeyResultIds to copy
* @return New Objective with copied KeyResults form the source Objective
*/
@Transactional
Expand All @@ -124,8 +126,8 @@ public Objective duplicateObjective(Objective objective, AuthorizationUser autho
Objective duplicatedObjective = createEntity(objective, authorizationUser);
for (Long keyResult : keyResultIds) {
duplicateKeyResult(authorizationUser,
keyResultBusinessService.getEntityById(keyResult),
duplicatedObjective);
keyResultBusinessService.getEntityById(keyResult),
duplicatedObjective);
}
return duplicatedObjective;
}
Expand All @@ -152,7 +154,7 @@ public KeyResult makeCopyOfKeyResultMetric(KeyResult keyResult, Objective duplic
.withBaseline(((KeyResultMetric) keyResult).getBaseline()) //
.withStretchGoal(((KeyResultMetric) keyResult).getStretchGoal()) //
.build();
copyOfMetricKeyResult.setActionList(duplicateActionList(keyResult, copyOfMetricKeyResult));
actionPersistenceService.duplicateActionList(keyResult, copyOfMetricKeyResult);
return copyOfMetricKeyResult;
}

Expand All @@ -167,28 +169,10 @@ public KeyResult makeCopyOfKeyResultOrdinal(KeyResult keyResult, Objective dupli
.withTargetZone(((KeyResultOrdinal) keyResult).getTargetZone()) //
.withStretchZone(((KeyResultOrdinal) keyResult).getStretchZone()) //
.build();

copyOfOrdinalKeyResult.setActionList(duplicateActionList(keyResult, copyOfOrdinalKeyResult));
actionPersistenceService.duplicateActionList(copyOfOrdinalKeyResult, keyResult);
return copyOfOrdinalKeyResult;
}

public List<Action> duplicateActionList(KeyResult oldKeyResult, KeyResult newKeyResult) {
List<Action> actionList = oldKeyResult.getActionList();
if (actionList != null) {
actionList = actionList
.stream()
.map(e -> Action.Builder
.builder()
.withKeyResult(newKeyResult)
.isChecked(e.isChecked())
.withAction(e.getActionPoint())
.withPriority(e.getPriority())
.withVersion(e.getVersion())
.build())
.toList();
}
return actionList;
}

@Transactional
public void deleteEntityById(Long id) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package ch.puzzle.okr.service.persistence;

import static ch.puzzle.okr.Constants.ACTION;

import ch.puzzle.okr.models.Action;
import ch.puzzle.okr.models.keyresult.KeyResult;
import ch.puzzle.okr.repository.ActionRepository;
import java.util.List;
import org.springframework.stereotype.Service;

import java.util.List;

import static ch.puzzle.okr.Constants.ACTION;

@Service
public class ActionPersistenceService extends PersistenceBase<Action, Long, ActionRepository> {

Expand All @@ -22,4 +24,15 @@ public String getModelName() {
public List<Action> getActionsByKeyResultIdOrderByPriorityAsc(Long keyResultId) {
return getRepository().getActionsByKeyResultIdOrderByPriorityAsc(keyResultId);
}

public void duplicateActionList(KeyResult oldKeyResult, KeyResult newKeyResult) {
List<Action> actionList = getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId());
if (actionList != null) {
actionList.forEach(action -> {
action.setKeyResult(newKeyResult);
action.resetId();
this.save(action);
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -43,6 +44,8 @@ class ObjectiveBusinessServiceTest {
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();
Expand Down Expand Up @@ -281,39 +284,6 @@ void shouldDuplicateOrdinalKeyResult() {
verify(keyResultBusinessService, times(1)).createEntity(any(KeyResult.class), any(AuthorizationUser.class));
}

@DisplayName("Should duplicate ActionList")
@Test
void shouldDuplicateActionList() {
KeyResult oldKeyResult = KeyResultMetric.Builder.builder().withId(1L).build();
KeyResult newKeyResult = KeyResultMetric.Builder.builder().withId(2L).build();

Action action = Action.Builder
.builder()
.withKeyResult(oldKeyResult)
.withPriority(1)
.withVersion(1)
.isChecked(true)
.withAction("Action Point 1")
.build();

oldKeyResult.setActionList(List.of(action));

newKeyResult.setActionList(objectiveBusinessService.duplicateActionList(oldKeyResult, newKeyResult));

// assert that id changes to new key result
assertNotEquals(newKeyResult.getActionList().getFirst().getKeyResult().getId(),
oldKeyResult.getActionList().getFirst().getKeyResult().getId());

// assert that all other attributes stay the same
assertThat(newKeyResult.getActionList().getFirst())
.extracting("priority", "version", "checked", "actionPoint")
.containsExactly(oldKeyResult.getActionList().getFirst().getPriority(),
oldKeyResult.getActionList().getFirst().getVersion(),
oldKeyResult.getActionList().getFirst().isChecked(),
oldKeyResult.getActionList().getFirst().getActionPoint());

}

@DisplayName("Should get all key result associated with the objective on getAllKeyResultsByObjective()")
@Test
void shouldGetAllKeyResultsByObjective() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,31 @@
package ch.puzzle.okr.service.persistence;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.*;
import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY;

import ch.puzzle.okr.dto.ErrorDto;
import ch.puzzle.okr.exception.OkrResponseStatusException;
import ch.puzzle.okr.models.Action;
import ch.puzzle.okr.models.Objective;
import ch.puzzle.okr.models.keyresult.KeyResult;
import ch.puzzle.okr.models.keyresult.KeyResultMetric;
import ch.puzzle.okr.multitenancy.TenantContext;
import ch.puzzle.okr.test.SpringIntegrationTest;
import ch.puzzle.okr.test.TestHelper;
import java.util.List;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.server.ResponseStatusException;

import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.when;
import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY;

@SpringIntegrationTest
class ActionPersistenceServiceIT {
private static final String UPDATED_ACTION = "Updated Action";
Action createdAction;
@Autowired
private ActionPersistenceService actionPersistenceService;
Expand All @@ -48,8 +52,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);
Expand Down Expand Up @@ -107,7 +109,7 @@ void shouldThrowExceptionWhenSaveIsCalledOnAlreadyUpdatedAction() {
changedAction.setActionPoint(UPDATED_ACTION);

OkrResponseStatusException exception = assertThrows(OkrResponseStatusException.class,
() -> actionPersistenceService.save(changedAction));
() -> actionPersistenceService.save(changedAction));
List<ErrorDto> expectedErrors = List.of(new ErrorDto("DATA_HAS_BEEN_UPDATED", List.of("Action")));

assertEquals(UNPROCESSABLE_ENTITY, exception.getStatusCode());
Expand All @@ -119,7 +121,7 @@ void shouldThrowExceptionWhenSaveIsCalledOnAlreadyUpdatedAction() {
@Test
void shouldReturnListOfAllActionsWhenFindAllIsCalled() {
List<Action> actions = actionPersistenceService.findAll();

System.out.println(actions.getFirst());
assertEquals(11, actions.size());
}

Expand Down Expand Up @@ -160,4 +162,31 @@ void shouldDeleteActionById() {
actions = actionPersistenceService.findAll();
assertEquals(11, actions.size());
}

@DisplayName("Should duplicate ActionList")
@Test
void shouldDuplicateActionList() {
KeyResult newKeyResult = KeyResultMetric.Builder.builder().withId(9L).build();
KeyResult oldKeyResult = KeyResultMetric.Builder.builder().withId(6L).build();

actionPersistenceService.duplicateActionList(oldKeyResult, newKeyResult);

List<Action> newKeyResultActionList = actionPersistenceService.getActionsByKeyResultIdOrderByPriorityAsc(newKeyResult.getId());
List<Action> oldKeyResultActionList = actionPersistenceService.getActionsByKeyResultIdOrderByPriorityAsc(oldKeyResult.getId());

// assert that id changes to new key result
assertNotEquals(newKeyResultActionList.getFirst().getKeyResult().getId(),
oldKeyResultActionList.getFirst().getKeyResult().getId());

// assert that all other attributes stay the same
assertThat(newKeyResultActionList.getFirst())
.extracting("priority", "version", "checked", "actionPoint")
.containsExactly(
oldKeyResultActionList.getFirst().getPriority(),
oldKeyResultActionList.getFirst().getVersion(),
oldKeyResultActionList.getFirst().isChecked(),
oldKeyResultActionList.getFirst().getActionPoint());

}

}

0 comments on commit 70d3ebd

Please sign in to comment.