Refactor code

backend/src/main/java/ch/puzzle/okr/Constants.java

@@ -17,4 +17,8 @@ private Constants() {
     public static final String QUARTER = "Quarter";
     public static final String TEAM = "Team";
     public static final String USER = "User";
+    public static final String BACKLOG = "Backlog";
+    public static final Long BACKLOG_QUARTER_ID = 999L;
+    public static final String ARCHIVE = "Archiv";
+    public static final Long ARCHIVE_QUARTER_ID = 998L;
 }

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

@@ -11,6 +11,7 @@
 import java.util.Map;
 import java.util.Objects;
 
+import static ch.puzzle.okr.Constants.ARCHIVE_QUARTER_ID;
 import static ch.puzzle.okr.service.authorization.AuthorizationService.hasRoleWriteAll;
 
 @Service
@@ -29,7 +30,8 @@ public List<Overview> getFilteredOverview(Long quarterId, List<Long> teamIds, St
         AuthorizationUser authorizationUser = authorizationService.getAuthorizationUser();
         List<Overview> overviews = overviewBusinessService.getFilteredOverview(quarterId, teamIds, objectiveQuery,
                 authorizationUser);
-        if (Objects.isNull(quarterId) || quarterId != 998) {
+        // TODO Search for 998 in all Flyway/SQL Scripts and check that it is everywhere considered
+        if (Objects.isNull(quarterId) || !quarterId.equals(ARCHIVE_QUARTER_ID)) {
             setRoleCreateOrUpdateTeam(overviews, authorizationUser);
         }
         return overviews;

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

@@ -17,6 +17,9 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+
+import static ch.puzzle.okr.Constants.*;
 
 @Service
 public class QuarterBusinessService {
@@ -46,8 +49,8 @@ public Quarter getQuarterById(Long quarterId) {
 
     public List<Quarter> getQuarters() {
         List<Quarter> mostCurrentQuarterList = quarterPersistenceService.getMostCurrentQuarters();
-        Quarter backlog = quarterPersistenceService.findByLabel("Backlog");
-        Quarter archive = quarterPersistenceService.findByLabel("Archiv");
+        Quarter backlog = quarterPersistenceService.findByLabel(BACKLOG);
+        Quarter archive = quarterPersistenceService.findByLabel(ARCHIVE);
         mostCurrentQuarterList.add(0, backlog);
         mostCurrentQuarterList.add(archive);
         return mostCurrentQuarterList;
@@ -88,7 +91,7 @@ protected void generateQuarter(LocalDateTime start, String label) {
                 .withEndDate(yearMonth.plusMonths(2).atEndOfMonth()).build();
         validator.validateOnGeneration(quarter);
         quarterPersistenceService.save(quarter);
-        handleQuarterArchive();
+        moveObjectsFromNonMostCurrentQuartersIntoArchive();
     }
 
     private boolean inLastMonthOfQuarter(int currentQuarter, int nextQuarter) {
@@ -114,12 +117,13 @@ Map<Integer, Integer> generateQuarters() {
     }
 
     @Transactional
-    protected void handleQuarterArchive() {
+    protected void moveObjectsFromNonMostCurrentQuartersIntoArchive() {
         List<Quarter> mostCurrentQuarterList = quarterPersistenceService.getMostCurrentQuarters();
         List<Objective> allObjectives = objectiveBusinessService.getAllObjectives();
 
         allObjectives.forEach(objective -> {
-            if (!mostCurrentQuarterList.contains(objective.getQuarter()) && objective.getQuarter().getId() != 999) {
+            if (!mostCurrentQuarterList.contains(objective.getQuarter())
+                    && !Objects.equals(objective.getQuarter().getId(), BACKLOG_QUARTER_ID)) {
                 objectiveBusinessService.archiveEntity(objective.getId());
             }
         });

backend/src/main/java/ch/puzzle/okr/service/persistence/OverviewPersistenceService.java

@@ -9,13 +9,16 @@
 import org.springframework.stereotype.Service;
 
 import java.util.List;
+import java.util.Objects;
+
+import static ch.puzzle.okr.Constants.ARCHIVE_QUARTER_ID;
 
 @Service
 public class OverviewPersistenceService {
 
     private static final Logger logger = LoggerFactory.getLogger(OverviewPersistenceService.class);
     private static final String SELECT_OVERVIEW = "SELECT o FROM Overview o WHERE o.quarterId=:quarterId";
-    private static final String SELECT_ARCHIVE = "SELECT o FROM Overview o WHERE o.objectiveArchived=true";
+    private static final String SELECT_OVERVIEW_FROM_ARCHIVE = "SELECT o FROM Overview o WHERE o.objectiveArchived=true";
 
     private final EntityManager entityManager;
     private final AuthorizationCriteria<Overview> authorizationCriteria;
@@ -28,9 +31,23 @@ public OverviewPersistenceService(EntityManager entityManager,
 
     public List<Overview> getFilteredOverview(Long quarterId, List<Long> teamIds, String objectiveQuery,
             AuthorizationUser authorizationUser) {
-        boolean isArchive = quarterId == 998;
-        String queryString = (isArchive ? SELECT_ARCHIVE : SELECT_OVERVIEW)
-                + authorizationCriteria.appendOverview(teamIds, objectiveQuery, authorizationUser);
+        boolean isArchive = Objects.equals(quarterId, ARCHIVE_QUARTER_ID);
+        String queryString = createQueryString(teamIds, objectiveQuery, authorizationUser, isArchive);
+        return createTypedQuery(quarterId, teamIds, objectiveQuery, authorizationUser, queryString, isArchive);
+    }
+
+    private String createQueryString(List<Long> teamIds, String objectiveQuery, AuthorizationUser authorizationUser,
+            boolean isArchive) {
+        if (isArchive) {
+            return SELECT_OVERVIEW_FROM_ARCHIVE
+                    + authorizationCriteria.appendOverview(teamIds, objectiveQuery, authorizationUser);
+        } else {
+            return SELECT_OVERVIEW + authorizationCriteria.appendOverview(teamIds, objectiveQuery, authorizationUser);
+        }
+    }
+
+    private List<Overview> createTypedQuery(Long quarterId, List<Long> teamIds, String objectiveQuery,
+            AuthorizationUser authorizationUser, String queryString, boolean isArchive) {
         TypedQuery<Overview> typedQuery = entityManager.createQuery(queryString, Overview.class);
         if (isArchive) {
             logger.debug("select overview by teamIds={}: {}", teamIds, queryString);

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

@@ -71,7 +71,7 @@ private static void throwExceptionWhenTeamHasChanged(Team team, Team savedTeam)
 
     private static void throwExceptionWhenNotDraftInBacklogQuarter(Objective model) {
         if (model.getQuarter().getStartDate() == null && model.getQuarter().getEndDate() == null
-                && model.getQuarter().getLabel().equals("Backlog") && (model.getState() != State.DRAFT)) {
+                && model.getQuarter().getLabel().equals(BACKLOG) && (model.getState() != State.DRAFT)) {
             throw new OkrResponseStatusException(HttpStatus.BAD_REQUEST, ErrorKey.ATTRIBUTE_MUST_BE_DRAFT,
                     List.of(OBJECTIVE, STATE_DRAFT, model.getState()));
         }

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

@@ -10,6 +10,8 @@
 
 import java.util.List;
 
+import static ch.puzzle.okr.Constants.BACKLOG;
+
 @Service
 public class QuarterValidationService
         extends ValidationBase<Quarter, Long, QuarterRepository, QuarterPersistenceService> {
@@ -29,7 +31,7 @@ public void validateOnUpdate(Long id, Quarter model) {
     }
 
     public static void throwExceptionWhenStartEndDateQuarterIsNull(Quarter model) {
-        if (!model.getLabel().equals("Backlog")) {
+        if (!model.getLabel().equals(BACKLOG)) {
             if (model.getStartDate() == null) {
                 throw new OkrResponseStatusException(HttpStatus.BAD_REQUEST, ErrorKey.ATTRIBUTE_NULL,
                         List.of("StartDate", model.getLabel()));

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

@@ -21,6 +21,7 @@
 import java.util.Collections;
 import java.util.List;
 
+import static ch.puzzle.okr.Constants.BACKLOG;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 
@@ -33,7 +34,7 @@ class QuarterControllerIT {
             .withStartDate(LocalDate.of(2022, 9, 1)).withEndDate(LocalDate.of(2022, 12, 31)).build();
     static Quarter quarter2 = Quarter.Builder.builder().withId(2L).withLabel("GJ 22/23-Q3")
             .withStartDate(LocalDate.of(2023, 1, 1)).withEndDate(LocalDate.of(2023, 3, 31)).build();
-    static Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel("Backlog").withStartDate(null)
+    static Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel(BACKLOG).withStartDate(null)
             .withEndDate(null).build();
     static List<Quarter> quaterList = Arrays.asList(quarter1, quarter2, backlogQuarter);
 
@@ -54,7 +55,7 @@ void shouldGetAllQuarters() throws Exception {
                 .andExpect(jsonPath("$[1].id", Is.is(2))).andExpect(jsonPath("$[1].label", Is.is("GJ 22/23-Q3")))
                 .andExpect(jsonPath("$[1].startDate", Is.is(LocalDate.of(2023, 1, 1).toString())))
                 .andExpect(jsonPath("$[1].endDate", Is.is(LocalDate.of(2023, 3, 31).toString())))
-                .andExpect(jsonPath("$[2].id", Is.is(199))).andExpect(jsonPath("$[2].label", Is.is("Backlog")));
+                .andExpect(jsonPath("$[2].id", Is.is(199))).andExpect(jsonPath("$[2].label", Is.is(BACKLOG)));
     }
 
     @Test

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

@@ -24,6 +24,7 @@
 import java.util.Map;
 import java.util.stream.Stream;
 
+import static ch.puzzle.okr.Constants.BACKLOG;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.mockito.Mockito.*;
@@ -81,13 +82,13 @@ void shouldGetBacklogQuarter() {
                 .withStartDate(LocalDate.of(2022, 8, 1)).withEndDate(LocalDate.of(2022, 11, 30)).build();
         List<Quarter> quarterList = new ArrayList<>(Arrays.asList(realQuarter1, realQuarter2));
 
-        Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel("Backlog").build();
+        Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel(BACKLOG).build();
         when(quarterPersistenceService.getMostCurrentQuarters()).thenReturn(quarterList);
-        when(quarterPersistenceService.findByLabel("Backlog")).thenReturn(backlogQuarter);
+        when(quarterPersistenceService.findByLabel(BACKLOG)).thenReturn(backlogQuarter);
 
         quarterList = quarterBusinessService.getQuarters();
         assertEquals(4, quarterList.size());
-        assertEquals("Backlog", quarterList.get(0).getLabel());
+        assertEquals(BACKLOG, quarterList.get(0).getLabel());
         assertNull(quarterList.get(0).getStartDate());
         assertNull(quarterList.get(0).getEndDate());
     }

backend/src/test/java/ch/puzzle/okr/service/validation/ObjectiveValidationServiceTest.java

@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.stream.Stream;
 
+import static ch.puzzle.okr.Constants.BACKLOG;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
@@ -384,7 +385,7 @@ void validateOnUpdateShouldThrowExceptionWheTeamHasChanged() {
     @ParameterizedTest
     @EnumSource(value = State.class, names = { "DRAFT" }, mode = EnumSource.Mode.EXCLUDE)
     void validateOnCreateShouldThrowExceptionWhenQuarterIsBacklogAndStateIsNotDraft(State state) {
-        Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel("Backlog").withStartDate(null)
+        Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel(BACKLOG).withStartDate(null)
                 .withEndDate(null).build();
 
         Objective invalidObjective = Objective.Builder.builder().withTitle("Invalid Objective").withCreatedBy(user)
@@ -403,7 +404,7 @@ void validateOnCreateShouldThrowExceptionWhenQuarterIsBacklogAndStateIsNotDraft(
     @ParameterizedTest
     @EnumSource(value = State.class, names = { "DRAFT" }, mode = EnumSource.Mode.EXCLUDE)
     void validateOnUpdateShouldThrowExceptionWhenQuarterIsBacklogAndStateIsNotDraft(State state) {
-        Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel("Backlog").withStartDate(null)
+        Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel(BACKLOG).withStartDate(null)
                 .withEndDate(null).build();
 
         Objective invalidObjective = Objective.Builder.builder().withId(1L).withTitle("Invalid Objective")
@@ -422,7 +423,7 @@ void validateOnUpdateShouldThrowExceptionWhenQuarterIsBacklogAndStateIsNotDraft(
 
     @Test
     void validateOnUpdateShouldPassWhenQuarterIsBacklogAndStateIsDraft() {
-        Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel("Backlog").withStartDate(null)
+        Quarter backlogQuarter = Quarter.Builder.builder().withId(199L).withLabel(BACKLOG).withStartDate(null)
                 .withEndDate(null).build();
 
         Objective validObjective = Objective.Builder.builder().withId(1L).withTitle("Invalid Objective")

frontend/src/app/overview/overview.component.ts

@@ -19,6 +19,7 @@ export class OverviewComponent implements OnInit, OnDestroy {
   hasAdminAccess: ReplaySubject<boolean> = new ReplaySubject<boolean>(1);
   overviewPadding: Subject<number> = new Subject();
   isArchiveQuarter: boolean = false;
+  archiveQuarterId: number = 998;
 
   constructor(
     private overviewService: OverviewService,
@@ -59,7 +60,7 @@ export class OverviewComponent implements OnInit, OnDestroy {
 
     const teamIds = getValueFromQuery(teamQuery);
     const quarterId = getValueFromQuery(quarterQuery)[0];
-    this.isArchiveQuarter = quarterId == 998;
+    this.isArchiveQuarter = quarterId == this.archiveQuarterId;
     const objectiveQueryString = getQueryString(objectiveQuery);
     this.loadOverview(quarterId, teamIds, objectiveQueryString);
   }