Rework counting of unread chats

Avoid counting chats twice that are actually unread and additionally
manually marked as unread.
Also fix some corner cases.

Change-Id: I0bc8a3bcdb8b658618b627648eec34305e66082b
This commit is contained in:
SpiritCroc 2021-11-09 10:28:45 +01:00
parent 19e065cbea
commit c3a6e3bd9b
14 changed files with 49 additions and 56 deletions

View file

@ -139,17 +139,12 @@ data class RoomSummary(
}
}
fun scNotificationCountWithManualUnread(): Int {
// Keep sync with RoomSummaryEntity.notificationCountOrMarkedUnread!
fun notificationCountOrMarkedUnread(): Int {
return when {
notificationCount > 0 -> {
notificationCount
}
markedUnread -> {
1
}
else -> {
0
}
notificationCount > 0 -> notificationCount
markedUnread -> 1
else -> 0
}
}

View file

@ -19,10 +19,8 @@ package org.matrix.android.sdk.api.session.room.summary
data class RoomAggregateNotificationCount(
val notificationCount: Int,
val highlightCount: Int,
val unreadCount: Int,
val markedUnreadCount: Int
val unreadCount: Int
) {
val totalCount = notificationCount + markedUnreadCount
val totalCount = notificationCount
val isHighlight = highlightCount > 0
val markedUnread = markedUnreadCount > 0
}

View file

@ -136,6 +136,15 @@ internal open class RoomSummaryEntity(
}
}
// Keep sync with RoomSummaryEntity.notificationCountOrMarkedUnread!
fun notificationCountOrMarkedUnread(): Int {
return when {
notificationCount > 0 -> notificationCount
markedUnread -> 1
else -> 0
}
}
var aggregatedUnreadCount: Int = 0
set(value) {
if (value != field) field = value

View file

@ -229,39 +229,36 @@ internal class RoomSummaryDataSource @Inject constructor(@SessionDatabase privat
val preferenceProvider = StaticScSdkHelper.scSdkPreferenceProvider
var notificationCount: RoomAggregateNotificationCount? = null
monarchy.doWithRealm { realm ->
val roomSummariesQuery = roomSummariesQuery(realm, queryParams)
val markedUnreadCount = roomSummariesQuery(realm, queryParams).equalTo(RoomSummaryEntityFields.MARKED_UNREAD, true).count().toInt()
notificationCount = if (preferenceProvider?.aggregateUnreadRoomCounts() != false) {
// Count chats
val notifCount = roomSummariesQuery.greaterThan(RoomSummaryEntityFields.NOTIFICATION_COUNT, 0).count().toInt()
val highlightCount = roomSummariesQuery.greaterThan(RoomSummaryEntityFields.HIGHLIGHT_COUNT, 0).count().toInt()
val notifCount = roomSummariesQuery(realm, queryParams).beginGroup().greaterThan(RoomSummaryEntityFields.NOTIFICATION_COUNT, 0).or().equalTo(RoomSummaryEntityFields.MARKED_UNREAD, true).endGroup().count().toInt()
val highlightCount = roomSummariesQuery(realm, queryParams).greaterThan(RoomSummaryEntityFields.HIGHLIGHT_COUNT, 0).count().toInt()
val unreadCount = max(
// Preferred since MSC 2654
roomSummariesQuery.greaterThan(RoomSummaryEntityFields.UNREAD_COUNT, 0).count().toInt(),
roomSummariesQuery(realm, queryParams).greaterThan(RoomSummaryEntityFields.UNREAD_COUNT, 0).count().toInt(),
// TODO-SC-merge: properly use dm/non-dm flag? (note that this will be likely overwritten either way by above field from MSC 2654)
roomSummariesQuery(realm, queryParams).equalTo(getUnreadRoomSummaryField(false), true).count().toInt()
)
RoomAggregateNotificationCount(
notifCount,
highlightCount,
unreadCount,
markedUnreadCount
unreadCount
)
} else {
// Sum unread counts
val notifCount = roomSummariesQuery.sum(RoomSummaryEntityFields.NOTIFICATION_COUNT).toInt()
val highlightCount = roomSummariesQuery.sum(RoomSummaryEntityFields.HIGHLIGHT_COUNT).toInt()
val notifCount = roomSummariesQuery(realm, queryParams).sum(RoomSummaryEntityFields.NOTIFICATION_COUNT).toInt() +
roomSummariesQuery(realm, queryParams).beginGroup().lessThanOrEqualTo(RoomSummaryEntityFields.NOTIFICATION_COUNT, 0).and().equalTo(RoomSummaryEntityFields.MARKED_UNREAD, true).endGroup().count().toInt()
val highlightCount = roomSummariesQuery(realm, queryParams).sum(RoomSummaryEntityFields.HIGHLIGHT_COUNT).toInt()
val unreadCount = max(
// Preferred since MSC 2654
roomSummariesQuery.sum(RoomSummaryEntityFields.UNREAD_COUNT).toInt(),
roomSummariesQuery(realm, queryParams).sum(RoomSummaryEntityFields.UNREAD_COUNT).toInt(),
// TODO-SC-merge: properly use dm/non-dm flag? (note that this will be likely overwritten either way by above field from MSC 2654)
roomSummariesQuery(realm, queryParams).equalTo(getUnreadRoomSummaryField(false), true).count().toInt()
)
RoomAggregateNotificationCount(
notifCount,
highlightCount,
unreadCount,
markedUnreadCount
unreadCount
)
}
}

View file

@ -450,7 +450,6 @@ internal class RoomSummaryUpdater @Inject constructor(
var highlightCount = 0
var notificationCount = 0
var unreadCount = 0
var markedUnreadCount = 0
var aggregateUnreadCount = 0
var aggregateNotificationCount = 0
realm.where(RoomSummaryEntity::class.java)
@ -462,16 +461,15 @@ internal class RoomSummaryUpdater @Inject constructor(
.findAll().forEach {
if (!it.isHiddenFromUser) {
highlightCount += it.highlightCount
notificationCount += it.notificationCount
notificationCount += it.notificationCountOrMarkedUnread()
unreadCount += it.safeUnreadCount()
aggregateNotificationCount += min(it.notificationCount, 1)
aggregateUnreadCount += min(it.safeUnreadCount(), 1)
markedUnreadCount += if (it.markedUnread) 1 else 0
aggregateNotificationCount += if (it.notificationCountOrMarkedUnread() > 0) 1 else 0
aggregateUnreadCount += if (it.safeUnreadCount() > 0) 1 else 0
}
}
space.highlightCount = highlightCount
space.notificationCount = notificationCount + markedUnreadCount
space.notificationCount = notificationCount
space.unreadCount = unreadCount
space.aggregatedUnreadCount = aggregateUnreadCount
space.aggregatedNotificationCount = aggregateNotificationCount

View file

@ -187,7 +187,7 @@ class HomeDetailFragment @Inject constructor(
count = state.otherSpacesUnread.totalCount,
highlighted = state.otherSpacesUnread.isHighlight,
unread = state.otherSpacesUnread.unreadCount,
markedUnread = state.otherSpacesUnread.markedUnread
markedUnread = false
)
)
}

View file

@ -44,8 +44,8 @@ import org.matrix.android.sdk.rx.asObservable
import java.util.concurrent.TimeUnit
data class UnreadMessagesState(
val homeSpaceUnread: RoomAggregateNotificationCount = RoomAggregateNotificationCount(0, 0, 0, 0),
val otherSpacesUnread: RoomAggregateNotificationCount = RoomAggregateNotificationCount(0, 0, 0, 0)
val homeSpaceUnread: RoomAggregateNotificationCount = RoomAggregateNotificationCount(0, 0, 0),
val otherSpacesUnread: RoomAggregateNotificationCount = RoomAggregateNotificationCount(0, 0, 0)
) : MavericksState
data class CountInfo(
@ -110,8 +110,7 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor(@Assisted initia
homeSpaceUnread = RoomAggregateNotificationCount(
counts.notificationCount + invites,
highlightCount = counts.highlightCount + invites,
unreadCount = counts.unreadCount,
markedUnreadCount = counts.markedUnreadCount
unreadCount = counts.unreadCount
)
)
}
@ -132,8 +131,8 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor(@Assisted initia
is RoomGroupingMethod.ByLegacyGroup -> {
// currently not supported
CountInfo(
RoomAggregateNotificationCount(0, 0, 0, 0),
RoomAggregateNotificationCount(0, 0, 0, 0)
RoomAggregateNotificationCount(0, 0, 0),
RoomAggregateNotificationCount(0, 0, 0)
)
}
is RoomGroupingMethod.BySpace -> {
@ -171,8 +170,7 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor(@Assisted initia
val counts = RoomAggregateNotificationCount(
totalCount.notificationCount + inviteCount,
totalCount.highlightCount + inviteCount,
totalCount.unreadCount,
totalCount.markedUnreadCount
totalCount.unreadCount
)
// SC: count total room notifications for drawer badge, instead of filtering for others like Element does,
@ -191,8 +189,7 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor(@Assisted initia
val topLevelCounts = RoomAggregateNotificationCount(
topLevelTotalCount.notificationCount + inviteCount + spaceInviteCount,
topLevelTotalCount.highlightCount + inviteCount + spaceInviteCount,
topLevelTotalCount.unreadCount,
topLevelTotalCount.markedUnreadCount
topLevelTotalCount.unreadCount
)
CountInfo(homeCount = counts, otherCount = topLevelCounts)
@ -225,16 +222,16 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor(@Assisted initia
}
null -> {
CountInfo(
RoomAggregateNotificationCount(0, 0, 0, 0),
RoomAggregateNotificationCount(0, 0, 0, 0)
RoomAggregateNotificationCount(0, 0, 0),
RoomAggregateNotificationCount(0, 0, 0)
)
}
}
}
).execute {
copy(
homeSpaceUnread = it.invoke()?.homeCount ?: RoomAggregateNotificationCount(0, 0, 0, 0),
otherSpacesUnread = it.invoke()?.otherCount ?: RoomAggregateNotificationCount(0, 0, 0, 0)
homeSpaceUnread = it.invoke()?.homeCount ?: RoomAggregateNotificationCount(0, 0, 0),
otherSpacesUnread = it.invoke()?.otherCount ?: RoomAggregateNotificationCount(0, 0, 0)
)
}
}

View file

@ -30,7 +30,7 @@ class RoomListDisplayModeFilter(private val displayMode: RoomListDisplayMode) :
return when (displayMode) {
RoomListDisplayMode.ALL -> true
RoomListDisplayMode.NOTIFICATIONS ->
roomSummary.scNotificationCountWithManualUnread() > 0 || roomSummary.membership == Membership.INVITE || roomSummary.userDrafts.isNotEmpty()
roomSummary.notificationCountOrMarkedUnread() > 0 || roomSummary.membership == Membership.INVITE || roomSummary.userDrafts.isNotEmpty()
RoomListDisplayMode.PEOPLE -> roomSummary.isDirect && roomSummary.membership.isActive()
RoomListDisplayMode.ROOMS -> !roomSummary.isDirect && roomSummary.membership.isActive()
RoomListDisplayMode.FILTERED -> roomSummary.membership == Membership.JOIN

View file

@ -309,7 +309,7 @@ class RoomListFragment @Inject constructor(
notificationCount = counts.totalCount,
isHighlighted = counts.isHighlight,
unread = counts.unreadCount,
markedUnread = counts.markedUnread
markedUnread = false
))
}
section.isExpanded.observe(viewLifecycleOwner) { _ ->
@ -350,7 +350,7 @@ class RoomListFragment @Inject constructor(
notificationCount = counts.totalCount,
isHighlighted = counts.isHighlight,
unread = counts.unreadCount,
markedUnread = counts.markedUnread
markedUnread = false
))
}
section.isExpanded.observe(viewLifecycleOwner) { _ ->

View file

@ -499,7 +499,7 @@ class RoomListSectionBuilderSpace(
?.notificationCount
?.postValue(
if (countRoomAsNotif) {
RoomAggregateNotificationCount(it.size, it.size, 0, 0)
RoomAggregateNotificationCount(it.size, it.size, 0)
} else {
session.getNotificationCountForRooms(
roomQueryParams.process(spaceFilterStrategy, appStateHandler.safeActiveSpaceId())

View file

@ -29,6 +29,6 @@ data class RoomsSection(
val liveList: LiveData<List<RoomSummary>>? = null,
val liveSuggested: LiveData<SuggestedRoomInfo>? = null,
val isExpanded: MutableLiveData<Boolean> = MutableLiveData(true),
val notificationCount: MutableLiveData<RoomAggregateNotificationCount> = MutableLiveData(RoomAggregateNotificationCount(0, 0, 0, 0)),
val notificationCount: MutableLiveData<RoomAggregateNotificationCount> = MutableLiveData(RoomAggregateNotificationCount(0, 0, 0)),
val notifyOfLocalEcho: Boolean = false
)

View file

@ -34,5 +34,5 @@ data class SpaceListViewState(
val spaceOrderLocalEchos: Map<String, String?>? = null,
val legacyGroups: List<GroupSummary>? = null,
val expandedStates: Map<String, Boolean> = emptyMap(),
val homeAggregateCount: RoomAggregateNotificationCount = RoomAggregateNotificationCount(0, 0, 0, 0)
val homeAggregateCount: RoomAggregateNotificationCount = RoomAggregateNotificationCount(0, 0, 0)
) : MavericksState

View file

@ -138,7 +138,7 @@ class SpaceSummaryController @Inject constructor(
homeSpaceSummaryItem {
id("space_home")
selected(selected is RoomGroupingMethod.BySpace && selected.space() == null)
countState(UnreadCounterBadgeView.State(homeCount.totalCount, homeCount.isHighlight, homeCount.unreadCount, homeCount.markedUnread))
countState(UnreadCounterBadgeView.State(homeCount.totalCount, homeCount.isHighlight, homeCount.unreadCount, false))
listener { host.callback?.onSpaceSelected(null) }
}

View file

@ -141,8 +141,7 @@ class SpacesListViewModel @AssistedInject constructor(@Assisted initialState: Sp
val counts = RoomAggregateNotificationCount(
totalCount.notificationCount + inviteCount,
totalCount.highlightCount + inviteCount,
totalCount.unreadCount,
if (totalCount.markedUnread) 1 else 0
totalCount.unreadCount
)
setState {
copy(