From eb55ec6ce0baade573d101f64c4869049749ec74 Mon Sep 17 00:00:00 2001 From: Rajesh Kantipudi <44539669+iamrajeshk@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:46:21 +0530 Subject: [PATCH 1/3] Conditionally hide "Show Quest Forms" debug option when no workspace is loaded and add unit tests, Ui fixes --- .../screens/user/UserActivity.kt | 12 +++++- .../screens/user/UserProfileScreen.kt | 15 ++++--- .../screens/workspaces/WorkSpaceActivity.kt | 2 + .../screens/user/ShowQuestFormsActionTest.kt | 39 +++++++++++++++++++ 4 files changed, 61 insertions(+), 7 deletions(-) create mode 100644 app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/screens/user/ShowQuestFormsActionTest.kt diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/UserActivity.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/UserActivity.kt index 160b89e022c..c7951b0ae62 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/UserActivity.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/UserActivity.kt @@ -7,6 +7,7 @@ import androidx.compose.material3.Surface import androidx.core.app.ActivityCompat import com.russhwolf.settings.SettingsListener import de.westnordost.streetcomplete.data.preferences.Preferences +import de.westnordost.streetcomplete.data.quest.QuestTypeRegistry import de.westnordost.streetcomplete.screens.BaseActivity import de.westnordost.streetcomplete.screens.settings.SettingsActivity import de.westnordost.streetcomplete.screens.settings.SettingsViewModel @@ -17,11 +18,20 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import org.koin.android.ext.android.inject import org.koin.androidx.viewmodel.ext.android.viewModel +/** The registry is only filled once a workspace is loaded (it's emptied again whenever the + * workspace list opens) - before that, the quest forms screen could only ever show "Nothing + * found", so the entry is hidden (null) instead */ +internal fun showQuestFormsActionOrNull( + questTypeRegistry: QuestTypeRegistry, + showQuestForms: () -> Unit, +): (() -> Unit)? = if (questTypeRegistry.isNotEmpty()) showQuestForms else null + class UserActivity : BaseActivity() { private val viewModel by viewModel() private val settingsViewModel by viewModel() private val preferences: Preferences by inject() + private val questTypeRegistry: QuestTypeRegistry by inject() private val listeners = mutableListOf() @@ -35,7 +45,7 @@ class UserActivity : BaseActivity() { settingsViewModel, preferences, onClickBack = { finish() }, - onClickShowQuestForms = { + onClickShowQuestForms = showQuestFormsActionOrNull(questTypeRegistry) { startActivity(SettingsActivity.createLaunchShowQuestFormsIntent(this)) }, onBiometricEnabledChanged = ::onBiometricEnabledChanged diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/UserProfileScreen.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/UserProfileScreen.kt index 6f422e39b04..205e7c25169 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/UserProfileScreen.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/user/UserProfileScreen.kt @@ -95,7 +95,8 @@ fun UserProfileScreen( settingsViewModel: SettingsViewModel, preferences: Preferences, onClickBack: () -> Unit, - onClickShowQuestForms: () -> Unit, + /** null hides the entry - there are no quest forms to show until a workspace is loaded */ + onClickShowQuestForms: (() -> Unit)?, onBiometricEnabledChanged: KSuspendFunction1, ) { var isBiometricEnabled by remember { mutableStateOf(preferences.isBiometricEnabled) } @@ -299,11 +300,13 @@ fun UserProfileScreen( Text(stringResource(theme.title)) } - PreferenceCategory("Debug") { - Preference( - name = "Show Quest Forms", - onClick = onClickShowQuestForms - ) { NextScreenIcon() } + if (onClickShowQuestForms != null) { + PreferenceCategory("Debug") { + Preference( + name = "Show Quest Forms", + onClick = onClickShowQuestForms + ) { NextScreenIcon() } + } } // PreferenceRow( diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkSpaceActivity.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkSpaceActivity.kt index ebb557f4634..073bbf080f9 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkSpaceActivity.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/screens/workspaces/WorkSpaceActivity.kt @@ -23,6 +23,7 @@ import androidx.compose.foundation.layout.statusBars import androidx.compose.foundation.layout.windowInsetsPadding import androidx.compose.material3.AlertDialog import androidx.compose.material3.Button +import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.material3.TextButton @@ -177,6 +178,7 @@ class WorkSpaceActivity : AppCompatActivity() { "$APP_NAME needs location to be enabled to fetch workspaces nearby", textAlign = TextAlign.Center, modifier = Modifier.padding(16.dp), + color = MaterialTheme.colorScheme.onSurfaceVariant, style = TextStyle(fontSize = 20.sp) ) if (!isLocationEnabled.value) { diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/screens/user/ShowQuestFormsActionTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/screens/user/ShowQuestFormsActionTest.kt new file mode 100644 index 00000000000..4342d519564 --- /dev/null +++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/screens/user/ShowQuestFormsActionTest.kt @@ -0,0 +1,39 @@ +package de.westnordost.streetcomplete.screens.user + +import de.westnordost.streetcomplete.data.quest.QuestTypeRegistry +import de.westnordost.streetcomplete.data.quest.TestQuestTypeA +import de.westnordost.streetcomplete.data.quest.TestQuestTypeB +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +class ShowQuestFormsActionTest { + + @Test fun `hidden before any workspace is loaded`() { + val registry = QuestTypeRegistry(emptyList()) + + assertNull(showQuestFormsActionOrNull(registry) {}) + } + + @Test fun `shown once a workspace has loaded its quest types`() { + val registry = QuestTypeRegistry(emptyList()) + // what MainActivity does when a workspace loads + registry.addItem(listOf(0 to TestQuestTypeA(), 1 to TestQuestTypeB())) + + var launched = 0 + val action = showQuestFormsActionOrNull(registry) { launched++ } + + assertNotNull(action) + action() + assertEquals(1, launched) + } + + @Test fun `hidden again after going back to the workspace list`() { + val registry = QuestTypeRegistry(listOf(0 to TestQuestTypeA())) + // what WorkSpaceActivity does when the workspace list opens + registry.addItem(emptyList()) + + assertNull(showQuestFormsActionOrNull(registry) {}) + } +} From ef41bf855cf41141049d49adfdf29ddd8333937e Mon Sep 17 00:00:00 2001 From: Rajesh Kantipudi <44539669+iamrajeshk@users.noreply.github.com> Date: Thu, 15 Oct 2026 12:45:26 +0530 Subject: [PATCH 2/3] Refactor logging and error reporting in ApplicationModule, Context, and MainActivity; implement cache clearing for quest types --- .../streetcomplete/ApplicationModule.kt | 35 +++++++++++++++---- .../screens/main/MainActivity.kt | 5 +++ .../streetcomplete/util/ktx/Context.kt | 17 ++++++++- .../data/quest/VisibleQuestsSource.kt | 4 +++ 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt index 37012923a0d..ac14b4c8e17 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/ApplicationModule.kt @@ -22,12 +22,12 @@ import de.westnordost.streetcomplete.util.network.retryOnTransientHttpFailure import de.westnordost.streetcomplete.util.satellite_layers.ImageryRepository import io.ktor.client.HttpClient import io.ktor.client.HttpClientConfig +import io.ktor.client.plugins.HttpRequestRetry import io.ktor.client.plugins.auth.Auth import io.ktor.client.plugins.auth.providers.BearerTokens import io.ktor.client.plugins.auth.providers.bearer import io.ktor.client.plugins.compression.ContentEncoding import io.ktor.client.plugins.contentnegotiation.ContentNegotiation -import io.ktor.client.plugins.HttpRequestRetry import io.ktor.client.plugins.defaultRequest import io.ktor.client.plugins.logging.DEFAULT import io.ktor.client.plugins.logging.LogLevel @@ -36,6 +36,7 @@ import io.ktor.client.plugins.logging.Logging import io.ktor.client.request.header import io.ktor.client.request.post import io.ktor.client.statement.bodyAsText +import io.ktor.http.HttpHeaders import io.ktor.http.HttpStatusCode import io.ktor.http.userAgent import io.ktor.serialization.kotlinx.json.json @@ -134,14 +135,21 @@ val appModule = module { install(ContentEncoding) { gzip() } - installWorkspaceBearerAuth(context, preferences, environmentManager, userLoginController, get(named("refreshClient"))) + installWorkspaceBearerAuth( + context, + preferences, + environmentManager, + userLoginController, + get(named("refreshClient")) + ) install(Logging) { logger = object : Logger { override fun log(message: String) { Log.d("OsmClient", message) // Avoid System.err } } - level = LogLevel.ALL + level = HTTP_LOG_LEVEL + sanitizeHeader { it == HttpHeaders.Authorization } } } } @@ -160,14 +168,21 @@ val appModule = module { install(ContentEncoding) { gzip() } - installWorkspaceBearerAuth(context, preferences, environmentManager, userLoginController, get(named("refreshClient"))) + installWorkspaceBearerAuth( + context, + preferences, + environmentManager, + userLoginController, + get(named("refreshClient")) + ) install(Logging) { logger = object : Logger { override fun log(message: String) { Log.d("KtorClient", message) // Avoid System.err } } - level = LogLevel.ALL + level = HTTP_LOG_LEVEL + sanitizeHeader { it == HttpHeaders.Authorization } } defaultRequest { userAgent(ApplicationConstants.USER_AGENT) @@ -185,7 +200,8 @@ val appModule = module { } install(Logging) { logger = Logger.DEFAULT - level = LogLevel.ALL + level = HTTP_LOG_LEVEL + sanitizeHeader { it == HttpHeaders.Authorization } } } } @@ -201,7 +217,8 @@ val appModule = module { Log.d("KtorClient", message) // Avoid System.err } } - level = LogLevel.ALL + level = HTTP_LOG_LEVEL + sanitizeHeader { it == HttpHeaders.Authorization } } install(HttpRequestRetry) { retryOnServerErrors(maxRetries = 3) @@ -274,3 +291,7 @@ suspend fun refreshJwtToken( null } } + +// HTTP logs end up in the log database and from there in emailed crash reports - full bodies +// (map data, long forms) make those multi-MB, so only log them in debug builds +private val HTTP_LOG_LEVEL = if (BuildConfig.DEBUG) LogLevel.ALL else LogLevel.INFO 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 af59650b5aa..ddfd038b133 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 @@ -1608,6 +1608,8 @@ class MainActivity : private suspend fun showQuestDetails(quest: Quest) { val mapFragment = mapFragment ?: return if (isQuestDetailsCurrentlyDisplayedFor(quest.key)) return + // AbstractQuestForm resolves its quest type by name from the registry + if (questTypeRegistry.getByName(quest.type.name) == null) return var checkResult: GigQuestCheckResult = GigQuestCheckResult.NotChecked if (quest is OsmQuest) { @@ -1785,6 +1787,9 @@ class MainActivity : questTypes.add(index to AddGenericLong(item, recencyPeriodInDays)) } questTypeRegistry.addItem(questTypes) + // the quest cache outlives this activity - quests cached under the previous long form + // would show stale pins whose quest type is no longer registered (form crashes on open) + visibleQuestsSource.clearAllCaches() allEditTypes.registries.clear() allEditTypes.registries.addAll(listOf(questTypeRegistry)) diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/ktx/Context.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/ktx/Context.kt index a3acac5e39e..93969c741ae 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/ktx/Context.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/ktx/Context.kt @@ -63,9 +63,24 @@ fun Context.sendEmail(to: String, subject: String, text: String? = null) { fun Context.sendErrorReportEmail(errorReport: String) = sendEmail( to = ApplicationConstants.ERROR_REPORTS_EMAIL, subject = ApplicationConstants.USER_AGENT + " " + "Error Report", - text = "Describe how to reproduce it here:\n\n\n\n$errorReport" + text = "Describe how to reproduce it here:\n\n\n\n${errorReport.truncatedForIntent()}" ) +// an intent extra must fit in the ~1MB binder transaction buffer (TransactionTooLargeException +// otherwise) - the attached log can be far larger. Keep the head (device info + stack trace) and +// the newest log lines. +private const val MAX_REPORT_HEAD_LENGTH = 20_000 +private const val MAX_REPORT_TAIL_LENGTH = 80_000 + +private fun String.truncatedForIntent(): String = + if (length <= MAX_REPORT_HEAD_LENGTH + MAX_REPORT_TAIL_LENGTH) { + this + } else { + take(MAX_REPORT_HEAD_LENGTH) + + "\n\n[... ${length - MAX_REPORT_HEAD_LENGTH - MAX_REPORT_TAIL_LENGTH} characters omitted ...]\n\n" + + takeLast(MAX_REPORT_TAIL_LENGTH) + } + fun Context.openUri(uri: String): Boolean = try { startActivity(Intent(Intent.ACTION_VIEW, uri.toUri())) diff --git a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/quest/VisibleQuestsSource.kt b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/quest/VisibleQuestsSource.kt index f67c9b43249..467c7152f09 100644 --- a/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/quest/VisibleQuestsSource.kt +++ b/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/quest/VisibleQuestsSource.kt @@ -203,6 +203,10 @@ class VisibleQuestsSource( fun clearCache() = cache.clear() + /** Cached quests hold the quest type instances they were created with - drop every + * workspace's cache when the quest type registry is rebuilt (see MainActivity.doLongForm) */ + fun clearAllCaches() = lock.withLock { workSpaceCache.clear() } + fun trimCache() = cache.trim(SPATIAL_CACHE_TILES / 3) private fun updateVisibleQuests( From 8ead61b2095e5216a89877fc7921327d0d4996b6 Mon Sep 17 00:00:00 2001 From: Rajesh Kantipudi <44539669+iamrajeshk@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:08:43 +0530 Subject: [PATCH 3/3] Refactor visibility of constants and update quest type handling in tests; add regression test for cache clearing --- .../screens/main/MapSyncTestBase.kt | 4 +++- .../streetcomplete/util/ktx/Context.kt | 6 ++--- .../data/quest/VisibleQuestsSourceTest.kt | 23 +++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncTestBase.kt b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncTestBase.kt index 9c71e8834da..1b2eabf6bd3 100644 --- a/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncTestBase.kt +++ b/app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/screens/main/MapSyncTestBase.kt @@ -138,6 +138,8 @@ abstract class MapSyncTestBase { protected lateinit var scenario: ActivityScenario @Volatile protected var overrideConflicts: String = "null" + /** The quest definition the workspace API returns - a test can change it, like an admin would. */ + @Volatile protected var longForm: String = LONG_FORM private var previousAnimationScales: List = emptyList() @Before @@ -463,7 +465,7 @@ abstract class MapSyncTestBase { "id": $WORKSPACE_ID, "title": "${FakeWorkspaceRepository.TEST_WORKSPACE_TITLE}", "type": "osw", "description": null, "createdAt": "2025-01-01T00:00:00Z", "createdBy": "user-1", "createdByName": "Jane Doe", "externalAppAccess": 1, "overrideConflicts": $overrideConflicts, - "imageryListDef": null, "kartaViewToken": null, "longFormQuestDef": $LONG_FORM, + "imageryListDef": null, "kartaViewToken": null, "longFormQuestDef": $longForm, "tdeiMetadata": null, "tdeiProjectGroupId": null, "tdeiRecordId": null, "tdeiServiceId": null }""" diff --git a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/ktx/Context.kt b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/ktx/Context.kt index 93969c741ae..0cc888333d0 100644 --- a/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/ktx/Context.kt +++ b/app/src/androidMain/kotlin/de/westnordost/streetcomplete/util/ktx/Context.kt @@ -69,10 +69,10 @@ fun Context.sendErrorReportEmail(errorReport: String) = sendEmail( // an intent extra must fit in the ~1MB binder transaction buffer (TransactionTooLargeException // otherwise) - the attached log can be far larger. Keep the head (device info + stack trace) and // the newest log lines. -private const val MAX_REPORT_HEAD_LENGTH = 20_000 -private const val MAX_REPORT_TAIL_LENGTH = 80_000 +internal const val MAX_REPORT_HEAD_LENGTH = 20_000 +internal const val MAX_REPORT_TAIL_LENGTH = 80_000 -private fun String.truncatedForIntent(): String = +internal fun String.truncatedForIntent(): String = if (length <= MAX_REPORT_HEAD_LENGTH + MAX_REPORT_TAIL_LENGTH) { this } else { diff --git a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/quest/VisibleQuestsSourceTest.kt b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/quest/VisibleQuestsSourceTest.kt index 8532c880db0..37fc48a0d01 100644 --- a/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/quest/VisibleQuestsSourceTest.kt +++ b/app/src/androidUnitTest/kotlin/de/westnordost/streetcomplete/data/quest/VisibleQuestsSourceTest.kt @@ -27,6 +27,7 @@ import org.mockito.Mockito.verifyNoInteractions import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNull import kotlin.test.assertTrue class VisibleQuestsSourceTest { @@ -155,6 +156,28 @@ class VisibleQuestsSourceTest { assertEquals(1, quests.size) } + // regression: the workspace's long form changed (element type "Sidewalk" -> "Sidewalks") while + // its quests were cached - the stale quest's form crashed looking its type up in the registry + @Test fun `clearAllCaches drops cached quests whose type is no longer registered`() { + val bboxCacheWillRequest = bbox.asBoundingBoxOfEnclosingTiles(16) + val staleQuest = OsmQuest(TestQuestTypeA(), ElementType.NODE, 1L, pGeom()) + on(osmQuestSource.getAllInBBox(bboxCacheWillRequest, questTypeNames)).thenReturn(listOf(staleQuest)) + on(questsHiddenSource.get(any())).thenReturn(null) + assertEquals(listOf(staleQuest), source.getAll(bbox)) + + val newQuestType = TestQuestTypeD() + val newQuest = OsmQuest(newQuestType, ElementType.NODE, 1L, pGeom()) + questTypeRegistry.addItem(listOf(0 to newQuestType)) + on(osmQuestSource.getAllInBBox(bboxCacheWillRequest, listOf(newQuestType.name))).thenReturn(listOf(newQuest)) + // the cache alone doesn't know the registry changed + assertEquals(staleQuest, source.get(staleQuest.key)) + + source.clearAllCaches() + + assertNull(source.get(staleQuest.key)) + assertEquals(listOf(newQuest), source.getAll(bbox)) + } + @Test fun `osm quests added or removed triggers listener`() { val quests = listOf(osmQuest(elementId = 1), osmQuest(elementId = 2)) val deleted = listOf(osmQuestKey(elementId = 3), osmQuestKey(elementId = 4))