From 714bde608d06535ddfa30b5d0cf43a3063f796ec Mon Sep 17 00:00:00 2001 From: Lakoja Date: Sun, 12 Mar 2023 14:39:41 +0100 Subject: [PATCH 1/7] 818: Show +numbers for favorites and boosts --- .../notifications/StatusNotificationViewHolder.kt | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/StatusNotificationViewHolder.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/StatusNotificationViewHolder.kt index 0b1d8dca44..392d8487ee 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/StatusNotificationViewHolder.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/StatusNotificationViewHolder.kt @@ -242,7 +242,9 @@ internal class StatusNotificationViewHolder( animateEmojis: Boolean ) { val statusViewData = notificationViewData.statusViewData - val displayName = notificationViewData.account.name.unicodeWrap() + val favoritesCount = statusViewData?.status?.favouritesCount ?: 0 + val reblogCount = statusViewData?.status?.reblogsCount ?: 0 + var userPart = notificationViewData.account.name.unicodeWrap() val type = notificationViewData.type val context = binding.notificationTopText.context val format: String @@ -251,10 +253,16 @@ internal class StatusNotificationViewHolder( Notification.Type.FAVOURITE -> { icon = getIconWithColor(context, R.drawable.ic_star_24dp, R.color.tusky_orange) format = context.getString(R.string.notification_favourite_format) + if (favoritesCount > 1) { + userPart += " +" + (favoritesCount - 1) + } } Notification.Type.REBLOG -> { icon = getIconWithColor(context, R.drawable.ic_repeat_24dp, R.color.tusky_blue) format = context.getString(R.string.notification_reblog_format) + if (reblogCount > 1) { + userPart += " +" + (reblogCount - 1) + } } Notification.Type.STATUS -> { icon = getIconWithColor(context, R.drawable.ic_home_24dp, R.color.tusky_blue) @@ -275,13 +283,13 @@ internal class StatusNotificationViewHolder( null, null ) - val wholeMessage = String.format(format, displayName) + val wholeMessage = String.format(format, userPart) val str = SpannableStringBuilder(wholeMessage) val displayNameIndex = format.indexOf("%s") str.setSpan( StyleSpan(Typeface.BOLD), displayNameIndex, - displayNameIndex + displayName.length, + displayNameIndex + userPart.length, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE ) val emojifiedText = str.emojify( From f1c115c1efab293736fec99d85fe2dffa077d160 Mon Sep 17 00:00:00 2001 From: Lakoja Date: Tue, 14 Mar 2023 09:33:04 +0100 Subject: [PATCH 2/7] 818: Track favorites and filter old duplicated entries --- .../notifications/NotificationsFragment.kt | 29 ++++++++++++- .../notifications/NotificationsViewModel.kt | 41 ++++++++++++++++++- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt index 3c0f0da4d5..5cda21cd85 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt @@ -220,10 +220,13 @@ class NotificationsFragment : (binding.recyclerView.itemAnimator as SimpleItemAnimator?)!!.supportsChangeAnimations = false - // Signal the user that a refresh has loaded new items above their current position - // by scrolling up slightly to disclose the new content + adapter.registerAdapterDataObserver(object : RecyclerView.AdapterDataObserver() { override fun onItemRangeInserted(positionStart: Int, itemCount: Int) { + removeDuplicateOlderEntries(positionStart) + + // Signal the user that a refresh has loaded new items above their current position + // by scrolling up slightly to disclose the new content if (positionStart == 0 && adapter.itemCount != itemCount) { binding.recyclerView.post { if (getView() != null) { @@ -250,6 +253,7 @@ class NotificationsFragment : viewModel.pagingData.collectLatest { pagingData -> Log.d(TAG, "Submitting data to adapter") adapter.submitData(pagingData) + // TODO why is this called always _before_ anything from NotificationsPagingSource is loaded (and with what data)? } } @@ -437,6 +441,27 @@ class NotificationsFragment : } } + private fun removeDuplicateOlderEntries(positionStart: Int) { + val dataList = adapter.snapshot().items + + Log.w(TAG, "found elements in adapter "+dataList.size) + + for (pos in dataList.size - 1 downTo positionStart) { + val notificationViewData = dataList[pos] + + val status = notificationViewData.statusViewData?.status + ?: continue + + if (!viewModel.hasNewestNotificationId(notificationViewData.type, status.id, notificationViewData.id)) { + Log.w(TAG, "Removing old notification at "+pos+" for "+status.id) + + adapter.notifyItemRemoved(pos) + } + } + + Log.w(TAG, "elements after second "+ adapter.snapshot().size) + } + override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { menuInflater.inflate(R.menu.fragment_notifications, menu) menu.findItem(R.id.action_refresh)?.apply { diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt index 1f3f982b9d..69fd9387e8 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt @@ -24,6 +24,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import androidx.paging.PagingData import androidx.paging.cachedIn +import androidx.paging.filter import androidx.paging.map import at.connyduck.calladapter.networkresult.getOrThrow import com.keylesspalace.tusky.R @@ -505,13 +506,28 @@ class NotificationsViewModel @Inject constructor( ) } + // Status id -> (highest) Notification id + val seenFavorites = HashMap() + private fun getNotifications( filters: Set, initialKey: String? = null ): Flow> { return repository.getNotificationsStream(filter = filters, initialKey = initialKey) .map { pagingData -> - pagingData.map { notification -> + pagingData.filter { notification -> + val status = notification.status + ?: return@filter true + + return@filter if (hasNewestNotificationId(notification.type, status.id, notification.id)) { + seenFavorites[status.id] = notification.id // TODO move to hasNewestNotificationId? + + true + } else { + false + } + } + .map { notification -> notification.toViewData( isShowingContent = statusDisplayOptions.value.showSensitiveMedia || !(notification.status?.actionableStatus?.sensitive ?: false), @@ -533,6 +549,29 @@ class NotificationsViewModel @Inject constructor( return initialKey } + fun hasNewestNotificationId(type: Notification.Type, statusId: String, notificationId: String): Boolean { + if (type != Notification.Type.FAVOURITE) { + return true + } + + val highestNotificationId = seenFavorites[statusId] + + return highestNotificationId == null || isEqualOrNewer(notificationId, highestNotificationId) + } + + /** + * NOTE this currently assumes that the ids are integers. Can that change? If it does all notifications are unequal. + */ + fun isEqualOrNewer(thisId: String, idToCompare: String): Boolean { + try { + return thisId.toInt() >= idToCompare.toInt() + } catch (exc: NumberFormatException) { + Log.e(TAG, "Cannot compare ids; not numbers: "+thisId+"/"+idToCompare) + + return false + } + } + /** * @return Flow of relevant preferences that change the UI */ From 6ca832f7ce3f15ebec4792a301efbb5e1b659c0d Mon Sep 17 00:00:00 2001 From: Lakoja Date: Wed, 15 Mar 2023 10:47:31 +0100 Subject: [PATCH 3/7] 818: Also support boosts; rework logging a bit --- .../notifications/NotificationsFragment.kt | 8 ++---- .../notifications/NotificationsViewModel.kt | 28 +++++++++++++------ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt index 5cda21cd85..3769b5979f 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt @@ -444,22 +444,18 @@ class NotificationsFragment : private fun removeDuplicateOlderEntries(positionStart: Int) { val dataList = adapter.snapshot().items - Log.w(TAG, "found elements in adapter "+dataList.size) - - for (pos in dataList.size - 1 downTo positionStart) { + for (pos in dataList.lastIndex downTo positionStart) { val notificationViewData = dataList[pos] val status = notificationViewData.statusViewData?.status ?: continue if (!viewModel.hasNewestNotificationId(notificationViewData.type, status.id, notificationViewData.id)) { - Log.w(TAG, "Removing old notification at "+pos+" for "+status.id) + Log.d(TAG, "Removing old notification at "+pos+" for "+status.id+" at "+status.createdAt) adapter.notifyItemRemoved(pos) } } - - Log.w(TAG, "elements after second "+ adapter.snapshot().size) } override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt index 69fd9387e8..1ea697ecbe 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt @@ -507,7 +507,8 @@ class NotificationsViewModel @Inject constructor( } // Status id -> (highest) Notification id - val seenFavorites = HashMap() + private val seenFavorites = HashMap() + private val seenBoosts = HashMap() private fun getNotifications( filters: Set, @@ -520,10 +521,9 @@ class NotificationsViewModel @Inject constructor( ?: return@filter true return@filter if (hasNewestNotificationId(notification.type, status.id, notification.id)) { - seenFavorites[status.id] = notification.id // TODO move to hasNewestNotificationId? - true } else { + Log.d(TAG, "Filtering notification for "+status.id+" at "+status.createdAt) false } } @@ -550,13 +550,25 @@ class NotificationsViewModel @Inject constructor( } fun hasNewestNotificationId(type: Notification.Type, statusId: String, notificationId: String): Boolean { - if (type != Notification.Type.FAVOURITE) { - return true - } + val trackerArray = when(type) { + Notification.Type.FAVOURITE -> seenFavorites + Notification.Type.REBLOG -> seenBoosts + else -> null + } ?: return true + + val highestNotificationId = trackerArray[statusId] - val highestNotificationId = seenFavorites[statusId] + return if (highestNotificationId == null || isEqualOrNewer(notificationId, highestNotificationId)) { + trackerArray[statusId] = notificationId - return highestNotificationId == null || isEqualOrNewer(notificationId, highestNotificationId) + true + } else { + // TODO edge case: a newer favorite has been removed: the old notification will not be added again + // (because the removed id is still in the seen array) + // The code could find this out only heuristically: "looking at these notification ids (range), one in the array is not amongst them" + + false + } } /** From ff6f991fd701729f6993c27e4a37cf5138603189 Mon Sep 17 00:00:00 2001 From: Lakoja Date: Mon, 17 Apr 2023 10:02:58 +0200 Subject: [PATCH 4/7] 818: Add another todo comment about tracker id refresh (edge case bug) --- .../components/notifications/NotificationsPagingSource.kt | 5 +++++ .../tusky/components/notifications/NotificationsViewModel.kt | 1 + 2 files changed, 6 insertions(+) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt index b754989d06..9493ff8ff8 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt @@ -93,6 +93,11 @@ class NotificationsPagingSource @Inject constructor( return LoadResult.Error(Throwable("HTTP $code: $msg")) } + // TODO this is the only location with the "raw" data for answering the TODO in NotificationsViewModel.hasNewestNotificationId()? + // However the heuristic there would need (at least) the full range of notification ids here (so multiple calls to this method)? + val apiNotificationIds = response.body()!!.map { it.id } + Log.w(TAG, "Got notifications from api "+apiNotificationIds.first()+"-"+apiNotificationIds.last()) + val links = Links.from(response.headers()["link"]) return LoadResult.Page( data = response.body()!!, diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt index 1ea697ecbe..43fcad9c8e 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt @@ -387,6 +387,7 @@ class NotificationsViewModel @Inject constructor( ) viewModelScope.launch { + // TODO small: With this starting with filterIsInstance from a flow I cannot handle any other events? eventHub.events .filterIsInstance() .filter { StatusDisplayOptions.prefKeys.contains(it.preferenceKey) } From 6c794f39247127a4426007af7ec61058df68f9f6 Mon Sep 17 00:00:00 2001 From: Lakoja Date: Mon, 17 Apr 2023 11:05:29 +0200 Subject: [PATCH 5/7] 818: Add another bug todo --- .../components/notifications/NotificationsViewModel.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt index 43fcad9c8e..f47d7e4ae5 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt @@ -515,16 +515,22 @@ class NotificationsViewModel @Inject constructor( filters: Set, initialKey: String? = null ): Flow> { + var n = 0 + return repository.getNotificationsStream(filter = filters, initialKey = initialKey) .map { pagingData -> pagingData.filter { notification -> val status = notification.status ?: return@filter true + n += 1 + return@filter if (hasNewestNotificationId(notification.type, status.id, notification.id)) { true } else { - Log.d(TAG, "Filtering notification for "+status.id+" at "+status.createdAt) + // TODO this now leads sometimes (?) to a "List is empty" being displayed as first item. + + Log.d(TAG, "Filtering notification "+(n-1)+" for "+status.id+" at "+status.createdAt) false } } From 7994c6353052429c18a264d7307f313ca3b1a56e Mon Sep 17 00:00:00 2001 From: Lakoja Date: Wed, 5 Jul 2023 10:36:50 +0200 Subject: [PATCH 6/7] 818: Use proper id comparison --- .../notifications/NotificationsViewModel.kt | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt index f47d7e4ae5..6c71e3b331 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt @@ -41,6 +41,7 @@ import com.keylesspalace.tusky.settings.PrefKeys import com.keylesspalace.tusky.usecase.TimelineCases import com.keylesspalace.tusky.util.StatusDisplayOptions import com.keylesspalace.tusky.util.deserialize +import com.keylesspalace.tusky.util.isLessThan import com.keylesspalace.tusky.util.serialize import com.keylesspalace.tusky.util.throttleFirst import com.keylesspalace.tusky.util.toViewData @@ -565,7 +566,7 @@ class NotificationsViewModel @Inject constructor( val highestNotificationId = trackerArray[statusId] - return if (highestNotificationId == null || isEqualOrNewer(notificationId, highestNotificationId)) { + return if (highestNotificationId == null || highestNotificationId.isLessThan(notificationId)) { trackerArray[statusId] = notificationId true @@ -578,19 +579,6 @@ class NotificationsViewModel @Inject constructor( } } - /** - * NOTE this currently assumes that the ids are integers. Can that change? If it does all notifications are unequal. - */ - fun isEqualOrNewer(thisId: String, idToCompare: String): Boolean { - try { - return thisId.toInt() >= idToCompare.toInt() - } catch (exc: NumberFormatException) { - Log.e(TAG, "Cannot compare ids; not numbers: "+thisId+"/"+idToCompare) - - return false - } - } - /** * @return Flow of relevant preferences that change the UI */ From 860e7d989b1eea9b569d5ede54e40aff6d95a623 Mon Sep 17 00:00:00 2001 From: Lakoja Date: Wed, 5 Jul 2023 17:29:53 +0200 Subject: [PATCH 7/7] 818: Avoid "list is empty" (due to a crash in load()) --- .../components/notifications/NotificationsPagingSource.kt | 7 ++++--- .../components/notifications/NotificationsViewModel.kt | 8 +++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt index 9493ff8ff8..0da1dcb22d 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsPagingSource.kt @@ -95,12 +95,13 @@ class NotificationsPagingSource @Inject constructor( // TODO this is the only location with the "raw" data for answering the TODO in NotificationsViewModel.hasNewestNotificationId()? // However the heuristic there would need (at least) the full range of notification ids here (so multiple calls to this method)? - val apiNotificationIds = response.body()!!.map { it.id } - Log.w(TAG, "Got notifications from api "+apiNotificationIds.first()+"-"+apiNotificationIds.last()) + val notificationList = response.body()!! + val apiNotificationIds = notificationList.map { it.id } + Log.w(TAG, (if (params is LoadParams.Refresh) "R" else "A/P") +" Got notifications from api "+apiNotificationIds.firstOrNull()+"-"+apiNotificationIds.lastOrNull()) val links = Links.from(response.headers()["link"]) return LoadResult.Page( - data = response.body()!!, + data = notificationList, nextKey = links.next, prevKey = links.prev ) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt index 6c71e3b331..066e9565ef 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt @@ -41,7 +41,7 @@ import com.keylesspalace.tusky.settings.PrefKeys import com.keylesspalace.tusky.usecase.TimelineCases import com.keylesspalace.tusky.util.StatusDisplayOptions import com.keylesspalace.tusky.util.deserialize -import com.keylesspalace.tusky.util.isLessThan +import com.keylesspalace.tusky.util.isLessThanOrEqual import com.keylesspalace.tusky.util.serialize import com.keylesspalace.tusky.util.throttleFirst import com.keylesspalace.tusky.util.toViewData @@ -529,9 +529,7 @@ class NotificationsViewModel @Inject constructor( return@filter if (hasNewestNotificationId(notification.type, status.id, notification.id)) { true } else { - // TODO this now leads sometimes (?) to a "List is empty" being displayed as first item. - - Log.d(TAG, "Filtering notification "+(n-1)+" for "+status.id+" at "+status.createdAt) + Log.d(TAG, "Filtering notification "+(n-1)+" for "+status.id+"/"+notification.id+" at "+status.createdAt) false } } @@ -566,7 +564,7 @@ class NotificationsViewModel @Inject constructor( val highestNotificationId = trackerArray[statusId] - return if (highestNotificationId == null || highestNotificationId.isLessThan(notificationId)) { + return if (highestNotificationId == null || highestNotificationId.isLessThanOrEqual(notificationId)) { trackerArray[statusId] = notificationId true