From 38d3b0d047592863137c42faa794a888bb504e8f Mon Sep 17 00:00:00 2001 From: Alinson Xavier Date: Fri, 23 Jun 2017 12:12:23 -0400 Subject: [PATCH] Update habit.position after reordering list --- .../core/models/memory/MemoryHabitList.java | 23 ++--- .../uhabits/core/test/HabitFixtures.java | 7 +- .../org/isoron/uhabits/core/BaseUnitTest.java | 2 +- .../database/migrations/Version22Test.java | 2 +- .../uhabits/core/models/HabitListTest.java | 51 ++++++----- .../models/sqlite/SQLiteHabitListTest.java | 89 ++++++++++--------- .../sqlite/SQLiteRepetitionListTest.java | 2 +- 7 files changed, 96 insertions(+), 80 deletions(-) diff --git a/uhabits-core/src/main/java/org/isoron/uhabits/core/models/memory/MemoryHabitList.java b/uhabits-core/src/main/java/org/isoron/uhabits/core/models/memory/MemoryHabitList.java index 949b340a5..5e2727114 100644 --- a/uhabits-core/src/main/java/org/isoron/uhabits/core/models/memory/MemoryHabitList.java +++ b/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.BY_NAME; -import static org.isoron.uhabits.core.models.HabitList.Order.BY_POSITION; -import static org.isoron.uhabits.core.models.HabitList.Order.BY_SCORE; +import static org.isoron.uhabits.core.models.HabitList.Order.*; /** * 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) - throw new IllegalArgumentException( - "list does not contain (from) habit"); + if (order != BY_POSITION) throw new IllegalStateException( + "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) - throw new IllegalArgumentException( - "list does not contain (to) habit"); + if (toPos < 0) 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(); } diff --git a/uhabits-core/src/main/java/org/isoron/uhabits/core/test/HabitFixtures.java b/uhabits-core/src/main/java/org/isoron/uhabits/core/test/HabitFixtures.java index 67a74c329..6f0a984ee 100644 --- a/uhabits-core/src/main/java/org/isoron/uhabits/core/test/HabitFixtures.java +++ b/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); } } diff --git a/uhabits-core/src/test/java/org/isoron/uhabits/core/BaseUnitTest.java b/uhabits-core/src/test/java/org/isoron/uhabits/core/BaseUnitTest.java index 1e119c0b9..cafdf371d 100644 --- a/uhabits-core/src/test/java/org/isoron/uhabits/core/BaseUnitTest.java +++ b/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); } diff --git a/uhabits-core/src/test/java/org/isoron/uhabits/core/database/migrations/Version22Test.java b/uhabits-core/src/test/java/org/isoron/uhabits/core/database/migrations/Version22Test.java index 8463b70e1..395f06c3f 100644 --- a/uhabits-core/src/test/java/org/isoron/uhabits/core/database/migrations/Version22Test.java +++ b/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 diff --git a/uhabits-core/src/test/java/org/isoron/uhabits/core/models/HabitListTest.java b/uhabits-core/src/test/java/org/isoron/uhabits/core/models/HabitListTest.java index 0a61299c3..206caafe6 100644 --- a/uhabits-core/src/test/java/org/isoron/uhabits/core/models/HabitListTest.java +++ b/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.isoron.uhabits.core.models.HabitList.Order.BY_COLOR; -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.hamcrest.MatcherAssert.assertThat; +import static org.isoron.uhabits.core.models.HabitList.Order.*; 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[][] = { - { 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 }, + int expectedSequence[][] = { + { 0, 1, 5, 2, 3, 4, 6, 7, 8, 9 }, + { 0, 1, 5, 2, 4, 6, 7, 3, 8, 9 }, + { 0, 1, 5, 2, 4, 6, 7, 3, 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]; - int to = operations[i][1]; - - Habit fromHabit = habitList.getByPosition(from); - Habit toHabit = habitList.getByPosition(to); + Habit fromHabit = habitsArray.get(operations[i][0]); + Habit toHabit = habitsArray.get(operations[i][1]); 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); - if (h == null) fail(); - actualPositions[j] = habitList.indexOf(h); + Habit h = habitList.getByPosition(j); + assertThat(h.getPosition(), equalTo(j)); + 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); + } } \ No newline at end of file diff --git a/uhabits-core/src/test/java/org/isoron/uhabits/core/models/sqlite/SQLiteHabitListTest.java b/uhabits-core/src/test/java/org/isoron/uhabits/core/models/sqlite/SQLiteHabitListTest.java index 72222413e..a1d79372d 100644 --- a/uhabits-core/src/test/java/org/isoron/uhabits/core/models/sqlite/SQLiteHabitListTest.java +++ b/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 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(); - h.setName("habit " + i); - h.setId((long) i); - if (i % 2 == 0) h.setArchived(true); - - HabitRecord record = new HabitRecord(); - record.copyFrom(h); - record.position = i; - repository.save(record); + Habit habit = fixtures.createEmptyHabit(); + habit.setName("habit " + i); + habitList.update(habit); + habitsArray.add(habit); + + if (i % 3 == 0) + habit.setReminder(new Reminder(8, 30, WeekdayList.EVERY_DAY)); } - 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 - int operations[][] = { - { 5, 2 }, { 3, 7 }, { 4, 4 }, { 3, 2 } - }; - - 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]; + Habit habit3 = habitList.getById(3); + Habit habit4 = habitList.getById(4); + assertNotNull(habit3); + assertNotNull(habit4); + habitList.reorder(habit4, habit3); + + HabitRecord record3 = repository.find(3L); + assertNotNull(record3); + assertThat(record3.position, equalTo(4)); + + HabitRecord record4 = repository.find(4L); + assertNotNull(record4); + assertThat(record4.position, equalTo(3)); + } - for (int j = 0; j < 10; j++) - { - Habit h = habitList.getById(j); - assertNotNull(h); - actualPositions[j] = habitList.indexOf(h); - } - assertThat(actualPositions, equalTo(expectedPosition[i])); - } - } } \ No newline at end of file diff --git a/uhabits-core/src/test/java/org/isoron/uhabits/core/models/sqlite/SQLiteRepetitionListTest.java b/uhabits-core/src/test/java/org/isoron/uhabits/core/models/sqlite/SQLiteRepetitionListTest.java index 45339e9cf..f26d06a62 100644 --- a/uhabits-core/src/test/java/org/isoron/uhabits/core/models/sqlite/SQLiteRepetitionListTest.java +++ b/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();