Merge pull request #680 from rivaldi8/bugfix-641-exports-weekday-ignored

Bugfix 641: Weekday is ignored in scheduled exports

Author: codinguser

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

@@ -24,6 +24,7 @@
 
 import java.sql.Timestamp;
 import java.text.SimpleDateFormat;
+import java.util.Calendar;
 import java.util.Date;
 import java.util.Locale;
 import java.util.TimeZone;
@@ -224,7 +225,7 @@ private long computeNextScheduledExecutionTimeStartingAt(long startTime) {
                 nextScheduledExecution = nextScheduledExecution.plusDays(multiplier);
                 break;
             case WEEK:
-                nextScheduledExecution = nextScheduledExecution.plusWeeks(multiplier);
+                nextScheduledExecution = computeNextWeeklyExecutionStartingAt(nextScheduledExecution);
                 break;
             case MONTH:
                 nextScheduledExecution = nextScheduledExecution.plusMonths(multiplier);
@@ -236,6 +237,43 @@ private long computeNextScheduledExecutionTimeStartingAt(long startTime) {
         return nextScheduledExecution.toDate().getTime();
     }
 
+    /**
+     * Computes the next time that this weekly scheduled action is supposed to be
+     * executed starting at startTime.
+     *
+     * @param startTime LocalDateTime to use as start to compute the next schedule.
+     *
+     * @return Next run time as a LocalDateTime
+     */
+    @NonNull
+    private LocalDateTime computeNextWeeklyExecutionStartingAt(LocalDateTime startTime) {
+        // Look into the week of startTime for another scheduled weekday
+        for (int weekDay : mRecurrence.getByDays() ) {
+            int jodaWeekDay = convertCalendarWeekdayToJoda(weekDay);
+            LocalDateTime candidateNextDueTime = startTime.withDayOfWeek(jodaWeekDay);
+            if (candidateNextDueTime.isAfter(startTime))
+                return candidateNextDueTime;
+        }
+
+        // Return the first scheduled weekday from the next due week
+        int firstScheduledWeekday = convertCalendarWeekdayToJoda(mRecurrence.getByDays().get(0));
+        return startTime.plusWeeks(mRecurrence.getMultiplier())
+                        .withDayOfWeek(firstScheduledWeekday);
+    }
+
+    /**
+     * Converts a java.util.Calendar weekday constant to the
+     * org.joda.time.DateTimeConstants equivalent.
+     *
+     * @param calendarWeekday weekday constant from java.util.Calendar
+     * @return weekday constant equivalent from org.joda.time.DateTimeConstants
+     */
+    private int convertCalendarWeekdayToJoda(int calendarWeekday) {
+        Calendar cal = Calendar.getInstance();
+        cal.set(Calendar.DAY_OF_WEEK, calendarWeekday);
+        return LocalDateTime.fromCalendarFields(cal).getDayOfWeek();
+    }
+
     /**
      * Set time of last execution of the scheduled action
      * @param nextRun Timestamp in milliseconds since Epoch

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

@@ -23,9 +23,13 @@
 import org.junit.Test;
 
 import java.sql.Timestamp;
+import java.util.Arrays;
 import java.util.Calendar;
+import java.util.Collections;
 
 import static org.assertj.core.api.Assertions.assertThat;
+
+
 /**
  * Test scheduled actions
  */
@@ -130,6 +134,48 @@ public void testComputingTimeOfLastSchedule(){
 
     }
 
+    /**
+     * Weekly actions scheduled to run on multiple weekdays should be due
+     * in each of them in the same week.
+     *
+     * For an action scheduled on Mondays and Thursdays, we test that, if
+     * the last run was on Monday, the next should be due on the Thursday
+     * of the same week instead of the following week.
+     */
+    @Test
+    public void multiWeekdayWeeklyActions_shouldBeDueOnEachWeekdaySet() {
+        ScheduledAction scheduledAction = new ScheduledAction(ScheduledAction.ActionType.BACKUP);
+        Recurrence recurrence = new Recurrence(PeriodType.WEEK);
+        recurrence.setByDays(Arrays.asList(Calendar.MONDAY, Calendar.THURSDAY));
+        scheduledAction.setRecurrence(recurrence);
+        scheduledAction.setStartTime(new DateTime(2016, 6, 6, 9, 0).getMillis());
+        scheduledAction.setLastRun(new DateTime(2017, 4, 17, 9, 0).getMillis()); // Monday
+
+        long expectedNextDueDate = new DateTime(2017, 4, 20, 9, 0).getMillis(); // Thursday
+        assertThat(scheduledAction.computeNextTimeBasedScheduledExecutionTime())
+                .isEqualTo(expectedNextDueDate);
+    }
+
+    /**
+     * Weekly actions scheduled with multiplier should skip intermediate
+     * weeks and be due in the specified weekday.
+     */
+    @Test
+    public void weeklyActionsWithMultiplier_shouldBeDueOnTheWeekdaySet() {
+        ScheduledAction scheduledAction = new ScheduledAction(ScheduledAction.ActionType.BACKUP);
+        Recurrence recurrence = new Recurrence(PeriodType.WEEK);
+        recurrence.setMultiplier(2);
+        recurrence.setByDays(Collections.singletonList(Calendar.WEDNESDAY));
+        scheduledAction.setRecurrence(recurrence);
+        scheduledAction.setStartTime(new DateTime(2016, 6, 6, 9, 0).getMillis());
+        scheduledAction.setLastRun(new DateTime(2017, 4, 12, 9, 0).getMillis()); // Wednesday
+
+        // Wednesday, 2 weeks after the last run
+        long expectedNextDueDate = new DateTime(2017, 4, 26, 9, 0).getMillis();
+        assertThat(scheduledAction.computeNextTimeBasedScheduledExecutionTime())
+                .isEqualTo(expectedNextDueDate);
+    }
+
     private long getTimeInMillis(int year, int month, int day) {
         Calendar calendar = Calendar.getInstance();
         calendar.set(year, month, day);

app/src/test/java/org/gnucash/android/test/unit/service/ScheduledActionServiceTest.java

@@ -48,6 +48,7 @@
 import org.gnucash.android.util.BookUtils;
 import org.gnucash.android.util.TimestampHelper;
 import org.joda.time.DateTime;
+import org.joda.time.DateTimeConstants;
 import org.joda.time.LocalDateTime;
 import org.joda.time.Weeks;
 import org.junit.After;
@@ -63,6 +64,8 @@
 import java.math.BigDecimal;
 import java.sql.Timestamp;
 import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Collections;
 import java.util.List;
 
 import javax.xml.parsers.ParserConfigurationException;
@@ -199,8 +202,10 @@ public void missedScheduledTransactions_shouldBeGenerated(){
 
         scheduledAction.setActionUID(mActionUID);
 
-        int multiplier = 2;
-        scheduledAction.setRecurrence(PeriodType.WEEK, multiplier);
+        Recurrence recurrence = new Recurrence(PeriodType.WEEK);
+        recurrence.setMultiplier(2);
+        recurrence.setByDays(Collections.singletonList(Calendar.MONDAY));
+        scheduledAction.setRecurrence(recurrence);
         ScheduledActionDbAdapter.getInstance().addRecord(scheduledAction, DatabaseAdapter.UpdateMethod.insert);
 
         TransactionsDbAdapter transactionsDbAdapter = TransactionsDbAdapter.getInstance();
@@ -249,7 +254,10 @@ public void scheduledTransactionsWithEndTimeInPast_shouldBeExecuted(){
         scheduledAction.setStartTime(startTime.getMillis());
         scheduledAction.setActionUID(mActionUID);
 
-        scheduledAction.setRecurrence(PeriodType.WEEK, 2);
+        Recurrence recurrence = new Recurrence(PeriodType.WEEK);
+        recurrence.setMultiplier(2);
+        recurrence.setByDays(Collections.singletonList(Calendar.MONDAY));
+        scheduledAction.setRecurrence(recurrence);
         scheduledAction.setEndTime(new DateTime(2016, 8, 8, 9, 0).getMillis());
         ScheduledActionDbAdapter.getInstance().addRecord(scheduledAction, DatabaseAdapter.UpdateMethod.insert);
 
@@ -345,11 +353,15 @@ public void scheduledBackups_shouldRunOnlyOnce(){
     @Test
     public void scheduledBackups_shouldNotRunBeforeNextScheduledExecution(){
         ScheduledAction scheduledBackup = new ScheduledAction(ScheduledAction.ActionType.BACKUP);
-        scheduledBackup.setStartTime(LocalDateTime.now().minusDays(2).toDate().getTime());
+        scheduledBackup.setStartTime(
+                LocalDateTime.now().withDayOfWeek(DateTimeConstants.WEDNESDAY).toDate().getTime());
         scheduledBackup.setLastRun(scheduledBackup.getStartTime());
         long previousLastRun = scheduledBackup.getLastRunTime();
         scheduledBackup.setExecutionCount(1);
-        scheduledBackup.setRecurrence(PeriodType.WEEK, 1);
+        Recurrence recurrence = new Recurrence(PeriodType.WEEK);
+        recurrence.setMultiplier(1);
+        recurrence.setByDays(Collections.singletonList(Calendar.MONDAY));
+        scheduledBackup.setRecurrence(recurrence);
 
         ExportParams backupParams = new ExportParams(ExportFormat.XML);
         backupParams.setExportTarget(ExportParams.ExportTarget.SD_CARD);
@@ -380,7 +392,10 @@ public void scheduledBackups_shouldNotIncludeTransactionsPreviousToTheLastRun()
         scheduledBackup.setLastRun(LocalDateTime.now().minusDays(8).toDate().getTime());
         long previousLastRun = scheduledBackup.getLastRunTime();
         scheduledBackup.setExecutionCount(1);
-        scheduledBackup.setRecurrence(PeriodType.WEEK, 1);
+        Recurrence recurrence = new Recurrence(PeriodType.WEEK);
+        recurrence.setMultiplier(1);
+        recurrence.setByDays(Collections.singletonList(Calendar.WEDNESDAY));
+        scheduledBackup.setRecurrence(recurrence);
         ExportParams backupParams = new ExportParams(ExportFormat.QIF);
         backupParams.setExportTarget(ExportParams.ExportTarget.SD_CARD);
         backupParams.setExportStartTime(new Timestamp(scheduledBackup.getStartTime()));
@@ -438,7 +453,10 @@ public void scheduledBackups_shouldIncludeTransactionsAfterTheLastRun() {
         scheduledBackup.setLastRun(LocalDateTime.now().minusDays(8).toDate().getTime());
         long previousLastRun = scheduledBackup.getLastRunTime();
         scheduledBackup.setExecutionCount(1);
-        scheduledBackup.setRecurrence(PeriodType.WEEK, 1);
+        Recurrence recurrence = new Recurrence(PeriodType.WEEK);
+        recurrence.setMultiplier(1);
+        recurrence.setByDays(Collections.singletonList(Calendar.FRIDAY));
+        scheduledBackup.setRecurrence(recurrence);
         ExportParams backupParams = new ExportParams(ExportFormat.QIF);
         backupParams.setExportTarget(ExportParams.ExportTarget.SD_CARD);
         backupParams.setExportStartTime(new Timestamp(scheduledBackup.getStartTime()));