From 8c833773ce395dcf8045cd0412d7e1511476a971 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Wed, 14 Feb 2024 20:10:37 +0200 Subject: [PATCH 01/10] Open comments that cannot be moderated in the reader --- .../NotificationsListFragmentPage.kt | 67 ++++++++++++++++--- 1 file changed, 57 insertions(+), 10 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt index 1948ca3ccf90..e73975aba362 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt @@ -28,6 +28,7 @@ import org.wordpress.android.WordPress import org.wordpress.android.analytics.AnalyticsTracker.Stat.APP_REVIEWS_EVENT_INCREMENTED_BY_CHECKING_NOTIFICATION import org.wordpress.android.databinding.NotificationsListFragmentPageBinding import org.wordpress.android.datasets.NotificationsTable +import org.wordpress.android.datasets.ReaderPostTable import org.wordpress.android.fluxc.model.CommentStatus import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.AccountStore @@ -56,6 +57,10 @@ import org.wordpress.android.ui.notifications.adapters.NotesAdapter.DataLoadedLi import org.wordpress.android.ui.notifications.adapters.NotesAdapter.FILTERS import org.wordpress.android.ui.notifications.services.NotificationsUpdateServiceStarter import org.wordpress.android.ui.notifications.utils.NotificationsActions +import org.wordpress.android.ui.reader.ReaderActivityLauncher +import org.wordpress.android.ui.reader.actions.ReaderActions +import org.wordpress.android.ui.reader.actions.ReaderPostActions +import org.wordpress.android.ui.reader.comments.ThreadedCommentsActionSource import org.wordpress.android.util.AniUtils import org.wordpress.android.util.AppLog import org.wordpress.android.util.AppLog.T @@ -525,21 +530,63 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l if (noteId == null || activity == null || activity.isFinishing) { return } - val detailIntent = getOpenNoteIntent(activity, noteId) - detailIntent.putExtra(NotificationsListFragment.NOTE_INSTANT_REPLY_EXTRA, shouldShowKeyboard) - if (!TextUtils.isEmpty(replyText)) { - detailIntent.putExtra(NotificationsListFragment.NOTE_PREFILLED_REPLY_EXTRA, replyText) - } - detailIntent.putExtra(NotificationsListFragment.NOTE_CURRENT_LIST_FILTER_EXTRA, filter) - detailIntent.putExtra( - NotificationsUpdateServiceStarter.IS_TAPPED_ON_NOTIFICATION, - isTappedFromPushNotification + canOpenInTheReader(noteId, + openInTheReader = { siteId, postId, commentId -> + ReaderActivityLauncher.showReaderComments( + activity, + siteId, + postId, + commentId, + ThreadedCommentsActionSource.COMMENT_NOTIFICATION.sourceDescription + ) + }, + openDetailView = { + val detailIntent = getOpenNoteIntent(activity, noteId) + detailIntent.putExtra(NotificationsListFragment.NOTE_INSTANT_REPLY_EXTRA, shouldShowKeyboard) + if (!TextUtils.isEmpty(replyText)) { + detailIntent.putExtra(NotificationsListFragment.NOTE_PREFILLED_REPLY_EXTRA, replyText) + } + detailIntent.putExtra(NotificationsListFragment.NOTE_CURRENT_LIST_FILTER_EXTRA, filter) + detailIntent.putExtra( + NotificationsUpdateServiceStarter.IS_TAPPED_ON_NOTIFICATION, + isTappedFromPushNotification + ) + openNoteForReplyWithParams(detailIntent, activity) + } ) - openNoteForReplyWithParams(detailIntent, activity) } private fun openNoteForReplyWithParams(detailIntent: Intent, activity: Activity) { activity.startActivityForResult(detailIntent, RequestCodes.NOTE_DETAIL) } + + private fun canOpenInTheReader( + noteId: String, + openInTheReader: (siteId: Long, postId: Long, commentId: Long) -> Unit, + openDetailView: () -> Unit + ) { + val note = NotificationsTable.getNoteById(noteId) + if (note != null && note.isCommentType && !note.canModerate()) { + val readerPost = ReaderPostTable.getBlogPost(note.siteId.toLong(), note.postId.toLong(), false) + if (readerPost != null) { + openInTheReader(note.siteId.toLong(), note.postId.toLong(), note.commentId) + } else { + ReaderPostActions.requestBlogPost( + note.siteId.toLong(), + note.postId.toLong(), + object : ReaderActions.OnRequestListener { + override fun onSuccess(result: String?) { + openInTheReader(note.siteId.toLong(), note.postId.toLong(), note.commentId) + } + + override fun onFailure(statusCode: Int) { + openDetailView() + } + }) + } + } else { + openDetailView() + } + } } } From 2532ec27d05ca2782319cba05b836716cf3be1bc Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 15 Feb 2024 09:42:59 +0200 Subject: [PATCH 02/10] Moves logic in the viewModel --- .../NotificationsListFragmentPage.kt | 85 ++++++------------- .../NotificationsListViewModel.kt | 32 +++++++ 2 files changed, 59 insertions(+), 58 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt index e73975aba362..ce8656146638 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt @@ -28,7 +28,6 @@ import org.wordpress.android.WordPress import org.wordpress.android.analytics.AnalyticsTracker.Stat.APP_REVIEWS_EVENT_INCREMENTED_BY_CHECKING_NOTIFICATION import org.wordpress.android.databinding.NotificationsListFragmentPageBinding import org.wordpress.android.datasets.NotificationsTable -import org.wordpress.android.datasets.ReaderPostTable import org.wordpress.android.fluxc.model.CommentStatus import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.AccountStore @@ -58,8 +57,6 @@ import org.wordpress.android.ui.notifications.adapters.NotesAdapter.FILTERS import org.wordpress.android.ui.notifications.services.NotificationsUpdateServiceStarter import org.wordpress.android.ui.notifications.utils.NotificationsActions import org.wordpress.android.ui.reader.ReaderActivityLauncher -import org.wordpress.android.ui.reader.actions.ReaderActions -import org.wordpress.android.ui.reader.actions.ReaderPostActions import org.wordpress.android.ui.reader.comments.ThreadedCommentsActionSource import org.wordpress.android.util.AniUtils import org.wordpress.android.util.AppLog @@ -232,9 +229,23 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l } incrementInteractions(APP_REVIEWS_EVENT_INCREMENTED_BY_CHECKING_NOTIFICATION) - // Open the latest version of this note in case it has changed, which can happen if the note was tapped - // from the list after it was updated by another fragment (such as NotificationsDetailListFragment). - openNoteForReply(activity, noteId, false, null, notesAdapter!!.currentFilter, false) + viewModel.openNote( + noteId, + { siteId, postId, commentId -> + ReaderActivityLauncher.showReaderComments( + activity, + siteId, + postId, + commentId, + ThreadedCommentsActionSource.COMMENT_NOTIFICATION.sourceDescription + ) + }, + { + // Open the latest version of this note in case it has changed, which can happen if the note was tapped + // from the list after it was updated by another fragment (such as NotificationsDetailListFragment). + openNoteForReply(activity, noteId, false, null, notesAdapter!!.currentFilter, false) + } + ) } } private val mOnScrollListener: OnScrollListener = object : OnScrollListener() { @@ -530,63 +541,21 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l if (noteId == null || activity == null || activity.isFinishing) { return } - canOpenInTheReader(noteId, - openInTheReader = { siteId, postId, commentId -> - ReaderActivityLauncher.showReaderComments( - activity, - siteId, - postId, - commentId, - ThreadedCommentsActionSource.COMMENT_NOTIFICATION.sourceDescription - ) - }, - openDetailView = { - val detailIntent = getOpenNoteIntent(activity, noteId) - detailIntent.putExtra(NotificationsListFragment.NOTE_INSTANT_REPLY_EXTRA, shouldShowKeyboard) - if (!TextUtils.isEmpty(replyText)) { - detailIntent.putExtra(NotificationsListFragment.NOTE_PREFILLED_REPLY_EXTRA, replyText) - } - detailIntent.putExtra(NotificationsListFragment.NOTE_CURRENT_LIST_FILTER_EXTRA, filter) - detailIntent.putExtra( - NotificationsUpdateServiceStarter.IS_TAPPED_ON_NOTIFICATION, - isTappedFromPushNotification - ) - openNoteForReplyWithParams(detailIntent, activity) - } + val detailIntent = getOpenNoteIntent(activity, noteId) + detailIntent.putExtra(NotificationsListFragment.NOTE_INSTANT_REPLY_EXTRA, shouldShowKeyboard) + if (!TextUtils.isEmpty(replyText)) { + detailIntent.putExtra(NotificationsListFragment.NOTE_PREFILLED_REPLY_EXTRA, replyText) + } + detailIntent.putExtra(NotificationsListFragment.NOTE_CURRENT_LIST_FILTER_EXTRA, filter) + detailIntent.putExtra( + NotificationsUpdateServiceStarter.IS_TAPPED_ON_NOTIFICATION, + isTappedFromPushNotification ) + openNoteForReplyWithParams(detailIntent, activity) } private fun openNoteForReplyWithParams(detailIntent: Intent, activity: Activity) { activity.startActivityForResult(detailIntent, RequestCodes.NOTE_DETAIL) } - - private fun canOpenInTheReader( - noteId: String, - openInTheReader: (siteId: Long, postId: Long, commentId: Long) -> Unit, - openDetailView: () -> Unit - ) { - val note = NotificationsTable.getNoteById(noteId) - if (note != null && note.isCommentType && !note.canModerate()) { - val readerPost = ReaderPostTable.getBlogPost(note.siteId.toLong(), note.postId.toLong(), false) - if (readerPost != null) { - openInTheReader(note.siteId.toLong(), note.postId.toLong(), note.commentId) - } else { - ReaderPostActions.requestBlogPost( - note.siteId.toLong(), - note.postId.toLong(), - object : ReaderActions.OnRequestListener { - override fun onSuccess(result: String?) { - openInTheReader(note.siteId.toLong(), note.postId.toLong(), note.commentId) - } - - override fun onFailure(statusCode: Int) { - openDetailView() - } - }) - } - } else { - openDetailView() - } - } } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt index 6299411f80d2..9905d311e610 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt @@ -8,6 +8,7 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.MutableSharedFlow import org.greenrobot.eventbus.EventBus import org.wordpress.android.datasets.NotificationsTable +import org.wordpress.android.datasets.ReaderPostTable import org.wordpress.android.models.Note import org.wordpress.android.models.Notification.PostNotification import org.wordpress.android.modules.UI_THREAD @@ -17,6 +18,8 @@ import org.wordpress.android.ui.jetpackoverlay.JetpackOverlayConnectedFeature.NO import org.wordpress.android.ui.notifications.NotificationEvents.NotificationsChanged import org.wordpress.android.ui.notifications.utils.NotificationsActions import org.wordpress.android.ui.prefs.AppPrefsWrapper +import org.wordpress.android.ui.reader.actions.ReaderActions +import org.wordpress.android.ui.reader.actions.ReaderPostActions import org.wordpress.android.util.JetpackBrandingUtils import org.wordpress.android.viewmodel.Event import org.wordpress.android.viewmodel.ScopedViewModel @@ -81,6 +84,35 @@ class NotificationsListViewModel @Inject constructor( } } + fun openNote( + noteId: String?, + openInTheReader: (siteId: Long, postId: Long, commentId: Long) -> Unit, + openDetailView: () -> Unit + ) { + val note = NotificationsTable.getNoteById(noteId) + if (note != null && note.isCommentType && !note.canModerate()) { + val readerPost = ReaderPostTable.getBlogPost(note.siteId.toLong(), note.postId.toLong(), false) + if (readerPost != null) { + openInTheReader(note.siteId.toLong(), note.postId.toLong(), note.commentId) + } else { + ReaderPostActions.requestBlogPost( + note.siteId.toLong(), + note.postId.toLong(), + object : ReaderActions.OnRequestListener { + override fun onSuccess(result: String?) { + openInTheReader(note.siteId.toLong(), note.postId.toLong(), note.commentId) + } + + override fun onFailure(statusCode: Int) { + openDetailView() + } + }) + } + } else { + openDetailView() + } + } + sealed class InlineActionEvent { data class SharePostButtonTapped(val notification: PostNotification): InlineActionEvent() } From 4b0d3a603438d83eb74f0e6a72a7c1aec02a9303 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 15 Feb 2024 09:46:16 +0200 Subject: [PATCH 03/10] Optimise function parameter list --- .../NotificationsListFragmentPage.kt | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt index ce8656146638..adbdc25472b6 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListFragmentPage.kt @@ -241,9 +241,10 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l ) }, { - // Open the latest version of this note in case it has changed, which can happen if the note was tapped - // from the list after it was updated by another fragment (such as NotificationsDetailListFragment). - openNoteForReply(activity, noteId, false, null, notesAdapter!!.currentFilter, false) + // Open the latest version of this note in case it has changed, which can happen if the note was + // tapped from the list after it was updated by another fragment (such as the + // NotificationsDetailListFragment). + openNoteForReply(activity, noteId, filter = notesAdapter?.currentFilter) } ) } @@ -533,10 +534,10 @@ class NotificationsListFragmentPage : ViewPagerFragment(R.layout.notifications_l fun openNoteForReply( activity: Activity?, noteId: String?, - shouldShowKeyboard: Boolean, - replyText: String?, - filter: FILTERS?, - isTappedFromPushNotification: Boolean + shouldShowKeyboard: Boolean = false, + replyText: String? = null, + filter: FILTERS? = null, + isTappedFromPushNotification: Boolean = false, ) { if (noteId == null || activity == null || activity.isFinishing) { return From b75ec326718c39720b8446e321d5346950f25e78 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 15 Feb 2024 09:55:32 +0200 Subject: [PATCH 04/10] Replaces static function calls with wrapped objects for testability --- .../NotificationsListViewModel.kt | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt index 9905d311e610..f788559f1b42 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt @@ -8,7 +8,7 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.MutableSharedFlow import org.greenrobot.eventbus.EventBus import org.wordpress.android.datasets.NotificationsTable -import org.wordpress.android.datasets.ReaderPostTable +import org.wordpress.android.datasets.wrappers.ReaderPostTableWrapper import org.wordpress.android.models.Note import org.wordpress.android.models.Notification.PostNotification import org.wordpress.android.modules.UI_THREAD @@ -17,9 +17,10 @@ import org.wordpress.android.ui.jetpackoverlay.JetpackFeatureRemovalOverlayUtil import org.wordpress.android.ui.jetpackoverlay.JetpackOverlayConnectedFeature.NOTIFICATIONS import org.wordpress.android.ui.notifications.NotificationEvents.NotificationsChanged import org.wordpress.android.ui.notifications.utils.NotificationsActions +import org.wordpress.android.ui.notifications.utils.NotificationsUtilsWrapper import org.wordpress.android.ui.prefs.AppPrefsWrapper import org.wordpress.android.ui.reader.actions.ReaderActions -import org.wordpress.android.ui.reader.actions.ReaderPostActions +import org.wordpress.android.ui.reader.actions.ReaderPostActionsWrapper import org.wordpress.android.util.JetpackBrandingUtils import org.wordpress.android.viewmodel.Event import org.wordpress.android.viewmodel.ScopedViewModel @@ -32,9 +33,11 @@ class NotificationsListViewModel @Inject constructor( private val appPrefsWrapper: AppPrefsWrapper, private val jetpackBrandingUtils: JetpackBrandingUtils, private val jetpackFeatureRemovalOverlayUtil: JetpackFeatureRemovalOverlayUtil, - private val gcmMessageHandler: GCMMessageHandler - -) : ScopedViewModel(mainDispatcher) { + private val gcmMessageHandler: GCMMessageHandler, + private val notificationsUtilsWrapper: NotificationsUtilsWrapper, + private val readerPostTableWrapper: ReaderPostTableWrapper, + private val readerPostActionsWrapper: ReaderPostActionsWrapper, + ) : ScopedViewModel(mainDispatcher) { private val _showJetpackPoweredBottomSheet = MutableLiveData>() val showJetpackPoweredBottomSheet: LiveData> = _showJetpackPoweredBottomSheet @@ -89,13 +92,13 @@ class NotificationsListViewModel @Inject constructor( openInTheReader: (siteId: Long, postId: Long, commentId: Long) -> Unit, openDetailView: () -> Unit ) { - val note = NotificationsTable.getNoteById(noteId) + val note = noteId?.let { notificationsUtilsWrapper.getNoteById(noteId) } if (note != null && note.isCommentType && !note.canModerate()) { - val readerPost = ReaderPostTable.getBlogPost(note.siteId.toLong(), note.postId.toLong(), false) + val readerPost = readerPostTableWrapper.getBlogPost(note.siteId.toLong(), note.postId.toLong(), false) if (readerPost != null) { openInTheReader(note.siteId.toLong(), note.postId.toLong(), note.commentId) } else { - ReaderPostActions.requestBlogPost( + readerPostActionsWrapper.requestBlogPost( note.siteId.toLong(), note.postId.toLong(), object : ReaderActions.OnRequestListener { From d29cf501fa6cbc4440ddbc88cf424fc4cee4b995 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 15 Feb 2024 12:05:41 +0200 Subject: [PATCH 05/10] Adds test cases --- .../NotificationsListViewModelTest.kt | 206 ++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt diff --git a/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt new file mode 100644 index 000000000000..8ea75b4dd1f4 --- /dev/null +++ b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt @@ -0,0 +1,206 @@ +package org.wordpress.android.ui.notifications + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.junit.MockitoJUnitRunner +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.BaseUnitTest +import org.wordpress.android.datasets.wrappers.ReaderPostTableWrapper +import org.wordpress.android.models.Note +import org.wordpress.android.models.ReaderPost +import org.wordpress.android.push.GCMMessageHandler +import org.wordpress.android.ui.jetpackoverlay.JetpackFeatureRemovalOverlayUtil +import org.wordpress.android.ui.notifications.utils.NotificationsUtilsWrapper +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.JetpackBrandingUtils + +private const val REQUEST_BLOG_LISTENER_PARAM_POSITION = 2 + +@ExperimentalCoroutinesApi +@RunWith(MockitoJUnitRunner::class) +class NotificationsListViewModelTest : BaseUnitTest() { + @Mock + private lateinit var appPrefsWrapper: AppPrefsWrapper + + @Mock + private lateinit var jetpackBrandingUtils: JetpackBrandingUtils + + @Mock + private lateinit var jetpackFeatureRemovalOverlayUtil: JetpackFeatureRemovalOverlayUtil + + @Mock + private lateinit var gcmMessageHandler: GCMMessageHandler + + @Mock + private lateinit var notificationsUtilsWrapper: NotificationsUtilsWrapper + + @Mock + private lateinit var readerPostTableWrapper: ReaderPostTableWrapper + + @Mock + private lateinit var readerPostActionsWrapper: ReaderPostActionsWrapper + + @Mock + private lateinit var action: ActionHandler + + private lateinit var viewModel: NotificationsListViewModel + + @Before + fun setup() { + viewModel = NotificationsListViewModel( + testDispatcher(), + appPrefsWrapper, + jetpackBrandingUtils, + jetpackFeatureRemovalOverlayUtil, + gcmMessageHandler, + notificationsUtilsWrapper, + readerPostTableWrapper, + readerPostActionsWrapper + ) + } + + @Test + fun `WHEN the note cannot be retrieved THEN try opening the detail view`() { + // Given + val noteId = "1" + whenever(notificationsUtilsWrapper.getNoteById(noteId)).thenReturn(null) + + // When + viewModel.openNote(noteId, action::openInTheReader, action::openDetailView) + + // Then + verify(action, times(0)).openInTheReader(any(), any(), any()) + verify(action, times(1)).openDetailView() + } + + @Test + fun `WHEN the note is not a comment THEN open detail view`() { + // Given + val noteId = "1" + val note = mock() + whenever(notificationsUtilsWrapper.getNoteById(noteId)).thenReturn(note) + whenever(note.isCommentType).thenReturn(false) + + // When + viewModel.openNote(noteId, action::openInTheReader, action::openDetailView) + + // Then + verify(action, times(0)).openInTheReader(any(), any(), any()) + verify(action, times(1)).openDetailView() + } + + @Test + fun `WHEN the note is a comment that can be moderated THEN open detail view`() { + // Given + val noteId = "1" + val note = mock() + whenever(notificationsUtilsWrapper.getNoteById(noteId)).thenReturn(note) + whenever(note.isCommentType).thenReturn(true) + whenever(note.canModerate()).thenReturn(true) + + // When + viewModel.openNote(noteId, action::openInTheReader, action::openDetailView) + + // Then + verify(action, times(0)).openInTheReader(any(), any(), any()) + verify(action, times(1)).openDetailView() + } + + @Test + fun `WHEN the note is a comment that cannot be moderated and the reader post exists THEN open in reader`() { + // Given + val noteId = "1" + val siteId = 1L + val postId = 2L + val commentId = 3L + val note = mock() + val readerPost = mock() + whenever(notificationsUtilsWrapper.getNoteById(noteId)).thenReturn(note) + whenever(note.siteId).thenReturn(siteId.toInt()) + whenever(note.postId).thenReturn(postId.toInt()) + whenever(note.commentId).thenReturn(commentId) + whenever(note.isCommentType).thenReturn(true) + whenever(note.canModerate()).thenReturn(false) + whenever(readerPostTableWrapper.getBlogPost(siteId, postId, false)).thenReturn(readerPost) + + // When + viewModel.openNote(noteId, action::openInTheReader, action::openDetailView) + + // Then + verify(action, times(1)).openInTheReader(siteId, postId, commentId) + verify(action, times(0)).openDetailView() + } + + @Test + fun `WHEN the note is a comment that cannot be moderated and the reader post is retrieved THEN open in reader`() { + // Given + val noteId = "1" + val siteId = 1L + val postId = 2L + val commentId = 3L + val note = mock() + whenever(notificationsUtilsWrapper.getNoteById(noteId)).thenReturn(note) + whenever(note.siteId).thenReturn(siteId.toInt()) + whenever(note.postId).thenReturn(postId.toInt()) + whenever(note.commentId).thenReturn(commentId) + whenever(note.isCommentType).thenReturn(true) + whenever(note.canModerate()).thenReturn(false) + whenever(readerPostTableWrapper.getBlogPost(siteId, postId, false)).thenReturn(null) + whenever(readerPostActionsWrapper.requestBlogPost(any(), any(), any())).then { + (it.arguments[REQUEST_BLOG_LISTENER_PARAM_POSITION] as ReaderActions.OnRequestListener<*>) + .onSuccess(null) + } + + // When + viewModel.openNote(noteId, action::openInTheReader, action::openDetailView) + + // Then + verify(action, times(1)).openInTheReader(siteId, postId, commentId) + verify(action, times(0)).openDetailView() + } + + @Test + fun `WHEN the comment note cannot be moderated and the reader post retrieval fails THEN open detail view`() { + // Given + val noteId = "1" + val siteId = 1L + val postId = 2L + val note = mock() + whenever(notificationsUtilsWrapper.getNoteById(noteId)).thenReturn(note) + whenever(note.siteId).thenReturn(siteId.toInt()) + whenever(note.postId).thenReturn(postId.toInt()) + whenever(note.isCommentType).thenReturn(true) + whenever(note.canModerate()).thenReturn(false) + whenever(readerPostTableWrapper.getBlogPost(siteId, postId, false)).thenReturn(null) + whenever(readerPostActionsWrapper.requestBlogPost(any(), any(), any())).then { + (it.arguments[REQUEST_BLOG_LISTENER_PARAM_POSITION] as ReaderActions.OnRequestListener<*>) + .onFailure(500) + } + + // When + viewModel.openNote(noteId, action::openInTheReader, action::openDetailView) + + // Then + verify(action, times(0)).openInTheReader(any(), any(), any()) + verify(action, times(1)).openDetailView() + } + + private class ActionHandler { + fun openInTheReader(siteId: Long, postId: Long, commentId: Long) { + println("openInTheReader($siteId, $postId, $commentId)") + } + + fun openDetailView() { + println("openDetailView") + } + } +} From 47720af7055d4bd90a15f7d37646242415533bab Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 15 Feb 2024 14:15:23 +0200 Subject: [PATCH 06/10] Changed mocked action handler to interface to avoid unneeded code --- .../ui/notifications/NotificationsListViewModelTest.kt | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt index 8ea75b4dd1f4..395d1aa11171 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt @@ -194,13 +194,9 @@ class NotificationsListViewModelTest : BaseUnitTest() { verify(action, times(1)).openDetailView() } - private class ActionHandler { - fun openInTheReader(siteId: Long, postId: Long, commentId: Long) { - println("openInTheReader($siteId, $postId, $commentId)") - } + interface ActionHandler { + fun openInTheReader(siteId: Long, postId: Long, commentId: Long) - fun openDetailView() { - println("openDetailView") - } + fun openDetailView() } } From 0c3601e7590c868721b922a318ca150dda5bc936 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 15 Feb 2024 16:02:08 +0200 Subject: [PATCH 07/10] Fixes broken tests after merge from trunk and feature branch --- .../NotificationsListViewModelTest.kt | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt index 395d1aa11171..5dfea78e7809 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt @@ -13,6 +13,8 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.BaseUnitTest import org.wordpress.android.datasets.wrappers.ReaderPostTableWrapper +import org.wordpress.android.fluxc.store.CommentsStore +import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.models.Note import org.wordpress.android.models.ReaderPost import org.wordpress.android.push.GCMMessageHandler @@ -21,7 +23,6 @@ import org.wordpress.android.ui.notifications.utils.NotificationsUtilsWrapper 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.JetpackBrandingUtils private const val REQUEST_BLOG_LISTENER_PARAM_POSITION = 2 @@ -31,9 +32,6 @@ class NotificationsListViewModelTest : BaseUnitTest() { @Mock private lateinit var appPrefsWrapper: AppPrefsWrapper - @Mock - private lateinit var jetpackBrandingUtils: JetpackBrandingUtils - @Mock private lateinit var jetpackFeatureRemovalOverlayUtil: JetpackFeatureRemovalOverlayUtil @@ -49,6 +47,12 @@ class NotificationsListViewModelTest : BaseUnitTest() { @Mock private lateinit var readerPostActionsWrapper: ReaderPostActionsWrapper + @Mock + private lateinit var siteStore: SiteStore + + @Mock + private lateinit var commentStore: CommentsStore + @Mock private lateinit var action: ActionHandler @@ -59,12 +63,13 @@ class NotificationsListViewModelTest : BaseUnitTest() { viewModel = NotificationsListViewModel( testDispatcher(), appPrefsWrapper, - jetpackBrandingUtils, jetpackFeatureRemovalOverlayUtil, gcmMessageHandler, notificationsUtilsWrapper, readerPostTableWrapper, - readerPostActionsWrapper + readerPostActionsWrapper, + siteStore, + commentStore ) } From b2d02dd9b18174e0f756f56865455611cda9ebe2 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 16 Feb 2024 08:39:05 +0200 Subject: [PATCH 08/10] Adds log in the case of post retrieval failure --- .../android/ui/notifications/NotificationsListViewModel.kt | 4 ++++ .../ui/notifications/NotificationsListViewModelTest.kt | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt index 8cab457aeceb..0bb1c0757f5e 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/notifications/NotificationsListViewModel.kt @@ -12,6 +12,7 @@ import org.wordpress.android.datasets.wrappers.ReaderPostTableWrapper import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.CommentsStore import org.wordpress.android.fluxc.store.SiteStore +import org.wordpress.android.fluxc.utils.AppLogWrapper import org.wordpress.android.models.Note import org.wordpress.android.models.Notification.PostNotification import org.wordpress.android.modules.BG_THREAD @@ -24,6 +25,7 @@ import org.wordpress.android.ui.notifications.utils.NotificationsUtilsWrapper 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.AppLog import org.wordpress.android.viewmodel.Event import org.wordpress.android.viewmodel.ScopedViewModel import javax.inject.Inject @@ -38,6 +40,7 @@ class NotificationsListViewModel @Inject constructor( private val notificationsUtilsWrapper: NotificationsUtilsWrapper, private val readerPostTableWrapper: ReaderPostTableWrapper, private val readerPostActionsWrapper: ReaderPostActionsWrapper, + private val appLogWrapper: AppLogWrapper, private val siteStore: SiteStore, private val commentStore: CommentsStore ) : ScopedViewModel(bgDispatcher) { @@ -118,6 +121,7 @@ class NotificationsListViewModel @Inject constructor( } override fun onFailure(statusCode: Int) { + appLogWrapper.w(AppLog.T.NOTIFS, "Failed to fetch post for comment: $statusCode") openDetailView() } }) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt index 5dfea78e7809..10894a678a84 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt @@ -15,6 +15,7 @@ import org.wordpress.android.BaseUnitTest import org.wordpress.android.datasets.wrappers.ReaderPostTableWrapper import org.wordpress.android.fluxc.store.CommentsStore import org.wordpress.android.fluxc.store.SiteStore +import org.wordpress.android.fluxc.utils.AppLogWrapper import org.wordpress.android.models.Note import org.wordpress.android.models.ReaderPost import org.wordpress.android.push.GCMMessageHandler @@ -47,6 +48,8 @@ class NotificationsListViewModelTest : BaseUnitTest() { @Mock private lateinit var readerPostActionsWrapper: ReaderPostActionsWrapper + @Mock lateinit var appLogWrapper: AppLogWrapper + @Mock private lateinit var siteStore: SiteStore @@ -68,6 +71,7 @@ class NotificationsListViewModelTest : BaseUnitTest() { notificationsUtilsWrapper, readerPostTableWrapper, readerPostActionsWrapper, + appLogWrapper, siteStore, commentStore ) From d089b0511ebc95c02a4d7241df8c89f415eeeec3 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 16 Feb 2024 10:08:27 +0200 Subject: [PATCH 09/10] Opens push notification comment that cannot be moderated directly in the reader --- .../android/ui/main/WPMainActivity.java | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java index 6d67608adebb..3d432ded823f 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/main/WPMainActivity.java @@ -108,6 +108,7 @@ import org.wordpress.android.ui.mysite.cards.quickstart.QuickStartRepository; import org.wordpress.android.ui.notifications.NotificationEvents; import org.wordpress.android.ui.notifications.NotificationsListFragment; +import org.wordpress.android.ui.notifications.NotificationsListViewModel; import org.wordpress.android.ui.notifications.SystemNotificationsTracker; import org.wordpress.android.ui.notifications.adapters.NotesAdapter; import org.wordpress.android.ui.notifications.receivers.NotificationsPendingDraftsReceiver; @@ -127,7 +128,9 @@ import org.wordpress.android.ui.prefs.privacy.banner.PrivacyBannerFragment; import org.wordpress.android.ui.quickstart.QuickStartMySitePrompts; import org.wordpress.android.ui.quickstart.QuickStartTracker; +import org.wordpress.android.ui.reader.ReaderActivityLauncher; import org.wordpress.android.ui.reader.ReaderFragment; +import org.wordpress.android.ui.reader.comments.ThreadedCommentsActionSource; import org.wordpress.android.ui.reader.services.update.ReaderUpdateLogic.UpdateTask; import org.wordpress.android.ui.reader.services.update.ReaderUpdateServiceStarter; import org.wordpress.android.ui.reader.tracker.ReaderTracker; @@ -190,6 +193,9 @@ import static org.wordpress.android.util.extensions.InAppReviewExtensionsKt.logException; import dagger.hilt.android.AndroidEntryPoint; +import kotlin.Unit; +import kotlin.jvm.functions.Function0; +import kotlin.jvm.functions.Function3; /** * Main activity which hosts sites, reader, me and notifications pages @@ -251,6 +257,7 @@ public class WPMainActivity extends LocaleAwareActivity implements private ModalLayoutPickerViewModel mMLPViewModel; @NonNull private ReviewViewModel mReviewViewModel; private BloggingRemindersViewModel mBloggingRemindersViewModel; + private NotificationsListViewModel mNotificationsViewModel; private FloatingActionButton mFloatingActionButton; private static final String MAIN_BOTTOM_SHEET_TAG = "MAIN_BOTTOM_SHEET_TAG"; private static final String BLOGGING_REMINDERS_BOTTOM_SHEET_TAG = "BLOGGING_REMINDERS_BOTTOM_SHEET_TAG"; @@ -1057,11 +1064,34 @@ public void onTokenInvalid() { // we processed the voice reply, so we exit this function immediately return; } else { - boolean shouldShowKeyboard = - getIntent().getBooleanExtra(NotificationsListFragment.NOTE_INSTANT_REPLY_EXTRA, false); - NotificationsListFragment - .openNoteForReply(this, noteId, shouldShowKeyboard, null, - NotesAdapter.FILTERS.FILTER_ALL, true); + if (mNotificationsViewModel == null) { + mNotificationsViewModel = new ViewModelProvider(this).get(NotificationsListViewModel.class); + } + mNotificationsViewModel.openNote(noteId, new Function3() { + @Nullable @Override + public Unit invoke(@NonNull Long siteId, @NonNull Long postId, + @NonNull Long commentId) { + ReaderActivityLauncher.showReaderComments( + WPMainActivity.this, + siteId, + postId, + commentId, + ThreadedCommentsActionSource.COMMENT_NOTIFICATION.getSourceDescription() + ); + return null; + } + }, new Function0() { + @Nullable @Override + public Unit invoke() { + boolean shouldShowKeyboard = getIntent().getBooleanExtra( + NotificationsListFragment.NOTE_INSTANT_REPLY_EXTRA, + false); + NotificationsListFragment.openNoteForReply(WPMainActivity.this, noteId, + shouldShowKeyboard, null, NotesAdapter.FILTERS.FILTER_ALL, true); + return null; + } + } + ); } } else { AppLog.e(T.NOTIFS, "app launched from a PN that doesn't have a note_id in it!!"); From ea7ee3bfa615b9da18897c54d3281f3fd4f2fb68 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 16 Feb 2024 11:04:12 +0200 Subject: [PATCH 10/10] Updates broken tests after merge --- .../notifications/NotificationsListViewModelTest.kt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt index 10894a678a84..ff44cca7c117 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/notifications/NotificationsListViewModelTest.kt @@ -13,6 +13,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.BaseUnitTest import org.wordpress.android.datasets.wrappers.ReaderPostTableWrapper +import org.wordpress.android.fluxc.store.AccountStore import org.wordpress.android.fluxc.store.CommentsStore import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.fluxc.utils.AppLogWrapper @@ -56,6 +57,9 @@ class NotificationsListViewModelTest : BaseUnitTest() { @Mock private lateinit var commentStore: CommentsStore + @Mock + private lateinit var accountStore: AccountStore + @Mock private lateinit var action: ActionHandler @@ -69,11 +73,12 @@ class NotificationsListViewModelTest : BaseUnitTest() { jetpackFeatureRemovalOverlayUtil, gcmMessageHandler, notificationsUtilsWrapper, - readerPostTableWrapper, - readerPostActionsWrapper, appLogWrapper, siteStore, - commentStore + commentStore, + readerPostTableWrapper, + readerPostActionsWrapper, + accountStore, ) }