From 813b9b72b77444b79558f24b7549a61d029a190b Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Fri, 24 May 2024 14:32:26 +0200 Subject: [PATCH 01/28] Added a button for importing layers --- .../maps/layers/OfflineMapLayersPicker.kt | 8 ++++++++ .../res/layout/offline_map_layers_picker.xml | 16 ++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index 00a177f1e28..5efe8b1a2c2 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -5,6 +5,7 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.activity.result.contract.ActivityResultContracts import androidx.fragment.app.viewModels import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider @@ -32,6 +33,9 @@ class OfflineMapLayersPicker( private lateinit var offlineMapLayersPickerBinding: OfflineMapLayersPickerBinding + private val getLayers = registerForActivityResult(ActivityResultContracts.GetMultipleContents()) { uris -> + } + override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -57,6 +61,10 @@ class OfflineMapLayersPicker( ) } + offlineMapLayersPickerBinding.addLayer.setOnClickListener { + getLayers.launch("*/*") + } + offlineMapLayersPickerBinding.cancel.setOnClickListener { dismiss() } diff --git a/maps/src/main/res/layout/offline_map_layers_picker.xml b/maps/src/main/res/layout/offline_map_layers_picker.xml index 1d8d46d4fca..b6c5d778549 100644 --- a/maps/src/main/res/layout/offline_map_layers_picker.xml +++ b/maps/src/main/res/layout/offline_map_layers_picker.xml @@ -94,7 +94,7 @@ app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager" app:layout_constraintStart_toStartOf="parent" app:layout_constraintEnd_toEndOf="parent" - app:layout_constraintBottom_toTopOf="@id/bottom_divider" + app:layout_constraintBottom_toTopOf="@id/add_layer" app:layout_constraintHeight_default="wrap" app:layout_constraintTop_toBottomOf="@id/top_divider" tools:visibility="visible" /> @@ -105,11 +105,23 @@ android:layout_height="wrap_content" android:layout_marginVertical="@dimen/margin_small" android:indeterminate="true" - app:layout_constraintBottom_toTopOf="@id/bottom_divider" + app:layout_constraintBottom_toTopOf="@id/add_layer" app:layout_constraintEnd_toEndOf="@id/message" app:layout_constraintStart_toStartOf="@id/message" app:layout_constraintTop_toBottomOf="@id/top_divider" /> + + Date: Sat, 25 May 2024 00:53:25 +0200 Subject: [PATCH 02/28] Display the confirmation dialog --- .../collect/androidshared/system/UriExt.kt | 38 ++++ .../res/drawable/ic_baseline_layers_24.xml | 13 ++ maps/build.gradle.kts | 1 + .../DirectoryReferenceLayerRepository.kt | 8 + .../layers/OfflineMapLayersImportAdapter.kt | 24 +++ .../layers/OfflineMapLayersImportDialog.kt | 98 ++++++++++ .../OfflineMapLayersImportDialogViewModel.kt | 63 +++++++ .../maps/layers/OfflineMapLayersPicker.kt | 34 ++++ .../layers/OfflineMapLayersPickerViewModel.kt | 24 +-- .../maps/layers/ReferenceLayerRepository.kt | 3 + .../offline_map_layers_import_dialog.xml | 167 ++++++++++++++++++ .../layout/offline_map_layers_import_item.xml | 26 +++ 12 files changed, 489 insertions(+), 10 deletions(-) create mode 100644 androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt create mode 100644 icons/src/main/res/drawable/ic_baseline_layers_24.xml create mode 100644 maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportAdapter.kt create mode 100644 maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialog.kt create mode 100644 maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt create mode 100644 maps/src/main/res/layout/offline_map_layers_import_dialog.xml create mode 100644 maps/src/main/res/layout/offline_map_layers_import_item.xml diff --git a/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt b/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt new file mode 100644 index 00000000000..99cc1709c57 --- /dev/null +++ b/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt @@ -0,0 +1,38 @@ +package org.odk.collect.androidshared.system + +import android.content.ContentResolver +import android.net.Uri +import android.provider.OpenableColumns +import timber.log.Timber +import java.io.File +import java.io.FileOutputStream +import java.io.IOException + +fun Uri.toFile(contentResolver: ContentResolver, dest: File) { + try { + contentResolver.openInputStream(this)?.use { inputStream -> + FileOutputStream(dest).use { outputStream -> + inputStream.copyTo(outputStream) + } + } + } catch (e: IOException) { + Timber.e(e) + } +} + +fun Uri.getFileName(contentResolver: ContentResolver): String? { + var fileName: String? = null + if (scheme == ContentResolver.SCHEME_CONTENT) { + val cursor = contentResolver.query(this, null, null, null, null) + cursor.use { + if (it != null && it.moveToFirst()) { + val fileNameColumnIndex = it.getColumnIndex(OpenableColumns.DISPLAY_NAME) + fileName = it.getString(fileNameColumnIndex) + } + } + } + if (fileName == null) { + fileName = path?.substringAfterLast("/") + } + return fileName +} diff --git a/icons/src/main/res/drawable/ic_baseline_layers_24.xml b/icons/src/main/res/drawable/ic_baseline_layers_24.xml new file mode 100644 index 00000000000..2a93aa2f714 --- /dev/null +++ b/icons/src/main/res/drawable/ic_baseline_layers_24.xml @@ -0,0 +1,13 @@ + + + + diff --git a/maps/build.gradle.kts b/maps/build.gradle.kts index 1807f5b2a85..66c2fd3597b 100644 --- a/maps/build.gradle.kts +++ b/maps/build.gradle.kts @@ -52,6 +52,7 @@ dependencies { implementation(project(":shared")) implementation(project(":androidshared")) implementation(project(":icons")) + implementation(project(":material")) implementation(project(":settings")) implementation(project(":strings")) implementation(project(":web-page")) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt b/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt index 4f3158c520f..101e0aabb5d 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt @@ -27,6 +27,14 @@ class DirectoryReferenceLayerRepository( } } + override fun getSharedLayersDirPath(): String { + return directoryPaths[1] + } + + override fun getProjectLayersDirPath(): String { + return directoryPaths[0] + } + private fun getAllFilesWithDirectory() = directoryPaths.flatMap { dir -> listFilesRecursively(File(dir)).map { file -> Pair(file, dir) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportAdapter.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportAdapter.kt new file mode 100644 index 00000000000..154a7940432 --- /dev/null +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportAdapter.kt @@ -0,0 +1,24 @@ +package org.odk.collect.maps.layers + +import android.view.LayoutInflater +import android.view.ViewGroup +import androidx.recyclerview.widget.RecyclerView +import org.odk.collect.maps.databinding.OfflineMapLayersImportItemBinding + +class OfflineMapLayersImportAdapter( + private val layers: List, +) : RecyclerView.Adapter() { + + override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder { + val binding = OfflineMapLayersImportItemBinding.inflate(LayoutInflater.from(parent.context), parent, false) + return ViewHolder(binding) + } + + override fun onBindViewHolder(holder: ViewHolder, position: Int) { + holder.binding.layerName.text = layers[position].name + } + + override fun getItemCount() = layers.size + + class ViewHolder(val binding: OfflineMapLayersImportItemBinding) : RecyclerView.ViewHolder(binding.root) +} diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialog.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialog.kt new file mode 100644 index 00000000000..da7b38dc638 --- /dev/null +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialog.kt @@ -0,0 +1,98 @@ +package org.odk.collect.maps.layers + +import android.os.Bundle +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup +import androidx.appcompat.widget.Toolbar +import androidx.core.os.bundleOf +import androidx.fragment.app.setFragmentResult +import androidx.fragment.app.viewModels +import androidx.lifecycle.ViewModel +import androidx.lifecycle.ViewModelProvider +import org.odk.collect.async.Scheduler +import org.odk.collect.maps.databinding.OfflineMapLayersImportDialogBinding +import org.odk.collect.material.MaterialFullScreenDialogFragment +import org.odk.collect.material.MaterialProgressDialogFragment + +class OfflineMapLayersImportDialog( + private val scheduler: Scheduler, + private val sharedLayersDirPath: String, + private val projectLayersDirPath: String +) : MaterialFullScreenDialogFragment() { + + private val viewModel: OfflineMapLayersImportDialogViewModel by viewModels { + object : ViewModelProvider.Factory { + override fun create(modelClass: Class): T { + return OfflineMapLayersImportDialogViewModel(scheduler, requireContext().contentResolver) as T + } + } + } + + private lateinit var binding: OfflineMapLayersImportDialogBinding + + override fun onCreateView( + inflater: LayoutInflater, + container: ViewGroup?, + savedInstanceState: Bundle? + ): View { + binding = OfflineMapLayersImportDialogBinding.inflate(inflater) + + viewModel.data.observe(this) { data -> + binding.progressIndicator.visibility = View.GONE + binding.layersList.visibility = View.VISIBLE + binding.addLayerButton.isEnabled = true + + val adapter = OfflineMapLayersImportAdapter(data) + binding.layersList.setAdapter(adapter) + } + viewModel.init(requireArguments().getStringArrayList(URIS)) + + binding.cancelButton.setOnClickListener { + dismiss() + } + + binding.addLayerButton.setOnClickListener { + val layersDir = if (binding.allProjectsOption.isChecked) { + sharedLayersDirPath + } else { + projectLayersDirPath + } + + val isLoading = viewModel.addLayers(layersDir) + MaterialProgressDialogFragment.showOn( + this, + isLoading, + childFragmentManager + ) { + MaterialProgressDialogFragment().also { dialog -> + dialog.message = getString(org.odk.collect.strings.R.string.loading) + } + } + + isLoading.observe(this) { + if (!it) { + setFragmentResult(RESULT_KEY, bundleOf()) + dismiss() + } + } + } + return binding.root + } + + override fun onCloseClicked() { + } + + override fun onBackPressed() { + dismiss() + } + + override fun getToolbar(): Toolbar { + return binding.toolbar + } + + companion object { + const val URIS = "uris" + const val RESULT_KEY = "layersAdded" + } +} diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt new file mode 100644 index 00000000000..7fe15d9d5b3 --- /dev/null +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt @@ -0,0 +1,63 @@ +package org.odk.collect.maps.layers + +import android.content.ContentResolver +import android.net.Uri +import androidx.lifecycle.LiveData +import androidx.lifecycle.MutableLiveData +import androidx.lifecycle.ViewModel +import org.odk.collect.androidshared.system.getFileName +import org.odk.collect.androidshared.system.toFile +import org.odk.collect.async.Scheduler +import org.odk.collect.shared.TempFiles +import java.io.File +import java.util.ArrayList + +class OfflineMapLayersImportDialogViewModel( + private val scheduler: Scheduler, + private val contentResolver: ContentResolver +) : ViewModel() { + + private val _data = MutableLiveData>() + val data: LiveData> = _data + + private lateinit var tempLayersDir: File + + fun init(uris: ArrayList?) { + scheduler.immediate( + background = { + tempLayersDir = TempFiles.createTempDir().also { + it.deleteOnExit() + } + val layers = mutableListOf() + uris?.forEach { uriString -> + val uri = Uri.parse(uriString) + uri.getFileName(contentResolver)?.let { fileName -> + val layerFile = File(tempLayersDir, fileName).also { file -> + uri.toFile(contentResolver, file) + } + layers.add(ReferenceLayer(layerFile.absolutePath, layerFile, MbtilesFile.readName(layerFile) ?: layerFile.name)) + } + } + _data.postValue(layers) + }, + foreground = { } + ) + } + + fun addLayers(layersDir: String): LiveData { + val isLoading = MutableLiveData(true) + scheduler.immediate( + background = { + val destDir = File(layersDir) + tempLayersDir.listFiles()?.forEach { + it.copyTo(File(destDir, it.name), true) + } + tempLayersDir.delete() + + isLoading.postValue(false) + }, + foreground = { } + ) + return isLoading + } +} diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index 5efe8b1a2c2..22145feec5c 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -6,11 +6,14 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.activity.result.contract.ActivityResultContracts +import androidx.fragment.app.setFragmentResultListener import androidx.fragment.app.viewModels import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import com.google.android.material.bottomsheet.BottomSheetBehavior import com.google.android.material.bottomsheet.BottomSheetDialogFragment +import org.odk.collect.androidshared.ui.DialogFragmentUtils +import org.odk.collect.androidshared.ui.FragmentFactoryBuilder import org.odk.collect.androidshared.ui.GroupClickListener.addOnClickListener import org.odk.collect.async.Scheduler import org.odk.collect.maps.databinding.OfflineMapLayersPickerBinding @@ -34,6 +37,36 @@ class OfflineMapLayersPicker( private lateinit var offlineMapLayersPickerBinding: OfflineMapLayersPickerBinding private val getLayers = registerForActivityResult(ActivityResultContracts.GetMultipleContents()) { uris -> + if (uris.isNotEmpty()) { + val uriStrings: MutableList = ArrayList() + for (uri in uris) { + uriStrings.add(uri.toString()) + } + + DialogFragmentUtils.showIfNotShowing( + OfflineMapLayersImportDialog::class.java, + Bundle().apply { putStringArrayList(OfflineMapLayersImportDialog.URIS, ArrayList(uriStrings)) }, + childFragmentManager + ) + } + } + + override fun onCreate(savedInstanceState: Bundle?) { + childFragmentManager.fragmentFactory = FragmentFactoryBuilder() + .forClass(OfflineMapLayersImportDialog::class) { + OfflineMapLayersImportDialog( + scheduler, + referenceLayerRepository.getSharedLayersDirPath(), + referenceLayerRepository.getProjectLayersDirPath() + ) + } + .build() + + super.onCreate(savedInstanceState) + + childFragmentManager.setFragmentResultListener(OfflineMapLayersImportDialog.RESULT_KEY, this) { _, _ -> + viewModel.refreshLayers() + } } override fun onCreateView( @@ -73,6 +106,7 @@ class OfflineMapLayersPicker( viewModel.saveSelectedLayer() dismiss() } + return offlineMapLayersPickerBinding.root } diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt index a53aa25b82b..95bc2ab1416 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt @@ -9,22 +9,14 @@ import org.odk.collect.settings.keys.ProjectKeys class OfflineMapLayersPickerViewModel( private val referenceLayerRepository: ReferenceLayerRepository, - scheduler: Scheduler, + private val scheduler: Scheduler, private val settingsProvider: SettingsProvider ) : ViewModel() { private val _data = MutableLiveData, String?>>() val data: LiveData, String?>> = _data init { - scheduler.immediate( - background = { - val layers = referenceLayerRepository.getAll() - val selectedLayerId = settingsProvider.getUnprotectedSettings().getString(ProjectKeys.KEY_REFERENCE_LAYER) - - _data.postValue(Pair(layers, selectedLayerId)) - }, - foreground = { } - ) + refreshLayers() } fun saveSelectedLayer() { @@ -35,4 +27,16 @@ class OfflineMapLayersPickerViewModel( fun changeSelectedLayerId(selectedLayerId: String?) { _data.postValue(_data.value?.copy(second = selectedLayerId)) } + + fun refreshLayers() { + scheduler.immediate( + background = { + val layers = referenceLayerRepository.getAll() + val selectedLayerId = settingsProvider.getUnprotectedSettings().getString(ProjectKeys.KEY_REFERENCE_LAYER) + + _data.postValue(Pair(layers, selectedLayerId)) + }, + foreground = { } + ) + } } diff --git a/maps/src/main/java/org/odk/collect/maps/layers/ReferenceLayerRepository.kt b/maps/src/main/java/org/odk/collect/maps/layers/ReferenceLayerRepository.kt index 58c17cadea5..fa981fd7cc2 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/ReferenceLayerRepository.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/ReferenceLayerRepository.kt @@ -6,6 +6,9 @@ interface ReferenceLayerRepository { fun getAll(): List fun get(id: String): ReferenceLayer? + + fun getSharedLayersDirPath(): String + fun getProjectLayersDirPath(): String } data class ReferenceLayer(val id: String, val file: File, val name: String) diff --git a/maps/src/main/res/layout/offline_map_layers_import_dialog.xml b/maps/src/main/res/layout/offline_map_layers_import_dialog.xml new file mode 100644 index 00000000000..537baf20bbf --- /dev/null +++ b/maps/src/main/res/layout/offline_map_layers_import_dialog.xml @@ -0,0 +1,167 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/maps/src/main/res/layout/offline_map_layers_import_item.xml b/maps/src/main/res/layout/offline_map_layers_import_item.xml new file mode 100644 index 00000000000..c564a94f6a4 --- /dev/null +++ b/maps/src/main/res/layout/offline_map_layers_import_item.xml @@ -0,0 +1,26 @@ + + + + + + + + \ No newline at end of file From 33426f806d2a1cf69839d5fca4629329d83c5be0 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Sat, 25 May 2024 14:08:29 +0200 Subject: [PATCH 03/28] Remove unused code --- .../res/layout/reference_layer_help_footer.xml | 18 ------------------ .../maps/layers/OfflineMapLayersPicker.kt | 4 ---- 2 files changed, 22 deletions(-) delete mode 100644 collect_app/src/main/res/layout/reference_layer_help_footer.xml diff --git a/collect_app/src/main/res/layout/reference_layer_help_footer.xml b/collect_app/src/main/res/layout/reference_layer_help_footer.xml deleted file mode 100644 index 83d0731ca8a..00000000000 --- a/collect_app/src/main/res/layout/reference_layer_help_footer.xml +++ /dev/null @@ -1,18 +0,0 @@ - - - - - - diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index 22145feec5c..796562aeeeb 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -120,8 +120,4 @@ class OfflineMapLayersPicker( // ignore } } - - companion object { - const val TAG = "OfflineMapLayersPicker" - } } From d4882e9d2722c8ee663499466c479ee3ae831655 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 4 Jun 2024 11:20:18 +0200 Subject: [PATCH 04/28] Improved DirectoryReferenceLayerRepository to make it clear which layers dir is used --- .../injection/config/AppDependencyModule.java | 4 +-- .../DirectoryReferenceLayerRepository.kt | 9 +++--- .../DirectoryReferenceLayerRepositoryTest.kt | 30 +++++++++---------- .../MapFragmentReferenceLayerUtilsTest.kt | 6 ++-- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java b/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java index 984c007bb61..c1acd235054 100644 --- a/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java +++ b/collect_app/src/main/java/org/odk/collect/android/injection/config/AppDependencyModule.java @@ -137,7 +137,6 @@ import org.odk.collect.webpage.ExternalWebPageHelper; import java.io.File; -import java.util.Arrays; import javax.inject.Named; import javax.inject.Singleton; @@ -573,7 +572,8 @@ public PreferenceVisibilityHandler providesDisabledPreferencesRemover(SettingsPr @Provides public ReferenceLayerRepository providesReferenceLayerRepository(StoragePathProvider storagePathProvider, SettingsProvider settingsProvider) { return new DirectoryReferenceLayerRepository( - Arrays.asList(storagePathProvider.getOdkDirPath(StorageSubdirectory.LAYERS), storagePathProvider.getOdkDirPath(StorageSubdirectory.SHARED_LAYERS)), + storagePathProvider.getOdkDirPath(StorageSubdirectory.SHARED_LAYERS), + storagePathProvider.getOdkDirPath(StorageSubdirectory.LAYERS), () -> MapConfiguratorProvider.getConfigurator( settingsProvider.getUnprotectedSettings().getString(ProjectKeys.KEY_BASEMAP_SOURCE) ) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt b/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt index 101e0aabb5d..627f635882d 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt @@ -6,7 +6,8 @@ import org.odk.collect.shared.files.DirectoryUtils.listFilesRecursively import java.io.File class DirectoryReferenceLayerRepository( - private val directoryPaths: List, + private val sharedLayersDirPath: String, + private val projectLayersDirPath: String, private val getMapConfigurator: () -> MapConfigurator ) : ReferenceLayerRepository { @@ -28,14 +29,14 @@ class DirectoryReferenceLayerRepository( } override fun getSharedLayersDirPath(): String { - return directoryPaths[1] + return sharedLayersDirPath } override fun getProjectLayersDirPath(): String { - return directoryPaths[0] + return projectLayersDirPath } - private fun getAllFilesWithDirectory() = directoryPaths.flatMap { dir -> + private fun getAllFilesWithDirectory() = listOf(sharedLayersDirPath, projectLayersDirPath).flatMap { dir -> listFilesRecursively(File(dir)).map { file -> Pair(file, dir) } diff --git a/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt index bcd5a6637f1..b75a929db45 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt @@ -25,7 +25,7 @@ class DirectoryReferenceLayerRepositoryTest { it.addFile(file3, true, file3.name) } - val repository = DirectoryReferenceLayerRepository(listOf(dir.absolutePath)) { mapConfigurator } + val repository = DirectoryReferenceLayerRepository(dir.absolutePath, "") { mapConfigurator } assertThat(repository.getAll().map { it.file }, containsInAnyOrder(file1, file3)) } @@ -42,7 +42,7 @@ class DirectoryReferenceLayerRepositoryTest { it.addFile(file3, true, file3.name) } - val repository = DirectoryReferenceLayerRepository(listOf(dir1.absolutePath)) { mapConfigurator } + val repository = DirectoryReferenceLayerRepository(dir1.absolutePath, "") { mapConfigurator } assertThat(repository.getAll().map { it.file }, containsInAnyOrder(file1, file3)) } @@ -50,18 +50,19 @@ class DirectoryReferenceLayerRepositoryTest { fun getAll_withMultipleDirectories_returnsAllSupportedLayersInAllDirectories() { val dir1 = TempFiles.createTempDir() val dir2 = TempFiles.createTempDir() - val dir3 = TempFiles.createTempDir() val file1 = TempFiles.createTempFile(dir1) - val file2 = TempFiles.createTempFile(dir2) - val file3 = TempFiles.createTempFile(dir3) + val file2 = TempFiles.createTempFile(dir1) + val file3 = TempFiles.createTempFile(dir2) + val file4 = TempFiles.createTempFile(dir2) val mapConfigurator = StubMapConfigurator().also { it.addFile(file1, true, file1.name) it.addFile(file2, false, file2.name) it.addFile(file3, true, file3.name) + it.addFile(file4, false, file4.name) } val repository = - DirectoryReferenceLayerRepository(listOf(dir1.absolutePath, dir2.absolutePath, dir3.absolutePath)) { mapConfigurator } + DirectoryReferenceLayerRepository(dir1.absolutePath, dir2.absolutePath) { mapConfigurator } assertThat(repository.getAll().map { it.file }, containsInAnyOrder(file1, file3)) } @@ -74,18 +75,15 @@ class DirectoryReferenceLayerRepositoryTest { fun getAll_withMultipleDirectoriesWithFilesWithTheSameRelativePath_onlyReturnsTheSupportedFileFromTheFirstDirectory() { val dir1 = TempFiles.createTempDir() val dir2 = TempFiles.createTempDir() - val dir3 = TempFiles.createTempDir() val file1 = TempFiles.createTempFile(dir1, "blah", ".temp") val file2 = TempFiles.createTempFile(dir2, "blah", ".temp") - val file3 = TempFiles.createTempFile(dir3, "blah", ".temp") val mapConfigurator = StubMapConfigurator().also { it.addFile(file1, true, file1.name) - it.addFile(file2, false, file2.name) - it.addFile(file3, true, file3.name) + it.addFile(file2, true, file2.name) } val repository = - DirectoryReferenceLayerRepository(listOf(dir1.absolutePath, dir2.absolutePath, dir3.absolutePath)) { mapConfigurator } + DirectoryReferenceLayerRepository(dir1.absolutePath, dir2.absolutePath) { mapConfigurator } assertThat(repository.getAll().map { it.file }, containsInAnyOrder(file1)) } @@ -99,7 +97,7 @@ class DirectoryReferenceLayerRepositoryTest { it.addFile(file2, true, file2.name) } - val repository = DirectoryReferenceLayerRepository(listOf(dir.absolutePath)) { mapConfigurator } + val repository = DirectoryReferenceLayerRepository(dir.absolutePath, "") { mapConfigurator } val file2Layer = repository.getAll().first { it.file == file2 } assertThat(repository.get(file2Layer.id)!!.file, equalTo(file2)) } @@ -115,7 +113,7 @@ class DirectoryReferenceLayerRepositoryTest { it.addFile(file2, true, file2.name) } - val repository = DirectoryReferenceLayerRepository(listOf(dir1.absolutePath, dir2.absolutePath)) { mapConfigurator } + val repository = DirectoryReferenceLayerRepository(dir1.absolutePath, dir2.absolutePath) { mapConfigurator } val file2Layer = repository.getAll().first { it.file == file2 } assertThat(repository.get(file2Layer.id)!!.file, equalTo(file2)) } @@ -131,7 +129,7 @@ class DirectoryReferenceLayerRepositoryTest { it.addFile(file2, true, file2.name) } - val repository = DirectoryReferenceLayerRepository(listOf(dir1.absolutePath, dir2.absolutePath)) { mapConfigurator } + val repository = DirectoryReferenceLayerRepository(dir1.absolutePath, dir2.absolutePath) { mapConfigurator } val layerId = repository.getAll().first().id assertThat(repository.get(layerId)!!.file, equalTo(file1)) } @@ -144,7 +142,7 @@ class DirectoryReferenceLayerRepositoryTest { it.addFile(file, true, file.name) } - val repository = DirectoryReferenceLayerRepository(listOf(dir.absolutePath)) { mapConfigurator } + val repository = DirectoryReferenceLayerRepository(dir.absolutePath, "") { mapConfigurator } val fileLayer = repository.getAll().first { it.file == file } file.delete() @@ -160,7 +158,7 @@ class DirectoryReferenceLayerRepositoryTest { it.addFile(file, true, file.name) } - val repository = DirectoryReferenceLayerRepository(listOf(dir.absolutePath)) { mapConfigurator } + val repository = DirectoryReferenceLayerRepository(dir.absolutePath, "") { mapConfigurator } val fileLayer = repository.getAll().first { it.file == file } assertThat(repository.get(fileLayer.id)!!.name, equalTo(file.name)) diff --git a/maps/src/test/java/org/odk/collect/maps/layers/MapFragmentReferenceLayerUtilsTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/MapFragmentReferenceLayerUtilsTest.kt index 70680f7360b..36b91bd7f7a 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/MapFragmentReferenceLayerUtilsTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/MapFragmentReferenceLayerUtilsTest.kt @@ -24,7 +24,7 @@ class MapFragmentReferenceLayerUtilsTest { assertNull( MapFragmentReferenceLayerUtils.getReferenceLayerFile( config, - DirectoryReferenceLayerRepository(listOf(layersPath), mock()) + DirectoryReferenceLayerRepository(layersPath, "", mock()) ) ) } @@ -37,7 +37,7 @@ class MapFragmentReferenceLayerUtilsTest { assertNull( MapFragmentReferenceLayerUtils.getReferenceLayerFile( config, - DirectoryReferenceLayerRepository(listOf(layersPath), mock()) + DirectoryReferenceLayerRepository(layersPath, "", mock()) ) ) } @@ -57,7 +57,7 @@ class MapFragmentReferenceLayerUtilsTest { assertNotNull( MapFragmentReferenceLayerUtils.getReferenceLayerFile( config, - DirectoryReferenceLayerRepository(listOf(layersPath)) { mapConfigurator } + DirectoryReferenceLayerRepository(layersPath, "") { mapConfigurator } ) ) } From 705b1b9feac0c8e86db4191430b6d28b9c9c1f0a Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Sat, 25 May 2024 15:39:39 +0200 Subject: [PATCH 05/28] Converted MapConfigurator to kotlin --- .../screens/MapsPreferencesFragment.kt | 6 +-- ...apConfigurator.java => MapConfigurator.kt} | 43 ++++++++----------- .../DirectoryReferenceLayerRepositoryTest.kt | 15 +++---- 3 files changed, 29 insertions(+), 35 deletions(-) rename maps/src/main/java/org/odk/collect/maps/{MapConfigurator.java => MapConfigurator.kt} (67%) diff --git a/collect_app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt b/collect_app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt index a20dc55e805..9bfa847b244 100644 --- a/collect_app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt +++ b/collect_app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt @@ -110,8 +110,8 @@ class MapsPreferencesFragment : BaseProjectPreferencesFragment(), Preference.OnP onBasemapSourceChanged(MapConfiguratorProvider.getConfigurator()) basemapSourcePref.setOnPreferenceChangeListener { _: Preference?, value: Any -> val cftor = MapConfiguratorProvider.getConfigurator(value.toString()) - if (!cftor.isAvailable(context)) { - cftor.showUnavailableMessage(context) + if (!cftor.isAvailable(requireContext())) { + cftor.showUnavailableMessage(requireContext()) false } else { onBasemapSourceChanged(cftor) @@ -142,7 +142,7 @@ class MapsPreferencesFragment : BaseProjectPreferencesFragment(), Preference.OnP val baseCategory = findPreference(CATEGORY_BASEMAP) baseCategory!!.removeAll() baseCategory.addPreference(basemapSourcePref) - for (pref in cftor.createPrefs(context, settingsProvider.getUnprotectedSettings())) { + for (pref in cftor.createPrefs(requireContext(), settingsProvider.getUnprotectedSettings())) { pref.isIconSpaceReserved = false baseCategory.addPreference(pref) } diff --git a/maps/src/main/java/org/odk/collect/maps/MapConfigurator.java b/maps/src/main/java/org/odk/collect/maps/MapConfigurator.kt similarity index 67% rename from maps/src/main/java/org/odk/collect/maps/MapConfigurator.java rename to maps/src/main/java/org/odk/collect/maps/MapConfigurator.kt index 4f7da5e86bf..6c8af62df40 100644 --- a/maps/src/main/java/org/odk/collect/maps/MapConfigurator.java +++ b/maps/src/main/java/org/odk/collect/maps/MapConfigurator.kt @@ -1,15 +1,10 @@ -package org.odk.collect.maps; +package org.odk.collect.maps -import android.content.Context; -import android.os.Bundle; - -import androidx.preference.Preference; - -import org.odk.collect.shared.settings.Settings; - -import java.io.File; -import java.util.Collection; -import java.util.List; +import android.content.Context +import android.os.Bundle +import androidx.preference.Preference +import org.odk.collect.shared.settings.Settings +import java.io.File /** * For each MapFragment implementation class, there is one instance of this @@ -22,33 +17,33 @@ * For example, the GoogleMapConfigurator can define a "Google map style" * preference with choices such as Terrain or Satellite. */ -public interface MapConfigurator { - /** Returns true if this MapFragment implementation is available on this device. */ - boolean isAvailable(Context context); +interface MapConfigurator { + /** Returns true if this MapFragment implementation is available on this device. */ + fun isAvailable(context: Context): Boolean /** * Displays a warning to the user that this MapFragment implementation is * unavailable. This will be invoked when isSupported() is false or * createMapFragment(context) returns null. */ - void showUnavailableMessage(Context context); + fun showUnavailableMessage(context: Context) - /** Constructs any preference widgets that are specific to this map implementation. */ - List createPrefs(Context context, Settings settings); + /** Constructs any preference widgets that are specific to this map implementation. */ + fun createPrefs(context: Context, settings: Settings): List - /** Gets the set of keys for preferences that should be watched for changes. */ - Collection getPrefKeys(); + /** Gets the set of keys for preferences that should be watched for changes. */ + val prefKeys: Collection - /** Packs map-related preferences into a Bundle for MapFragment.applyConfig(). */ - Bundle buildConfig(Settings prefs); + /** Packs map-related preferences into a Bundle for MapFragment.applyConfig(). */ + fun buildConfig(prefs: Settings): Bundle /** * Returns true if map fragments obtained from this MapConfigurator are * expected to be able to render the given file as an overlay. This * check determines which files appear as available Reference Layers. */ - boolean supportsLayer(File file); + fun supportsLayer(file: File): Boolean - /** Returns a String name for a given overlay file, or null if unsupported. */ - String getDisplayName(File file); + /** Returns a String name for a given overlay file, or null if unsupported. */ + fun getDisplayName(file: File): String } diff --git a/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt index b75a929db45..154c8b6a840 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt @@ -167,11 +167,11 @@ class DirectoryReferenceLayerRepositoryTest { private class StubMapConfigurator : MapConfigurator { private val files = mutableMapOf>() - override fun supportsLayer(file: File?): Boolean { + override fun supportsLayer(file: File): Boolean { return files[file]!!.first } - override fun getDisplayName(file: File?): String { + override fun getDisplayName(file: File): String { return files[file]!!.second } @@ -179,21 +179,20 @@ class DirectoryReferenceLayerRepositoryTest { files[file] = Pair(isSupported, displayName) } - override fun isAvailable(context: Context?): Boolean { + override fun isAvailable(context: Context): Boolean { TODO("Not yet implemented") } - override fun showUnavailableMessage(context: Context?) { + override fun showUnavailableMessage(context: Context) { TODO("Not yet implemented") } - override fun createPrefs(context: Context?, settings: Settings?): MutableList { + override fun createPrefs(context: Context, settings: Settings): MutableList { TODO("Not yet implemented") } - override fun getPrefKeys(): MutableCollection { - TODO("Not yet implemented") - } + override val prefKeys: Collection + get() = TODO("Not yet implemented") override fun buildConfig(prefs: Settings): Bundle { TODO("Not yet implemented") From 3d81f40a5245889fdcd69c0a3457c5280b0b4559 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 4 Jun 2024 11:38:58 +0200 Subject: [PATCH 06/28] Display progress indicator when the list of layers is being refreshed --- .../maps/layers/OfflineMapLayersPicker.kt | 20 ++++++++++++------- .../layers/OfflineMapLayersPickerViewModel.kt | 6 ++++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index 796562aeeeb..72979e00977 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -77,14 +77,20 @@ class OfflineMapLayersPicker( offlineMapLayersPickerBinding = OfflineMapLayersPickerBinding.inflate(inflater) viewModel.data.observe(this) { data -> - offlineMapLayersPickerBinding.progressIndicator.visibility = View.GONE - offlineMapLayersPickerBinding.layers.visibility = View.VISIBLE - offlineMapLayersPickerBinding.save.isEnabled = true - - val offlineMapLayersAdapter = OfflineMapLayersAdapter(data.first, data.second) { - viewModel.changeSelectedLayerId(it) + if (data == null) { + offlineMapLayersPickerBinding.progressIndicator.visibility = View.VISIBLE + offlineMapLayersPickerBinding.layers.visibility = View.GONE + offlineMapLayersPickerBinding.save.isEnabled = false + } else { + offlineMapLayersPickerBinding.progressIndicator.visibility = View.GONE + offlineMapLayersPickerBinding.layers.visibility = View.VISIBLE + offlineMapLayersPickerBinding.save.isEnabled = true + + val offlineMapLayersAdapter = OfflineMapLayersAdapter(data.first, data.second) { + viewModel.changeSelectedLayerId(it) + } + offlineMapLayersPickerBinding.layers.setAdapter(offlineMapLayersAdapter) } - offlineMapLayersPickerBinding.layers.setAdapter(offlineMapLayersAdapter) } offlineMapLayersPickerBinding.mbtilesInfoGroup.addOnClickListener { diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt index 95bc2ab1416..65ee45d63cf 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt @@ -12,8 +12,8 @@ class OfflineMapLayersPickerViewModel( private val scheduler: Scheduler, private val settingsProvider: SettingsProvider ) : ViewModel() { - private val _data = MutableLiveData, String?>>() - val data: LiveData, String?>> = _data + private val _data = MutableLiveData, String?>?>(null) + val data: LiveData, String?>?> = _data init { refreshLayers() @@ -29,6 +29,8 @@ class OfflineMapLayersPickerViewModel( } fun refreshLayers() { + _data.value = null + scheduler.immediate( background = { val layers = referenceLayerRepository.getAll() From 5f0b710f72aa0212d35f142b1d78af9acda94073 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 4 Jun 2024 11:43:27 +0200 Subject: [PATCH 07/28] Added new tets in OfflineMapLayersPickerTest --- maps/build.gradle.kts | 1 + .../maps/layers/OfflineMapLayersPickerTest.kt | 69 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/maps/build.gradle.kts b/maps/build.gradle.kts index 66c2fd3597b..09c5f1f8e68 100644 --- a/maps/build.gradle.kts +++ b/maps/build.gradle.kts @@ -71,4 +71,5 @@ dependencies { testImplementation(Dependencies.robolectric) testImplementation(Dependencies.mockito_kotlin) testImplementation(Dependencies.androidx_test_espresso_core) + testImplementation(Dependencies.androidx_test_espresso_intents) } diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt index e4e6c551b2e..eabd0a8294a 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt @@ -1,10 +1,14 @@ package org.odk.collect.maps.layers +import android.content.Intent import android.net.Uri +import androidx.core.os.bundleOf import androidx.fragment.app.testing.FragmentScenario import androidx.test.espresso.Espresso.onView import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.assertion.ViewAssertions.matches +import androidx.test.espresso.intent.Intents +import androidx.test.espresso.intent.matcher.IntentMatchers import androidx.test.espresso.matcher.ViewMatchers.assertThat import androidx.test.espresso.matcher.ViewMatchers.isChecked import androidx.test.espresso.matcher.ViewMatchers.isDisplayed @@ -259,6 +263,71 @@ class OfflineMapLayersPickerTest { onView(withRecyclerView(R.id.layers).atPositionOnView(1, R.id.radio_button)).check(matches(isChecked())) } + @Test + fun `clicking the 'add layer' button opens file picker that allows selecting multiple files`() { + Intents.init() + launchFragment() + + onView(withText(string.add_layer)).perform(click()) + + Intents.getIntents()[0].apply { + assertThat(this, IntentMatchers.hasAction(Intent.ACTION_GET_CONTENT)) + assertThat(categories.containsAll(listOf(Intent.CATEGORY_OPENABLE)), equalTo(true)) + assertThat(this, IntentMatchers.hasType("*/*")) + assertThat(this, IntentMatchers.hasExtra(Intent.EXTRA_ALLOW_MULTIPLE, true)) + } + + Intents.release() + } + + @Test + fun `progress indicator is displayed during loading layers after receiving new ones`() { + whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + ReferenceLayer("1", TempFiles.createTempFile(), "layer1") + )) + val scenario = launchFragment() + + scheduler.flush() + + scenario.onFragment { + it.childFragmentManager.setFragmentResult(OfflineMapLayersImportDialog.RESULT_KEY, bundleOf()) + } + + onView(withId(R.id.progress_indicator)).check(matches(isDisplayed())) + onView(withId(R.id.layers)).check(matches(not(isDisplayed()))) + + scheduler.flush() + onView(withId(R.id.progress_indicator)).check(matches(not(isDisplayed()))) + onView(withId(R.id.layers)).check(matches(isDisplayed())) + } + + @Test + fun `when new layers added the list should be updated`() { + whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + ReferenceLayer("1", TempFiles.createTempFile(), "layer1") + )) + + val scenario = launchFragment() + + scheduler.flush() + + whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + ReferenceLayer("1", TempFiles.createTempFile(), "layer1"), + ReferenceLayer("2", TempFiles.createTempFile(), "layer2") + )) + + scenario.onFragment { + it.childFragmentManager.setFragmentResult(OfflineMapLayersImportDialog.RESULT_KEY, bundleOf()) + } + + scheduler.flush() + + onView(withId(R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(3))) + onView(withRecyclerView(R.id.layers).atPositionOnView(0, R.id.radio_button)).check(matches(withText(string.none))) + onView(withRecyclerView(R.id.layers).atPositionOnView(1, R.id.radio_button)).check(matches(withText("layer1"))) + onView(withRecyclerView(R.id.layers).atPositionOnView(2, R.id.radio_button)).check(matches(withText("layer2"))) + } + private fun launchFragment(): FragmentScenario { return fragmentScenarioLauncherRule.launchInContainer(OfflineMapLayersPicker::class.java) } From 44860bde0da49ee97175e26fd1e6b67dd26ec816 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 4 Jun 2024 11:48:04 +0200 Subject: [PATCH 08/28] Ignor non .mbtile files --- .../maps/layers/OfflineMapLayersImportDialogViewModel.kt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt index 7fe15d9d5b3..eb71da83a78 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt @@ -32,10 +32,12 @@ class OfflineMapLayersImportDialogViewModel( uris?.forEach { uriString -> val uri = Uri.parse(uriString) uri.getFileName(contentResolver)?.let { fileName -> - val layerFile = File(tempLayersDir, fileName).also { file -> - uri.toFile(contentResolver, file) + if (fileName.endsWith(".mbtiles")) { + val layerFile = File(tempLayersDir, fileName).also { file -> + uri.toFile(contentResolver, file) + } + layers.add(ReferenceLayer(layerFile.absolutePath, layerFile, MbtilesFile.readName(layerFile) ?: layerFile.name)) } - layers.add(ReferenceLayer(layerFile.absolutePath, layerFile, MbtilesFile.readName(layerFile) ?: layerFile.name)) } } _data.postValue(layers) From 87a8d691e6ef97d5020215f52d33dbc0a09c8911 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 4 Jun 2024 12:11:01 +0200 Subject: [PATCH 09/28] Added tests for OfflineMapLayersImportDialog --- .../OfflineMapLayersImportDialogTest.kt | 273 ++++++++++++++++++ .../java/org/odk/collect/shared/TempFiles.kt | 2 +- 2 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogTest.kt diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogTest.kt new file mode 100644 index 00000000000..34cf4ffc152 --- /dev/null +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogTest.kt @@ -0,0 +1,273 @@ +package org.odk.collect.maps.layers + +import android.os.Bundle +import androidx.core.net.toUri +import androidx.fragment.app.testing.FragmentScenario +import androidx.test.espresso.Espresso.onView +import androidx.test.espresso.action.ViewActions.click +import androidx.test.espresso.action.ViewActions.scrollTo +import androidx.test.espresso.assertion.ViewAssertions.doesNotExist +import androidx.test.espresso.assertion.ViewAssertions.matches +import androidx.test.espresso.matcher.ViewMatchers.isChecked +import androidx.test.espresso.matcher.ViewMatchers.isDisplayed +import androidx.test.espresso.matcher.ViewMatchers.isEnabled +import androidx.test.espresso.matcher.ViewMatchers.withId +import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.hamcrest.CoreMatchers.equalTo +import org.hamcrest.CoreMatchers.not +import org.hamcrest.MatcherAssert.assertThat +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.odk.collect.androidshared.ui.FragmentFactoryBuilder +import org.odk.collect.fragmentstest.FragmentScenarioLauncherRule +import org.odk.collect.shared.TempFiles +import org.odk.collect.strings.R +import org.odk.collect.testshared.FakeScheduler +import org.odk.collect.testshared.RecyclerViewMatcher +import org.odk.collect.testshared.RobolectricHelpers +import java.io.File + +@RunWith(AndroidJUnit4::class) +class OfflineMapLayersImportDialogTest { + private val scheduler = FakeScheduler() + private val sharedLayersDirPath = TempFiles.createTempDir().absolutePath + private val projectLayersDirPath = TempFiles.createTempDir().absolutePath + + @get:Rule + val fragmentScenarioLauncherRule = FragmentScenarioLauncherRule( + FragmentFactoryBuilder() + .forClass(OfflineMapLayersImportDialog::class) { + OfflineMapLayersImportDialog(scheduler, sharedLayersDirPath, projectLayersDirPath) + }.build() + ) + + @Test + fun `clicking the 'cancel' button dismisses the dialog`() { + launchFragment(arrayListOf()).onFragment { + assertThat(it.isVisible, equalTo(true)) + onView(withText(R.string.cancel)).perform(click()) + assertThat(it.isVisible, equalTo(false)) + } + } + + @Test + fun `clicking the 'cancel' button does not set fragment result`() { + launchFragment(arrayListOf()).onFragment { + var resultReceived = false + it.parentFragmentManager.setFragmentResultListener( + OfflineMapLayersImportDialog.RESULT_KEY, + it + ) { _, _ -> + resultReceived = true + } + + onView(withId(org.odk.collect.maps.R.id.cancel_button)).perform(click()) + assertThat(resultReceived, equalTo(false)) + } + } + + @Test + fun `clicking the 'add layer' button dismisses the dialog`() { + launchFragment(arrayListOf()).onFragment { + scheduler.flush() + assertThat(it.isVisible, equalTo(true)) + onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) + scheduler.flush() + RobolectricHelpers.runLooper() + assertThat(it.isVisible, equalTo(false)) + } + } + + @Test + fun `clicking the 'add layer' button sets fragment result`() { + launchFragment(arrayListOf()).onFragment { + scheduler.flush() + var resultReceived = false + it.parentFragmentManager.setFragmentResultListener( + OfflineMapLayersImportDialog.RESULT_KEY, + it + ) { _, _ -> + resultReceived = true + } + + onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) + scheduler.flush() + RobolectricHelpers.runLooper() + assertThat(resultReceived, equalTo(true)) + } + } + + @Test + fun `progress indicator is displayed during loading layers`() { + val file1 = TempFiles.createTempFile("layer1", ".mbtiles") + val file2 = TempFiles.createTempFile("layer2", ".mbtiles") + + launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + + onView(withId(org.odk.collect.maps.R.id.progress_indicator)).check(matches(isDisplayed())) + onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(not(isDisplayed()))) + + scheduler.flush() + + onView(withId(org.odk.collect.maps.R.id.progress_indicator)).check(matches(not(isDisplayed()))) + onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(isDisplayed())) + } + + @Test + fun `the 'cancel' button is enabled during loading layers`() { + launchFragment(arrayListOf()) + + onView(withId(org.odk.collect.maps.R.id.cancel_button)).check(matches(isEnabled())) + scheduler.flush() + onView(withId(org.odk.collect.maps.R.id.cancel_button)).check(matches(isEnabled())) + } + + @Test + fun `the 'add layer' button is disabled during loading layers`() { + launchFragment(arrayListOf()) + + onView(withId(org.odk.collect.maps.R.id.add_layer_button)).check(matches(not(isEnabled()))) + scheduler.flush() + onView(withId(org.odk.collect.maps.R.id.add_layer_button)).check(matches(isEnabled())) + } + + @Test + fun `'All projects' location should be selected by default`() { + launchFragment(arrayListOf()) + + onView(withId(org.odk.collect.maps.R.id.all_projects_option)).check(matches(isChecked())) + onView(withId(org.odk.collect.maps.R.id.current_project_option)).check(matches(not(isChecked()))) + } + + @Test + fun `checking location sets selection correctly`() { + launchFragment(arrayListOf()) + onView(withId(org.odk.collect.maps.R.id.current_project_option)).perform(click()) + + onView(withId(org.odk.collect.maps.R.id.all_projects_option)).check(matches(not(isChecked()))) + onView(withId(org.odk.collect.maps.R.id.current_project_option)).check(matches(isChecked())) + + onView(withId(org.odk.collect.maps.R.id.all_projects_option)).perform(click()) + + onView(withId(org.odk.collect.maps.R.id.all_projects_option)).check(matches(isChecked())) + onView(withId(org.odk.collect.maps.R.id.current_project_option)).check(matches(not(isChecked()))) + } + + @Test + fun `recreating maintains the selected layers location`() { + val scenario = launchFragment(arrayListOf()) + onView(withId(org.odk.collect.maps.R.id.current_project_option)).perform(click()) + + scenario.recreate() + + onView(withId(org.odk.collect.maps.R.id.all_projects_option)).check(matches(not(isChecked()))) + onView(withId(org.odk.collect.maps.R.id.current_project_option)).check(matches(isChecked())) + } + + @Test + fun `the list of selected layers should be displayed`() { + val file1 = TempFiles.createTempFile("layer1", ".mbtiles") + val file2 = TempFiles.createTempFile("layer2", ".mbtiles") + + launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + + scheduler.flush() + + onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(RecyclerViewMatcher.withListSize(2))) + onView(withText("layer1.mbtiles")).check(matches(isDisplayed())) + onView(withText("layer2.mbtiles")).check(matches(isDisplayed())) + } + + @Test + fun `recreating maintains the list of selected layers`() { + val file1 = TempFiles.createTempFile("layer1", ".mbtiles") + val file2 = TempFiles.createTempFile("layer2", ".mbtiles") + + val scenario = launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + scheduler.flush() + + scenario.recreate() + + onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(RecyclerViewMatcher.withListSize(2))) + onView(withText("layer1.mbtiles")).check(matches(isDisplayed())) + onView(withText("layer2.mbtiles")).check(matches(isDisplayed())) + } + + @Test + fun `only mbtiles files are taken into account`() { + val file1 = TempFiles.createTempFile("layer1", ".mbtiles") + val file2 = TempFiles.createTempFile("layer2", ".txt") + + launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + scheduler.flush() + + onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(RecyclerViewMatcher.withListSize(1))) + onView(withText("layer1.mbtiles")).check(matches(isDisplayed())) + onView(withText("layer2.txt")).check(doesNotExist()) + } + + @Test + fun `clicking the 'add layer' button moves the files to the shared layers dir if it is selected`() { + val file1 = TempFiles.createTempFile("layer1", ".mbtiles").also { + it.writeText("blah1") + } + val file2 = TempFiles.createTempFile("layer2", ".mbtiles").also { + it.writeText("blah2") + } + + launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + scheduler.flush() + + onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) + scheduler.flush() + + assertThat(File(sharedLayersDirPath).listFiles().size, equalTo(2)) + assertThat(File(projectLayersDirPath).listFiles().size, equalTo(0)) + + val copiedFile1 = File(sharedLayersDirPath, "layer1.mbtiles") + assertThat(copiedFile1.exists(), equalTo(true)) + assertThat(copiedFile1.readText(), equalTo("blah1")) + + val copiedFile2 = File(sharedLayersDirPath, "layer2.mbtiles") + assertThat(copiedFile2.exists(), equalTo(true)) + assertThat(copiedFile2.readText(), equalTo("blah2")) + } + + @Test + fun `clicking the 'add layer' button moves the files to the project layers dir if it is selected`() { + val file1 = TempFiles.createTempFile("layer1", ".mbtiles").also { + it.writeText("blah1") + } + val file2 = TempFiles.createTempFile("layer2", ".mbtiles").also { + it.writeText("blah2") + } + + launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + scheduler.flush() + + onView(withId(org.odk.collect.maps.R.id.current_project_option)).perform(scrollTo(), click()) + + onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) + scheduler.flush() + + assertThat(File(sharedLayersDirPath).listFiles().size, equalTo(0)) + assertThat(File(projectLayersDirPath).listFiles().size, equalTo(2)) + + val copiedFile1 = File(projectLayersDirPath, "layer1.mbtiles") + assertThat(copiedFile1.exists(), equalTo(true)) + assertThat(copiedFile1.readText(), equalTo("blah1")) + + val copiedFile2 = File(projectLayersDirPath, "layer2.mbtiles") + assertThat(copiedFile2.exists(), equalTo(true)) + assertThat(copiedFile2.readText(), equalTo("blah2")) + } + + private fun launchFragment(uris: ArrayList): FragmentScenario { + return fragmentScenarioLauncherRule.launchInContainer( + OfflineMapLayersImportDialog::class.java, + Bundle().apply { putStringArrayList(OfflineMapLayersImportDialog.URIS, uris) } + ) + } +} diff --git a/shared/src/main/java/org/odk/collect/shared/TempFiles.kt b/shared/src/main/java/org/odk/collect/shared/TempFiles.kt index f5b8fb422e1..4c4bde65b64 100644 --- a/shared/src/main/java/org/odk/collect/shared/TempFiles.kt +++ b/shared/src/main/java/org/odk/collect/shared/TempFiles.kt @@ -26,7 +26,7 @@ object TempFiles { @JvmStatic fun createTempFile(name: String, extension: String): File { val tmpDir = getTempDir() - return File(tmpDir, name + getRandomName(tmpDir) + extension).also { + return File(tmpDir, name + extension).also { it.createNewFile() it.deleteOnExit() } From fa473d1d89bd8c95dd69e80715751ec096b1672c Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 4 Jun 2024 12:13:54 +0200 Subject: [PATCH 10/28] Created a const for the mbtiles file extension --- .../odk/collect/maps/layers/MbtilesFile.java | 4 +++- .../OfflineMapLayersImportDialogViewModel.kt | 2 +- .../OfflineMapLayersImportDialogTest.kt | 22 +++++++++---------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/MbtilesFile.java b/maps/src/main/java/org/odk/collect/maps/layers/MbtilesFile.java index 0e15ba40ba1..8dfbb202e8a 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/MbtilesFile.java +++ b/maps/src/main/java/org/odk/collect/maps/layers/MbtilesFile.java @@ -28,6 +28,8 @@ * See https://github.com/mapbox/mbtiles-spec for the detailed specification. */ public class MbtilesFile implements Closeable, TileSource { + public static final String FILE_EXTENSION = ".mbtiles"; + public enum LayerType { RASTER, VECTOR } private final File file; @@ -166,7 +168,7 @@ private static String detectContentType(File file) throws MbtilesException { if (!file.exists() || !file.isFile()) { throw new NotFileException(file); } - if (!file.getName().toLowerCase(Locale.US).endsWith(".mbtiles")) { + if (!file.getName().toLowerCase(Locale.US).endsWith(FILE_EXTENSION)) { throw new UnsupportedFilenameException(file); } try (SQLiteDatabase db = openSqliteReadOnly(file)) { diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt index eb71da83a78..51388b6d459 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt @@ -32,7 +32,7 @@ class OfflineMapLayersImportDialogViewModel( uris?.forEach { uriString -> val uri = Uri.parse(uriString) uri.getFileName(contentResolver)?.let { fileName -> - if (fileName.endsWith(".mbtiles")) { + if (fileName.endsWith(MbtilesFile.FILE_EXTENSION)) { val layerFile = File(tempLayersDir, fileName).also { file -> uri.toFile(contentResolver, file) } diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogTest.kt index 34cf4ffc152..369a3d2176a 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogTest.kt @@ -101,8 +101,8 @@ class OfflineMapLayersImportDialogTest { @Test fun `progress indicator is displayed during loading layers`() { - val file1 = TempFiles.createTempFile("layer1", ".mbtiles") - val file2 = TempFiles.createTempFile("layer2", ".mbtiles") + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) + val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) @@ -168,8 +168,8 @@ class OfflineMapLayersImportDialogTest { @Test fun `the list of selected layers should be displayed`() { - val file1 = TempFiles.createTempFile("layer1", ".mbtiles") - val file2 = TempFiles.createTempFile("layer2", ".mbtiles") + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) + val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) @@ -182,8 +182,8 @@ class OfflineMapLayersImportDialogTest { @Test fun `recreating maintains the list of selected layers`() { - val file1 = TempFiles.createTempFile("layer1", ".mbtiles") - val file2 = TempFiles.createTempFile("layer2", ".mbtiles") + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) + val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) val scenario = launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) scheduler.flush() @@ -197,7 +197,7 @@ class OfflineMapLayersImportDialogTest { @Test fun `only mbtiles files are taken into account`() { - val file1 = TempFiles.createTempFile("layer1", ".mbtiles") + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) val file2 = TempFiles.createTempFile("layer2", ".txt") launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) @@ -210,10 +210,10 @@ class OfflineMapLayersImportDialogTest { @Test fun `clicking the 'add layer' button moves the files to the shared layers dir if it is selected`() { - val file1 = TempFiles.createTempFile("layer1", ".mbtiles").also { + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION).also { it.writeText("blah1") } - val file2 = TempFiles.createTempFile("layer2", ".mbtiles").also { + val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION).also { it.writeText("blah2") } @@ -237,10 +237,10 @@ class OfflineMapLayersImportDialogTest { @Test fun `clicking the 'add layer' button moves the files to the project layers dir if it is selected`() { - val file1 = TempFiles.createTempFile("layer1", ".mbtiles").also { + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION).also { it.writeText("blah1") } - val file2 = TempFiles.createTempFile("layer2", ".mbtiles").also { + val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION).also { it.writeText("blah2") } From 686bb5bbae8a404e13a2189c9cf40fc439258062 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 4 Jun 2024 12:17:35 +0200 Subject: [PATCH 11/28] Fixed the section about modules in CODE-GUIDELINES --- docs/CODE-GUIDELINES.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/CODE-GUIDELINES.md b/docs/CODE-GUIDELINES.md index 754eea08896..4870b38cd90 100644 --- a/docs/CODE-GUIDELINES.md +++ b/docs/CODE-GUIDELINES.md @@ -178,12 +178,12 @@ Collect is a multi module Gradle project. Modules should have a focused feature There's no easy way to define exactly when a new module should be pulled out of an existing one or when new code calls for a new module - it's best to discuss that with the team before making any decisions. Once a structure has been agreed on, to add a new module: 1. Click `File > New > New module...` in Android Studio -1. Decide whether the new module should be an "Android Library" or "Java or Kotlin Library" - ideally as much code as possible could avoid relying on Android but a lot of features will require at least one Android Library module -1. Review the generated `build.gradle` and remove any unnecessary dependencies or setup -1. Add quality checks to the module's `build.gradle`: +2. Decide whether the new module should be an "Android Library" or "Java or Kotlin Library" - ideally as much code as possible could avoid relying on Android but a lot of features will require at least one Android Library module +3. Review the generated `build.gradle` and remove any unnecessary dependencies or setup +4. Add quality checks to the module's `build.gradle`: ``` apply from: '../config/quality.gradle' ``` -1. If the module will have tests, make sure they get run on CI by adding a line to `test_modules.txt` with `:test` for a Java Library or `:testDebug` for an Android library +5. If the module will have tests, make sure they get run on CI by adding a line to `test_modules.txt` with `` From e3c54044c0696009f47862f1c6bce2ce67d28b66 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Tue, 4 Jun 2024 12:30:02 +0200 Subject: [PATCH 12/28] Naming improvements --- ...tDialog.kt => OfflineMapLayersImporter.kt} | 18 ++++++------- ....kt => OfflineMapLayersImporterAdapter.kt} | 10 +++---- ...t => OfflineMapLayersImporterViewModel.kt} | 2 +- .../maps/layers/OfflineMapLayersPicker.kt | 15 +++++------ ...er.kt => OfflineMapLayersPickerAdapter.kt} | 10 +++---- ...og.xml => offline_map_layers_importer.xml} | 4 +-- ...l => offline_map_layers_importer_item.xml} | 0 ...xml => offline_map_layers_picker_item.xml} | 0 ...est.kt => OfflineMapLayersImporterTest.kt} | 26 +++++++++---------- .../maps/layers/OfflineMapLayersPickerTest.kt | 4 +-- 10 files changed, 44 insertions(+), 45 deletions(-) rename maps/src/main/java/org/odk/collect/maps/layers/{OfflineMapLayersImportDialog.kt => OfflineMapLayersImporter.kt} (80%) rename maps/src/main/java/org/odk/collect/maps/layers/{OfflineMapLayersImportAdapter.kt => OfflineMapLayersImporterAdapter.kt} (55%) rename maps/src/main/java/org/odk/collect/maps/layers/{OfflineMapLayersImportDialogViewModel.kt => OfflineMapLayersImporterViewModel.kt} (98%) rename maps/src/main/java/org/odk/collect/maps/layers/{OfflineMapLayersAdapter.kt => OfflineMapLayersPickerAdapter.kt} (77%) rename maps/src/main/res/layout/{offline_map_layers_import_dialog.xml => offline_map_layers_importer.xml} (98%) rename maps/src/main/res/layout/{offline_map_layers_import_item.xml => offline_map_layers_importer_item.xml} (100%) rename maps/src/main/res/layout/{offline_map_layer.xml => offline_map_layers_picker_item.xml} (100%) rename maps/src/test/java/org/odk/collect/maps/layers/{OfflineMapLayersImportDialogTest.kt => OfflineMapLayersImporterTest.kt} (91%) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialog.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt similarity index 80% rename from maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialog.kt rename to maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt index da7b38dc638..cc93d5d595d 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialog.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt @@ -11,40 +11,40 @@ import androidx.fragment.app.viewModels import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import org.odk.collect.async.Scheduler -import org.odk.collect.maps.databinding.OfflineMapLayersImportDialogBinding +import org.odk.collect.maps.databinding.OfflineMapLayersImporterBinding import org.odk.collect.material.MaterialFullScreenDialogFragment import org.odk.collect.material.MaterialProgressDialogFragment -class OfflineMapLayersImportDialog( +class OfflineMapLayersImporter( private val scheduler: Scheduler, private val sharedLayersDirPath: String, private val projectLayersDirPath: String ) : MaterialFullScreenDialogFragment() { - private val viewModel: OfflineMapLayersImportDialogViewModel by viewModels { + private val viewModel: OfflineMapLayersImporterViewModel by viewModels { object : ViewModelProvider.Factory { override fun create(modelClass: Class): T { - return OfflineMapLayersImportDialogViewModel(scheduler, requireContext().contentResolver) as T + return OfflineMapLayersImporterViewModel(scheduler, requireContext().contentResolver) as T } } } - private lateinit var binding: OfflineMapLayersImportDialogBinding + private lateinit var binding: OfflineMapLayersImporterBinding override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? ): View { - binding = OfflineMapLayersImportDialogBinding.inflate(inflater) + binding = OfflineMapLayersImporterBinding.inflate(inflater) viewModel.data.observe(this) { data -> binding.progressIndicator.visibility = View.GONE - binding.layersList.visibility = View.VISIBLE + binding.layers.visibility = View.VISIBLE binding.addLayerButton.isEnabled = true - val adapter = OfflineMapLayersImportAdapter(data) - binding.layersList.setAdapter(adapter) + val adapter = OfflineMapLayersImporterAdapter(data) + binding.layers.setAdapter(adapter) } viewModel.init(requireArguments().getStringArrayList(URIS)) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportAdapter.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterAdapter.kt similarity index 55% rename from maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportAdapter.kt rename to maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterAdapter.kt index 154a7940432..d011f7e8d61 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportAdapter.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterAdapter.kt @@ -3,14 +3,14 @@ package org.odk.collect.maps.layers import android.view.LayoutInflater import android.view.ViewGroup import androidx.recyclerview.widget.RecyclerView -import org.odk.collect.maps.databinding.OfflineMapLayersImportItemBinding +import org.odk.collect.maps.databinding.OfflineMapLayersImporterItemBinding -class OfflineMapLayersImportAdapter( +class OfflineMapLayersImporterAdapter( private val layers: List, -) : RecyclerView.Adapter() { +) : RecyclerView.Adapter() { override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder { - val binding = OfflineMapLayersImportItemBinding.inflate(LayoutInflater.from(parent.context), parent, false) + val binding = OfflineMapLayersImporterItemBinding.inflate(LayoutInflater.from(parent.context), parent, false) return ViewHolder(binding) } @@ -20,5 +20,5 @@ class OfflineMapLayersImportAdapter( override fun getItemCount() = layers.size - class ViewHolder(val binding: OfflineMapLayersImportItemBinding) : RecyclerView.ViewHolder(binding.root) + class ViewHolder(val binding: OfflineMapLayersImporterItemBinding) : RecyclerView.ViewHolder(binding.root) } diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt similarity index 98% rename from maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt rename to maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt index 51388b6d459..02f1c807643 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImportDialogViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt @@ -12,7 +12,7 @@ import org.odk.collect.shared.TempFiles import java.io.File import java.util.ArrayList -class OfflineMapLayersImportDialogViewModel( +class OfflineMapLayersImporterViewModel( private val scheduler: Scheduler, private val contentResolver: ContentResolver ) : ViewModel() { diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index 72979e00977..4bd8dcdb2d6 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -6,7 +6,6 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.activity.result.contract.ActivityResultContracts -import androidx.fragment.app.setFragmentResultListener import androidx.fragment.app.viewModels import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider @@ -44,8 +43,8 @@ class OfflineMapLayersPicker( } DialogFragmentUtils.showIfNotShowing( - OfflineMapLayersImportDialog::class.java, - Bundle().apply { putStringArrayList(OfflineMapLayersImportDialog.URIS, ArrayList(uriStrings)) }, + OfflineMapLayersImporter::class.java, + Bundle().apply { putStringArrayList(OfflineMapLayersImporter.URIS, ArrayList(uriStrings)) }, childFragmentManager ) } @@ -53,8 +52,8 @@ class OfflineMapLayersPicker( override fun onCreate(savedInstanceState: Bundle?) { childFragmentManager.fragmentFactory = FragmentFactoryBuilder() - .forClass(OfflineMapLayersImportDialog::class) { - OfflineMapLayersImportDialog( + .forClass(OfflineMapLayersImporter::class) { + OfflineMapLayersImporter( scheduler, referenceLayerRepository.getSharedLayersDirPath(), referenceLayerRepository.getProjectLayersDirPath() @@ -64,7 +63,7 @@ class OfflineMapLayersPicker( super.onCreate(savedInstanceState) - childFragmentManager.setFragmentResultListener(OfflineMapLayersImportDialog.RESULT_KEY, this) { _, _ -> + childFragmentManager.setFragmentResultListener(OfflineMapLayersImporter.RESULT_KEY, this) { _, _ -> viewModel.refreshLayers() } } @@ -86,10 +85,10 @@ class OfflineMapLayersPicker( offlineMapLayersPickerBinding.layers.visibility = View.VISIBLE offlineMapLayersPickerBinding.save.isEnabled = true - val offlineMapLayersAdapter = OfflineMapLayersAdapter(data.first, data.second) { + val adapter = OfflineMapLayersPickerAdapter(data.first, data.second) { viewModel.changeSelectedLayerId(it) } - offlineMapLayersPickerBinding.layers.setAdapter(offlineMapLayersAdapter) + offlineMapLayersPickerBinding.layers.setAdapter(adapter) } } diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersAdapter.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerAdapter.kt similarity index 77% rename from maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersAdapter.kt rename to maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerAdapter.kt index 18896722cf4..e4be8ce6bdb 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersAdapter.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerAdapter.kt @@ -3,17 +3,17 @@ package org.odk.collect.maps.layers import android.view.LayoutInflater import android.view.ViewGroup import androidx.recyclerview.widget.RecyclerView -import org.odk.collect.maps.databinding.OfflineMapLayerBinding +import org.odk.collect.maps.databinding.OfflineMapLayersPickerItemBinding import org.odk.collect.strings.localization.getLocalizedString -class OfflineMapLayersAdapter( +class OfflineMapLayersPickerAdapter( private val layers: List, private var selectedLayerId: String?, private val onSelectedLayerChanged: (String?) -> Unit -) : RecyclerView.Adapter() { +) : RecyclerView.Adapter() { override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder { - val binding = OfflineMapLayerBinding.inflate(LayoutInflater.from(parent.context), parent, false) + val binding = OfflineMapLayersPickerItemBinding.inflate(LayoutInflater.from(parent.context), parent, false) return ViewHolder(binding) } @@ -41,5 +41,5 @@ class OfflineMapLayersAdapter( override fun getItemCount() = layers.size + 1 - class ViewHolder(val binding: OfflineMapLayerBinding) : RecyclerView.ViewHolder(binding.root) + class ViewHolder(val binding: OfflineMapLayersPickerItemBinding) : RecyclerView.ViewHolder(binding.root) } diff --git a/maps/src/main/res/layout/offline_map_layers_import_dialog.xml b/maps/src/main/res/layout/offline_map_layers_importer.xml similarity index 98% rename from maps/src/main/res/layout/offline_map_layers_import_dialog.xml rename to maps/src/main/res/layout/offline_map_layers_importer.xml index 537baf20bbf..dbc456424e8 100644 --- a/maps/src/main/res/layout/offline_map_layers_import_dialog.xml +++ b/maps/src/main/res/layout/offline_map_layers_importer.xml @@ -51,7 +51,7 @@ app:layout_constraintEnd_toEndOf="parent" /> + app:constraint_referenced_ids="layers,progress_indicator" /> resultReceived = true @@ -86,7 +86,7 @@ class OfflineMapLayersImportDialogTest { scheduler.flush() var resultReceived = false it.parentFragmentManager.setFragmentResultListener( - OfflineMapLayersImportDialog.RESULT_KEY, + OfflineMapLayersImporter.RESULT_KEY, it ) { _, _ -> resultReceived = true @@ -107,12 +107,12 @@ class OfflineMapLayersImportDialogTest { launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) onView(withId(org.odk.collect.maps.R.id.progress_indicator)).check(matches(isDisplayed())) - onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(not(isDisplayed()))) + onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(not(isDisplayed()))) scheduler.flush() onView(withId(org.odk.collect.maps.R.id.progress_indicator)).check(matches(not(isDisplayed()))) - onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(isDisplayed())) + onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(isDisplayed())) } @Test @@ -175,7 +175,7 @@ class OfflineMapLayersImportDialogTest { scheduler.flush() - onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(RecyclerViewMatcher.withListSize(2))) + onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(2))) onView(withText("layer1.mbtiles")).check(matches(isDisplayed())) onView(withText("layer2.mbtiles")).check(matches(isDisplayed())) } @@ -190,7 +190,7 @@ class OfflineMapLayersImportDialogTest { scenario.recreate() - onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(RecyclerViewMatcher.withListSize(2))) + onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(2))) onView(withText("layer1.mbtiles")).check(matches(isDisplayed())) onView(withText("layer2.mbtiles")).check(matches(isDisplayed())) } @@ -203,7 +203,7 @@ class OfflineMapLayersImportDialogTest { launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) scheduler.flush() - onView(withId(org.odk.collect.maps.R.id.layers_list)).check(matches(RecyclerViewMatcher.withListSize(1))) + onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(1))) onView(withText("layer1.mbtiles")).check(matches(isDisplayed())) onView(withText("layer2.txt")).check(doesNotExist()) } @@ -264,10 +264,10 @@ class OfflineMapLayersImportDialogTest { assertThat(copiedFile2.readText(), equalTo("blah2")) } - private fun launchFragment(uris: ArrayList): FragmentScenario { + private fun launchFragment(uris: ArrayList): FragmentScenario { return fragmentScenarioLauncherRule.launchInContainer( - OfflineMapLayersImportDialog::class.java, - Bundle().apply { putStringArrayList(OfflineMapLayersImportDialog.URIS, uris) } + OfflineMapLayersImporter::class.java, + Bundle().apply { putStringArrayList(OfflineMapLayersImporter.URIS, uris) } ) } } diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt index eabd0a8294a..7378a82fdae 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt @@ -290,7 +290,7 @@ class OfflineMapLayersPickerTest { scheduler.flush() scenario.onFragment { - it.childFragmentManager.setFragmentResult(OfflineMapLayersImportDialog.RESULT_KEY, bundleOf()) + it.childFragmentManager.setFragmentResult(OfflineMapLayersImporter.RESULT_KEY, bundleOf()) } onView(withId(R.id.progress_indicator)).check(matches(isDisplayed())) @@ -317,7 +317,7 @@ class OfflineMapLayersPickerTest { )) scenario.onFragment { - it.childFragmentManager.setFragmentResult(OfflineMapLayersImportDialog.RESULT_KEY, bundleOf()) + it.childFragmentManager.setFragmentResult(OfflineMapLayersImporter.RESULT_KEY, bundleOf()) } scheduler.flush() From 8392808c06a2b9fb4c8257cd80691e933db018a0 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 5 Jun 2024 13:11:44 +0200 Subject: [PATCH 13/28] Improved OfflineMapLayersPickerTest --- .../screens/MapsPreferencesFragment.kt | 2 +- .../geo/geopoint/GeoPointMapActivity.java | 2 +- .../collect/geo/geopoly/GeoPolyActivity.java | 2 +- .../geo/selection/SelectionMapFragment.kt | 2 +- maps/build.gradle.kts | 1 - .../maps/layers/OfflineMapLayersPicker.kt | 4 +- .../maps/layers/OfflineMapLayersPickerTest.kt | 56 ++++++++++++++----- 7 files changed, 50 insertions(+), 19 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt b/collect_app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt index 9bfa847b244..b6948878a19 100644 --- a/collect_app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt +++ b/collect_app/src/main/java/org/odk/collect/android/preferences/screens/MapsPreferencesFragment.kt @@ -56,7 +56,7 @@ class MapsPreferencesFragment : BaseProjectPreferencesFragment(), Preference.OnP override fun onCreate(savedInstanceState: Bundle?) { childFragmentManager.fragmentFactory = FragmentFactoryBuilder() .forClass(OfflineMapLayersPicker::class) { - OfflineMapLayersPicker(referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper) + OfflineMapLayersPicker(requireActivity().activityResultRegistry, referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper) } .build() 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 index b731fc95187..071e441d7e1 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointMapActivity.java +++ b/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointMapActivity.java @@ -141,7 +141,7 @@ public void onCreate(Bundle savedInstanceState) { getSupportFragmentManager().setFragmentFactory(new FragmentFactoryBuilder() .forClass(MapFragment.class, () -> (Fragment) mapFragmentFactory.createMapFragment()) - .forClass(OfflineMapLayersPicker.class, () -> new OfflineMapLayersPicker(referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper)) + .forClass(OfflineMapLayersPicker.class, () -> new OfflineMapLayersPicker(getActivityResultRegistry(), referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper)) .build() ); 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 index 634377fdf43..ae842ddf123 100644 --- a/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyActivity.java +++ b/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyActivity.java @@ -154,7 +154,7 @@ public void handleOnBackPressed() { getSupportFragmentManager().setFragmentFactory(new FragmentFactoryBuilder() .forClass(MapFragment.class, () -> (Fragment) mapFragmentFactory.createMapFragment()) - .forClass(OfflineMapLayersPicker.class, () -> new OfflineMapLayersPicker(referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper)) + .forClass(OfflineMapLayersPicker.class, () -> new OfflineMapLayersPicker(getActivityResultRegistry(), referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper)) .build() ); 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 index 3ddb596d2f7..1aebdbe9a67 100644 --- a/geo/src/main/java/org/odk/collect/geo/selection/SelectionMapFragment.kt +++ b/geo/src/main/java/org/odk/collect/geo/selection/SelectionMapFragment.kt @@ -97,7 +97,7 @@ class SelectionMapFragment( mapFragmentFactory.createMapFragment() as Fragment } .forClass(OfflineMapLayersPicker::class) { - OfflineMapLayersPicker(referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper) + OfflineMapLayersPicker(requireActivity().activityResultRegistry, referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper) } .build() diff --git a/maps/build.gradle.kts b/maps/build.gradle.kts index 09c5f1f8e68..66c2fd3597b 100644 --- a/maps/build.gradle.kts +++ b/maps/build.gradle.kts @@ -71,5 +71,4 @@ dependencies { testImplementation(Dependencies.robolectric) testImplementation(Dependencies.mockito_kotlin) testImplementation(Dependencies.androidx_test_espresso_core) - testImplementation(Dependencies.androidx_test_espresso_intents) } diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index 4bd8dcdb2d6..0a004c2a8ac 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -5,6 +5,7 @@ import android.os.Bundle import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.activity.result.ActivityResultRegistry import androidx.activity.result.contract.ActivityResultContracts import androidx.fragment.app.viewModels import androidx.lifecycle.ViewModel @@ -20,6 +21,7 @@ import org.odk.collect.settings.SettingsProvider import org.odk.collect.webpage.ExternalWebPageHelper class OfflineMapLayersPicker( + registry: ActivityResultRegistry, private val referenceLayerRepository: ReferenceLayerRepository, private val scheduler: Scheduler, private val settingsProvider: SettingsProvider, @@ -35,7 +37,7 @@ class OfflineMapLayersPicker( private lateinit var offlineMapLayersPickerBinding: OfflineMapLayersPickerBinding - private val getLayers = registerForActivityResult(ActivityResultContracts.GetMultipleContents()) { uris -> + private val getLayers = registerForActivityResult(ActivityResultContracts.GetMultipleContents(), registry) { uris -> if (uris.isNotEmpty()) { val uriStrings: MutableList = ArrayList() for (uri in uris) { diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt index 7378a82fdae..7d523a15226 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt @@ -1,14 +1,15 @@ package org.odk.collect.maps.layers -import android.content.Intent import android.net.Uri +import androidx.activity.result.ActivityResultRegistry +import androidx.activity.result.contract.ActivityResultContract +import androidx.activity.result.contract.ActivityResultContracts +import androidx.core.app.ActivityOptionsCompat import androidx.core.os.bundleOf import androidx.fragment.app.testing.FragmentScenario import androidx.test.espresso.Espresso.onView import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.assertion.ViewAssertions.matches -import androidx.test.espresso.intent.Intents -import androidx.test.espresso.intent.matcher.IntentMatchers import androidx.test.espresso.matcher.ViewMatchers.assertThat import androidx.test.espresso.matcher.ViewMatchers.isChecked import androidx.test.espresso.matcher.ViewMatchers.isDisplayed @@ -17,6 +18,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.ext.junit.runners.AndroidJUnit4 import org.hamcrest.CoreMatchers.equalTo +import org.hamcrest.CoreMatchers.instanceOf import org.hamcrest.CoreMatchers.not import org.junit.Rule import org.junit.Test @@ -43,16 +45,32 @@ import org.odk.collect.webpage.ExternalWebPageHelper class OfflineMapLayersPickerTest { private val referenceLayerRepository = mock().also { whenever(it.getAll()).thenReturn(emptyList()) + whenever(it.getSharedLayersDirPath()).thenReturn("") + whenever(it.getProjectLayersDirPath()).thenReturn("") } private val scheduler = FakeScheduler() private val settingsProvider = InMemSettingsProvider() private val externalWebPageHelper = mock() + private val uris = mutableListOf() + private val testRegistry = object : ActivityResultRegistry() { + override fun onLaunch( + requestCode: Int, + contract: ActivityResultContract, + input: I, + options: ActivityOptionsCompat? + ) { + assertThat(contract, instanceOf(ActivityResultContracts.GetMultipleContents()::class.java)) + assertThat(input, equalTo("*/*")) + dispatchResult(requestCode, uris) + } + } + @get:Rule val fragmentScenarioLauncherRule = FragmentScenarioLauncherRule( FragmentFactoryBuilder() .forClass(OfflineMapLayersPicker::class) { - OfflineMapLayersPicker(referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper) + OfflineMapLayersPicker(testRegistry, referenceLayerRepository, scheduler, settingsProvider, externalWebPageHelper) }.build() ) @@ -264,20 +282,32 @@ class OfflineMapLayersPickerTest { } @Test - fun `clicking the 'add layer' button opens file picker that allows selecting multiple files`() { - Intents.init() - launchFragment() + fun `clicking the 'add layer' and selecting layers displays the confirmation dialog`() { + val scenario = launchFragment() + uris.add(Uri.parse("blah")) onView(withText(string.add_layer)).perform(click()) - Intents.getIntents()[0].apply { - assertThat(this, IntentMatchers.hasAction(Intent.ACTION_GET_CONTENT)) - assertThat(categories.containsAll(listOf(Intent.CATEGORY_OPENABLE)), equalTo(true)) - assertThat(this, IntentMatchers.hasType("*/*")) - assertThat(this, IntentMatchers.hasExtra(Intent.EXTRA_ALLOW_MULTIPLE, true)) + scenario.onFragment { + assertThat( + it.childFragmentManager.findFragmentByTag(OfflineMapLayersImporter::class.java.name), + instanceOf(OfflineMapLayersImporter::class.java) + ) } + } - Intents.release() + @Test + fun `clicking the 'add layer' and selecting nothing does not display the confirmation dialog`() { + val scenario = launchFragment() + + onView(withText(string.add_layer)).perform(click()) + + scenario.onFragment { + assertThat( + it.childFragmentManager.findFragmentByTag(OfflineMapLayersImporter::class.java.name), + equalTo(null) + ) + } } @Test From 78bfdc5db5d50e0bc660ce6f4578f6b5b056e486 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 5 Jun 2024 13:19:36 +0200 Subject: [PATCH 14/28] Improved CODE_GUIDELINES.md --- docs/CODE-GUIDELINES.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/CODE-GUIDELINES.md b/docs/CODE-GUIDELINES.md index 4870b38cd90..c2daebe7584 100644 --- a/docs/CODE-GUIDELINES.md +++ b/docs/CODE-GUIDELINES.md @@ -186,4 +186,10 @@ There's no easy way to define exactly when a new module should be pulled out of apply from: '../config/quality.gradle' ``` -5. If the module will have tests, make sure they get run on CI by adding a line to `test_modules.txt` with `` +5. If the module will have tests, make sure they get run on CI by adding a line to `test_modules.txt` with `` and if it's a non-Android module, registering the `testDebug` task in its `build.gradle` file: + + ``` + tasks.register("testDebug") { + dependsOn("test") + } + ``` From c07020d1227e36308920aeb9e131f637db68004a Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 5 Jun 2024 13:34:22 +0200 Subject: [PATCH 15/28] Reverted the change in createTempFile() method --- .../layers/OfflineMapLayersImporterTest.kt | 20 +++++++++---------- .../java/org/odk/collect/shared/TempFiles.kt | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt index c8215f477f4..b8564323800 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt @@ -176,8 +176,8 @@ class OfflineMapLayersImporterTest { scheduler.flush() onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(2))) - onView(withText("layer1.mbtiles")).check(matches(isDisplayed())) - onView(withText("layer2.mbtiles")).check(matches(isDisplayed())) + onView(withText(file1.name)).check(matches(isDisplayed())) + onView(withText(file2.name)).check(matches(isDisplayed())) } @Test @@ -191,8 +191,8 @@ class OfflineMapLayersImporterTest { scenario.recreate() onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(2))) - onView(withText("layer1.mbtiles")).check(matches(isDisplayed())) - onView(withText("layer2.mbtiles")).check(matches(isDisplayed())) + onView(withText(file1.name)).check(matches(isDisplayed())) + onView(withText(file2.name)).check(matches(isDisplayed())) } @Test @@ -204,8 +204,8 @@ class OfflineMapLayersImporterTest { scheduler.flush() onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(1))) - onView(withText("layer1.mbtiles")).check(matches(isDisplayed())) - onView(withText("layer2.txt")).check(doesNotExist()) + onView(withText(file1.name)).check(matches(isDisplayed())) + onView(withText(file2.name)).check(doesNotExist()) } @Test @@ -226,11 +226,11 @@ class OfflineMapLayersImporterTest { assertThat(File(sharedLayersDirPath).listFiles().size, equalTo(2)) assertThat(File(projectLayersDirPath).listFiles().size, equalTo(0)) - val copiedFile1 = File(sharedLayersDirPath, "layer1.mbtiles") + val copiedFile1 = File(sharedLayersDirPath, file1.name) assertThat(copiedFile1.exists(), equalTo(true)) assertThat(copiedFile1.readText(), equalTo("blah1")) - val copiedFile2 = File(sharedLayersDirPath, "layer2.mbtiles") + val copiedFile2 = File(sharedLayersDirPath, file2.name) assertThat(copiedFile2.exists(), equalTo(true)) assertThat(copiedFile2.readText(), equalTo("blah2")) } @@ -255,11 +255,11 @@ class OfflineMapLayersImporterTest { assertThat(File(sharedLayersDirPath).listFiles().size, equalTo(0)) assertThat(File(projectLayersDirPath).listFiles().size, equalTo(2)) - val copiedFile1 = File(projectLayersDirPath, "layer1.mbtiles") + val copiedFile1 = File(projectLayersDirPath, file1.name) assertThat(copiedFile1.exists(), equalTo(true)) assertThat(copiedFile1.readText(), equalTo("blah1")) - val copiedFile2 = File(projectLayersDirPath, "layer2.mbtiles") + val copiedFile2 = File(projectLayersDirPath, file2.name) assertThat(copiedFile2.exists(), equalTo(true)) assertThat(copiedFile2.readText(), equalTo("blah2")) } diff --git a/shared/src/main/java/org/odk/collect/shared/TempFiles.kt b/shared/src/main/java/org/odk/collect/shared/TempFiles.kt index 4c4bde65b64..f5b8fb422e1 100644 --- a/shared/src/main/java/org/odk/collect/shared/TempFiles.kt +++ b/shared/src/main/java/org/odk/collect/shared/TempFiles.kt @@ -26,7 +26,7 @@ object TempFiles { @JvmStatic fun createTempFile(name: String, extension: String): File { val tmpDir = getTempDir() - return File(tmpDir, name + extension).also { + return File(tmpDir, name + getRandomName(tmpDir) + extension).also { it.createNewFile() it.deleteOnExit() } From b91a106aa26690d06e1e720bd1ddae602db76be6 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 5 Jun 2024 14:59:12 +0200 Subject: [PATCH 16/28] Fixed observations in OfflineMapLayersImporter --- .../maps/layers/OfflineMapLayersImporter.kt | 52 +++++++++++-------- .../OfflineMapLayersImporterViewModel.kt | 15 ++++-- .../layout/offline_map_layers_importer.xml | 21 +------- .../layers/OfflineMapLayersImporterTest.kt | 31 ++++++++--- 4 files changed, 66 insertions(+), 53 deletions(-) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt index cc93d5d595d..ec598ee1432 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt @@ -38,14 +38,6 @@ class OfflineMapLayersImporter( ): View { binding = OfflineMapLayersImporterBinding.inflate(inflater) - viewModel.data.observe(this) { data -> - binding.progressIndicator.visibility = View.GONE - binding.layers.visibility = View.VISIBLE - binding.addLayerButton.isEnabled = true - - val adapter = OfflineMapLayersImporterAdapter(data) - binding.layers.setAdapter(adapter) - } viewModel.init(requireArguments().getStringArrayList(URIS)) binding.cancelButton.setOnClickListener { @@ -59,25 +51,39 @@ class OfflineMapLayersImporter( projectLayersDirPath } - val isLoading = viewModel.addLayers(layersDir) - MaterialProgressDialogFragment.showOn( - this, - isLoading, - childFragmentManager - ) { - MaterialProgressDialogFragment().also { dialog -> - dialog.message = getString(org.odk.collect.strings.R.string.loading) - } + viewModel.addLayers(layersDir) + } + return binding.root + } + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + + MaterialProgressDialogFragment.showOn( + this, + viewModel.isLoading, + childFragmentManager + ) { + MaterialProgressDialogFragment().also { dialog -> + dialog.message = getString(org.odk.collect.strings.R.string.loading) } + } + + viewModel.isLoading.observe(this) { isLoading -> + binding.addLayerButton.isEnabled = !isLoading + } - isLoading.observe(this) { - if (!it) { - setFragmentResult(RESULT_KEY, bundleOf()) - dismiss() - } + viewModel.data.observe(this) { data -> + val adapter = OfflineMapLayersImporterAdapter(data) + binding.layers.setAdapter(adapter) + } + + viewModel.isAddingNewLayersFinished.observe(this) { isAddingNewLayersFinished -> + if (isAddingNewLayersFinished) { + setFragmentResult(RESULT_KEY, bundleOf()) + dismiss() } } - return binding.root } override fun onCloseClicked() { diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt index 02f1c807643..4b297b40033 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt @@ -16,6 +16,11 @@ class OfflineMapLayersImporterViewModel( private val scheduler: Scheduler, private val contentResolver: ContentResolver ) : ViewModel() { + private val _isLoading = MutableLiveData() + val isLoading: LiveData = _isLoading + + private val _isAddingNewLayersFinished = MutableLiveData() + val isAddingNewLayersFinished: LiveData = _isAddingNewLayersFinished private val _data = MutableLiveData>() val data: LiveData> = _data @@ -23,6 +28,7 @@ class OfflineMapLayersImporterViewModel( private lateinit var tempLayersDir: File fun init(uris: ArrayList?) { + _isLoading.value = true scheduler.immediate( background = { tempLayersDir = TempFiles.createTempDir().also { @@ -40,14 +46,15 @@ class OfflineMapLayersImporterViewModel( } } } + _isLoading.postValue(false) _data.postValue(layers) }, foreground = { } ) } - fun addLayers(layersDir: String): LiveData { - val isLoading = MutableLiveData(true) + fun addLayers(layersDir: String) { + _isLoading.value = true scheduler.immediate( background = { val destDir = File(layersDir) @@ -56,10 +63,10 @@ class OfflineMapLayersImporterViewModel( } tempLayersDir.delete() - isLoading.postValue(false) + _isLoading.postValue(false) + _isAddingNewLayersFinished.postValue(true) }, foreground = { } ) - return isLoading } } diff --git a/maps/src/main/res/layout/offline_map_layers_importer.xml b/maps/src/main/res/layout/offline_map_layers_importer.xml index dbc456424e8..75cc50804eb 100644 --- a/maps/src/main/res/layout/offline_map_layers_importer.xml +++ b/maps/src/main/res/layout/offline_map_layers_importer.xml @@ -54,7 +54,6 @@ android:id="@+id/layers" android:layout_width="0dp" android:layout_height="wrap_content" - android:visibility="gone" android:layout_marginTop="@dimen/margin_small" app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager" app:layout_constraintStart_toStartOf="parent" @@ -62,24 +61,6 @@ app:layout_constraintTop_toBottomOf="@id/layers_title" tools:visibility="visible" /> - - - - diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt index b8564323800..914cd633e4e 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt @@ -15,6 +15,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.ext.junit.runners.AndroidJUnit4 import org.hamcrest.CoreMatchers.equalTo +import org.hamcrest.CoreMatchers.instanceOf import org.hamcrest.CoreMatchers.not import org.hamcrest.MatcherAssert.assertThat import org.junit.Rule @@ -22,6 +23,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.odk.collect.androidshared.ui.FragmentFactoryBuilder import org.odk.collect.fragmentstest.FragmentScenarioLauncherRule +import org.odk.collect.material.MaterialProgressDialogFragment import org.odk.collect.shared.TempFiles import org.odk.collect.strings.R import org.odk.collect.testshared.FakeScheduler @@ -46,6 +48,7 @@ class OfflineMapLayersImporterTest { @Test fun `clicking the 'cancel' button dismisses the dialog`() { launchFragment(arrayListOf()).onFragment { + scheduler.flush() assertThat(it.isVisible, equalTo(true)) onView(withText(R.string.cancel)).perform(click()) assertThat(it.isVisible, equalTo(false)) @@ -69,11 +72,15 @@ class OfflineMapLayersImporterTest { } @Test - fun `clicking the 'add layer' button dismisses the dialog`() { + fun `clicking the 'add layer' button displays progress indicator before adding layers and dismisses the dialog after`() { launchFragment(arrayListOf()).onFragment { scheduler.flush() assertThat(it.isVisible, equalTo(true)) onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) + assertThat( + it.childFragmentManager.findFragmentByTag(MaterialProgressDialogFragment::class.java.name), + instanceOf(MaterialProgressDialogFragment::class.java) + ) scheduler.flush() RobolectricHelpers.runLooper() assertThat(it.isVisible, equalTo(false)) @@ -104,15 +111,23 @@ class OfflineMapLayersImporterTest { val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) - launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + val scenario = launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) - onView(withId(org.odk.collect.maps.R.id.progress_indicator)).check(matches(isDisplayed())) - onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(not(isDisplayed()))) + scenario.onFragment { + assertThat( + it.childFragmentManager.findFragmentByTag(MaterialProgressDialogFragment::class.java.name), + instanceOf(MaterialProgressDialogFragment::class.java) + ) + } scheduler.flush() - onView(withId(org.odk.collect.maps.R.id.progress_indicator)).check(matches(not(isDisplayed()))) - onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(isDisplayed())) + scenario.onFragment { + assertThat( + it.childFragmentManager.findFragmentByTag(MaterialProgressDialogFragment::class.java.name), + equalTo(null) + ) + } } @Test @@ -144,6 +159,8 @@ class OfflineMapLayersImporterTest { @Test fun `checking location sets selection correctly`() { launchFragment(arrayListOf()) + scheduler.flush() + onView(withId(org.odk.collect.maps.R.id.current_project_option)).perform(click()) onView(withId(org.odk.collect.maps.R.id.all_projects_option)).check(matches(not(isChecked()))) @@ -158,6 +175,8 @@ class OfflineMapLayersImporterTest { @Test fun `recreating maintains the selected layers location`() { val scenario = launchFragment(arrayListOf()) + scheduler.flush() + onView(withId(org.odk.collect.maps.R.id.current_project_option)).perform(click()) scenario.recreate() From 2b49ef9db0e926d7444e51ef496c466dc85809da Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 5 Jun 2024 15:00:09 +0200 Subject: [PATCH 17/28] Fixed observations in OfflineMapLayersPicker --- .../maps/layers/OfflineMapLayersPicker.kt | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index 0a004c2a8ac..b0e1c5849c0 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -77,23 +77,6 @@ class OfflineMapLayersPicker( ): View { offlineMapLayersPickerBinding = OfflineMapLayersPickerBinding.inflate(inflater) - viewModel.data.observe(this) { data -> - if (data == null) { - offlineMapLayersPickerBinding.progressIndicator.visibility = View.VISIBLE - offlineMapLayersPickerBinding.layers.visibility = View.GONE - offlineMapLayersPickerBinding.save.isEnabled = false - } else { - offlineMapLayersPickerBinding.progressIndicator.visibility = View.GONE - offlineMapLayersPickerBinding.layers.visibility = View.VISIBLE - offlineMapLayersPickerBinding.save.isEnabled = true - - val adapter = OfflineMapLayersPickerAdapter(data.first, data.second) { - viewModel.changeSelectedLayerId(it) - } - offlineMapLayersPickerBinding.layers.setAdapter(adapter) - } - } - offlineMapLayersPickerBinding.mbtilesInfoGroup.addOnClickListener { externalWebPageHelper.openWebPageInCustomTab( requireActivity(), @@ -117,6 +100,27 @@ class OfflineMapLayersPicker( return offlineMapLayersPickerBinding.root } + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + + viewModel.data.observe(this) { data -> + if (data == null) { + offlineMapLayersPickerBinding.progressIndicator.visibility = View.VISIBLE + offlineMapLayersPickerBinding.layers.visibility = View.GONE + offlineMapLayersPickerBinding.save.isEnabled = false + } else { + offlineMapLayersPickerBinding.progressIndicator.visibility = View.GONE + offlineMapLayersPickerBinding.layers.visibility = View.VISIBLE + offlineMapLayersPickerBinding.save.isEnabled = true + + val adapter = OfflineMapLayersPickerAdapter(data.first, data.second) { + viewModel.changeSelectedLayerId(it) + } + offlineMapLayersPickerBinding.layers.setAdapter(adapter) + } + } + } + override fun onStart() { super.onStart() try { From 8da1c7f2ffea4f1d810d8b58df7e6f8b60166a86 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 5 Jun 2024 22:02:29 +0200 Subject: [PATCH 18/28] Use one shared viewmodel --- .../maps/layers/OfflineMapLayersImporter.kt | 57 +++------ .../maps/layers/OfflineMapLayersPicker.kt | 61 ++++----- .../layers/OfflineMapLayersPickerViewModel.kt | 44 ------- ...wModel.kt => OfflineMapLayersViewModel.kt} | 58 +++++++-- .../layout/offline_map_layers_importer.xml | 25 +++- .../res/layout/offline_map_layers_picker.xml | 5 +- .../layers/OfflineMapLayersImporterTest.kt | 121 +++++++----------- .../maps/layers/OfflineMapLayersPickerTest.kt | 89 +++++++------ 8 files changed, 207 insertions(+), 253 deletions(-) delete mode 100644 maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt rename maps/src/main/java/org/odk/collect/maps/layers/{OfflineMapLayersImporterViewModel.kt => OfflineMapLayersViewModel.kt} (53%) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt index ec598ee1432..bef347ec9e6 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt @@ -5,26 +5,25 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.appcompat.widget.Toolbar -import androidx.core.os.bundleOf -import androidx.fragment.app.setFragmentResult -import androidx.fragment.app.viewModels +import androidx.fragment.app.activityViewModels import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import org.odk.collect.async.Scheduler import org.odk.collect.maps.databinding.OfflineMapLayersImporterBinding import org.odk.collect.material.MaterialFullScreenDialogFragment -import org.odk.collect.material.MaterialProgressDialogFragment +import org.odk.collect.settings.SettingsProvider class OfflineMapLayersImporter( + private val referenceLayerRepository: ReferenceLayerRepository, private val scheduler: Scheduler, + private val settingsProvider: SettingsProvider, private val sharedLayersDirPath: String, private val projectLayersDirPath: String ) : MaterialFullScreenDialogFragment() { - - private val viewModel: OfflineMapLayersImporterViewModel by viewModels { + val viewModel: OfflineMapLayersViewModel by activityViewModels { object : ViewModelProvider.Factory { override fun create(modelClass: Class): T { - return OfflineMapLayersImporterViewModel(scheduler, requireContext().contentResolver) as T + return OfflineMapLayersViewModel(referenceLayerRepository, scheduler, settingsProvider, requireContext().contentResolver) as T } } } @@ -38,8 +37,6 @@ class OfflineMapLayersImporter( ): View { binding = OfflineMapLayersImporterBinding.inflate(inflater) - viewModel.init(requireArguments().getStringArrayList(URIS)) - binding.cancelButton.setOnClickListener { dismiss() } @@ -51,7 +48,8 @@ class OfflineMapLayersImporter( projectLayersDirPath } - viewModel.addLayers(layersDir) + viewModel.importNewLayers(layersDir) + dismiss() } return binding.root } @@ -59,35 +57,25 @@ class OfflineMapLayersImporter( override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - MaterialProgressDialogFragment.showOn( - this, - viewModel.isLoading, - childFragmentManager - ) { - MaterialProgressDialogFragment().also { dialog -> - dialog.message = getString(org.odk.collect.strings.R.string.loading) - } - } - viewModel.isLoading.observe(this) { isLoading -> - binding.addLayerButton.isEnabled = !isLoading + if (isLoading) { + binding.addLayerButton.isEnabled = false + binding.layers.visibility = View.GONE + binding.progressIndicator.visibility = View.VISIBLE + } else { + binding.addLayerButton.isEnabled = true + binding.layers.visibility = View.VISIBLE + binding.progressIndicator.visibility = View.GONE + } } - viewModel.data.observe(this) { data -> - val adapter = OfflineMapLayersImporterAdapter(data) + viewModel.layersToImport.observe(this) { layersToImport -> + val adapter = OfflineMapLayersImporterAdapter(layersToImport) binding.layers.setAdapter(adapter) } - - viewModel.isAddingNewLayersFinished.observe(this) { isAddingNewLayersFinished -> - if (isAddingNewLayersFinished) { - setFragmentResult(RESULT_KEY, bundleOf()) - dismiss() - } - } } - override fun onCloseClicked() { - } + override fun onCloseClicked() = Unit override fun onBackPressed() { dismiss() @@ -96,9 +84,4 @@ class OfflineMapLayersImporter( override fun getToolbar(): Toolbar { return binding.toolbar } - - companion object { - const val URIS = "uris" - const val RESULT_KEY = "layersAdded" - } } diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index b0e1c5849c0..ed7439bf144 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -7,7 +7,7 @@ import android.view.View import android.view.ViewGroup import androidx.activity.result.ActivityResultRegistry import androidx.activity.result.contract.ActivityResultContracts -import androidx.fragment.app.viewModels +import androidx.fragment.app.activityViewModels import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import com.google.android.material.bottomsheet.BottomSheetBehavior @@ -27,26 +27,21 @@ class OfflineMapLayersPicker( private val settingsProvider: SettingsProvider, private val externalWebPageHelper: ExternalWebPageHelper ) : BottomSheetDialogFragment() { - private val viewModel: OfflineMapLayersPickerViewModel by viewModels { + private val viewModel: OfflineMapLayersViewModel by activityViewModels { object : ViewModelProvider.Factory { override fun create(modelClass: Class): T { - return OfflineMapLayersPickerViewModel(referenceLayerRepository, scheduler, settingsProvider) as T + return OfflineMapLayersViewModel(referenceLayerRepository, scheduler, settingsProvider, requireContext().contentResolver) as T } } } - private lateinit var offlineMapLayersPickerBinding: OfflineMapLayersPickerBinding + private lateinit var binding: OfflineMapLayersPickerBinding private val getLayers = registerForActivityResult(ActivityResultContracts.GetMultipleContents(), registry) { uris -> if (uris.isNotEmpty()) { - val uriStrings: MutableList = ArrayList() - for (uri in uris) { - uriStrings.add(uri.toString()) - } - + viewModel.loadLayersToImport(uris) DialogFragmentUtils.showIfNotShowing( OfflineMapLayersImporter::class.java, - Bundle().apply { putStringArrayList(OfflineMapLayersImporter.URIS, ArrayList(uriStrings)) }, childFragmentManager ) } @@ -56,7 +51,9 @@ class OfflineMapLayersPicker( childFragmentManager.fragmentFactory = FragmentFactoryBuilder() .forClass(OfflineMapLayersImporter::class) { OfflineMapLayersImporter( + referenceLayerRepository, scheduler, + settingsProvider, referenceLayerRepository.getSharedLayersDirPath(), referenceLayerRepository.getProjectLayersDirPath() ) @@ -64,10 +61,6 @@ class OfflineMapLayersPicker( .build() super.onCreate(savedInstanceState) - - childFragmentManager.setFragmentResultListener(OfflineMapLayersImporter.RESULT_KEY, this) { _, _ -> - viewModel.refreshLayers() - } } override fun onCreateView( @@ -75,49 +68,51 @@ class OfflineMapLayersPicker( container: ViewGroup?, savedInstanceState: Bundle? ): View { - offlineMapLayersPickerBinding = OfflineMapLayersPickerBinding.inflate(inflater) + binding = OfflineMapLayersPickerBinding.inflate(inflater) - offlineMapLayersPickerBinding.mbtilesInfoGroup.addOnClickListener { + binding.mbtilesInfoGroup.addOnClickListener { externalWebPageHelper.openWebPageInCustomTab( requireActivity(), Uri.parse("https://docs.getodk.org/collect-offline-maps/#transferring-offline-tilesets-to-devices") ) } - offlineMapLayersPickerBinding.addLayer.setOnClickListener { + binding.addLayer.setOnClickListener { getLayers.launch("*/*") } - offlineMapLayersPickerBinding.cancel.setOnClickListener { + binding.cancel.setOnClickListener { dismiss() } - offlineMapLayersPickerBinding.save.setOnClickListener { + binding.save.setOnClickListener { viewModel.saveSelectedLayer() dismiss() } - return offlineMapLayersPickerBinding.root + return binding.root } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - viewModel.data.observe(this) { data -> - if (data == null) { - offlineMapLayersPickerBinding.progressIndicator.visibility = View.VISIBLE - offlineMapLayersPickerBinding.layers.visibility = View.GONE - offlineMapLayersPickerBinding.save.isEnabled = false + viewModel.isLoading.observe(this) { isLoading -> + if (isLoading) { + binding.progressIndicator.visibility = View.VISIBLE + binding.layers.visibility = View.GONE + binding.save.isEnabled = false } else { - offlineMapLayersPickerBinding.progressIndicator.visibility = View.GONE - offlineMapLayersPickerBinding.layers.visibility = View.VISIBLE - offlineMapLayersPickerBinding.save.isEnabled = true - - val adapter = OfflineMapLayersPickerAdapter(data.first, data.second) { - viewModel.changeSelectedLayerId(it) - } - offlineMapLayersPickerBinding.layers.setAdapter(adapter) + binding.progressIndicator.visibility = View.GONE + binding.layers.visibility = View.VISIBLE + binding.save.isEnabled = true + } + } + + viewModel.existingLayers.observe(this) { layers -> + val adapter = OfflineMapLayersPickerAdapter(layers.first, layers.second) { + viewModel.changeSelectedLayerId(it) } + binding.layers.setAdapter(adapter) } } diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt deleted file mode 100644 index 65ee45d63cf..00000000000 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPickerViewModel.kt +++ /dev/null @@ -1,44 +0,0 @@ -package org.odk.collect.maps.layers - -import androidx.lifecycle.LiveData -import androidx.lifecycle.MutableLiveData -import androidx.lifecycle.ViewModel -import org.odk.collect.async.Scheduler -import org.odk.collect.settings.SettingsProvider -import org.odk.collect.settings.keys.ProjectKeys - -class OfflineMapLayersPickerViewModel( - private val referenceLayerRepository: ReferenceLayerRepository, - private val scheduler: Scheduler, - private val settingsProvider: SettingsProvider -) : ViewModel() { - private val _data = MutableLiveData, String?>?>(null) - val data: LiveData, String?>?> = _data - - init { - refreshLayers() - } - - fun saveSelectedLayer() { - val selectedLayerId = data.value?.second - settingsProvider.getUnprotectedSettings().save(ProjectKeys.KEY_REFERENCE_LAYER, selectedLayerId) - } - - fun changeSelectedLayerId(selectedLayerId: String?) { - _data.postValue(_data.value?.copy(second = selectedLayerId)) - } - - fun refreshLayers() { - _data.value = null - - scheduler.immediate( - background = { - val layers = referenceLayerRepository.getAll() - val selectedLayerId = settingsProvider.getUnprotectedSettings().getString(ProjectKeys.KEY_REFERENCE_LAYER) - - _data.postValue(Pair(layers, selectedLayerId)) - }, - foreground = { } - ) - } -} diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt similarity index 53% rename from maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt rename to maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt index 4b297b40033..0b632059048 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporterViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt @@ -8,26 +8,48 @@ import androidx.lifecycle.ViewModel import org.odk.collect.androidshared.system.getFileName import org.odk.collect.androidshared.system.toFile import org.odk.collect.async.Scheduler +import org.odk.collect.settings.SettingsProvider +import org.odk.collect.settings.keys.ProjectKeys import org.odk.collect.shared.TempFiles import java.io.File -import java.util.ArrayList -class OfflineMapLayersImporterViewModel( +class OfflineMapLayersViewModel( + private val referenceLayerRepository: ReferenceLayerRepository, private val scheduler: Scheduler, + private val settingsProvider: SettingsProvider, private val contentResolver: ContentResolver ) : ViewModel() { private val _isLoading = MutableLiveData() val isLoading: LiveData = _isLoading - private val _isAddingNewLayersFinished = MutableLiveData() - val isAddingNewLayersFinished: LiveData = _isAddingNewLayersFinished + private val _existingLayers = MutableLiveData, String?>>() + val existingLayers: LiveData, String?>> = _existingLayers - private val _data = MutableLiveData>() - val data: LiveData> = _data + private val _layersToImport = MutableLiveData>() + val layersToImport: LiveData> = _layersToImport private lateinit var tempLayersDir: File - fun init(uris: ArrayList?) { + init { + loadExistingLayers() + } + + private fun loadExistingLayers() { + _isLoading.value = true + scheduler.immediate( + background = { + val layers = referenceLayerRepository.getAll() + val selectedLayerId = + settingsProvider.getUnprotectedSettings().getString(ProjectKeys.KEY_REFERENCE_LAYER) + + _isLoading.postValue(false) + _existingLayers.postValue(Pair(layers, selectedLayerId)) + }, + foreground = { } + ) + } + + fun loadLayersToImport(uris: List) { _isLoading.value = true scheduler.immediate( background = { @@ -35,8 +57,7 @@ class OfflineMapLayersImporterViewModel( it.deleteOnExit() } val layers = mutableListOf() - uris?.forEach { uriString -> - val uri = Uri.parse(uriString) + uris.forEach { uri -> uri.getFileName(contentResolver)?.let { fileName -> if (fileName.endsWith(MbtilesFile.FILE_EXTENSION)) { val layerFile = File(tempLayersDir, fileName).also { file -> @@ -47,13 +68,13 @@ class OfflineMapLayersImporterViewModel( } } _isLoading.postValue(false) - _data.postValue(layers) + _layersToImport.postValue(layers) }, foreground = { } ) } - fun addLayers(layersDir: String) { + fun importNewLayers(layersDir: String) { _isLoading.value = true scheduler.immediate( background = { @@ -62,11 +83,20 @@ class OfflineMapLayersImporterViewModel( it.copyTo(File(destDir, it.name), true) } tempLayersDir.delete() - _isLoading.postValue(false) - _isAddingNewLayersFinished.postValue(true) }, - foreground = { } + foreground = { + loadExistingLayers() + } ) } + + fun saveSelectedLayer() { + val selectedLayerId = existingLayers.value?.second + settingsProvider.getUnprotectedSettings().save(ProjectKeys.KEY_REFERENCE_LAYER, selectedLayerId) + } + + fun changeSelectedLayerId(selectedLayerId: String?) { + _existingLayers.postValue(_existingLayers.value?.copy(second = selectedLayerId)) + } } diff --git a/maps/src/main/res/layout/offline_map_layers_importer.xml b/maps/src/main/res/layout/offline_map_layers_importer.xml index 75cc50804eb..2b678de5f32 100644 --- a/maps/src/main/res/layout/offline_map_layers_importer.xml +++ b/maps/src/main/res/layout/offline_map_layers_importer.xml @@ -1,7 +1,6 @@ @@ -58,8 +57,25 @@ app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager" app:layout_constraintStart_toStartOf="parent" app:layout_constraintEnd_toEndOf="parent" - app:layout_constraintTop_toBottomOf="@id/layers_title" - tools:visibility="visible" /> + app:layout_constraintTop_toBottomOf="@id/layers_title" /> + + + + @@ -141,7 +157,6 @@ android:layout_marginEnd="@dimen/margin_standard" android:layout_marginBottom="@dimen/margin_extra_small" android:text="@string/add_layer" - android:enabled="false" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" /> diff --git a/maps/src/main/res/layout/offline_map_layers_picker.xml b/maps/src/main/res/layout/offline_map_layers_picker.xml index b6c5d778549..5e86a2eeebe 100644 --- a/maps/src/main/res/layout/offline_map_layers_picker.xml +++ b/maps/src/main/res/layout/offline_map_layers_picker.xml @@ -1,7 +1,6 @@ @@ -90,14 +89,12 @@ android:id="@+id/layers" android:layout_width="0dp" android:layout_height="0dp" - android:visibility="gone" app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager" app:layout_constraintStart_toStartOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintBottom_toTopOf="@id/add_layer" app:layout_constraintHeight_default="wrap" - app:layout_constraintTop_toBottomOf="@id/top_divider" - tools:visibility="visible" /> + app:layout_constraintTop_toBottomOf="@id/top_divider" /> ().also { + whenever(it.getAll()).thenReturn(emptyList()) + whenever(it.getSharedLayersDirPath()).thenReturn(sharedLayersDirPath) + whenever(it.getProjectLayersDirPath()).thenReturn(projectLayersDirPath) + } + private val settingsProvider = InMemSettingsProvider() @get:Rule val fragmentScenarioLauncherRule = FragmentScenarioLauncherRule( FragmentFactoryBuilder() .forClass(OfflineMapLayersImporter::class) { - OfflineMapLayersImporter(scheduler, sharedLayersDirPath, projectLayersDirPath) + OfflineMapLayersImporter(referenceLayerRepository, scheduler, settingsProvider, sharedLayersDirPath, projectLayersDirPath) }.build() ) @Test fun `clicking the 'cancel' button dismisses the dialog`() { - launchFragment(arrayListOf()).onFragment { + launchFragment().onFragment { scheduler.flush() assertThat(it.isVisible, equalTo(true)) onView(withText(R.string.cancel)).perform(click()) @@ -56,83 +62,39 @@ class OfflineMapLayersImporterTest { } @Test - fun `clicking the 'cancel' button does not set fragment result`() { - launchFragment(arrayListOf()).onFragment { - var resultReceived = false - it.parentFragmentManager.setFragmentResultListener( - OfflineMapLayersImporter.RESULT_KEY, - it - ) { _, _ -> - resultReceived = true - } - - onView(withId(org.odk.collect.maps.R.id.cancel_button)).perform(click()) - assertThat(resultReceived, equalTo(false)) - } - } - - @Test - fun `clicking the 'add layer' button displays progress indicator before adding layers and dismisses the dialog after`() { - launchFragment(arrayListOf()).onFragment { + fun `clicking the 'add layer' button dismisses the dialog`() { + launchFragment().onFragment { scheduler.flush() assertThat(it.isVisible, equalTo(true)) + it.viewModel.loadLayersToImport(emptyList()) onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) - assertThat( - it.childFragmentManager.findFragmentByTag(MaterialProgressDialogFragment::class.java.name), - instanceOf(MaterialProgressDialogFragment::class.java) - ) scheduler.flush() RobolectricHelpers.runLooper() assertThat(it.isVisible, equalTo(false)) } } - @Test - fun `clicking the 'add layer' button sets fragment result`() { - launchFragment(arrayListOf()).onFragment { - scheduler.flush() - var resultReceived = false - it.parentFragmentManager.setFragmentResultListener( - OfflineMapLayersImporter.RESULT_KEY, - it - ) { _, _ -> - resultReceived = true - } - - onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) - scheduler.flush() - RobolectricHelpers.runLooper() - assertThat(resultReceived, equalTo(true)) - } - } - @Test fun `progress indicator is displayed during loading layers`() { val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) - val scenario = launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) - - scenario.onFragment { - assertThat( - it.childFragmentManager.findFragmentByTag(MaterialProgressDialogFragment::class.java.name), - instanceOf(MaterialProgressDialogFragment::class.java) - ) + launchFragment().onFragment { + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) } + onView(withId(org.odk.collect.maps.R.id.progress_indicator)).check(matches(isDisplayed())) + onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(not(isDisplayed()))) + scheduler.flush() - scenario.onFragment { - assertThat( - it.childFragmentManager.findFragmentByTag(MaterialProgressDialogFragment::class.java.name), - equalTo(null) - ) - } + onView(withId(org.odk.collect.maps.R.id.progress_indicator)).check(matches(not(isDisplayed()))) + onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(isDisplayed())) } @Test fun `the 'cancel' button is enabled during loading layers`() { - launchFragment(arrayListOf()) + launchFragment() onView(withId(org.odk.collect.maps.R.id.cancel_button)).check(matches(isEnabled())) scheduler.flush() @@ -141,7 +103,7 @@ class OfflineMapLayersImporterTest { @Test fun `the 'add layer' button is disabled during loading layers`() { - launchFragment(arrayListOf()) + launchFragment() onView(withId(org.odk.collect.maps.R.id.add_layer_button)).check(matches(not(isEnabled()))) scheduler.flush() @@ -150,7 +112,7 @@ class OfflineMapLayersImporterTest { @Test fun `'All projects' location should be selected by default`() { - launchFragment(arrayListOf()) + launchFragment() onView(withId(org.odk.collect.maps.R.id.all_projects_option)).check(matches(isChecked())) onView(withId(org.odk.collect.maps.R.id.current_project_option)).check(matches(not(isChecked()))) @@ -158,7 +120,7 @@ class OfflineMapLayersImporterTest { @Test fun `checking location sets selection correctly`() { - launchFragment(arrayListOf()) + launchFragment() scheduler.flush() onView(withId(org.odk.collect.maps.R.id.current_project_option)).perform(click()) @@ -174,7 +136,7 @@ class OfflineMapLayersImporterTest { @Test fun `recreating maintains the selected layers location`() { - val scenario = launchFragment(arrayListOf()) + val scenario = launchFragment() scheduler.flush() onView(withId(org.odk.collect.maps.R.id.current_project_option)).perform(click()) @@ -190,7 +152,9 @@ class OfflineMapLayersImporterTest { val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) - launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + launchFragment().onFragment { + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + } scheduler.flush() @@ -204,7 +168,10 @@ class OfflineMapLayersImporterTest { val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) - val scenario = launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + val scenario = launchFragment().onFragment { + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + } + scheduler.flush() scenario.recreate() @@ -219,7 +186,10 @@ class OfflineMapLayersImporterTest { val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) val file2 = TempFiles.createTempFile("layer2", ".txt") - launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + launchFragment().onFragment { + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + } + scheduler.flush() onView(withId(org.odk.collect.maps.R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(1))) @@ -236,7 +206,10 @@ class OfflineMapLayersImporterTest { it.writeText("blah2") } - launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + launchFragment().onFragment { + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + } + scheduler.flush() onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) @@ -263,7 +236,10 @@ class OfflineMapLayersImporterTest { it.writeText("blah2") } - launchFragment(arrayListOf(file1.toUri().toString(), file2.toUri().toString())) + launchFragment().onFragment { + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + } + scheduler.flush() onView(withId(org.odk.collect.maps.R.id.current_project_option)).perform(scrollTo(), click()) @@ -283,10 +259,7 @@ class OfflineMapLayersImporterTest { assertThat(copiedFile2.readText(), equalTo("blah2")) } - private fun launchFragment(uris: ArrayList): FragmentScenario { - return fragmentScenarioLauncherRule.launchInContainer( - OfflineMapLayersImporter::class.java, - Bundle().apply { putStringArrayList(OfflineMapLayersImporter.URIS, uris) } - ) + private fun launchFragment(): FragmentScenario { + return fragmentScenarioLauncherRule.launchInContainer(OfflineMapLayersImporter::class.java) } } diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt index 7d523a15226..2c3e0cdebb4 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt @@ -5,11 +5,12 @@ import androidx.activity.result.ActivityResultRegistry import androidx.activity.result.contract.ActivityResultContract import androidx.activity.result.contract.ActivityResultContracts import androidx.core.app.ActivityOptionsCompat -import androidx.core.os.bundleOf +import androidx.core.net.toUri import androidx.fragment.app.testing.FragmentScenario import androidx.test.espresso.Espresso.onView import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.assertion.ViewAssertions.matches +import androidx.test.espresso.matcher.RootMatchers.isDialog import androidx.test.espresso.matcher.ViewMatchers.assertThat import androidx.test.espresso.matcher.ViewMatchers.isChecked import androidx.test.espresso.matcher.ViewMatchers.isDisplayed @@ -45,8 +46,8 @@ import org.odk.collect.webpage.ExternalWebPageHelper class OfflineMapLayersPickerTest { private val referenceLayerRepository = mock().also { whenever(it.getAll()).thenReturn(emptyList()) - whenever(it.getSharedLayersDirPath()).thenReturn("") - whenever(it.getProjectLayersDirPath()).thenReturn("") + whenever(it.getSharedLayersDirPath()).thenReturn(TempFiles.createTempDir().absolutePath) + whenever(it.getProjectLayersDirPath()).thenReturn(TempFiles.createTempDir().absolutePath) } private val scheduler = FakeScheduler() private val settingsProvider = InMemSettingsProvider() @@ -76,7 +77,7 @@ class OfflineMapLayersPickerTest { @Test fun `clicking the 'cancel' button dismisses the layers picker`() { - val scenario = launchFragment() + val scenario = launchOfflineMapLayersPicker() scenario.onFragment { assertThat(it.isVisible, equalTo(true)) @@ -91,7 +92,7 @@ class OfflineMapLayersPickerTest { ReferenceLayer("1", TempFiles.createTempFile(), "layer1") )) - launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() @@ -101,14 +102,14 @@ class OfflineMapLayersPickerTest { @Test fun `the 'cancel' button should be enabled during loading layers`() { - launchFragment() + launchOfflineMapLayersPicker() onView(withText(string.cancel)).check(matches(isEnabled())) } @Test fun `clicking the 'save' button dismisses the layers picker`() { - val scenario = launchFragment() + val scenario = launchOfflineMapLayersPicker() scheduler.flush() @@ -121,7 +122,7 @@ class OfflineMapLayersPickerTest { @Test fun `the 'save' button should be disabled during loading layers`() { - launchFragment() + launchOfflineMapLayersPicker() onView(withText(string.save)).check(matches(not(isEnabled()))) scheduler.flush() @@ -134,7 +135,7 @@ class OfflineMapLayersPickerTest { ReferenceLayer("1", TempFiles.createTempFile(), "layer1") )) - launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() @@ -148,7 +149,7 @@ class OfflineMapLayersPickerTest { ReferenceLayer("1", TempFiles.createTempFile(), "layer1") )) - launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() @@ -163,7 +164,7 @@ class OfflineMapLayersPickerTest { ReferenceLayer("1", TempFiles.createTempFile(), "layer1") )) - launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() @@ -179,7 +180,7 @@ class OfflineMapLayersPickerTest { )) settingsProvider.getUnprotectedSettings().save(ProjectKeys.KEY_REFERENCE_LAYER, "2") - launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() @@ -190,7 +191,7 @@ class OfflineMapLayersPickerTest { @Test fun `progress indicator is displayed during loading layers`() { - launchFragment() + launchOfflineMapLayersPicker() onView(withId(R.id.progress_indicator)).check(matches(isDisplayed())) onView(withId(R.id.layers)).check(matches(not(isDisplayed()))) @@ -203,14 +204,14 @@ class OfflineMapLayersPickerTest { @Test fun `the 'learn more' button should be enabled during loading layers`() { - launchFragment() + launchOfflineMapLayersPicker() onView(withText(string.get_help_with_reference_layers)).check(matches(isEnabled())) } @Test fun `clicking the 'learn more' button opens the forum thread`() { - launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() @@ -221,7 +222,7 @@ class OfflineMapLayersPickerTest { @Test fun `if there are no layers the 'none' option is displayed`() { - launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() @@ -236,7 +237,7 @@ class OfflineMapLayersPickerTest { ReferenceLayer("2", TempFiles.createTempFile(), "layer2") )) - launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() @@ -252,7 +253,7 @@ class OfflineMapLayersPickerTest { ReferenceLayer("1", TempFiles.createTempFile(), "layer1") )) - launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() @@ -271,7 +272,7 @@ class OfflineMapLayersPickerTest { ReferenceLayer("1", TempFiles.createTempFile(), "layer1") )) - val scenario = launchFragment() + val scenario = launchOfflineMapLayersPicker() scheduler.flush() @@ -283,7 +284,7 @@ class OfflineMapLayersPickerTest { @Test fun `clicking the 'add layer' and selecting layers displays the confirmation dialog`() { - val scenario = launchFragment() + val scenario = launchOfflineMapLayersPicker() uris.add(Uri.parse("blah")) onView(withText(string.add_layer)).perform(click()) @@ -298,7 +299,7 @@ class OfflineMapLayersPickerTest { @Test fun `clicking the 'add layer' and selecting nothing does not display the confirmation dialog`() { - val scenario = launchFragment() + val scenario = launchOfflineMapLayersPicker() onView(withText(string.add_layer)).perform(click()) @@ -312,53 +313,57 @@ class OfflineMapLayersPickerTest { @Test fun `progress indicator is displayed during loading layers after receiving new ones`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1") - )) - val scenario = launchFragment() + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) + val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) + + launchOfflineMapLayersPicker() scheduler.flush() - scenario.onFragment { - it.childFragmentManager.setFragmentResult(OfflineMapLayersImporter.RESULT_KEY, bundleOf()) - } + uris.add(file1.toUri()) + uris.add(file2.toUri()) + + onView(withText(string.add_layer)).perform(click()) + scheduler.flush() + onView(withId(R.id.add_layer_button)).inRoot(isDialog()).perform(click()) onView(withId(R.id.progress_indicator)).check(matches(isDisplayed())) onView(withId(R.id.layers)).check(matches(not(isDisplayed()))) scheduler.flush() + onView(withId(R.id.progress_indicator)).check(matches(not(isDisplayed()))) onView(withId(R.id.layers)).check(matches(isDisplayed())) } @Test fun `when new layers added the list should be updated`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1") - )) + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) + val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) - val scenario = launchFragment() + launchOfflineMapLayersPicker() scheduler.flush() + uris.add(file1.toUri()) + uris.add(file2.toUri()) + + onView(withText(string.add_layer)).perform(click()) + scheduler.flush() + onView(withId(R.id.add_layer_button)).inRoot(isDialog()).perform(click()) whenever(referenceLayerRepository.getAll()).thenReturn(listOf( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1"), - ReferenceLayer("2", TempFiles.createTempFile(), "layer2") + ReferenceLayer("1", TempFiles.createTempFile(), file1.name), + ReferenceLayer("1", TempFiles.createTempFile(), file2.name) )) - - scenario.onFragment { - it.childFragmentManager.setFragmentResult(OfflineMapLayersImporter.RESULT_KEY, bundleOf()) - } - scheduler.flush() onView(withId(R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(3))) onView(withRecyclerView(R.id.layers).atPositionOnView(0, R.id.radio_button)).check(matches(withText(string.none))) - onView(withRecyclerView(R.id.layers).atPositionOnView(1, R.id.radio_button)).check(matches(withText("layer1"))) - onView(withRecyclerView(R.id.layers).atPositionOnView(2, R.id.radio_button)).check(matches(withText("layer2"))) + onView(withRecyclerView(R.id.layers).atPositionOnView(1, R.id.radio_button)).check(matches(withText(file1.name))) + onView(withRecyclerView(R.id.layers).atPositionOnView(2, R.id.radio_button)).check(matches(withText(file2.name))) } - private fun launchFragment(): FragmentScenario { + private fun launchOfflineMapLayersPicker(): FragmentScenario { return fragmentScenarioLauncherRule.launchInContainer(OfflineMapLayersPicker::class.java) } } From 3cb509accae85fcb1a9afd6ffc6833e5a2366640 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Wed, 5 Jun 2024 22:58:49 +0200 Subject: [PATCH 19/28] Improved tests --- .../maps/layers/OfflineMapLayersImporter.kt | 8 +-- .../maps/layers/OfflineMapLayersPicker.kt | 8 +-- .../layers/OfflineMapLayersImporterTest.kt | 31 ++++------ .../maps/layers/OfflineMapLayersPickerTest.kt | 58 +++++++++---------- .../layers/TestReferenceLayerRepository.kt | 32 ++++++++++ 5 files changed, 75 insertions(+), 62 deletions(-) create mode 100644 maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt index bef347ec9e6..ce2ce693339 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt @@ -16,9 +16,7 @@ import org.odk.collect.settings.SettingsProvider class OfflineMapLayersImporter( private val referenceLayerRepository: ReferenceLayerRepository, private val scheduler: Scheduler, - private val settingsProvider: SettingsProvider, - private val sharedLayersDirPath: String, - private val projectLayersDirPath: String + private val settingsProvider: SettingsProvider ) : MaterialFullScreenDialogFragment() { val viewModel: OfflineMapLayersViewModel by activityViewModels { object : ViewModelProvider.Factory { @@ -43,9 +41,9 @@ class OfflineMapLayersImporter( binding.addLayerButton.setOnClickListener { val layersDir = if (binding.allProjectsOption.isChecked) { - sharedLayersDirPath + referenceLayerRepository.getSharedLayersDirPath() } else { - projectLayersDirPath + referenceLayerRepository.getProjectLayersDirPath() } viewModel.importNewLayers(layersDir) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index ed7439bf144..739e1c932df 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -50,13 +50,7 @@ class OfflineMapLayersPicker( override fun onCreate(savedInstanceState: Bundle?) { childFragmentManager.fragmentFactory = FragmentFactoryBuilder() .forClass(OfflineMapLayersImporter::class) { - OfflineMapLayersImporter( - referenceLayerRepository, - scheduler, - settingsProvider, - referenceLayerRepository.getSharedLayersDirPath(), - referenceLayerRepository.getProjectLayersDirPath() - ) + OfflineMapLayersImporter(referenceLayerRepository, scheduler, settingsProvider) } .build() diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt index 756503dcd35..a1ae2b54cfa 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt @@ -19,13 +19,12 @@ import org.hamcrest.MatcherAssert.assertThat import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.mockito.kotlin.mock -import org.mockito.kotlin.whenever import org.odk.collect.androidshared.ui.FragmentFactoryBuilder import org.odk.collect.fragmentstest.FragmentScenarioLauncherRule import org.odk.collect.settings.InMemSettingsProvider import org.odk.collect.shared.TempFiles import org.odk.collect.strings.R +import org.odk.collect.testshared.EspressoHelpers import org.odk.collect.testshared.FakeScheduler import org.odk.collect.testshared.RecyclerViewMatcher import org.odk.collect.testshared.RobolectricHelpers @@ -34,20 +33,14 @@ import java.io.File @RunWith(AndroidJUnit4::class) class OfflineMapLayersImporterTest { private val scheduler = FakeScheduler() - private val sharedLayersDirPath = TempFiles.createTempDir().absolutePath - private val projectLayersDirPath = TempFiles.createTempDir().absolutePath - private val referenceLayerRepository = mock().also { - whenever(it.getAll()).thenReturn(emptyList()) - whenever(it.getSharedLayersDirPath()).thenReturn(sharedLayersDirPath) - whenever(it.getProjectLayersDirPath()).thenReturn(projectLayersDirPath) - } + private val referenceLayerRepository = TestReferenceLayerRepository() private val settingsProvider = InMemSettingsProvider() @get:Rule val fragmentScenarioLauncherRule = FragmentScenarioLauncherRule( FragmentFactoryBuilder() .forClass(OfflineMapLayersImporter::class) { - OfflineMapLayersImporter(referenceLayerRepository, scheduler, settingsProvider, sharedLayersDirPath, projectLayersDirPath) + OfflineMapLayersImporter(referenceLayerRepository, scheduler, settingsProvider) }.build() ) @@ -56,7 +49,7 @@ class OfflineMapLayersImporterTest { launchFragment().onFragment { scheduler.flush() assertThat(it.isVisible, equalTo(true)) - onView(withText(R.string.cancel)).perform(click()) + EspressoHelpers.clickOnText(R.string.cancel) assertThat(it.isVisible, equalTo(false)) } } @@ -215,14 +208,14 @@ class OfflineMapLayersImporterTest { onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) scheduler.flush() - assertThat(File(sharedLayersDirPath).listFiles().size, equalTo(2)) - assertThat(File(projectLayersDirPath).listFiles().size, equalTo(0)) + assertThat(File(referenceLayerRepository.getSharedLayersDirPath()).listFiles().size, equalTo(2)) + assertThat(File(referenceLayerRepository.getProjectLayersDirPath()).listFiles().size, equalTo(0)) - val copiedFile1 = File(sharedLayersDirPath, file1.name) + val copiedFile1 = File(referenceLayerRepository.getSharedLayersDirPath(), file1.name) assertThat(copiedFile1.exists(), equalTo(true)) assertThat(copiedFile1.readText(), equalTo("blah1")) - val copiedFile2 = File(sharedLayersDirPath, file2.name) + val copiedFile2 = File(referenceLayerRepository.getSharedLayersDirPath(), file2.name) assertThat(copiedFile2.exists(), equalTo(true)) assertThat(copiedFile2.readText(), equalTo("blah2")) } @@ -247,14 +240,14 @@ class OfflineMapLayersImporterTest { onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) scheduler.flush() - assertThat(File(sharedLayersDirPath).listFiles().size, equalTo(0)) - assertThat(File(projectLayersDirPath).listFiles().size, equalTo(2)) + assertThat(File(referenceLayerRepository.getSharedLayersDirPath()).listFiles().size, equalTo(0)) + assertThat(File(referenceLayerRepository.getProjectLayersDirPath()).listFiles().size, equalTo(2)) - val copiedFile1 = File(projectLayersDirPath, file1.name) + val copiedFile1 = File(referenceLayerRepository.getProjectLayersDirPath(), file1.name) assertThat(copiedFile1.exists(), equalTo(true)) assertThat(copiedFile1.readText(), equalTo("blah1")) - val copiedFile2 = File(projectLayersDirPath, file2.name) + val copiedFile2 = File(referenceLayerRepository.getProjectLayersDirPath(), file2.name) assertThat(copiedFile2.exists(), equalTo(true)) assertThat(copiedFile2.readText(), equalTo("blah2")) } diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt index 2c3e0cdebb4..655941a0fad 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt @@ -28,7 +28,6 @@ import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever import org.odk.collect.androidshared.ui.FragmentFactoryBuilder import org.odk.collect.fragmentstest.FragmentScenarioLauncherRule import org.odk.collect.maps.R @@ -44,11 +43,7 @@ import org.odk.collect.webpage.ExternalWebPageHelper @RunWith(AndroidJUnit4::class) class OfflineMapLayersPickerTest { - private val referenceLayerRepository = mock().also { - whenever(it.getAll()).thenReturn(emptyList()) - whenever(it.getSharedLayersDirPath()).thenReturn(TempFiles.createTempDir().absolutePath) - whenever(it.getProjectLayersDirPath()).thenReturn(TempFiles.createTempDir().absolutePath) - } + private val referenceLayerRepository = TestReferenceLayerRepository() private val scheduler = FakeScheduler() private val settingsProvider = InMemSettingsProvider() private val externalWebPageHelper = mock() @@ -88,9 +83,9 @@ class OfflineMapLayersPickerTest { @Test fun `clicking the 'cancel' button does not save the layer`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + referenceLayerRepository.addLayers( ReferenceLayer("1", TempFiles.createTempFile(), "layer1") - )) + ) launchOfflineMapLayersPicker() @@ -131,9 +126,9 @@ class OfflineMapLayersPickerTest { @Test fun `clicking the 'save' button saves null when 'None' option is checked`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + referenceLayerRepository.addLayers( ReferenceLayer("1", TempFiles.createTempFile(), "layer1") - )) + ) launchOfflineMapLayersPicker() @@ -145,9 +140,9 @@ class OfflineMapLayersPickerTest { @Test fun `clicking the 'save' button saves the layer id if any is checked`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + referenceLayerRepository.addLayers( ReferenceLayer("1", TempFiles.createTempFile(), "layer1") - )) + ) launchOfflineMapLayersPicker() @@ -160,9 +155,9 @@ class OfflineMapLayersPickerTest { @Test fun `when no layer id is saved in settings the 'None' option should be checked`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + referenceLayerRepository.addLayers( ReferenceLayer("1", TempFiles.createTempFile(), "layer1") - )) + ) launchOfflineMapLayersPicker() @@ -174,10 +169,11 @@ class OfflineMapLayersPickerTest { @Test fun `when layer id is saved in settings the layer it belongs to should be checked`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + referenceLayerRepository.addLayers( ReferenceLayer("1", TempFiles.createTempFile(), "layer1"), ReferenceLayer("2", TempFiles.createTempFile(), "layer2") - )) + ) + settingsProvider.getUnprotectedSettings().save(ProjectKeys.KEY_REFERENCE_LAYER, "2") launchOfflineMapLayersPicker() @@ -215,7 +211,7 @@ class OfflineMapLayersPickerTest { scheduler.flush() - onView(withText(string.get_help_with_reference_layers)).perform(click()) + EspressoHelpers.clickOnText(string.get_help_with_reference_layers) verify(externalWebPageHelper).openWebPageInCustomTab(any(), eq(Uri.parse("https://docs.getodk.org/collect-offline-maps/#transferring-offline-tilesets-to-devices"))) } @@ -232,10 +228,10 @@ class OfflineMapLayersPickerTest { @Test fun `if there are multiple layers all of them are displayed along with the 'None'`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + referenceLayerRepository.addLayers( ReferenceLayer("1", TempFiles.createTempFile(), "layer1"), ReferenceLayer("2", TempFiles.createTempFile(), "layer2") - )) + ) launchOfflineMapLayersPicker() @@ -249,9 +245,9 @@ class OfflineMapLayersPickerTest { @Test fun `checking layers sets selection correctly`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + referenceLayerRepository.addLayers( ReferenceLayer("1", TempFiles.createTempFile(), "layer1") - )) + ) launchOfflineMapLayersPicker() @@ -268,9 +264,9 @@ class OfflineMapLayersPickerTest { @Test fun `recreating maintains selection`() { - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( + referenceLayerRepository.addLayers( ReferenceLayer("1", TempFiles.createTempFile(), "layer1") - )) + ) val scenario = launchOfflineMapLayersPicker() @@ -287,7 +283,7 @@ class OfflineMapLayersPickerTest { val scenario = launchOfflineMapLayersPicker() uris.add(Uri.parse("blah")) - onView(withText(string.add_layer)).perform(click()) + EspressoHelpers.clickOnText(string.add_layer) scenario.onFragment { assertThat( @@ -301,7 +297,7 @@ class OfflineMapLayersPickerTest { fun `clicking the 'add layer' and selecting nothing does not display the confirmation dialog`() { val scenario = launchOfflineMapLayersPicker() - onView(withText(string.add_layer)).perform(click()) + EspressoHelpers.clickOnText(string.add_layer) scenario.onFragment { assertThat( @@ -323,7 +319,7 @@ class OfflineMapLayersPickerTest { uris.add(file1.toUri()) uris.add(file2.toUri()) - onView(withText(string.add_layer)).perform(click()) + EspressoHelpers.clickOnText(string.add_layer) scheduler.flush() onView(withId(R.id.add_layer_button)).inRoot(isDialog()).perform(click()) @@ -348,13 +344,13 @@ class OfflineMapLayersPickerTest { uris.add(file1.toUri()) uris.add(file2.toUri()) - onView(withText(string.add_layer)).perform(click()) + EspressoHelpers.clickOnText(string.add_layer) scheduler.flush() onView(withId(R.id.add_layer_button)).inRoot(isDialog()).perform(click()) - whenever(referenceLayerRepository.getAll()).thenReturn(listOf( - ReferenceLayer("1", TempFiles.createTempFile(), file1.name), - ReferenceLayer("1", TempFiles.createTempFile(), file2.name) - )) + referenceLayerRepository.addLayers( + ReferenceLayer("1", file1, file1.name), + ReferenceLayer("2", file2, file2.name) + ) scheduler.flush() onView(withId(R.id.layers)).check(matches(RecyclerViewMatcher.withListSize(3))) diff --git a/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt b/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt new file mode 100644 index 00000000000..2226f0c854c --- /dev/null +++ b/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt @@ -0,0 +1,32 @@ +package org.odk.collect.maps.layers + +import org.odk.collect.shared.TempFiles + +class TestReferenceLayerRepository : ReferenceLayerRepository { + private val sharedLayersDirPath: String = TempFiles.createTempDir().absolutePath + private val projectLayersDirPath: String = TempFiles.createTempDir().absolutePath + + private val layers = mutableListOf() + + override fun getAll(): List { + return layers + } + + override fun get(id: String): ReferenceLayer? { + return layers.find { it.id == id } + } + + override fun getSharedLayersDirPath(): String { + return sharedLayersDirPath + } + + override fun getProjectLayersDirPath(): String { + return projectLayersDirPath + } + + fun addLayers(vararg newLayers: ReferenceLayer) { + newLayers.forEach { newLayer -> + layers.add(newLayer) + } + } +} From 3cb44082ca65c7ea9ae6b8a8b2a6efcb78f78a24 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 6 Jun 2024 17:12:04 +0200 Subject: [PATCH 20/28] Naming improvements --- .../main/java/org/odk/collect/androidshared/system/UriExt.kt | 2 +- .../org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt b/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt index 99cc1709c57..5d8701ee35e 100644 --- a/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt +++ b/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt @@ -8,7 +8,7 @@ import java.io.File import java.io.FileOutputStream import java.io.IOException -fun Uri.toFile(contentResolver: ContentResolver, dest: File) { +fun Uri.copyToFile(contentResolver: ContentResolver, dest: File) { try { contentResolver.openInputStream(this)?.use { inputStream -> FileOutputStream(dest).use { outputStream -> 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 index 0b632059048..40b5e366204 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt @@ -5,8 +5,8 @@ import android.net.Uri import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel +import org.odk.collect.androidshared.system.copyToFile import org.odk.collect.androidshared.system.getFileName -import org.odk.collect.androidshared.system.toFile import org.odk.collect.async.Scheduler import org.odk.collect.settings.SettingsProvider import org.odk.collect.settings.keys.ProjectKeys @@ -61,7 +61,7 @@ class OfflineMapLayersViewModel( uri.getFileName(contentResolver)?.let { fileName -> if (fileName.endsWith(MbtilesFile.FILE_EXTENSION)) { val layerFile = File(tempLayersDir, fileName).also { file -> - uri.toFile(contentResolver, file) + uri.copyToFile(contentResolver, file) } layers.add(ReferenceLayer(layerFile.absolutePath, layerFile, MbtilesFile.readName(layerFile) ?: layerFile.name)) } From 5881d1543d40ecc1c086026e119597df0f230f5d Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 6 Jun 2024 17:13:11 +0200 Subject: [PATCH 21/28] Ignore exceptions in the copyToFile method --- .../java/org/odk/collect/androidshared/system/UriExt.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt b/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt index 5d8701ee35e..11092645aac 100644 --- a/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt +++ b/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt @@ -3,10 +3,8 @@ package org.odk.collect.androidshared.system import android.content.ContentResolver import android.net.Uri import android.provider.OpenableColumns -import timber.log.Timber import java.io.File import java.io.FileOutputStream -import java.io.IOException fun Uri.copyToFile(contentResolver: ContentResolver, dest: File) { try { @@ -15,8 +13,8 @@ fun Uri.copyToFile(contentResolver: ContentResolver, dest: File) { inputStream.copyTo(outputStream) } } - } catch (e: IOException) { - Timber.e(e) + } catch (e: Exception) { + // ignore } } From cf8566aabcb1b395fe03538b92d3cb8ec6f10c87 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 6 Jun 2024 17:29:45 +0200 Subject: [PATCH 22/28] Removed redundant iconTint --- maps/src/main/res/layout/offline_map_layers_picker.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/maps/src/main/res/layout/offline_map_layers_picker.xml b/maps/src/main/res/layout/offline_map_layers_picker.xml index 5e86a2eeebe..2530b21e876 100644 --- a/maps/src/main/res/layout/offline_map_layers_picker.xml +++ b/maps/src/main/res/layout/offline_map_layers_picker.xml @@ -115,7 +115,6 @@ android:text="@string/add_layer" android:layout_marginBottom="@dimen/margin_extra_small" app:icon="@drawable/ic_add_white_24" - app:iconTint="?colorAccent" app:layout_constraintBottom_toTopOf="@id/bottom_divider" app:layout_constraintStart_toEndOf="@id/guideline_start" /> From beda06b94fcb85364622dd80213a56b376459a31 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 6 Jun 2024 17:33:04 +0200 Subject: [PATCH 23/28] Added more margin between the list of layers and the 'Add layer' button --- maps/src/main/res/layout/offline_map_layers_picker.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/maps/src/main/res/layout/offline_map_layers_picker.xml b/maps/src/main/res/layout/offline_map_layers_picker.xml index 2530b21e876..44c525d0d96 100644 --- a/maps/src/main/res/layout/offline_map_layers_picker.xml +++ b/maps/src/main/res/layout/offline_map_layers_picker.xml @@ -90,6 +90,7 @@ android:layout_width="0dp" android:layout_height="0dp" app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager" + android:layout_marginBottom="@dimen/margin_extra_small" app:layout_constraintStart_toStartOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintBottom_toTopOf="@id/add_layer" From 4bf56609a47ebce419d985ecccb0f4eec91e48cc Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 6 Jun 2024 17:35:05 +0200 Subject: [PATCH 24/28] Added a new line at the end of the file --- maps/src/main/res/layout/offline_map_layers_picker.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maps/src/main/res/layout/offline_map_layers_picker.xml b/maps/src/main/res/layout/offline_map_layers_picker.xml index 44c525d0d96..2c407c36d13 100644 --- a/maps/src/main/res/layout/offline_map_layers_picker.xml +++ b/maps/src/main/res/layout/offline_map_layers_picker.xml @@ -148,4 +148,4 @@ app:layout_constraintBottom_toBottomOf="@id/save" app:layout_constraintEnd_toStartOf="@id/save" app:layout_constraintTop_toTopOf="@id/save" /> - \ No newline at end of file + From f3b8bc4a51b128a5b946b0ca97d3c21559804918 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 6 Jun 2024 19:19:12 +0200 Subject: [PATCH 25/28] Do not expose layer dirs in repository --- .../DirectoryReferenceLayerRepository.kt | 12 ++--- .../maps/layers/OfflineMapLayersImporter.kt | 8 +-- .../maps/layers/OfflineMapLayersViewModel.kt | 5 +- .../maps/layers/ReferenceLayerRepository.kt | 4 +- .../DirectoryReferenceLayerRepositoryTest.kt | 42 +++++++++++++++ .../layers/OfflineMapLayersImporterTest.kt | 54 ++++++++----------- .../layers/TestReferenceLayerRepository.kt | 13 ++--- 7 files changed, 78 insertions(+), 60 deletions(-) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt b/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt index 627f635882d..384385ffb35 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepository.kt @@ -28,12 +28,12 @@ class DirectoryReferenceLayerRepository( } } - override fun getSharedLayersDirPath(): String { - return sharedLayersDirPath - } - - override fun getProjectLayersDirPath(): String { - return projectLayersDirPath + override fun addLayer(file: File, shared: Boolean) { + if (shared) { + file.copyTo(File(sharedLayersDirPath, file.name), true) + } else { + file.copyTo(File(projectLayersDirPath, file.name), true) + } } private fun getAllFilesWithDirectory() = listOf(sharedLayersDirPath, projectLayersDirPath).flatMap { dir -> diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt index ce2ce693339..0066f79bc7f 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt @@ -40,13 +40,7 @@ class OfflineMapLayersImporter( } binding.addLayerButton.setOnClickListener { - val layersDir = if (binding.allProjectsOption.isChecked) { - referenceLayerRepository.getSharedLayersDirPath() - } else { - referenceLayerRepository.getProjectLayersDirPath() - } - - viewModel.importNewLayers(layersDir) + viewModel.importNewLayers(binding.allProjectsOption.isChecked) dismiss() } return binding.root 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 index 40b5e366204..e58bcce56af 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt @@ -74,13 +74,12 @@ class OfflineMapLayersViewModel( ) } - fun importNewLayers(layersDir: String) { + fun importNewLayers(shared: Boolean) { _isLoading.value = true scheduler.immediate( background = { - val destDir = File(layersDir) tempLayersDir.listFiles()?.forEach { - it.copyTo(File(destDir, it.name), true) + referenceLayerRepository.addLayer(it, shared) } tempLayersDir.delete() _isLoading.postValue(false) diff --git a/maps/src/main/java/org/odk/collect/maps/layers/ReferenceLayerRepository.kt b/maps/src/main/java/org/odk/collect/maps/layers/ReferenceLayerRepository.kt index fa981fd7cc2..f8dadbaa540 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/ReferenceLayerRepository.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/ReferenceLayerRepository.kt @@ -6,9 +6,7 @@ interface ReferenceLayerRepository { fun getAll(): List fun get(id: String): ReferenceLayer? - - fun getSharedLayersDirPath(): String - fun getProjectLayersDirPath(): String + fun addLayer(file: File, shared: Boolean) } data class ReferenceLayer(val id: String, val file: File, val name: String) diff --git a/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt index 154c8b6a840..10aa5d8513e 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt @@ -164,6 +164,48 @@ class DirectoryReferenceLayerRepositoryTest { assertThat(repository.get(fileLayer.id)!!.name, equalTo(file.name)) } + @Test + fun addLayer_movesFileToTheSharedLayersDir_whenSharedIsTrue() { + val sharedLayersDir = TempFiles.createTempDir() + val projectLayersDir = TempFiles.createTempDir() + val file = TempFiles.createTempFile().also { + it.writeText("blah") + } + + val repository = DirectoryReferenceLayerRepository( + sharedLayersDir.absolutePath, + projectLayersDir.absolutePath + ) { StubMapConfigurator() } + + repository.addLayer(file, true) + + assertThat(sharedLayersDir.listFiles().size, equalTo(1)) + assertThat(sharedLayersDir.listFiles()[0].name, equalTo(file.name)) + assertThat(sharedLayersDir.listFiles()[0].readText(), equalTo("blah")) + assertThat(projectLayersDir.listFiles().size, equalTo(0)) + } + + @Test + fun addLayer_movesFileToTheProjectLayersDir_whenSharedIsFalse() { + val sharedLayersDir = TempFiles.createTempDir() + val projectLayersDir = TempFiles.createTempDir() + val file = TempFiles.createTempFile().also { + it.writeText("blah") + } + + val repository = DirectoryReferenceLayerRepository( + sharedLayersDir.absolutePath, + projectLayersDir.absolutePath + ) { StubMapConfigurator() } + + repository.addLayer(file, false) + + assertThat(sharedLayersDir.listFiles().size, equalTo(0)) + assertThat(projectLayersDir.listFiles().size, equalTo(1)) + assertThat(projectLayersDir.listFiles()[0].name, equalTo(file.name)) + assertThat(projectLayersDir.listFiles()[0].readText(), equalTo("blah")) + } + private class StubMapConfigurator : MapConfigurator { private val files = mutableMapOf>() diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt index a1ae2b54cfa..ef76e2238c3 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt @@ -19,6 +19,10 @@ import org.hamcrest.MatcherAssert.assertThat import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify import org.odk.collect.androidshared.ui.FragmentFactoryBuilder import org.odk.collect.fragmentstest.FragmentScenarioLauncherRule import org.odk.collect.settings.InMemSettingsProvider @@ -33,7 +37,7 @@ import java.io.File @RunWith(AndroidJUnit4::class) class OfflineMapLayersImporterTest { private val scheduler = FakeScheduler() - private val referenceLayerRepository = TestReferenceLayerRepository() + private val referenceLayerRepository = mock() private val settingsProvider = InMemSettingsProvider() @get:Rule @@ -192,12 +196,8 @@ class OfflineMapLayersImporterTest { @Test fun `clicking the 'add layer' button moves the files to the shared layers dir if it is selected`() { - val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION).also { - it.writeText("blah1") - } - val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION).also { - it.writeText("blah2") - } + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) + val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) launchFragment().onFragment { it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) @@ -208,26 +208,20 @@ class OfflineMapLayersImporterTest { onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) scheduler.flush() - assertThat(File(referenceLayerRepository.getSharedLayersDirPath()).listFiles().size, equalTo(2)) - assertThat(File(referenceLayerRepository.getProjectLayersDirPath()).listFiles().size, equalTo(0)) - - val copiedFile1 = File(referenceLayerRepository.getSharedLayersDirPath(), file1.name) - assertThat(copiedFile1.exists(), equalTo(true)) - assertThat(copiedFile1.readText(), equalTo("blah1")) + val fileCaptor = argumentCaptor() + val booleanCaptor = argumentCaptor() - val copiedFile2 = File(referenceLayerRepository.getSharedLayersDirPath(), file2.name) - assertThat(copiedFile2.exists(), equalTo(true)) - assertThat(copiedFile2.readText(), equalTo("blah2")) + verify(referenceLayerRepository, times(2)).addLayer(fileCaptor.capture(), booleanCaptor.capture()) + assertThat(fileCaptor.allValues.any { file -> file.name == file1.name }, equalTo(true)) + assertThat(fileCaptor.allValues.any { file -> file.name == file2.name }, equalTo(true)) + assertThat(booleanCaptor.firstValue, equalTo(true)) + assertThat(booleanCaptor.secondValue, equalTo(true)) } @Test fun `clicking the 'add layer' button moves the files to the project layers dir if it is selected`() { - val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION).also { - it.writeText("blah1") - } - val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION).also { - it.writeText("blah2") - } + val file1 = TempFiles.createTempFile("layer1", MbtilesFile.FILE_EXTENSION) + val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) launchFragment().onFragment { it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) @@ -240,16 +234,14 @@ class OfflineMapLayersImporterTest { onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) scheduler.flush() - assertThat(File(referenceLayerRepository.getSharedLayersDirPath()).listFiles().size, equalTo(0)) - assertThat(File(referenceLayerRepository.getProjectLayersDirPath()).listFiles().size, equalTo(2)) - - val copiedFile1 = File(referenceLayerRepository.getProjectLayersDirPath(), file1.name) - assertThat(copiedFile1.exists(), equalTo(true)) - assertThat(copiedFile1.readText(), equalTo("blah1")) + val fileCaptor = argumentCaptor() + val booleanCaptor = argumentCaptor() - val copiedFile2 = File(referenceLayerRepository.getProjectLayersDirPath(), file2.name) - assertThat(copiedFile2.exists(), equalTo(true)) - assertThat(copiedFile2.readText(), equalTo("blah2")) + verify(referenceLayerRepository, times(2)).addLayer(fileCaptor.capture(), booleanCaptor.capture()) + assertThat(fileCaptor.allValues.any { file -> file.name == file1.name }, equalTo(true)) + assertThat(fileCaptor.allValues.any { file -> file.name == file2.name }, equalTo(true)) + assertThat(booleanCaptor.firstValue, equalTo(false)) + assertThat(booleanCaptor.secondValue, equalTo(false)) } private fun launchFragment(): FragmentScenario { diff --git a/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt b/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt index 2226f0c854c..af87b0c2311 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt @@ -1,11 +1,8 @@ package org.odk.collect.maps.layers -import org.odk.collect.shared.TempFiles +import java.io.File class TestReferenceLayerRepository : ReferenceLayerRepository { - private val sharedLayersDirPath: String = TempFiles.createTempDir().absolutePath - private val projectLayersDirPath: String = TempFiles.createTempDir().absolutePath - private val layers = mutableListOf() override fun getAll(): List { @@ -16,12 +13,8 @@ class TestReferenceLayerRepository : ReferenceLayerRepository { return layers.find { it.id == id } } - override fun getSharedLayersDirPath(): String { - return sharedLayersDirPath - } - - override fun getProjectLayersDirPath(): String { - return projectLayersDirPath + override fun addLayer(file: File, shared: Boolean) { + TODO("Not yet implemented") } fun addLayers(vararg newLayers: ReferenceLayer) { From 6bfa2e1ca57277a1e052174331a24d0d7345f1c0 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 6 Jun 2024 19:44:59 +0200 Subject: [PATCH 26/28] Simplified DirectoryReferenceLayerRepositoryTest --- .../DirectoryReferenceLayerRepositoryTest.kt | 154 +++++++----------- 1 file changed, 62 insertions(+), 92 deletions(-) diff --git a/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt index 10aa5d8513e..e8e52ce54f2 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/DirectoryReferenceLayerRepositoryTest.kt @@ -13,56 +13,56 @@ import org.odk.collect.shared.settings.Settings import java.io.File class DirectoryReferenceLayerRepositoryTest { + private val sharedLayersDir = TempFiles.createTempDir() + private val projectLayersDir = TempFiles.createTempDir() + private val mapConfigurator = StubMapConfigurator() + private val repository = DirectoryReferenceLayerRepository( + sharedLayersDir.absolutePath, + projectLayersDir.absolutePath + ) { mapConfigurator } + @Test fun getAll_returnsAllSupportedLayersInTheDirectory() { - val dir = TempFiles.createTempDir() - val file1 = TempFiles.createTempFile(dir) - val file2 = TempFiles.createTempFile(dir) - val file3 = TempFiles.createTempFile(dir) - val mapConfigurator = StubMapConfigurator().also { - it.addFile(file1, true, file1.name) - it.addFile(file2, false, file2.name) - it.addFile(file3, true, file3.name) + val file1 = TempFiles.createTempFile(sharedLayersDir) + val file2 = TempFiles.createTempFile(sharedLayersDir) + val file3 = TempFiles.createTempFile(sharedLayersDir) + mapConfigurator.apply { + addFile(file1, true, file1.name) + addFile(file2, false, file2.name) + addFile(file3, true, file3.name) } - val repository = DirectoryReferenceLayerRepository(dir.absolutePath, "") { mapConfigurator } assertThat(repository.getAll().map { it.file }, containsInAnyOrder(file1, file3)) } @Test fun getAll_returnsAllSupportedLayersInSubDirectories() { - val dir1 = TempFiles.createTempDir() - val dir2 = TempFiles.createTempDir(dir1) - val file1 = TempFiles.createTempFile(dir2) - val file2 = TempFiles.createTempFile(dir2) - val file3 = TempFiles.createTempFile(dir2) - val mapConfigurator = StubMapConfigurator().also { - it.addFile(file1, true, file1.name) - it.addFile(file2, false, file2.name) - it.addFile(file3, true, file3.name) - } - - val repository = DirectoryReferenceLayerRepository(dir1.absolutePath, "") { mapConfigurator } + val subDir = TempFiles.createTempDir(sharedLayersDir) + val file1 = TempFiles.createTempFile(subDir) + val file2 = TempFiles.createTempFile(subDir) + val file3 = TempFiles.createTempFile(subDir) + mapConfigurator.apply { + addFile(file1, true, file1.name) + addFile(file2, false, file2.name) + addFile(file3, true, file3.name) + } + assertThat(repository.getAll().map { it.file }, containsInAnyOrder(file1, file3)) } @Test fun getAll_withMultipleDirectories_returnsAllSupportedLayersInAllDirectories() { - val dir1 = TempFiles.createTempDir() - val dir2 = TempFiles.createTempDir() - val file1 = TempFiles.createTempFile(dir1) - val file2 = TempFiles.createTempFile(dir1) - val file3 = TempFiles.createTempFile(dir2) - val file4 = TempFiles.createTempFile(dir2) - val mapConfigurator = StubMapConfigurator().also { - it.addFile(file1, true, file1.name) - it.addFile(file2, false, file2.name) - it.addFile(file3, true, file3.name) - it.addFile(file4, false, file4.name) - } - - val repository = - DirectoryReferenceLayerRepository(dir1.absolutePath, dir2.absolutePath) { mapConfigurator } + val file1 = TempFiles.createTempFile(sharedLayersDir) + val file2 = TempFiles.createTempFile(sharedLayersDir) + val file3 = TempFiles.createTempFile(projectLayersDir) + val file4 = TempFiles.createTempFile(projectLayersDir) + mapConfigurator.apply { + addFile(file1, true, file1.name) + addFile(file2, false, file2.name) + addFile(file3, true, file3.name) + addFile(file4, false, file4.name) + } + assertThat(repository.getAll().map { it.file }, containsInAnyOrder(file1, file3)) } @@ -73,76 +73,62 @@ class DirectoryReferenceLayerRepositoryTest { */ @Test fun getAll_withMultipleDirectoriesWithFilesWithTheSameRelativePath_onlyReturnsTheSupportedFileFromTheFirstDirectory() { - val dir1 = TempFiles.createTempDir() - val dir2 = TempFiles.createTempDir() - val file1 = TempFiles.createTempFile(dir1, "blah", ".temp") - val file2 = TempFiles.createTempFile(dir2, "blah", ".temp") - val mapConfigurator = StubMapConfigurator().also { - it.addFile(file1, true, file1.name) - it.addFile(file2, true, file2.name) + val file1 = TempFiles.createTempFile(sharedLayersDir, "blah", ".temp") + val file2 = TempFiles.createTempFile(projectLayersDir, "blah", ".temp") + mapConfigurator.apply { + addFile(file1, true, file1.name) + addFile(file2, true, file2.name) } - val repository = - DirectoryReferenceLayerRepository(dir1.absolutePath, dir2.absolutePath) { mapConfigurator } assertThat(repository.getAll().map { it.file }, containsInAnyOrder(file1)) } @Test fun get_returnsLayer() { - val dir = TempFiles.createTempDir() - val file1 = TempFiles.createTempFile(dir) - val file2 = TempFiles.createTempFile(dir) - val mapConfigurator = StubMapConfigurator().also { - it.addFile(file1, true, file1.name) - it.addFile(file2, true, file2.name) + val file1 = TempFiles.createTempFile(sharedLayersDir) + val file2 = TempFiles.createTempFile(sharedLayersDir) + mapConfigurator.apply { + addFile(file1, true, file1.name) + addFile(file2, true, file2.name) } - val repository = DirectoryReferenceLayerRepository(dir.absolutePath, "") { mapConfigurator } val file2Layer = repository.getAll().first { it.file == file2 } assertThat(repository.get(file2Layer.id)!!.file, equalTo(file2)) } @Test fun get_withMultipleDirectories_returnsLayer() { - val dir1 = TempFiles.createTempDir() - val dir2 = TempFiles.createTempDir() - val file1 = TempFiles.createTempFile(dir1) - val file2 = TempFiles.createTempFile(dir2) - val mapConfigurator = StubMapConfigurator().also { - it.addFile(file1, true, file1.name) - it.addFile(file2, true, file2.name) + val file1 = TempFiles.createTempFile(sharedLayersDir) + val file2 = TempFiles.createTempFile(projectLayersDir) + mapConfigurator.apply { + addFile(file1, true, file1.name) + addFile(file2, true, file2.name) } - val repository = DirectoryReferenceLayerRepository(dir1.absolutePath, dir2.absolutePath) { mapConfigurator } val file2Layer = repository.getAll().first { it.file == file2 } assertThat(repository.get(file2Layer.id)!!.file, equalTo(file2)) } @Test fun get_withMultipleDirectoriesWithFilesWithTheSameRelativePath_returnsLayerFromFirstDirectory() { - val dir1 = TempFiles.createTempDir() - val dir2 = TempFiles.createTempDir() - val file1 = TempFiles.createTempFile(dir1, "blah", ".temp") - val file2 = TempFiles.createTempFile(dir2, "blah", ".temp") - val mapConfigurator = StubMapConfigurator().also { - it.addFile(file1, true, file1.name) - it.addFile(file2, true, file2.name) + val file1 = TempFiles.createTempFile(sharedLayersDir, "blah", ".temp") + val file2 = TempFiles.createTempFile(projectLayersDir, "blah", ".temp") + mapConfigurator.apply { + addFile(file1, true, file1.name) + addFile(file2, true, file2.name) } - val repository = DirectoryReferenceLayerRepository(dir1.absolutePath, dir2.absolutePath) { mapConfigurator } val layerId = repository.getAll().first().id assertThat(repository.get(layerId)!!.file, equalTo(file1)) } @Test fun get_whenFileDoesNotExist_returnsNull() { - val dir = TempFiles.createTempDir() - val file = TempFiles.createTempFile(dir) - val mapConfigurator = StubMapConfigurator().also { - it.addFile(file, true, file.name) + val file = TempFiles.createTempFile(sharedLayersDir) + mapConfigurator.apply { + addFile(file, true, file.name) } - val repository = DirectoryReferenceLayerRepository(dir.absolutePath, "") { mapConfigurator } val fileLayer = repository.getAll().first { it.file == file } file.delete() @@ -151,14 +137,12 @@ class DirectoryReferenceLayerRepositoryTest { @Test fun get_returnsLayerWithCorrectName() { - val dir = TempFiles.createTempDir() - val file = TempFiles.createTempFile(dir) + val file = TempFiles.createTempFile(sharedLayersDir) - val mapConfigurator = StubMapConfigurator().also { - it.addFile(file, true, file.name) + mapConfigurator.apply { + addFile(file, true, file.name) } - val repository = DirectoryReferenceLayerRepository(dir.absolutePath, "") { mapConfigurator } val fileLayer = repository.getAll().first { it.file == file } assertThat(repository.get(fileLayer.id)!!.name, equalTo(file.name)) @@ -166,17 +150,10 @@ class DirectoryReferenceLayerRepositoryTest { @Test fun addLayer_movesFileToTheSharedLayersDir_whenSharedIsTrue() { - val sharedLayersDir = TempFiles.createTempDir() - val projectLayersDir = TempFiles.createTempDir() val file = TempFiles.createTempFile().also { it.writeText("blah") } - val repository = DirectoryReferenceLayerRepository( - sharedLayersDir.absolutePath, - projectLayersDir.absolutePath - ) { StubMapConfigurator() } - repository.addLayer(file, true) assertThat(sharedLayersDir.listFiles().size, equalTo(1)) @@ -187,17 +164,10 @@ class DirectoryReferenceLayerRepositoryTest { @Test fun addLayer_movesFileToTheProjectLayersDir_whenSharedIsFalse() { - val sharedLayersDir = TempFiles.createTempDir() - val projectLayersDir = TempFiles.createTempDir() val file = TempFiles.createTempFile().also { it.writeText("blah") } - val repository = DirectoryReferenceLayerRepository( - sharedLayersDir.absolutePath, - projectLayersDir.absolutePath - ) { StubMapConfigurator() } - repository.addLayer(file, false) assertThat(sharedLayersDir.listFiles().size, equalTo(0)) From ef62803add29308a4da1a67e663880c5a2c680d0 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Thu, 6 Jun 2024 19:49:06 +0200 Subject: [PATCH 27/28] Removed TestReferenceLayersRepository used only in one class --- .../maps/layers/OfflineMapLayersPickerTest.kt | 51 +++++++++++-------- .../layers/TestReferenceLayerRepository.kt | 25 --------- 2 files changed, 29 insertions(+), 47 deletions(-) delete mode 100644 maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt index 655941a0fad..cfff10481f9 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersPickerTest.kt @@ -28,6 +28,7 @@ import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import org.odk.collect.androidshared.ui.FragmentFactoryBuilder import org.odk.collect.fragmentstest.FragmentScenarioLauncherRule import org.odk.collect.maps.R @@ -43,7 +44,7 @@ import org.odk.collect.webpage.ExternalWebPageHelper @RunWith(AndroidJUnit4::class) class OfflineMapLayersPickerTest { - private val referenceLayerRepository = TestReferenceLayerRepository() + private val referenceLayerRepository = mock() private val scheduler = FakeScheduler() private val settingsProvider = InMemSettingsProvider() private val externalWebPageHelper = mock() @@ -83,8 +84,8 @@ class OfflineMapLayersPickerTest { @Test fun `clicking the 'cancel' button does not save the layer`() { - referenceLayerRepository.addLayers( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1") + whenever(referenceLayerRepository.getAll()).thenReturn( + listOf(ReferenceLayer("1", TempFiles.createTempFile(), "layer1")) ) launchOfflineMapLayersPicker() @@ -126,8 +127,8 @@ class OfflineMapLayersPickerTest { @Test fun `clicking the 'save' button saves null when 'None' option is checked`() { - referenceLayerRepository.addLayers( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1") + whenever(referenceLayerRepository.getAll()).thenReturn( + listOf(ReferenceLayer("1", TempFiles.createTempFile(), "layer1")) ) launchOfflineMapLayersPicker() @@ -140,8 +141,8 @@ class OfflineMapLayersPickerTest { @Test fun `clicking the 'save' button saves the layer id if any is checked`() { - referenceLayerRepository.addLayers( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1") + whenever(referenceLayerRepository.getAll()).thenReturn( + listOf(ReferenceLayer("1", TempFiles.createTempFile(), "layer1")) ) launchOfflineMapLayersPicker() @@ -155,8 +156,8 @@ class OfflineMapLayersPickerTest { @Test fun `when no layer id is saved in settings the 'None' option should be checked`() { - referenceLayerRepository.addLayers( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1") + whenever(referenceLayerRepository.getAll()).thenReturn( + listOf(ReferenceLayer("1", TempFiles.createTempFile(), "layer1")) ) launchOfflineMapLayersPicker() @@ -169,9 +170,11 @@ class OfflineMapLayersPickerTest { @Test fun `when layer id is saved in settings the layer it belongs to should be checked`() { - referenceLayerRepository.addLayers( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1"), - ReferenceLayer("2", TempFiles.createTempFile(), "layer2") + whenever(referenceLayerRepository.getAll()).thenReturn( + listOf( + ReferenceLayer("1", TempFiles.createTempFile(), "layer1"), + ReferenceLayer("2", TempFiles.createTempFile(), "layer2") + ) ) settingsProvider.getUnprotectedSettings().save(ProjectKeys.KEY_REFERENCE_LAYER, "2") @@ -228,9 +231,11 @@ class OfflineMapLayersPickerTest { @Test fun `if there are multiple layers all of them are displayed along with the 'None'`() { - referenceLayerRepository.addLayers( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1"), - ReferenceLayer("2", TempFiles.createTempFile(), "layer2") + whenever(referenceLayerRepository.getAll()).thenReturn( + listOf( + ReferenceLayer("1", TempFiles.createTempFile(), "layer1"), + ReferenceLayer("2", TempFiles.createTempFile(), "layer2") + ) ) launchOfflineMapLayersPicker() @@ -245,8 +250,8 @@ class OfflineMapLayersPickerTest { @Test fun `checking layers sets selection correctly`() { - referenceLayerRepository.addLayers( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1") + whenever(referenceLayerRepository.getAll()).thenReturn( + listOf(ReferenceLayer("1", TempFiles.createTempFile(), "layer1")) ) launchOfflineMapLayersPicker() @@ -264,8 +269,8 @@ class OfflineMapLayersPickerTest { @Test fun `recreating maintains selection`() { - referenceLayerRepository.addLayers( - ReferenceLayer("1", TempFiles.createTempFile(), "layer1") + whenever(referenceLayerRepository.getAll()).thenReturn( + listOf(ReferenceLayer("1", TempFiles.createTempFile(), "layer1")) ) val scenario = launchOfflineMapLayersPicker() @@ -347,9 +352,11 @@ class OfflineMapLayersPickerTest { EspressoHelpers.clickOnText(string.add_layer) scheduler.flush() onView(withId(R.id.add_layer_button)).inRoot(isDialog()).perform(click()) - referenceLayerRepository.addLayers( - ReferenceLayer("1", file1, file1.name), - ReferenceLayer("2", file2, file2.name) + whenever(referenceLayerRepository.getAll()).thenReturn( + listOf( + ReferenceLayer("1", TempFiles.createTempFile(), file1.name), + ReferenceLayer("2", TempFiles.createTempFile(), file2.name) + ) ) scheduler.flush() diff --git a/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt b/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt deleted file mode 100644 index af87b0c2311..00000000000 --- a/maps/src/test/java/org/odk/collect/maps/layers/TestReferenceLayerRepository.kt +++ /dev/null @@ -1,25 +0,0 @@ -package org.odk.collect.maps.layers - -import java.io.File - -class TestReferenceLayerRepository : ReferenceLayerRepository { - private val layers = mutableListOf() - - override fun getAll(): List { - return layers - } - - override fun get(id: String): ReferenceLayer? { - return layers.find { it.id == id } - } - - override fun addLayer(file: File, shared: Boolean) { - TODO("Not yet implemented") - } - - fun addLayers(vararg newLayers: ReferenceLayer) { - newLayers.forEach { newLayer -> - layers.add(newLayer) - } - } -} From 63a7a4bfb3969fa2585476c5d6d2532d5b56f423 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Fri, 7 Jun 2024 00:40:51 +0200 Subject: [PATCH 28/28] Improved UriExt and added tests --- .../collect/androidshared/system/UriExt.kt | 81 ++++++++++++++++--- .../androidshared/system/UriExtTest.kt | 44 ++++++++++ .../maps/layers/OfflineMapLayersImporter.kt | 2 +- .../maps/layers/OfflineMapLayersPicker.kt | 4 +- .../maps/layers/OfflineMapLayersViewModel.kt | 14 ++-- .../layers/OfflineMapLayersImporterTest.kt | 14 ++-- 6 files changed, 130 insertions(+), 29 deletions(-) create mode 100644 androidshared/src/test/java/org/odk/collect/androidshared/system/UriExtTest.kt diff --git a/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt b/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt index 11092645aac..9244e13c53f 100644 --- a/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt +++ b/androidshared/src/main/java/org/odk/collect/androidshared/system/UriExt.kt @@ -1,14 +1,17 @@ package org.odk.collect.androidshared.system import android.content.ContentResolver +import android.content.Context import android.net.Uri import android.provider.OpenableColumns +import android.webkit.MimeTypeMap +import androidx.core.net.toFile import java.io.File import java.io.FileOutputStream -fun Uri.copyToFile(contentResolver: ContentResolver, dest: File) { +fun Uri.copyToFile(context: Context, dest: File) { try { - contentResolver.openInputStream(this)?.use { inputStream -> + context.contentResolver.openInputStream(this)?.use { inputStream -> FileOutputStream(dest).use { outputStream -> inputStream.copyTo(outputStream) } @@ -18,19 +21,73 @@ fun Uri.copyToFile(contentResolver: ContentResolver, dest: File) { } } -fun Uri.getFileName(contentResolver: ContentResolver): String? { +fun Uri.getFileExtension(context: Context): String? { + var extension = getFileName(context)?.substringAfterLast(".", "") + + if (extension.isNullOrEmpty()) { + val mimeType = context.contentResolver.getType(this) + + extension = if (scheme == ContentResolver.SCHEME_CONTENT) { + MimeTypeMap.getSingleton().getExtensionFromMimeType(mimeType) + } else { + MimeTypeMap.getFileExtensionFromUrl(toString()) + } + + if (extension.isNullOrEmpty()) { + extension = mimeType?.substringAfterLast("/", "") + } + } + + return if (extension.isNullOrEmpty()) { + null + } else { + ".$extension" + } +} + +fun Uri.getFileName(context: Context): String? { var fileName: String? = null - if (scheme == ContentResolver.SCHEME_CONTENT) { - val cursor = contentResolver.query(this, null, null, null, null) - cursor.use { - if (it != null && it.moveToFirst()) { - val fileNameColumnIndex = it.getColumnIndex(OpenableColumns.DISPLAY_NAME) - fileName = it.getString(fileNameColumnIndex) + + try { + when (scheme) { + ContentResolver.SCHEME_FILE -> fileName = toFile().name + ContentResolver.SCHEME_CONTENT -> { + val cursor = context.contentResolver.query(this, null, null, null, null) + cursor?.use { + if (it.moveToFirst()) { + val fileNameColumnIndex = it.getColumnIndex(OpenableColumns.DISPLAY_NAME) + if (fileNameColumnIndex != -1) { + fileName = it.getString(fileNameColumnIndex) + } + } + } + } + ContentResolver.SCHEME_ANDROID_RESOURCE -> { + // for uris like [android.resource://com.example.app/1234567890] + val resourceId = lastPathSegment?.toIntOrNull() + if (resourceId != null) { + fileName = context.resources.getResourceName(resourceId) + } else { + // for uris like [android.resource://com.example.app/raw/sample] + val packageName = authority + if (pathSegments.size >= 2) { + val resourceType = pathSegments[0] + val resourceEntryName = pathSegments[1] + val resId = context.resources.getIdentifier(resourceEntryName, resourceType, packageName) + if (resId != 0) { + fileName = context.resources.getResourceName(resId) + } + } + } } } + + if (fileName == null) { + fileName = path?.substringAfterLast("/") + } + } catch (e: Exception) { + // ignore } - if (fileName == null) { - fileName = path?.substringAfterLast("/") - } + return fileName } diff --git a/androidshared/src/test/java/org/odk/collect/androidshared/system/UriExtTest.kt b/androidshared/src/test/java/org/odk/collect/androidshared/system/UriExtTest.kt new file mode 100644 index 00000000000..9dafdde3ef4 --- /dev/null +++ b/androidshared/src/test/java/org/odk/collect/androidshared/system/UriExtTest.kt @@ -0,0 +1,44 @@ +package org.odk.collect.androidshared.system + +import android.app.Application +import androidx.core.net.toUri +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.hamcrest.CoreMatchers.equalTo +import org.hamcrest.MatcherAssert.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.odk.collect.shared.TempFiles + +@RunWith(AndroidJUnit4::class) +class UriExtTest { + private val context = ApplicationProvider.getApplicationContext() + + @Test + fun `copyToFile copies the source file to the target file`() { + val sourceFile = TempFiles.createTempFile().also { + it.writeText("blah") + } + val sourceFileUri = sourceFile.toUri() + val targetFile = TempFiles.createTempFile() + + sourceFileUri.copyToFile(context, targetFile) + assertThat(targetFile.readText(), equalTo(sourceFile.readText())) + } + + @Test + fun `getFileExtension returns file extension`() { + val file = TempFiles.createTempFile(".jpg") + val fileUri = file.toUri() + + assertThat(fileUri.getFileExtension(context), equalTo(".jpg")) + } + + @Test + fun `getFileName returns file name`() { + val file = TempFiles.createTempFile() + val fileUri = file.toUri() + + assertThat(fileUri.getFileName(context), equalTo(file.name)) + } +} diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt index 0066f79bc7f..a3ca996b4ba 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersImporter.kt @@ -21,7 +21,7 @@ class OfflineMapLayersImporter( val viewModel: OfflineMapLayersViewModel by activityViewModels { object : ViewModelProvider.Factory { override fun create(modelClass: Class): T { - return OfflineMapLayersViewModel(referenceLayerRepository, scheduler, settingsProvider, requireContext().contentResolver) as T + return OfflineMapLayersViewModel(referenceLayerRepository, scheduler, settingsProvider) as T } } } diff --git a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt index 739e1c932df..360b692c5c7 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersPicker.kt @@ -30,7 +30,7 @@ class OfflineMapLayersPicker( private val viewModel: OfflineMapLayersViewModel by activityViewModels { object : ViewModelProvider.Factory { override fun create(modelClass: Class): T { - return OfflineMapLayersViewModel(referenceLayerRepository, scheduler, settingsProvider, requireContext().contentResolver) as T + return OfflineMapLayersViewModel(referenceLayerRepository, scheduler, settingsProvider) as T } } } @@ -39,7 +39,7 @@ class OfflineMapLayersPicker( private val getLayers = registerForActivityResult(ActivityResultContracts.GetMultipleContents(), registry) { uris -> if (uris.isNotEmpty()) { - viewModel.loadLayersToImport(uris) + viewModel.loadLayersToImport(uris, requireContext()) DialogFragmentUtils.showIfNotShowing( OfflineMapLayersImporter::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 index e58bcce56af..f23e7bc6871 100644 --- a/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt +++ b/maps/src/main/java/org/odk/collect/maps/layers/OfflineMapLayersViewModel.kt @@ -1,11 +1,12 @@ package org.odk.collect.maps.layers -import android.content.ContentResolver +import android.content.Context import android.net.Uri import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel import org.odk.collect.androidshared.system.copyToFile +import org.odk.collect.androidshared.system.getFileExtension import org.odk.collect.androidshared.system.getFileName import org.odk.collect.async.Scheduler import org.odk.collect.settings.SettingsProvider @@ -16,8 +17,7 @@ import java.io.File class OfflineMapLayersViewModel( private val referenceLayerRepository: ReferenceLayerRepository, private val scheduler: Scheduler, - private val settingsProvider: SettingsProvider, - private val contentResolver: ContentResolver + private val settingsProvider: SettingsProvider ) : ViewModel() { private val _isLoading = MutableLiveData() val isLoading: LiveData = _isLoading @@ -49,7 +49,7 @@ class OfflineMapLayersViewModel( ) } - fun loadLayersToImport(uris: List) { + fun loadLayersToImport(uris: List, context: Context) { _isLoading.value = true scheduler.immediate( background = { @@ -58,10 +58,10 @@ class OfflineMapLayersViewModel( } val layers = mutableListOf() uris.forEach { uri -> - uri.getFileName(contentResolver)?.let { fileName -> - if (fileName.endsWith(MbtilesFile.FILE_EXTENSION)) { + if (uri.getFileExtension(context) == MbtilesFile.FILE_EXTENSION) { + uri.getFileName(context)?.let { fileName -> val layerFile = File(tempLayersDir, fileName).also { file -> - uri.copyToFile(contentResolver, file) + uri.copyToFile(context, file) } layers.add(ReferenceLayer(layerFile.absolutePath, layerFile, MbtilesFile.readName(layerFile) ?: layerFile.name)) } diff --git a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt index ef76e2238c3..f304774db04 100644 --- a/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt +++ b/maps/src/test/java/org/odk/collect/maps/layers/OfflineMapLayersImporterTest.kt @@ -63,7 +63,7 @@ class OfflineMapLayersImporterTest { launchFragment().onFragment { scheduler.flush() assertThat(it.isVisible, equalTo(true)) - it.viewModel.loadLayersToImport(emptyList()) + it.viewModel.loadLayersToImport(emptyList(), it.requireContext()) onView(withId(org.odk.collect.maps.R.id.add_layer_button)).perform(click()) scheduler.flush() RobolectricHelpers.runLooper() @@ -77,7 +77,7 @@ class OfflineMapLayersImporterTest { val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) launchFragment().onFragment { - it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri()), it.requireContext()) } onView(withId(org.odk.collect.maps.R.id.progress_indicator)).check(matches(isDisplayed())) @@ -150,7 +150,7 @@ class OfflineMapLayersImporterTest { val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) launchFragment().onFragment { - it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri()), it.requireContext()) } scheduler.flush() @@ -166,7 +166,7 @@ class OfflineMapLayersImporterTest { val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) val scenario = launchFragment().onFragment { - it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri()), it.requireContext()) } scheduler.flush() @@ -184,7 +184,7 @@ class OfflineMapLayersImporterTest { val file2 = TempFiles.createTempFile("layer2", ".txt") launchFragment().onFragment { - it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri()), it.requireContext()) } scheduler.flush() @@ -200,7 +200,7 @@ class OfflineMapLayersImporterTest { val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) launchFragment().onFragment { - it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri()), it.requireContext()) } scheduler.flush() @@ -224,7 +224,7 @@ class OfflineMapLayersImporterTest { val file2 = TempFiles.createTempFile("layer2", MbtilesFile.FILE_EXTENSION) launchFragment().onFragment { - it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri())) + it.viewModel.loadLayersToImport(listOf(file1.toUri(), file2.toUri()), it.requireContext()) } scheduler.flush()