RemoteFileBrowser: do sorting in ViewModel, not activity

Signed-off-by: Álvaro Brey <alvaro.brey@nextcloud.com>

app/src/main/java/com/nextcloud/talk/remotefilebrowser/activities/RemoteFileBrowserActivity.kt

@@ -45,10 +45,7 @@ import com.nextcloud.talk.remotefilebrowser.viewmodels.RemoteFileBrowserItemsVie
 import com.nextcloud.talk.ui.dialog.SortingOrderDialogFragment
 import com.nextcloud.talk.utils.DisplayUtils
 import com.nextcloud.talk.utils.FileSortOrder
-import com.nextcloud.talk.utils.FileSortOrderNew
 import com.nextcloud.talk.utils.database.user.UserUtils
-import com.nextcloud.talk.utils.preferences.AppPreferences
-import net.orange_box.storebox.listeners.OnPreferenceValueChangedListener
 import java.io.File
 import java.util.Collections
 import java.util.TreeSet
@@ -60,9 +57,7 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe
     @Inject
     lateinit var viewModelFactory: ViewModelProvider.Factory
 
-    @Inject
-    lateinit var appPreferences: AppPreferences
-
+    // TODO use CurrentUserProvider instead for narrower scope
     @Inject
     lateinit var userUtils: UserUtils
 
@@ -74,11 +69,6 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe
     private val selectedPaths: MutableSet<String> = Collections.synchronizedSet(TreeSet())
     private var currentPath: String = "/"
 
-    private var browserItems: List<RemoteFileBrowserItem> = emptyList()
-    private var adapter: RemoteFileBrowserItemsAdapter? = null
-
-    private var sortingChangeListener: OnPreferenceValueChangedListener<String>? = null
-
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         NextcloudTalkApplication.sharedApplication!!.componentApplication.inject(this)
@@ -107,11 +97,8 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe
         binding.swipeRefreshList.setColorSchemeResources(R.color.colorPrimary)
         binding.swipeRefreshList.setProgressBackgroundColorSchemeResource(R.color.refresh_spinner_background)
 
-        appPreferences.registerSortingChangeListener(
-            SortingChangeListener(this).also {
-                sortingChangeListener = it
-            }
-        )
+        binding.pathNavigationBackButton.setOnClickListener { goBack() }
+        binding.sortButton.setOnClickListener { changeSorting() }
 
         viewModel.loadItems(currentPath)
     }
@@ -125,11 +112,9 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe
                 is RemoteFileBrowserItemsViewModel.LoadingItemsState, RemoteFileBrowserItemsViewModel.InitialState -> {
                     showLoading()
                 }
-
                 is RemoteFileBrowserItemsViewModel.NoRemoteFileItemsState -> {
                     showEmpty()
                 }
-
                 is RemoteFileBrowserItemsViewModel.LoadedState -> {
                     val remoteFileBrowserItems = state.items
                     Log.d(TAG, "Items received: $remoteFileBrowserItems")
@@ -144,6 +129,7 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe
 
                     // TODO make mimeTypeSelectionFilter a bundled arg for the activity
                     val mimeTypeSelectionFilter = "image/"
+                    // TODO do not needlesly recreate adapter if it can be reused
                     val adapter = RemoteFileBrowserItemsAdapter(
                         showGrid = showGrid,
                         mimeTypeSelectionFilter = mimeTypeSelectionFilter,
@@ -158,7 +144,6 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe
                             } else {
                                 ArrayList()
                             }
-                            browserItems = items
                         }
 
                     binding.recyclerView.adapter = adapter
@@ -169,6 +154,12 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe
                 }
             }
         }
+
+        viewModel.fileSortOrder.observe(this) { sortOrder ->
+            if (sortOrder != null) {
+                binding.sortButton.setText(DisplayUtils.getSortOrderStringId(sortOrder))
+            }
+        }
     }
 
     override fun onCreateOptionsMenu(menu: Menu?): Boolean {
@@ -190,20 +181,12 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe
 
     override fun onResume() {
         super.onResume()
-
-        binding.pathNavigationBackButton.setOnClickListener { goBack() }
-        binding.sortButton.setOnClickListener { changeSorting() }
-
-        binding.sortButton.setText(
-            DisplayUtils.getSortOrderStringId(FileSortOrder.getFileSortOrder(appPreferences.sorting))
-        )
-
         refreshCurrentPath()
     }
 
-    fun changeSorting() {
+    private fun changeSorting() {
         val newFragment: DialogFragment = SortingOrderDialogFragment
-            .newInstance(FileSortOrder.getFileSortOrder(appPreferences.sorting))
+            .newInstance(FileSortOrder.getFileSortOrder(viewModel.fileSortOrder.value!!.name))
         newFragment.show(
             supportFragmentManager,
             SortingOrderDialogFragment.SORTING_ORDER_FRAGMENT
@@ -321,25 +304,4 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe
     override fun shouldOnlySelectOneImageFile(): Boolean {
         return true
     }
-
-    @Suppress("Detekt.TooGenericExceptionCaught")
-    private class SortingChangeListener(private val activity: RemoteFileBrowserActivity) :
-        OnPreferenceValueChangedListener<String> {
-        override fun onChanged(newValue: String) {
-            try {
-                val sortOrder = FileSortOrderNew.getFileSortOrder(newValue)
-
-                activity.binding.sortButton.setText(DisplayUtils.getSortOrderStringId(sortOrder))
-                activity.browserItems = sortOrder.sortCloudFiles(activity.browserItems)
-
-                activity.runOnUiThread {
-                    activity.adapter!!.updateDataSet(activity.browserItems)
-                }
-            } catch (npe: NullPointerException) {
-                // view binding can be null
-                // since this is called asynchronously and UI might have been destroyed in the meantime
-                Log.i(TAG, "UI destroyed - view binding already gone")
-            }
-        }
-    }
 }

app/src/main/java/com/nextcloud/talk/remotefilebrowser/adapters/RemoteFileBrowserItemsListViewHolder.kt

@@ -21,7 +21,6 @@
 package com.nextcloud.talk.remotefilebrowser.adapters
 
 import android.text.format.Formatter
-import android.util.Log
 import android.view.View
 import androidx.appcompat.content.res.AppCompatResources
 import autodagger.AutoInjector
@@ -51,8 +50,8 @@ class RemoteFileBrowserItemsListViewHolder(
     override val fileIcon: SimpleDraweeView
         get() = binding.fileIcon
 
-    private var selectable : Boolean = true
-    private var clickable : Boolean = true
+    private var selectable: Boolean = true
+    private var clickable: Boolean = true
 
     init {
         itemView.setOnClickListener {

app/src/main/java/com/nextcloud/talk/remotefilebrowser/viewmodels/RemoteFileBrowserItemsViewModel.kt

@@ -26,14 +26,18 @@ import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.ViewModel
 import com.nextcloud.talk.remotefilebrowser.model.RemoteFileBrowserItem
 import com.nextcloud.talk.remotefilebrowser.repositories.RemoteFileBrowserItemsRepository
+import com.nextcloud.talk.utils.FileSortOrderNew
+import com.nextcloud.talk.utils.preferences.AppPreferences
 import io.reactivex.Observer
 import io.reactivex.android.schedulers.AndroidSchedulers
 import io.reactivex.disposables.Disposable
 import io.reactivex.schedulers.Schedulers
+import net.orange_box.storebox.listeners.OnPreferenceValueChangedListener
 import javax.inject.Inject
 
 class RemoteFileBrowserItemsViewModel @Inject constructor(
-    private val repository: RemoteFileBrowserItemsRepository
+    private val repository: RemoteFileBrowserItemsRepository,
+    private val appPreferences: AppPreferences
 ) :
     ViewModel() {
 
@@ -43,11 +47,33 @@ class RemoteFileBrowserItemsViewModel @Inject constructor(
     object LoadingItemsState : ViewState
     class LoadedState(val items: List<RemoteFileBrowserItem>) : ViewState
 
-    private val _viewState: MutableLiveData<ViewState> = MutableLiveData(InitialState)
+    private val initialSortOrder = FileSortOrderNew.getFileSortOrder(appPreferences.sorting)
+    private val sortingPrefListener: SortChangeListener = SortChangeListener()
 
+    private val _viewState: MutableLiveData<ViewState> = MutableLiveData(InitialState)
     val viewState: LiveData<ViewState>
         get() = _viewState
 
+    // TODO incorporate into view state object?
+    private val _fileSortOrder: MutableLiveData<FileSortOrderNew> = MutableLiveData(initialSortOrder)
+    val fileSortOrder: LiveData<FileSortOrderNew>
+        get() = _fileSortOrder
+
+    init {
+        appPreferences.registerSortingChangeListener(sortingPrefListener)
+    }
+
+    inner class SortChangeListener : OnPreferenceValueChangedListener<String> {
+        override fun onChanged(newValue: String) {
+            onSelectSortOrder(newValue)
+        }
+    }
+
+    override fun onCleared() {
+        super.onCleared()
+        appPreferences.unregisterSortingChangeListener(sortingPrefListener)
+    }
+
     fun loadItems(path: String) {
         _viewState.value = LoadingItemsState
         repository.listFolder(path).subscribeOn(Schedulers.io())
@@ -62,7 +88,7 @@ class RemoteFileBrowserItemsViewModel @Inject constructor(
         override fun onSubscribe(d: Disposable) = Unit
 
         override fun onNext(response: List<RemoteFileBrowserItem>) {
-            newRemoteFileBrowserItems = response
+            newRemoteFileBrowserItems = fileSortOrder.value!!.sortCloudFiles(response)
         }
 
         override fun onError(e: Throwable) {
@@ -87,6 +113,18 @@ class RemoteFileBrowserItemsViewModel @Inject constructor(
         }
     }
 
+    private fun onSelectSortOrder(newSortOrderString: String) {
+        val newSortOrder = FileSortOrderNew.getFileSortOrder(newSortOrderString)
+        if (newSortOrder.name != fileSortOrder.value?.name) {
+            _fileSortOrder.value = newSortOrder
+            val currentState = viewState.value
+            if (currentState is LoadedState) {
+                val sortedItems = newSortOrder.sortCloudFiles(currentState.items)
+                _viewState.value = LoadedState(sortedItems)
+            }
+        }
+    }
+
     companion object {
         private val TAG = RemoteFileBrowserItemsViewModel::class.simpleName
     }