refactoring action services

backend/src/main/java/ch/puzzle/okr/controller/ActionController.java

@@ -32,7 +32,7 @@ public ActionController(ActionAuthorizationService actionAuthorizationService, A
     @PutMapping
     public void updateActions(
             @io.swagger.v3.oas.annotations.parameters.RequestBody(description = "The Action as json to update existing Actions.", required = true) @RequestBody List<ActionDto> actionDtoList) {
-        List<Action> actionList = this.actionMapper.toActions(actionDtoList);
+        List<Action> actionList = actionMapper.toActions(actionDtoList);
         actionAuthorizationService.updateEntities(actionList);
     }
 

backend/src/main/java/ch/puzzle/okr/controller/KeyResultController.java

@@ -1,6 +1,5 @@
 package ch.puzzle.okr.controller;
 
-import ch.puzzle.okr.dto.ActionDto;
 import ch.puzzle.okr.dto.checkin.CheckInDto;
 import ch.puzzle.okr.dto.keyresult.KeyResultDto;
 import ch.puzzle.okr.dto.keyresult.KeyResultMetricDto;
@@ -10,6 +9,7 @@
 import ch.puzzle.okr.mapper.keyresult.KeyResultMapper;
 import ch.puzzle.okr.models.Action;
 import ch.puzzle.okr.models.keyresult.KeyResult;
+import ch.puzzle.okr.models.keyresult.KeyResultWithActionList;
 import ch.puzzle.okr.service.authorization.ActionAuthorizationService;
 import ch.puzzle.okr.service.authorization.KeyResultAuthorizationService;
 import io.swagger.v3.oas.annotations.Operation;
@@ -54,7 +54,9 @@ public KeyResultController(KeyResultAuthorizationService keyResultAuthorizationS
             @ApiResponse(responseCode = "404", description = "Did not find the KeyResult with requested id", content = @Content) })
     @GetMapping("/{id}")
     public KeyResultDto getKeyResultById(@PathVariable long id) {
-        return keyResultMapper.toDto(keyResultAuthorizationService.getEntityById(id));
+        KeyResult keyResult = keyResultAuthorizationService.getEntityById(id);
+        List<Action> actionList = actionAuthorizationService.getActionsByKeyResult(keyResult);
+        return keyResultMapper.toDto(keyResult, actionList);
     }
 
     @Operation(summary = "Get Check-ins from KeyResult", description = "Get all Check-ins from one KeyResult by keyResultId.")
@@ -79,11 +81,9 @@ public List<CheckInDto> getCheckInsFromKeyResult(
     @PostMapping
     public ResponseEntity<KeyResultDto> createKeyResult(@RequestBody KeyResultDto keyResultDto) {
         KeyResult keyResult = keyResultAuthorizationService.createEntity(keyResultMapper.toKeyResult(keyResultDto));
-        List<ActionDto> actionDTOs = keyResultDto.getActionList().stream()
-                .map(actionDto -> actionDto.withKeyResultId(keyResult.getId())).toList();
-        List<Action> actionList = actionMapper.toActions(actionDTOs);
-        actionAuthorizationService.createEntities(actionList);
-        KeyResultDto createdKeyResult = keyResultMapper.toDto(keyResult);
+        List<Action> actionList = actionMapper.toActions(keyResultDto.getActionList(), keyResult);
+        List<Action> savedActions = actionAuthorizationService.createEntities(actionList);
+        KeyResultDto createdKeyResult = keyResultMapper.toDto(keyResult, savedActions);
         return ResponseEntity.status(CREATED).body(createdKeyResult);
     }
 
@@ -102,12 +102,12 @@ public ResponseEntity<KeyResultDto> updateKeyResult(
             @Parameter(description = "The ID for updating a KeyResult.", required = true) @PathVariable long id,
             @RequestBody KeyResultDto keyResultDto) {
         KeyResult keyResult = keyResultMapper.toKeyResult(keyResultDto);
+        List<Action> actionList = actionMapper.toActions(keyResultDto.getActionList(), keyResult);
         boolean isKeyResultImUsed = keyResultAuthorizationService.isImUsed(id, keyResult);
-        keyResult = keyResultAuthorizationService.updateEntity(id, keyResult);
-        List<Action> actionList = actionMapper.toActions(keyResultDto.getActionList());
-        actionAuthorizationService.updateEntities(actionList);
-        KeyResultDto updatedKeyResult = keyResultMapper.toDto(keyResult);
-        return ResponseEntity.status(isKeyResultImUsed ? IM_USED : OK).body(updatedKeyResult);
+        KeyResultWithActionList updatedKeyResult = keyResultAuthorizationService.updateEntities(id, keyResult,
+                actionList);
+        return ResponseEntity.status(isKeyResultImUsed ? IM_USED : OK)
+                .body(keyResultMapper.toDto(updatedKeyResult.keyResult(), updatedKeyResult.actionList()));
     }
 
     @Operation(summary = "Delete KeyResult by Id", description = "Delete KeyResult by Id")

backend/src/main/java/ch/puzzle/okr/dto/ActionDto.java

@@ -1,8 +1,5 @@
 package ch.puzzle.okr.dto;
 
-public record ActionDto(Long id, int version, String action, int priority, boolean isChecked, Long keyResultId) {
-
-    public ActionDto withKeyResultId(Long keyResultId) {
-        return new ActionDto(id(), version(), action(), priority(), isChecked(), keyResultId);
-    }
+public record ActionDto(Long id, int version, String action, int priority, boolean isChecked, Long keyResultId,
+        boolean writeable) {
 }

backend/src/main/java/ch/puzzle/okr/dto/keyresult/KeyResultDto.java

@@ -8,5 +8,5 @@
 
 @JsonDeserialize(using = KeyResultDeserializer.class)
 public interface KeyResultDto {
-    public List<ActionDto> getActionList();
+    List<ActionDto> getActionList();
 }

backend/src/main/java/ch/puzzle/okr/dto/keyresult/KeyResultMetricDto.java

@@ -14,6 +14,6 @@ public record KeyResultMetricDto(Long id, int version, String keyResultType, Str
         List<ActionDto> actionList) implements KeyResultDto {
     @Override
     public List<ActionDto> getActionList() {
-        return this.actionList;
+        return actionList;
     }
 }

backend/src/main/java/ch/puzzle/okr/dto/keyresult/KeyResultOrdinalDto.java

@@ -13,6 +13,6 @@ public record KeyResultOrdinalDto(Long id, int version, String keyResultType, St
         LocalDateTime modifiedOn, boolean writeable, List<ActionDto> actionList) implements KeyResultDto {
     @Override
     public List<ActionDto> getActionList() {
-        return this.actionList;
+        return actionList;
     }
 }

backend/src/main/java/ch/puzzle/okr/mapper/ActionMapper.java

@@ -2,34 +2,39 @@
 
 import ch.puzzle.okr.dto.ActionDto;
 import ch.puzzle.okr.models.Action;
-import ch.puzzle.okr.service.authorization.KeyResultAuthorizationService;
+import ch.puzzle.okr.models.keyresult.KeyResult;
+import ch.puzzle.okr.service.business.KeyResultBusinessService;
 import org.springframework.stereotype.Component;
 
 import java.util.List;
 
 @Component
 public class ActionMapper {
 
-    private final KeyResultAuthorizationService keyResultAuthorizationService;
+    private final KeyResultBusinessService keyResultBusinessService;
 
-    public ActionMapper(KeyResultAuthorizationService keyResultAuthorizationService) {
-        this.keyResultAuthorizationService = keyResultAuthorizationService;
+    public ActionMapper(KeyResultBusinessService keyResultBusinessService) {
+        this.keyResultBusinessService = keyResultBusinessService;
     }
 
     public ActionDto toDto(Action action) {
         return new ActionDto(action.getId(), action.getVersion(), action.getAction(), action.getPriority(),
-                action.isChecked(), action.getKeyResult().getId());
+                action.isChecked(), action.getKeyResult().getId(), action.isWriteable());
+    }
+
+    public List<Action> toActions(List<ActionDto> actionDtos, KeyResult keyResult) {
+        return actionDtos.stream().map(actionDto -> toAction(actionDto, keyResult)).toList();
     }
 
     public List<Action> toActions(List<ActionDto> actionDtos) {
-        return actionDtos.stream().map(this::toAction).toList();
+        return actionDtos.stream()
+                .map(actionDto -> toAction(actionDto, keyResultBusinessService.getEntityById(actionDto.keyResultId())))
+                .toList();
     }
 
-    public Action toAction(ActionDto actionDto) {
+    private Action toAction(ActionDto actionDto, KeyResult keyResult) {
         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();
+                .withKeyResult(keyResult).build();
     }
 }

backend/src/main/java/ch/puzzle/okr/mapper/keyresult/KeyResultMapper.java

@@ -3,13 +3,17 @@
 import ch.puzzle.okr.dto.keyresult.KeyResultDto;
 import ch.puzzle.okr.dto.keyresult.KeyResultMetricDto;
 import ch.puzzle.okr.dto.keyresult.KeyResultOrdinalDto;
+import ch.puzzle.okr.models.Action;
 import ch.puzzle.okr.models.keyresult.KeyResult;
 import ch.puzzle.okr.models.keyresult.KeyResultMetric;
 import ch.puzzle.okr.models.keyresult.KeyResultOrdinal;
-import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Component;
 import org.springframework.web.server.ResponseStatusException;
 
+import java.util.List;
+
+import static org.springframework.http.HttpStatus.BAD_REQUEST;
+
 @Component
 public class KeyResultMapper {
 
@@ -21,13 +25,13 @@ public KeyResultMapper(KeyResultOrdinalMapper keyResultOrdinalMapper, KeyResultM
         this.keyResultMetricMapper = keyResultMetricMapper;
     }
 
-    public KeyResultDto toDto(KeyResult keyResult) {
+    public KeyResultDto toDto(KeyResult keyResult, List<Action> actionList) {
         if (keyResult instanceof KeyResultMetric keyResultMetric) {
-            return keyResultMetricMapper.toKeyResultMetricDto(keyResultMetric);
+            return keyResultMetricMapper.toKeyResultMetricDto(keyResultMetric, actionList);
         } else if (keyResult instanceof KeyResultOrdinal keyResultOrdinal) {
-            return keyResultOrdinalMapper.toKeyResultOrdinalDto(keyResultOrdinal);
+            return keyResultOrdinalMapper.toKeyResultOrdinalDto(keyResultOrdinal, actionList);
         } else {
-            throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+            throw new ResponseStatusException(BAD_REQUEST,
                     String.format("The KeyResult %s can't be converted to a metric or ordinal KeyResult", keyResult));
         }
     }
@@ -38,7 +42,7 @@ public KeyResult toKeyResult(KeyResultDto keyResultDto) {
         } else if (keyResultDto instanceof KeyResultOrdinalDto) {
             return keyResultOrdinalMapper.toKeyResultOrdinal((KeyResultOrdinalDto) keyResultDto);
         } else {
-            throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+            throw new ResponseStatusException(BAD_REQUEST,
                     String.format("The provided KeyResultDto %s is neither metric nor ordinal", keyResultDto));
         }
     }

backend/src/main/java/ch/puzzle/okr/mapper/keyresult/KeyResultMetricMapper.java

@@ -7,7 +7,6 @@
 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.business.ActionBusinessService;
 import ch.puzzle.okr.service.business.CheckInBusinessService;
 import ch.puzzle.okr.service.business.ObjectiveBusinessService;
 import ch.puzzle.okr.service.business.UserBusinessService;
@@ -20,21 +19,19 @@ public class KeyResultMetricMapper {
 
     private final UserBusinessService userBusinessService;
     private final ObjectiveBusinessService objectiveBusinessService;
-    private final ActionBusinessService actionBusinessService;
     private final CheckInBusinessService checkInBusinessService;
     private final ActionMapper actionMapper;
 
     public KeyResultMetricMapper(UserBusinessService userBusinessService,
             ObjectiveBusinessService objectiveBusinessService, CheckInBusinessService checkInBusinessService,
-            ActionBusinessService actionBusinessService, ActionMapper actionMapper) {
+            ActionMapper actionMapper) {
         this.userBusinessService = userBusinessService;
         this.objectiveBusinessService = objectiveBusinessService;
         this.checkInBusinessService = checkInBusinessService;
-        this.actionBusinessService = actionBusinessService;
         this.actionMapper = actionMapper;
     }
 
-    public KeyResultDto toKeyResultMetricDto(KeyResultMetric keyResult) {
+    public KeyResultDto toKeyResultMetricDto(KeyResultMetric keyResult, List<Action> actionList) {
         KeyResultUserDto ownerDto = new KeyResultUserDto(keyResult.getOwner().getId(),
                 keyResult.getOwner().getFirstname(), keyResult.getOwner().getLastname());
         KeyResultQuarterDto quarterDto = new KeyResultQuarterDto(keyResult.getObjective().getQuarter().getId(),
@@ -43,7 +40,6 @@ 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 = actionBusinessService.getActionsByKeyResultId(keyResult.getId());
 
         return new KeyResultMetricDto(keyResult.getId(), keyResult.getVersion(), keyResult.getKeyResultType(),
                 keyResult.getTitle(), keyResult.getDescription(), keyResult.getBaseline(), keyResult.getStretchGoal(),

backend/src/main/java/ch/puzzle/okr/mapper/keyresult/KeyResultOrdinalMapper.java

@@ -7,34 +7,31 @@
 import ch.puzzle.okr.models.checkin.CheckInOrdinal;
 import ch.puzzle.okr.models.keyresult.KeyResult;
 import ch.puzzle.okr.models.keyresult.KeyResultOrdinal;
-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.persistence.UserPersistenceService;
+import ch.puzzle.okr.service.business.UserBusinessService;
 import org.springframework.stereotype.Component;
 
 import java.util.List;
 
 @Component
 public class KeyResultOrdinalMapper {
 
-    private final UserPersistenceService userPersistenceService;
+    private final UserBusinessService userBusinessService;
     private final ObjectiveBusinessService objectiveBusinessService;
-    private final ActionBusinessService actionBusinessService;
     private final CheckInBusinessService checkInBusinessService;
     private final ActionMapper actionMapper;
 
-    public KeyResultOrdinalMapper(UserPersistenceService userPersistenceService,
+    public KeyResultOrdinalMapper(UserBusinessService userBusinessService,
             ObjectiveBusinessService objectiveBusinessService, CheckInBusinessService checkInBusinessService,
-            ActionBusinessService actionBusinessService, ActionMapper actionMapper) {
-        this.userPersistenceService = userPersistenceService;
+            ActionMapper actionMapper) {
+        this.userBusinessService = userBusinessService;
         this.objectiveBusinessService = objectiveBusinessService;
         this.checkInBusinessService = checkInBusinessService;
-        this.actionBusinessService = actionBusinessService;
         this.actionMapper = actionMapper;
     }
 
-    public KeyResultDto toKeyResultOrdinalDto(KeyResultOrdinal keyResult) {
+    public KeyResultDto toKeyResultOrdinalDto(KeyResultOrdinal keyResult, List<Action> actionList) {
         KeyResultUserDto ownerDto = new KeyResultUserDto(keyResult.getOwner().getId(),
                 keyResult.getOwner().getFirstname(), keyResult.getOwner().getLastname());
         KeyResultQuarterDto quarterDto = new KeyResultQuarterDto(keyResult.getObjective().getQuarter().getId(),
@@ -43,7 +40,6 @@ public KeyResultDto toKeyResultOrdinalDto(KeyResultOrdinal keyResult) {
         KeyResultObjectiveDto objectiveDto = new KeyResultObjectiveDto(keyResult.getObjective().getId(),
                 keyResult.getObjective().getState().toString(), quarterDto);
         KeyResultLastCheckInOrdinalDto lastCheckInDto = getLastCheckInDto(keyResult.getId());
-        List<Action> actionList = actionBusinessService.getActionsByKeyResultId(keyResult.getId());
 
         return new KeyResultOrdinalDto(keyResult.getId(), keyResult.getVersion(), keyResult.getKeyResultType(),
                 keyResult.getTitle(), keyResult.getDescription(), keyResult.getCommitZone(), keyResult.getTargetZone(),
@@ -58,7 +54,7 @@ public KeyResult toKeyResultOrdinal(KeyResultOrdinalDto keyResultOrdinalDto) {
                 .withId(keyResultOrdinalDto.id()).withVersion(keyResultOrdinalDto.version())
                 .withObjective(objectiveBusinessService.getEntityById(keyResultOrdinalDto.objective().id()))
                 .withTitle(keyResultOrdinalDto.title()).withDescription(keyResultOrdinalDto.description())
-                .withOwner(userPersistenceService.findById(keyResultOrdinalDto.owner().id()))
+                .withOwner(userBusinessService.getOwnerById(keyResultOrdinalDto.owner().id()))
                 .withModifiedOn(keyResultOrdinalDto.modifiedOn()).build();
     }
 

backend/src/main/java/ch/puzzle/okr/models/Action.java

@@ -48,6 +48,10 @@ public Long getId() {
         return id;
     }
 
+    public void resetId() {
+        id = null;
+    }
+
     public String getAction() {
         return action;
     }

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

@@ -0,0 +1,8 @@
+package ch.puzzle.okr.models.keyresult;
+
+import ch.puzzle.okr.models.Action;
+
+import java.util.List;
+
+public record KeyResultWithActionList(KeyResult keyResult, List<Action> actionList) {
+}

backend/src/main/java/ch/puzzle/okr/service/authorization/ActionAuthorizationService.java

@@ -2,6 +2,7 @@
 
 import ch.puzzle.okr.models.Action;
 import ch.puzzle.okr.models.authorization.AuthorizationUser;
+import ch.puzzle.okr.models.keyresult.KeyResult;
 import ch.puzzle.okr.service.business.ActionBusinessService;
 import org.springframework.stereotype.Service;
 
@@ -19,26 +20,26 @@ public ActionAuthorizationService(ActionBusinessService actionBusinessService,
         this.authorizationService = authorizationService;
     }
 
-    public void createEntities(List<Action> actionList) {
-        AuthorizationUser authorizationUser = authorizationService.getAuthorizationUser();
-        actionList.forEach(action -> {
-            hasRoleCreateOrUpdate(action, authorizationUser);
-            action.setWriteable(true);
-        });
-        actionBusinessService.createEntities(actionList);
+    public List<Action> getActionsByKeyResult(KeyResult keyResult) {
+        List<Action> actionList = actionBusinessService.getActionsByKeyResultId(keyResult.getId());
+        actionList.forEach(action -> action.setWriteable(keyResult.isWriteable()));
+        return actionList;
     }
 
-    public void updateEntities(List<Action> actionList) {
+    public List<Action> createEntities(List<Action> actionList) {
         AuthorizationUser authorizationUser = authorizationService.getAuthorizationUser();
-        actionList.forEach(action -> {
-            hasRoleCreateOrUpdate(action, authorizationUser);
-            action.setWriteable(true);
-        });
-        actionBusinessService.updateEntities(actionList);
+        actionList.forEach(action -> hasRoleCreateOrUpdate(action, authorizationUser));
+        List<Action> savedActions = actionBusinessService.createEntities(actionList);
+        savedActions.forEach(action -> action.setWriteable(true));
+        return savedActions;
     }
 
-    protected void hasRoleCreateOrUpdate(Action entity, AuthorizationUser authorizationUser) {
-        authorizationService.hasRoleCreateOrUpdate(entity.getKeyResult(), authorizationUser);
+    public List<Action> updateEntities(List<Action> actionList) {
+        AuthorizationUser authorizationUser = authorizationService.getAuthorizationUser();
+        actionList.forEach(action -> hasRoleCreateOrUpdate(action, authorizationUser));
+        List<Action> updatedActions = actionBusinessService.updateEntities(actionList);
+        updatedActions.forEach(action -> action.setWriteable(true));
+        return updatedActions;
     }
 
     public void deleteActionByActionId(Long actionId) {
@@ -47,7 +48,7 @@ public void deleteActionByActionId(Long actionId) {
         actionBusinessService.deleteEntityById(actionId);
     }
 
-    protected boolean isWriteable(Action entity, AuthorizationUser authorizationUser) {
-        return authorizationService.isWriteable(entity.getKeyResult(), authorizationUser);
+    private void hasRoleCreateOrUpdate(Action entity, AuthorizationUser authorizationUser) {
+        authorizationService.hasRoleCreateOrUpdate(entity.getKeyResult(), authorizationUser);
     }
 }