Skip to content

Commit

Permalink
Add version to action, adjust tests and remove bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
lkleisa committed Nov 2, 2023
1 parent 7c72f3b commit ddbf9f3
Show file tree
Hide file tree
Showing 24 changed files with 117 additions and 150 deletions.
4 changes: 2 additions & 2 deletions backend/src/main/java/ch/puzzle/okr/dto/ActionDto.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package ch.puzzle.okr.dto;

public record ActionDto(Long id, String action, int priority, boolean isChecked, Long keyResultId) {
public record ActionDto(Long id, int version, String action, int priority, boolean isChecked, Long keyResultId) {

public ActionDto withKeyResultId(Long keyResultId) {
return new ActionDto(id(), action(), priority(), isChecked(), keyResultId);
return new ActionDto(id(), version(), action(), priority(), isChecked(), keyResultId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
@JsonDeserialize(as = KeyResultMetricDto.class)
public record KeyResultMetricDto(Long id, int version, String keyResultType, String title, String description,
Double baseline, Double stretchGoal, Unit unit, KeyResultUserDto owner, KeyResultObjectiveDto objective,
KeyResultLastCheckInMetricDto lastCheckIn, LocalDateTime createdOn, LocalDateTime modifiedOn, boolean writeable, List<ActionDto> actionList)
implements KeyResultDto {
KeyResultLastCheckInMetricDto lastCheckIn, LocalDateTime createdOn, LocalDateTime modifiedOn, boolean writeable,
List<ActionDto> actionList) implements KeyResultDto {
@Override
public List<ActionDto> getActionList() {
return this.actionList;
Expand Down
8 changes: 4 additions & 4 deletions backend/src/main/java/ch/puzzle/okr/mapper/ActionMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ public ActionMapper(KeyResultAuthorizationService keyResultAuthorizationService)
}

public ActionDto toDto(Action action) {
return new ActionDto(action.getId(), action.getAction(), action.getPriority(), action.isChecked(),
action.getKeyResult().getId());
return new ActionDto(action.getId(), action.getVersion(), action.getAction(), action.getPriority(),
action.isChecked(), action.getKeyResult().getId());
}

public List<Action> toActions(List<ActionDto> actionDtos) {
return actionDtos.stream().map(this::toAction).toList();
}

public Action toAction(ActionDto actionDto) {
return Action.Builder.builder().withId(actionDto.id()).withAction(actionDto.action())
.withPriority(actionDto.priority()).withIsChecked(actionDto.isChecked())
return Action.Builder.builder().withId(actionDto.id()).withVersion(actionDto.version())
.withAction(actionDto.action()).withPriority(actionDto.priority()).withIsChecked(actionDto.isChecked())
.withKeyResult(
keyResultAuthorizationService.getBusinessService().getEntityById(actionDto.keyResultId()))
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import ch.puzzle.okr.models.checkin.CheckInMetric;
import ch.puzzle.okr.models.keyresult.KeyResult;
import ch.puzzle.okr.models.keyresult.KeyResultMetric;
import ch.puzzle.okr.service.authorization.ActionAuthorizationService;
import ch.puzzle.okr.service.business.ActionBusinessService;
import ch.puzzle.okr.service.business.CheckInBusinessService;
import ch.puzzle.okr.service.business.ObjectiveBusinessService;
import ch.puzzle.okr.service.business.UserBusinessService;
Expand All @@ -20,17 +20,17 @@ public class KeyResultMetricMapper {

private final UserBusinessService userBusinessService;
private final ObjectiveBusinessService objectiveBusinessService;
private final ActionAuthorizationService actionAuthorizationService;
private final ActionBusinessService actionBusinessService;
private final CheckInBusinessService checkInBusinessService;
private final ActionMapper actionMapper;

public KeyResultMetricMapper(UserBusinessService userBusinessService,
ObjectiveBusinessService objectiveBusinessService, CheckInBusinessService checkInBusinessService,
ActionAuthorizationService actionAuthorizationService, ActionMapper actionMapper) {
ActionBusinessService actionBusinessService, ActionMapper actionMapper) {
this.userBusinessService = userBusinessService;
this.objectiveBusinessService = objectiveBusinessService;
this.checkInBusinessService = checkInBusinessService;
this.actionAuthorizationService = actionAuthorizationService;
this.actionBusinessService = actionBusinessService;
this.actionMapper = actionMapper;
}

Expand All @@ -43,12 +43,13 @@ public KeyResultDto toKeyResultMetricDto(KeyResultMetric keyResult) {
KeyResultObjectiveDto objectiveDto = new KeyResultObjectiveDto(keyResult.getObjective().getId(),
keyResult.getObjective().getState().toString(), quarterDto);
KeyResultLastCheckInMetricDto lastCheckInDto = getLastCheckInDto(keyResult.getId());
List<Action> actionList = actionAuthorizationService.getEntitiesByKeyResultId(keyResult.getId());
List<Action> actionList = actionBusinessService.getActionsByKeyResultId(keyResult.getId());

return new KeyResultMetricDto(keyResult.getId(), keyResult.getVersion(), keyResult.getKeyResultType(),
keyResult.getTitle(), keyResult.getDescription(), keyResult.getBaseline(), keyResult.getStretchGoal(),
keyResult.getUnit(), ownerDto, objectiveDto, lastCheckInDto, keyResult.getCreatedOn(),
keyResult.getModifiedOn(), keyResult.isWriteable(), actionList.stream().map(actionMapper::toDto).toList());
keyResult.getModifiedOn(), keyResult.isWriteable(),
actionList.stream().map(actionMapper::toDto).toList());
}

public KeyResult toKeyResultMetric(KeyResultMetricDto keyResultMetricDto) {
Expand Down
27 changes: 23 additions & 4 deletions backend/src/main/java/ch/puzzle/okr/models/Action.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ public class Action implements WriteableInterface {
@GeneratedValue(strategy = GenerationType.AUTO, generator = "sequence_action")
private Long id;

@Version
private int version;

@NotNull(message = "Action must not be null")
@Size(max = 4096, message = "Attribute Action has a max length of 4096 characters")
private String action;
Expand All @@ -34,6 +37,7 @@ public Action() {

private Action(Builder builder) {
id = builder.id;
version = builder.version;
action = builder.action;
priority = builder.priority;
isChecked = builder.isChecked;
Expand Down Expand Up @@ -76,6 +80,14 @@ public void setKeyResult(KeyResult keyResult) {
this.keyResult = keyResult;
}

public int getVersion() {
return version;
}

public void setVersion(int version) {
this.version = version;
}

@Override
public boolean isWriteable() {
return writeable;
Expand All @@ -88,8 +100,8 @@ public void setWriteable(boolean writeable) {

@Override
public String toString() {
return "Action{" + "id=" + id + ", action='" + action + '\'' + ", priority=" + priority + ", isChecked="
+ isChecked + ", keyResult=" + keyResult + '}';
return "Action{" + "id=" + id + ", version=" + version + ", action='" + action + '\'' + ", priority=" + priority
+ ", isChecked=" + isChecked + ", keyResult=" + keyResult + ", writeable=" + writeable + '}';
}

@Override
Expand All @@ -99,17 +111,19 @@ public boolean equals(Object o) {
if (o == null || getClass() != o.getClass())
return false;
Action action1 = (Action) o;
return priority == action1.priority && isChecked == action1.isChecked && Objects.equals(id, action1.id)
return version == action1.version && priority == action1.priority && isChecked == action1.isChecked
&& writeable == action1.writeable && Objects.equals(id, action1.id)
&& Objects.equals(action, action1.action) && Objects.equals(keyResult, action1.keyResult);
}

@Override
public int hashCode() {
return Objects.hash(id, action, priority, isChecked, keyResult);
return Objects.hash(id, version, action, priority, isChecked, keyResult, writeable);
}

public static final class Builder {
private Long id;
private int version;
private String action;
private int priority;
private boolean isChecked;
Expand All @@ -127,6 +141,11 @@ public Builder withId(Long id) {
return this;
}

public Builder withVersion(int version) {
this.version = version;
return this;
}

public Builder withAction(String action) {
this.action = action;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,6 @@ public ActionAuthorizationService(ActionBusinessService actionBusinessService,
this.authorizationService = authorizationService;
}

public List<Action> getEntitiesByKeyResultId(Long keyResultId) {
AuthorizationUser authorizationUser = authorizationService.getAuthorizationUser();
hasRoleReadById(keyResultId, authorizationUser);
List<Action> actionList = actionBusinessService.getActionsByKeyResultId(keyResultId);
actionList.forEach(action -> setRoleCreateOrUpdate(action, authorizationUser));
return actionList;
}

public void createEntities(List<Action> actionList) {
AuthorizationUser authorizationUser = authorizationService.getAuthorizationUser();
actionList.forEach(action -> hasRoleCreateOrUpdate(action, authorizationUser));
Expand All @@ -42,10 +34,6 @@ public void updateEntities(List<Action> actionList) {
actionBusinessService.updateEntities(actionList);
}

protected void hasRoleReadById(Long id, AuthorizationUser authorizationUser) {
authorizationService.hasRoleReadByKeyResultId(id, authorizationUser);
}

protected void hasRoleCreateOrUpdate(Action entity, AuthorizationUser authorizationUser) {
authorizationService.hasRoleCreateOrUpdate(entity.getKeyResult(), authorizationUser);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import ch.puzzle.okr.models.Action;
import ch.puzzle.okr.models.checkin.CheckIn;
import ch.puzzle.okr.models.keyresult.KeyResult;
import ch.puzzle.okr.service.authorization.ActionAuthorizationService;
import ch.puzzle.okr.service.persistence.KeyResultPersistenceService;
import ch.puzzle.okr.service.validation.KeyResultValidationService;
import org.slf4j.Logger;
Expand All @@ -21,16 +20,16 @@ public class KeyResultBusinessService implements BusinessServiceInterface<Long,

private final KeyResultPersistenceService keyResultPersistenceService;
private final CheckInBusinessService checkInBusinessService;
private final ActionAuthorizationService actionAuthorizationService;
private final ActionBusinessService actionBusinessService;
private final KeyResultValidationService validator;
private static final Logger logger = LoggerFactory.getLogger(KeyResultBusinessService.class);

public KeyResultBusinessService(KeyResultPersistenceService keyResultPersistenceService,
KeyResultValidationService validator, CheckInBusinessService checkInBusinessService,
ActionAuthorizationService actionAuthorizationService) {
ActionBusinessService actionBusinessService) {
this.keyResultPersistenceService = keyResultPersistenceService;
this.checkInBusinessService = checkInBusinessService;
this.actionAuthorizationService = actionAuthorizationService;
this.actionBusinessService = actionBusinessService;
this.validator = validator;
}

Expand Down Expand Up @@ -62,9 +61,9 @@ public KeyResult updateEntity(Long id, KeyResult keyResult, AuthorizationUser au
if (isKeyResultTypeChangeable(id)) {
logger.debug("keyResultType has changed and is changeable, {}", keyResult);
validator.validateOnUpdate(id, keyResult);
List<Action> actionList = actionAuthorizationService.getEntitiesByKeyResultId(id);
List<Action> actionList = actionBusinessService.getActionsByKeyResultId(id);
KeyResult createdKeyResult = keyResultPersistenceService.recreateEntity(id, keyResult);
actionAuthorizationService.updateEntities(actionList);
actionBusinessService.updateEntities(actionList);
return createdKeyResult;
} else {
savedKeyResult.setTitle(keyResult.getTitle());
Expand All @@ -83,8 +82,8 @@ public void deleteEntityById(Long id) {
validator.validateOnDelete(id);
checkInBusinessService.getCheckInsByKeyResultId(id)
.forEach(checkIn -> checkInBusinessService.deleteEntityById(checkIn.getId()));
actionAuthorizationService.getEntitiesByKeyResultId(id)
.forEach(action -> actionAuthorizationService.deleteActionByActionId(action.getId()));
actionBusinessService.getActionsByKeyResultId(id)
.forEach(action -> actionBusinessService.deleteEntityById(action.getId()));
keyResultPersistenceService.deleteById(id);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ public class ActionValidationService extends ValidationBase<Action, Long, Action

private final KeyResultValidationService keyResultValidationService;

public ActionValidationService(ActionPersistenceService actionPersistenceService, KeyResultValidationService keyResultValidationService) {
public ActionValidationService(ActionPersistenceService actionPersistenceService,
KeyResultValidationService keyResultValidationService) {
super(actionPersistenceService);
this.keyResultValidationService = keyResultValidationService;
}
Expand All @@ -21,15 +22,15 @@ public void validateOnGetByKeyResultId(Long keyResultId) {

@Override
public void validateOnCreate(Action model) {
throwExceptionIfModelIsNull(model);
throwExceptionWhenModelIsNull(model);
throwExceptionWhenIdIsNotNull(model.getId());

validate(model);
}

@Override
public void validateOnUpdate(Long id, Action model) {
throwExceptionIfModelIsNull(model);
throwExceptionWhenModelIsNull(model);
throwExceptionWhenIdIsNull(model.getId());
throwExceptionWhenIdHasChanged(id, model.getId());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,15 @@ insert into team_organisation (team_id, organisation_id) values
(8, 2),
(8, 5);

insert into action (id, action, priority, is_checked, key_result_id) values
(1, 'Neues Haus', 1, true, 8),
(2, 'Neue Katze', 2, true, 8),
(3, 'Neuer Garten', 3, true, 10),
(4, 'Essen kaufen', 1, true, 10),
(5, 'Mehr Licht', 2, true, 10),
(6, 'Neue Pflanzen', 1, true, 6),
(7, 'Mehr Getränke', 3, true, 6),
(8, 'Ein Buch', 2, true, 6),
(9, 'Lenovo Laptop', 1, true, 19),
(10, 'Türen säubern', 2, true, 19),
(11, 'Stühle ölen', 3, true, 19);
insert into action (id, version, action, priority, is_checked, key_result_id) values
(1, 1, 'Neues Haus', 1, true, 8),
(2, 1, 'Neue Katze', 2, true, 8),
(3, 1, 'Neuer Garten', 3, true, 10),
(4, 1, 'Essen kaufen', 1, true, 10),
(5, 1, 'Mehr Licht', 2, true, 10),
(6, 1, 'Neue Pflanzen', 1, true, 6),
(7, 1, 'Mehr Getränke', 3, true, 6),
(8, 1, 'Ein Buch', 2, true, 6),
(9, 1, 'Lenovo Laptop', 1, true, 19),
(10, 1, 'Türen säubern', 2, true, 19),
(11, 1, 'Stühle ölen', 3, true, 19);
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ create table action
(
id bigint not null
primary key,
version int not null,
action varchar(4096) not null,
priority integer not null,
is_checked boolean not null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ ALTER SEQUENCE sequence_action RESTART WITH 500;
create table if not exists action
(
id bigint not null primary key,
version int not null,
action varchar(4096) not null,
priority int not null,
is_checked boolean not null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ public class KeyResultTestHelpers {
keyResultLastCheckInDto, LocalDateTime.MIN, LocalDateTime.MAX, true, List.of());
public static final KeyResultOrdinalDto keyResultOrdinalDto = new KeyResultOrdinalDto(5L, 1,
KEY_RESULT_TYPE_ORDINAL, TITLE, DESCRIPTION, COMMIT_ZONE, TARGET_ZONE, STRETCH_ZONE, keyResultUserDto,
keyResultObjectiveDto, keyResultLastCheckInOrdinalDto, LocalDateTime.MIN, LocalDateTime.MAX, true, List.of());
keyResultObjectiveDto, keyResultLastCheckInOrdinalDto, LocalDateTime.MIN, LocalDateTime.MAX, true,
List.of());
public static final Objective objective = Objective.Builder.builder().withId(5L).withTitle("Objective 1").build();
public static final KeyResult ordinalKeyResult = KeyResultOrdinal.Builder.builder().withId(3L)
.withTitle("Keyresult 2").withOwner(user).withObjective(objective).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.BDDMockito;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.security.test.context.support.WithMockUser;
Expand Down Expand Up @@ -61,9 +61,9 @@ class ActionControllerIT {
}
]
""";
@Mock
@MockBean
ActionAuthorizationService actionAuthorizationService;
@Mock
@MockBean
ActionMapper actionMapper;
@Autowired
private MockMvc mvc;
Expand All @@ -88,8 +88,8 @@ void updateSuccessfulActions() throws Exception {

@Test
void updateSuccessfulOnlyOneAction() throws Exception {
mvc.perform(put(BASEURL).content(SUCCESSFUL_UPDATE_BODY_SINGLE_ACTION)
.contentType(MediaType.APPLICATION_JSON).with(SecurityMockMvcRequestPostProcessors.csrf()))
mvc.perform(put(BASEURL).content(SUCCESSFUL_UPDATE_BODY_SINGLE_ACTION).contentType(MediaType.APPLICATION_JSON)
.with(SecurityMockMvcRequestPostProcessors.csrf()))
.andExpect(MockMvcResultMatchers.status().is2xxSuccessful());

verify(actionMapper, times(1)).toActions(any());
Expand All @@ -98,7 +98,7 @@ void updateSuccessfulOnlyOneAction() throws Exception {

@Test
void shouldDeleteAction() throws Exception {
mvc.perform(delete(BASEURL).with(SecurityMockMvcRequestPostProcessors.csrf()))
mvc.perform(delete("/api/v2/action/1").with(SecurityMockMvcRequestPostProcessors.csrf()))
.andExpect(MockMvcResultMatchers.status().isOk());
}

Expand Down
Loading

0 comments on commit ddbf9f3

Please sign in to comment.