Merge pull request #960 from puzzle/feature/948-improve-test-code-cov…

…erage-part2

Feature/948 improve test code coverage part2

backend/src/main/java/ch/puzzle/okr/service/business/ObjectiveBusinessService.java

@@ -55,16 +55,25 @@ public Objective updateEntity(Long id, Objective objective, AuthorizationUser au
         objective.setCreatedOn(savedObjective.getCreatedOn());
         objective.setModifiedBy(authorizationUser.user());
         objective.setModifiedOn(LocalDateTime.now());
-        String not = " ";
-        if (isImUsed(objective, savedObjective)) {
-            objective.setQuarter(savedObjective.getQuarter());
-            not = " NOT ";
-        }
-        logger.debug("quarter has changed and is{}changeable, {}", not, objective);
+        setQuarterIfIsImUsed(objective, savedObjective);
         validator.validateOnUpdate(id, objective);
         return objectivePersistenceService.save(objective);
     }
 
+    private void setQuarterIfIsImUsed(Objective objective, Objective savedObjective) {
+        boolean imUsed = isImUsed(objective, savedObjective);
+        if (imUsed) {
+            objective.setQuarter(savedObjective.getQuarter());
+        }
+        logger.debug("quarter has changed and is{}changeable, {}", spaceOrNot(imUsed), objective);
+    }
+
+    private String spaceOrNot(boolean imUsed) {
+        if (imUsed)
+            return " NOT ";
+        return " ";
+    }
+
     public boolean isImUsed(Objective objective) {
         Objective savedObjective = objectivePersistenceService.findById(objective.getId());
         return isImUsed(objective, savedObjective);
@@ -107,19 +116,23 @@ public Objective createEntity(Objective objective, AuthorizationUser authorizati
     @Transactional
     public Objective duplicateObjective(Long id, Objective objective, AuthorizationUser authorizationUser) {
         Objective duplicatedObjective = createEntity(objective, authorizationUser);
-        List<KeyResult> keyResultsOfDuplicatedObjective = keyResultBusinessService.getAllKeyResultsByObjective(id);
-        for (KeyResult keyResult : keyResultsOfDuplicatedObjective) {
-            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);
-            }
+        for (KeyResult keyResult : keyResultBusinessService.getAllKeyResultsByObjective(id)) {
+            duplicateKeyResult(authorizationUser, 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) //
@@ -148,7 +161,8 @@ private KeyResult makeCopyOfKeyResultOrdinal(KeyResult keyResult, Objective dupl
     public void deleteEntityById(Long id) {
         validator.validateOnDelete(id);
         completedBusinessService.deleteCompletedByObjectiveId(id);
-        keyResultBusinessService.getAllKeyResultsByObjective(id)
+        keyResultBusinessService //
+                .getAllKeyResultsByObjective(id) //
                 .forEach(keyResult -> keyResultBusinessService.deleteEntityById(keyResult.getId()));
         objectivePersistenceService.deleteById(id);
     }

backend/src/main/java/ch/puzzle/okr/service/business/OverviewBusinessService.java

@@ -45,7 +45,7 @@ private List<Overview> sortOverview(List<Overview> overviews, AuthorizationUser
         return overviews;
     }
 
-    private record OverviewComparator(AuthorizationUser authorizationUser) implements Comparator<Overview> {
+    record OverviewComparator(AuthorizationUser authorizationUser) implements Comparator<Overview> {
 
         @Override
         public int compare(Overview o1, Overview o2) {
@@ -56,21 +56,27 @@ public int compare(Overview o1, Overview o2) {
             boolean containsFirstLevelTeam2 = authorizationUser.firstLevelTeamIds()
                     .contains(o2.getOverviewId().getTeamId());
 
-            if (containsUserTeam1 == containsUserTeam2) {
-                if (containsFirstLevelTeam1 == containsFirstLevelTeam2) {
-                    if (Objects.equals(o1.getTeamName(), o2.getTeamName())) {
-                        if (Objects.equals(o1.getObjectiveCreatedOn(), o2.getObjectiveCreatedOn())) {
-                            return o1.getOverviewId().compareTo(o2.getOverviewId());
-                        }
-                        return o1.getObjectiveCreatedOn().compareTo(o2.getObjectiveCreatedOn());
-                    } else {
-                        return o1.getTeamName().compareTo(o2.getTeamName());
-                    }
-                } else {
-                    return (containsFirstLevelTeam1 && !containsFirstLevelTeam2) ? -1 : 1;
-                }
+            return compare(o1, o2, containsUserTeam1, containsUserTeam2, containsFirstLevelTeam1,
+                    containsFirstLevelTeam2);
+        }
+
+        public int compare(Overview o1, Overview o2, boolean containsUserTeam1, boolean containsUserTeam2,
+                boolean containsFirstLevelTeam1, boolean containsFirstLevelTeam2) {
+            if (containsUserTeam1 != containsUserTeam2) {
+                return containsUserTeam1 ? -1 : 1;
+            }
+            if (containsFirstLevelTeam1 != containsFirstLevelTeam2) {
+                return containsFirstLevelTeam1 ? -1 : 1;
+            }
+            if (!Objects.equals(o1.getTeamName(), o2.getTeamName())) {
+                return o1.getTeamName().compareTo(o2.getTeamName());
             }
-            return (containsUserTeam1 && !containsUserTeam2) ? -1 : 1;
+            if (!Objects.equals(o1.getObjectiveCreatedOn(), o2.getObjectiveCreatedOn())) {
+                return o1.getObjectiveCreatedOn().compareTo(o2.getObjectiveCreatedOn());
+            }
+            return o1.getOverviewId().compareTo(o2.getOverviewId());
         }
+
     }
+
 }

backend/src/main/java/ch/puzzle/okr/service/validation/ObjectiveValidationService.java

@@ -47,33 +47,38 @@ public void validateOnUpdate(Long id, Objective model) {
         validate(model);
     }
 
-    private static void throwExceptionWhenModifiedByIsSet(Objective model) {
+    private void throwExceptionWhenModifiedByIsSet(Objective model) {
         if (model.getModifiedBy() != null) {
-
             throw new OkrResponseStatusException(HttpStatus.BAD_REQUEST, ErrorKey.ATTRIBUTE_SET_FORBIDDEN,
                     List.of("ModifiedBy", model.getModifiedBy()));
         }
     }
 
-    private static void throwExceptionWhenModifiedByIsNull(Objective model) {
+    private void throwExceptionWhenModifiedByIsNull(Objective model) {
         if (model.getModifiedBy() == null) {
             throw new OkrResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, ErrorKey.ATTRIBUTE_NOT_SET,
                     "modifiedBy");
         }
     }
 
-    private static void throwExceptionWhenTeamHasChanged(Team team, Team savedTeam) {
+    private void throwExceptionWhenTeamHasChanged(Team team, Team savedTeam) {
         if (!Objects.equals(team, savedTeam)) {
             throw new OkrResponseStatusException(HttpStatus.BAD_REQUEST, ErrorKey.ATTRIBUTE_CANNOT_CHANGE,
                     List.of(TEAM, OBJECTIVE));
         }
     }
 
-    private static void throwExceptionWhenNotDraftInBacklogQuarter(Objective model) {
-        if (model.getQuarter().getStartDate() == null && model.getQuarter().getEndDate() == null
-                && model.getQuarter().getLabel().equals("Backlog") && (model.getState() != State.DRAFT)) {
+    private void throwExceptionWhenNotDraftInBacklogQuarter(Objective model) {
+        if (isInvalidBacklogObjective(model)) {
             throw new OkrResponseStatusException(HttpStatus.BAD_REQUEST, ErrorKey.ATTRIBUTE_MUST_BE_DRAFT,
                     List.of(OBJECTIVE, STATE_DRAFT, model.getState()));
         }
     }
+
+    private boolean isInvalidBacklogObjective(Objective model) {
+        return model.getQuarter().getLabel().equals("Backlog") //
+                && model.getQuarter().getStartDate() == null //
+                && model.getQuarter().getEndDate() == null //
+                && (model.getState() != State.DRAFT);
+    }
 }

backend/src/test/java/ch/puzzle/okr/controller/CheckInControllerIT.java

@@ -27,7 +27,7 @@
 
 import java.time.LocalDateTime;
 
-import static ch.puzzle.okr.CheckInTestHelpers.*;
+import static ch.puzzle.okr.test.CheckInTestHelpers.*;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.*;

backend/src/test/java/ch/puzzle/okr/controller/CompletedControllerIT.java

@@ -22,7 +22,7 @@
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
 import org.springframework.web.server.ResponseStatusException;
 
-import static ch.puzzle.okr.KeyResultTestHelpers.JSON_PATH_ID;
+import static ch.puzzle.okr.test.KeyResultTestHelpers.JSON_PATH_ID;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.doThrow;

backend/src/test/java/ch/puzzle/okr/controller/KeyResultControllerIT.java

@@ -34,7 +34,7 @@
 
 import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_METRIC;
 import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_ORDINAL;
-import static ch.puzzle.okr.KeyResultTestHelpers.*;
+import static ch.puzzle.okr.test.KeyResultTestHelpers.*;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.*;

backend/src/test/java/ch/puzzle/okr/converter/JwtOrganisationConverterTest.java

@@ -6,7 +6,7 @@
 
 import java.util.List;
 
-import static ch.puzzle.okr.TestHelper.defaultJwtToken;
+import static ch.puzzle.okr.test.TestHelper.defaultJwtToken;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.springframework.test.util.ReflectionTestUtils.setField;
 

backend/src/test/java/ch/puzzle/okr/converter/JwtUserConverterTest.java

@@ -1,6 +1,6 @@
 package ch.puzzle.okr.converter;
 
-import ch.puzzle.okr.TestHelper;
+import ch.puzzle.okr.test.TestHelper;
 import ch.puzzle.okr.dto.ErrorDto;
 import ch.puzzle.okr.exception.OkrResponseStatusException;
 import ch.puzzle.okr.models.User;
@@ -10,7 +10,7 @@
 
 import java.util.List;
 
-import static ch.puzzle.okr.TestHelper.defaultJwtToken;
+import static ch.puzzle.okr.test.TestHelper.defaultJwtToken;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.springframework.http.HttpStatus.BAD_REQUEST;

backend/src/test/java/ch/puzzle/okr/mapper/OverviewMapperTest.java

@@ -1,6 +1,6 @@
 package ch.puzzle.okr.mapper;
 
-import ch.puzzle.okr.TestHelper;
+import ch.puzzle.okr.test.TestHelper;
 import ch.puzzle.okr.dto.ErrorDto;
 import ch.puzzle.okr.dto.overview.OverviewDto;
 import ch.puzzle.okr.dto.overview.OverviewKeyResultDto;

backend/src/test/java/ch/puzzle/okr/service/authorization/ActionAuthorizationServiceTest.java

@@ -17,7 +17,7 @@
 
 import java.util.List;
 
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;

backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationRegistrationServiceIT.java

@@ -16,9 +16,9 @@
 
 import static ch.puzzle.okr.SpringCachingConfig.AUTHORIZATION_USER_CACHE;
 import static ch.puzzle.okr.SpringCachingConfig.USER_CACHE;
-import static ch.puzzle.okr.TestConstants.*;
-import static ch.puzzle.okr.TestHelper.defaultUser;
-import static ch.puzzle.okr.TestHelper.mockJwtToken;
+import static ch.puzzle.okr.test.TestConstants.*;
+import static ch.puzzle.okr.test.TestHelper.defaultUser;
+import static ch.puzzle.okr.test.TestHelper.mockJwtToken;
 import static ch.puzzle.okr.models.authorization.AuthorizationRole.*;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertNotNull;

backend/src/test/java/ch/puzzle/okr/service/authorization/AuthorizationServiceTest.java

@@ -1,6 +1,6 @@
 package ch.puzzle.okr.service.authorization;
 
-import ch.puzzle.okr.TestHelper;
+import ch.puzzle.okr.test.TestHelper;
 import ch.puzzle.okr.converter.JwtConverterFactory;
 import ch.puzzle.okr.converter.JwtUserConverter;
 import ch.puzzle.okr.dto.ErrorDto;
@@ -33,8 +33,8 @@
 import java.util.List;
 
 import static ch.puzzle.okr.ErrorKey.*;
-import static ch.puzzle.okr.TestConstants.ORGANISATION_FIRST_LEVEL;
-import static ch.puzzle.okr.TestHelper.*;
+import static ch.puzzle.okr.test.TestConstants.ORGANISATION_FIRST_LEVEL;
+import static ch.puzzle.okr.test.TestHelper.*;
 import static ch.puzzle.okr.models.authorization.AuthorizationRole.*;
 import static ch.puzzle.okr.service.authorization.AuthorizationService.*;
 import static org.assertj.core.api.Assertions.assertThat;

backend/src/test/java/ch/puzzle/okr/service/authorization/CheckInAuthorizationServiceTest.java

@@ -11,8 +11,8 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.web.server.ResponseStatusException;
 
-import static ch.puzzle.okr.CheckInTestHelpers.checkInMetric;
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.CheckInTestHelpers.checkInMetric;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.when;

backend/src/test/java/ch/puzzle/okr/service/authorization/CompletedAuthorizationServiceTest.java

@@ -12,7 +12,7 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.web.server.ResponseStatusException;
 
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.doThrow;

backend/src/test/java/ch/puzzle/okr/service/authorization/KeyResultAuthorizationServiceTest.java

@@ -16,9 +16,9 @@
 
 import java.util.List;
 
-import static ch.puzzle.okr.KeyResultTestHelpers.checkIn1;
-import static ch.puzzle.okr.KeyResultTestHelpers.metricKeyResult;
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.KeyResultTestHelpers.checkIn1;
+import static ch.puzzle.okr.test.KeyResultTestHelpers.metricKeyResult;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.doThrow;

backend/src/test/java/ch/puzzle/okr/service/authorization/ObjectiveAuthorizationServiceTest.java

@@ -12,7 +12,7 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.web.server.ResponseStatusException;
 
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.when;

backend/src/test/java/ch/puzzle/okr/service/authorization/OrganisationAuthorizationServiceTest.java

@@ -13,8 +13,8 @@
 
 import java.util.List;
 
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
-import static ch.puzzle.okr.TestHelper.userWithoutWriteAllRole;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.TestHelper.userWithoutWriteAllRole;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
 import static org.springframework.http.HttpStatus.UNAUTHORIZED;

backend/src/test/java/ch/puzzle/okr/service/authorization/OverviewAuthorizationServiceTest.java

@@ -15,7 +15,7 @@
 
 import java.util.List;
 
-import static ch.puzzle.okr.TestHelper.*;
+import static ch.puzzle.okr.test.TestHelper.*;
 import static ch.puzzle.okr.models.authorization.AuthorizationRole.READ_ALL_PUBLISHED;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;

backend/src/test/java/ch/puzzle/okr/service/authorization/TeamAuthorizationServiceTest.java

@@ -19,7 +19,7 @@
 import java.util.List;
 import java.util.stream.Stream;
 
-import static ch.puzzle.okr.TestHelper.*;
+import static ch.puzzle.okr.test.TestHelper.*;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;

backend/src/test/java/ch/puzzle/okr/service/authorization/UserAuthorizationServiceTest.java

@@ -12,8 +12,8 @@
 
 import java.util.List;
 
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
-import static ch.puzzle.okr.TestHelper.userWithoutWriteAllRole;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.TestHelper.userWithoutWriteAllRole;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.Mockito.when;
 

backend/src/test/java/ch/puzzle/okr/service/business/ActionBusinessServiceTest.java

@@ -15,7 +15,7 @@
 
 import java.util.List;
 
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertIterableEquals;
 import static org.mockito.ArgumentMatchers.any;

backend/src/test/java/ch/puzzle/okr/service/business/CheckInBusinessServiceTest.java

@@ -1,6 +1,5 @@
 package ch.puzzle.okr.service.business;
 
-import ch.puzzle.okr.TestHelper;
 import ch.puzzle.okr.models.Unit;
 import ch.puzzle.okr.models.User;
 import ch.puzzle.okr.models.authorization.AuthorizationUser;
@@ -19,7 +18,7 @@
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.*;

backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceIT.java

@@ -27,7 +27,7 @@
 
 import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_METRIC;
 import static ch.puzzle.okr.Constants.KEY_RESULT_TYPE_ORDINAL;
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.Mockito.when;

backend/src/test/java/ch/puzzle/okr/service/business/KeyResultBusinessServiceTest.java

@@ -25,7 +25,7 @@
 import java.util.Collections;
 import java.util.List;
 
-import static ch.puzzle.okr.TestHelper.defaultAuthorizationUser;
+import static ch.puzzle.okr.test.TestHelper.defaultAuthorizationUser;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.*;