From dd5f621c38580bf80cdc0bfcd7388bd60b38e60a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Mon, 2 Oct 2017 13:25:23 +0200 Subject: [PATCH] make ReminderScheduler remember if a reminder is custom (postponed) or not Without this, scheduling of any habit caused all other habits to be rescheduled at their configured time, even if they had been snoozed (thus discarding the snooze). This includes when the app gets closed and reschedules all habits the next time it is launched. --- .../uhabits/receivers/ReminderController.java | 2 +- .../receivers/ReminderControllerTest.java | 2 +- .../core/commands/DeleteHabitsCommand.java | 8 +++ .../core/commands/EditHabitCommand.java | 2 + .../core/reminders/ReminderScheduler.java | 42 +++++++++++- .../core/reminders/ReminderSchedulerTest.java | 64 ++++++++++++++++++- 6 files changed, 114 insertions(+), 6 deletions(-) diff --git a/uhabits-android/src/main/java/org/isoron/uhabits/receivers/ReminderController.java b/uhabits-android/src/main/java/org/isoron/uhabits/receivers/ReminderController.java index 179b16e39..ec0bca0be 100644 --- a/uhabits-android/src/main/java/org/isoron/uhabits/receivers/ReminderController.java +++ b/uhabits-android/src/main/java/org/isoron/uhabits/receivers/ReminderController.java @@ -94,7 +94,7 @@ public class ReminderController public void snoozeNotificationSetReminderTime(@NonNull Habit habit, long reminderTime) { - reminderScheduler.schedule(habit, reminderTime); + reminderScheduler.scheduleHabitAtCustom(habit, reminderTime); notificationTray.cancel(habit); } diff --git a/uhabits-android/src/test/java/org/isoron/uhabits/receivers/ReminderControllerTest.java b/uhabits-android/src/test/java/org/isoron/uhabits/receivers/ReminderControllerTest.java index 7b0406832..bb6338bd3 100644 --- a/uhabits-android/src/test/java/org/isoron/uhabits/receivers/ReminderControllerTest.java +++ b/uhabits-android/src/test/java/org/isoron/uhabits/receivers/ReminderControllerTest.java @@ -72,7 +72,7 @@ public class ReminderControllerTest extends BaseAndroidJVMTest controller.onSnooze(habit,null); - verify(reminderScheduler).schedule(habit, nowTz + 900000); + verify(reminderScheduler).scheduleHabitAtCustom(habit, nowTz + 900000); verify(notificationTray).cancel(habit); } diff --git a/uhabits-core/src/main/java/org/isoron/uhabits/core/commands/DeleteHabitsCommand.java b/uhabits-core/src/main/java/org/isoron/uhabits/core/commands/DeleteHabitsCommand.java index 1a99aa723..e5b476e78 100644 --- a/uhabits-core/src/main/java/org/isoron/uhabits/core/commands/DeleteHabitsCommand.java +++ b/uhabits-core/src/main/java/org/isoron/uhabits/core/commands/DeleteHabitsCommand.java @@ -69,6 +69,14 @@ public class DeleteHabitsCommand extends Command throw new UnsupportedOperationException(); } + public List getHabitIds() + { + List list = new LinkedList(); + for( Habit habit: selected) + list.add(habit.getId()); + return list; + } + public static class Record { @NonNull diff --git a/uhabits-core/src/main/java/org/isoron/uhabits/core/commands/EditHabitCommand.java b/uhabits-core/src/main/java/org/isoron/uhabits/core/commands/EditHabitCommand.java index 3d4f010d6..1a8f58d35 100644 --- a/uhabits-core/src/main/java/org/isoron/uhabits/core/commands/EditHabitCommand.java +++ b/uhabits-core/src/main/java/org/isoron/uhabits/core/commands/EditHabitCommand.java @@ -137,4 +137,6 @@ public class EditHabitCommand extends Command return command; } } + + public Long getHabitId() { return savedId; } } \ No newline at end of file diff --git a/uhabits-core/src/main/java/org/isoron/uhabits/core/reminders/ReminderScheduler.java b/uhabits-core/src/main/java/org/isoron/uhabits/core/reminders/ReminderScheduler.java index e31ed71ed..ec29328f0 100644 --- a/uhabits-core/src/main/java/org/isoron/uhabits/core/reminders/ReminderScheduler.java +++ b/uhabits-core/src/main/java/org/isoron/uhabits/core/reminders/ReminderScheduler.java @@ -41,6 +41,8 @@ public class ReminderScheduler implements CommandRunner.Listener private SystemScheduler sys; + private Map< Long, Long > customReminders; // Habit id, Timestamp + @Inject public ReminderScheduler(@NonNull CommandRunner commandRunner, @NonNull HabitList habitList, @@ -49,6 +51,7 @@ public class ReminderScheduler implements CommandRunner.Listener this.commandRunner = commandRunner; this.habitList = habitList; this.sys = sys; + customReminders = new HashMap< Long, Long >(); } @Override @@ -57,10 +60,45 @@ public class ReminderScheduler implements CommandRunner.Listener { if(command instanceof ToggleRepetitionCommand) return; if(command instanceof ChangeHabitColorCommand) return; + if(command instanceof EditHabitCommand) + { + Long habitId = ((EditHabitCommand)command).getHabitId(); + Habit habit = habitList.getById( habitId ); + customReminders.remove( habitId ); + scheduleHabit( habit ); + return; + } + if(command instanceof DeleteHabitsCommand) + { + List habitIds = ((DeleteHabitsCommand)command).getHabitIds(); + for(Long id : habitIds) + customReminders.remove( id ); + return; + } scheduleAll(); } - public void schedule(@NonNull Habit habit, @Nullable Long reminderTime) + public void scheduleHabit(@NonNull Habit habit) + { + Long reminderTime = null; + if( customReminders.containsKey( habit.getId())) + reminderTime = customReminders.get( habit.getId()); + scheduleInternal(habit, reminderTime); + } + + public void scheduleHabitAtReminder(@NonNull Habit habit) + { + customReminders.remove( habit.getId()); + scheduleInternal( habit, null ); + } + + public void scheduleHabitAtCustom(@NonNull Habit habit, @NonNull Long reminderTime) + { + customReminders.put( habit.getId(), reminderTime ); + scheduleInternal(habit, reminderTime); + } + + private void scheduleInternal(@NonNull Habit habit, @Nullable Long reminderTime) { if (!habit.hasReminder()) return; if (habit.isArchived()) return; @@ -76,7 +114,7 @@ public class ReminderScheduler implements CommandRunner.Listener HabitList reminderHabits = habitList.getFiltered(HabitMatcher.WITH_ALARM); for (Habit habit : reminderHabits) - schedule(habit, null); + scheduleHabit(habit); } public void startListening() diff --git a/uhabits-core/src/test/java/org/isoron/uhabits/core/reminders/ReminderSchedulerTest.java b/uhabits-core/src/test/java/org/isoron/uhabits/core/reminders/ReminderSchedulerTest.java index 480471670..e5b872d6e 100644 --- a/uhabits-core/src/test/java/org/isoron/uhabits/core/reminders/ReminderSchedulerTest.java +++ b/uhabits-core/src/test/java/org/isoron/uhabits/core/reminders/ReminderSchedulerTest.java @@ -20,6 +20,7 @@ package org.isoron.uhabits.core.reminders; import org.isoron.uhabits.core.*; +import org.isoron.uhabits.core.commands.*; import org.isoron.uhabits.core.models.*; import org.isoron.uhabits.core.utils.*; import org.junit.*; @@ -118,10 +119,69 @@ public class ReminderSchedulerTest extends BaseUnitTest @Test public void testSchedule_withoutReminder() { - reminderScheduler.schedule(habit, null); + reminderScheduler.scheduleHabit(habit); Mockito.verifyZeroInteractions(sys); } + @Test + public void testOnCommand() throws Exception + { + long now = timestamp(2015, 1, 26, 8, 0); + DateUtils.setFixedLocalTime(now); + + Habit h1 = fixtures.createEmptyHabit(); + h1.setReminder(new Reminder(14, 30, WeekdayList.EVERY_DAY)); + habitList.add(h1); + reminderScheduler.scheduleAll(); + + verify(sys).scheduleShowReminder(eq(timestamp(2015, 1, 26, 18, 30)), + eq(h1), anyLong()); + + reset(sys); + + reminderScheduler.scheduleHabitAtCustom(h1, + DateUtils.applyTimezone( timestamp( 2015, 1, 26, 15, 15))); + + verify(sys).scheduleShowReminder(eq(timestamp(2015, 1, 26, 19, 15)), + eq(h1), anyLong()); + + Habit h2Model = fixtures.createEmptyHabit(); + h2Model.setReminder(new Reminder(15,30, WeekdayList.EVERY_DAY)); + CreateHabitCommand createCommand = new CreateHabitCommand(modelFactory,habitList,h2Model); + createCommand.execute(); + reminderScheduler.onCommandExecuted(createCommand, null); + Habit h2 = habitList.getByPosition(1); + + verify(sys).scheduleShowReminder(eq(timestamp(2015, 1, 26, 19, 30)), + eq(h2), anyLong()); + verify(sys, never()).scheduleShowReminder(eq(timestamp(2015, 1, 26, 18, 30)), + eq(h1), anyLong()); + verify(sys,never()).scheduleShowReminder(eq(timestamp(2015, 1, 26, 19, 30)), + eq(h1), anyLong()); + + Habit h1New = fixtures.createEmptyHabit(); + h1New.copyFrom(h1); + h1New.setReminder(new Reminder(17,30, WeekdayList.EVERY_DAY)); + EditHabitCommand editCommand = new EditHabitCommand(modelFactory,habitList,h1,h1New); + editCommand.execute(); + reminderScheduler.onCommandExecuted(editCommand, null); + + verify(sys).scheduleShowReminder(eq(timestamp(2015, 1, 26, 21, 30)), + eq(h1), anyLong()); + verify(sys,never()).scheduleShowReminder(eq(timestamp(2015, 1, 26, 21, 30)), + eq(h2), anyLong()); + + h1.setReminder(new Reminder( 18, 30, WeekdayList.EVERY_DAY)); + reminderScheduler.scheduleHabitAtCustom(h1, + DateUtils.applyTimezone( timestamp( 2015, 1, 26,18, 45))); + reminderScheduler.scheduleAll(); + + verify(sys, atLeastOnce()).scheduleShowReminder(eq(timestamp(2015, 1, 26, 22, 45)), + eq(h1), anyLong()); + verify(sys, never()).scheduleShowReminder(eq(timestamp(2015, 1, 26, 22, 30)), + eq(h1), anyLong()); + } + public long timestamp(int year, int month, int day, int hour, int minute) { Calendar cal = DateUtils.getStartOfTodayCalendar(); @@ -133,7 +193,7 @@ public class ReminderSchedulerTest extends BaseUnitTest long expectedCheckmarkTime, long expectedReminderTime) { - reminderScheduler.schedule(habit, atTime); + reminderScheduler.scheduleHabitAtCustom(habit, atTime); verify(sys).scheduleShowReminder(expectedReminderTime, habit, expectedCheckmarkTime); }