Merge pull request #6438 from grzesiek2010/COLLECT-6174

Address review comments for PR 6118, 6169, 6175
getodk · Nov 21, 2024 · ef74c4d · ef74c4d
2 parents 1950fdf + 091a995
commit ef74c4d
Show file tree

Hide file tree

Showing 14 changed files with 486 additions and 461 deletions.
diff --git a/..._app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt b/..._app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt
@@ -27,7 +27,7 @@ import org.odk.collect.androidshared.ui.PrefUtils
 import org.odk.collect.androidshared.ui.multiclicksafe.MultiClickGuard.allowClick
 import org.odk.collect.async.Scheduler
 import org.odk.collect.maps.MapConfigurator
-import org.odk.collect.maps.layers.OfflineMapLayersPicker
+import org.odk.collect.maps.layers.OfflineMapLayersPickerBottomSheetDialogFragment
 import org.odk.collect.maps.layers.ReferenceLayerRepository
 import org.odk.collect.settings.keys.ProjectKeys
 import org.odk.collect.settings.keys.ProjectKeys.CATEGORY_BASEMAP
@@ -56,8 +56,8 @@ class MapsPreferencesFragment : BaseProjectPreferencesFragment(), Preference.OnP
 
     override fun onCreate(savedInstanceState: Bundle?) {
         childFragmentManager.fragmentFactory = FragmentFactoryBuilder()
-            .forClass(OfflineMapLayersPicker::class) {
-                OfflineMapLayersPicker(requireActivity().activityResultRegistry, referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper)
+            .forClass(OfflineMapLayersPickerBottomSheetDialogFragment::class) {
+                OfflineMapLayersPickerBottomSheetDialogFragment(requireActivity().activityResultRegistry, referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper)
             }
             .build()
 
@@ -90,7 +90,7 @@ class MapsPreferencesFragment : BaseProjectPreferencesFragment(), Preference.OnP
             when (preference.key) {
                 ProjectKeys.KEY_REFERENCE_LAYER -> {
                     DialogFragmentUtils.showIfNotShowing(
-                        OfflineMapLayersPicker::class.java,
+                        OfflineMapLayersPickerBottomSheetDialogFragment::class.java,
                         childFragmentManager
                     )
                 }

diff --git a/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointMapActivity.java b/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointMapActivity.java
@@ -43,7 +43,7 @@
 import org.odk.collect.maps.MapFragment;
 import org.odk.collect.maps.MapFragmentFactory;
 import org.odk.collect.maps.MapPoint;
-import org.odk.collect.maps.layers.OfflineMapLayersPicker;
+import org.odk.collect.maps.layers.OfflineMapLayersPickerBottomSheetDialogFragment;
 import org.odk.collect.maps.layers.ReferenceLayerRepository;
 import org.odk.collect.maps.markers.MarkerDescription;
 import org.odk.collect.maps.markers.MarkerIconDescription;
@@ -140,7 +140,7 @@ public void onCreate(Bundle savedInstanceState) {
         ((GeoDependencyComponentProvider) getApplication()).getGeoDependencyComponent().inject(this);
         getSupportFragmentManager().setFragmentFactory(new FragmentFactoryBuilder()
                 .forClass(MapFragment.class, () -> (Fragment) mapFragmentFactory.createMapFragment())
-                .forClass(OfflineMapLayersPicker.class, () -> new OfflineMapLayersPicker(getActivityResultRegistry(), referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper))
+                .forClass(OfflineMapLayersPickerBottomSheetDialogFragment.class, () -> new OfflineMapLayersPickerBottomSheetDialogFragment(getActivityResultRegistry(), referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper))
                 .build()
         );
         super.onCreate(savedInstanceState);
@@ -242,7 +242,7 @@ public void initMap(MapFragment newMapFragment) {
 
         // Menu Layer Toggle
         findViewById(R.id.layer_menu).setOnClickListener(v -> {
-            DialogFragmentUtils.showIfNotShowing(OfflineMapLayersPicker.class, getSupportFragmentManager());
+            DialogFragmentUtils.showIfNotShowing(OfflineMapLayersPickerBottomSheetDialogFragment.class, getSupportFragmentManager());
         });
 
         clearButton = findViewById(R.id.clear);

diff --git a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyActivity.java b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyActivity.java
@@ -48,7 +48,7 @@
 import org.odk.collect.maps.MapFragment;
 import org.odk.collect.maps.MapFragmentFactory;
 import org.odk.collect.maps.MapPoint;
-import org.odk.collect.maps.layers.OfflineMapLayersPicker;
+import org.odk.collect.maps.layers.OfflineMapLayersPickerBottomSheetDialogFragment;
 import org.odk.collect.maps.layers.ReferenceLayerRepository;
 import org.odk.collect.settings.SettingsProvider;
 import org.odk.collect.strings.localization.LocalizedActivity;
@@ -154,7 +154,7 @@ public void handleOnBackPressed() {
 
         getSupportFragmentManager().setFragmentFactory(new FragmentFactoryBuilder()
                 .forClass(MapFragment.class, () -> (Fragment) mapFragmentFactory.createMapFragment())
-                .forClass(OfflineMapLayersPicker.class, () -> new OfflineMapLayersPicker(getActivityResultRegistry(), referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper))
+                .forClass(OfflineMapLayersPickerBottomSheetDialogFragment.class, () -> new OfflineMapLayersPickerBottomSheetDialogFragment(getActivityResultRegistry(), referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper))
                 .build()
         );
 
@@ -266,7 +266,7 @@ public void initMap(MapFragment newMapFragment) {
         recordButton.setOnClickListener(v -> recordPoint(map.getGpsLocation()));
 
         findViewById(R.id.layers).setOnClickListener(v -> {
-            DialogFragmentUtils.showIfNotShowing(OfflineMapLayersPicker.class, getSupportFragmentManager());
+            DialogFragmentUtils.showIfNotShowing(OfflineMapLayersPickerBottomSheetDialogFragment.class, getSupportFragmentManager());
         });
 
         zoomButton = findViewById(R.id.zoom);

diff --git a/geo/src/main/java/org/odk/collect/geo/selection/SelectionMapFragment.kt b/geo/src/main/java/org/odk/collect/geo/selection/SelectionMapFragment.kt
@@ -30,7 +30,7 @@ import org.odk.collect.maps.MapFragment
 import org.odk.collect.maps.MapFragmentFactory
 import org.odk.collect.maps.MapPoint
 import org.odk.collect.maps.PolygonDescription
-import org.odk.collect.maps.layers.OfflineMapLayersPicker
+import org.odk.collect.maps.layers.OfflineMapLayersPickerBottomSheetDialogFragment
 import org.odk.collect.maps.layers.ReferenceLayerRepository
 import org.odk.collect.maps.markers.MarkerDescription
 import org.odk.collect.maps.markers.MarkerIconDescription
@@ -96,8 +96,8 @@ class SelectionMapFragment(
             .forClass(MapFragment::class.java) {
                 mapFragmentFactory.createMapFragment() as Fragment
             }
-            .forClass(OfflineMapLayersPicker::class) {
-                OfflineMapLayersPicker(requireActivity().activityResultRegistry, referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper)
+            .forClass(OfflineMapLayersPickerBottomSheetDialogFragment::class) {
+                OfflineMapLayersPickerBottomSheetDialogFragment(requireActivity().activityResultRegistry, referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper)
             }
             .build()
 
@@ -198,7 +198,7 @@ class SelectionMapFragment(
 
         binding.layerMenu.setMultiClickSafeOnClickListener {
             DialogFragmentUtils.showIfNotShowing(
-                OfflineMapLayersPicker::class.java,
+                OfflineMapLayersPickerBottomSheetDialogFragment::class.java,
                 childFragmentManager
             )
         }

diff --git a/geo/src/test/java/org/odk/collect/geo/selection/SelectionMapFragmentTest.kt b/geo/src/test/java/org/odk/collect/geo/selection/SelectionMapFragmentTest.kt
@@ -46,7 +46,7 @@ import org.odk.collect.geo.support.RobolectricApplication
 import org.odk.collect.maps.MapFragment
 import org.odk.collect.maps.MapFragmentFactory
 import org.odk.collect.maps.MapPoint
-import org.odk.collect.maps.layers.OfflineMapLayersPicker
+import org.odk.collect.maps.layers.OfflineMapLayersPickerBottomSheetDialogFragment
 import org.odk.collect.maps.layers.ReferenceLayerRepository
 import org.odk.collect.material.BottomSheetBehavior
 import org.odk.collect.material.MaterialProgressDialogFragment
@@ -428,8 +428,8 @@ class SelectionMapFragmentTest {
         onView(withId(R.id.layer_menu)).perform(click())
         scenario.onFragment {
             assertThat(
-                it.childFragmentManager.findFragmentByTag(OfflineMapLayersPicker::class.java.name),
-                instanceOf(OfflineMapLayersPicker::class.java)
+                it.childFragmentManager.findFragmentByTag(OfflineMapLayersPickerBottomSheetDialogFragment::class.java.name),
+                instanceOf(OfflineMapLayersPickerBottomSheetDialogFragment::class.java)
             )
         }
     }

diff --git a/...t/maps/layers/OfflineMapLayersImporter.kt → ...OfflineMapLayersImporterDialogFragment.kt b/...t/maps/layers/OfflineMapLayersImporter.kt → ...OfflineMapLayersImporterDialogFragment.kt
@@ -17,7 +17,7 @@ import org.odk.collect.strings.R
 import org.odk.collect.strings.localization.getLocalizedQuantityString
 import org.odk.collect.strings.localization.getLocalizedString
 
-class OfflineMapLayersImporter(
+class OfflineMapLayersImporterDialogFragment(
     private val referenceLayerRepository: ReferenceLayerRepository,
     private val scheduler: Scheduler,
     private val settingsProvider: SettingsProvider

diff --git a/...ect/maps/layers/OfflineMapLayersPicker.kt → ...pLayersPickerBottomSheetDialogFragment.kt b/...ect/maps/layers/OfflineMapLayersPicker.kt → ...pLayersPickerBottomSheetDialogFragment.kt
@@ -31,7 +31,7 @@ import org.odk.collect.settings.keys.ProjectKeys
 import org.odk.collect.strings.localization.getLocalizedString
 import org.odk.collect.webpage.ExternalWebPageHelper
 
-class OfflineMapLayersPicker(
+class OfflineMapLayersPickerBottomSheetDialogFragment(
     registry: ActivityResultRegistry,
     private val referenceLayerRepository: ReferenceLayerRepository,
     private val scheduler: Scheduler,
@@ -74,7 +74,7 @@ class OfflineMapLayersPicker(
         if (uris.isNotEmpty()) {
             sharedViewModel.loadLayersToImport(uris, requireContext())
             DialogFragmentUtils.showIfNotShowing(
-                OfflineMapLayersImporter::class.java,
+                OfflineMapLayersImporterDialogFragment::class.java,
                 childFragmentManager
             )
         }
@@ -87,8 +87,8 @@ class OfflineMapLayersPicker(
 
     override fun onCreate(savedInstanceState: Bundle?) {
         childFragmentManager.fragmentFactory = FragmentFactoryBuilder()
-            .forClass(OfflineMapLayersImporter::class) {
-                OfflineMapLayersImporter(referenceLayerRepository, scheduler, settingsProvider)
+            .forClass(OfflineMapLayersImporterDialogFragment::class) {
+                OfflineMapLayersImporterDialogFragment(referenceLayerRepository, scheduler, settingsProvider)
             }
             .build()
 
@@ -124,7 +124,7 @@ class OfflineMapLayersPicker(
 
         if (sharedViewModel.layersToImport.value?.value == null) {
             DialogFragmentUtils.dismissDialog(
-                OfflineMapLayersImporter::class.java,
+                OfflineMapLayersImporterDialogFragment::class.java,
                 childFragmentManager
             )
         }

diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt
@@ -34,42 +34,44 @@ class OfflineMapLayersViewModel(
     private lateinit var tempLayersDir: File
 
     fun loadExistingLayers() {
-        trackableWorker.immediate(
-            background = {
-                val layers = referenceLayerRepository.getAll().sortedBy { it.name }
-                _existingLayers.postValue(layers)
-            }
-        )
+        trackableWorker.immediate {
+            val layers = referenceLayerRepository.getAll().sortedBy { it.name }
+            _existingLayers.postValue(layers)
+        }
     }
 
     fun loadLayersToImport(uris: List<Uri>, context: Context) {
-        trackableWorker.immediate(
-            background = {
-                tempLayersDir = TempFiles.createTempDir().also {
-                    it.deleteOnExit()
-                }
-                val layers = mutableListOf<ReferenceLayer>()
-                uris.forEach { uri ->
-                    if (uri.getFileExtension(context) == MbtilesFile.FILE_EXTENSION) {
-                        uri.getFileName(context)?.let { fileName ->
-                            val layerFile = File(tempLayersDir, fileName).also { file ->
-                                uri.copyToFile(context, file)
-                            }
-                            layers.add(ReferenceLayer(layerFile.absolutePath, layerFile, MbtilesFile.readName(layerFile) ?: layerFile.name))
+        trackableWorker.immediate {
+            tempLayersDir = TempFiles.createTempDir().also {
+                it.deleteOnExit()
+            }
+            val layers = mutableListOf<ReferenceLayer>()
+            uris.forEach { uri ->
+                if (uri.getFileExtension(context) == MbtilesFile.FILE_EXTENSION) {
+                    uri.getFileName(context)?.let { fileName ->
+                        val layerFile = File(tempLayersDir, fileName).also { file ->
+                            uri.copyToFile(context, file)
                         }
+                        layers.add(
+                            ReferenceLayer(
+                                layerFile.absolutePath,
+                                layerFile,
+                                MbtilesFile.readName(layerFile) ?: layerFile.name
+                            )
+                        )
                     }
                 }
-                _layersToImport.postValue(
-                    Consumable(
-                        LayersToImport(
-                            uris.size,
-                            uris.size - layers.size,
-                            layers.sortedBy { it.name }
-                        )
+            }
+            _layersToImport.postValue(
+                Consumable(
+                    LayersToImport(
+                        uris.size,
+                        uris.size - layers.size,
+                        layers.sortedBy { it.name }
                     )
                 )
-            }
-        )
+            )
+        }
     }
 
     fun importNewLayers(shared: Boolean) {

diff --git a/maps/src/test/java/org/odk/collect/maps/layers/InMemReferenceLayerRepository.kt b/maps/src/test/java/org/odk/collect/maps/layers/InMemReferenceLayerRepository.kt
@@ -0,0 +1,29 @@
+package org.odk.collect.maps.layers
+
+import java.io.File
+
+class InMemReferenceLayerRepository : ReferenceLayerRepository {
+    val sharedLayers = mutableListOf<ReferenceLayer>()
+    val projectLayers = mutableListOf<ReferenceLayer>()
+
+    override fun getAll(): List<ReferenceLayer> {
+        return sharedLayers + projectLayers
+    }
+
+    override fun get(id: String): ReferenceLayer? {
+        return sharedLayers.find { it.id == id } ?: projectLayers.find { it.id == id }
+    }
+
+    override fun addLayer(file: File, shared: Boolean) {
+        if (shared) {
+            sharedLayers.add(ReferenceLayer(file.absolutePath, file, file.name))
+        } else {
+            projectLayers.add(ReferenceLayer(file.absolutePath, file, file.name))
+        }
+    }
+
+    override fun delete(id: String) {
+        sharedLayers.removeIf { it.id == id }
+        projectLayers.removeIf { it.id == id }
+    }
+}