Skip to content

Commit a74311a

Browse files
committed
Take into account weekdays on weekly scheduled actions
The actions were run just once per week without taking into account the weekdays set by the user. Fixes #641
1 parent 5630786 commit a74311a

2 files changed

Lines changed: 85 additions & 1 deletion

File tree

app/src/main/java/org/gnucash/android/model/ScheduledAction.java

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import java.sql.Timestamp;
2626
import java.text.SimpleDateFormat;
27+
import java.util.Calendar;
2728
import java.util.Date;
2829
import java.util.Locale;
2930
import java.util.TimeZone;
@@ -224,7 +225,7 @@ private long computeNextScheduledExecutionTimeStartingAt(long startTime) {
224225
nextScheduledExecution = nextScheduledExecution.plusDays(multiplier);
225226
break;
226227
case WEEK:
227-
nextScheduledExecution = nextScheduledExecution.plusWeeks(multiplier);
228+
nextScheduledExecution = computeNextWeeklyExecutionStartingAt(nextScheduledExecution);
228229
break;
229230
case MONTH:
230231
nextScheduledExecution = nextScheduledExecution.plusMonths(multiplier);
@@ -236,6 +237,43 @@ private long computeNextScheduledExecutionTimeStartingAt(long startTime) {
236237
return nextScheduledExecution.toDate().getTime();
237238
}
238239

240+
/**
241+
* Computes the next time that this weekly scheduled action is supposed to be
242+
* executed starting at startTime.
243+
*
244+
* @param startTime LocalDateTime to use as start to compute the next schedule.
245+
*
246+
* @return Next run time as a LocalDateTime
247+
*/
248+
@NonNull
249+
private LocalDateTime computeNextWeeklyExecutionStartingAt(LocalDateTime startTime) {
250+
// Look into the week of startTime for another scheduled weekday
251+
for (int weekDay : mRecurrence.getByDays() ) {
252+
int jodaWeekDay = convertCalendarWeekdayToJoda(weekDay);
253+
LocalDateTime candidateNextDueTime = startTime.withDayOfWeek(jodaWeekDay);
254+
if (candidateNextDueTime.isAfter(startTime))
255+
return candidateNextDueTime;
256+
}
257+
258+
// Return the first scheduled weekday from the next due week
259+
int firstScheduledWeekday = convertCalendarWeekdayToJoda(mRecurrence.getByDays().get(0));
260+
return startTime.plusWeeks(mRecurrence.getMultiplier())
261+
.withDayOfWeek(firstScheduledWeekday);
262+
}
263+
264+
/**
265+
* Converts a java.util.Calendar weekday constant to the
266+
* org.joda.time.DateTimeConstants equivalent.
267+
*
268+
* @param calendarWeekday weekday constant from java.util.Calendar
269+
* @return weekday constant equivalent from org.joda.time.DateTimeConstants
270+
*/
271+
private int convertCalendarWeekdayToJoda(int calendarWeekday) {
272+
Calendar cal = Calendar.getInstance();
273+
cal.set(Calendar.DAY_OF_WEEK, calendarWeekday);
274+
return LocalDateTime.fromCalendarFields(cal).getDayOfWeek();
275+
}
276+
239277
/**
240278
* Set time of last execution of the scheduled action
241279
* @param nextRun Timestamp in milliseconds since Epoch

app/src/test/java/org/gnucash/android/test/unit/model/ScheduledActionTest.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@
2323
import org.junit.Test;
2424

2525
import java.sql.Timestamp;
26+
import java.util.Arrays;
2627
import java.util.Calendar;
28+
import java.util.Collections;
2729

2830
import static org.assertj.core.api.Assertions.assertThat;
31+
32+
2933
/**
3034
* Test scheduled actions
3135
*/
@@ -130,6 +134,48 @@ public void testComputingTimeOfLastSchedule(){
130134

131135
}
132136

137+
/**
138+
* Weekly actions scheduled to run on multiple weekdays should be due
139+
* in each of them in the same week.
140+
*
141+
* For an action scheduled on Mondays and Thursdays, we test that, if
142+
* the last run was on Monday, the next should be due on the Thursday
143+
* of the same week instead of the following week.
144+
*/
145+
@Test
146+
public void multiWeekdayWeeklyActions_shouldBeDueOnEachWeekdaySet() {
147+
ScheduledAction scheduledAction = new ScheduledAction(ScheduledAction.ActionType.BACKUP);
148+
Recurrence recurrence = new Recurrence(PeriodType.WEEK);
149+
recurrence.setByDays(Arrays.asList(Calendar.MONDAY, Calendar.THURSDAY));
150+
scheduledAction.setRecurrence(recurrence);
151+
scheduledAction.setStartTime(new DateTime(2016, 6, 6, 9, 0).getMillis());
152+
scheduledAction.setLastRun(new DateTime(2017, 4, 17, 9, 0).getMillis()); // Monday
153+
154+
long expectedNextDueDate = new DateTime(2017, 4, 20, 9, 0).getMillis(); // Thursday
155+
assertThat(scheduledAction.computeNextTimeBasedScheduledExecutionTime())
156+
.isEqualTo(expectedNextDueDate);
157+
}
158+
159+
/**
160+
* Weekly actions scheduled with multiplier should skip intermediate
161+
* weeks and be due in the specified weekday.
162+
*/
163+
@Test
164+
public void weeklyActionsWithMultiplier_shouldBeDueOnTheWeekdaySet() {
165+
ScheduledAction scheduledAction = new ScheduledAction(ScheduledAction.ActionType.BACKUP);
166+
Recurrence recurrence = new Recurrence(PeriodType.WEEK);
167+
recurrence.setMultiplier(2);
168+
recurrence.setByDays(Collections.singletonList(Calendar.WEDNESDAY));
169+
scheduledAction.setRecurrence(recurrence);
170+
scheduledAction.setStartTime(new DateTime(2016, 6, 6, 9, 0).getMillis());
171+
scheduledAction.setLastRun(new DateTime(2017, 4, 12, 9, 0).getMillis()); // Wednesday
172+
173+
// Wednesday, 2 weeks after the last run
174+
long expectedNextDueDate = new DateTime(2017, 4, 26, 9, 0).getMillis();
175+
assertThat(scheduledAction.computeNextTimeBasedScheduledExecutionTime())
176+
.isEqualTo(expectedNextDueDate);
177+
}
178+
133179
private long getTimeInMillis(int year, int month, int day) {
134180
Calendar calendar = Calendar.getInstance();
135181
calendar.set(year, month, day);

0 commit comments

Comments
 (0)