Tidy and simplify using PR suggestions

features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/AndroidLocationActions.kt

@@ -19,45 +19,28 @@ package io.element.android.features.location.impl.show
 import android.content.Context
 import android.content.Intent
 import android.net.Uri
-import androidx.compose.runtime.Composable
+import androidx.annotation.VisibleForTesting
-import androidx.compose.runtime.DisposableEffect
-import androidx.compose.ui.platform.LocalContext
 import com.squareup.anvil.annotations.ContributesBinding
 import io.element.android.features.location.api.Location
-import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.di.AppScope
-import kotlinx.coroutines.withContext
+import io.element.android.libraries.di.ApplicationContext
 import timber.log.Timber
 import javax.inject.Inject
 
 @ContributesBinding(AppScope::class)
 class AndroidLocationActions @Inject constructor(
-    private val coroutineDispatchers: CoroutineDispatchers
+    @ApplicationContext private val appContext: Context
 ) : LocationActions {
 
     private var activityContext: Context? = null
 
-    @Composable
+    override fun share(location: Location, label: String?) {
-    override fun Configure() {
-        val context = LocalContext.current
-        return DisposableEffect(Unit) {
-            activityContext = context
-            onDispose {
-                activityContext = null
-            }
-        }
-    }
-
-    override suspend fun share(location: Location, label: String?) {
         runCatching {
-            // Ref: https://developer.android.com/guide/components/intents-common#ViewMap
+            val uri = Uri.parse(buildUrl(location, label))
-            val suffix = if (label != null) "(${Uri.encode(label)})" else ""
-            val uri = Uri.parse("geo:0,0?q=${location.lat},${location.lon}$suffix")
             val showMapsIntent = Intent(Intent.ACTION_VIEW).setData(uri)
             val chooserIntent = Intent.createChooser(showMapsIntent, null)
-            withContext(coroutineDispatchers.main) {
+            chooserIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
-                activityContext!!.startActivity(chooserIntent)
+            appContext.startActivity(chooserIntent)
-            }
         }.onSuccess {
             Timber.v("Open location succeed")
         }.onFailure {
@@ -65,3 +48,18 @@ class AndroidLocationActions @Inject constructor(
         }
     }
 }
+
+@VisibleForTesting
+internal fun buildUrl(
+    location: Location,
+    label: String?,
+    urlEncoder: (String) -> String = Uri::encode
+): String {
+    // Ref: https://developer.android.com/guide/components/intents-common#ViewMap
+    val base = "geo:0,0?q=%.6f,%.6f".format(location.lat, location.lon)
+    return if (label == null) {
+        base
+    } else {
+        "%s (%s)".format(base, urlEncoder(label))
+    }
+}

features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/LocationActions.kt

@@ -16,14 +16,8 @@
 
 package io.element.android.features.location.impl.show
 
-import androidx.compose.runtime.Composable
 import io.element.android.features.location.api.Location
 
 interface LocationActions {
-
+    fun share(location: Location, label: String?)
-    @Composable
-    fun Configure()
-
-    suspend fun share(location: Location, label: String?)
-
 }

features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenter.kt

@@ -23,8 +23,6 @@ import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
 import io.element.android.features.location.api.Location
 import io.element.android.libraries.architecture.Presenter
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch
 
 class ShowLocationPresenter @AssistedInject constructor(
     private val actions: LocationActions,
@@ -39,20 +37,13 @@ class ShowLocationPresenter @AssistedInject constructor(
 
     @Composable
     override fun present(): ShowLocationState {
-        val coroutineScope = rememberCoroutineScope()
-        actions.Configure()
-
         return ShowLocationState(
             location = location,
             description = description
         ) {
             when (it) {
-                ShowLocationEvents.Share -> coroutineScope.share(location, description)
+                ShowLocationEvents.Share -> actions.share(location, description)
             }
         }
     }
-
-    private fun CoroutineScope.share(location: Location, label: String?) = launch {
-        actions.share(location, label)
-    }
 }

features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/AndroidLocationActionsTest.kt

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.features.location.impl.show
+
+import com.google.common.truth.Truth.assertThat
+import io.element.android.features.location.api.Location
+import org.junit.Test
+import java.net.URLEncoder
+
+internal class AndroidLocationActionsTest {
+
+    // We use an Android-native encoder in the actual app, switch to an equivalent JVM one for the tests
+    private fun urlEncoder(input: String) = URLEncoder.encode(input, "US-ASCII")
+
+    @Test
+    fun `buildUrl - truncates excessive decimals to 6dp`() {
+        val location = Location(
+            lat = 1.234567890123,
+            lon = 123.456789012345,
+            accuracy = 0f
+        )
+
+        val actual = buildUrl(location, null, ::urlEncoder)
+        val expected = "geo:0,0?q=1.234568,123.456789"
+
+        assertThat(actual).isEqualTo(expected)
+    }
+
+    @Test
+    fun `buildUrl - appends label if set`() {
+        val location = Location(
+            lat = 1.000001,
+            lon = 2.000001,
+            accuracy = 0f
+        )
+
+        val actual = buildUrl(location, "point", ::urlEncoder)
+        val expected = "geo:0,0?q=1.000001,2.000001 (point)"
+
+        assertThat(actual).isEqualTo(expected)
+    }
+
+    @Test
+    fun `buildUrl - URL encodes label`() {
+        val location = Location(
+            lat = 1.000001,
+            lon = 2.000001,
+            accuracy = 0f
+        )
+
+        val actual = buildUrl(location, "(weird/stuff here)", ::urlEncoder)
+        val expected = "geo:0,0?q=1.000001,2.000001 (%28weird%2Fstuff+here%29)"
+
+        assertThat(actual).isEqualTo(expected)
+    }
+}

features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/FakeLocationActions.kt

@@ -16,26 +16,17 @@
 
 package io.element.android.features.location.impl.show
 
-import androidx.compose.runtime.Composable
 import io.element.android.features.location.api.Location
 
 class FakeLocationActions : LocationActions {
 
-    var configured = false
-        private set
-
     var sharedLocation: Location? = null
         private set
 
     var sharedLabel: String? = null
         private set
 
-    @Composable
+    override fun share(location: Location, label: String?) {
-    override fun Configure() {
-        configured = true
-    }
-
-    override suspend fun share(location: Location, label: String?) {
         sharedLocation = location
         sharedLabel = label
     }

features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenterTest.kt

@@ -60,7 +60,6 @@ class ShowLocationPresenterTest {
             val initialState = awaitItem()
             initialState.eventSink(ShowLocationEvents.Share)
 
-            Truth.assertThat(actions.configured).isTrue()
             Truth.assertThat(actions.sharedLocation).isEqualTo(location)
             Truth.assertThat(actions.sharedLabel).isEqualTo(A_DESCRIPTION)
         }