diff --git a/app/src/main/java/com/todoroo/astrid/repeats/RepeatTaskHelper.kt b/app/src/main/java/com/todoroo/astrid/repeats/RepeatTaskHelper.kt index 8cf0f309c..b0f2ff59a 100644 --- a/app/src/main/java/com/todoroo/astrid/repeats/RepeatTaskHelper.kt +++ b/app/src/main/java/com/todoroo/astrid/repeats/RepeatTaskHelper.kt @@ -49,24 +49,20 @@ class RepeatTaskHelper @Inject constructor( } newDueDate = computeNextDueDate(task, recurrence, repeatAfterCompletion) if (newDueDate == -1L) { + broadcastCompletion(task) return } } catch (e: ParseException) { Timber.e(e) return } - val oldDueDate = task.dueDate - val repeatUntil = task.repeatUntil - if (repeatFinished(newDueDate, repeatUntil)) { - broadcastCompletion(task) - return - } if (count > 1) { rrule.count = count - 1 task.setRecurrence(rrule) } task.reminderLast = 0L task.completionDate = 0L + val oldDueDate = task.dueDate task.setDueDateAdjustingHideUntil(newDueDate) gcalHelper.rescheduleRepeatingTask(task) taskDao.save(task) @@ -124,29 +120,35 @@ class RepeatTaskHelper @Inject constructor( companion object { private val weekdayCompare = Comparator { object1: WeekDay, object2: WeekDay -> WeekDay.getCalendarDay(object1) - WeekDay.getCalendarDay(object2) } - private fun repeatFinished(newDueDate: Long, repeatUntil: Long): Boolean { - return repeatUntil > 0 && newDateTime(newDueDate).startOfDay().millis > repeatUntil - } /** Compute next due date */ @Throws(ParseException::class) fun computeNextDueDate(task: Task, recurrence: String, repeatAfterCompletion: Boolean): Long { val rrule = initRRule(recurrence) + if (rrule.until != null && rrule.until is Date && task.hasDueTime()) { + // Tasks lets you create tasks with due date-times, but recurrence until with due dates + // This violates the spec and should be fixed in the picker + rrule.until = DateTime.from(rrule.until).endOfDay().toDateTime() + } // initialize startDateAsDV val original = setUpStartDate(task, repeatAfterCompletion, rrule.frequency) val startDateAsDV = setUpStartDateAsDV(task, original) - return if (rrule.frequency == Recur.Frequency.HOURLY || rrule.frequency == Recur.Frequency.MINUTELY) { - handleSubdayRepeat(original, rrule) - } else if (rrule.frequency == Recur.Frequency.WEEKLY && rrule.dayList.isNotEmpty() && repeatAfterCompletion) { - handleWeeklyRepeatAfterComplete(rrule, original, task.hasDueTime()) - } else if (rrule.frequency == Recur.Frequency.MONTHLY && rrule.dayList.isEmpty()) { - handleMonthlyRepeat(original, startDateAsDV, task.hasDueTime(), rrule) - } else { - invokeRecurrence(rrule, original, startDateAsDV) + return when { + rrule.frequency == Recur.Frequency.SECONDLY || + rrule.frequency == Recur.Frequency.MINUTELY || + rrule.frequency == Recur.Frequency.HOURLY -> + handleSubdayRepeat(original, rrule) + rrule.frequency == Recur.Frequency.WEEKLY && rrule.dayList.isNotEmpty() && repeatAfterCompletion -> + handleWeeklyRepeatAfterComplete(rrule, original, task.hasDueTime()) + rrule.frequency == Recur.Frequency.MONTHLY && rrule.dayList.isEmpty() -> + handleMonthlyRepeat(original, startDateAsDV, task.hasDueTime(), rrule) + else -> + invokeRecurrence(rrule, original, startDateAsDV) } } + @Deprecated("probably don't need this?") private fun handleWeeklyRepeatAfterComplete( recur: Recur, original: DateTime, hasDueTime: Boolean): Long { val byDay = recur.dayList @@ -166,6 +168,7 @@ class RepeatTaskHelper @Inject constructor( } } + @Deprecated("Properly support last day of month and remove this") private fun handleMonthlyRepeat( original: DateTime, startDateAsDV: Date, hasDueTime: Boolean, recur: Recur): Long { return if (original.isLastDayOfMonth) { @@ -195,7 +198,7 @@ class RepeatTaskHelper @Inject constructor( private fun invokeRecurrence(recur: Recur, original: DateTime, startDateAsDV: Date): Long { return recur.getNextDate(startDateAsDV, startDateAsDV) ?.let { buildNewDueDate(original, it) } - ?: throw IllegalStateException("recur=$recur original=$original startDateAsDv=$startDateAsDV") + ?: -1 } /** Compute long due date from DateValue */ @@ -253,6 +256,7 @@ class RepeatTaskHelper @Inject constructor( } } + @Deprecated("probably don't need this?") private fun handleSubdayRepeat(startDate: DateTime, recur: Recur): Long { val millis: Long = when (recur.frequency) { Recur.Frequency.HOURLY -> DateUtilities.ONE_HOUR diff --git a/app/src/test/java/com/todoroo/astrid/repeats/RepeatTaskHelperTests.kt b/app/src/test/java/com/todoroo/astrid/repeats/RepeatTaskHelperTests.kt index 9bd0b0e81..3a7ce8212 100644 --- a/app/src/test/java/com/todoroo/astrid/repeats/RepeatTaskHelperTests.kt +++ b/app/src/test/java/com/todoroo/astrid/repeats/RepeatTaskHelperTests.kt @@ -1,7 +1,9 @@ package com.todoroo.astrid.repeats import com.natpryce.makeiteasy.MakeItEasy.with -import org.junit.Assert.* +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Test import org.tasks.Freeze.Companion.freezeAt import org.tasks.makers.TaskMaker.COMPLETION_TIME @@ -62,19 +64,4 @@ class RepeatTaskHelperTests : RepeatTests() { assertEquals(newDay(2021, 2, 2), next) } - - @Test - fun useNowWhenNoCompletion() { - val task = newFromDue( - "FREQ=DAILY;INTERVAL=1", - newDay(2021, 3, 15), - afterComplete = true - ) - - val next = freezeAt(newDayTime(2021, 3, 29, 14, 32)) { - calculateNextDueDate(task) - } - - assertEquals(newDayTime(2021, 3, 30, 12, 0), next) - } } \ No newline at end of file diff --git a/app/src/test/java/com/todoroo/astrid/repeats/RepeatTests.kt b/app/src/test/java/com/todoroo/astrid/repeats/RepeatTests.kt index f6309487f..7bd1384e7 100644 --- a/app/src/test/java/com/todoroo/astrid/repeats/RepeatTests.kt +++ b/app/src/test/java/com/todoroo/astrid/repeats/RepeatTests.kt @@ -6,11 +6,12 @@ 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.service.TaskCompleter import kotlinx.coroutines.runBlocking import org.junit.Before -import org.mockito.Mockito -import org.mockito.Mockito.* +import org.mockito.Mockito.anyLong +import org.mockito.Mockito.anySet +import org.mockito.Mockito.mock +import org.mockito.Mockito.`when` import org.tasks.LocalBroadcastManager import org.tasks.makers.TaskMaker import org.tasks.time.DateTime @@ -62,6 +63,7 @@ abstract class RepeatTests { MakeItEasy.with(TaskMaker.RECUR, recur), MakeItEasy.with(TaskMaker.AFTER_COMPLETE, afterComplete), MakeItEasy.with(TaskMaker.DUE_TIME, due), + MakeItEasy.with(TaskMaker.COMPLETION_TIME, DateTime()), *properties ) } \ No newline at end of file diff --git a/app/src/test/java/com/todoroo/astrid/repeats/RepeatWeeklyTests.kt b/app/src/test/java/com/todoroo/astrid/repeats/RepeatWeeklyTests.kt index 37e340944..b10807893 100644 --- a/app/src/test/java/com/todoroo/astrid/repeats/RepeatWeeklyTests.kt +++ b/app/src/test/java/com/todoroo/astrid/repeats/RepeatWeeklyTests.kt @@ -2,6 +2,7 @@ package com.todoroo.astrid.repeats import com.natpryce.makeiteasy.MakeItEasy.with import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Test import org.tasks.makers.TaskMaker.COMPLETION_TIME @@ -196,4 +197,28 @@ class RepeatWeeklyTests : RepeatTests() { assertEquals(newDayTime(2016, 9, 12, 0, 25), next) } + + @Test + fun weeklyByDayFromDueDateOnUntil() { + val task = newFromDue( + "FREQ=WEEKLY;UNTIL=20240315;BYDAY=MO,TU,WE,TH,FR,SU", + newDayTime(2024, 3, 14, 3, 45) + ) + + val next = calculateNextDueDate(task) + + assertEquals(newDayTime(2024, 3, 15, 3, 45), next) + } + + @Test + fun weeklyByDayFromDueDateEndAfterUntil() { + val task = newFromDue( + "FREQ=WEEKLY;UNTIL=20240315;BYDAY=MO,TU,WE,TH,FR,SU", + newDayTime(2024, 3, 15, 3, 45) + ) + + calculateNextDueDate(task) + + assertTrue(task.isCompleted) + } } \ No newline at end of file