Update habit.position after reordering list

uhabits-core/src/main/java/org/isoron/uhabits/core/models/memory/MemoryHabitList.java

@@ -25,10 +25,7 @@ import org.isoron.uhabits.core.models.*;
 
 import java.util.*;
 
-import static org.isoron.uhabits.core.models.HabitList.Order.BY_COLOR;
+import static org.isoron.uhabits.core.models.HabitList.Order.*;
-import static org.isoron.uhabits.core.models.HabitList.Order.BY_NAME;
-import static org.isoron.uhabits.core.models.HabitList.Order.BY_POSITION;
-import static org.isoron.uhabits.core.models.HabitList.Order.BY_SCORE;
 
 /**
  * In-memory implementation of {@link HabitList}.
@@ -182,17 +179,23 @@ public class MemoryHabitList extends HabitList
     public synchronized void reorder(@NonNull Habit from, @NonNull Habit to)
     {
         throwIfHasParent();
-        if (indexOf(from) < 0)
+        if (order != BY_POSITION) throw new IllegalStateException(
-            throw new IllegalArgumentException(
+            "cannot reorder automatically sorted list");
+
+        if (indexOf(from) < 0) throw new IllegalArgumentException(
             "list does not contain (from) habit");
 
         int toPos = indexOf(to);
-        if (toPos < 0)
+        if (toPos < 0) throw new IllegalArgumentException(
-            throw new IllegalArgumentException(
             "list does not contain (to) habit");
 
         list.remove(from);
         list.add(toPos, from);
+
+        int position = 0;
+        for(Habit h : list)
+            h.setPosition(position++);
+
         getObservable().notifyListeners();
     }
 

uhabits-core/src/main/java/org/isoron/uhabits/core/test/HabitFixtures.java

@@ -31,9 +31,12 @@ public class HabitFixtures
 
     private final ModelFactory modelFactory;
 
-    public HabitFixtures(ModelFactory modelFactory)
+    private HabitList habitList;
+
+    public HabitFixtures(ModelFactory modelFactory, HabitList habitList)
     {
         this.modelFactory = modelFactory;
+        this.habitList = habitList;
     }
 
     public Habit createEmptyHabit()
@@ -113,6 +116,6 @@ public class HabitFixtures
     private void saveIfSQLite(Habit habit)
     {
         if (!(habit.getRepetitions() instanceof SQLiteRepetitionList)) return;
-        new SQLiteHabitList(modelFactory).add(habit);
+        habitList.add(habit);
     }
 }

uhabits-core/src/test/java/org/isoron/uhabits/core/BaseUnitTest.java

@@ -80,7 +80,7 @@ public class BaseUnitTest
 
         modelFactory = new MemoryModelFactory();
         habitList = spy(modelFactory.buildHabitList());
-        fixtures = new HabitFixtures(modelFactory);
+        fixtures = new HabitFixtures(modelFactory, habitList);
         taskRunner = new SingleThreadTaskRunner();
         commandRunner = new CommandRunner(taskRunner);
     }

uhabits-core/src/test/java/org/isoron/uhabits/core/database/migrations/Version22Test.java

@@ -48,7 +48,7 @@ public class Version22Test extends BaseUnitTest
         helper = new MigrationHelper(db);
         modelFactory = new SQLModelFactory(db);
         habitList = modelFactory.buildHabitList();
-        fixtures = new HabitFixtures(modelFactory);
+        fixtures = new HabitFixtures(modelFactory, habitList);
     }
 
     @Test

uhabits-core/src/test/java/org/isoron/uhabits/core/models/HabitListTest.java

@@ -26,13 +26,11 @@ import org.junit.rules.*;
 import java.io.*;
 import java.util.*;
 
+import static java.lang.Math.*;
 import static junit.framework.TestCase.assertFalse;
-import static junit.framework.TestCase.fail;
 import static org.hamcrest.CoreMatchers.*;
-import static org.hamcrest.MatcherAssert.*;
+import static org.hamcrest.MatcherAssert.assertThat;
-import static org.isoron.uhabits.core.models.HabitList.Order.BY_COLOR;
+import static org.isoron.uhabits.core.models.HabitList.Order.*;
-import static org.isoron.uhabits.core.models.HabitList.Order.BY_NAME;
-import static org.isoron.uhabits.core.models.HabitList.Order.BY_POSITION;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
@@ -173,36 +171,35 @@ public class HabitListTest extends BaseUnitTest
     public void testReorder()
     {
         int operations[][] = {
-            { 5, 2 }, { 3, 7 }, { 4, 4 }, { 3, 2 }
+            { 5, 2 }, { 3, 7 }, { 4, 4 }, { 8, 3 }
         };
 
-        int expectedPosition[][] = {
+        int expectedSequence[][] = {
-            { 0, 1, 3, 4, 5, 2, 6, 7, 8, 9 },
+            { 0, 1, 5, 2, 3, 4, 6, 7, 8, 9 },
-            { 0, 1, 7, 3, 4, 2, 5, 6, 8, 9 },
+            { 0, 1, 5, 2, 4, 6, 7, 3, 8, 9 },
-            { 0, 1, 7, 3, 4, 2, 5, 6, 8, 9 },
+            { 0, 1, 5, 2, 4, 6, 7, 3, 8, 9 },
-            { 0, 1, 7, 2, 4, 3, 5, 6, 8, 9 },
+            { 0, 1, 5, 2, 4, 6, 7, 8, 3, 9 },
         };
 
         for (int i = 0; i < operations.length; i++)
         {
-            int from = operations[i][0];
+            Habit fromHabit = habitsArray.get(operations[i][0]);
-            int to = operations[i][1];
+            Habit toHabit = habitsArray.get(operations[i][1]);
-
-            Habit fromHabit = habitList.getByPosition(from);
-            Habit toHabit = habitList.getByPosition(to);
             habitList.reorder(fromHabit, toHabit);
 
-            int actualPositions[] = new int[10];
+            int actualSequence[] = new int[10];
-
             for (int j = 0; j < 10; j++)
             {
-                Habit h = habitList.getById(j);
+                Habit h = habitList.getByPosition(j);
-                if (h == null) fail();
+                assertThat(h.getPosition(), equalTo(j));
-                actualPositions[j] = habitList.indexOf(h);
+                actualSequence[j] = toIntExact(h.getId());
             }
 
-            assertThat(actualPositions, equalTo(expectedPosition[i]));
+            assertThat(actualSequence, equalTo(expectedSequence[i]));
         }
+
+        assertThat(activeHabits.indexOf(habitsArray.get(5)), equalTo(0));
+        assertThat(activeHabits.indexOf(habitsArray.get(2)), equalTo(1));
     }
 
     @Test
@@ -281,4 +278,14 @@ public class HabitListTest extends BaseUnitTest
         thrown.expect(IllegalStateException.class);
         activeHabits.reorder(h1, h2);
     }
+
+    @Test
+    public void testReorder_onSortedList() throws Exception
+    {
+        habitList.setOrder(BY_SCORE);
+        Habit h1 = habitsArray.get(1);
+        Habit h2 = habitsArray.get(2);
+        thrown.expect(IllegalStateException.class);
+        habitList.reorder(h1, h2);
+    }
 }

uhabits-core/src/test/java/org/isoron/uhabits/core/models/sqlite/SQLiteHabitListTest.java

@@ -25,9 +25,12 @@ import org.isoron.uhabits.core.*;
 import org.isoron.uhabits.core.database.*;
 import org.isoron.uhabits.core.models.*;
 import org.isoron.uhabits.core.models.sqlite.records.*;
+import org.isoron.uhabits.core.test.*;
 import org.junit.*;
 import org.junit.rules.*;
 
+import java.util.*;
+
 import static org.hamcrest.CoreMatchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.*;
@@ -44,28 +47,46 @@ public class SQLiteHabitListTest extends BaseUnitTest
 
     private ModelObservable.Listener listener;
 
+    private ArrayList<Habit> habitsArray;
+
+    private HabitList activeHabits;
+
+    private HabitList reminderHabits;
+
     @Override
     public void setUp() throws Exception
     {
         super.setUp();
         Database db = buildMemoryDatabase();
+        modelFactory = new SQLModelFactory(db);
+        habitList = new SQLiteHabitList(modelFactory);
+        fixtures = new HabitFixtures(modelFactory, habitList);
         repository = new Repository<>(HabitRecord.class, db);
-        habitList = new SQLiteHabitList(new SQLModelFactory(db));
+        habitsArray = new ArrayList<>();
 
         for (int i = 0; i < 10; i++)
         {
-            Habit h = modelFactory.buildHabit();
+            Habit habit = fixtures.createEmptyHabit();
-            h.setName("habit " + i);
+            habit.setName("habit " + i);
-            h.setId((long) i);
+            habitList.update(habit);
-            if (i % 2 == 0) h.setArchived(true);
+            habitsArray.add(habit);
-
+
-            HabitRecord record = new HabitRecord();
+            if (i % 3 == 0)
-            record.copyFrom(h);
+                habit.setReminder(new Reminder(8, 30, WeekdayList.EVERY_DAY));
-            record.position = i;
-            repository.save(record);
         }
 
-        habitList.reload();
+        habitsArray.get(0).setArchived(true);
+        habitsArray.get(1).setArchived(true);
+        habitsArray.get(4).setArchived(true);
+        habitsArray.get(7).setArchived(true);
+        habitList.update(habitsArray);
+
+        activeHabits = habitList.getFiltered(new HabitMatcherBuilder().build());
+
+        reminderHabits = habitList.getFiltered(new HabitMatcherBuilder()
+            .setArchivedAllowed(true)
+            .setReminderRequired(true)
+            .build());
 
         listener = mock(ModelObservable.Listener.class);
         habitList.getObservable().addListener(listener);
@@ -185,38 +206,20 @@ public class SQLiteHabitListTest extends BaseUnitTest
     @Test
     public void testReorder()
     {
-        // Same as HabitListTest#testReorder
+        Habit habit3 = habitList.getById(3);
-        int operations[][] = {
+        Habit habit4 = habitList.getById(4);
-            { 5, 2 }, { 3, 7 }, { 4, 4 }, { 3, 2 }
+        assertNotNull(habit3);
-        };
+        assertNotNull(habit4);
-
+        habitList.reorder(habit4, habit3);
-        int expectedPosition[][] = {
-            { 0, 1, 3, 4, 5, 2, 6, 7, 8, 9 },
-            { 0, 1, 7, 3, 4, 2, 5, 6, 8, 9 },
-            { 0, 1, 7, 3, 4, 2, 5, 6, 8, 9 },
-            { 0, 1, 7, 2, 4, 3, 5, 6, 8, 9 },
-        };
-
-        for (int i = 0; i < operations.length; i++)
-        {
-            int from = operations[i][0];
-            int to = operations[i][1];
-
-            Habit fromHabit = habitList.getByPosition(from);
-            Habit toHabit = habitList.getByPosition(to);
-            habitList.reorder(fromHabit, toHabit);
-            habitList.reload();
 
-            int actualPositions[] = new int[10];
+        HabitRecord record3 = repository.find(3L);
+        assertNotNull(record3);
+        assertThat(record3.position, equalTo(4));
 
-            for (int j = 0; j < 10; j++)
+        HabitRecord record4 = repository.find(4L);
-            {
+        assertNotNull(record4);
-                Habit h = habitList.getById(j);
+        assertThat(record4.position, equalTo(3));
-                assertNotNull(h);
-                actualPositions[j] = habitList.indexOf(h);
     }
 
-            assertThat(actualPositions, equalTo(expectedPosition[i]));
+
-        }
-    }
 }

uhabits-core/src/test/java/org/isoron/uhabits/core/models/sqlite/SQLiteRepetitionListTest.java

@@ -56,8 +56,8 @@ public class SQLiteRepetitionListTest extends BaseUnitTest
 
         Database db = buildMemoryDatabase();
         modelFactory = new SQLModelFactory(db);
-        fixtures = new HabitFixtures(modelFactory);
         habitList = modelFactory.buildHabitList();
+        fixtures = new HabitFixtures(modelFactory, habitList);
         repository = new Repository<>(RepetitionRecord.class, db);
         habit = fixtures.createLongHabit();