Merge pull request #4274 from vector-im/feature/adm/notification-redesign

Notifications redesign

Author: ouchadam

changelog.d/1491.bugfix

@@ -0,0 +1 @@
+Stops showing a dedicated redacted event notification, the message notifications will update accordingly

changelog.d/3395.bugfix

@@ -0,0 +1 @@
+Fixes marking individual notifications as read causing other notifications to be dismissed

changelog.d/4152.bugfix

@@ -0,0 +1 @@
+Tentatively fixing the doubled notifications by updating the group summary at specific points in the notification rendering cycle

changelog.d/582.feature

@@ -0,0 +1 @@
+Adding the room name to the invitation notification (if the room summary is available)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt

@@ -22,6 +22,8 @@ import org.json.JSONObject
 import org.matrix.android.sdk.api.extensions.tryOrNull
 import org.matrix.android.sdk.api.failure.MatrixError
 import org.matrix.android.sdk.api.session.crypto.MXCryptoError
+import org.matrix.android.sdk.api.session.room.model.Membership
+import org.matrix.android.sdk.api.session.room.model.RoomMemberContent
 import org.matrix.android.sdk.api.session.room.model.message.MessageContent
 import org.matrix.android.sdk.api.session.room.model.message.MessageType
 import org.matrix.android.sdk.api.session.room.model.relation.RelationDefaultContent
@@ -310,3 +312,6 @@ fun Event.isEdition(): Boolean {
 fun Event.getPresenceContent(): PresenceContent? {
     return content.toModel<PresenceContent>()
 }
+
+fun Event.isInvitation(): Boolean = type == EventType.STATE_ROOM_MEMBER &&
+        content?.toModel<RoomMemberContent>()?.membership == Membership.INVITE

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/ProcessEventForPushTask.kt

@@ -18,6 +18,7 @@ package org.matrix.android.sdk.internal.session.notification
 
 import org.matrix.android.sdk.api.pushrules.rest.PushRule
 import org.matrix.android.sdk.api.session.events.model.EventType
+import org.matrix.android.sdk.api.session.events.model.isInvitation
 import org.matrix.android.sdk.api.session.sync.model.RoomsSyncResponse
 import org.matrix.android.sdk.internal.di.UserId
 import org.matrix.android.sdk.internal.task.Task
@@ -48,14 +49,18 @@ internal class DefaultProcessEventForPushTask @Inject constructor(
         }
         val newJoinEvents = params.syncResponse.join
                 .mapNotNull { (key, value) ->
-                    value.timeline?.events?.map { it.copy(roomId = key) }
+                    value.timeline?.events?.mapNotNull {
+                        it.takeIf { !it.isInvitation() }?.copy(roomId = key)
+                    }
                 }
                 .flatten()
+
         val inviteEvents = params.syncResponse.invite
                 .mapNotNull { (key, value) ->
                     value.inviteState?.events?.map { it.copy(roomId = key) }
                 }
                 .flatten()
+
         val allEvents = (newJoinEvents + inviteEvents).filter { event ->
             when (event.type) {
                 EventType.MESSAGE,

tools/check/forbidden_strings_in_code.txt

@@ -160,7 +160,7 @@ Formatter\.formatShortFileSize===1
 # android\.text\.TextUtils
 
 ### This is not a rule, but a warning: the number of "enum class" has changed. For Json classes, it is mandatory that they have `@JsonClass(generateAdapter = false)`. If the enum is not used as a Json class, change the value in file forbidden_strings_in_code.txt
-enum class===106
+enum class===107
 
 ### Do not import temporary legacy classes
 import org.matrix.android.sdk.internal.legacy.riot===3

vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt

@@ -29,16 +29,13 @@ import com.google.firebase.messaging.FirebaseMessagingService
 import com.google.firebase.messaging.RemoteMessage
 import dagger.hilt.android.AndroidEntryPoint
 import im.vector.app.BuildConfig
-import im.vector.app.R
 import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.core.network.WifiDetector
 import im.vector.app.core.pushers.PushersManager
 import im.vector.app.features.badge.BadgeProxy
 import im.vector.app.features.notifications.NotifiableEventResolver
-import im.vector.app.features.notifications.NotifiableMessageEvent
 import im.vector.app.features.notifications.NotificationDrawerManager
 import im.vector.app.features.notifications.NotificationUtils
-import im.vector.app.features.notifications.SimpleNotifiableEvent
 import im.vector.app.features.settings.VectorDataStore
 import im.vector.app.features.settings.VectorPreferences
 import im.vector.app.push.fcm.FcmHelper
@@ -48,9 +45,7 @@ import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import org.matrix.android.sdk.api.extensions.tryOrNull
 import org.matrix.android.sdk.api.logger.LoggerTag
-import org.matrix.android.sdk.api.pushrules.Action
 import org.matrix.android.sdk.api.session.Session
-import org.matrix.android.sdk.api.session.events.model.Event
 import timber.log.Timber
 import javax.inject.Inject
 
@@ -201,12 +196,11 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() {
             Timber.tag(loggerTag.value).d("Fast lane: start request")
             val event = tryOrNull { session.getEvent(roomId, eventId) } ?: return@launch
 
-            val resolvedEvent = notifiableEventResolver.resolveInMemoryEvent(session, event)
+            val resolvedEvent = notifiableEventResolver.resolveInMemoryEvent(session, event, canBeReplaced = true)
 
             resolvedEvent
                     ?.also { Timber.tag(loggerTag.value).d("Fast lane: notify drawer") }
                     ?.let {
-                        it.isPushGatewayEvent = true
                         notificationDrawerManager.onNotifiableEventReceived(it)
                         notificationDrawerManager.refreshNotificationDrawer()
                     }
@@ -227,87 +221,4 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() {
         }
         return false
     }
-
-    private fun handleNotificationWithoutSyncingMode(data: Map<String, String>, session: Session?) {
-        if (session == null) {
-            Timber.tag(loggerTag.value).e("## handleNotificationWithoutSyncingMode cannot find session")
-            return
-        }
-
-        // The Matrix event ID of the event being notified about.
-        // This is required if the notification is about a particular Matrix event.
-        // It may be omitted for notifications that only contain updated badge counts.
-        // This ID can and should be used to detect duplicate notification requests.
-        val eventId = data["event_id"] ?: return // Just ignore
-
-        val eventType = data["type"]
-        if (eventType == null) {
-            // Just add a generic unknown event
-            val simpleNotifiableEvent = SimpleNotifiableEvent(
-                    session.myUserId,
-                    eventId,
-                    null,
-                    true, // It's an issue in this case, all event will bing even if expected to be silent.
-                    title = getString(R.string.notification_unknown_new_event),
-                    description = "",
-                    type = null,
-                    timestamp = System.currentTimeMillis(),
-                    soundName = Action.ACTION_OBJECT_VALUE_VALUE_DEFAULT,
-                    isPushGatewayEvent = true
-            )
-            notificationDrawerManager.onNotifiableEventReceived(simpleNotifiableEvent)
-            notificationDrawerManager.refreshNotificationDrawer()
-        } else {
-            val event = parseEvent(data) ?: return
-
-            val notifiableEvent = notifiableEventResolver.resolveEvent(event, session)
-
-            if (notifiableEvent == null) {
-                Timber.tag(loggerTag.value).e("Unsupported notifiable event $eventId")
-                if (BuildConfig.LOW_PRIVACY_LOG_ENABLE) {
-                    Timber.tag(loggerTag.value).e("--> $event")
-                }
-            } else {
-                if (notifiableEvent is NotifiableMessageEvent) {
-                    if (notifiableEvent.senderName.isNullOrEmpty()) {
-                        notifiableEvent.senderName = data["sender_display_name"] ?: data["sender"] ?: ""
-                    }
-                    if (notifiableEvent.roomName.isNullOrEmpty()) {
-                        notifiableEvent.roomName = findRoomNameBestEffort(data, session) ?: ""
-                    }
-                }
-
-                notifiableEvent.isPushGatewayEvent = true
-                notifiableEvent.matrixID = session.myUserId
-                notificationDrawerManager.onNotifiableEventReceived(notifiableEvent)
-                notificationDrawerManager.refreshNotificationDrawer()
-            }
-        }
-    }
-
-    private fun findRoomNameBestEffort(data: Map<String, String>, session: Session?): String? {
-        var roomName: String? = data["room_name"]
-        val roomId = data["room_id"]
-        if (null == roomName && null != roomId) {
-            // Try to get the room name from our store
-            roomName = session?.getRoom(roomId)?.roomSummary()?.displayName
-        }
-        return roomName
-    }
-
-    /**
-     * Try to create an event from the FCM data
-     *
-     * @param data the FCM data
-     * @return the event or null if required data are missing
-     */
-    private fun parseEvent(data: Map<String, String>?): Event? {
-        return Event(
-                eventId = data?.get("event_id") ?: return null,
-                senderId = data["sender"],
-                roomId = data["room_id"] ?: return null,
-                type = data["type"] ?: return null,
-                originServerTs = System.currentTimeMillis()
-        )
-    }
 }

vector/src/main/java/im/vector/app/features/notifications/InviteNotifiableEvent.kt

@@ -15,22 +15,18 @@
  */
 package im.vector.app.features.notifications
 
-import androidx.core.app.NotificationCompat
-
 data class InviteNotifiableEvent(
-        override var matrixID: String?,
+        val matrixID: String?,
         override val eventId: String,
         override val editedEventId: String?,
-        var roomId: String,
-        override var noisy: Boolean,
-        override val title: String,
-        override val description: String,
-        override val type: String?,
-        override val timestamp: Long,
-        override var soundName: String?,
-        override var isPushGatewayEvent: Boolean = false) : NotifiableEvent {
-
-    override var hasBeenDisplayed: Boolean = false
-    override var isRedacted: Boolean = false
-    override var lockScreenVisibility = NotificationCompat.VISIBILITY_PUBLIC
-}
+        override val canBeReplaced: Boolean,
+        val roomId: String,
+        val roomName: String?,
+        val noisy: Boolean,
+        val title: String,
+        val description: String,
+        val type: String?,
+        val timestamp: Long,
+        val soundName: String?,
+        override val isRedacted: Boolean = false
+) : NotifiableEvent

vector/src/main/java/im/vector/app/features/notifications/NotifiableEvent.kt

@@ -20,24 +20,11 @@ import java.io.Serializable
 /**
  * Parent interface for all events which can be displayed as a Notification
  */
-interface NotifiableEvent : Serializable {
-    var matrixID: String?
+sealed interface NotifiableEvent : Serializable {
     val eventId: String
     val editedEventId: String?
-    var noisy: Boolean
-    val title: String
-    val description: String?
-    val type: String?
-    val timestamp: Long
-
-    // NotificationCompat.VISIBILITY_PUBLIC , VISIBILITY_PRIVATE , VISIBILITY_SECRET
-    var lockScreenVisibility: Int
-
-    // Compat: Only for android <7, for newer version the sound is defined in the channel
-    var soundName: String?
-    var hasBeenDisplayed: Boolean
-    var isRedacted: Boolean
 
     // Used to know if event should be replaced with the one coming from eventstream
-    var isPushGatewayEvent: Boolean
+    val canBeReplaced: Boolean
+    val isRedacted: Boolean
 }