Skip to content

Commit edb166a

Browse files
Miguel7373Miguel7373
authored
Bug/1556 Next Quarter generation does not work (#1597)
* fix(cronjob): Generate new Quarter at the start of the month rather then the end #1556 * fix: Calculation of the first month of the quarter should be dependent on the business year start #1556 * style: run formatter * test: Change test variables to match new logic * test: Change test variables to match new logic in persitence service * refactor: Change the logic of the quarter generation of cron-job so it is more readable #1556 * test: Add and update persistence and business service tests for Quarter Generation #1556 * style: Run formatter * test: Add tests for the double generation if the current quarter is missing #1556 * style: Run formatter #1556 * refactor: Removed useless schema passing * test: add mocks for save that now returns the generated quarter #1556 * refactor: put long if condition into helper
1 parent 5b2e227 commit edb166a

File tree

3 files changed

+103
-46
lines changed

3 files changed

+103
-46
lines changed

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

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import java.util.HashMap;
1313
import java.util.List;
1414
import java.util.Map;
15-
import java.util.Set;
1615
import org.apache.commons.lang3.StringUtils;
1716
import org.slf4j.Logger;
1817
import org.slf4j.LoggerFactory;
@@ -83,8 +82,7 @@ private int getStartOfBusinessYear(YearMonth startOfQuarter, int quarter) {
8382
return startOfQuarter.minusMonths((quarter - 1) * 3L).getYear();
8483
}
8584

86-
private void generateQuarter(LocalDate start, String label, String schema) {
87-
TenantContext.setCurrentTenant(schema);
85+
private Quarter generateQuarter(LocalDate start, String label) {
8886

8987
YearMonth yearMonth = YearMonth.from(start);
9088
Quarter quarter = Quarter.Builder
@@ -94,14 +92,7 @@ private void generateQuarter(LocalDate start, String label, String schema) {
9492
.withEndDate(yearMonth.plusMonths(2).atEndOfMonth())
9593
.build();
9694
validator.validateOnGeneration(quarter);
97-
quarterPersistenceService.save(quarter);
98-
}
99-
100-
private boolean isInLastMonthOfQuarter(int currentQuarter, int nextQuarter) {
101-
// If the quarter 4 months in the future and the current are exactly 2 apart,
102-
// we are in the final month of the current quarter. This works for all 4 cases:
103-
// 1 -> 3 | 2 -> 4 | 3 -> 1 | 4 -> 2
104-
return Math.abs(nextQuarter - currentQuarter) == 2;
95+
return quarterPersistenceService.save(quarter);
10596
}
10697

10798
public YearMonth getCurrentYearMonth() {
@@ -119,27 +110,42 @@ Map<Integer, Integer> generateQuarters() {
119110
return quarters;
120111
}
121112

122-
@Scheduled(cron = "0 59 23 L * ?") // Cron expression for 23:59:00 on the last day of every month
123-
public void scheduledGenerationQuarters() {
124-
Map<Integer, Integer> quarters = generateQuarters();
125-
YearMonth currentYearMonth = getCurrentYearMonth();
126-
YearMonth nextQuarterYearMonth = currentYearMonth.plusMonths(4);
113+
private Quarter getOrCreateCurrentQuarter() {
114+
Quarter current = getCurrentQuarter();
115+
if (current == null) {
116+
current = createQuarter(getCurrentYearMonth());
117+
}
118+
return current;
119+
}
127120

128-
int currentQuarter = quarters.get(currentYearMonth.getMonthValue());
129-
int nextQuarter = quarters.get(nextQuarterYearMonth.getMonthValue());
121+
private Quarter createQuarter(YearMonth creationDate) {
122+
String label = createQuarterLabel(creationDate, generateQuarters().get(creationDate.getMonthValue()));
123+
return generateQuarter(creationDate.atDay(1), label);
124+
}
125+
126+
private boolean isFirstMonthOfQuarter(Quarter currentQuarter) {
127+
YearMonth firstMonth = YearMonth.from(currentQuarter.getStartDate());
128+
return getCurrentYearMonth().equals(firstMonth);
129+
}
130130

131+
@Scheduled(cron = "0 1 0 1 * ?") // Runs at 00:01 on the 1st of each month
132+
public void scheduledGenerationQuarters() {
133+
logger.warn("Start scheduled generation of quarters");
131134
String initialTenant = TenantContext.getCurrentTenant();
132135

133-
Set<String> tenantSchemas = this.tenantConfigProvider.getAllTenantIds();
134-
// If we are in the last month of a quarter, generate the next quarter
135-
if (isInLastMonthOfQuarter(currentQuarter, nextQuarter)) {
136-
for (String schema : tenantSchemas) {
137-
logger.info("Generated quarters on last day of month for tenant {}", schema);
138-
String label = createQuarterLabel(nextQuarterYearMonth, nextQuarter);
139-
generateQuarter(nextQuarterYearMonth.atDay(1), label, schema);
136+
for (String schema : tenantConfigProvider.getAllTenantIds()) {
137+
logger.warn("Start generating quarters on first day of month for tenant {}", schema);
138+
TenantContext.setCurrentTenant(schema);
139+
140+
Quarter currentQuarter = getOrCreateCurrentQuarter();
141+
if (isFirstMonthOfQuarter(currentQuarter)) {
142+
YearMonth nextQuarter = YearMonth.from(currentQuarter.getEndDate()).plusMonths(1);
143+
createQuarter(nextQuarter);
140144
}
145+
logger.warn("Successfully generated quarters on first day of month for tenant {}", schema);
141146
}
142147

143148
TenantContext.setCurrentTenant(initialTenant);
149+
logger.warn("End scheduled generation of quarters");
144150
}
145-
}
151+
}

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

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,35 +107,64 @@ void shouldGetBacklogQuarter() {
107107
assertNull(quarterList.get(0).getEndDate());
108108
}
109109

110-
@ParameterizedTest(name = "Should not generate a new quarter on scheduledGenerationQuarters() when it is not the last month of the quarter such as {0}")
111-
@ValueSource(ints = { 1, 2, 4, 5, 7, 8, 10, 11 })
110+
@ParameterizedTest(name = "Should not generate a new quarter on scheduledGenerationQuarters() when it is not the first month of the quarter such as {0}")
111+
@ValueSource(ints = { 2, 3, 5, 6, 8, 9, 11, 12 })
112112
void shouldNotGenerateQuarterIfNotLastMonthOfQuarter(int month) {
113113
ReflectionTestUtils.setField(quarterBusinessService, "quarterStart", 7);
114114

115-
Mockito.when(quarterBusinessService.getCurrentYearMonth()).thenReturn(YearMonth.of(2030, month));
116115
quarterBusinessService.scheduledGenerationQuarters();
117116
verify(quarterPersistenceService, never()).save(any());
118117
}
119118

120-
@ParameterizedTest(name = "Should generate new quarter on scheduledGenerationQuarters() when it is the last month of the quarter such as {0}")
121-
@ValueSource(ints = { 3, 6, 9, 12 })
122-
void shouldGenerateQuarterIfLastMonthOfQuarter(int month) {
119+
@ParameterizedTest(name = "Should generate both new quarter if the current one does not exist on scheduledGenerationQuarters() when it is the first month of the quarter such as {0}")
120+
@ValueSource(ints = { 1, 4, 7, 10 })
121+
void shouldGenerateBothQuartersIfLastMonthOfQuarterAndCurrentQuarterDoesNotExist(int month) {
123122
ReflectionTestUtils.setField(quarterBusinessService, "quarterStart", 7);
124123
Mockito.doReturn(Set.of(TestHelper.SCHEMA_PITC)).when(tenantConfigProvider).getAllTenantIds();
125124

126125
Mockito.when(quarterBusinessService.getCurrentYearMonth()).thenReturn(YearMonth.of(2030, month));
126+
127+
LocalDate currentQuarterStart = LocalDate.of(2030, month, 1);
128+
LocalDate currentQuarterEnd = currentQuarterStart.plusMonths(3).minusDays(1);
129+
130+
Quarter currentQuarter = new Quarter();
131+
currentQuarter.setStartDate(currentQuarterStart);
132+
currentQuarter.setEndDate(currentQuarterEnd);
133+
134+
Mockito.when(quarterPersistenceService.save(currentQuarter)).thenReturn(currentQuarter);
127135
quarterBusinessService.scheduledGenerationQuarters();
136+
verify(quarterPersistenceService, times(2)).save(any());
137+
}
138+
139+
@ParameterizedTest(name = "Should generate new quarter on scheduledGenerationQuarters() when it is the first month of the quarter such as {0}")
140+
@ValueSource(ints = { 1, 4, 7, 10 })
141+
void shouldGenerateQuarterIfFirstMonthOfQuarter(int month) {
142+
ReflectionTestUtils.setField(quarterBusinessService, "quarterStart", 7);
143+
Mockito.doReturn(Set.of(TestHelper.SCHEMA_PITC)).when(tenantConfigProvider).getAllTenantIds();
144+
145+
Mockito.when(quarterBusinessService.getCurrentYearMonth()).thenReturn(YearMonth.of(2030, month));
146+
147+
LocalDate currentQuarterStart = LocalDate.of(2030, month, 1);
148+
LocalDate currentQuarterEnd = currentQuarterStart.plusMonths(3).minusDays(1);
149+
150+
Quarter currentQuarter = new Quarter();
151+
currentQuarter.setStartDate(currentQuarterStart);
152+
currentQuarter.setEndDate(currentQuarterEnd);
153+
154+
Mockito.when(quarterBusinessService.getCurrentQuarter()).thenReturn(currentQuarter);
155+
quarterBusinessService.scheduledGenerationQuarters();
156+
128157
verify(quarterPersistenceService, times(1)).save(any());
129158
}
130159

131160
private static Stream<Arguments> generateQuarterParams() {
132161
return Stream
133-
.of(Arguments.of(7, "GJ xx/yy-Qzz", YearMonth.of(2030, 3), "GJ 30/31-Q1"),
134-
Arguments.of(7, "GJ xx/yy-Qzz", YearMonth.of(2030, 9), "GJ 30/31-Q3"),
135-
Arguments.of(5, "GJ xx/yy-Qzz", YearMonth.of(2030, 4), "GJ 30/31-Q2"),
136-
Arguments.of(1, "GJ xx-Qzz", YearMonth.of(2030, 9), "GJ 31-Q1"),
137-
Arguments.of(1, "GJ xxxx-Qzz", YearMonth.of(2030, 6), "GJ 2030-Q4"),
138-
Arguments.of(2, "xx-yy-xxxx-yyyy-Qzz", YearMonth.of(2030, 1), "30-31-2030-2031-Q2"));
162+
.of(Arguments.of(7, "GJ xx/yy-Qzz", YearMonth.of(2030, 4), "GJ 30/31-Q1"),
163+
Arguments.of(7, "GJ xx/yy-Qzz", YearMonth.of(2030, 10), "GJ 30/31-Q3"),
164+
Arguments.of(5, "GJ xx/yy-Qzz", YearMonth.of(2030, 5), "GJ 30/31-Q2"),
165+
Arguments.of(1, "GJ xx-Qzz", YearMonth.of(2030, 10), "GJ 31-Q1"),
166+
Arguments.of(1, "GJ xxxx-Qzz", YearMonth.of(2030, 7), "GJ 2030-Q4"),
167+
Arguments.of(2, "xx-yy-xxxx-yyyy-Qzz", YearMonth.of(2030, 2), "30-31-2030-2031-Q2"));
139168
}
140169

141170
@ParameterizedTest(name = "Should generate quarters correctly on scheduledGenerationQuarters() with quarter start {0}, format {1}, current month of year {2} and label {3}")
@@ -147,10 +176,10 @@ void shouldGenerateCorrectQuarter(int quarterStart, String quarterFormat, YearMo
147176
ReflectionTestUtils.setField(quarterBusinessService, "quarterStart", quarterStart);
148177
ReflectionTestUtils.setField(quarterBusinessService, "quarterFormat", quarterFormat);
149178

150-
int monthsToNextQuarterStart = 4;
179+
int monthsToNextQuarterStart = 3;
151180
LocalDate expectedStart = currentYearMonth.plusMonths(monthsToNextQuarterStart).atDay(1);
152181

153-
int monthsToNextQuarterEnd = 6;
182+
int monthsToNextQuarterEnd = 5;
154183
LocalDate expectedEnd = currentYearMonth.plusMonths(monthsToNextQuarterEnd).atEndOfMonth();
155184

156185
Quarter expectedQuarter = Quarter.Builder
@@ -161,7 +190,15 @@ void shouldGenerateCorrectQuarter(int quarterStart, String quarterFormat, YearMo
161190
.withEndDate(expectedEnd)
162191
.build();
163192

193+
LocalDate currentQuarterStart = LocalDate.of(currentYearMonth.getYear(), currentYearMonth.getMonth(), 1);
194+
LocalDate currentQuarterEnd = currentQuarterStart.plusMonths(3).minusDays(1);
195+
196+
Quarter currentQuarter = new Quarter();
197+
currentQuarter.setStartDate(currentQuarterStart);
198+
currentQuarter.setEndDate(currentQuarterEnd);
199+
164200
Mockito.when(quarterBusinessService.getCurrentYearMonth()).thenReturn(currentYearMonth);
201+
Mockito.when(quarterPersistenceService.getCurrentQuarter()).thenReturn(currentQuarter);
165202

166203
quarterBusinessService.scheduledGenerationQuarters();
167204

@@ -207,7 +244,6 @@ void shouldGetQuartersBasedOnStart(int start, int month, int quarter) {
207244
@DisplayName("Should return null on scheduledGenerationQuarters() when no quarters need to be generated")
208245
@Test
209246
void shouldReturnNullWhenNoQuarterGenerationNeeded() {
210-
Mockito.when(quarterBusinessService.getCurrentYearMonth()).thenReturn(YearMonth.of(2030, 4));
211247
quarterBusinessService.scheduledGenerationQuarters();
212248
verify(quarterPersistenceService, times(0)).save(any());
213249
}

‎backend/src/test/java/ch/puzzle/okr/service/persistence/QuarterPersistenceServiceIT.java‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,30 @@ void getModelNameShouldReturnQuarter() {
138138
assertEquals(QUARTER, quarterPersistenceService.getModelName());
139139
}
140140

141-
@ParameterizedTest(name = "Should generate quarter with Cron-Job when current month is the last month of the current quarter (Month: {0}, Quarter: {1})")
142-
@CsvSource(value = { "1,1,0", "2,1,0", "3,1,1", "4,1,0", "5,1,0", "6,2,1", "7,1,0", "8,1,0", "9,3,1", "10,3,0",
143-
"11,1,0", "12,4,1" })
144-
void shouldGenerateQuarterWithCronJob(int month, int quarterIndex, int amountOfInvocations) {
141+
@ParameterizedTest(name = "Should generate quarter with Cron-Job when current month is the first month of the current quarter (Month: {0}, Quarter: {1})")
142+
@CsvSource(value = { "1,4,1,1", "2,4,0,1", "3,4,0,1", "4,1,1,4", "5,1,0,4", "6,1,0,4", "7,2,1,7", "8,2,0,7",
143+
"9,2,0,7", "10,3,1,10", "11,3,0,10", "12,3,0,10" })
144+
void shouldGenerateQuarterWithCronJob(int month, int quarterIndex, int amountOfInvocations,
145+
int currentQuarterStart) {
145146
int startQuarter = 7;
146147
ReflectionTestUtils.setField(quarterBusinessService, "quarterStart", startQuarter);
147148
int nextYear = Year.now().atMonth(startQuarter).plusMonths(month + 12 - 1).getYear();
148149
int nextYearShort = nextYear % 1000;
149-
String expectedLabel = "GJ " + nextYearShort + "/" + (nextYearShort + 1) + "-Q" + quarterIndex;
150+
String expectedLabel;
151+
if (quarterIndex == 4) {
152+
expectedLabel = "GJ " + (nextYearShort - 1) + "/" + nextYearShort + "-Q" + quarterIndex;
153+
} else {
154+
expectedLabel = "GJ " + nextYearShort + "/" + (nextYearShort + 1) + "-Q" + quarterIndex;
155+
}
156+
157+
LocalDate start = LocalDate.of(nextYear, currentQuarterStart, 1);
158+
LocalDate end = start.plusMonths(3).minusDays(1);
159+
160+
Quarter currentQuarter = new Quarter();
161+
currentQuarter.setStartDate(start);
162+
currentQuarter.setEndDate(end);
163+
164+
Mockito.when(quarterBusinessService.getCurrentQuarter()).thenReturn(currentQuarter);
150165

151166
Mockito.doReturn(YearMonth.of(nextYear, month)).when(quarterBusinessService).getCurrentYearMonth();
152167
Mockito.doReturn(Set.of(TestHelper.SCHEMA_PITC)).when(tenantConfigProvider).getAllTenantIds();

0 commit comments

Comments
 (0)