Fix repeat count

app/src/androidTest/java/com/todoroo/astrid/repeats/RepeatTaskHelperTest.java

@@ -3,105 +3,211 @@ package com.todoroo.astrid.repeats;
 import android.annotation.SuppressLint;
 import android.support.test.runner.AndroidJUnit4;
 
-import com.google.ical.values.Frequency;
 import com.google.ical.values.RRule;
+import com.todoroo.astrid.alarms.AlarmService;
+import com.todoroo.astrid.dao.TaskDao;
 import com.todoroo.astrid.data.Task;
+import com.todoroo.astrid.gcal.GCalHelper;
+import com.todoroo.astrid.test.DatabaseTestCase;
 
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.InOrder;
+import org.tasks.LocalBroadcastManager;
+import org.tasks.injection.TestComponent;
 import org.tasks.time.DateTime;
 
 import java.text.ParseException;
 
-import static com.google.ical.values.Frequency.DAILY;
+import javax.inject.Inject;
-import static com.google.ical.values.Frequency.HOURLY;
+
-import static com.google.ical.values.Frequency.MINUTELY;
+import static com.natpryce.makeiteasy.MakeItEasy.with;
-import static com.google.ical.values.Frequency.WEEKLY;
-import static com.google.ical.values.Frequency.YEARLY;
-import static com.todoroo.andlib.utility.DateUtilities.addCalendarMonthsToUnixtime;
-import static com.todoroo.astrid.repeats.RepeatTaskHelper.computeNextDueDate;
-import static java.util.concurrent.TimeUnit.DAYS;
-import static java.util.concurrent.TimeUnit.HOURS;
-import static java.util.concurrent.TimeUnit.MINUTES;
 import static junit.framework.Assert.assertEquals;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.tasks.makers.TaskMaker.AFTER_COMPLETE;
+import static org.tasks.makers.TaskMaker.COMPLETION_TIME;
+import static org.tasks.makers.TaskMaker.DUE_TIME;
+import static org.tasks.makers.TaskMaker.ID;
+import static org.tasks.makers.TaskMaker.RRULE;
+import static org.tasks.makers.TaskMaker.newTask;
 
 @SuppressLint("NewApi")
 @RunWith(AndroidJUnit4.class)
-public class RepeatTaskHelperTest {
+public class RepeatTaskHelperTest extends DatabaseTestCase {
 
     private final Task task = new Task();
-    private final long dueDate;
-    private final long completionDate;
 
     {
-        completionDate = new DateTime(2014, 1, 7, 17, 17, 32, 900).getMillis();
+        task.setDueDate(new DateTime(2013, 12, 31, 17, 17, 32, 900).getMillis());
-        dueDate = new DateTime(2013, 12, 31, 17, 17, 32, 900).getMillis();
+        task.setCompletionDate(new DateTime(2014, 1, 7, 17, 17, 32, 900).getMillis());
-        task.setDueDate(dueDate);
+    }
-        task.setCompletionDate(completionDate);
+
+    @Inject TaskDao taskDao;
+
+    private LocalBroadcastManager localBroadcastManager;
+    private AlarmService alarmService;
+    private GCalHelper gCalHelper;
+    private RepeatTaskHelper helper;
+    private InOrder mocks;
+
+    @Before
+    public void before() {
+        alarmService = mock(AlarmService.class);
+        gCalHelper = mock(GCalHelper.class);
+        localBroadcastManager = mock(LocalBroadcastManager.class);
+        mocks = inOrder(alarmService, gCalHelper, localBroadcastManager);
+        helper = new RepeatTaskHelper(gCalHelper, alarmService, taskDao, localBroadcastManager);
+    }
+
+    @After
+    public void after() {
+        verifyNoMoreInteractions(localBroadcastManager, gCalHelper, alarmService);
     }
 
     @Test
-    public void testMinutelyRepeat() {
+    public void noRepeat() {
-        checkFrequency(6, MINUTES.toMillis(1), MINUTELY);
+        helper.handleRepeat(newTask(with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30))));
     }
 
     @Test
-    public void testHourlyRepeat() {
+    public void testMinutelyRepeat() throws ParseException {
-        checkFrequency(6, HOURS.toMillis(1), HOURLY);
+        Task task = newTask(with(ID, 1L),
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
+                with(RRULE, new RRule("RRULE:FREQ=MINUTELY;INTERVAL=30")));
+
+        repeatAndVerify(task,
+                new DateTime(2017, 10, 4, 13, 30, 1),
+                new DateTime(2017, 10, 4, 14, 0, 1));
     }
 
     @Test
-    public void testDailyRepeat() {
+    public void testMinutelyRepeatAfterCompletion() throws ParseException {
-        checkFrequency(6, DAYS.toMillis(1), DAILY);
+        Task task = newTask(with(ID, 1L),
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
+                with(COMPLETION_TIME, new DateTime(2017, 10, 4, 13, 17, 45, 340)),
+                with(RRULE, new RRule("RRULE:FREQ=MINUTELY;INTERVAL=30")),
+                with(AFTER_COMPLETE, true));
+
+        repeatAndVerify(task,
+                new DateTime(2017, 10, 4, 13, 30, 1),
+                new DateTime(2017, 10, 4, 13, 47, 1));
     }
 
     @Test
-    public void testWeeklyRepeat() {
+    public void testMinutelyDecrementCount() throws ParseException {
-        checkFrequency(6, DAYS.toMillis(7), WEEKLY);
+        Task task = newTask(with(ID, 1L),
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
+                with(RRULE, new RRule("RRULE:FREQ=MINUTELY;COUNT=2;INTERVAL=30")));
+
+        repeatAndVerify(task,
+                new DateTime(2017, 10, 4, 13, 30, 1),
+                new DateTime(2017, 10, 4, 14, 0, 1));
+
+        assertEquals(1, new RRule(task.getRecurrenceWithoutFrom()).getCount());
     }
 
     @Test
-    public void testMonthlyRepeat() {
+    public void testMinutelyLastOccurrence() throws ParseException {
-        assertEquals(
+        Task task = newTask(with(ID, 1L),
-                new DateTime(2014, 7, 7, 17, 17, 1, 0).getMillis(),
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
-                nextDueDate(6, Frequency.MONTHLY, true));
+                with(RRULE, new RRule("RRULE:FREQ=MINUTELY;COUNT=1;INTERVAL=30")));
+
+        helper.handleRepeat(task);
     }
 
     @Test
-    public void testMonthlyRepeatAtEndOfMonth() {
+    public void testHourlyRepeat() throws ParseException {
-        assertEquals(
+        Task task = newTask(with(ID, 1L),
-                new DateTime(2014, 6, 30, 17, 17, 1, 0).getMillis(),
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
-                nextDueDate(6, Frequency.MONTHLY, false));
+                with(RRULE, new RRule("RRULE:FREQ=HOURLY;INTERVAL=6")));
+
+        repeatAndVerify(task,
+                new DateTime(2017, 10, 4, 13, 30, 1),
+                new DateTime(2017, 10, 4, 19, 30, 1));
     }
 
     @Test
-    public void testYearlyRepeat() {
+    public void testHourlyRepeatAfterCompletion() throws ParseException {
-        checkExpected(6, addCalendarMonthsToUnixtime(dueDate, 6 * 12), YEARLY, false);
+        Task task = newTask(with(ID, 1L),
-        checkExpected(6, addCalendarMonthsToUnixtime(completionDate, 6 * 12), YEARLY, true);
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
+                with(COMPLETION_TIME, new DateTime(2017, 10, 4, 13, 17, 45, 340)),
+                with(RRULE, new RRule("RRULE:FREQ=HOURLY;INTERVAL=6")),
+                with(AFTER_COMPLETE, true));
+
+        repeatAndVerify(task,
+                new DateTime(2017, 10, 4, 13, 30, 1),
+                new DateTime(2017, 10, 4, 19, 17, 1));
     }
 
-    private void checkFrequency(int count, long interval, Frequency frequency) {
+    @Test
-        checkExpected(count, dueDate + count * interval, frequency, false);
+    public void testDailyRepeat() throws ParseException {
-        checkExpected(count, completionDate + count * interval, frequency, true);
+        Task task = newTask(with(ID, 1L),
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
+                with(RRULE, new RRule("RRULE:FREQ=DAILY;INTERVAL=6")));
+
+        repeatAndVerify(task,
+                new DateTime(2017, 10, 4, 13, 30, 1),
+                new DateTime(2017, 10, 10, 13, 30, 1));
     }
 
-    private void checkExpected(int count, long expected, Frequency frequency, boolean repeatAfterCompletion) {
+    @Test
-        assertEquals(
+    public void testRepeatWeeklyNoDays() throws ParseException {
-                new DateTime(expected)
+        Task task = newTask(with(ID, 1L),
-                        .withSecondOfMinute(1)
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
-                        .withMillisOfSecond(0)
+                with(RRULE, new RRule("RRULE:FREQ=WEEKLY;INTERVAL=2")));
-                        .getMillis(),
+
-                nextDueDate(count, frequency, repeatAfterCompletion));
+        repeatAndVerify(task,
+                new DateTime(2017, 10, 4, 13, 30, 1),
+                new DateTime(2017, 10, 18, 13, 30, 1));
     }
 
-    private long nextDueDate(int count, Frequency frequency, boolean repeatAfterCompletion) {
+    @Test
-        RRule rrule = new RRule();
+    public void testYearly() throws ParseException {
-        rrule.setInterval(count);
+        Task task = newTask(with(ID, 1L),
-        rrule.setFreq(frequency);
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
-        try {
+                with(RRULE, new RRule("RRULE:FREQ=YEARLY;INTERVAL=3")));
-            return computeNextDueDate(task, rrule.toIcal(), repeatAfterCompletion);
+
-        } catch (ParseException e) {
+        repeatAndVerify(task,
-            throw new RuntimeException(e);
+                new DateTime(2017, 10, 4, 13, 30, 1),
+                new DateTime(2020, 10, 4, 13, 30, 1));
     }
+
+    @Test
+    public void testMonthlyRepeat() throws ParseException {
+        Task task = newTask(with(ID, 1L),
+                with(DUE_TIME, new DateTime(2017, 10, 4, 13, 30)),
+                with(RRULE, new RRule("RRULE:FREQ=MONTHLY;INTERVAL=3")));
+
+        repeatAndVerify(task,
+                new DateTime(2017, 10, 4, 13, 30, 1),
+                new DateTime(2018, 1, 4, 13, 30, 1));
+    }
+
+    @Test
+    public void testMonthlyRepeatAtEndOfMonth() throws ParseException {
+        Task task = newTask(with(ID, 1L),
+                with(DUE_TIME, new DateTime(2017, 1, 31, 13, 30)),
+                with(RRULE, new RRule("RRULE:FREQ=MONTHLY;INTERVAL=1")));
+
+        repeatAndVerify(task,
+                new DateTime(2017, 1, 31, 13, 30, 1),
+                new DateTime(2017, 2, 28, 13, 30, 1));
+    }
+
+    private void repeatAndVerify(Task task, DateTime oldDueDate, DateTime newDueDate) {
+        helper.handleRepeat(task);
+
+        mocks.verify(gCalHelper).rescheduleRepeatingTask(task);
+        mocks.verify(alarmService).rescheduleAlarms(1, oldDueDate.getMillis(), newDueDate.getMillis());
+        mocks.verify(localBroadcastManager).broadcastRepeat(1, oldDueDate.getMillis(), newDueDate.getMillis());
+    }
+
+    @Override
+    protected void inject(TestComponent component) {
+        component.inject(this);
     }
 }

app/src/androidTest/java/org/tasks/makers/TaskMaker.java

@@ -1,6 +1,7 @@
 package org.tasks.makers;
 
 import com.google.common.base.Strings;
+import com.google.ical.values.RRule;
 import com.natpryce.makeiteasy.Instantiator;
 import com.natpryce.makeiteasy.Property;
 import com.natpryce.makeiteasy.PropertyValue;
@@ -27,6 +28,8 @@ public class TaskMaker {
     public static Property<Task, DateTime> COMPLETION_TIME = newProperty();
     public static Property<Task, DateTime> DELETION_TIME = newProperty();
     public static Property<Task, DateTime> SNOOZE_TIME = newProperty();
+    public static Property<Task, RRule> RRULE = newProperty();
+    public static Property<Task, Boolean> AFTER_COMPLETE = newProperty();
 
     @SafeVarargs
     public static Task newTask(PropertyValue<? super Task, ?>... properties) {
@@ -96,6 +99,11 @@ public class TaskMaker {
             task.setReminderPeriod(randomReminderPeriod);
         }
 
+        RRule rrule = lookup.valueOf(RRULE, (RRule) null);
+        if (rrule != null) {
+            task.setRecurrence(rrule, lookup.valueOf(AFTER_COMPLETE, false));
+        }
+
         DateTime creationTime = lookup.valueOf(CREATION_TIME, newDateTime());
         task.setCreationDate(creationTime.getMillis());
 

app/src/androidTestGoogleplay/java/org/tasks/injection/TestComponent.java

@@ -14,6 +14,7 @@ import com.todoroo.astrid.provider.Astrid3ProviderTests;
 import com.todoroo.astrid.reminders.NotificationTests;
 import com.todoroo.astrid.reminders.ReminderServiceTest;
 import com.todoroo.astrid.repeats.NewRepeatTests;
+import com.todoroo.astrid.repeats.RepeatTaskHelperTest;
 import com.todoroo.astrid.service.QuickAddMarkupTest;
 import com.todoroo.astrid.service.TitleParserTest;
 import com.todoroo.astrid.subtasks.SubtasksHelperTest;
@@ -67,4 +68,6 @@ public interface TestComponent {
     NotificationTests.NotificationTestsComponent plus(NotificationTests.NotificationTestsModule notificationTestsModule);
 
     void inject(AlarmServiceTest alarmServiceTest);
+
+    void inject(RepeatTaskHelperTest repeatTaskHelperTest);
 }

app/src/main/java/com/todoroo/astrid/alarms/AlarmService.java

@@ -26,6 +26,7 @@ import org.tasks.jobs.JobQueue;
 
 import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -53,6 +54,18 @@ public class AlarmService {
         jobs = jobQueue;
     }
 
+    public void rescheduleAlarms(long taskId, long oldDueDate, long newDueDate) {
+        if(newDueDate <= 0 || newDueDate <= oldDueDate) {
+            return;
+        }
+
+        final Set<Long> alarms = new LinkedHashSet<>();
+        getAlarms(taskId, metadata -> alarms.add(metadata.getValue(AlarmFields.TIME) + (newDueDate - oldDueDate)));
+        if (!alarms.isEmpty()) {
+            synchronizeAlarms(taskId, alarms);
+        }
+    }
+
     public void getAlarms(long taskId, Callback<Metadata> callback) {
         metadataDao.query(callback, Query.select(
                 Metadata.PROPERTIES).where(MetadataCriteria.byTaskAndwithKey(

app/src/main/java/com/todoroo/astrid/repeats/RepeatControlSet.java

@@ -32,6 +32,8 @@ import com.todoroo.andlib.utility.DateUtilities;
 import com.todoroo.astrid.data.Task;
 
 import org.tasks.R;
+import org.tasks.analytics.Tracker;
+import org.tasks.analytics.Tracking;
 import org.tasks.dialogs.DialogBuilder;
 import org.tasks.injection.ForActivity;
 import org.tasks.injection.FragmentComponent;
@@ -81,6 +83,7 @@ public class RepeatControlSet extends TaskEditControlFragment
     @Override
     public void onSelected(RRule rrule) {
         this.rrule = rrule;
+        tracker.reportEvent(Tracking.Events.RECURRENCE_CUSTOM, rrule.toIcal());
         refreshDisplayView();
     }
 
@@ -98,6 +101,7 @@ public class RepeatControlSet extends TaskEditControlFragment
     @Inject @ForActivity Context context;
     @Inject Theme theme;
     @Inject Locale locale;
+    @Inject Tracker tracker;
 
     @BindView(R.id.display_row_edit) TextView displayView;
     @BindView(R.id.repeatType) Spinner typeSpinner;
@@ -260,6 +264,8 @@ public class RepeatControlSet extends TaskEditControlFragment
                                 rrule.setFreq(YEARLY);
                                 break;
                         }
+
+                        tracker.reportEvent(Tracking.Events.RECURRENCE_PRESET, rrule.toIcal());
                     }
 
                     callback.repeatChanged(rrule != null);

app/src/main/java/com/todoroo/astrid/repeats/RepeatTaskHelper.java

@@ -14,7 +14,6 @@ import com.google.ical.values.Frequency;
 import com.google.ical.values.RRule;
 import com.google.ical.values.WeekdayNum;
 import com.todoroo.andlib.utility.DateUtilities;
-import com.todoroo.astrid.alarms.AlarmFields;
 import com.todoroo.astrid.alarms.AlarmService;
 import com.todoroo.astrid.dao.TaskDao;
 import com.todoroo.astrid.data.Task;
@@ -26,9 +25,7 @@ import org.tasks.time.DateTime;
 import java.text.ParseException;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 import java.util.TimeZone;
 
 import javax.inject.Inject;
@@ -81,43 +78,31 @@ public class RepeatTaskHelper {
             }
 
             int count = rrule.getCount();
+            if (count == 1) {
+                return;
+            }
             if (count > 1) {
                 rrule.setCount(count - 1);
                 task.setRecurrence(rrule, repeatAfterCompletion);
             }
 
-            rescheduleTask(task, newDueDate);
-            rescheduleAlarms(task.getId(), oldDueDate, newDueDate);
-
-            localBroadcastManager.broadcastRepeat(task.getId(), oldDueDate, newDueDate);
-        }
-    }
-
-    private static boolean repeatFinished(long newDueDate, long repeatUntil) {
-        return repeatUntil > 0 && newDateTime(newDueDate).startOfDay().isAfter(newDateTime(repeatUntil).startOfDay());
-    }
-
-    private void rescheduleTask(Task task, long newDueDate) {
             task.setReminderSnooze(0L);
             task.setCompletionDate(0L);
             task.setDueDateAdjustingHideUntil(newDueDate);
 
             gcalHelper.rescheduleRepeatingTask(task);
             taskDao.save(task);
-    }
 
-    private void rescheduleAlarms(long taskId, long oldDueDate, long newDueDate) {
+            alarmService.rescheduleAlarms(task.getId(), oldDueDate, newDueDate);
-        if(newDueDate <= 0 || newDueDate <= oldDueDate) {
-            return;
-        }
 
-        final Set<Long> alarms = new LinkedHashSet<>();
+            localBroadcastManager.broadcastRepeat(task.getId(), oldDueDate, newDueDate);
-        alarmService.getAlarms(taskId, metadata -> alarms.add(metadata.getValue(AlarmFields.TIME) + (newDueDate - oldDueDate)));
-        if (!alarms.isEmpty()) {
-            alarmService.synchronizeAlarms(taskId, alarms);
         }
     }
 
+    private static boolean repeatFinished(long newDueDate, long repeatUntil) {
+        return repeatUntil > 0 && newDateTime(newDueDate).startOfDay().isAfter(newDateTime(repeatUntil).startOfDay());
+    }
+
     /** Compute next due date */
     static long computeNextDueDate(Task task, String recurrence, boolean repeatAfterCompletion) throws ParseException {
         RRule rrule = initRRule(recurrence);
@@ -243,7 +228,7 @@ public class RepeatTaskHelper {
         // handle the iCalendar "byDay" field differently depending on if
         // we are weekly or otherwise
         if(rrule.getFreq() != Frequency.WEEKLY) {
-            rrule.setByDay(Collections.EMPTY_LIST);
+            rrule.setByDay(Collections.emptyList());
         }
 
         return rrule;

app/src/main/java/org/tasks/analytics/Tracking.java

@@ -29,7 +29,9 @@ public class Tracking {
         UPGRADE(R.string.tracking_category_event, R.string.tracking_event_upgrade),
         NIGHT_MODE_MISMATCH(R.string.tracking_category_event, R.string.tracking_event_night_mode_mismatch),
         SET_PREFERENCE(R.string.tracking_category_preferences, 0),
-        PLAY_SERVICES_WARNING(R.string.tracking_category_event, R.string.tracking_event_play_services_error);
+        PLAY_SERVICES_WARNING(R.string.tracking_category_event, R.string.tracking_event_play_services_error),
+        RECURRENCE_CUSTOM(R.string.tracking_category_recurrence, R.string.tracking_action_custom),
+        RECURRENCE_PRESET(R.string.tracking_category_recurrence, R.string.tracking_action_preset);
 
         public final int category;
         public final int action;

app/src/main/java/org/tasks/time/DateTime.java

@@ -1,5 +1,6 @@
 package org.tasks.time;
 
+import com.google.ical.values.DateTimeValue;
 import com.google.ical.values.DateValue;
 import com.google.ical.values.DateValueImpl;
 
@@ -23,9 +24,14 @@ public class DateTime {
     private final long timestamp;
 
     public static DateTime from(DateValue dateValue) {
-        return dateValue == null
+        if (dateValue == null) {
-                ? new DateTime(0)
+            return new DateTime(0);
-                : new DateTime(dateValue.year(), dateValue.month(), dateValue.day());
+        }
+        if (dateValue instanceof DateTimeValue) {
+            DateTimeValue dt = (DateTimeValue) dateValue;
+            return new DateTime(dt.year(), dt.month(), dt.day(), dt.hour(), dt.minute(), dt.second());
+        }
+        return new DateTime(dateValue.year(), dateValue.month(), dateValue.day());
     }
 
     public DateTime(int year, int month, int day) {

app/src/main/res/values/keys.xml

@@ -227,6 +227,7 @@
   <string name="tracking_category_timer">Timer</string>
   <string name="tracking_category_tags">Tags</string>
   <string name="tracking_category_iab">IAB</string>
+  <string name="tracking_category_recurrence">Recurrence</string>
   <string name="tracking_category_google_tasks">Gtask</string>
   <string name="tracking_category_event">Event</string>
   <string name="tracking_action_add">Add</string>
@@ -239,6 +240,8 @@
   <string name="tracking_action_off">Off</string>
   <string name="tracking_action_clear">Clear</string>
   <string name="tracking_action_clear_completed">Clear completed</string>
+  <string name="tracking_action_custom">Custom</string>
+  <string name="tracking_action_preset">Preset</string>
   <string name="tracking_event_night_mode_mismatch">Night Mismatch</string>
   <string name="tracking_event_play_services_error">Play Services Error</string>
   <string name="tracking_event_upgrade">Upgrade</string>

app/src/main/res/values/strings.xml

@@ -586,7 +586,7 @@ File %1$s contained %2$s.\n\n
 
     <!-- text for button when repeating task until specified date (%s -> date string) -->
     <string name="repeat_until">Repeat until %s</string>
-  <string name="repeat_number_of_times">Occurs a number of times</string>
+  <string name="repeat_number_of_times">Repeat a number of times</string>
   <string name="repeat_occurs">Occurs</string>
 
     <string name="repeat_snackbar">%1$s rescheduled for %2$s</string>