Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Mark all as read in notifications #20649

Merged
merged 4 commits into from
Apr 17, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions WordPress/src/main/java/org/wordpress/android/models/Note.kt
Original file line number Diff line number Diff line change
@@ -224,9 +224,13 @@ class Note {
* Setters
*/

fun setRead() {
fun setRead() = updateReadState(1)

fun setUnread() = updateReadState(0)

private fun updateReadState(read: Int) {
try {
mNoteJSON?.putOpt("read", 1)
mNoteJSON?.putOpt("read", read)
} catch (e: JSONException) {
AppLog.e(AppLog.T.NOTIFS, "Failed to set 'read' property", e)
}
Original file line number Diff line number Diff line change
@@ -6,6 +6,8 @@ import androidx.lifecycle.MutableLiveData
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.withContext
import org.wordpress.android.R
import org.wordpress.android.datasets.wrappers.NotificationsTableWrapper
import org.wordpress.android.datasets.wrappers.ReaderPostTableWrapper
import org.wordpress.android.fluxc.model.SiteModel
@@ -16,6 +18,7 @@ import org.wordpress.android.fluxc.utils.AppLogWrapper
import org.wordpress.android.models.Note
import org.wordpress.android.models.Notification.PostLike
import org.wordpress.android.modules.BG_THREAD
import org.wordpress.android.modules.UI_THREAD
import org.wordpress.android.push.GCMMessageHandler
import org.wordpress.android.ui.jetpackoverlay.JetpackFeatureRemovalOverlayUtil
import org.wordpress.android.ui.jetpackoverlay.JetpackOverlayConnectedFeature.NOTIFICATIONS
@@ -28,6 +31,8 @@ import org.wordpress.android.ui.reader.actions.ReaderActions
import org.wordpress.android.ui.reader.actions.ReaderPostActionsWrapper
import org.wordpress.android.util.AppLog
import org.wordpress.android.util.EventBusWrapper
import org.wordpress.android.util.NetworkUtilsWrapper
import org.wordpress.android.util.ToastUtilsWrapper
import org.wordpress.android.viewmodel.Event
import org.wordpress.android.viewmodel.ScopedViewModel
import javax.inject.Inject
@@ -36,9 +41,12 @@ import javax.inject.Named
@HiltViewModel
class NotificationsListViewModel @Inject constructor(
@Named(BG_THREAD) bgDispatcher: CoroutineDispatcher,
@Named(UI_THREAD) private val mainDispatcher: CoroutineDispatcher,
private val appPrefsWrapper: AppPrefsWrapper,
private val jetpackFeatureRemovalOverlayUtil: JetpackFeatureRemovalOverlayUtil,
private val gcmMessageHandler: GCMMessageHandler,
private val networkUtilsWrapper: NetworkUtilsWrapper,
private val toastUtilsWrapper: ToastUtilsWrapper,
private val notificationsUtilsWrapper: NotificationsUtilsWrapper,
private val appLogWrapper: AppLogWrapper,
private val siteStore: SiteStore,
@@ -81,16 +89,34 @@ class NotificationsListViewModel @Inject constructor(
appPrefsWrapper.notificationPermissionsWarningDismissed = false
}

fun markNoteAsRead(context: Context, notes: List<Note>) {
fun markNoteAsRead(context: Context, notes: List<Note>) = launch {
if (networkUtilsWrapper.isNetworkAvailable().not()) {
withContext(mainDispatcher) {
toastUtilsWrapper.showToast(R.string.error_network_connection)
}
return@launch
}
notes.filter { it.isUnread }
.map {
gcmMessageHandler.removeNotificationWithNoteIdFromSystemBar(context, it.id)
notificationsActionsWrapper.markNoteAsRead(it)
it.setRead()
it
}.takeIf { it.isNotEmpty() }?.let {
notificationsTableWrapper.saveNotes(it, false)
it.apply { setRead() }
}.takeIf { it.isNotEmpty() }?.let { notes ->
// update the UI before the API request
notificationsTableWrapper.saveNotes(notes, false)
eventBusWrapper.post(NotificationsChanged())
// mark notes as read, this might wait for a long time
notificationsActionsWrapper.markNoteAsRead(notes)?.let { result ->
antonis marked this conversation as resolved.
Show resolved Hide resolved
if (result.isError) {
appLogWrapper.e(AppLog.T.NOTIFS, "Failed to mark notes as read: ${result.error}")
// revert the UI changes and display the error message
val revertedNotes = notes.map { it.apply { setUnread() } }
notificationsTableWrapper.saveNotes(revertedNotes, false)
eventBusWrapper.post(NotificationsChanged())
withContext(mainDispatcher) {
toastUtilsWrapper.showToast(R.string.error_generic)
}
}
}
}
}

Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
package org.wordpress.android.ui.notifications.utils

import dagger.Reusable
import org.wordpress.android.fluxc.model.notification.NotificationModel
import org.wordpress.android.fluxc.store.NotificationStore
import org.wordpress.android.models.Note
import org.wordpress.android.util.AppLog
import javax.inject.Inject
import kotlin.coroutines.resume
import kotlin.coroutines.suspendCoroutine

@Reusable
class NotificationsActionsWrapper @Inject constructor() {
class NotificationsActionsWrapper @Inject constructor(
private val notificationStore: NotificationStore
) {
suspend fun downloadNoteAndUpdateDB(noteId: String): Boolean =
suspendCoroutine { continuation ->
NotificationsActions.downloadNoteAndUpdateDB(
@@ -16,7 +21,21 @@ class NotificationsActionsWrapper @Inject constructor() {
{ continuation.resume(true) })
}

fun markNoteAsRead(note: Note?) {
NotificationsActions.markNoteAsRead(note)
@Suppress("TooGenericExceptionCaught")
suspend fun markNoteAsRead(notes: List<Note>): NotificationStore.OnNotificationChanged? {
val noteIds = notes.map {
try {
it.id.toLong()
} catch (ex: Exception) {
// id might be empty
AppLog.e(AppLog.T.NOTIFS, "Error parsing note id: ${it.id}", ex)
-1L
}
}.filter { it != -1L }
if (noteIds.isEmpty()) return null

return notificationStore.markNotificationsRead(
NotificationStore.MarkNotificationsReadPayload(noteIds.map { NotificationModel(remoteNoteId = it) })
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.wordpress.android.util

import androidx.annotation.StringRes
import dagger.Reusable
import org.wordpress.android.WordPress
import javax.inject.Inject

@Reusable
class ToastUtilsWrapper @Inject constructor() {
fun showToast(@StringRes messageRes: Int) =
ToastUtils.showToast(WordPress.getContext(), messageRes)
}
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.AccountStore
import org.wordpress.android.fluxc.store.CommentStore
import org.wordpress.android.fluxc.store.CommentsStore
import org.wordpress.android.fluxc.store.NotificationStore
import org.wordpress.android.fluxc.store.SiteStore
import org.wordpress.android.fluxc.utils.AppLogWrapper
import org.wordpress.android.models.Note
@@ -33,6 +34,8 @@ import org.wordpress.android.ui.prefs.AppPrefsWrapper
import org.wordpress.android.ui.reader.actions.ReaderActions
import org.wordpress.android.ui.reader.actions.ReaderPostActionsWrapper
import org.wordpress.android.util.EventBusWrapper
import org.wordpress.android.util.NetworkUtilsWrapper
import org.wordpress.android.util.ToastUtilsWrapper

private const val REQUEST_BLOG_LISTENER_PARAM_POSITION = 2

@@ -76,7 +79,13 @@ class NotificationsListViewModelTest : BaseUnitTest() {
private lateinit var commentStore: CommentsStore

@Mock
private lateinit var accountStore: AccountStore
private lateinit var accountStore: AccountStore

@Mock
private lateinit var toastUtilsWrapper: ToastUtilsWrapper

@Mock
private lateinit var networkUtilsWrapper: NetworkUtilsWrapper

@Mock
private lateinit var action: ActionHandler
@@ -86,10 +95,13 @@ class NotificationsListViewModelTest : BaseUnitTest() {
@Before
fun setup() {
viewModel = NotificationsListViewModel(
testDispatcher(),
testDispatcher(),
appPrefsWrapper,
jetpackFeatureRemovalOverlayUtil,
gcmMessageHandler,
networkUtilsWrapper,
toastUtilsWrapper,
notificationsUtilsWrapper,
appLogWrapper,
siteStore,
@@ -104,33 +116,37 @@ class NotificationsListViewModelTest : BaseUnitTest() {
}

@Test
fun `WHEN marking a note as read THEN the note is marked as read and the notification removed from system bar`() {
// Given
val noteId = "1"
val context: Context = mock()
val note = mock<Note>()
whenever(note.id).thenReturn(noteId)
whenever(note.isUnread).thenReturn(true)

// When
viewModel.markNoteAsRead(context, listOf(note))

// Then
verify(gcmMessageHandler, times(1)).removeNotificationWithNoteIdFromSystemBar(context, noteId)
verify(notificationsActionsWrapper, times(1)).markNoteAsRead(note)
verify(note, times(1)).setRead()
verify(notificationsTableWrapper, times(1)).saveNotes(listOf(note), false)
verify(eventBusWrapper, times(1)).post(any())
}
fun `WHEN marking a note as read THEN the note is marked as read and the notification removed from system bar`() =
test {
// Given
val noteId = "1"
val context: Context = mock()
val note = mock<Note>()
val notes = listOf(note)
whenever(note.id).thenReturn(noteId)
whenever(note.isUnread).thenReturn(true)
whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(true)

// When
viewModel.markNoteAsRead(context, notes)

// Then
verify(gcmMessageHandler, times(1)).removeNotificationWithNoteIdFromSystemBar(context, noteId)
verify(note, times(1)).setRead()
verify(notificationsActionsWrapper).markNoteAsRead(notes)
verify(notificationsTableWrapper, times(1)).saveNotes(notes, false)
verify(eventBusWrapper, times(1)).post(any())
}

@Test
fun `WHEN marking a note as read THEN the read note is saved`() {
fun `WHEN marking a note as read THEN the read note is saved`() = test {
// Given
val noteId = "1"
val context: Context = mock()
val note = mock<Note>()
whenever(note.id).thenReturn(noteId)
whenever(note.isUnread).thenReturn(true)
whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(true)

// When
viewModel.markNoteAsRead(context, listOf(note))
@@ -141,7 +157,7 @@ class NotificationsListViewModelTest : BaseUnitTest() {
}

@Test
fun `WHEN marking all as read THEN only the unread notes are marked as read and saved`() {
fun `WHEN marking all as read THEN only the unread notes are marked as read and saved`() = test {
// Given
val noteId1 = "1"
val noteId2 = "2"
@@ -151,19 +167,57 @@ class NotificationsListViewModelTest : BaseUnitTest() {
whenever(note1.id).thenReturn(noteId1)
whenever(note1.isUnread).thenReturn(true)
whenever(note2.isUnread).thenReturn(false)
whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(true)

// When
viewModel.markNoteAsRead(context, listOf(note1, note2))

// Then
verify(gcmMessageHandler, times(1)).removeNotificationWithNoteIdFromSystemBar(context, noteId1)
verify(notificationsActionsWrapper, times(1)).markNoteAsRead(note1)
verify(note1, times(1)).setRead()
verify(gcmMessageHandler, times(0)).removeNotificationWithNoteIdFromSystemBar(context, noteId2)
verify(notificationsActionsWrapper, times(0)).markNoteAsRead(note2)
verify(note2, times(0)).setRead()
verify(notificationsTableWrapper, times(1)).saveNotes(listOf(note1), false)
verify(eventBusWrapper, times(1)).post(any())
verify(notificationsActionsWrapper, times(1)).markNoteAsRead(listOf(note1))
}

@Test
fun `GIVEN a interrupted network WHEN marking a note as read THEN show a network error toast`() = test {
// Given
val note = mock<Note>()
whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(false)

// When
viewModel.markNoteAsRead(mock(), listOf(note))

// Then
verify(toastUtilsWrapper, times(1)).showToast(any())
}

@Test
fun `GIVEN a stable network WHEN making a note as read fails THEN show a generic error toast`() = test {
// Given
val note = mock<Note>()
whenever(note.id).thenReturn("123")
whenever(note.isUnread).thenReturn(true)
whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(true)
whenever(notificationsActionsWrapper.markNoteAsRead(listOf(note))).thenReturn(
NotificationStore.OnNotificationChanged(1).apply {
error = NotificationStore.NotificationError(
NotificationStore.NotificationErrorType.GENERIC_ERROR, "error"
)
}
)

// When
viewModel.markNoteAsRead(mock(), listOf(note))

// Then
verify(gcmMessageHandler).removeNotificationWithNoteIdFromSystemBar(any(), eq("123"))
verify(notificationsTableWrapper, times(2)).saveNotes(any(), eq(false))
verify(eventBusWrapper, times(2)).post(any())
verify(toastUtilsWrapper, times(1)).showToast(any())
}

@Test
@@ -230,7 +284,8 @@ class NotificationsListViewModelTest : BaseUnitTest() {
whenever(note.commentId).thenReturn(commentId)
whenever(commentStore.likeComment(site, commentId, null, true)).thenReturn(
CommentsStore.CommentsActionPayload(
CommentStore.CommentError(CommentStore.CommentErrorType.GENERIC_ERROR,"error"), null)
CommentStore.CommentError(CommentStore.CommentErrorType.GENERIC_ERROR, "error"), null
)
)

// When