From e6fbb929121b7ab3ed1e8f58d0518f9accd7ca33 Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Tue, 2 Feb 2021 12:52:45 -0600 Subject: [PATCH] Use ical4j in RepeatRuleToString --- .../tasks/repeats/RepeatRuleToStringTest.kt | 85 +++++++++++++++++-- .../main/java/com/todoroo/astrid/data/Task.kt | 3 + .../astrid/repeats/RepeatControlSet.kt | 2 +- .../org/tasks/caldav/CaldavConverter.java | 3 +- .../main/java/org/tasks/caldav/iCalendar.kt | 2 +- .../preferences/fragments/TaskDefaults.kt | 2 +- .../tasks/repeats/BasicRecurrenceDialog.java | 2 +- .../org/tasks/repeats/RepeatRuleToString.java | 64 +++++++++----- .../main/java/org/tasks/time/DateTime.java | 5 ++ 9 files changed, 133 insertions(+), 35 deletions(-) diff --git a/app/src/androidTest/java/org/tasks/repeats/RepeatRuleToStringTest.kt b/app/src/androidTest/java/org/tasks/repeats/RepeatRuleToStringTest.kt index a58ba9489..10b3ee463 100644 --- a/app/src/androidTest/java/org/tasks/repeats/RepeatRuleToStringTest.kt +++ b/app/src/androidTest/java/org/tasks/repeats/RepeatRuleToStringTest.kt @@ -1,16 +1,25 @@ package org.tasks.repeats -import androidx.test.InstrumentationRegistry -import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.google.ical.values.RRule +import com.todoroo.astrid.data.Task.Companion.withoutRRULE +import dagger.hilt.android.testing.HiltAndroidTest +import dagger.hilt.android.testing.UninstallModules +import net.fortuna.ical4j.model.property.RRule import org.junit.Assert.assertEquals import org.junit.Test -import org.junit.runner.RunWith +import org.tasks.TestUtilities.withTZ +import org.tasks.analytics.Firebase +import org.tasks.injection.InjectingTestCase +import org.tasks.injection.ProductionModule import org.tasks.locale.Locale import java.text.ParseException +import java.util.* +import javax.inject.Inject + +@UninstallModules(ProductionModule::class) +@HiltAndroidTest +class RepeatRuleToStringTest : InjectingTestCase() { + @Inject lateinit var firebase: Firebase -@RunWith(AndroidJUnit4::class) -class RepeatRuleToStringTest { @Test fun daily() { assertEquals("Repeats daily", toString("RRULE:FREQ=DAILY")) @@ -47,6 +56,60 @@ class RepeatRuleToStringTest { toString("de", "RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=SA,SU")) } + @Test + fun everyFifthTuesday() { + assertEquals( + "Repeats monthly on every fifth Tuesday", + toString("RRULE:FREQ=MONTHLY;INTERVAL=1;BYDAY=5TU") + ) + } + + @Test + fun everyLastWednesday() { + assertEquals( + "Repeats monthly on every last Wednesday", + toString("RRULE:FREQ=MONTHLY;INTERVAL=1;BYDAY=-1WE") + ) + } + + @Test + fun everyFirstThursday() { + assertEquals( + "Repeats every 2 months on every first Thursday", + toString("RRULE:FREQ=MONTHLY;INTERVAL=2;BYDAY=1TH") + ) + } + + @Test + fun repeatUntilPositiveOffset() { + withTZ(BERLIN) { + assertEquals( + "Repeats daily until February 23", + toString("RRULE:FREQ=DAILY;UNTIL=20210223;INTERVAL=1") + ) + } + } + + @Test + fun repeatUntilNoOffset() { + withTZ(LONDON) { + assertEquals( + "Repeats daily until February 23", + toString("RRULE:FREQ=DAILY;UNTIL=20210223;INTERVAL=1") + ) + } + } + + @Test + fun repeatUntilNegativeOffset() { + withTZ(NEW_YORK) { + assertEquals( + "Repeats daily until February 23", + toString("RRULE:FREQ=DAILY;UNTIL=20210223;INTERVAL=1") + ) + } + } + private fun toString(rrule: String): String { return toString(null, rrule) } @@ -54,10 +117,16 @@ class RepeatRuleToStringTest { private fun toString(language: String?, rrule: String): String { return try { val locale = Locale(java.util.Locale.getDefault(), language) - RepeatRuleToString(locale.createConfigurationContext(InstrumentationRegistry.getTargetContext()), locale) - .toString(RRule(rrule)) + RepeatRuleToString(locale.createConfigurationContext(context), locale, firebase) + .toString(RRule(rrule.withoutRRULE())) } catch (e: ParseException) { throw RuntimeException(e) } } + + companion object { + private val BERLIN = TimeZone.getTimeZone("Europe/Berlin") + private val LONDON = TimeZone.getTimeZone("Europe/London") + private val NEW_YORK = TimeZone.getTimeZone("America/New_York") + } } \ No newline at end of file diff --git a/app/src/main/java/com/todoroo/astrid/data/Task.kt b/app/src/main/java/com/todoroo/astrid/data/Task.kt index 9da567d25..3e8ba2b72 100644 --- a/app/src/main/java/com/todoroo/astrid/data/Task.kt +++ b/app/src/main/java/com/todoroo/astrid/data/Task.kt @@ -614,5 +614,8 @@ class Task : Parcelable { fun String?.isRepeatAfterCompletion() = this?.contains("FROM=COMPLETION") ?: false fun String?.withoutFrom(): String? = this?.replace(";?FROM=[^;]*".toRegex(), "") + + @JvmStatic + fun String?.withoutRRULE(): String? = this?.replace("^RRULE:".toRegex(), "") } } \ No newline at end of file diff --git a/app/src/main/java/com/todoroo/astrid/repeats/RepeatControlSet.kt b/app/src/main/java/com/todoroo/astrid/repeats/RepeatControlSet.kt index b80276536..2d1330602 100644 --- a/app/src/main/java/com/todoroo/astrid/repeats/RepeatControlSet.kt +++ b/app/src/main/java/com/todoroo/astrid/repeats/RepeatControlSet.kt @@ -145,7 +145,7 @@ class RepeatControlSet : TaskEditControlFragment() { displayView.text = null repeatTypeContainer.visibility = View.GONE } else { - displayView.text = repeatRuleToString.toString(it) + displayView.text = repeatRuleToString.toString(it.toIcal()) repeatTypeContainer.visibility = View.VISIBLE } } diff --git a/app/src/main/java/org/tasks/caldav/CaldavConverter.java b/app/src/main/java/org/tasks/caldav/CaldavConverter.java index d4fed037d..8ecc5df03 100644 --- a/app/src/main/java/org/tasks/caldav/CaldavConverter.java +++ b/app/src/main/java/org/tasks/caldav/CaldavConverter.java @@ -1,6 +1,7 @@ package org.tasks.caldav; import static com.todoroo.andlib.utility.DateUtilities.now; +import static com.todoroo.astrid.data.Task.withoutRRULE; import static org.tasks.caldav.iCalendar.getLocal; import static org.tasks.date.DateTimeUtils.newDateTime; import static org.tasks.time.DateTime.UTC; @@ -102,7 +103,7 @@ public class CaldavConverter { } if (task.isRecurring()) { try { - RRule rrule = new RRule(task.getRecurrenceWithoutFrom().replace("RRULE:", "")); + RRule rrule = new RRule(withoutRRULE(task.getRecurrenceWithoutFrom())); long repeatUntil = task.getRepeatUntil(); rrule .getRecur() diff --git a/app/src/main/java/org/tasks/caldav/iCalendar.kt b/app/src/main/java/org/tasks/caldav/iCalendar.kt index 992117d25..114345a09 100644 --- a/app/src/main/java/org/tasks/caldav/iCalendar.kt +++ b/app/src/main/java/org/tasks/caldav/iCalendar.kt @@ -202,7 +202,7 @@ class iCalendar @Inject constructor( dt.timeZone ?: if (dt.isUtc) UTC else TimeZone.getDefault() ) } else { - org.tasks.time.DateTime(property.date.time).let { it.minusMillis(it.offset) } + org.tasks.time.DateTime.from(property.date) } return dateTime?.toLocal()?.millis ?: 0 } diff --git a/app/src/main/java/org/tasks/preferences/fragments/TaskDefaults.kt b/app/src/main/java/org/tasks/preferences/fragments/TaskDefaults.kt index 1e1fae2b2..bfd0fd2d4 100644 --- a/app/src/main/java/org/tasks/preferences/fragments/TaskDefaults.kt +++ b/app/src/main/java/org/tasks/preferences/fragments/TaskDefaults.kt @@ -225,7 +225,7 @@ class TaskDefaults : InjectingPreferenceFragment() { ?.takeIf { it.isNotBlank() } ?.let { try { - repeatRuleToString.toString(RRule(it)) + repeatRuleToString.toString(it) } catch (e: Exception) { null } diff --git a/app/src/main/java/org/tasks/repeats/BasicRecurrenceDialog.java b/app/src/main/java/org/tasks/repeats/BasicRecurrenceDialog.java index 8b54921bc..c4c4690fd 100644 --- a/app/src/main/java/org/tasks/repeats/BasicRecurrenceDialog.java +++ b/app/src/main/java/org/tasks/repeats/BasicRecurrenceDialog.java @@ -77,7 +77,7 @@ public class BasicRecurrenceDialog extends DialogFragment { new SingleCheckedArrayAdapter(context, repeatOptions); int selected = 0; if (customPicked) { - adapter.insert(repeatRuleToString.toString(rrule), 0); + adapter.insert(repeatRuleToString.toString(rule), 0); } else if (rrule != null) { switch (rrule.getFreq()) { case DAILY: diff --git a/app/src/main/java/org/tasks/repeats/RepeatRuleToString.java b/app/src/main/java/org/tasks/repeats/RepeatRuleToString.java index 8e175b617..2f1c90d07 100644 --- a/app/src/main/java/org/tasks/repeats/RepeatRuleToString.java +++ b/app/src/main/java/org/tasks/repeats/RepeatRuleToString.java @@ -1,23 +1,27 @@ package org.tasks.repeats; -import static com.google.ical.values.Frequency.MONTHLY; -import static com.google.ical.values.Frequency.WEEKLY; +import static com.todoroo.astrid.data.Task.withoutRRULE; +import static net.fortuna.ical4j.model.Recur.Frequency.MONTHLY; +import static net.fortuna.ical4j.model.Recur.Frequency.WEEKLY; import android.content.Context; import com.google.common.base.Joiner; -import com.google.ical.values.Frequency; -import com.google.ical.values.RRule; -import com.google.ical.values.Weekday; -import com.google.ical.values.WeekdayNum; import com.todoroo.andlib.utility.DateUtilities; import dagger.hilt.android.qualifiers.ApplicationContext; import java.text.DateFormatSymbols; +import java.text.ParseException; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; import java.util.List; import javax.inject.Inject; +import net.fortuna.ical4j.model.Recur; +import net.fortuna.ical4j.model.Recur.Frequency; +import net.fortuna.ical4j.model.WeekDay; +import net.fortuna.ical4j.model.WeekDay.Day; +import net.fortuna.ical4j.model.property.RRule; import org.tasks.R; +import org.tasks.analytics.Firebase; import org.tasks.locale.Locale; import org.tasks.time.DateTime; @@ -25,24 +29,40 @@ public class RepeatRuleToString { private final Context context; private final Locale locale; - private final List weekdays = Arrays.asList(Weekday.values()); + private final Firebase firebase; + private final List weekdays = Arrays.asList(Day.values()); @Inject - public RepeatRuleToString(@ApplicationContext Context context, Locale locale) { + public RepeatRuleToString( + @ApplicationContext Context context, + Locale locale, + Firebase firebase + ) { this.context = context; this.locale = locale; + this.firebase = firebase; } - public String toString(RRule rrule) { + public String toString(String rrule) { + try { + return toString(new RRule(withoutRRULE(rrule))); + } catch (ParseException e) { + firebase.reportException(e); + } + return null; + } + + public String toString(RRule r) { + Recur rrule = r.getRecur(); int interval = rrule.getInterval(); - Frequency frequency = rrule.getFreq(); + Frequency frequency = rrule.getFrequency(); DateTime repeatUntil = rrule.getUntil() == null ? null : DateTime.from(rrule.getUntil()); int count = rrule.getCount(); String countString = count > 0 ? context.getResources().getQuantityString(R.plurals.repeat_times, count) : ""; if (interval <= 1) { String frequencyString = context.getString(getSingleFrequencyResource(frequency)); - if ((frequency == WEEKLY || frequency == MONTHLY) && !rrule.getByDay().isEmpty()) { + if ((frequency == WEEKLY || frequency == MONTHLY) && !rrule.getDayList().isEmpty()) { String dayString = getDayString(rrule); if (count > 0) { return context.getString( @@ -74,7 +94,7 @@ public class RepeatRuleToString { } else { int plural = getFrequencyPlural(frequency); String frequencyPlural = context.getResources().getQuantityString(plural, interval, interval); - if ((frequency == WEEKLY || frequency == MONTHLY) && !rrule.getByDay().isEmpty()) { + if ((frequency == WEEKLY || frequency == MONTHLY) && !rrule.getDayList().isEmpty()) { String dayString = getDayString(rrule); if (count > 0) { return context.getString( @@ -106,23 +126,23 @@ public class RepeatRuleToString { } } - private String getDayString(RRule rrule) { + private String getDayString(Recur rrule) { DateFormatSymbols dfs = new DateFormatSymbols(locale.getLocale()); - if (rrule.getFreq() == WEEKLY) { + if (rrule.getFrequency() == WEEKLY) { String[] shortWeekdays = dfs.getShortWeekdays(); List days = new ArrayList<>(); - for (WeekdayNum weekday : rrule.getByDay()) { - days.add(shortWeekdays[weekdays.indexOf(weekday.wday) + 1]); + for (WeekDay weekday : rrule.getDayList()) { + days.add(shortWeekdays[weekdays.indexOf(weekday.getDay()) + 1]); } return Joiner.on(context.getString(R.string.list_separator_with_space)).join(days); - } else if (rrule.getFreq() == MONTHLY) { + } else if (rrule.getFrequency() == MONTHLY) { String[] longWeekdays = dfs.getWeekdays(); - WeekdayNum weekdayNum = rrule.getByDay().get(0); + WeekDay weekdayNum = rrule.getDayList().get(0); String weekday; Calendar dayOfWeekCalendar = Calendar.getInstance(locale.getLocale()); - dayOfWeekCalendar.set(Calendar.DAY_OF_WEEK, weekdayToCalendarDay(weekdayNum.wday)); + dayOfWeekCalendar.set(Calendar.DAY_OF_WEEK, weekdayToCalendarDay(weekdayNum.getDay())); weekday = longWeekdays[dayOfWeekCalendar.get(Calendar.DAY_OF_WEEK)]; - if (weekdayNum.num == -1) { + if (weekdayNum.getOffset() == -1) { return context.getString( R.string.repeat_monthly_every_day_of_nth_week, context.getString(R.string.repeat_monthly_last_week), @@ -138,7 +158,7 @@ public class RepeatRuleToString { }; return context.getString( R.string.repeat_monthly_every_day_of_nth_week, - context.getString(resources[weekdayNum.num - 1]), + context.getString(resources[weekdayNum.getOffset() - 1]), weekday); } } else { @@ -146,7 +166,7 @@ public class RepeatRuleToString { } } - private int weekdayToCalendarDay(Weekday weekday) { + private int weekdayToCalendarDay(Day weekday) { switch (weekday) { case SU: return Calendar.SUNDAY; diff --git a/app/src/main/java/org/tasks/time/DateTime.java b/app/src/main/java/org/tasks/time/DateTime.java index 9f62c1fe0..9f94f9c12 100644 --- a/app/src/main/java/org/tasks/time/DateTime.java +++ b/app/src/main/java/org/tasks/time/DateTime.java @@ -85,6 +85,11 @@ public class DateTime { this(calendar.getTimeInMillis(), calendar.getTimeZone()); } + public static DateTime from(Date date) { + DateTime dateTime = new DateTime(date.getTime()); + return dateTime.minusMillis(dateTime.getOffset()); + } + public static DateTime from(DateValue dateValue) { if (dateValue == null) { return new DateTime(0);