Merge pull request #6195 from element-hq/feature/bma/callButtonColor

Fix call button color and ensure call can always be declined from the notification

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/receivers/DeclineCallBroadcastReceiver.kt

@@ -29,6 +29,7 @@ class DeclineCallBroadcastReceiver : BroadcastReceiver() {
     companion object {
         const val EXTRA_NOTIFICATION_DATA = "EXTRA_NOTIFICATION_DATA"
     }
+
     @Inject
     lateinit var activeCallManager: ActiveCallManager
 
@@ -40,7 +41,13 @@ class DeclineCallBroadcastReceiver : BroadcastReceiver() {
             ?: return
         context.bindings<CallBindings>().inject(this)
         appCoroutineScope.launch {
-            activeCallManager.hungUpCall(callType = CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
+            activeCallManager.hangUpCall(
+                callType = CallType.RoomCall(
+                    sessionId = notificationData.sessionId,
+                    roomId = notificationData.roomId,
+                ),
+                notificationData = notificationData,
+            )
         }
     }
 }

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/CallScreenPresenter.kt

@@ -100,7 +100,7 @@ class CallScreenPresenter(
                 )
             }
             onDispose {
-                appCoroutineScope.launch { activeCallManager.hungUpCall(callType) }
+                appCoroutineScope.launch { activeCallManager.hangUpCall(callType) }
             }
         }
 

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/IncomingCallActivity.kt

@@ -118,7 +118,7 @@ class IncomingCallActivity : AppCompatActivity() {
     private fun onCancel() {
         val activeCall = activeCallManager.activeCall.value ?: return
         appCoroutineScope.launch {
-            activeCallManager.hungUpCall(callType = activeCall.callType)
+            activeCallManager.hangUpCall(callType = activeCall.callType)
         }
     }
 }

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/ui/IncomingCallScreen.kt

@@ -51,6 +51,9 @@ import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.ui.strings.CommonStrings
 
+/**
+ * Ref: https://www.figma.com/design/0MMNu7cTOzLOlWb7ctTkv3/Element-X?node-id=16501-5740
+ */
 @Composable
 internal fun IncomingCallScreen(
     notificationData: CallNotificationData,
@@ -94,11 +97,8 @@ internal fun IncomingCallScreen(
             )
         }
         Row(
-            modifier = Modifier
-                .fillMaxWidth()
-                .padding(start = 24.dp, end = 24.dp, bottom = 64.dp),
-            horizontalArrangement = Arrangement.SpaceBetween,
-            verticalAlignment = Alignment.CenterVertically
+            modifier = Modifier.padding(bottom = 64.dp),
+            horizontalArrangement = Arrangement.spacedBy(48.dp),
         ) {
             ActionButton(
                 size = 64.dp,
@@ -108,7 +108,6 @@ internal fun IncomingCallScreen(
                 backgroundColor = ElementTheme.colors.iconSuccessPrimary,
                 borderColor = ElementTheme.colors.borderSuccessSubtle
             )
-
             ActionButton(
                 size = 64.dp,
                 onClick = onCancel,
@@ -143,7 +142,7 @@ private fun ActionButton(
             onClick = onClick,
             colors = IconButtonDefaults.filledIconButtonColors(
                 containerColor = backgroundColor,
-                contentColor = Color.White,
+                contentColor = ElementTheme.colors.iconOnSolidPrimary,
             )
         ) {
             Icon(

features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/ActiveCallManager.kt

@@ -72,10 +72,14 @@ interface ActiveCallManager {
     suspend fun registerIncomingCall(notificationData: CallNotificationData)
 
     /**
-     * Called when the active call has been hung up. It will remove any existing UI and the active call.
-     * @param callType The type of call that the user hung up, either an external url one or a room one.
+     * Called to hang up the active call. It will hang up the call and remove any existing UI and the active call.
+     * @param callType The type of call that the user hangs up, either an external url one or a room one.
+     * @param notificationData The data for the incoming call notification.
      */
-    suspend fun hungUpCall(callType: CallType)
+    suspend fun hangUpCall(
+        callType: CallType,
+        notificationData: CallNotificationData? = null,
+    )
 
     /**
      * Called after the user joined a call. It will remove any existing UI and set the call state as [CallState.InCall].
@@ -192,12 +196,28 @@ class DefaultActiveCallManager(
         }
     }
 
-    override suspend fun hungUpCall(callType: CallType) = mutex.withLock {
-        Timber.tag(tag).d("Hung up call: $callType")
+    override suspend fun hangUpCall(
+        callType: CallType,
+        notificationData: CallNotificationData?,
+    ) = mutex.withLock {
+        Timber.tag(tag).d("Hang up call: $callType")
+        cancelIncomingCallNotification()
         val currentActiveCall = activeCall.value ?: run {
+            // activeCall.value can be null if the application has been killed while the call was ringing
+            // Build a currentActiveCall with the provided parameters.
+            notificationData?.let {
+                ActiveCall(
+                    callType = callType,
+                    callState = CallState.Ringing(
+                        notificationData = notificationData,
+                    )
+                )
+            }
+        } ?: run {
             Timber.tag(tag).w("No active call, ignoring hang up")
             return@withLock
         }
+
         if (currentActiveCall.callType != callType) {
             Timber.tag(tag).w("Call type $callType does not match the active call type, ignoring")
             return@withLock
@@ -208,9 +228,13 @@ class DefaultActiveCallManager(
             matrixClientProvider.getOrRestore(notificationData.sessionId).getOrNull()
                 ?.getRoom(notificationData.roomId)
                 ?.declineCall(notificationData.eventId)
+                ?.onFailure {
+                    Timber.e(it, "Failed to decline incoming call")
+                }
+                ?: run {
+                    Timber.tag(tag).d("Couldn't find session or room to decline call for incoming call")
+                }
         }
-
-        cancelIncomingCallNotification()
         if (activeWakeLock?.isHeld == true) {
             Timber.tag(tag).d("Releasing partial wakelock after hang up")
             activeWakeLock.release()
@@ -221,7 +245,6 @@ class DefaultActiveCallManager(
 
     override suspend fun joinedCall(callType: CallType) = mutex.withLock {
         Timber.tag(tag).d("Joined call: $callType")
-
         cancelIncomingCallNotification()
         if (activeWakeLock?.isHeld == true) {
             Timber.tag(tag).d("Releasing partial wakelock after joining call")

features/call/impl/src/test/kotlin/io/element/android/features/call/utils/DefaultActiveCallManagerTest.kt

@@ -155,7 +155,7 @@ class DefaultActiveCallManagerTest {
     }
 
     @Test
-    fun `hungUpCall - removes existing call if the CallType matches`() = runTest {
+    fun `hangUpCall - removes existing call if the CallType matches`() = runTest {
         setupShadowPowerManager()
         val notificationManagerCompat = mockk<NotificationManagerCompat>(relaxed = true)
         val manager = createActiveCallManager(notificationManagerCompat = notificationManagerCompat)
@@ -165,7 +165,7 @@ class DefaultActiveCallManagerTest {
         assertThat(manager.activeCall.value).isNotNull()
         assertThat(manager.activeWakeLock?.isHeld).isTrue()
 
-        manager.hungUpCall(CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
+        manager.hangUpCall(CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
         assertThat(manager.activeCall.value).isNull()
         assertThat(manager.activeWakeLock?.isHeld).isFalse()
 
@@ -192,13 +192,41 @@ class DefaultActiveCallManagerTest {
         val notificationData = aCallNotificationData(roomId = A_ROOM_ID)
         manager.registerIncomingCall(notificationData)
 
-        manager.hungUpCall(CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
+        manager.hangUpCall(CallType.RoomCall(notificationData.sessionId, notificationData.roomId))
 
         coVerify {
             room.declineCall(notificationEventId = notificationData.eventId)
         }
     }
 
+    @Test
+    fun `Decline event - Hangup on a unknown call should send a decline event`() = runTest {
+        setupShadowPowerManager()
+        val notificationManagerCompat = mockk<NotificationManagerCompat>(relaxed = true)
+
+        val room = mockk<JoinedRoom>(relaxed = true)
+
+        val matrixClient = FakeMatrixClient().apply {
+            givenGetRoomResult(A_ROOM_ID, room)
+        }
+        val clientProvider = FakeMatrixClientProvider({ Result.success(matrixClient) })
+
+        val manager = createActiveCallManager(
+            matrixClientProvider = clientProvider,
+            notificationManagerCompat = notificationManagerCompat
+        )
+
+        val notificationData = aCallNotificationData(roomId = A_ROOM_ID)
+        // Do not register the incoming call, so the manager doesn't know about it
+        manager.hangUpCall(
+            callType = CallType.RoomCall(notificationData.sessionId, notificationData.roomId),
+            notificationData = notificationData,
+        )
+        coVerify {
+            room.declineCall(notificationEventId = notificationData.eventId)
+        }
+    }
+
     @OptIn(ExperimentalCoroutinesApi::class)
     @Test
     fun `Decline event - Declining from another session should stop ringing`() = runTest {
@@ -269,7 +297,7 @@ class DefaultActiveCallManagerTest {
     }
 
     @Test
-    fun `hungUpCall - does nothing if the CallType doesn't match`() = runTest {
+    fun `hangUpCall - does nothing if the CallType doesn't match`() = runTest {
         setupShadowPowerManager()
         val notificationManagerCompat = mockk<NotificationManagerCompat>(relaxed = true)
         val manager = createActiveCallManager(notificationManagerCompat = notificationManagerCompat)
@@ -278,11 +306,12 @@ class DefaultActiveCallManagerTest {
         assertThat(manager.activeCall.value).isNotNull()
         assertThat(manager.activeWakeLock?.isHeld).isTrue()
 
-        manager.hungUpCall(CallType.ExternalUrl("https://example.com"))
+        manager.hangUpCall(CallType.ExternalUrl("https://example.com"))
         assertThat(manager.activeCall.value).isNotNull()
         assertThat(manager.activeWakeLock?.isHeld).isTrue()
 
-        verify(exactly = 0) { notificationManagerCompat.cancel(notificationId) }
+        // The notification is always cancelled do not block the user
+        verify(exactly = 1) { notificationManagerCompat.cancel(notificationId) }
     }
 
     @OptIn(ExperimentalCoroutinesApi::class)

features/call/impl/src/test/kotlin/io/element/android/features/call/utils/FakeActiveCallManager.kt

@@ -17,7 +17,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
 
 class FakeActiveCallManager(
     var registerIncomingCallResult: (CallNotificationData) -> Unit = {},
-    var hungUpCallResult: (CallType) -> Unit = {},
+    var hangUpCallResult: (CallType, CallNotificationData?) -> Unit = { _, _ -> },
     var joinedCallResult: (CallType) -> Unit = {},
 ) : ActiveCallManager {
     override val activeCall = MutableStateFlow<ActiveCall?>(null)
@@ -26,8 +26,8 @@ class FakeActiveCallManager(
         registerIncomingCallResult(notificationData)
     }
 
-    override suspend fun hungUpCall(callType: CallType) = simulateLongTask {
-        hungUpCallResult(callType)
+    override suspend fun hangUpCall(callType: CallType, notificationData: CallNotificationData?) = simulateLongTask {
+        hangUpCallResult(callType, notificationData)
     }
 
     override suspend fun joinedCall(callType: CallType) = simulateLongTask {