Merge pull request #619 from rivaldi8/bugfix-617-weekly-exports-listed-wrong-day

Bugfix 617: Weekly scheduled exports are listed with the wrong day
Fixes #617

Author: codinguser

app/src/main/java/org/gnucash/android/db/adapter/RecurrenceDbAdapter.java

@@ -20,12 +20,17 @@
 import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteStatement;
 import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 
 import org.gnucash.android.app.GnuCashApplication;
 import org.gnucash.android.model.PeriodType;
 import org.gnucash.android.model.Recurrence;
 
 import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Collections;
+import java.util.List;
 
 import static org.gnucash.android.db.DatabaseSchema.RecurrenceEntry;
 
@@ -58,7 +63,7 @@ public Recurrence buildModelInstance(@NonNull Cursor cursor) {
         long multiplier = cursor.getLong(cursor.getColumnIndexOrThrow(RecurrenceEntry.COLUMN_MULTIPLIER));
         String periodStart = cursor.getString(cursor.getColumnIndexOrThrow(RecurrenceEntry.COLUMN_PERIOD_START));
         String periodEnd = cursor.getString(cursor.getColumnIndexOrThrow(RecurrenceEntry.COLUMN_PERIOD_END));
-        String byDay = cursor.getString(cursor.getColumnIndexOrThrow(RecurrenceEntry.COLUMN_BYDAY));
+        String byDays = cursor.getString(cursor.getColumnIndexOrThrow(RecurrenceEntry.COLUMN_BYDAY));
 
         PeriodType periodType = PeriodType.valueOf(type);
         periodType.setMultiplier((int) multiplier);
@@ -67,7 +72,7 @@ public Recurrence buildModelInstance(@NonNull Cursor cursor) {
         recurrence.setPeriodStart(Timestamp.valueOf(periodStart));
         if (periodEnd != null)
             recurrence.setPeriodEnd(Timestamp.valueOf(periodEnd));
-        recurrence.setByDay(byDay);
+        recurrence.setByDays(stringToByDays(byDays));
 
         populateBaseModelAttributes(cursor, recurrence);
 
@@ -79,8 +84,8 @@ public Recurrence buildModelInstance(@NonNull Cursor cursor) {
         stmt.clearBindings();
         stmt.bindLong(1, recurrence.getPeriodType().getMultiplier());
         stmt.bindString(2, recurrence.getPeriodType().name());
-        if (recurrence.getByDay() != null)
-            stmt.bindString(3, recurrence.getByDay());
+        if (!recurrence.getByDays().isEmpty())
+            stmt.bindString(3, byDaysToString(recurrence.getByDays()));
         //recurrence should always have a start date
         stmt.bindString(4, recurrence.getPeriodStart().toString());
 
@@ -90,4 +95,87 @@ public Recurrence buildModelInstance(@NonNull Cursor cursor) {
 
         return stmt;
     }
+
+    /**
+     * Converts a list of days of week as Calendar constants to an String for
+     * storing in the database.
+     *
+     * @param byDays list of days of week constants from Calendar
+     * @return String of days of the week or null if {@code byDays} was empty
+     */
+    private static @NonNull String byDaysToString(@NonNull List<Integer> byDays) {
+        StringBuilder builder = new StringBuilder();
+        for (int day : byDays) {
+            switch (day) {
+                case Calendar.MONDAY:
+                    builder.append("MO");
+                    break;
+                case Calendar.TUESDAY:
+                    builder.append("TU");
+                    break;
+                case Calendar.WEDNESDAY:
+                    builder.append("WE");
+                    break;
+                case Calendar.THURSDAY:
+                    builder.append("TH");
+                    break;
+                case Calendar.FRIDAY:
+                    builder.append("FR");
+                    break;
+                case Calendar.SATURDAY:
+                    builder.append("SA");
+                    break;
+                case Calendar.SUNDAY:
+                    builder.append("SU");
+                    break;
+                default:
+                    throw new RuntimeException("bad day of week: " + day);
+            }
+            builder.append(",");
+        }
+        builder.deleteCharAt(builder.length()-1);
+        return builder.toString();
+    }
+
+    /**
+     * Converts a String with the comma-separated days of the week into a
+     * list of Calendar constants.
+     *
+     * @param byDaysString String with comma-separated days fo the week
+     * @return list of days of the week as Calendar constants.
+     */
+    private static @NonNull List<Integer> stringToByDays(@Nullable String byDaysString) {
+        if (byDaysString == null)
+            return Collections.emptyList();
+
+        List<Integer> byDaysList = new ArrayList<>();
+        for (String day : byDaysString.split(",")) {
+            switch (day) {
+                case "MO":
+                    byDaysList.add(Calendar.MONDAY);
+                    break;
+                case "TU":
+                    byDaysList.add(Calendar.TUESDAY);
+                    break;
+                case "WE":
+                    byDaysList.add(Calendar.WEDNESDAY);
+                    break;
+                case "TH":
+                    byDaysList.add(Calendar.THURSDAY);
+                    break;
+                case "FR":
+                    byDaysList.add(Calendar.FRIDAY);
+                    break;
+                case "SA":
+                    byDaysList.add(Calendar.SATURDAY);
+                    break;
+                case "SU":
+                    byDaysList.add(Calendar.SUNDAY);
+                    break;
+                default:
+                    throw new RuntimeException("bad day of week: " + day);
+            }
+        }
+        return byDaysList;
+    }
 }

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

@@ -22,7 +22,6 @@
 import org.gnucash.android.R;
 import org.gnucash.android.app.GnuCashApplication;
 import org.gnucash.android.ui.util.RecurrenceParser;
-import org.joda.time.DateTime;
 import org.joda.time.Days;
 import org.joda.time.LocalDate;
 import org.joda.time.LocalDateTime;
@@ -32,8 +31,13 @@
 import org.joda.time.Years;
 
 import java.sql.Timestamp;
+import java.text.DateFormat;
 import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Collections;
 import java.util.Date;
+import java.util.List;
 
 /**
  * Model for recurrences in the database
@@ -55,9 +59,9 @@ public class Recurrence extends BaseModel {
     private Timestamp mPeriodEnd;
 
     /**
-     * Describes which day on which to run the recurrence
+     * Days of week on which to run the recurrence
      */
-    private String mByDay;
+    private List<Integer> mByDays = Collections.emptyList();
 
     public Recurrence(@NonNull PeriodType periodType){
         setPeriodType(periodType);
@@ -131,10 +135,9 @@ public String getRepeatString(){
         StringBuilder repeatBuilder = new StringBuilder(mPeriodType.getFrequencyRepeatString());
         Context context = GnuCashApplication.getAppContext();
 
-        String dayOfWeek = new SimpleDateFormat("EEEE", GnuCashApplication.getDefaultLocale())
-                .format(new Date(mPeriodStart.getTime()));
         if (mPeriodType == PeriodType.WEEK) {
-            repeatBuilder.append(" ").append(context.getString(R.string.repeat_on_weekday, dayOfWeek));
+            repeatBuilder.append(" ").
+                    append(context.getString(R.string.repeat_on_weekday, getDaysOfWeekString()));
         }
 
         if (mPeriodEnd != null){
@@ -144,7 +147,26 @@ public String getRepeatString(){
         return repeatBuilder.toString();
     }
 
-        /**
+    /**
+     * Returns a string with the days of the week set in the recurrence separated by commas.
+     * @return string with the days of the week set in the recurrence separated by commas.
+     */
+    private @NonNull String getDaysOfWeekString() {
+        // XXX: mByDays should never be empty with PeriodType.WEEK, but we don't enforce it yet
+        if (mByDays.isEmpty())
+            return "";
+        StringBuilder daysOfWeekString = new StringBuilder();
+        Calendar calendar = Calendar.getInstance();
+        DateFormat dayOfWeekFormatter =
+                new SimpleDateFormat("EEEE", GnuCashApplication.getDefaultLocale());
+        for (int day : mByDays) {
+            calendar.set(Calendar.DAY_OF_WEEK, day);
+            daysOfWeekString.append(dayOfWeekFormatter.format(calendar.getTime())).append(", ");
+        }
+        return daysOfWeekString.substring(0, daysOfWeekString.length()-2);
+    }
+
+    /**
          * Creates an RFC 2445 string which describes this recurring event.
          * <p>See http://recurrance.sourceforge.net/</p>
          * <p>The output of this method is not meant for human consumption</p>
@@ -251,19 +273,27 @@ public String getTextOfCurrentPeriod(int periodNum){
     }
 
     /**
-     * Sets the string which determines on which day the recurrence will be run
-     * @param byDay Byday string of recurrence rule (RFC 2445)
+     * Return the days of week on which to run the recurrence.
+     *
+     * <p>Days are expressed as defined in {@link java.util.Calendar}.
+     * For example, Calendar.MONDAY</p>
+     *
+     * @return list of days of week on which to run the recurrence.
      */
-    public void setByDay(String byDay){
-        this.mByDay = byDay;
+    public @NonNull List<Integer> getByDays(){
+        return Collections.unmodifiableList(mByDays);
     }
 
     /**
-     * Return the byDay string of recurrence rule (RFC 2445)
-     * @return String with by day specification
+     * Sets the days on which to run the recurrence.
+     *
+     * <p>Days must be expressed as defined in {@link java.util.Calendar}.
+     * For example, Calendar.MONDAY</p>
+     *
+     * @param byDays list of days of week on which to run the recurrence.
      */
-    public String getByDay(){
-        return mByDay;
+    public void setByDays(@NonNull List<Integer> byDays){
+        mByDays = new ArrayList<>(byDays);
     }
 
     /**

app/src/main/java/org/gnucash/android/ui/util/RecurrenceParser.java

@@ -16,6 +16,8 @@
 
 package org.gnucash.android.ui.util;
 
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 import android.text.format.Time;
 
 import com.codetroopers.betterpickers.recurrencepicker.EventRecurrence;
@@ -24,7 +26,10 @@
 import org.gnucash.android.model.Recurrence;
 
 import java.sql.Timestamp;
+import java.util.ArrayList;
 import java.util.Calendar;
+import java.util.Collections;
+import java.util.List;
 
 /**
  * Parses {@link EventRecurrence}s to generate
@@ -76,7 +81,7 @@ public static Recurrence parse(EventRecurrence eventRecurrence){
         periodType.setMultiplier(interval);
         Recurrence recurrence = new Recurrence(periodType);
         parseEndTime(eventRecurrence, recurrence);
-        recurrence.setByDay(parseByDay(eventRecurrence.byday));
+        recurrence.setByDays(parseByDay(eventRecurrence.byday));
         if (eventRecurrence.startDate != null)
             recurrence.setPeriodStart(new Timestamp(eventRecurrence.startDate.toMillis(false)));
 
@@ -85,7 +90,7 @@ public static Recurrence parse(EventRecurrence eventRecurrence){
 
     /**
      * Parses the end time from an EventRecurrence object and sets it to the <code>scheduledEvent</code>.
-     * The end time is specified in the dialog either by number of occurences or a date.
+     * The end time is specified in the dialog either by number of occurrences or a date.
      * @param eventRecurrence Event recurrence pattern obtained from dialog
      * @param recurrence Recurrence event to set the end period to
      */
@@ -100,90 +105,24 @@ private static void parseEndTime(EventRecurrence eventRecurrence, Recurrence rec
     }
 
     /**
-     * Returns the date for the next day of the week
-     * @param dow Day of the week (Calendar constants)
-     * @return Calendar instance with the next day of the week
+     * Parses an array of byDay values to return a list of days of week
+     * constants from {@link Calendar}.
+     *
+     * <p>Currently only supports byDay values for weeks.</p>
+     *
+     * @param byDay Array of byDay values
+     * @return list of days of week constants from Calendar.
      */
-    private static Calendar nextDayOfWeek(int dow) {
-        Calendar date = Calendar.getInstance();
-        int diff = dow - date.get(Calendar.DAY_OF_WEEK);
-        if (!(diff > 0)) {
-            diff += 7;
+    private static @NonNull List<Integer> parseByDay(@Nullable int[] byDay) {
+        if (byDay == null) {
+            return Collections.emptyList();
         }
-        date.add(Calendar.DAY_OF_MONTH, diff);
-        return date;
-    }
 
-    /**
-     * Parses an array of byday values to return the string concatenation of days of the week.
-     * <p>Currently only supports byDay values for weeks</p>
-     * @param byday Array of byday values
-     * @return String concat of days of the week or null if {@code byday} was empty
-     */
-    private static String parseByDay(int[] byday){
-        if (byday == null || byday.length == 0){
-            return null;
-        }
-        //todo: parse for month and year as well, when our dialog supports those
-        StringBuilder builder = new StringBuilder();
-        for (int day : byday) {
-            switch (day)
-            {
-                case EventRecurrence.SU:
-                    builder.append("SU");
-                    break;
-                case EventRecurrence.MO:
-                    builder.append("MO");
-                    break;
-                case EventRecurrence.TU:
-                    builder.append("TU");
-                    break;
-                case EventRecurrence.WE:
-                    builder.append("WE");
-                    break;
-                case EventRecurrence.TH:
-                    builder.append("TH");
-                    break;
-                case EventRecurrence.FR:
-                    builder.append("FR");
-                    break;
-                case EventRecurrence.SA:
-                    builder.append("SA");
-                    break;
-                default:
-                    throw new RuntimeException("bad day of week: " + day);
-            }
-            builder.append(",");
+        List<Integer> byDaysList = new ArrayList<>(byDay.length);
+        for (int day : byDay) {
+            byDaysList.add(EventRecurrence.day2CalendarDay(day));
         }
-        builder.deleteCharAt(builder.length()-1);
-        return builder.toString();
-    }
 
-    /**
-     * Converts one of the SU, MO, etc. constants to the Calendar.SUNDAY
-     * constants.  btw, I think we should switch to those here too, to
-     * get rid of this function, if possible.
-     */
-    public static int day2CalendarDay(int day)
-    {
-        switch (day)
-        {
-            case EventRecurrence.SU:
-                return Calendar.SUNDAY;
-            case EventRecurrence.MO:
-                return Calendar.MONDAY;
-            case EventRecurrence.TU:
-                return Calendar.TUESDAY;
-            case EventRecurrence.WE:
-                return Calendar.WEDNESDAY;
-            case EventRecurrence.TH:
-                return Calendar.THURSDAY;
-            case EventRecurrence.FR:
-                return Calendar.FRIDAY;
-            case EventRecurrence.SA:
-                return Calendar.SATURDAY;
-            default:
-                throw new RuntimeException("bad day of week: " + day);
-        }
+        return byDaysList;
     }
 }