From b477c825c28f9fac27b7db1d3fed7ada98cc78a7 Mon Sep 17 00:00:00 2001 From: Lias Kleisa Date: Thu, 13 Jun 2024 10:30:23 +0200 Subject: [PATCH] Refactor code --- .../main/java/ch/puzzle/okr/Constants.java | 4 +++ .../OverviewAuthorizationService.java | 4 ++- .../business/QuarterBusinessService.java | 14 +++++++---- .../OverviewPersistenceService.java | 25 ++++++++++++++++--- .../ObjectiveValidationService.java | 2 +- .../validation/QuarterValidationService.java | 4 ++- .../okr/controller/QuarterControllerIT.java | 5 ++-- .../business/QuarterBusinessServiceTest.java | 7 +++--- .../ObjectiveValidationServiceTest.java | 7 +++--- .../src/app/overview/overview.component.ts | 3 ++- 10 files changed, 54 insertions(+), 21 deletions(-) diff --git a/backend/src/main/java/ch/puzzle/okr/Constants.java b/backend/src/main/java/ch/puzzle/okr/Constants.java index 38220a0e91..8e85267ef5 100644 --- a/backend/src/main/java/ch/puzzle/okr/Constants.java +++ b/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; } diff --git a/backend/src/main/java/ch/puzzle/okr/service/authorization/OverviewAuthorizationService.java b/backend/src/main/java/ch/puzzle/okr/service/authorization/OverviewAuthorizationService.java index 76b82744eb..b28850db0f 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/authorization/OverviewAuthorizationService.java +++ b/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 getFilteredOverview(Long quarterId, List teamIds, St AuthorizationUser authorizationUser = authorizationService.getAuthorizationUser(); List 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; diff --git a/backend/src/main/java/ch/puzzle/okr/service/business/QuarterBusinessService.java b/backend/src/main/java/ch/puzzle/okr/service/business/QuarterBusinessService.java index c6a450ff29..a00870b0fd 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/business/QuarterBusinessService.java +++ b/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 getQuarters() { List 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 generateQuarters() { } @Transactional - protected void handleQuarterArchive() { + protected void moveObjectsFromNonMostCurrentQuartersIntoArchive() { List mostCurrentQuarterList = quarterPersistenceService.getMostCurrentQuarters(); List 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()); } }); diff --git a/backend/src/main/java/ch/puzzle/okr/service/persistence/OverviewPersistenceService.java b/backend/src/main/java/ch/puzzle/okr/service/persistence/OverviewPersistenceService.java index 116a717701..5733d44256 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/persistence/OverviewPersistenceService.java +++ b/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 authorizationCriteria; @@ -28,9 +31,23 @@ public OverviewPersistenceService(EntityManager entityManager, public List getFilteredOverview(Long quarterId, List 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 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 createTypedQuery(Long quarterId, List teamIds, String objectiveQuery, + AuthorizationUser authorizationUser, String queryString, boolean isArchive) { TypedQuery typedQuery = entityManager.createQuery(queryString, Overview.class); if (isArchive) { logger.debug("select overview by teamIds={}: {}", teamIds, queryString); diff --git a/backend/src/main/java/ch/puzzle/okr/service/validation/ObjectiveValidationService.java b/backend/src/main/java/ch/puzzle/okr/service/validation/ObjectiveValidationService.java index c302db645d..1c01ffa592 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/validation/ObjectiveValidationService.java +++ b/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())); } diff --git a/backend/src/main/java/ch/puzzle/okr/service/validation/QuarterValidationService.java b/backend/src/main/java/ch/puzzle/okr/service/validation/QuarterValidationService.java index e129abac70..bc6aefd59f 100644 --- a/backend/src/main/java/ch/puzzle/okr/service/validation/QuarterValidationService.java +++ b/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 { @@ -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())); diff --git a/backend/src/test/java/ch/puzzle/okr/controller/QuarterControllerIT.java b/backend/src/test/java/ch/puzzle/okr/controller/QuarterControllerIT.java index 76f9f27da6..12a57df8e8 100644 --- a/backend/src/test/java/ch/puzzle/okr/controller/QuarterControllerIT.java +++ b/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 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 diff --git a/backend/src/test/java/ch/puzzle/okr/service/business/QuarterBusinessServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/business/QuarterBusinessServiceTest.java index b5f1b78142..2dd0e0b52c 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/business/QuarterBusinessServiceTest.java +++ b/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 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()); } diff --git a/backend/src/test/java/ch/puzzle/okr/service/validation/ObjectiveValidationServiceTest.java b/backend/src/test/java/ch/puzzle/okr/service/validation/ObjectiveValidationServiceTest.java index 172095b4ee..2eca77a820 100644 --- a/backend/src/test/java/ch/puzzle/okr/service/validation/ObjectiveValidationServiceTest.java +++ b/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") diff --git a/frontend/src/app/overview/overview.component.ts b/frontend/src/app/overview/overview.component.ts index a72e2aa26d..cf6727c07b 100644 --- a/frontend/src/app/overview/overview.component.ts +++ b/frontend/src/app/overview/overview.component.ts @@ -19,6 +19,7 @@ export class OverviewComponent implements OnInit, OnDestroy { hasAdminAccess: ReplaySubject = new ReplaySubject(1); overviewPadding: Subject = 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); }