Fix bug with permission checking

Fix a bug with the permission checking where the MusicRepository would return NO_PERMS to the LoadingFragment faster than the creation of the view, causing the error message to never display. Permission checks are now handled by LoadingFragment instead.

app/src/main/java/org/oxycblt/auxio/loading/LoadingFragment.kt

@@ -1,6 +1,7 @@
 package org.oxycblt.auxio.loading
 
 import android.Manifest
+import android.content.pm.PackageManager
 import android.os.Bundle
 import android.util.Log
 import android.view.LayoutInflater
@@ -8,6 +9,7 @@ import android.view.View
 import android.view.ViewGroup
 import androidx.activity.result.ActivityResultLauncher
 import androidx.activity.result.contract.ActivityResultContracts
+import androidx.core.content.ContextCompat
 import androidx.databinding.DataBindingUtil
 import androidx.fragment.app.Fragment
 import androidx.lifecycle.ViewModelProvider
@@ -16,7 +18,7 @@ import org.oxycblt.auxio.R
 import org.oxycblt.auxio.databinding.FragmentLoadingBinding
 import org.oxycblt.auxio.music.processing.MusicLoaderResponse
 
-class LoadingFragment : Fragment() {
+class LoadingFragment : Fragment(R.layout.fragment_loading) {
 
     private val loadingModel: LoadingViewModel by lazy {
         ViewModelProvider(
@@ -27,6 +29,8 @@ class LoadingFragment : Fragment() {
         ).get(LoadingViewModel::class.java)
     }
 
+    private var noPerms = false
+
     private lateinit var binding: FragmentLoadingBinding
     private lateinit var permLauncher: ActivityResultLauncher<String>
 
@@ -69,72 +73,81 @@ class LoadingFragment : Fragment() {
 
                 // If its actually granted, restart the loading process again.
                 if (granted) {
-                    binding.loadingBar.visibility = View.VISIBLE
+                    wipeViews()
-                    binding.errorText.visibility = View.GONE
+
-                    binding.statusIcon.visibility = View.GONE
+                    noPerms = false
-                    binding.retryButton.visibility = View.GONE
-                    binding.grantButton.visibility = View.GONE
 
                     loadingModel.retry()
                 }
             }
 
-        // This never seems to return true but Im apparently supposed to use it so
+        // Force an error screen if the permissions are denied or the prompt needs to be shown.
-        if (shouldShowRequestPermissionRationale(Manifest.permission.READ_EXTERNAL_STORAGE)) {
+        // This should be in MusicRepository, but the response comes faster than the view creation
-            onMusicLoadResponse(MusicLoaderResponse.NO_PERMS)
+        // itself and therefore causes the error screen to not appear.
+        if (checkPerms()) {
+            noPerms = true
+            onNoPerms()
         } else {
             loadingModel.go()
         }
-
         Log.d(this::class.simpleName, "Fragment created.")
 
         return binding.root
     }
 
+    // Check for permissions. God help us all.
+    private fun checkPerms(): Boolean {
+        return shouldShowRequestPermissionRationale(
+            Manifest.permission.READ_EXTERNAL_STORAGE
+        ) || ContextCompat.checkSelfPermission(
+            requireContext(), Manifest.permission.READ_EXTERNAL_STORAGE
+        ) == PackageManager.PERMISSION_DENIED || noPerms
+    }
+
     private fun onMusicLoadResponse(repoResponse: MusicLoaderResponse?) {
         // Don't run this if the value is null, Which is what the value changes to after
         // this is run.
         repoResponse?.let { response ->
+
             if (response == MusicLoaderResponse.DONE) {
                 this.findNavController().navigate(
                     LoadingFragmentDirections.actionToMain()
                 )
             } else {
                 // If the response wasn't a success, then show the specific error message
-                // depending on which error response was given, along with a retry or grant button
+                // depending on which error response was given, along with a retry button
                 binding.loadingBar.visibility = View.GONE
                 binding.errorText.visibility = View.VISIBLE
                 binding.statusIcon.visibility = View.VISIBLE
+                binding.retryButton.visibility = View.VISIBLE
 
-                when (response) {
+                binding.errorText.text =
-                    MusicLoaderResponse.NO_PERMS -> {
+                    if (response == MusicLoaderResponse.NO_MUSIC)
-                        binding.grantButton.visibility = View.VISIBLE
+                        getString(R.string.error_no_music)
-                        binding.errorText.text = getString(R.string.error_no_perms)
+                    else
-                    }
+                        getString(R.string.error_music_load_failed)
-
-                    MusicLoaderResponse.NO_MUSIC -> {
-                        binding.retryButton.visibility = View.VISIBLE
-                        binding.errorText.text = getString(R.string.error_no_music)
-                    }
-
-                    else -> {
-                        binding.retryButton.visibility = View.VISIBLE
-                        binding.errorText.text = getString(R.string.error_music_load_failed)
-                    }
-                }
             }
 
             loadingModel.doneWithResponse()
         }
     }
 
+    private fun onNoPerms() {
+        // If there are no perms, switch out the view elements as if an error screen was being
+        // shown, but show the label that Auxio needs to read external storage to function,
+        // along with a GRANT button
+
+        binding.loadingBar.visibility = View.GONE
+        binding.errorText.visibility = View.VISIBLE
+        binding.statusIcon.visibility = View.VISIBLE
+        binding.grantButton.visibility = View.VISIBLE
+
+        binding.errorText.text = getString(R.string.error_no_perms)
+    }
+
     private fun onRetry(retry: Boolean) {
         if (retry) {
-            binding.loadingBar.visibility = View.VISIBLE
+            wipeViews()
-            binding.errorText.visibility = View.GONE
-            binding.statusIcon.visibility = View.GONE
-            binding.retryButton.visibility = View.GONE
-            binding.grantButton.visibility = View.GONE
 
             loadingModel.doneWithRetry()
         }
@@ -147,4 +160,13 @@ class LoadingFragment : Fragment() {
             loadingModel.doneWithGrant()
         }
     }
+
+    // Wipe views and switch back to the plain LoadingBar
+    private fun wipeViews() {
+        binding.loadingBar.visibility = View.VISIBLE
+        binding.errorText.visibility = View.GONE
+        binding.statusIcon.visibility = View.GONE
+        binding.retryButton.visibility = View.GONE
+        binding.grantButton.visibility = View.GONE
+    }
 }

app/src/main/java/org/oxycblt/auxio/loading/LoadingViewModel.kt

@@ -34,6 +34,8 @@ class LoadingViewModel(private val app: Application) : ViewModel() {
     // Start the music loading. It has already been called, one needs to call retry() instead.
     fun go() {
         if (!started) {
+            started = true
+
             startMusicRepo()
         }
     }
@@ -49,8 +51,6 @@ class LoadingViewModel(private val app: Application) : ViewModel() {
             // Then actually notify listeners of the response in the Main thread
             withContext(Dispatchers.Main) {
                 mMusicRepoResponse.value = response
-
-                started = true
             }
         }
     }

app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt

@@ -1,10 +1,7 @@
 package org.oxycblt.auxio.music
 
-import android.Manifest
 import android.app.Application
-import android.content.pm.PackageManager
 import android.util.Log
-import androidx.core.content.ContextCompat
 import org.oxycblt.auxio.R
 import org.oxycblt.auxio.music.models.Album
 import org.oxycblt.auxio.music.models.Artist
@@ -23,12 +20,6 @@ class MusicRepository {
     lateinit var songs: List<Song>
 
     fun init(app: Application): MusicLoaderResponse {
-        if (!checkPerms(app)) {
-            Log.i(this::class.simpleName, "No permissions, aborting...")
-
-            return MusicLoaderResponse.NO_PERMS
-        }
-
         Log.i(this::class.simpleName, "Starting initial music load...")
 
         val start = System.currentTimeMillis()
@@ -64,12 +55,6 @@ class MusicRepository {
         return loader.response
     }
 
-    private fun checkPerms(app: Application): Boolean {
-        return ContextCompat.checkSelfPermission(
-            app.applicationContext, Manifest.permission.READ_EXTERNAL_STORAGE
-        ) == PackageManager.PERMISSION_GRANTED
-    }
-
     companion object {
         @Volatile
         private var INSTANCE: MusicRepository? = null

app/src/main/java/org/oxycblt/auxio/music/processing/MusicLoader.kt

@@ -15,7 +15,7 @@ import org.oxycblt.auxio.music.toAlbumArtURI
 import org.oxycblt.auxio.music.toNamedGenre
 
 enum class MusicLoaderResponse {
-    DONE, FAILURE, NO_MUSIC, NO_PERMS
+    DONE, FAILURE, NO_MUSIC
 }
 
 // Class that loads music from the FileSystem.

app/src/main/res/drawable/ripple.xml

@@ -1,10 +1,10 @@
 <?xml version="1.0" encoding="utf-8"?>
 <ripple xmlns:android="http://schemas.android.com/apk/res/android"
-    android:color="?attr/colorPrimary">
+    android:color="@color/selection_color">
     <!-- TODO: Find better color to override -->
     <item android:id="@android:id/mask">
         <shape android:shape="rectangle">
-            <solid android:color="?attr/colorPrimary" />
+            <solid android:color="@color/selection_color" />
         </shape>
     </item>
 </ripple>

app/src/main/res/layout/fragment_main.xml

@@ -27,7 +27,7 @@
             app:tabIconTint="?android:attr/colorPrimary"
             app:tabIconTintMode="src_in"
             app:tabIndicator="@drawable/indicator"
-            app:tabRippleColor="?android:attr/colorPrimary" />
+            app:tabRippleColor="@color/selection_color" />
 
     </LinearLayout>
 </layout>