From 79b4dcbaf8e8a1e2aada3f74cc2e69b706aa7b96 Mon Sep 17 00:00:00 2001
From: James O Smith <james.smith@willowtreeapps.com>
Date: Thu, 11 Apr 2024 13:22:20 -0400
Subject: [PATCH] Fix crash in EditCategoriesViewModel

---
 .../settings/EditCategoriesViewModel.kt       | 85 +++++++------------
 .../vocable/FakeCategoriesUseCase.kt          | 11 ++-
 2 files changed, 40 insertions(+), 56 deletions(-)

diff --git a/app/src/main/java/com/willowtree/vocable/settings/EditCategoriesViewModel.kt b/app/src/main/java/com/willowtree/vocable/settings/EditCategoriesViewModel.kt
index aa5e9241..7e0cde26 100644
--- a/app/src/main/java/com/willowtree/vocable/settings/EditCategoriesViewModel.kt
+++ b/app/src/main/java/com/willowtree/vocable/settings/EditCategoriesViewModel.kt
@@ -17,11 +17,7 @@ class EditCategoriesViewModel(
     private val categoriesUseCase: ICategoriesUseCase
 ) : ViewModel() {
 
-    val categoryList: LiveData<List<Category>> = categoriesUseCase.categories()
-        .map { categories ->
-            categories.sortedBy { it.sortOrder }
-        }
-        .asLiveData()
+    val categoryList: LiveData<List<Category>> = categoriesUseCase.categories().asLiveData()
 
     val categoryPages = categoriesUseCase.categories().map { categories ->
         val pageSize = 8
@@ -39,8 +35,6 @@ class EditCategoriesViewModel(
             val oldCategories = overallCategories
 
             overallCategories = categoriesUseCase.categories().first()
-            overallCategories =
-                overallCategories.filter { !it.hidden } + overallCategories.filter { it.hidden }
 
             // Check if a new category was added and scroll to it
             if (oldCategories.isNotEmpty() && oldCategories.size < overallCategories.size) {
@@ -63,63 +57,44 @@ class EditCategoriesViewModel(
 
     fun moveCategoryUp(categoryId: String) {
         viewModelScope.launch {
-            val catIndex = overallCategories.indexOfFirst { it.categoryId == categoryId }
-            if (catIndex > 0) {
-                val previousCat = overallCategories[catIndex - 1]
-
-                overallCategories = overallCategories.map {
-                    when (it.categoryId) {
-                        categoryId -> {
-                            it.withSortOrder(it.sortOrder - 1)
-                        }
-
-                        previousCat.categoryId -> {
-                            it.withSortOrder(it.sortOrder + 1)
-                        }
-
-                        else -> {
-                            it
-                        }
-                    }
-                }
+            val categories = categoriesUseCase.categories().first()
+            val catIndex = categories.indexOfFirst { it.categoryId == categoryId }
+            if (catIndex in 1 until categories.size) {
+                val category = categories[catIndex]
+                val previousCat = categories[catIndex - 1]
 
-                categoriesUseCase.updateCategorySortOrders(
-                    overallCategories.map {
-                        CategorySortOrder(it.categoryId, it.sortOrder)
-                    }
-                )
+                swapSortOrders(categories, previousCat, category)
             }
         }
     }
 
     fun moveCategoryDown(categoryId: String) {
         viewModelScope.launch {
-            val catIndex = overallCategories.indexOfFirst { it.categoryId == categoryId }
-            if (catIndex > -1) {
-                val nextCat = overallCategories[catIndex + 1]
-
-                overallCategories = overallCategories.map {
-                    when (it.categoryId) {
-                        categoryId -> {
-                            it.withSortOrder(it.sortOrder + 1)
-                        }
-
-                        nextCat.categoryId -> {
-                            it.withSortOrder(it.sortOrder - 1)
-                        }
-
-                        else -> {
-                            it
-                        }
-                    }
-                }
+            val categories = categoriesUseCase.categories().first()
+            val catIndex = categories.indexOfFirst { it.categoryId == categoryId }
+            if (catIndex in 0 until categories.size - 1) {
+                val category = categories[catIndex]
+                val nextCat = categories[catIndex + 1]
 
-                categoriesUseCase.updateCategorySortOrders(
-                    overallCategories.map {
-                        CategorySortOrder(it.categoryId, it.sortOrder)
-                    }
-                )
+                swapSortOrders(categories, category, nextCat)
             }
         }
     }
+
+    private suspend fun swapSortOrders(
+        categories: List<Category>,
+        leftCategory: Category,
+        rightCategory: Category
+    ) {
+        categoriesUseCase.updateCategorySortOrders(
+            categories.map {
+                val sortOrder = when (it.categoryId) {
+                    rightCategory.categoryId -> leftCategory.sortOrder
+                    leftCategory.categoryId -> rightCategory.sortOrder
+                    else -> it.sortOrder
+                }
+                CategorySortOrder(it.categoryId, sortOrder)
+            }
+        )
+    }
 }
\ No newline at end of file
diff --git a/app/src/test/java/com/willowtree/vocable/FakeCategoriesUseCase.kt b/app/src/test/java/com/willowtree/vocable/FakeCategoriesUseCase.kt
index d3d501ab..4f5a607a 100644
--- a/app/src/test/java/com/willowtree/vocable/FakeCategoriesUseCase.kt
+++ b/app/src/test/java/com/willowtree/vocable/FakeCategoriesUseCase.kt
@@ -5,6 +5,7 @@ import com.willowtree.vocable.room.CategorySortOrder
 import com.willowtree.vocable.utils.locale.LocalesWithText
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.update
 
 @Deprecated("This fake is too complex, tests using it should migrate to integration tests with" +
@@ -23,7 +24,15 @@ class FakeCategoriesUseCase : ICategoriesUseCase {
     )
 
     override fun categories(): Flow<List<Category>> {
-        return _categories
+        return _categories.map {
+            it.sortedBy { category ->
+                if (category.hidden) {
+                    Int.MAX_VALUE
+                } else {
+                    category.sortOrder
+                }
+            }
+        }
     }
 
     override suspend fun updateCategoryName(