Fixing issues with reminder tests and reminders themselves

astrid/plugin-src/com/todoroo/astrid/reminders/ReminderService.java

@@ -164,6 +164,10 @@ public final class ReminderService  {
         // notifications after due date
         long whenOverdue = calculateNextOverdueReminder(task);
 
+        // if random reminders are too close to due date, favor due date
+        if(whenRandom != NO_ALARM && whenDueDate - whenRandom < DateUtilities.ONE_DAY)
+            whenRandom = NO_ALARM;
+
         if(whenRandom < whenDueDate && whenRandom < whenOverdue)
             scheduler.createAlarm(task, whenRandom, TYPE_RANDOM);
         else if(whenDueDate < whenOverdue)
@@ -233,7 +237,7 @@ public final class ReminderService  {
             if(when == 0) {
                 when = DateUtilities.now();
                 task.setValue(Task.REMINDER_LAST, when);
-                taskDao.save(task, false);
+                taskDao.saveExisting(task);
             }
 
             when += (long)(reminderPeriod * (0.85f + 0.3f * random.nextFloat()));

astrid/src/com/todoroo/astrid/dao/TaskDao.java

@@ -215,42 +215,6 @@ public class TaskDao extends GenericDao<Task> {
 
         if(skipHooks)
             return;
-
-        // TODO due date was updated, update calendar event
-        /*if((values.containsKey(AbstractTaskModel.DEFINITE_DUE_DATE) ||
-                values.containsKey(AbstractTaskModel.PREFERRED_DUE_DATE)) &&
-                !values.containsKey(AbstractTaskModel.CALENDAR_URI)) {
-            try {
-                Cursor cursor = fetchTaskCursor(task.getTaskIdentifier(),
-                        new String[] { AbstractTaskModel.CALENDAR_URI });
-                cursor.moveToFirst();
-                String uriAsString = cursor.getString(0);
-                cursor.close();
-                if(uriAsString != null && uriAsString.length() > 0) {
-                    ContentResolver cr = context.getContentResolver();
-                    Uri uri = Uri.parse(uriAsString);
-
-                    Integer estimated = null;
-                    if(values.containsKey(AbstractTaskModel.ESTIMATED_SECONDS))
-                        estimated = values.getAsInteger(AbstractTaskModel.ESTIMATED_SECONDS);
-                    else { // read from event
-                        Cursor event = cr.query(uri, new String[] {"dtstart", "dtend"},
-                                null, null, null);
-                        event.moveToFirst();
-                        estimated = (event.getInt(1) - event.getInt(0))/1000;
-                    }
-
-                    // create new start and end date for this event
-                    ContentValues newValues = new ContentValues();
-                    TaskEditActivity.createCalendarStartEndTimes(task.getPreferredDueDate(),
-                            task.getDefiniteDueDate(), estimated, newValues);
-                    cr.update(uri, newValues, null, null);
-                }
-            } catch (Exception e) {
-                // ignore calendar event - event could be deleted or whatever
-                Log.e("astrid", "Error moving calendar event", e);
-            }
-        }*/
     }
 
     /**
@@ -261,34 +225,6 @@ public class TaskDao extends GenericDao<Task> {
      * @param duringSync
      */
     private void afterComplete(Task task, ContentValues values, boolean duringSync) {
-
-
-        /*Cursor cursor = fetchTaskCursor(task.getTaskIdentifier(),
-                TaskModelForHandlers.FIELD_LIST);
-        TaskModelForHandlers model = new TaskModelForHandlers(cursor, values);
-
-        // handle repeat
-        RepeatInfo repeatInfo = model.getRepeat();
-        if(repeatInfo != null) {
-            model.repeatTaskBy(context, this, repeatInfo);
-            database.update(tasksTable, values, KEY_ROWID + "=" +
-                    task.getTaskIdentifier().getId(), null);
-        }
-
-        // handle sync-on-complete
-        if((model.getFlags() & TaskModelForHandlers.FLAG_SYNC_ON_COMPLETE) > 0 &&
-                !duringSync) {
-            Synchronizer synchronizer = new Synchronizer(model.getTaskIdentifier());
-            synchronizer.synchronize(context, new SynchronizerListener() {
-                public void onSynchronizerFinished(int numServicesSynced) {
-                    TaskListSubActivity.shouldRefreshTaskList = true;
-                }
-            });
-        }
-
-        cursor.close();
-        cleanupTask(task.getTaskIdentifier(), repeatInfo != null);*/
-
         // send broadcast
         if(!duringSync) {
             Context context = ContextManager.getContext();

tests/src/com/todoroo/astrid/reminders/ReminderServiceTests.java

@@ -74,12 +74,6 @@ public class ReminderServiceTests extends DatabaseTestCase {
         });
         taskDao.save(task, false);
         assertTrue(((AlarmExpected)service.getScheduler()).alarmCreated);
-
-        // test random with last notify way in the past
-        task.setValue(Task.REMINDER_LAST, DateUtilities.now() - 2 * DateUtilities.ONE_WEEK);
-        ((AlarmExpected)service.getScheduler()).alarmCreated = false;
-        service.scheduleAlarm(task);
-        assertTrue(((AlarmExpected)service.getScheduler()).alarmCreated);
     }
 
     /** tests with overdue */
@@ -114,7 +108,7 @@ public class ReminderServiceTests extends DatabaseTestCase {
         task.setValue(Task.TITLE, "water");
         task.setValue(Task.DUE_DATE, DateUtilities.now() + DateUtilities.ONE_WEEK);
         task.setValue(Task.REMINDER_FLAGS, Task.NOTIFY_AT_DEADLINE);
-        task.setValue(Task.REMINDER_PERIOD, DateUtilities.ONE_DAY);
+        task.setValue(Task.REMINDER_PERIOD, DateUtilities.ONE_HOUR);
         service.setScheduler(new AlarmExpected() {
             @Override
             public void createAlarm(Task task, long time, int type) {