diff --git a/.gitignore b/.gitignore index ba3cf311e5a..1a7b071ef7e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ *.ignore quest-list.csv /keystore.properties +/secrets.properties /projectFilesBackup*/ app/release *.iml diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 74b670a2c6a..42bc99c1d54 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -9,8 +9,8 @@ import java.util.Properties /** App version name, code and flavor */ -val appVersionName = "1.2.4" -val appVersionCode = 14 +val appVersionName = "1.2.7" +val appVersionCode = 16 /** Localizations the app should be available in */ val bcp47ExportLanguages = setOf( @@ -54,6 +54,14 @@ repositories { maven { url = uri("https://www.jitpack.io") } } +/** KartaView access token: from env var, else root secrets.properties (git-ignored), else empty + * (build succeeds but KartaView photo uploads fail auth at runtime). Never commit the token. */ +val kartaViewAccessToken: String = System.getenv("KARTAVIEW_ACCESS_TOKEN") + ?: rootProject.file("secrets.properties").takeIf { it.exists() }?.let { file -> + Properties().apply { FileInputStream(file).use { load(it) } }.getProperty("kartaViewAccessToken") + } + ?: "" + buildkonfig { packageName = "de.westnordost.streetcomplete" objectName = "BuildConfig" @@ -62,6 +70,7 @@ buildkonfig { buildConfigField(BOOLEAN, "IS_FROM_MONOPOLISTIC_APP_STORE", properties["app.streetcomplete.monopolistic_app_store"]!!.toString()) buildConfigField(STRING, "VERSION_NAME", appVersionName) buildConfigField(BOOLEAN, "DEBUG", "true") + buildConfigField(STRING, "KARTAVIEW_ACCESS_TOKEN", kartaViewAccessToken) } targetConfigs { diff --git a/app/src/androidDebug/res/values/conf.xml b/app/src/androidDebug/res/values/conf.xml index e0acbd8a56a..157e85a2a43 100644 --- a/app/src/androidDebug/res/values/conf.xml +++ b/app/src/androidDebug/res/values/conf.xml @@ -1,4 +1,4 @@ - de.westnordost.streetcomplete.debug.fileprovider + net.opentoall.aviv.scoutroute.debug.fileprovider diff --git a/app/src/androidDebug/res/xml/file_paths.xml b/app/src/androidDebug/res/xml/file_paths.xml deleted file mode 100644 index b9f955c5421..00000000000 --- a/app/src/androidDebug/res/xml/file_paths.xml +++ /dev/null @@ -1,4 +0,0 @@ - - - - diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt index 3168ee2666b..7ab8599724b 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt @@ -3,6 +3,7 @@ package de.westnordost.streetcomplete import android.content.Intent import android.content.res.AssetManager import android.content.res.Resources +import de.westnordost.streetcomplete.data.karta_view.KartaViewApiClient import de.westnordost.streetcomplete.data.preferences.EnvironmentManager import de.westnordost.streetcomplete.data.preferences.Preferences import de.westnordost.streetcomplete.data.workspace.domain.model.LoginResponse @@ -147,6 +148,8 @@ val appModule = module { } } + single { KartaViewApiClient(get(), get(named("kartaViewClient")), get()) } + single { Res } single { SystemFileSystem } single { DefaultResourceProvider(androidContext(), get()) } @@ -170,7 +173,7 @@ suspend fun refreshJwtToken( } val response = - tempClient.post(environmentManager.currentEnvironment.loginUrl + "/refresh-token") { + tempClient.post(environmentManager.currentEnvironment.tdeiApiBaseUrl + "/refresh-token") { setBody(preferences.workspaceRefreshToken) headers { contentType(ContentType.Application.Json) diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/workspace/WorkspaceModule.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/workspace/WorkspaceModule.kt index 71934f94651..a78d5b07557 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/workspace/WorkspaceModule.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/workspace/WorkspaceModule.kt @@ -15,6 +15,6 @@ val workspaceModule = module { // single { GetWorkspaceUseCase(get()) } // single { LoginUseCase(get()) } - viewModel { WorkspaceViewModelImpl(get(), get()) } + viewModel { WorkspaceViewModelImpl(get(), get(), get()) } } diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/workspace/data/remote/WorkspaceApiService.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/workspace/data/remote/WorkspaceApiService.kt index aad680d15ba..dd2060a8ffc 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/workspace/data/remote/WorkspaceApiService.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/workspace/data/remote/WorkspaceApiService.kt @@ -64,7 +64,7 @@ class WorkspaceApiService( } suspend fun getTDEIUserDetails(emailId: String): UserInfoResponse { - val url = environmentManager.currentEnvironment.tdeiUrl + val url = environmentManager.currentEnvironment.tdeiUserManagementBaseurl try { val response = performHttpCallWithFirebaseTracing( @@ -107,7 +107,7 @@ class WorkspaceApiService( } suspend fun loginToWorkspace(username: String, password: String): LoginResponse { - val url = environmentManager.currentEnvironment.loginUrl + "/authenticate" + val url = environmentManager.currentEnvironment.tdeiApiBaseUrl + "/authenticate" try { val response = performHttpCallWithFirebaseTracing( client = httpClient, @@ -145,7 +145,7 @@ class WorkspaceApiService( } suspend fun refreshToken(refreshToken: String): LoginResponse { - val url = environmentManager.currentEnvironment.loginUrl + "/refresh-token" + val url = environmentManager.currentEnvironment.tdeiApiBaseUrl + "/refresh-token" try { val response = performHttpCallWithFirebaseTracing( diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/ALongForm.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/ALongForm.kt index 0472c7c3b76..3ec5a1d0c64 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/ALongForm.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/ALongForm.kt @@ -9,6 +9,7 @@ import android.view.inputmethod.InputMethodManager import android.widget.Toast import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView +import androidx.recyclerview.widget.SimpleItemAnimator import de.westnordost.streetcomplete.R import de.westnordost.streetcomplete.databinding.QuestLongFormListBinding import de.westnordost.streetcomplete.quests.sidewalk_long_form.data.LongFormAdapter @@ -74,6 +75,13 @@ abstract class ALongForm : AbstractOsmQuestForm() { super.onViewCreated(view, savedInstanceState) binding.recyclerView.apply { layoutManager = LinearLayoutManager(activity) + // DiffUtil-driven partial updates (see LongFormAdapter.items) now issue targeted + // notifyItemChanged() calls instead of notifyDataSetChanged(). The default item + // animator runs a cross-fade "change" animation on those, which reads as flicker on + // image content - notifyDataSetChanged() never triggered that. Keep insert/remove/move + // animations (questions appearing/disappearing still animates nicely), just drop the + // content-change cross-fade. + (itemAnimator as? SimpleItemAnimator)?.supportsChangeAnimations = false } setVisibilityOfItems() binding.recyclerView.adapter = adapter diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt index 6d217ea7ab9..3a0ab647c17 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt @@ -27,8 +27,7 @@ import com.google.android.material.snackbar.Snackbar import de.westnordost.osmfeatures.Feature import de.westnordost.osmfeatures.FeatureDictionary import de.westnordost.streetcomplete.R -import de.westnordost.streetcomplete.data.karta_view.domain.model.CreateSequenceResponse -import de.westnordost.streetcomplete.data.karta_view.domain.model.PhotoLookupResponse +import de.westnordost.streetcomplete.data.karta_view.KartaViewApiClient import de.westnordost.streetcomplete.data.location.SurveyChecker import de.westnordost.streetcomplete.data.osm.edits.AddElementEditsController import de.westnordost.streetcomplete.data.osm.edits.ElementEditAction @@ -43,6 +42,7 @@ import de.westnordost.streetcomplete.data.osm.geometry.ElementGeometry import de.westnordost.streetcomplete.data.osm.geometry.ElementPolylinesGeometry import de.westnordost.streetcomplete.data.osm.mapdata.Element import de.westnordost.streetcomplete.data.osm.mapdata.ElementType +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osm.mapdata.Node import de.westnordost.streetcomplete.data.osm.mapdata.Way import de.westnordost.streetcomplete.data.osm.osmquests.OsmElementQuestType @@ -53,7 +53,6 @@ import de.westnordost.streetcomplete.data.quest.Quest import de.westnordost.streetcomplete.data.quest.QuestKey import de.westnordost.streetcomplete.data.visiblequests.HideQuestController import de.westnordost.streetcomplete.data.visiblequests.QuestsHiddenController -import de.westnordost.streetcomplete.data.preferences.Preferences import de.westnordost.streetcomplete.osm.applyReplacePlaceTo import de.westnordost.streetcomplete.quests.sidewalk_long_form.AddGenericLong import de.westnordost.streetcomplete.screens.main.map.Compass @@ -62,17 +61,6 @@ import de.westnordost.streetcomplete.util.ktx.isSplittable import de.westnordost.streetcomplete.util.ktx.viewLifecycleScope import de.westnordost.streetcomplete.view.add import de.westnordost.streetcomplete.view.confirmIsSurvey -import io.ktor.client.HttpClient -import io.ktor.client.call.body -import io.ktor.client.request.forms.MultiPartFormDataContent -import io.ktor.client.request.forms.formData -import io.ktor.client.request.get -import io.ktor.client.request.parameter -import io.ktor.client.request.post -import io.ktor.client.request.setBody -import io.ktor.http.Headers -import io.ktor.http.HttpHeaders -import io.ktor.http.HttpStatusCode import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -95,8 +83,7 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta private val surveyChecker: SurveyChecker by inject() protected val featureDictionary: FeatureDictionary get() = featureDictionaryLazy.value - private val httpClient: HttpClient by inject(named("kartaViewClient")) - private val prefs: Preferences by inject() + private val kartaViewApiClient: KartaViewApiClient by inject() private lateinit var cameraLauncher: ActivityResultLauncher // only used for testing / only used for ShowQuestFormsScreen! Found no better way to do this @@ -169,14 +156,13 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta if (getElement != null) { element = getElement } - val displayedLocation = args.getParcelable(ARG_DISPLAYED_LOCATION) cameraLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> if (result.resultCode == Activity.RESULT_OK) { showProgressbar() // Handle the image capture result here val bitmap = result.data?.extras?.getParcelable("data") - startKartViewFlow(bitmap, displayedLocation) + startKartViewFlow(bitmap) } else { // Handle the error state here @@ -192,7 +178,11 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta super.onViewCreated(view, savedInstanceState) if (osmElementQuestType is AddGenericLong) { - setTitle((osmElementQuestType as AddGenericLong).item.elementType) + val category = (osmElementQuestType as AddGenericLong).item.elementType + // the category alone ("Sidewalk", "Kerb"...) doesn't distinguish between several + // queued quests of the same type - add the OSM element type and id + val typeAndId = "${element.type.name.lowercase().replaceFirstChar { it.uppercase() }} #${element.id}" + setTitle("$category — $typeAndId") } setHideQuestOnClick { hideQuest() } @@ -332,7 +322,7 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta solve( UpdateElementTagsAction( element.first, - createQuestChanges(answer, extraTagList) + createQuestChanges(answer, extraTagList, element.first, element.second) ), element.second ) } @@ -352,10 +342,12 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta private fun createQuestChanges( answer: T, extraTagList: MutableList> = mutableListOf(), + forElement: Element = element, + forGeometry: ElementGeometry = geometry, ): StringMapChanges { - val changesBuilder = StringMapChangesBuilder(element.tags) + val changesBuilder = StringMapChangesBuilder(forElement.tags) extraTagList.forEach { changesBuilder[it.first] = it.second } - osmElementQuestType.applyAnswerTo(answer, changesBuilder, geometry, element.timestampEdited) + osmElementQuestType.applyAnswerTo(answer, changesBuilder, forGeometry, forElement.timestampEdited) val changes = changesBuilder.create() require(!changes.isEmpty()) { "${osmElementQuestType.name} was answered by the user but there are no changes!" @@ -440,109 +432,37 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta } } - private fun startKartViewFlow(bitmap: Bitmap?, displayedLocation: Location?) { + private fun startKartViewFlow(bitmap: Bitmap?) { viewLifecycleScope.launch { - val sequenceId = createSequence() ?: return@launch - val uploaded = uploadImageInSequence(sequenceId, bitmap, displayedLocation) - if (uploaded) { - closeSequence(sequenceId) - val lthUrl = getPhotoLthUrl(sequenceId, sequenceIndex = 1) - if (lthUrl != null) { - Log.d("KartViewFlow", lthUrl) - onImageUrlReceived(lthUrl) - } else { - hideProgressbar() - Log.e("KartViewFlow", "Failed to retrieve photo URL") - showSnackBar("Failed to retrieve photo URL. Please try again later.", view, requireActivity() as ComponentActivity) - } - } else { + val displayedLocation = listener?.displayedMapLocation + if (bitmap == null || displayedLocation == null) { hideProgressbar() - Log.e("KartViewFlow", "Image upload failed") - showSnackBar("Image upload failed. Please try again later.", view, requireActivity() as ComponentActivity) + return@launch } - } - } - - private suspend fun getPhotoLthUrl(sequenceId: String, sequenceIndex: Int): String? { - val token = prefs.kartaViewAccessToken - val response = httpClient.get("https://api.openstreetcam.org/2.0/photo/") { - parameter("access_token", token) - parameter("sequenceId", sequenceId) - parameter("sequenceIndex", sequenceIndex) - } - if (response.status == HttpStatusCode.OK) { - val photoResponse = response.body() - return photoResponse.result?.data?.firstOrNull()?.imageLthUrl - } - Log.e("KartViewFlow", "Photo lookup failed: ${response.status}") - return null - } - - private suspend fun closeSequence(sequenceId: String) { - val token = prefs.kartaViewAccessToken - val response = - httpClient.post("https://api.openstreetcam.org/1.0/sequence/finished-uploading/") { - setBody(MultiPartFormDataContent(formData { - append("access_token", token) - append("sequenceId", sequenceId) - })) + val bearing = if (displayedLocation.hasBearing() && displayedLocation.bearing != 0f) { + displayedLocation.bearing + } else { + compassBearing.toFloat() + } + val byteArrayOutputStream = ByteArrayOutputStream() + bitmap.compress(Bitmap.CompressFormat.JPEG, 100, byteArrayOutputStream) + try { + val urls = kartaViewApiClient.uploadImages( + listOf(byteArrayOutputStream.toByteArray()), + LatLon(displayedLocation.latitude, displayedLocation.longitude), + bearing + ) + showSnackBar("Image Uploaded Successfully", view, requireActivity() as ComponentActivity) + onImageUrlReceived(urls.first()) + } catch (e: Exception) { + Log.e("KartViewFlow", "KartaView upload failed", e) + showSnackBar( + e.message ?: "Image upload failed. Please try again later.", + view, requireActivity() as ComponentActivity + ) + } finally { + hideProgressbar() } - if (response.status == HttpStatusCode.OK) { - val sequence = response.body() - Log.d("KartViewSequence", sequence.status.httpMessage) - } else { - showSnackBar( - "Failed to close KartaView Sequence. Please try again later " + response.status, - view, requireActivity() as ComponentActivity - ) - } - hideProgressbar() - } - - private suspend fun uploadImageInSequence( - sequenceId: String, - bitmap: Bitmap?, - location: Location?, - ): Boolean { - val displayedLocation = listener?.displayedMapLocation ?: return false - bitmap ?: return false - - val byteArrayOutputStream = ByteArrayOutputStream() - bitmap.compress(Bitmap.CompressFormat.JPEG, 100, byteArrayOutputStream) - val byteArray = byteArrayOutputStream.toByteArray() - - val finalBearing = if (displayedLocation.hasBearing() && displayedLocation.bearing != 0f) { - displayedLocation.bearing - } else { - compassBearing.toFloat() - } - - val response = httpClient.post("https://api.openstreetcam.org/1.0/photo/") { - setBody(MultiPartFormDataContent(formData { - append("access_token", prefs.kartaViewAccessToken) - append("sequenceId", sequenceId) - append("sequenceIndex", 1) - append("coordinate", "${displayedLocation.latitude},${displayedLocation.longitude}") - append("headers", finalBearing.toInt().toString()) - append("photo", byteArray, Headers.build { - append(HttpHeaders.ContentType, "image/jpeg") - append(HttpHeaders.ContentDisposition, "filename=\"wework-kartaview.jpg\"") - }) - })) - } - - return if (response.status == HttpStatusCode.OK) { - showSnackBar("Image Uploaded Successfully", view, requireActivity() as ComponentActivity) - Log.d("UploadImage", "Image uploaded successfully") - true - } else { - Log.e("UploadImage", "Image upload failed: ${response.status}") - showSnackBar( - "Failed to upload image to KartaView. Please try again later " + response.status, - view, requireActivity() as ComponentActivity - ) - hideProgressbar() - false } } @@ -552,28 +472,6 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta } } - private suspend fun createSequence(): String? { - val token = prefs.kartaViewAccessToken - val response = httpClient.post("https://api.openstreetcam.org/1.0/sequence/") { - setBody(MultiPartFormDataContent(formData { - append("access_token", token) - })) - } - if (response.status == HttpStatusCode.OK) { - val sequence = response.body() - sequence.osv.sequence?.id?.let { Log.d("KartViewSequence", it) } - Log.d("KartViewStatus", sequence.status.httpMessage) - return sequence.osv.sequence?.id - } else { - hideProgressbar() - showSnackBar( - "Failed to create KartaView sequence. Image upload failed. Please try again later " + response.status, - view, requireActivity() as ComponentActivity - ) - return null - } - } - companion object { private const val ARG_ELEMENT = "element" private const val ARG_DISPLAYED_LOCATION = "displayedLocation" diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormAdapter.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormAdapter.kt index 92f4f7c7609..e894dba34b3 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormAdapter.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/LongFormAdapter.kt @@ -15,6 +15,7 @@ import android.widget.TextView import androidx.core.graphics.drawable.toDrawable import androidx.core.view.ViewCompat import androidx.core.view.accessibility.AccessibilityNodeInfoCompat +import androidx.recyclerview.widget.DiffUtil import androidx.recyclerview.widget.GridLayoutManager import androidx.recyclerview.widget.RecyclerView import androidx.recyclerview.widget.RecyclerView.ViewHolder @@ -49,8 +50,12 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : }.flatten() } - field = manageVisibility(value).filter { it.visible } - notifyDataSetChanged() + // snapshot each item rather than keeping the live (mutated-in-place) instances, so the + // diff below compares genuinely independent before/after values - see LongFormQuest.snapshot() + val newList = manageVisibility(value).filter { it.visible }.map { it.snapshot() } + val diff = DiffUtil.calculateDiff(LongFormQuestDiffCallback(field, newList)) + field = newList + diff.dispatchUpdatesTo(this) } enum class ViewType(val value: Int) { @@ -66,6 +71,8 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : } private fun manageVisibility(itemCopy: List): List { + val byQuestId = itemCopy.associateBy { it.questId } + for (quest in itemCopy) { val dependencies = quest.questAnswerDependency ?: emptyList() var isVisible = true @@ -76,7 +83,7 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : if (requiredUserInput == null || requiredQuestId == null) continue - val filteredQuest = itemCopy.find { it.questId == requiredQuestId } + val filteredQuest = byQuestId[requiredQuestId] if (filteredQuest != null) { when (filteredQuest.userInput) { is UserInput.Single -> { @@ -101,7 +108,7 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : } } } - itemCopy[itemCopy.indexOf(quest)].visible = isVisible + quest.visible = isVisible } itemCopy.forEach { @@ -112,6 +119,22 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : return itemCopy } + /** Diffs two snapshots of the visible question list so only rows that actually appeared, + * disappeared, moved or changed content get rebound - not the whole list every time. */ + private class LongFormQuestDiffCallback( + private val oldList: List, + private val newList: List, + ) : DiffUtil.Callback() { + override fun getOldListSize() = oldList.size + override fun getNewListSize() = newList.size + + override fun areItemsTheSame(oldItemPosition: Int, newItemPosition: Int) = + oldList[oldItemPosition].questId == newList[newItemPosition].questId + + override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int) = + oldList[oldItemPosition] == newList[newItemPosition] + } + class DefaultViewHolder(val binding: CellLongFormItemBinding) : ViewHolder(binding.root) { fun bind(item: LongFormQuest) { if (item.visible) binding.container.visibility = @@ -339,6 +362,12 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : ImageSelectAdapter(if (allowMultiChoice) -1 else 1) private var selectionListener: ImageSelectAdapter.OnItemSelectionListener? = null + // the choices for a given question never change once loaded, so this lets bind() skip + // rebuilding imageSelectAdapter.items (and the image reloads that would trigger) when this + // row was rebound for a reason unrelated to its own choices, e.g. another question's answer + // changing this row's position or an unrelated selection elsewhere in the form + private var boundQuestId: Int? = null + init { binding.list.layoutManager = GridLayoutManager(binding.root.context, 3) binding.list.isNestedScrollingEnabled = false @@ -348,6 +377,11 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : fun bind(item: LongFormQuest, position: Int) { binding.title.text = item.questTitle + binding.title.contentDescription = if (allowMultiChoice) { + "${item.questTitle}. Multiple items can be selected" + } else { + "${item.questTitle}. Only one item can be selected" + } if (!item.questImageUrl.isNullOrBlank() && !preferences.isLowBandwidthModeEnabled) { binding.imageView.setImage( ImageUrl(item.questImageUrl), @@ -367,6 +401,9 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : binding.choiceFollowUp.setOnClickListener { cameraIntent() } + // apply from current state, not just on selection events - the follow-up must also + // survive rebinds and holders recycled from other questions + updateChoiceFollowUp(item) imageSelectAdapter.selectedIndices = item.selectedIndex ?: emptyList() @@ -394,15 +431,10 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : } fun handleChoiceFollowUp() { - item.selectedIndex?.forEach { index -> - if (!item.questAnswerChoices?.get(index)?.choiceFollowUp.isNullOrBlank()) { - binding.choiceFollowUp.visibility = View.VISIBLE - binding.choiceFollowUp.text = - item.questAnswerChoices[index]?.choiceFollowUp - return - } - } - binding.choiceFollowUp.visibility = View.GONE + // item is a snapshot taken at bind time - the selection that was just made + // lives in givenItems, so read the follow-up state from there + val live = givenItems.firstOrNull { it.questId == item.questId } ?: item + updateChoiceFollowUp(live) } override fun onLongPress(index: Int, drawable: Drawable?) { @@ -452,14 +484,35 @@ class LongFormAdapter(val cameraIntent: () -> Unit) : imageSelectAdapter.listeners.add(listener) selectionListener = listener - imageSelectAdapter.items = item.questAnswerChoices?.map { - Item2( - item, - ImageUrl(it?.imageUrl), - CharSequenceText(it?.choiceText!!), - CharSequenceText("") - ) - }!! + if (item.questId != boundQuestId) { + boundQuestId = item.questId + imageSelectAdapter.items = item.questAnswerChoices?.map { + Item2( + item, + ImageUrl(it?.imageUrl), + CharSequenceText(it?.choiceText!!), + CharSequenceText("") + ) + }!! + } else { + // choices are unchanged, so the items setter above (and its own notifyDataSetChanged) + // was skipped - refresh explicitly so the selected/deselected highlight still updates + imageSelectAdapter.notifyDataSetChanged() + } + } + + /** Show the follow-up prompt (e.g. "Please take a photo of the obstruction.") of the + * first selected choice that has one, hide it if none of the selected choices do */ + private fun updateChoiceFollowUp(quest: LongFormQuest) { + quest.selectedIndex?.forEach { index -> + val followUp = quest.questAnswerChoices?.get(index)?.choiceFollowUp + if (!followUp.isNullOrBlank()) { + binding.choiceFollowUp.visibility = View.VISIBLE + binding.choiceFollowUp.text = followUp + return + } + } + binding.choiceFollowUp.visibility = View.GONE } fun handleDeselection( diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/Quest.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/Quest.kt index d7771aad98d..e7cad19ba3f 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/Quest.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/quests/sidewalk_long_form/data/Quest.kt @@ -46,3 +46,18 @@ sealed class UserInput : Parcelable { } } } + +/** A frozen copy of this quest's current state, safe to keep around and compare against later - + * [LongFormQuest.selectedIndex] and [UserInput.Multiple.answers] are mutated in place elsewhere + * (`.add()`/`.remove()`), so a shallow `.copy()` alone would still share those mutable lists with + * the live, later-mutated instance. Used to give DiffUtil genuinely independent before/after + * values to compare (see LongFormAdapter). */ +fun LongFormQuest.snapshot(): LongFormQuest = copy( + selectedIndex = selectedIndex?.toMutableList(), + userInput = userInput?.snapshot() +) + +fun UserInput.snapshot(): UserInput = when (this) { + is UserInput.Single -> copy() + is UserInput.Multiple -> copy(answers = answers.toMutableList()) +} diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt index 5e2311b4c27..1c6a872014d 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainActivity.kt @@ -158,6 +158,7 @@ import de.westnordost.streetcomplete.util.satellite_layers.Imagery import de.westnordost.streetcomplete.util.satellite_layers.ImageryRepository import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import org.koin.android.ext.android.inject @@ -1190,11 +1191,30 @@ class MainActivity : onClickImageryLayerButton() } + lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.STARTED) { + // badge shows unsynced edits (which already includes edits blocked on + // unresolved tag conflicts) and notices about discarded edits still to be + // acknowledged - everything that still needs attention. Conflicts are not + // added separately, that would double-count their blocked edit + combine( + viewModel.unsyncedEditsCount, + viewModel.discardedNoticesCount + ) { edits, discarded -> + edits + discarded + }.collect { totalPendingCount -> + uploadButton.uploadableCount = totalPendingCount + } + } + } + lifecycleScope.launch { repeatOnLifecycle(Lifecycle.State.STARTED) { viewModel.unsyncedEditsCount.collect { count -> - uploadButton.uploadableCount = count uploadButton.setOnClickListener { + // an explicit tap also re-opens the conflict sheet if the user + // postponed it with Cancel earlier + viewModel.requestConflictReview() if (count > 0) { if (viewModel.isConnected) { viewModel.upload() diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainModule.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainModule.kt index d020eebf72a..f8e054b6ddd 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainModule.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainModule.kt @@ -15,7 +15,7 @@ val mainModule = module { viewModel { MainViewModelImpl( get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), - get(), get(), get(), get(), get(), get(), get() + get(), get(), get(), get(), get(), get(), get(), get(), get() ) } viewModel { MultiSelectViewModel() } viewModel { EditHistoryViewModelImpl(get(), get(), get(named("FeatureDictionaryLazy"))) } diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainScreen.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainScreen.kt index bf882942e02..7e30bef50dd 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainScreen.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainScreen.kt @@ -52,9 +52,12 @@ import de.westnordost.streetcomplete.R import de.westnordost.streetcomplete.data.edithistory.Edit import de.westnordost.streetcomplete.data.edithistory.EditKey import de.westnordost.streetcomplete.data.messages.Message +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNotice +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflict import de.westnordost.streetcomplete.data.osm.geometry.ElementGeometry import de.westnordost.streetcomplete.data.osm.mapdata.BoundingBox import de.westnordost.streetcomplete.data.osm.mapdata.Element +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType import de.westnordost.streetcomplete.data.overlays.Overlay import de.westnordost.streetcomplete.data.quest.QuestType import de.westnordost.streetcomplete.data.urlconfig.UrlConfig @@ -73,6 +76,8 @@ import de.westnordost.streetcomplete.screens.main.controls.PointerPinButton import de.westnordost.streetcomplete.screens.main.controls.ScaleBar import de.westnordost.streetcomplete.screens.main.controls.ZoomButtons import de.westnordost.streetcomplete.screens.main.controls.findEllipsisIntersection +import de.westnordost.streetcomplete.screens.main.conflicts.DiscardedEditNoticeEffect +import de.westnordost.streetcomplete.screens.main.conflicts.TagConflictResolutionEffect import de.westnordost.streetcomplete.screens.main.edithistory.EditHistorySidebar import de.westnordost.streetcomplete.screens.main.edithistory.EditHistoryViewModel import de.westnordost.streetcomplete.screens.main.edithistory.EditItem @@ -159,6 +164,9 @@ fun MainScreen( val hasEdits by remember { derivedStateOf { editItems.isNotEmpty() } } val showZoomButtons by viewModel.showZoomButtons.collectAsState() + val pendingConflictsCount by viewModel.pendingConflictsCount.collectAsState() + val conflictReviewRequests by viewModel.conflictReviewRequests.collectAsState() + val discardedNoticesCount by viewModel.discardedNoticesCount.collectAsState() var showOverlaysDropdown by remember { mutableStateOf(false) } var showTeamModeWizard by remember { mutableStateOf(false) } @@ -505,6 +513,22 @@ fun MainScreen( lastUploadError?.let { error -> LastUploadErrorEffect(lastError = error, onReportError = ::sendErrorReport) } + TagConflictResolutionEffect( + pendingConflictsCount = pendingConflictsCount, + reviewRequests = conflictReviewRequests, + onPopNextConflictGroup = { viewModel.popNextConflictGroup() }, + onResolveKeepMine = { viewModel.resolveConflictKeepMine(it) }, + onResolveKeepTheirs = { viewModel.resolveConflictKeepTheirs(it) }, + onGetElementLabel = { type, id -> viewModel.getElementLabel(type, id) }, + // resolving unblocked the held edit - upload it right away + onResolutionFinished = { viewModel.upload() } + ) + DiscardedEditNoticeEffect( + discardedNoticesCount = discardedNoticesCount, + onPopNextDiscardedNotice = { viewModel.popNextDiscardedNotice() }, + onDismissDiscardedNotice = { viewModel.dismissDiscardedNotice(it) }, + onGetElementLabel = { type, id -> viewModel.getElementLabel(type, id) } + ) lastCrashReport?.let { report -> LastCrashEffect(lastReport = report, onReport = { context.sendErrorReportEmail(it) }) } @@ -668,6 +692,19 @@ object PreviewMainViewModel : MainViewModel() { get() = TODO("Not yet implemented") override val isUploadingOrDownloading: StateFlow get() = MutableStateFlow(true) + override val pendingConflictsCount: StateFlow + get() = MutableStateFlow(0) + override val conflictReviewRequests: StateFlow + get() = MutableStateFlow(0) + override fun requestConflictReview() {} + override suspend fun popNextConflictGroup(): List = emptyList() + override suspend fun resolveConflictKeepMine(conflict: PendingTagConflict) {} + override suspend fun resolveConflictKeepTheirs(conflict: PendingTagConflict) {} + override val discardedNoticesCount: StateFlow + get() = MutableStateFlow(0) + override suspend fun popNextDiscardedNotice(): DiscardedEditNotice? = null + override suspend fun dismissDiscardedNotice(notice: DiscardedEditNotice) {} + override suspend fun getElementLabel(type: ElementType, id: Long): String? = null override val isUserInitiatedDownloadInProgress: Boolean get() = TODO("Not yet implemented") override val isLoggedIn: StateFlow diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainViewModel.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainViewModel.kt index f5faf6c8443..e0ccfe7a177 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainViewModel.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainViewModel.kt @@ -3,7 +3,10 @@ package de.westnordost.streetcomplete.screens.main import androidx.compose.ui.geometry.Offset import androidx.lifecycle.ViewModel import de.westnordost.streetcomplete.data.messages.Message +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNotice +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflict import de.westnordost.streetcomplete.data.osm.mapdata.BoundingBox +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType import de.westnordost.streetcomplete.data.overlays.Overlay import de.westnordost.streetcomplete.data.quest.QuestType import de.westnordost.streetcomplete.data.urlconfig.UrlConfig @@ -58,6 +61,25 @@ abstract class MainViewModel : ViewModel() { abstract val isUploading: StateFlow abstract val isUploadingOrDownloading: StateFlow + /* tag conflicts held back instead of being discarded, to be resolved one at a time */ + abstract val pendingConflictsCount: StateFlow + /** Bumped each time the user explicitly asks to review pending conflicts (tapping the toolbar + * upload button), so a conflict sheet postponed via Cancel re-opens without an app restart */ + abstract val conflictReviewRequests: StateFlow + abstract fun requestConflictReview() + abstract suspend fun popNextConflictGroup(): List + abstract suspend fun resolveConflictKeepMine(conflict: PendingTagConflict) + abstract suspend fun resolveConflictKeepTheirs(conflict: PendingTagConflict) + + /* notices about edits discarded due to an unsalvageable conflict, to be acknowledged one at a time */ + abstract val discardedNoticesCount: StateFlow + abstract suspend fun popNextDiscardedNotice(): DiscardedEditNotice? + abstract suspend fun dismissDiscardedNotice(notice: DiscardedEditNotice) + + /** A short, human-readable label identifying an element (id, and name/ref if it has one), for + * display in the conflict/discard dialogs so the user can recognize which feature they're about */ + abstract suspend fun getElementLabel(type: ElementType, id: Long): String? + abstract val isUserInitiatedDownloadInProgress: Boolean abstract val isLoggedIn: StateFlow abstract val isConnected: Boolean diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainViewModelImpl.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainViewModelImpl.kt index c3161da564d..da25350a68c 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainViewModelImpl.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/MainViewModelImpl.kt @@ -8,10 +8,15 @@ import de.westnordost.streetcomplete.data.download.DownloadController import de.westnordost.streetcomplete.data.download.DownloadProgressSource import de.westnordost.streetcomplete.data.messages.Message import de.westnordost.streetcomplete.data.messages.MessagesSource +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNotice +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesController import de.westnordost.streetcomplete.data.osm.edits.EditType import de.westnordost.streetcomplete.data.osm.edits.ElementEdit import de.westnordost.streetcomplete.data.osm.edits.ElementEditsSource +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflict +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsController import de.westnordost.streetcomplete.data.osm.mapdata.BoundingBox +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEdit import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsSource @@ -72,6 +77,8 @@ class MainViewModelImpl( private val elementEditsSource: ElementEditsSource, private val noteEditsSource: NoteEditsSource, private val prefs: Preferences, + private val pendingTagConflictsController: PendingTagConflictsController, + private val discardedEditNoticesController: DiscardedEditNoticesController, ) : MainViewModel() { /* error handling */ @@ -263,8 +270,9 @@ class MainViewModelImpl( }.stateIn(viewModelScope, SharingStarted.Eagerly, downloadProgressSource.isDownloadInProgress) override val isUploadingOrDownloading: StateFlow = - combine(isUploading, isDownloading) { it1, it2 -> it1 || it2 } - .stateIn(viewModelScope, SharingStarted.Eagerly, false) + combine(isUploading, isDownloading) { uploading, downloading -> + uploading || downloading + }.stateIn(viewModelScope, SharingStarted.Eagerly, false) override val isUserInitiatedDownloadInProgress: Boolean get() = downloadProgressSource.isUserInitiatedDownloadInProgress @@ -289,6 +297,62 @@ class MainViewModelImpl( } } + /* tag conflicts blocking their edit from uploading until the user resolves them */ + + override val pendingConflictsCount: StateFlow = callbackFlow { + send(pendingTagConflictsController.getCount()) + val listener = object : PendingTagConflictsController.Listener { + override fun onAdded(conflict: PendingTagConflict) { trySend(pendingTagConflictsController.getCount()) } + override fun onRemoved(conflict: PendingTagConflict) { trySend(pendingTagConflictsController.getCount()) } + } + pendingTagConflictsController.addListener(listener) + awaitClose { pendingTagConflictsController.removeListener(listener) } + }.stateIn(viewModelScope + IO, SharingStarted.Eagerly, 0) + + override val conflictReviewRequests = MutableStateFlow(0) + + override fun requestConflictReview() { + conflictReviewRequests.value++ + } + + override suspend fun popNextConflictGroup(): List = withContext(IO) { + pendingTagConflictsController.getOldestGroup() + } + + // resolving is a local DB operation - the actual upload of the unblocked edit happens through + // the normal sync path (triggered by the conflict sheet once the whole group is resolved) and + // is what drives the progress spinner + override suspend fun resolveConflictKeepMine(conflict: PendingTagConflict) { + pendingTagConflictsController.resolveKeepMine(conflict) + } + + override suspend fun resolveConflictKeepTheirs(conflict: PendingTagConflict) { + pendingTagConflictsController.resolveKeepTheirs(conflict) + } + + /* notices about edits discarded due to an unsalvageable conflict */ + + override val discardedNoticesCount: StateFlow = callbackFlow { + send(discardedEditNoticesController.getCount()) + val listener = object : DiscardedEditNoticesController.Listener { + override fun onAdded(notice: DiscardedEditNotice) { trySend(discardedEditNoticesController.getCount()) } + override fun onRemoved(notice: DiscardedEditNotice) { trySend(discardedEditNoticesController.getCount()) } + } + discardedEditNoticesController.addListener(listener) + awaitClose { discardedEditNoticesController.removeListener(listener) } + }.stateIn(viewModelScope + IO, SharingStarted.Eagerly, 0) + + override suspend fun popNextDiscardedNotice(): DiscardedEditNotice? = withContext(IO) { + discardedEditNoticesController.getOldest() + } + + override suspend fun dismissDiscardedNotice(notice: DiscardedEditNotice) = withContext(IO) { + discardedEditNoticesController.dismiss(notice) + } + + override suspend fun getElementLabel(type: ElementType, id: Long): String? = + "${type.name.lowercase().replaceFirstChar { it.uppercase() }} #$id" + private val elementEditsListener = object : ElementEditsSource.Listener { override fun onAddedEdit(edit: ElementEdit) { launch { ensureLoggedIn() } } override fun onSyncedEdit(edit: ElementEdit) {} diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/conflicts/DiscardedEditNoticeEffect.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/conflicts/DiscardedEditNoticeEffect.kt new file mode 100644 index 00000000000..089b014d67c --- /dev/null +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/conflicts/DiscardedEditNoticeEffect.kt @@ -0,0 +1,77 @@ +package de.westnordost.streetcomplete.screens.main.conflicts + +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNotice +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType +import kotlinx.coroutines.launch + +/** + * Shows notices about edits that had to be discarded one at a time: the underlying element was + * deleted or changed too substantially in the meantime for the answer to still apply. Unlike + * [TagConflictResolutionEffect], there is nothing to decide here - the quest already disappeared + * from the map when it was answered, and without this, the user would only find out something + * went wrong much later when the quest silently reappears with no explanation. + * + * Driven by [discardedNoticesCount] rather than a one-shot event, same as [TagConflictResolutionEffect], + * so it naturally re-triggers and shows the next queued notice, one dialog at a time. + */ +@Composable +fun DiscardedEditNoticeEffect( + discardedNoticesCount: Int, + onPopNextDiscardedNotice: suspend () -> DiscardedEditNotice?, + onDismissDiscardedNotice: suspend (DiscardedEditNotice) -> Unit, + onGetElementLabel: suspend (ElementType, Long) -> String?, +) { + val scope = rememberCoroutineScope() + var current by remember { mutableStateOf(null) } + var elementLabel by remember { mutableStateOf(null) } + var isDismissing by remember { mutableStateOf(false) } + + LaunchedEffect(discardedNoticesCount, current) { + if (current == null && discardedNoticesCount > 0) { + val notice = onPopNextDiscardedNotice() + elementLabel = if (notice?.elementType != null && notice.elementId != null) { + onGetElementLabel(notice.elementType, notice.elementId) + } else null + current = notice + } + } + + val notice = current ?: return + + fun dismiss() { + if (isDismissing) return + isDismissing = true + scope.launch { + onDismissDiscardedNotice(notice) + isDismissing = false + current = null + } + } + + AlertDialog( + onDismissRequest = ::dismiss, + title = { Text("Answer could not be saved") }, + text = { + val where = elementLabel ?: "near here" + Text( + "Your answer for ${notice.editType.name} on $where could not be saved \n Reason: " + + "${notice.reason}." + ) + }, + confirmButton = { + TextButton(enabled = !isDismissing, onClick = ::dismiss) { + Text("OK") + } + } + ) +} diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/conflicts/TagConflictResolutionEffect.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/conflicts/TagConflictResolutionEffect.kt new file mode 100644 index 00000000000..7d1acff9eda --- /dev/null +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/main/conflicts/TagConflictResolutionEffect.kt @@ -0,0 +1,305 @@ +package de.westnordost.streetcomplete.screens.main.conflicts + +import android.util.Log +import androidx.compose.foundation.Image +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.verticalScroll +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Check +import androidx.compose.material3.Card +import androidx.compose.material3.CardDefaults +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.HorizontalDivider +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.ModalBottomSheet +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.material3.rememberModalBottomSheetState +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateMapOf +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.painterResource +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.unit.dp +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflict +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType +import de.westnordost.streetcomplete.quests.sidewalk_long_form.AddGenericLong +import kotlinx.coroutines.launch + +/** + * Shows all pending tag conflicts *of one held-back edit* together, in a single "Resolve + * Conflicts" bottom sheet: while answering a quest, a concurrent remote edit changed some of the + * same OSM tags on the same element. The whole answer is held back from uploading - nothing has + * been submitted yet. Each conflict is shown as the long-form question it came from (falling back + * to the raw tag key for non-long-form edits) with two tappable rows - the user's own answer + * (selected by default) and the value someone else set. Confirm folds the per-row choices into + * the held edit and it then uploads as one unit through the normal sync path. + * + * Cancel (or swiping the sheet away) postpones instead of resolving: the answer stays held (and + * counted in the toolbar badge as an unsynced edit), and the sheet comes back when the pending + * count changes (new conflict, sync), when the user taps the toolbar upload button + * ([reviewRequests] bumps), or on app restart. + * + * Confirm dismisses the sheet immediately; the decisions are applied locally in the background + * and [onResolutionFinished] then triggers the upload, whose progress shows in the toolbar + * spinner as usual. + * + * Driven by [pendingConflictsCount] rather than a one-shot event so it naturally re-triggers + * (fetching the next edit's group of conflicts) whenever the current one is applied, giving a + * strictly one-sheet-at-a-time queue even if several edits' conflicts piled up while the app + * was backgrounded. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun TagConflictResolutionEffect( + pendingConflictsCount: Int, + reviewRequests: Int, + onPopNextConflictGroup: suspend () -> List, + onResolveKeepMine: suspend (PendingTagConflict) -> Unit, + onResolveKeepTheirs: suspend (PendingTagConflict) -> Unit, + onGetElementLabel: suspend (ElementType, Long) -> String?, + onResolutionFinished: () -> Unit, +) { + val scope = rememberCoroutineScope() + var currentGroup by remember { mutableStateOf>(emptyList()) } + var elementLabel by remember { mutableStateOf(null) } + var isApplying by remember { mutableStateOf(false) } + // the count at which the user hit Cancel - suppresses re-fetching until the count changes + // (i.e. something new happened), the user taps the upload button, or the app restarts + var postponedAtCount by remember { mutableStateOf(null) } + var lastReviewRequests by remember { mutableStateOf(reviewRequests) } + val keepMine = remember { mutableStateMapOf() } + + LaunchedEffect(pendingConflictsCount, currentGroup, reviewRequests, isApplying) { + if (reviewRequests != lastReviewRequests) { + lastReviewRequests = reviewRequests + postponedAtCount = null + } + if (postponedAtCount != null && postponedAtCount != pendingConflictsCount) { + postponedAtCount = null + } + // !isApplying: the rows being resolved in the background are still in the DB until each + // one completes - fetching now would just pop the same group again + if (currentGroup.isEmpty() && pendingConflictsCount > 0 && postponedAtCount == null && !isApplying) { + val group = onPopNextConflictGroup() + keepMine.clear() + // default to keeping the user's own answer - they answered these, assume they still + // want them unless they pick the existing value + group.forEach { keepMine[it.id] = true } + elementLabel = group.firstOrNull()?.let { onGetElementLabel(it.elementType, it.elementId) } + currentGroup = group + } + } + + val group = currentGroup + if (group.isEmpty()) return + + fun postpone() { + postponedAtCount = pendingConflictsCount + currentGroup = emptyList() + } + + fun apply() { + if (isApplying) return + isApplying = true + val choices = group.map { it to (keepMine[it.id] ?: true) } + // dismiss right away - resolution continues in the background, surfaced via the toolbar + // spinner, and rows only leave the DB/badge as they actually resolve + currentGroup = emptyList() + scope.launch { + try { + for ((conflict, keep) in choices) { + if (keep) onResolveKeepMine(conflict) else onResolveKeepTheirs(conflict) + } + // the edit is unblocked now - push it right away instead of waiting for the + // next auto-sync + onResolutionFinished() + } catch (e: Exception) { + Log.w( + "TagConflictResolution", + "Resolving conflicts failed, unresolved ones stay pending", e + ) + } finally { + isApplying = false + } + } + } + + val editType = group.first().editType + val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) + + ModalBottomSheet( + onDismissRequest = ::postpone, + sheetState = sheetState, + shape = RoundedCornerShape(topStart = 24.dp, topEnd = 24.dp), + containerColor = MaterialTheme.colorScheme.surface, + ) { + Column( + modifier = Modifier + .fillMaxWidth() + .verticalScroll(rememberScrollState()) + .padding(start = 20.dp, end = 20.dp, bottom = 24.dp) + ) { + Box(modifier = Modifier.fillMaxWidth()) { + TextButton( + onClick = ::postpone, + modifier = Modifier.align(Alignment.CenterStart) + ) { + Text("Cancel") + } + Text( + "Resolve Conflicts", + style = MaterialTheme.typography.titleMedium.copy(fontWeight = FontWeight.Bold), + modifier = Modifier.align(Alignment.Center) + ) + TextButton( + onClick = ::apply, + modifier = Modifier.align(Alignment.CenterEnd) + ) { + Text("Confirm", fontWeight = FontWeight.Bold) + } + } + + Spacer(Modifier.height(12.dp)) + + Card( + colors = CardDefaults.cardColors( + containerColor = MaterialTheme.colorScheme.surfaceVariant.copy(alpha = 0.4f) + ), + modifier = Modifier.fillMaxWidth() + ) { + Column(modifier = Modifier.padding(16.dp)) { + Row(verticalAlignment = Alignment.CenterVertically) { + Image( + painter = painterResource(editType.icon), + contentDescription = null, + modifier = Modifier.size(40.dp) + ) + Column(modifier = Modifier.padding(start = 12.dp)) { + // same category name the long-form quest title shows + // ("Sidewalks — Way #123"), so the user can relate this sheet back + // to the quest they answered; editType.title would be the question + // sentence ("Does this street have a sidewalk?") instead + Text( + (editType as? AddGenericLong)?.item?.elementType + ?: stringResource(editType.title), + style = MaterialTheme.typography.titleMedium.copy(fontWeight = FontWeight.Bold) + ) + elementLabel?.let { + Text( + it, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + } + } + HorizontalDivider(modifier = Modifier.padding(vertical = 12.dp)) + Text( + "This element was changed by someone else while you were answering, so " + + "your answer has not been submitted yet. Choose which value to keep for " + + "each question below - everything is submitted together once you confirm.", + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + } + + group.forEach { conflict -> + Spacer(Modifier.height(20.dp)) + + // the long-form question this tag came from reads much better than the raw OSM + // key; the key is still shown underneath so the value can be traced in OSM data + val question = questionTitleFor(conflict) + Text( + question ?: conflict.tagKey.uppercase(), + style = MaterialTheme.typography.titleSmall, + modifier = Modifier.padding(horizontal = 4.dp) + ) + if (question != null) { + Text( + conflict.tagKey.uppercase(), + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 4.dp, vertical = 2.dp) + ) + } + Spacer(Modifier.height(6.dp)) + + val mine = keepMine[conflict.id] ?: true + Card( + colors = CardDefaults.cardColors( + containerColor = MaterialTheme.colorScheme.surfaceVariant.copy(alpha = 0.4f) + ), + modifier = Modifier.fillMaxWidth() + ) { + ConflictChoiceRow( + text = "Your answer: ${conflict.mineValue ?: "(removed)"}", + selected = mine, + onClick = { keepMine[conflict.id] = true } + ) + HorizontalDivider(modifier = Modifier.padding(start = 16.dp)) + ConflictChoiceRow( + text = "Existing value: ${conflict.theirsValueAtDetection ?: "(removed)"}", + selected = !mine, + onClick = { keepMine[conflict.id] = false } + ) + } + } + } + } +} + +@Composable +private fun ConflictChoiceRow( + text: String, + selected: Boolean, + onClick: () -> Unit, +) { + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier + .fillMaxWidth() + .clickable(onClick = onClick) + .padding(horizontal = 16.dp, vertical = 14.dp) + ) { + Text( + text, + style = MaterialTheme.typography.bodyLarge, + modifier = Modifier.weight(1f) + ) + if (selected) { + Icon( + Icons.Default.Check, + contentDescription = "Selected", + tint = MaterialTheme.colorScheme.primary + ) + } + } +} + +/** The long-form question text the conflicting tag was answered through, if this conflict came + * from a long-form quest ([AddGenericLong] carries its workspace-defined question catalog) */ +private fun questionTitleFor(conflict: PendingTagConflict): String? = + (conflict.editType as? AddGenericLong)?.item?.quests + ?.firstOrNull { it?.questTag == conflict.tagKey } + ?.questTitle diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkspaceLoginScreen.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkspaceLoginScreen.kt index 0ca1640270b..c2bca6e7479 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkspaceLoginScreen.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkspaceLoginScreen.kt @@ -15,13 +15,16 @@ import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size +import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.ArrowDropDown import androidx.compose.material.icons.filled.Fingerprint @@ -44,6 +47,7 @@ import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Surface import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.MutableState @@ -58,7 +62,6 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip -import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalFocusManager @@ -310,13 +313,16 @@ fun LoginCard( onClick = { focusManager.clearFocus() } ) ) { - Column(verticalArrangement = Arrangement.SpaceEvenly) { + Column( + verticalArrangement = Arrangement.SpaceEvenly, + modifier = Modifier.fillMaxSize() + ) { Column( verticalArrangement = Arrangement.Center, horizontalAlignment = Alignment.CenterHorizontally, modifier = modifier .fillMaxWidth() - .fillMaxHeight(0.3f) + .weight(0.3f) .clearAndSetSemantics {} .background(MaterialTheme.colorScheme.surfaceVariant) ) { @@ -352,142 +358,186 @@ fun LoginCard( Column( horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.spacedBy(18.dp), modifier = modifier - .fillMaxSize() - .padding(horizontal = 48.dp) - .semantics { - contentDescription = screenTitle - } + .weight(0.7f) + .fillMaxWidth() + .imePadding() ) { - val context = LocalContext.current - var visibility by rememberSaveable { mutableStateOf(false) } - OutlinedTextField( - value = email.value, onValueChange = { newText -> email.value = newText }, - label = { - Text( - text = stringResource( - id = R.string.email - ), - color = MaterialTheme.colorScheme.onSurface, - ) - }, - keyboardOptions = KeyboardOptions( - keyboardType = KeyboardType.Email, - imeAction = ImeAction.Next - ), - trailingIcon = { - if (preferences.isBiometricEnabled) { - val coroutineScope = rememberCoroutineScope() - val creds = SecureCredentialStorage.getCredential( - LocalContext.current, - selectedEnvironment.value.name + Column( + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.spacedBy(18.dp), + modifier = Modifier + .weight(1f) + .fillMaxWidth() + .verticalScroll(rememberScrollState()) + .padding(horizontal = 48.dp) + .semantics { + contentDescription = screenTitle + } + ) { + val context = LocalContext.current + var visibility by rememberSaveable { mutableStateOf(false) } + OutlinedTextField( + value = email.value, onValueChange = { newText -> email.value = newText }, + label = { + Text( + text = stringResource( + id = R.string.email + ), + color = MaterialTheme.colorScheme.onSurface, ) - if (creds != null) { - IconButton(onClick = { - coroutineScope.launch { - val authenticated = authenticateWithBiometrics( - context, - activity = activity - ) - if (!authenticated) { - Toast.makeText( + }, + keyboardOptions = KeyboardOptions( + keyboardType = KeyboardType.Email, + imeAction = ImeAction.Next + ), + trailingIcon = { + if (preferences.isBiometricEnabled) { + val coroutineScope = rememberCoroutineScope() + val creds = SecureCredentialStorage.getCredential( + LocalContext.current, + selectedEnvironment.value.name + ) + if (creds != null) { + IconButton(onClick = { + coroutineScope.launch { + val authenticated = authenticateWithBiometrics( context, - "Failed to authenticate", - Toast.LENGTH_SHORT + activity = activity ) - .show() - } else { - email.value = creds.username - password.value = creds.password - viewModel.loginToWorkspace(email.value, password.value) + if (!authenticated) { + Toast.makeText( + context, + "Failed to authenticate", + Toast.LENGTH_SHORT + ) + .show() + } else { + email.value = creds.username + password.value = creds.password + viewModel.loginToWorkspace( + email.value, + password.value + ) + } } + }) { + Icon( + imageVector = Icons.Default.Fingerprint, + contentDescription = "Login with Device Authentication" + ) } - }) { - Icon( - imageVector = Icons.Default.Fingerprint, - contentDescription = "Login with Device Authentication" - ) } } - } - }, - colors = OutlinedTextFieldDefaults.colors( - focusedLabelColor = MaterialTheme.colorScheme.onSurface, - unfocusedLabelColor = MaterialTheme.colorScheme.onSurface, - focusedTextColor = MaterialTheme.colorScheme.onSurface, - unfocusedTextColor = MaterialTheme.colorScheme.onSurface - ), - modifier = Modifier - .fillMaxWidth() - ) - OutlinedTextField( - value = password.value, - onValueChange = { newText -> password.value = newText }, - label = { - Text( - text = stringResource( - id = R.string.password - ), - color = MaterialTheme.colorScheme.onSurface - ) - }, - visualTransformation = if (visibility) VisualTransformation.None else PasswordVisualTransformation(), - keyboardOptions = KeyboardOptions( - keyboardType = KeyboardType.Password, - imeAction = ImeAction.Done - ), - trailingIcon = { - val image = - if (visibility) Icons.Default.Visibility else Icons.Default.VisibilityOff - IconButton(onClick = { visibility = !visibility }) { - Icon( - imageVector = image, - contentDescription = "Toggle password visibility" + }, + colors = OutlinedTextFieldDefaults.colors( + focusedLabelColor = MaterialTheme.colorScheme.onSurface, + unfocusedLabelColor = MaterialTheme.colorScheme.onSurface, + focusedTextColor = MaterialTheme.colorScheme.onSurface, + unfocusedTextColor = MaterialTheme.colorScheme.onSurface + ), + modifier = Modifier + .fillMaxWidth() + ) + OutlinedTextField( + value = password.value, + onValueChange = { newText -> password.value = newText }, + label = { + Text( + text = stringResource( + id = R.string.password + ), + color = MaterialTheme.colorScheme.onSurface + ) + }, + visualTransformation = if (visibility) VisualTransformation.None else PasswordVisualTransformation(), + keyboardOptions = KeyboardOptions( + keyboardType = KeyboardType.Password, + imeAction = ImeAction.Done + ), + trailingIcon = { + val image = + if (visibility) Icons.Default.Visibility else Icons.Default.VisibilityOff + IconButton(onClick = { visibility = !visibility }) { + Icon( + imageVector = image, + contentDescription = "Toggle password visibility" + ) + } + }, + colors = OutlinedTextFieldDefaults.colors( + focusedLabelColor = MaterialTheme.colorScheme.onSurface, + unfocusedLabelColor = MaterialTheme.colorScheme.onSurface, + focusedTextColor = MaterialTheme.colorScheme.onSurface, + unfocusedTextColor = MaterialTheme.colorScheme.onSurface + ), + modifier = Modifier + .fillMaxWidth() + ) + + if (isDebugModeEnabled) { + EnvironmentDropdownMenu(viewModel, selectedEnvironment, modifier = Modifier) + } + Spacer(modifier = Modifier.height(8.dp)) + + Column( + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.spacedBy(4.dp), + modifier = Modifier.fillMaxWidth() + ) { + Button( + onClick = { + if (email.value.isEmpty() || password.value.isEmpty()) { + Toast.makeText( + context, + "Please enter email and password", + Toast.LENGTH_SHORT + ).show() + return@Button + } + viewModel.loginToWorkspace(email.value, password.value) + }, modifier = Modifier.fillMaxWidth() + ) { + Text( + text = "Login", + style = MaterialTheme.typography.titleMedium, + modifier = Modifier.padding(vertical = 4.dp) ) } - }, - colors = OutlinedTextFieldDefaults.colors( - focusedLabelColor = MaterialTheme.colorScheme.onSurface, - unfocusedLabelColor = MaterialTheme.colorScheme.onSurface, - focusedTextColor = MaterialTheme.colorScheme.onSurface, - unfocusedTextColor = MaterialTheme.colorScheme.onSurface - ), - modifier = Modifier - .fillMaxWidth() - ) - if (isDebugModeEnabled) { - EnvironmentDropdownMenu(viewModel, selectedEnvironment, modifier = Modifier) - } - Button( - onClick = { - if (email.value.isEmpty() || password.value.isEmpty()) { - Toast.makeText( - context, - "Please enter email and password", - Toast.LENGTH_SHORT - ).show() - return@Button + + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.Center + ) { + TextButton(onClick = { + // open URL in browser + val url = + selectedEnvironment.value.tdeiWebUrl + "/ForgotPassword" + val intent = Intent(Intent.ACTION_VIEW) + intent.data = url.toUri() + context.startActivity(intent) + }) { + Text( + "Forgot password?", + style = MaterialTheme.typography.bodyLarge.copy( + color = MaterialTheme.colorScheme.primary, + fontWeight = FontWeight.Bold + ) + ) + } } - viewModel.loginToWorkspace(email.value, password.value) - }, modifier = Modifier - .fillMaxWidth() - .padding(vertical = 16.dp) - ) { - Text( - text = "Login", - style = MaterialTheme.typography.titleMedium, - modifier = Modifier.padding(vertical = 4.dp) - ) + } + Spacer(modifier = Modifier.height(24.dp)) + + UserInfoComponent() } - UserInfoComponent() + DebuggableBuild( viewModel, selectedEnvironment, preferences, modifier = modifier ) - } } } @@ -497,7 +547,7 @@ fun LoginCard( fun UserInfoComponent() { Column( modifier = Modifier.fillMaxWidth(), - verticalArrangement = Arrangement.spacedBy(16.dp), + verticalArrangement = Arrangement.spacedBy(8.dp), horizontalAlignment = Alignment.CenterHorizontally ) { val context = LocalContext.current @@ -509,7 +559,10 @@ fun UserInfoComponent() { intent.data = url.toUri() context.startActivity(intent) }, - style = MaterialTheme.typography.bodyLarge.copy(color = MaterialTheme.colorScheme.primary) + style = MaterialTheme.typography.bodyLarge.copy( + color = MaterialTheme.colorScheme.primary, + fontWeight = FontWeight.Bold + ) ) Text( @@ -533,7 +586,10 @@ fun UserInfoComponent() { } } .padding(top = 16.dp), - style = MaterialTheme.typography.bodyLarge.copy(color = MaterialTheme.colorScheme.primary) + style = MaterialTheme.typography.bodyLarge.copy( + color = MaterialTheme.colorScheme.primary, + fontWeight = FontWeight.Bold + ) ) Text( @@ -545,7 +601,10 @@ fun UserInfoComponent() { intent.data = url.toUri() context.startActivity(intent) }, - style = MaterialTheme.typography.bodyLarge.copy(color = MaterialTheme.colorScheme.primary) + style = MaterialTheme.typography.bodyLarge.copy( + color = MaterialTheme.colorScheme.primary, + fontWeight = FontWeight.Bold + ) ) } } @@ -562,10 +621,8 @@ fun DebuggableBuild( var clickCount by remember { mutableIntStateOf(0) } Box( - modifier = modifier - .fillMaxWidth() - .fillMaxHeight(), - contentAlignment = Alignment.BottomCenter + modifier = modifier.fillMaxWidth(), + contentAlignment = Alignment.Center ) { Column( horizontalAlignment = Alignment.CenterHorizontally, diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkspaceViewModel.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkspaceViewModel.kt index 385ff245e35..148ecb409c7 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkspaceViewModel.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkspaceViewModel.kt @@ -4,6 +4,7 @@ import android.location.Location import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import de.westnordost.streetcomplete.BuildConfig +import de.westnordost.streetcomplete.data.download.tiles.DownloadedTilesController import de.westnordost.streetcomplete.data.elementfilter.ParseException import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression import de.westnordost.streetcomplete.data.preferences.Environment @@ -56,6 +57,7 @@ abstract class WorkspaceViewModel : ViewModel() { class WorkspaceViewModelImpl( private val workspaceRepository: WorkspaceRepository, private val preferences: Preferences, + private val downloadedTilesController: DownloadedTilesController, ) : WorkspaceViewModel() { val isLoggedIn: Boolean = preferences.workspaceLogin @@ -74,6 +76,10 @@ class WorkspaceViewModelImpl( _selectedWorkspace.value = (showWorkspaces.value as WorkspaceListState.Success).workspaces .filter { it.externalAppAccess == 1 && it.type == "osw" }[index] preferences.workspaceId = _selectedWorkspace.value?.id + // the "has this area already been downloaded" bookkeeping isn't scoped per workspace, so + // without this, switching workspaces while looking at the same map area makes auto-download + // wrongly think the new workspace's data is already fresh and skip fetching it + downloadedTilesController.invalidateAll() } private var userLocation: Location? = null diff --git a/app/src/androidMain/res/layout/form_leave_note.xml b/app/src/androidMain/res/layout/form_leave_note.xml index efc9130b0d5..d92be965f84 100644 --- a/app/src/androidMain/res/layout/form_leave_note.xml +++ b/app/src/androidMain/res/layout/form_leave_note.xml @@ -24,4 +24,11 @@ android:background="@drawable/background_edittext_outline" /> + + diff --git a/app/src/androidMain/res/values/conf.xml b/app/src/androidMain/res/values/conf.xml index c5e57139d76..2697885525a 100644 --- a/app/src/androidMain/res/values/conf.xml +++ b/app/src/androidMain/res/values/conf.xml @@ -1,4 +1,4 @@ - de.westnordost.streetcomplete.fileprovider + net.opentoall.aviv.scoutroute.fileprovider diff --git a/app/src/androidMain/res/xml/file_paths.xml b/app/src/androidMain/res/xml/file_paths.xml index 195dcc2cab0..bc55a97853d 100644 --- a/app/src/androidMain/res/xml/file_paths.xml +++ b/app/src/androidMain/res/xml/file_paths.xml @@ -1,4 +1,6 @@ - + + diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/DatabaseInitializer.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/DatabaseInitializer.kt index 35512450756..ccf96d866bd 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/DatabaseInitializer.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/DatabaseInitializer.kt @@ -3,9 +3,11 @@ package de.westnordost.streetcomplete.data import de.westnordost.streetcomplete.data.download.tiles.DownloadedTilesTable import de.westnordost.streetcomplete.data.logs.LogsTable import de.westnordost.streetcomplete.data.osm.created_elements.CreatedElementsTable +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable import de.westnordost.streetcomplete.data.osm.edits.EditElementsTable import de.westnordost.streetcomplete.data.osm.edits.ElementEditsTable import de.westnordost.streetcomplete.data.osm.edits.ElementIdProviderTable +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable import de.westnordost.streetcomplete.data.osm.edits.upload.changesets.OpenChangesetsTable import de.westnordost.streetcomplete.data.osm.geometry.RelationGeometryTable import de.westnordost.streetcomplete.data.osm.geometry.WayGeometryTable @@ -30,7 +32,7 @@ import de.westnordost.streetcomplete.util.logs.Log /** Creates the database and upgrades it */ object DatabaseInitializer { - const val DB_VERSION = 23 + const val DB_VERSION = 24 fun onCreate(db: Database) { // OSM notes @@ -70,6 +72,12 @@ object DatabaseInitializer { db.exec(CreatedElementsTable.CREATE) + // tag conflicts held back for the user to resolve instead of being discarded + db.exec(PendingTagConflictsTable.CREATE) + + // notices about edits that had to be discarded (unsalvageable structural conflicts) + db.exec(DiscardedEditNoticesTable.CREATE) + // quests db.exec(VisibleEditTypeTable.CREATE) db.exec(QuestTypeOrderTable.CREATE) @@ -305,6 +313,14 @@ object DatabaseInitializer { db.tryExec("ALTER TABLE osm_note_edits ADD COLUMN workspace_id INTEGER DEFAULT 0 NOT NULL") db.tryExec("ALTER TABLE osm_notes_hidden ADD COLUMN workspace_id INTEGER DEFAULT 0 NOT NULL") } + + if (oldVersion <= 23 && newVersion >= 24) { + // both tables of the conflict-resolution feature (developed together, released together) + db.exec(PendingTagConflictsTable.CREATE) + db.exec(DiscardedEditNoticesTable.CREATE) + // edits with unresolved tag conflicts are held back from uploading via this flag + db.tryExec("ALTER TABLE osm_element_edits ADD COLUMN blocked_on_conflict int NOT NULL DEFAULT 0") + } } } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClient.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClient.kt new file mode 100644 index 00000000000..a4a6b708416 --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/KartaViewApiClient.kt @@ -0,0 +1,138 @@ +package de.westnordost.streetcomplete.data.karta_view + +import de.westnordost.streetcomplete.data.karta_view.domain.model.CreateSequenceResponse +import de.westnordost.streetcomplete.data.karta_view.domain.model.PhotoLookupResponse +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon +import de.westnordost.streetcomplete.data.preferences.Preferences +import de.westnordost.streetcomplete.util.logs.Log +import io.ktor.client.HttpClient +import io.ktor.client.call.body +import io.ktor.client.request.forms.MultiPartFormDataContent +import io.ktor.client.request.forms.formData +import io.ktor.client.request.get +import io.ktor.client.request.parameter +import io.ktor.client.request.post +import io.ktor.client.request.setBody +import io.ktor.http.Headers +import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode +import kotlinx.io.buffered +import kotlinx.io.files.FileSystem +import kotlinx.io.files.Path +import kotlinx.io.readByteArray + +/** Uploads photos to KartaView (api.openstreetcam.org). */ +class KartaViewApiClient( + private val fileSystem: FileSystem, + private val httpClient: HttpClient, + private val prefs: Preferences, +) { + /** Uploads the image files at [imagePaths] into one new KartaView sequence at [position], + * in list order. Paths that don't exist are skipped. Returns the lth photo URL for each + * uploaded image, in the same order. + * + * @throws KartaViewException naming the step that failed */ + suspend fun upload(imagePaths: List, position: LatLon, bearing: Float = 0f): List { + val images = imagePaths.mapNotNull { path -> + val file = Path(path) + if (fileSystem.exists(file)) fileSystem.source(file).buffered().readByteArray() else null + } + return uploadImages(images, position, bearing) + } + + /** Uploads [images] (JPEG-encoded) into one new KartaView sequence at [position], in list + * order (sequenceIndex 1..n), closes the sequence and returns the lth photo URL for each + * image, in the same order. + * + * @throws KartaViewException naming the step that failed */ + suspend fun uploadImages(images: List, position: LatLon, bearing: Float = 0f): List { + if (images.isEmpty()) return emptyList() + val sequenceId = createSequence() + images.forEachIndexed { index, image -> + uploadPhoto(sequenceId, index + 1, image, position, bearing) + } + closeSequence(sequenceId) + return List(images.size) { index -> getPhotoLthUrl(sequenceId, index + 1) } + } + + private suspend fun createSequence(): String { + val response = httpClient.post(BASE_URL + "1.0/sequence/") { + setBody(MultiPartFormDataContent(formData { + append("access_token", prefs.kartaViewAccessToken) + })) + } + if (response.status == HttpStatusCode.OK) { + val sequence = response.body() + Log.d(TAG, "Sequence created: ${sequence.status.httpMessage}") + sequence.osv.sequence?.id?.let { return it } + } + throw KartaViewException( + "Failed to create KartaView sequence. Image upload failed. Please try again later " + response.status + ) + } + + private suspend fun uploadPhoto( + sequenceId: String, + sequenceIndex: Int, + image: ByteArray, + position: LatLon, + bearing: Float, + ) { + val response = httpClient.post(BASE_URL + "1.0/photo/") { + setBody(MultiPartFormDataContent(formData { + append("access_token", prefs.kartaViewAccessToken) + append("sequenceId", sequenceId) + append("sequenceIndex", sequenceIndex) + append("coordinate", "${position.latitude},${position.longitude}") + append("headers", bearing.toInt().toString()) + append("photo", image, Headers.build { + append(HttpHeaders.ContentType, "image/jpeg") + append(HttpHeaders.ContentDisposition, "filename=\"wework-kartaview.jpg\"") + }) + })) + } + if (response.status != HttpStatusCode.OK) { + throw KartaViewException( + "Failed to upload image to KartaView. Please try again later " + response.status + ) + } + Log.d(TAG, "Image $sequenceIndex uploaded to sequence $sequenceId") + } + + private suspend fun closeSequence(sequenceId: String) { + val response = httpClient.post(BASE_URL + "1.0/sequence/finished-uploading/") { + setBody(MultiPartFormDataContent(formData { + append("access_token", prefs.kartaViewAccessToken) + append("sequenceId", sequenceId) + })) + } + if (response.status != HttpStatusCode.OK) { + throw KartaViewException( + "Failed to close KartaView Sequence. Please try again later " + response.status + ) + } + Log.d(TAG, "Sequence closed: ${response.body().status.httpMessage}") + } + + private suspend fun getPhotoLthUrl(sequenceId: String, sequenceIndex: Int): String { + val response = httpClient.get(BASE_URL + "2.0/photo/") { + parameter("access_token", prefs.kartaViewAccessToken) + parameter("sequenceId", sequenceId) + parameter("sequenceIndex", sequenceIndex) + } + if (response.status == HttpStatusCode.OK) { + val url = response.body().result?.data?.firstOrNull()?.imageLthUrl + if (url != null) return url + } + throw KartaViewException( + "Failed to retrieve photo URL. Please try again later " + response.status + ) + } + + companion object { + private const val TAG = "KartaViewApiClient" + private const val BASE_URL = "https://api.openstreetcam.org/" + } +} + +class KartaViewException(message: String) : RuntimeException(message) diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/CreateSequenceResponse.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/CreateSequenceResponse.kt similarity index 100% rename from app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/CreateSequenceResponse.kt rename to app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/CreateSequenceResponse.kt diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/ImageUploadResponse.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/ImageUploadResponse.kt similarity index 100% rename from app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/ImageUploadResponse.kt rename to app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/ImageUploadResponse.kt diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Osv.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Osv.kt similarity index 100% rename from app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Osv.kt rename to app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Osv.kt diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/OsvX.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/OsvX.kt similarity index 100% rename from app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/OsvX.kt rename to app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/OsvX.kt diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Photo.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Photo.kt similarity index 100% rename from app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Photo.kt rename to app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Photo.kt diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/PhotoLookupResponse.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/PhotoLookupResponse.kt similarity index 100% rename from app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/PhotoLookupResponse.kt rename to app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/PhotoLookupResponse.kt diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Sequence.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Sequence.kt similarity index 100% rename from app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Sequence.kt rename to app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Sequence.kt diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Status.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Status.kt similarity index 100% rename from app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Status.kt rename to app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/Status.kt diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/StatusX.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/StatusX.kt similarity index 100% rename from app/src/androidMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/StatusX.kt rename to app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/karta_view/domain/model/StatusX.kt diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNotice.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNotice.kt new file mode 100644 index 00000000000..2dac574250e --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNotice.kt @@ -0,0 +1,22 @@ +package de.westnordost.streetcomplete.data.osm.edits + +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon + +/** A local edit that could not be applied because the underlying element was deleted or changed + * too substantially in the meantime (see ElementEditsUploader). Unlike a [de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflict], + * there is nothing to resolve here - the edit is unsalvageable - so this is purely informational: + * it lets the user find out their answer was discarded instead of the quest just silently + * reappearing later with no explanation. */ +data class DiscardedEditNotice( + var id: Long, + val editType: ElementEditType, + /** null if the action this edit was for didn't refer to any existing element (e.g. creating a + * new node) - there's nothing to identify in that case */ + val elementType: ElementType?, + val elementId: Long?, + val position: LatLon, + val reason: String, + val createdTimestamp: Long, + var workspaceId: Int = 0 +) diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNoticesController.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNoticesController.kt new file mode 100644 index 00000000000..34d270b368e --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNoticesController.kt @@ -0,0 +1,37 @@ +package de.westnordost.streetcomplete.data.osm.edits + +import de.westnordost.streetcomplete.util.Listeners + +/** Holds notices about edits that had to be discarded because the underlying element was deleted + * or changed too substantially in the meantime (see ElementEditsUploader) - purely informational, + * there is nothing to resolve, unlike [de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsController]. + * Lets the user find out an answer was lost instead of the quest just silently reappearing later. */ +class DiscardedEditNoticesController( + private val dao: DiscardedEditNoticesDao, +) { + interface Listener { + fun onAdded(notice: DiscardedEditNotice) + fun onRemoved(notice: DiscardedEditNotice) + } + private val listeners = Listeners() + + fun addListener(listener: Listener) { listeners.add(listener) } + fun removeListener(listener: Listener) { listeners.remove(listener) } + + fun add(notice: DiscardedEditNotice) { + val added = dao.add(notice) + listeners.forEach { it.onAdded(added) } + } + + fun getAll(): List = dao.getAll() + + fun getCount(): Int = dao.getCount() + + /** Returns the oldest notice without removing it - it stays queryable/re-showable until + * actually dismissed, so it survives the app being killed before the user sees it */ + fun getOldest(): DiscardedEditNotice? = dao.getAll().firstOrNull() + + fun dismiss(notice: DiscardedEditNotice) { + if (dao.delete(notice.id)) listeners.forEach { it.onRemoved(notice) } + } +} diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNoticesDao.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNoticesDao.kt new file mode 100644 index 00000000000..b11e998669c --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNoticesDao.kt @@ -0,0 +1,71 @@ +package de.westnordost.streetcomplete.data.osm.edits + +import de.westnordost.streetcomplete.data.AllEditTypes +import de.westnordost.streetcomplete.data.CursorPosition +import de.westnordost.streetcomplete.data.Database +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.Columns.CREATED_TIMESTAMP +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.Columns.ELEMENT_ID +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.Columns.ELEMENT_TYPE +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.Columns.ID +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.Columns.LATITUDE +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.Columns.LONGITUDE +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.Columns.QUEST_TYPE +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.Columns.REASON +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.Columns.WORKSPACE_ID +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesTable.NAME +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon +import de.westnordost.streetcomplete.data.preferences.Preferences + +class DiscardedEditNoticesDao( + private val db: Database, + private val allEditTypes: AllEditTypes, + private val preferences: Preferences? = null, +) { + private val workspaceId + get() = preferences?.workspaceId ?: 0 + + fun add(notice: DiscardedEditNotice): DiscardedEditNotice { + val rowId = db.insert(NAME, notice.toPairs()) + return notice.copy(id = rowId, workspaceId = workspaceId) + } + + fun getAll(): List = + db.query( + NAME, + where = "$WORKSPACE_ID = $workspaceId", + orderBy = CREATED_TIMESTAMP + ) { it.toDiscardedEditNotice() } + + fun getCount(): Int = + db.queryOne( + NAME, + columns = arrayOf("COUNT(*) AS count"), + where = "$WORKSPACE_ID = $workspaceId" + ) { it.getInt("count") } ?: 0 + + fun delete(id: Long): Boolean = + db.delete(NAME, "$WORKSPACE_ID = $workspaceId AND $ID = $id") == 1 + + private fun DiscardedEditNotice.toPairs(): List> = listOf( + QUEST_TYPE to editType.name, + ELEMENT_TYPE to elementType?.name, + ELEMENT_ID to elementId, + REASON to reason, + LATITUDE to position.latitude, + LONGITUDE to position.longitude, + CREATED_TIMESTAMP to createdTimestamp, + WORKSPACE_ID to workspaceId + ) + + private fun CursorPosition.toDiscardedEditNotice() = DiscardedEditNotice( + id = getLong(ID), + editType = allEditTypes.getByName(getString(QUEST_TYPE)) as ElementEditType, + elementType = getStringOrNull(ELEMENT_TYPE)?.let { ElementType.valueOf(it) }, + elementId = getLongOrNull(ELEMENT_ID), + reason = getString(REASON), + position = LatLon(getDouble(LATITUDE), getDouble(LONGITUDE)), + createdTimestamp = getLong(CREATED_TIMESTAMP), + workspaceId = getInt(WORKSPACE_ID) + ) +} diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNoticesTable.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNoticesTable.kt new file mode 100644 index 00000000000..325f244b81b --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/DiscardedEditNoticesTable.kt @@ -0,0 +1,31 @@ +package de.westnordost.streetcomplete.data.osm.edits + +object DiscardedEditNoticesTable { + const val NAME = "osm_discarded_edit_notices" + + object Columns { + const val ID = "id" + const val QUEST_TYPE = "quest_type" + const val ELEMENT_TYPE = "element_type" + const val ELEMENT_ID = "element_id" + const val REASON = "reason" + const val LATITUDE = "latitude" + const val LONGITUDE = "longitude" + const val CREATED_TIMESTAMP = "created" + const val WORKSPACE_ID = "workspace_id" + } + + const val CREATE = """ + CREATE TABLE $NAME ( + ${Columns.ID} INTEGER PRIMARY KEY AUTOINCREMENT, + ${Columns.QUEST_TYPE} varchar(255) NOT NULL, + ${Columns.ELEMENT_TYPE} varchar(255), + ${Columns.ELEMENT_ID} int, + ${Columns.REASON} varchar(255) NOT NULL, + ${Columns.LATITUDE} double NOT NULL, + ${Columns.LONGITUDE} double NOT NULL, + ${Columns.CREATED_TIMESTAMP} int NOT NULL, + ${Columns.WORKSPACE_ID} int NOT NULL + ); + """ +} diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEdit.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEdit.kt index 91f49c831a7..8e584adc1f3 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEdit.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEdit.kt @@ -31,7 +31,13 @@ data class ElementEdit( /** Whether the user was near the element that is being edited when this edit was created */ val isNearUserLocation: Boolean, - override var workspaceId: Int = 0 + override var workspaceId: Int = 0, + + /** Whether this (unsynced) edit is excluded from uploading because some of its tag changes + * collided with a concurrent remote edit - nothing of it is uploaded until the user has + * resolved all its pending tag conflicts. The edit still counts as unsynced and is still + * applied to the local map view in the meantime. */ + val isBlockedOnConflict: Boolean = false, ) : Edit { override val isUndoable: Boolean get() = !isSynced || action is IsActionRevertable override val key: ElementEditKey get() = ElementEditKey(id) diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsController.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsController.kt index 68b1b8160e4..4b0ecc9f711 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsController.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsController.kt @@ -103,6 +103,24 @@ class ElementEditsController( elementIdProviderDB.updateIds(elementUpdates.idUpdates) } + /** Replace the stored version of the given edit, e.g. when a pending tag conflict resolution + * changed which tag changes it should upload. Must not change which elements the edit + * concerns. */ + fun updateAction(edit: ElementEdit) { + lock.withLock { editsDB.put(edit) } + } + + /** Exclude the given (unsynced) edit from uploading until its pending tag conflicts are + * resolved. It stays unsynced, applied to the local map view, and undoable. */ + fun markBlockedOnConflict(edit: ElementEdit) { + lock.withLock { editsDB.put(edit.copy(isBlockedOnConflict = true)) } + } + + /** Re-include the given edit in uploading, all its pending tag conflicts being resolved */ + fun markUnblocked(edit: ElementEdit) { + lock.withLock { editsDB.put(edit.copy(isBlockedOnConflict = false)) } + } + fun markSyncFailed(edit: ElementEdit) { delete(edit) } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsDao.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsDao.kt index 14e0ea2a553..cfcbdc115b6 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsDao.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsDao.kt @@ -7,6 +7,7 @@ import de.westnordost.streetcomplete.data.osm.edits.ElementEditsTable.Columns.AC import de.westnordost.streetcomplete.data.osm.edits.ElementEditsTable.Columns.CREATED_TIMESTAMP import de.westnordost.streetcomplete.data.osm.edits.ElementEditsTable.Columns.GEOMETRY import de.westnordost.streetcomplete.data.osm.edits.ElementEditsTable.Columns.ID +import de.westnordost.streetcomplete.data.osm.edits.ElementEditsTable.Columns.IS_BLOCKED import de.westnordost.streetcomplete.data.osm.edits.ElementEditsTable.Columns.IS_NEAR_USER_LOCATION import de.westnordost.streetcomplete.data.osm.edits.ElementEditsTable.Columns.IS_SYNCED import de.westnordost.streetcomplete.data.osm.edits.ElementEditsTable.Columns.LATITUDE @@ -72,7 +73,9 @@ class ElementEditsDao( fun getOldestUnsynced(): ElementEdit? = db.queryOne( NAME, - where = "$IS_SYNCED = 0 AND $WORKSPACE_ID = $workspaceId", + // edits blocked on unresolved tag conflicts are skipped by the upload queue but stay + // unsynced (still counted, still applied to the local map view) + where = "$IS_SYNCED = 0 AND $IS_BLOCKED = 0 AND $WORKSPACE_ID = $workspaceId", orderBy = CREATED_TIMESTAMP ) { it.toElementEdit() } @@ -125,7 +128,8 @@ class ElementEditsDao( IS_SYNCED to if (isSynced) 1 else 0, ACTION to json.encodeToString(action), IS_NEAR_USER_LOCATION to if (isNearUserLocation) 1 else 0, - WORKSPACE_ID to workspaceId + WORKSPACE_ID to workspaceId, + IS_BLOCKED to if (isBlockedOnConflict) 1 else 0 ) private fun CursorPosition.toElementEdit() = ElementEdit( @@ -137,6 +141,7 @@ class ElementEditsDao( getInt(IS_SYNCED) == 1, json.decodeFromString(getString(ACTION)), getInt(IS_NEAR_USER_LOCATION) == 1, - getInt(WORKSPACE_ID) + getInt(WORKSPACE_ID), + getInt(IS_BLOCKED) == 1 ) } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsModule.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsModule.kt index bce8c8c3c49..76346bc34ce 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsModule.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsModule.kt @@ -1,5 +1,7 @@ package de.westnordost.streetcomplete.data.osm.edits +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsController +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsDao import de.westnordost.streetcomplete.data.osm.edits.upload.ElementEditUploader import de.westnordost.streetcomplete.data.osm.edits.upload.ElementEditsUploader import de.westnordost.streetcomplete.data.osm.edits.upload.changesets.OpenChangesetsDao @@ -7,18 +9,25 @@ import de.westnordost.streetcomplete.data.osm.edits.upload.changesets.OpenChange import org.koin.dsl.module val elementEditsModule = module { - factory { ElementEditUploader(get(), get(), get()) } + factory { ElementEditUploader(get(), get(), get(), get()) } factory { ElementEditsDao(get(), get(), get()) } factory { ElementIdProviderDao(get()) } factory { OpenChangesetsDao(get()) } factory { EditElementsDao(get()) } + factory { PendingTagConflictsDao(get(), get(), get()) } + factory { DiscardedEditNoticesDao(get(), get(), get()) } single { OpenChangesetsManager(get(), get(), get(), get()) } - single { ElementEditsUploader(get(), get(), get(), get(), get(), get()) } + single { ElementEditsUploader(get(), get(), get(), get(), get(), get(), get()) } single { get() } single { ElementEditsController(get(), get(), get(), get()) } single { MapDataWithEditsSource(get(), get(), get()) } + + /* must be a singleton because there is a listener that should respond to a change in the + * underlying database table */ + single { PendingTagConflictsController(get(), get()) } + single { DiscardedEditNoticesController(get()) } } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsTable.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsTable.kt index dbfef0fffa5..5c1a65d9a70 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsTable.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/ElementEditsTable.kt @@ -15,6 +15,9 @@ object ElementEditsTable { const val ACTION = "action" const val IS_NEAR_USER_LOCATION = "is_near" const val WORKSPACE_ID = "workspace_id" + /** whether this (unsynced) edit is excluded from uploading until the user has resolved + * its pending tag conflicts */ + const val IS_BLOCKED = "blocked_on_conflict" } const val CREATE = """ @@ -29,7 +32,8 @@ object ElementEditsTable { ${Columns.IS_SYNCED} int NOT NULL, ${Columns.ACTION} text, ${Columns.IS_NEAR_USER_LOCATION} int NOT NULL, - ${Columns.WORKSPACE_ID} int NOT NULL + ${Columns.WORKSPACE_ID} int NOT NULL, + ${Columns.IS_BLOCKED} int NOT NULL DEFAULT 0 ); """ } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflict.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflict.kt new file mode 100644 index 00000000000..342746e4c99 --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflict.kt @@ -0,0 +1,29 @@ +package de.westnordost.streetcomplete.data.osm.edits.update_tags + +import de.westnordost.streetcomplete.data.osm.edits.ElementEditType +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon + +/** A single OSM tag on a specific element where the user's local edit collided with a concurrent + * remote edit on the same key. The whole edit is held back from uploading (blocked) while any of + * these exist for it; once the user has decided per tag - keep their own answer or accept the + * other edit's value - the decisions are folded into the edit and it uploads as one unit. */ +data class PendingTagConflict( + var id: Long, + /** id of the [de.westnordost.streetcomplete.data.osm.edits.ElementEdit] held back by this conflict */ + val editId: Long, + val elementType: ElementType, + val elementId: Long, + val tagKey: String, + /** the value the user answered locally, or null if the user's edit was to delete this tag */ + val mineValue: String?, + /** the conflicting value found on the server when the conflict was detected. Only used for + * display purposes - resolving re-fetches the current value to avoid re-racing a third edit + * that may have landed in the meantime */ + val theirsValueAtDetection: String?, + val editType: ElementEditType, + val source: String, + val position: LatLon, + val createdTimestamp: Long, + var workspaceId: Int = 0 +) diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflictsController.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflictsController.kt new file mode 100644 index 00000000000..341884e4267 --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflictsController.kt @@ -0,0 +1,138 @@ +package de.westnordost.streetcomplete.data.osm.edits.update_tags + +import de.westnordost.streetcomplete.data.osm.edits.ElementEdit +import de.westnordost.streetcomplete.data.osm.edits.ElementEditsController +import de.westnordost.streetcomplete.data.osm.edits.ElementEditsSource +import de.westnordost.streetcomplete.util.Listeners +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.IO +import kotlinx.coroutines.withContext + +/** Holds tag-level conflicts of edits that are blocked from uploading (see + * [de.westnordost.streetcomplete.data.osm.edits.upload.ElementEditUploader]): while any of these + * exist for an edit, nothing of that edit is uploaded. The user resolves them per tag - keep + * their own answer or accept the concurrent remote edit's value - and each decision is folded + * into the blocked edit's stored action. Once the last conflict of an edit is resolved, the edit + * is unblocked and uploads through the normal sync path as one unit, so the edit history shows + * exactly what was pushed. */ +class PendingTagConflictsController( + private val dao: PendingTagConflictsDao, + private val elementEditsController: ElementEditsController, +) { + interface Listener { + fun onAdded(conflict: PendingTagConflict) + fun onRemoved(conflict: PendingTagConflict) + } + + private val listeners = Listeners() + + init { + // an edit deleted while blocked (e.g. undone from the edit history) takes its pending + // conflicts with it + elementEditsController.addListener(object : ElementEditsSource.Listener { + override fun onAddedEdit(edit: ElementEdit) {} + override fun onSyncedEdit(edit: ElementEdit) {} + override fun onDeletedEdits(edits: List) { + val editIds = edits.mapTo(HashSet()) { it.id } + for (conflict in dao.getAll().filter { it.editId in editIds }) { + remove(conflict) + } + } + }) + } + + fun addListener(listener: Listener) { + listeners.add(listener) + } + + fun removeListener(listener: Listener) { + listeners.remove(listener) + } + + fun add(conflict: PendingTagConflict) { + val added = dao.add(conflict) + listeners.forEach { it.onAdded(added) } + } + + fun getAll(): List = dao.getAll() + + /** Number of blocked edits with at least one pending conflict, not the number of conflicting + * tags - matches the grouped dialog (see TagConflictResolutionEffect), where all conflicting + * tags of the same edit are shown and resolved together as one */ + fun getCount(): Int = dao.getAll().distinctBy { it.editId }.size + + /** Returns the oldest pending conflict without removing it - it stays queryable/re-showable + * until actually resolved, so it survives the app being killed mid-decision */ + fun getOldest(): PendingTagConflict? = dao.getAll().firstOrNull() + + /** All pending conflicts of the same (blocked) edit as the oldest one, so they can be shown - + * and decided on - together in a single dialog instead of one at a time */ + fun getOldestGroup(): List { + val all = dao.getAll() + val oldest = all.firstOrNull() ?: return emptyList() + return all.filter { it.editId == oldest.editId } + } + + /** Re-assert the user's own answer for this tag: rebuild the kept change in the blocked + * edit's action against the value the conflict was detected against, so the re-upload's + * conflict check doesn't re-flag it for the very collision the user just decided on. (If a + * *third* value lands before the re-upload, upload-time detection catches it and the edit + * is held again with fresh data.) */ + suspend fun resolveKeepMine(conflict: PendingTagConflict) = withContext(Dispatchers.IO) { + updateBlockedEdit(conflict) { change -> change.rebuiltAgainst(conflict.theirsValueAtDetection) } + remove(conflict) + unblockIfFullyResolved(conflict.editId) + } + + /** Accept the concurrent remote edit's value: drop this tag's change from the blocked edit's + * action - the server already has the value being kept */ + suspend fun resolveKeepTheirs(conflict: PendingTagConflict) = withContext(Dispatchers.IO) { + updateBlockedEdit(conflict) { null } + remove(conflict) + unblockIfFullyResolved(conflict.editId) + } + + /** Rewrite the conflict's tag change within its blocked edit's action; a null result from + * [rewrite] drops the change */ + private fun updateBlockedEdit( + conflict: PendingTagConflict, + rewrite: (StringMapEntryChange) -> StringMapEntryChange?, + ) { + val edit = elementEditsController.get(conflict.editId) ?: return + val action = edit.action as? UpdateElementTagsAction ?: return + val newChanges = action.changes.changes.mapNotNull { change -> + if (change.key == conflict.tagKey) rewrite(change) else change + }.toSet() + if (newChanges != action.changes.changes) { + elementEditsController.updateAction(edit.copy(action = action.copy(changes = StringMapChanges(newChanges)))) + } + } + + private fun unblockIfFullyResolved(editId: Long) { + if (dao.getAll().any { it.editId == editId }) return + val edit = elementEditsController.get(editId) ?: return + if (edit.isBlockedOnConflict) elementEditsController.markUnblocked(edit) + } + + private fun remove(conflict: PendingTagConflict) { + if (dao.delete(conflict.id)) listeners.forEach { it.onRemoved(conflict) } + } +} + +internal fun StringMapEntryChange.mineValue(): String? = when (this) { + is StringMapEntryAdd -> value + is StringMapEntryModify -> value + is StringMapEntryDelete -> null +} + +/** The same intended end value (or deletion), but expressed as a diff from [currentValue] instead + * of from whatever this change's original baseline was */ +internal fun StringMapEntryChange.rebuiltAgainst(currentValue: String?): StringMapEntryChange { + val mine = mineValue() + return when { + mine != null && currentValue != null -> StringMapEntryModify(key, currentValue, mine) + mine != null && currentValue == null -> StringMapEntryAdd(key, mine) + mine == null && currentValue != null -> StringMapEntryDelete(key, currentValue) + else -> this + } +} diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflictsDao.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflictsDao.kt new file mode 100644 index 00000000000..0c882cad370 --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflictsDao.kt @@ -0,0 +1,83 @@ +package de.westnordost.streetcomplete.data.osm.edits.update_tags + +import de.westnordost.streetcomplete.data.AllEditTypes +import de.westnordost.streetcomplete.data.CursorPosition +import de.westnordost.streetcomplete.data.Database +import de.westnordost.streetcomplete.data.osm.edits.ElementEditType +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.CREATED_TIMESTAMP +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.EDIT_ID +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.ELEMENT_ID +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.ELEMENT_TYPE +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.ID +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.LATITUDE +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.LONGITUDE +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.MINE_VALUE +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.QUEST_TYPE +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.SOURCE +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.THEIRS_VALUE +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.Columns.WORKSPACE_ID +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsTable.NAME +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon +import de.westnordost.streetcomplete.data.preferences.Preferences + +class PendingTagConflictsDao( + private val db: Database, + private val allEditTypes: AllEditTypes, + private val preferences: Preferences? = null, +) { + private val workspaceId + get() = preferences?.workspaceId ?: 0 + + fun add(conflict: PendingTagConflict): PendingTagConflict { + val rowId = db.insert(NAME, conflict.toPairs()) + return conflict.copy(id = rowId, workspaceId = workspaceId) + } + + fun getAll(): List = + db.query( + NAME, + where = "$WORKSPACE_ID = $workspaceId", + orderBy = CREATED_TIMESTAMP + ) { it.toPendingTagConflict() } + + fun getCount(): Int = + db.queryOne( + NAME, + columns = arrayOf("COUNT(*) AS count"), + where = "$WORKSPACE_ID = $workspaceId" + ) { it.getInt("count") } ?: 0 + + fun delete(id: Long): Boolean = + db.delete(NAME, "$WORKSPACE_ID = $workspaceId AND $ID = $id") == 1 + + private fun PendingTagConflict.toPairs(): List> = listOf( + EDIT_ID to editId, + ELEMENT_TYPE to elementType.name, + ELEMENT_ID to elementId, + PendingTagConflictsTable.Columns.TAG_KEY to tagKey, + MINE_VALUE to mineValue, + THEIRS_VALUE to theirsValueAtDetection, + QUEST_TYPE to editType.name, + SOURCE to source, + LATITUDE to position.latitude, + LONGITUDE to position.longitude, + CREATED_TIMESTAMP to createdTimestamp, + WORKSPACE_ID to workspaceId + ) + + private fun CursorPosition.toPendingTagConflict() = PendingTagConflict( + id = getLong(ID), + editId = getLong(EDIT_ID), + elementType = ElementType.valueOf(getString(ELEMENT_TYPE)), + elementId = getLong(ELEMENT_ID), + tagKey = getString(PendingTagConflictsTable.Columns.TAG_KEY), + mineValue = getStringOrNull(MINE_VALUE), + theirsValueAtDetection = getStringOrNull(THEIRS_VALUE), + editType = allEditTypes.getByName(getString(QUEST_TYPE)) as ElementEditType, + source = getString(SOURCE), + position = LatLon(getDouble(LATITUDE), getDouble(LONGITUDE)), + createdTimestamp = getLong(CREATED_TIMESTAMP), + workspaceId = getInt(WORKSPACE_ID) + ) +} diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflictsTable.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflictsTable.kt new file mode 100644 index 00000000000..d8bd545b04c --- /dev/null +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/update_tags/PendingTagConflictsTable.kt @@ -0,0 +1,39 @@ +package de.westnordost.streetcomplete.data.osm.edits.update_tags + +object PendingTagConflictsTable { + const val NAME = "osm_pending_tag_conflicts" + + object Columns { + const val ID = "id" + const val EDIT_ID = "edit_id" + const val ELEMENT_TYPE = "element_type" + const val ELEMENT_ID = "element_id" + const val TAG_KEY = "tag_key" + const val MINE_VALUE = "mine_value" + const val THEIRS_VALUE = "theirs_value" + const val QUEST_TYPE = "quest_type" + const val SOURCE = "source" + const val LATITUDE = "latitude" + const val LONGITUDE = "longitude" + const val CREATED_TIMESTAMP = "created" + const val WORKSPACE_ID = "workspace_id" + } + + const val CREATE = """ + CREATE TABLE $NAME ( + ${Columns.ID} INTEGER PRIMARY KEY AUTOINCREMENT, + ${Columns.EDIT_ID} int NOT NULL, + ${Columns.ELEMENT_TYPE} varchar(255) NOT NULL, + ${Columns.ELEMENT_ID} int NOT NULL, + ${Columns.TAG_KEY} varchar(255) NOT NULL, + ${Columns.MINE_VALUE} varchar(255), + ${Columns.THEIRS_VALUE} varchar(255), + ${Columns.QUEST_TYPE} varchar(255) NOT NULL, + ${Columns.SOURCE} varchar(255) NOT NULL, + ${Columns.LATITUDE} double NOT NULL, + ${Columns.LONGITUDE} double NOT NULL, + ${Columns.CREATED_TIMESTAMP} int NOT NULL, + ${Columns.WORKSPACE_ID} int NOT NULL + ); + """ +} diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploader.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploader.kt index ab225e64903..287367badef 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploader.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditUploader.kt @@ -5,23 +5,38 @@ import de.westnordost.streetcomplete.ApplicationConstants.EDIT_ACTIONS_NOT_ALLOW import de.westnordost.streetcomplete.data.ConflictException import de.westnordost.streetcomplete.data.osm.edits.ElementEdit import de.westnordost.streetcomplete.data.osm.edits.ElementIdProvider +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflict +import de.westnordost.streetcomplete.data.osm.edits.update_tags.PendingTagConflictsController +import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapChanges +import de.westnordost.streetcomplete.data.osm.edits.update_tags.UpdateElementTagsAction +import de.westnordost.streetcomplete.data.osm.edits.update_tags.changesApplied +import de.westnordost.streetcomplete.data.osm.edits.update_tags.isGeometrySubstantiallyDifferent +import de.westnordost.streetcomplete.data.osm.edits.update_tags.mineValue +import de.westnordost.streetcomplete.data.osm.edits.update_tags.rebuiltAgainst import de.westnordost.streetcomplete.data.osm.edits.upload.changesets.OpenChangesetsManager import de.westnordost.streetcomplete.data.osm.mapdata.ChangesetTooLargeException +import de.westnordost.streetcomplete.data.osm.mapdata.Element +import de.westnordost.streetcomplete.data.osm.mapdata.ElementType import de.westnordost.streetcomplete.data.osm.mapdata.MapDataApiClient import de.westnordost.streetcomplete.data.osm.mapdata.MapDataChanges import de.westnordost.streetcomplete.data.osm.mapdata.MapDataController import de.westnordost.streetcomplete.data.osm.mapdata.MapDataUpdates import de.westnordost.streetcomplete.data.osm.mapdata.RemoteMapDataRepository +import de.westnordost.streetcomplete.util.ktx.nowAsEpochMilliseconds class ElementEditUploader( private val changesetManager: OpenChangesetsManager, private val mapDataApi: MapDataApiClient, - private val mapDataController: MapDataController + private val mapDataController: MapDataController, + private val pendingTagConflictsController: PendingTagConflictsController, ) { /** Apply the given change to the given element and upload it * * @throws ConflictException if element has been changed server-side in an incompatible way + * @throws HeldForConflictResolutionException if some of the edit's tag changes collided with + * a concurrent remote edit - nothing was uploaded, the edit should be blocked until + * the user has resolved the pending conflicts */ suspend fun upload(edit: ElementEdit, getIdProvider: () -> ElementIdProvider): MapDataUpdates { // certain edit types don't allow building changes on top of cached map data @@ -60,9 +75,16 @@ class ElementEditUploader( * @throws ConflictException if element has been changed on remote in an incompatible way * */ private suspend fun uploadUsingRemoteRepo(edit: ElementEdit, getIdProvider: () -> ElementIdProvider): MapDataUpdates { + val action = edit.action + // tag-only edits get special handling: a colliding key doesn't have to sink the whole + // edit, see uploadTagChangesUsingRemoteRepo + if (action is UpdateElementTagsAction) { + return uploadTagChangesUsingRemoteRepo(edit, action) + } + // If a conflict is thrown here, it definitely means that the element has been changed on // remote in an incompatible way. So, we don't catch the exception but exit - val remoteChanges = edit.action.createUpdates(RemoteMapDataRepository(mapDataApi), getIdProvider()) + val remoteChanges = action.createUpdates(RemoteMapDataRepository(mapDataApi), getIdProvider()) return try { uploadChanges(edit, remoteChanges, false) @@ -77,6 +99,85 @@ class ElementEditUploader( } } + /** + * Applies a tag-update edit onto the element's current (freshly fetched) remote state, but + * unlike other edit types, a per-key value collision does not discard the whole edit: + * - the two bookkeeping keys are always force-overwritten with the local value, never treated + * as a conflict + * - if any other key genuinely collides, the *whole edit* is held back: nothing is uploaded, + * a [PendingTagConflict] is recorded per colliding key, and + * [HeldForConflictResolutionException] is thrown so the caller blocks the edit until the + * user has decided per tag. The decisions are folded into the edit and it then uploads + * normally, as one unit - so the edit history always shows exactly what was pushed. + * + * Structural issues (element deleted, geometry changed substantially) are still a hard, + * all-or-nothing failure - there's no sensible per-tag override for those. + */ + private suspend fun uploadTagChangesUsingRemoteRepo(edit: ElementEdit, action: UpdateElementTagsAction): MapDataUpdates { + val original = action.originalElement + val currentElement = fetchElement(original.type, original.id) + ?: throw ConflictException("Element deleted") + + if (isGeometrySubstantiallyDifferent(original, currentElement)) { + throw ConflictException("Element geometry changed substantially") + } + + // bookkeeping tags are never shown to the user as a conflict - rebuild them against the + // element's current value so they never register as colliding and always win with the + // local (i.e. most recent) value + val reconciledChanges = action.changes.changes.map { change -> + if (change.key in SILENTLY_OVERRIDDEN_TAG_KEYS) { + change.rebuiltAgainst(currentElement.tags[change.key]) + } else { + change + } + }.toSet() + + val realConflicts = StringMapChanges(reconciledChanges).getConflictsTo(currentElement.tags).toSet() + + if (realConflicts.isNotEmpty()) { + for (conflict in realConflicts) { + pendingTagConflictsController.add( + PendingTagConflict( + id = 0, + editId = edit.id, + elementType = currentElement.type, + elementId = currentElement.id, + tagKey = conflict.key, + mineValue = conflict.mineValue(), + theirsValueAtDetection = currentElement.tags[conflict.key], + editType = edit.type, + source = edit.source, + position = edit.position, + createdTimestamp = nowAsEpochMilliseconds(), + workspaceId = edit.workspaceId + ) + ) + } + throw HeldForConflictResolutionException(currentElement) + } + + val changes = MapDataChanges( + modifications = listOf(currentElement.changesApplied(StringMapChanges(reconciledChanges))) + ) + return try { + uploadChanges(edit, changes, false) + } + // probably changeset was closed -> try again once with new changeset + catch (e: ConflictException) { + uploadChanges(edit, changes, true) + } + catch (e: ChangesetTooLargeException) { + uploadChanges(edit, changes, true) + } + } + + private suspend fun fetchElement(type: ElementType, id: Long): Element? = when (type) { + ElementType.NODE -> mapDataApi.getNode(id) + ElementType.WAY -> mapDataApi.getWay(id) + ElementType.RELATION -> mapDataApi.getRelation(id) + } + private suspend fun uploadChanges( edit: ElementEdit, changes: MapDataChanges, @@ -89,4 +190,18 @@ class ElementEditUploader( } return mapDataApi.uploadChanges(changesetId, changes, ApplicationConstants::ignoreRelation) } + + companion object { + /** tags that are set on every long-form answer purely for bookkeeping/quest-visibility + * purposes - never worth bothering the user with a conflict prompt about, see + * EditDescription.kt which excludes the same two keys from edit-history display */ + private val SILENTLY_OVERRIDDEN_TAG_KEYS = setOf("ext:gig_complete", "ext:gig_last_updated") + } } + +/** Thrown when an edit's tag changes collided with a concurrent remote edit: nothing was + * uploaded, [PendingTagConflict]s were recorded, and the edit should be blocked from uploading + * until the user has resolved them. Carries the freshly fetched [currentElement] so the caller + * can refresh the local cache to the state the conflicts were detected against. */ +class HeldForConflictResolutionException(val currentElement: Element) : + RuntimeException("Edit held back until its tag conflicts are resolved") diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploader.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploader.kt index 077c2219d9d..678dfbae132 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploader.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/upload/ElementEditsUploader.kt @@ -1,6 +1,8 @@ package de.westnordost.streetcomplete.data.osm.edits.upload import de.westnordost.streetcomplete.data.ConflictException +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNotice +import de.westnordost.streetcomplete.data.osm.edits.DiscardedEditNoticesController import de.westnordost.streetcomplete.data.osm.edits.ElementEdit import de.westnordost.streetcomplete.data.osm.edits.ElementEditsController import de.westnordost.streetcomplete.data.osm.edits.ElementIdProvider @@ -16,6 +18,7 @@ import de.westnordost.streetcomplete.data.osm.mapdata.MutableMapData import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditsController import de.westnordost.streetcomplete.data.upload.OnUploadedChangeListener import de.westnordost.streetcomplete.data.user.statistics.StatisticsController +import de.westnordost.streetcomplete.util.ktx.nowAsEpochMilliseconds import de.westnordost.streetcomplete.util.logs.Log import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope @@ -32,7 +35,8 @@ class ElementEditsUploader( private val mapDataController: MapDataController, private val singleUploader: ElementEditUploader, private val mapDataApi: MapDataApiClient, - private val statisticsController: StatisticsController + private val statisticsController: StatisticsController, + private val discardedEditNoticesController: DiscardedEditNoticesController, ) { var uploadedChangeListener: OnUploadedChangeListener? = null @@ -68,11 +72,31 @@ class ElementEditsUploader( } else { statisticsController.addOne(edit.type.name, edit.position) } + } catch (e: HeldForConflictResolutionException) { + // nothing was uploaded and nothing is discarded: the edit is excluded from uploading + // until the user has resolved its pending tag conflicts, then re-enters this queue + Log.d(TAG, "Held a $editActionClassName for conflict resolution") + elementEditsController.markBlockedOnConflict(edit) + // refresh the local cache to the remote state the conflicts were detected against + mapDataController.updateAll(MapDataUpdates(updated = listOf(e.currentElement))) } catch (e: ConflictException) { Log.d(TAG, "Dropped a $editActionClassName: ${e.message}") uploadedChangeListener?.onDiscarded(edit.type.name, edit.position) elementEditsController.markSyncFailed(edit) + val elementKey = edit.action.elementKeys.firstOrNull() + discardedEditNoticesController.add( + DiscardedEditNotice( + id = 0, + editType = edit.type, + elementType = elementKey?.type, + elementId = elementKey?.id, + position = edit.position, + reason = e.message ?: "Could not be applied to the current state of the map", + createdTimestamp = nowAsEpochMilliseconds(), + workspaceId = edit.workspaceId + ) + ) /* fetching the current version of the element(s) edited on conflict and persisting them is not really optional, as when the edit has been deleted due to the conflict, diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiClient.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiClient.kt index 47fa030d0c0..bf6782b9452 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiClient.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/mapdata/MapDataApiClient.kt @@ -122,6 +122,7 @@ class MapDataApiClient( val response = httpClient.get(workspaceConfigProvider.osmBaseUrl + "map") { parameter("bbox", bounds.toOsmApiString()) header("X-Workspace", workspaceConfigProvider.workspaceId.toString()) + workspaceConfigProvider.workspaceToken?.let { bearerAuth(it) } expectSuccess = true } val source = response.bodyAsChannel().asSource().buffered() @@ -212,6 +213,7 @@ class MapDataApiClient( try { val response = httpClient.get(workspaceConfigProvider.osmBaseUrl + query) { header("X-Workspace", workspaceConfigProvider.workspaceId.toString()) + workspaceConfigProvider.workspaceToken?.let { bearerAuth(it) } expectSuccess = true } val source = response.bodyAsChannel().asSource().buffered() diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/NotesApiClient.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/NotesApiClient.kt index 4f5ca2fff59..df2d33bc733 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/NotesApiClient.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/NotesApiClient.kt @@ -47,8 +47,8 @@ class NotesApiClient( */ suspend fun create(pos: LatLon, text: String): Note = wrapApiClientExceptions { val response = httpClient.post(workspaceConfigProvider.osmBaseUrl + "notes") { - userAccessTokenSource.accessToken?.let { bearerAuth(it) } header("X-Workspace", workspaceConfigProvider.workspaceId.toString()) + workspaceConfigProvider.workspaceToken?.let { bearerAuth(it) } parameter("lat", pos.latitude.format(7)) parameter("lon", pos.longitude.format(7)) parameter("text", text) @@ -72,7 +72,7 @@ class NotesApiClient( suspend fun comment(id: Long, text: String): Note = wrapApiClientExceptions { try { val response = httpClient.post(workspaceConfigProvider.osmBaseUrl + "notes/$id/comment") { - userAccessTokenSource.accessToken?.let { bearerAuth(it) } + workspaceConfigProvider.workspaceToken?.let { bearerAuth(it) } parameter("text", text) expectSuccess = true } @@ -100,6 +100,7 @@ class NotesApiClient( try { val response = httpClient.get(workspaceConfigProvider.osmBaseUrl + "notes/$id") { expectSuccess = true header("X-Workspace", workspaceConfigProvider.workspaceId.toString()) + workspaceConfigProvider.workspaceToken?.let { bearerAuth(it) } } val source = response.bodyAsChannel().asSource().buffered() return notesApiParser.parseNotes(source, workspaceConfigProvider.workspaceId).singleOrNull() @@ -133,8 +134,7 @@ class NotesApiClient( try { val response = httpClient.get(workspaceConfigProvider.osmBaseUrl + "notes") { header("X-Workspace", workspaceConfigProvider.workspaceId.toString()) - - userAccessTokenSource.accessToken?.let { bearerAuth(it) } + workspaceConfigProvider.workspaceToken?.let { bearerAuth(it) } parameter("bbox", bounds.toOsmApiString()) parameter("limit", limit) parameter("closed", 0) diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/NotesModule.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/NotesModule.kt index 52ef0e7841c..dedf4d32675 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/NotesModule.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/NotesModule.kt @@ -1,6 +1,5 @@ package de.westnordost.streetcomplete.data.osmnotes -import de.westnordost.streetcomplete.ApplicationConstants import org.koin.core.qualifier.named import org.koin.dsl.module @@ -9,7 +8,6 @@ val notesModule = module { factory { AvatarsInNotesUpdater(get()) } factory { NoteDao(get(), get()) } factory { NotesDownloader(get(), get()) } - factory { PhotoServiceApiClient(get(), get(), ApplicationConstants.SC_PHOTO_SERVICE_URL) } single { NoteController(get()).apply { diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt index f7166502eaf..b1dee408b47 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osmnotes/edits/NoteEditsUploader.kt @@ -2,9 +2,10 @@ package de.westnordost.streetcomplete.data.osmnotes.edits import de.westnordost.streetcomplete.ApplicationConstants import de.westnordost.streetcomplete.data.ConflictException +import de.westnordost.streetcomplete.data.karta_view.KartaViewApiClient +import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osmnotes.NoteController import de.westnordost.streetcomplete.data.osmnotes.NotesApiClient -import de.westnordost.streetcomplete.data.osmnotes.PhotoServiceApiClient import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditAction.COMMENT import de.westnordost.streetcomplete.data.osmnotes.edits.NoteEditAction.CREATE import de.westnordost.streetcomplete.data.osmtracks.Trackpoint @@ -30,7 +31,7 @@ class NoteEditsUploader( private val userDataSource: UserDataSource, private val notesApi: NotesApiClient, private val tracksApi: TracksApiClient, - private val imageUploader: PhotoServiceApiClient, + private val imageUploader: KartaViewApiClient, private val fileSystem: FileSystem, ) { var uploadedChangeListener: OnUploadedChangeListener? = null @@ -51,13 +52,11 @@ class NoteEditsUploader( } } private suspend fun uploadMissedImageActivations() { + // KartaView photos need no activation step (their URLs are embedded in the note text at + // upload) - just clear any flags still set, e.g. from before the switch to KartaView while (true) { val edit = noteEditsController.getOldestNeedingImagesActivation() ?: break - // see uploadEdits - withContext(scope.coroutineContext) { - imageUploader.activate(edit.noteId) - noteEditsController.markImagesActivated(edit.id) - } + noteEditsController.markImagesActivated(edit.id) } } @@ -73,7 +72,7 @@ class NoteEditsUploader( private suspend fun uploadEdit(edit: NoteEdit) { // try to upload the image and track if we have them - val imageText = uploadAndGetAttachedPhotosText(edit.imagePaths) + val imageText = uploadAndGetAttachedPhotosText(edit.imagePaths, edit.position) val trackText = uploadAndGetAttachedTrackText(edit.track, edit.text) val text = edit.text.orEmpty() + imageText + trackText @@ -94,7 +93,6 @@ class NoteEditsUploader( noteController.put(note) if (edit.imagePaths.isNotEmpty()) { - imageUploader.activate(note.id) noteEditsController.markImagesActivated(note.id) } @@ -124,9 +122,9 @@ class NoteEditsUploader( } } - private suspend fun uploadAndGetAttachedPhotosText(imagePaths: List): String { + private suspend fun uploadAndGetAttachedPhotosText(imagePaths: List, position: LatLon): String { if (imagePaths.isNotEmpty()) { - val urls = imageUploader.upload(imagePaths) + val urls = imageUploader.upload(imagePaths, position) if (urls.isNotEmpty()) { return "\n\nAttached photo(s):\n" + urls.joinToString("\n") } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/Preferences.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/Preferences.kt index d2228f9e1f3..d5bba25c365 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/Preferences.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/Preferences.kt @@ -8,6 +8,7 @@ import com.russhwolf.settings.int import com.russhwolf.settings.long import com.russhwolf.settings.nullableString import com.russhwolf.settings.set +import de.westnordost.streetcomplete.BuildConfig import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.util.ktx.putStringOrNull import kotlinx.coroutines.flow.MutableStateFlow @@ -214,7 +215,7 @@ class Preferences(private val prefs: ObservableSettings) { var kartaViewAccessToken: String set(value) { prefs.putString(KARTAVIEW_ACCESS_TOKEN, value) } - get() = prefs.getString(KARTAVIEW_ACCESS_TOKEN, DEFAULT_KARTAVIEW_ACCESS_TOKEN) + get() = prefs.getString(KARTAVIEW_ACCESS_TOKEN, BuildConfig.KARTAVIEW_ACCESS_TOKEN) var isFollowModeEnabled: Boolean set(value) { @@ -445,7 +446,5 @@ class Preferences(private val prefs: ObservableSettings) { private const val FOLLOW_MODE_ENABLED = "follow.mode.enabled" private const val DEBUG_MODE_ENABLED = "debug.mode.enabled" private const val KARTAVIEW_ACCESS_TOKEN = "kartaview.access_token" - private const val DEFAULT_KARTAVIEW_ACCESS_TOKEN = - "96aca5c4b80709fc6d9aced613b51905c0fbc37870640d7bdabede269165bde7" } } diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/WorkspaceConstants.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/WorkspaceConstants.kt index ec9f7663fc7..a4a4c209044 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/WorkspaceConstants.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/preferences/WorkspaceConstants.kt @@ -2,9 +2,10 @@ package de.westnordost.streetcomplete.data.preferences enum class Environment( val baseUrl: String, - val loginUrl: String, - val tdeiUrl: String, + val tdeiApiBaseUrl: String, + val tdeiUserManagementBaseurl: String, val osmUrl: String, + val tdeiWebUrl: String, val appUpdateVersionCheckUrl: String = APP_UPDATE_VERSION_CHECKER_URL, val firebaseUpdateUrl: String = FIREBASE_UPDATE_URL ) { @@ -12,19 +13,22 @@ enum class Environment( "https://api.workspaces-stage.sidewalks.washington.edu/api/v1/workspaces", "https://tdei-gateway-stage.azurewebsites.net/api/v1", "https://tdei-usermanagement-stage.azurewebsites.net/api/v1/user-profile", - "https://osm-workspaces-proxy.azurewebsites.net/stage/api/0.6/" + "https://osm-workspaces-proxy.azurewebsites.net/stage/api/0.6/", + "https://portal-stage.tdei.us" ), DEV( "https://api.workspaces-dev.sidewalks.washington.edu/api/v1/workspaces", "https://tdei-api-dev.azurewebsites.net/api/v1", "https://tdei-usermanagement-be-dev.azurewebsites.net/api/v1/user-profile", - "https://osm-workspaces-proxy.azurewebsites.net/dev/api/0.6/" + "https://osm-workspaces-proxy.azurewebsites.net/dev/api/0.6/", + "https://portal-dev.tdei.us" ), PROD( "https://api.workspaces.sidewalks.washington.edu/api/v1/workspaces", "https://tdei-gateway-prod.azurewebsites.net/api/v1", "https://tdei-usermanagement-prod.azurewebsites.net/api/v1/user-profile", - "https://osm-workspaces-proxy.azurewebsites.net/prod/api/0.6/" + "https://osm-workspaces-proxy.azurewebsites.net/prod/api/0.6/", + "https://portal.tdei.us" ); companion object { diff --git a/secrets.properties.example b/secrets.properties.example new file mode 100644 index 00000000000..91b949ea791 --- /dev/null +++ b/secrets.properties.example @@ -0,0 +1,8 @@ +# Copy this file to secrets.properties (git-ignored) and fill in the values. +# Alternatively, each value can be supplied via an environment variable of the +# same name as noted below (the env var takes precedence). + +# KartaView (api.openstreetcam.org) access token used for photo uploads. +# Obtain one from the KartaView account that owns the app's photo sequences. +# Env var alternative: KARTAVIEW_ACCESS_TOKEN +kartaViewAccessToken=