Merge pull request #583 from vector-im/feature/invot_notification

Invitation notifications are not dismissed automatically if room is joined from another client (#347)

CHANGES.md

@@ -18,6 +18,7 @@ Bugfix:
  - Fix opening a permalink paginates all the history up to the last event (#282)
  - after login, the icon in the top left is a green 'A' for (all communities) rather than my avatar (#267)
  - Picture uploads are unreliable, pictures are shown in wrong aspect ratio on desktop client (#517)
+ - Invitation notifications are not dismissed automatically if room is joined from another client (#347)
 
 Translations:
  -

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/pushrules/PushRuleService.kt

@@ -42,6 +42,7 @@ interface PushRuleService {
 
     interface PushRuleListener {
         fun onMatchRule(event: Event, actions: List<Action>)
+        fun onRoomJoined(roomId: String)
         fun onRoomLeft(roomId: String)
         fun onEventRedacted(redactedEventId: String)
         fun batchFinish()

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/notification/DefaultPushRuleService.kt

@@ -42,7 +42,7 @@ internal class DefaultPushRuleService @Inject constructor(private val getPushRul
                                                           private val monarchy: Monarchy
 ) : PushRuleService {
 
-    private var listeners = ArrayList<PushRuleService.PushRuleListener>()
+    private var listeners = mutableSetOf<PushRuleService.PushRuleListener>()
 
     override fun fetchPushRules(scope: String) {
         getPushRulesTask
@@ -90,22 +90,25 @@ internal class DefaultPushRuleService @Inject constructor(private val getPushRul
     }
 
     override fun updatePushRuleEnableStatus(kind: RuleKind, pushRule: PushRule, enabled: Boolean, callback: MatrixCallback<Unit>): Cancelable {
+        // The rules will be updated, and will come back from the next sync response
         return updatePushRuleEnableStatusTask
                 .configureWith(UpdatePushRuleEnableStatusTask.Params(kind, pushRule, enabled)) {
                     this.callback = callback
                 }
-                // TODO Fetch the rules
                 .executeBy(taskExecutor)
     }
 
     override fun removePushRuleListener(listener: PushRuleService.PushRuleListener) {
-        listeners.remove(listener)
+        synchronized(listeners) {
+            listeners.remove(listener)
+        }
     }
 
 
     override fun addPushRuleListener(listener: PushRuleService.PushRuleListener) {
-        if (!listeners.contains(listener))
+        synchronized(listeners) {
             listeners.add(listener)
+        }
     }
 
 //    fun processEvents(events: List<Event>) {
@@ -121,43 +124,63 @@ internal class DefaultPushRuleService @Inject constructor(private val getPushRul
 //    }
 
     fun dispatchBing(event: Event, rule: PushRule) {
-        try {
+        synchronized(listeners) {
             val actionsList = rule.getActions()
             listeners.forEach {
-                it.onMatchRule(event, actionsList)
+                try {
+                    it.onMatchRule(event, actionsList)
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching bing")
+                }
             }
-        } catch (e: Throwable) {
-            Timber.e(e, "Error while dispatching bing")
         }
     }
 
-    fun dispatchRoomLeft(roomid: String) {
+    fun dispatchRoomJoined(roomId: String) {
-        try {
+        synchronized(listeners) {
             listeners.forEach {
-                it.onRoomLeft(roomid)
+                try {
+                    it.onRoomJoined(roomId)
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching room joined")
+                }
+            }
+        }
+    }
+
+    fun dispatchRoomLeft(roomId: String) {
+        synchronized(listeners) {
+            listeners.forEach {
+                try {
+                    it.onRoomLeft(roomId)
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching room left")
+                }
             }
-        } catch (e: Throwable) {
-            Timber.e(e, "Error while dispatching room left")
         }
     }
 
     fun dispatchRedactedEventId(redactedEventId: String) {
-        try {
+        synchronized(listeners) {
             listeners.forEach {
-                it.onEventRedacted(redactedEventId)
+                try {
+                    it.onEventRedacted(redactedEventId)
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching redacted event")
+                }
             }
-        } catch (e: Throwable) {
-            Timber.e(e, "Error while dispatching room left")
         }
     }
 
     fun dispatchFinish() {
-        try {
+        synchronized(listeners) {
             listeners.forEach {
-                it.batchFinish()
+                try {
+                    it.batchFinish()
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching finish")
+                }
             }
-        } catch (e: Throwable) {
-            Timber.e(e, "Error while dispatching finish")
         }
     }
 }

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

@@ -45,6 +45,10 @@ internal class DefaultProcessEventForPushTask @Inject constructor(
         params.syncResponse.leave.keys.forEach {
             defaultPushRuleService.dispatchRoomLeft(it)
         }
+        // Handle joined rooms
+        params.syncResponse.join.keys.forEach {
+            defaultPushRuleService.dispatchRoomJoined(it)
+        }
         val newJoinEvents = params.syncResponse.join
                 .map { entries ->
                     entries.value.timeline?.events?.map { it.copy(roomId = entries.key) }

vector/src/main/java/im/vector/riotx/features/home/room/list/RoomListViewModel.kt

@@ -173,8 +173,10 @@ class RoomListViewModel @AssistedInject constructor(@Assisted initialState: Room
 
         session.getRoom(roomId)?.leave(object : MatrixCallback<Unit> {
             override fun onSuccess(data: Unit) {
-                // We do not update the joiningRoomsIds here, because, the room is not joined yet regarding the sync data.
+                // We do not update the rejectingRoomsIds here, because, the room is not rejected yet regarding the sync data.
-                // Instead, we wait for the room to be joined
+                // Instead, we wait for the room to be rejected
+                // Known bug: if the user is invited again (after rejecting the first invitation), the loading will be displayed instead of the buttons.
+                // If we update the state, the button will be displayed again, so it's not ideal...
             }
 
             override fun onFailure(failure: Throwable) {

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

@@ -17,6 +17,9 @@ package im.vector.riotx.features.notifications
 
 import java.io.Serializable
 
+/**
+ * Parent interface for all events which can be displayed as a Notification
+ */
 interface NotifiableEvent : Serializable {
     var matrixID: String?
     val eventId: String

vector/src/main/java/im/vector/riotx/features/notifications/NotifiableEventResolver.kt

@@ -150,7 +150,6 @@ class NotifiableEventResolver @Inject constructor(private val stringProvider: St
             notifiableEvent.soundName = null
 
             // Get the avatars URL
-            // TODO They will be not displayed the first time (known limitation)
             notifiableEvent.roomAvatarPath = session.contentUrlResolver()
                     .resolveThumbnail(room.roomSummary()?.avatarUrl,
                             250,

vector/src/main/java/im/vector/riotx/features/notifications/NotificationDrawerManager.kt

@@ -138,7 +138,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
     }
 
     /**
-    Clear all known events and refresh the notification drawer
+     * Clear all known events and refresh the notification drawer
      */
     fun clearAllEvents() {
         synchronized(eventList) {
@@ -147,7 +147,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         refreshNotificationDrawer()
     }
 
-    /** Clear all known message events for this room and refresh the notification drawer */
+    /** Clear all known message events for this room */
     fun clearMessageEventOfRoom(roomId: String?) {
         Timber.v("clearMessageEventOfRoom $roomId")
 
@@ -159,7 +159,6 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
             }
             notificationUtils.cancelNotificationMessage(roomId, ROOM_MESSAGES_NOTIFICATION_ID)
         }
-        refreshNotificationDrawer()
     }
 
     /**
@@ -177,21 +176,14 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         }
     }
 
-    fun homeActivityDidResume(matrixID: String?) {
-        synchronized(eventList) {
-            eventList.removeAll { e ->
-                // messages are cleared when entering room
-                e !is NotifiableMessageEvent
-            }
-        }
-    }
-
     fun clearMemberShipNotificationForRoom(roomId: String) {
         synchronized(eventList) {
             eventList.removeAll { e ->
                 e is InviteNotifiableEvent && e.roomId == roomId
             }
         }
+
+        notificationUtils.cancelNotificationMessage(roomId, ROOM_INVITATION_NOTIFICATION_ID)
     }
 
 
@@ -226,23 +218,26 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
 
             //group events by room to create a single MessagingStyle notif
             val roomIdToEventMap: MutableMap<String, MutableList<NotifiableMessageEvent>> = LinkedHashMap()
-            val simpleEvents: MutableList<NotifiableEvent> = ArrayList()
+            val simpleEvents: MutableList<SimpleNotifiableEvent> = ArrayList()
+            val invitationEvents: MutableList<InviteNotifiableEvent> = ArrayList()
 
             val eventIterator = eventList.listIterator()
             while (eventIterator.hasNext()) {
-                val event = eventIterator.next()
+                when (val event = eventIterator.next()) {
-                if (event is NotifiableMessageEvent) {
+                    is NotifiableMessageEvent -> {
-                    val roomId = event.roomId
+                        val roomId = event.roomId
-                    val roomEvents = roomIdToEventMap.getOrPut(roomId) { ArrayList() }
+                        val roomEvents = roomIdToEventMap.getOrPut(roomId) { ArrayList() }
 
-                    if (shouldIgnoreMessageEventInRoom(roomId) || outdatedDetector?.isMessageOutdated(event) == true) {
+                        if (shouldIgnoreMessageEventInRoom(roomId) || outdatedDetector?.isMessageOutdated(event) == true) {
-                        //forget this event
+                            //forget this event
-                        eventIterator.remove()
+                            eventIterator.remove()
-                    } else {
+                        } else {
-                        roomEvents.add(event)
+                            roomEvents.add(event)
+                        }
                     }
-                } else {
+                    is InviteNotifiableEvent  -> invitationEvents.add(event)
-                    simpleEvents.add(event)
+                    is SimpleNotifiableEvent  -> simpleEvents.add(event)
+                    else                      -> Timber.w("Type not handled")
                 }
             }
 
@@ -251,7 +246,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
 
             var globalLastMessageTimestamp = 0L
 
-            //events have been grouped by roomId
+            // events have been grouped by roomId
             for ((roomId, events) in roomIdToEventMap) {
                 // Build the notification for the room
                 if (events.isEmpty() || events.all { it.isRedacted }) {
@@ -372,11 +367,24 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
             }
 
 
-            //Handle simple events
+            // Handle invitation events
-            for (event in simpleEvents) {
+            for (event in invitationEvents) {
-                //We build a simple event
+                //We build a invitation notification
                 if (firstTime || !event.hasBeenDisplayed) {
-                    val notification = notificationUtils.buildSimpleEventNotification(event, null, session.myUserId)
+                    val notification = notificationUtils.buildRoomInvitationNotification(event, session.myUserId)
+                    notificationUtils.showNotificationMessage(event.roomId, ROOM_INVITATION_NOTIFICATION_ID, notification)
+                    event.hasBeenDisplayed = true //we can consider it as displayed
+                    hasNewEvent = true
+                    summaryIsNoisy = summaryIsNoisy || event.noisy
+                    summaryInboxStyle.addLine(event.description)
+                }
+            }
+
+            // Handle simple events
+            for (event in simpleEvents) {
+                //We build a simple notification
+                if (firstTime || !event.hasBeenDisplayed) {
+                    val notification = notificationUtils.buildSimpleEventNotification(event, session.myUserId)
                     notificationUtils.showNotificationMessage(event.eventId, ROOM_EVENT_NOTIFICATION_ID, notification)
                     event.hasBeenDisplayed = true //we can consider it as displayed
                     hasNewEvent = true
@@ -500,6 +508,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         private const val SUMMARY_NOTIFICATION_ID = 0
         private const val ROOM_MESSAGES_NOTIFICATION_ID = 1
         private const val ROOM_EVENT_NOTIFICATION_ID = 2
+        private const val ROOM_INVITATION_NOTIFICATION_ID = 3
 
         // TODO Mutliaccount
         private const val ROOMS_NOTIFICATIONS_FILE_NAME = "im.vector.notifications.cache"

vector/src/main/java/im/vector/riotx/features/notifications/NotificationUtils.kt

@@ -484,8 +484,70 @@ class NotificationUtils @Inject constructor(private val context: Context,
     }
 
 
-    fun buildSimpleEventNotification(simpleNotifiableEvent: NotifiableEvent,
+    fun buildRoomInvitationNotification(inviteNotifiableEvent: InviteNotifiableEvent,
-                                     largeIcon: Bitmap?,
+                                        matrixId: String): Notification {
+        val accentColor = ContextCompat.getColor(context, R.color.notification_accent_color)
+        // Build the pending intent for when the notification is clicked
+        val smallIcon = R.drawable.ic_status_bar
+
+        val channelID = if (inviteNotifiableEvent.noisy) NOISY_NOTIFICATION_CHANNEL_ID else SILENT_NOTIFICATION_CHANNEL_ID
+
+        return NotificationCompat.Builder(context, channelID)
+                .setContentTitle(stringProvider.getString(R.string.app_name))
+                .setContentText(inviteNotifiableEvent.description)
+                .setGroup(stringProvider.getString(R.string.app_name))
+                .setGroupAlertBehavior(NotificationCompat.GROUP_ALERT_SUMMARY)
+                .setSmallIcon(smallIcon)
+                .setColor(accentColor)
+                .apply {
+                    val roomId = inviteNotifiableEvent.roomId
+                    // offer to type a quick reject button
+                    val rejectIntent = Intent(context, NotificationBroadcastReceiver::class.java)
+                    rejectIntent.action = REJECT_ACTION
+                    rejectIntent.data = Uri.parse("foobar://$roomId&$matrixId")
+                    rejectIntent.putExtra(NotificationBroadcastReceiver.KEY_ROOM_ID, roomId)
+                    val rejectIntentPendingIntent = PendingIntent.getBroadcast(context, System.currentTimeMillis().toInt(), rejectIntent,
+                            PendingIntent.FLAG_UPDATE_CURRENT)
+
+                    addAction(
+                            R.drawable.vector_notification_reject_invitation,
+                            stringProvider.getString(R.string.reject),
+                            rejectIntentPendingIntent)
+
+                    // offer to type a quick accept button
+                    val joinIntent = Intent(context, NotificationBroadcastReceiver::class.java)
+                    joinIntent.action = JOIN_ACTION
+                    joinIntent.data = Uri.parse("foobar://$roomId&$matrixId")
+                    rejectIntent.putExtra(NotificationBroadcastReceiver.KEY_ROOM_ID, roomId)
+                    val joinIntentPendingIntent = PendingIntent.getBroadcast(context, System.currentTimeMillis().toInt(), joinIntent,
+                            PendingIntent.FLAG_UPDATE_CURRENT)
+                    addAction(
+                            R.drawable.vector_notification_accept_invitation,
+                            stringProvider.getString(R.string.join),
+                            joinIntentPendingIntent)
+
+                    val contentIntent = Intent(context, HomeActivity::class.java)
+                    contentIntent.flags = Intent.FLAG_ACTIVITY_CLEAR_TOP or Intent.FLAG_ACTIVITY_SINGLE_TOP
+                    //pending intent get reused by system, this will mess up the extra params, so put unique info to avoid that
+                    contentIntent.data = Uri.parse("foobar://" + inviteNotifiableEvent.eventId)
+                    setContentIntent(PendingIntent.getActivity(context, 0, contentIntent, 0))
+
+                    if (inviteNotifiableEvent.noisy) {
+                        //Compat
+                        priority = NotificationCompat.PRIORITY_DEFAULT
+                        vectorPreferences.getNotificationRingTone()?.let {
+                            setSound(it)
+                        }
+                        setLights(accentColor, 500, 500)
+                    } else {
+                        priority = NotificationCompat.PRIORITY_LOW
+                    }
+                    setAutoCancel(true)
+                }
+                .build()
+    }
+
+    fun buildSimpleEventNotification(simpleNotifiableEvent: SimpleNotifiableEvent,
                                      matrixId: String): Notification {
         val accentColor = ContextCompat.getColor(context, R.color.notification_accent_color)
         // Build the pending intent for when the notification is clicked
@@ -500,47 +562,14 @@ class NotificationUtils @Inject constructor(private val context: Context,
                 .setGroupAlertBehavior(NotificationCompat.GROUP_ALERT_SUMMARY)
                 .setSmallIcon(smallIcon)
                 .setColor(accentColor)
+                .setAutoCancel(true)
                 .apply {
-                    if (simpleNotifiableEvent is InviteNotifiableEvent) {
-                        val roomId = simpleNotifiableEvent.roomId
-                        // offer to type a quick reject button
-                        val rejectIntent = Intent(context, NotificationBroadcastReceiver::class.java)
-                        rejectIntent.action = REJECT_ACTION
-                        rejectIntent.data = Uri.parse("foobar://$roomId&$matrixId")
-                        rejectIntent.putExtra(NotificationBroadcastReceiver.KEY_ROOM_ID, roomId)
-                        val rejectIntentPendingIntent = PendingIntent.getBroadcast(context, System.currentTimeMillis().toInt(), rejectIntent,
-                                PendingIntent.FLAG_UPDATE_CURRENT)
-
-                        addAction(
-                                R.drawable.vector_notification_reject_invitation,
-                                stringProvider.getString(R.string.reject),
-                                rejectIntentPendingIntent)
-
-                        // offer to type a quick accept button
-                        val joinIntent = Intent(context, NotificationBroadcastReceiver::class.java)
-                        joinIntent.action = JOIN_ACTION
-                        joinIntent.data = Uri.parse("foobar://$roomId&$matrixId")
-                        rejectIntent.putExtra(NotificationBroadcastReceiver.KEY_ROOM_ID, roomId)
-                        val joinIntentPendingIntent = PendingIntent.getBroadcast(context, System.currentTimeMillis().toInt(), joinIntent,
-                                PendingIntent.FLAG_UPDATE_CURRENT)
-                        addAction(
-                                R.drawable.vector_notification_accept_invitation,
-                                stringProvider.getString(R.string.join),
-                                joinIntentPendingIntent)
-                    } else {
-                        setAutoCancel(true)
-                    }
-
                     val contentIntent = Intent(context, HomeActivity::class.java)
                     contentIntent.flags = Intent.FLAG_ACTIVITY_CLEAR_TOP or Intent.FLAG_ACTIVITY_SINGLE_TOP
                     //pending intent get reused by system, this will mess up the extra params, so put unique info to avoid that
                     contentIntent.data = Uri.parse("foobar://" + simpleNotifiableEvent.eventId)
                     setContentIntent(PendingIntent.getActivity(context, 0, contentIntent, 0))
 
-                    if (largeIcon != null) {
-                        setLargeIcon(largeIcon)
-                    }
-
                     if (simpleNotifiableEvent.noisy) {
                         //Compat
                         priority = NotificationCompat.PRIORITY_DEFAULT

vector/src/main/java/im/vector/riotx/features/notifications/PushRuleTriggerListener.kt

@@ -58,6 +58,11 @@ class PushRuleTriggerListener @Inject constructor(
 
     override fun onRoomLeft(roomId: String) {
         notificationDrawerManager.clearMessageEventOfRoom(roomId)
+        notificationDrawerManager.clearMemberShipNotificationForRoom(roomId)
+    }
+
+    override fun onRoomJoined(roomId: String) {
+        notificationDrawerManager.clearMemberShipNotificationForRoom(roomId)
     }
 
     override fun onEventRedacted(redactedEventId: String) {