From 48c1adb3bb5e2f6189e40aae7da1e64fadc7b28f Mon Sep 17 00:00:00 2001 From: "Alinson S. Xavier" Date: Sat, 30 Jan 2021 10:26:07 -0600 Subject: [PATCH] HabitCardListAdapter: Return copy of list of selected items Previously, HabitCardListAdapter returned a pointer to the list, instead of a copy. By the time other parts of the application were reading the list, its contents had already changed. This prevented the user from deleting or archiving habits. --- .../habits/list/views/HabitCardListAdapter.kt | 6 ++++- .../list/ListHabitsSelectionMenuBehavior.kt | 24 +++++++++---------- .../ListHabitsSelectionMenuBehaviorTest.kt | 22 ++++++++--------- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardListAdapter.kt b/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardListAdapter.kt index bdfaebd62..ab6caf87e 100644 --- a/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardListAdapter.kt +++ b/uhabits-android/src/main/java/org/isoron/uhabits/activities/habits/list/views/HabitCardListAdapter.kt @@ -53,7 +53,7 @@ class HabitCardListAdapter @Inject constructor( ListHabitsSelectionMenuBehavior.Adapter { val observable: ModelObservable = ModelObservable() private var listView: HabitCardListView? = null - override val selected: LinkedList = LinkedList() + val selected: LinkedList = LinkedList() override fun atMidnight() { cache.refreshAllHabits() } @@ -71,6 +71,10 @@ class HabitCardListAdapter @Inject constructor( observable.notifyListeners() } + override fun getSelected(): List { + return ArrayList(selected) + } + /** * Returns the item that occupies a certain position on the list * diff --git a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/list/ListHabitsSelectionMenuBehavior.kt b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/list/ListHabitsSelectionMenuBehavior.kt index cc43ecba4..c978eba83 100644 --- a/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/list/ListHabitsSelectionMenuBehavior.kt +++ b/uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/ui/screens/habits/list/ListHabitsSelectionMenuBehavior.kt @@ -37,28 +37,28 @@ class ListHabitsSelectionMenuBehavior @Inject constructor( var commandRunner: CommandRunner ) { fun canArchive(): Boolean { - for (habit in adapter.selected) if (habit.isArchived) return false + for (habit in adapter.getSelected()) if (habit.isArchived) return false return true } fun canEdit(): Boolean { - return adapter.selected.size == 1 + return adapter.getSelected().size == 1 } fun canUnarchive(): Boolean { - for (habit in adapter.selected) if (!habit.isArchived) return false + for (habit in adapter.getSelected()) if (!habit.isArchived) return false return true } fun onArchiveHabits() { - commandRunner.run(ArchiveHabitsCommand(habitList, adapter.selected)) + commandRunner.run(ArchiveHabitsCommand(habitList, adapter.getSelected())) adapter.clearSelection() } fun onChangeColor() { - val (color) = adapter.selected[0] + val (color) = adapter.getSelected()[0] screen.showColorPicker(color) { selectedColor: PaletteColor -> - commandRunner.run(ChangeHabitColorCommand(habitList, adapter.selected, selectedColor)) + commandRunner.run(ChangeHabitColorCommand(habitList, adapter.getSelected(), selectedColor)) adapter.clearSelection() } } @@ -66,27 +66,27 @@ class ListHabitsSelectionMenuBehavior @Inject constructor( fun onDeleteHabits() { screen.showDeleteConfirmationScreen( { - adapter.performRemove(adapter.selected) - commandRunner.run(DeleteHabitsCommand(habitList, adapter.selected)) + adapter.performRemove(adapter.getSelected()) + commandRunner.run(DeleteHabitsCommand(habitList, adapter.getSelected())) adapter.clearSelection() }, - adapter.selected.size + adapter.getSelected().size ) } fun onEditHabits() { - screen.showEditHabitsScreen(adapter.selected) + screen.showEditHabitsScreen(adapter.getSelected()) adapter.clearSelection() } fun onUnarchiveHabits() { - commandRunner.run(UnarchiveHabitsCommand(habitList, adapter.selected)) + commandRunner.run(UnarchiveHabitsCommand(habitList, adapter.getSelected())) adapter.clearSelection() } interface Adapter { fun clearSelection() - val selected: List + fun getSelected(): List fun performRemove(selected: List) } diff --git a/uhabits-core/src/jvmTest/java/org/isoron/uhabits/core/ui/screens/habits/list/ListHabitsSelectionMenuBehaviorTest.kt b/uhabits-core/src/jvmTest/java/org/isoron/uhabits/core/ui/screens/habits/list/ListHabitsSelectionMenuBehaviorTest.kt index 5974d3ad1..56fe20a3f 100644 --- a/uhabits-core/src/jvmTest/java/org/isoron/uhabits/core/ui/screens/habits/list/ListHabitsSelectionMenuBehaviorTest.kt +++ b/uhabits-core/src/jvmTest/java/org/isoron/uhabits/core/ui/screens/habits/list/ListHabitsSelectionMenuBehaviorTest.kt @@ -52,27 +52,27 @@ class ListHabitsSelectionMenuBehaviorTest : BaseUnitTest() { @Test @Throws(Exception::class) fun canArchive() { - whenever(adapter.selected).thenReturn(listOf(habit1, habit2)) + whenever(adapter.getSelected()).thenReturn(listOf(habit1, habit2)) assertFalse(behavior.canArchive()) - whenever(adapter.selected).thenReturn(listOf(habit2, habit3)) + whenever(adapter.getSelected()).thenReturn(listOf(habit2, habit3)) assertTrue(behavior.canArchive()) } @Test @Throws(Exception::class) fun canEdit() { - whenever(adapter.selected).thenReturn(listOf(habit1)) + whenever(adapter.getSelected()).thenReturn(listOf(habit1)) assertTrue(behavior.canEdit()) - whenever(adapter.selected).thenReturn(listOf(habit1, habit2)) + whenever(adapter.getSelected()).thenReturn(listOf(habit1, habit2)) assertFalse(behavior.canEdit()) } @Test @Throws(Exception::class) fun canUnarchive() { - whenever(adapter.selected).thenReturn(listOf(habit1, habit2)) + whenever(adapter.getSelected()).thenReturn(listOf(habit1, habit2)) assertFalse(behavior.canUnarchive()) - whenever(adapter.selected).thenReturn(listOf(habit1)) + whenever(adapter.getSelected()).thenReturn(listOf(habit1)) assertTrue(behavior.canUnarchive()) } @@ -80,7 +80,7 @@ class ListHabitsSelectionMenuBehaviorTest : BaseUnitTest() { @Throws(Exception::class) fun onArchiveHabits() { assertFalse(habit2.isArchived) - whenever(adapter.selected).thenReturn(listOf(habit2)) + whenever(adapter.getSelected()).thenReturn(listOf(habit2)) behavior.onArchiveHabits() assertTrue(habit2.isArchived) } @@ -90,7 +90,7 @@ class ListHabitsSelectionMenuBehaviorTest : BaseUnitTest() { fun onChangeColor() { assertThat(habit1.color, equalTo(PaletteColor(8))) assertThat(habit2.color, equalTo(PaletteColor(8))) - whenever(adapter.selected).thenReturn(listOf(habit1, habit2)) + whenever(adapter.getSelected()).thenReturn(listOf(habit1, habit2)) behavior.onChangeColor() verify(screen) .showColorPicker(eq(PaletteColor(8)), colorPickerCallback.capture()) @@ -103,7 +103,7 @@ class ListHabitsSelectionMenuBehaviorTest : BaseUnitTest() { fun onDeleteHabits() { val id = habit1.id!! habitList.getById(id)!! - whenever(adapter.selected).thenReturn(listOf(habit1)) + whenever(adapter.getSelected()).thenReturn(listOf(habit1)) behavior.onDeleteHabits() verify(screen).showDeleteConfirmationScreen(deleteCallback.capture(), eq(1)) deleteCallback.lastValue.onConfirmed() @@ -114,7 +114,7 @@ class ListHabitsSelectionMenuBehaviorTest : BaseUnitTest() { @Throws(Exception::class) fun onEditHabits() { val selected: List = listOf(habit1, habit2) - whenever(adapter.selected).thenReturn(selected) + whenever(adapter.getSelected()).thenReturn(selected) behavior.onEditHabits() verify(screen).showEditHabitsScreen(selected) } @@ -123,7 +123,7 @@ class ListHabitsSelectionMenuBehaviorTest : BaseUnitTest() { @Throws(Exception::class) fun onUnarchiveHabits() { assertTrue(habit1.isArchived) - whenever(adapter.selected).thenReturn(listOf(habit1)) + whenever(adapter.getSelected()).thenReturn(listOf(habit1)) behavior.onUnarchiveHabits() assertFalse(habit1.isArchived) }