From 974a178d202242a50b3b6299178fa59232737357 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 27 Nov 2020 11:14:42 +0100 Subject: [PATCH 1/6] Pass initial activity type selection from parent to child fragment --- .../ui/activitylog/list/ActivityLogListFragment.kt | 9 +++++---- .../list/filter/ActivityLogTypeFilterFragment.kt | 5 ++++- .../viewmodel/activitylog/ActivityLogViewModel.kt | 12 ++++++++---- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/ActivityLogListFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/ActivityLogListFragment.kt index 5eb1ef43a399..fcf19953ed48 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/ActivityLogListFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/ActivityLogListFragment.kt @@ -124,8 +124,8 @@ class ActivityLogListFragment : Fragment() { uiHelpers.updateVisibility(activity_type_filter, visibility) }) - viewModel.showActivityTypeFilterDialog.observe(viewLifecycleOwner, Observer { remoteSiteId -> - showActivityTypeFilterDialog(remoteSiteId) + viewModel.showActivityTypeFilterDialog.observe(viewLifecycleOwner, Observer { event -> + showActivityTypeFilterDialog(event.siteId, event.initialSelection) }) viewModel.showDateRangePicker.observe(viewLifecycleOwner, Observer { event -> @@ -181,8 +181,9 @@ class ActivityLogListFragment : Fragment() { picker.addOnPositiveButtonClickListener { viewModel.onDateRangeSelected(it) } } - private fun showActivityTypeFilterDialog(remoteSiteId: RemoteId) { - ActivityLogTypeFilterFragment.newInstance(remoteSiteId).show(childFragmentManager, ACTIVITY_TYPE_FILTER_TAG) + private fun showActivityTypeFilterDialog(remoteSiteId: RemoteId, initialSelection: List) { + ActivityLogTypeFilterFragment.newInstance(remoteSiteId, initialSelection) + .show(childFragmentManager, ACTIVITY_TYPE_FILTER_TAG) } private fun refreshProgressBars(eventListStatus: ActivityLogViewModel.ActivityLogListStatus?) { diff --git a/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterFragment.kt index 1e0abcc85c7a..43d0e7423ee4 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterFragment.kt @@ -27,6 +27,7 @@ import org.wordpress.android.util.getColorResIdFromAttribute import org.wordpress.android.viewmodel.activitylog.ActivityLogViewModel import javax.inject.Inject +private const val ARG_INITIAL_SELECTION = "arg_initial_selection" private const val ACTIONS_MENU_GROUP = 1 /** @@ -101,6 +102,7 @@ class ActivityLogTypeFilterFragment : DialogFragment() { }) viewModel.start( remoteSiteId = RemoteId(requireNotNull(arguments).getLong(WordPress.REMOTE_SITE_ID)), + initialSelection = requireNotNull(arguments).getIntArray(ARG_INITIAL_SELECTION)?.toList() ?: listOf(), parentViewModel = parentViewModel ) } @@ -151,8 +153,9 @@ class ActivityLogTypeFilterFragment : DialogFragment() { companion object { @JvmStatic - fun newInstance(remoteSiteId: RemoteId): ActivityLogTypeFilterFragment { + fun newInstance(remoteSiteId: RemoteId, initialSelection: List): ActivityLogTypeFilterFragment { val args = Bundle() + args.putIntArray(ARG_INITIAL_SELECTION, initialSelection.toIntArray()) args.putLong(WordPress.REMOTE_SITE_ID, remoteSiteId.value) return ActivityLogTypeFilterFragment().apply { arguments = args } } diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModel.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModel.kt index 0c0e219dfbdd..a6a9d4b01b58 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModel.kt @@ -67,8 +67,8 @@ class ActivityLogViewModel @Inject constructor( val showRewindDialog: LiveData get() = _showRewindDialog - private val _showActivityTypeFilterDialog = SingleLiveEvent() - val showActivityTypeFilterDialog: LiveData + private val _showActivityTypeFilterDialog = SingleLiveEvent() + val showActivityTypeFilterDialog: LiveData get() = _showActivityTypeFilterDialog private val _showDateRangePicker = SingleLiveEvent() @@ -101,7 +101,10 @@ class ActivityLogViewModel @Inject constructor( private var lastRewindActivityId: String? = null private var lastRewindStatus: Status? = null + private var currentDateRangeFilter: DateRange? = null + private var currentActivityTypeFilter: List = listOf() + private val rewindProgressObserver = Observer { if (it?.activityLogItem?.activityID != lastRewindActivityId || it?.status != lastRewindStatus) { lastRewindActivityId = it?.activityLogItem?.activityID @@ -175,11 +178,11 @@ class ActivityLogViewModel @Inject constructor( fun onActivityTypeFilterClicked() { // TODO malinjir pass initially selected activity types - _showActivityTypeFilterDialog.value = RemoteId(site.siteId) + _showActivityTypeFilterDialog.value = ShowActivityTypePicker(RemoteId(site.siteId), currentActivityTypeFilter) } fun onActivityTypesSelected(activityTypeIds: List) { - // TODO malinjir store the ids + currentActivityTypeFilter = activityTypeIds // TODO malinjir: refetch/load data } @@ -327,4 +330,5 @@ class ActivityLogViewModel @Inject constructor( } data class ShowDateRangePicker(val initialSelection: DateRange?) + data class ShowActivityTypePicker(val siteId: RemoteId, val initialSelection: List) } From d9e3db551f081ff28238874b735969901bfdb2ba Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 27 Nov 2020 11:15:44 +0100 Subject: [PATCH 2/6] Check items provided in initiallySelected list --- .../list/filter/ActivityLogTypeFilterViewModel.kt | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModel.kt index 8e867bf60e49..76383e4b8f50 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModel.kt @@ -30,6 +30,7 @@ class ActivityLogTypeFilterViewModel @Inject constructor( private var isStarted = false private lateinit var remoteSiteId: RemoteId private lateinit var parentViewModel: ActivityLogViewModel + private lateinit var initialSelection: List private val _uiState = MutableLiveData() val uiState: LiveData = _uiState @@ -37,11 +38,16 @@ class ActivityLogTypeFilterViewModel @Inject constructor( private val _dismissDialog = MutableLiveData>() val dismissDialog: LiveData> = _dismissDialog - fun start(remoteSiteId: RemoteId, parentViewModel: ActivityLogViewModel) { + fun start( + remoteSiteId: RemoteId, + parentViewModel: ActivityLogViewModel, + initialSelection: List + ) { if (isStarted) return isStarted = true this.remoteSiteId = remoteSiteId this.parentViewModel = parentViewModel + this.initialSelection = initialSelection fetchAvailableActivityTypes() } @@ -61,7 +67,7 @@ class ActivityLogTypeFilterViewModel @Inject constructor( private fun buildErrorUiState() = UiState.Error(Action(UiStringRes(R.string.retry)).apply { action = ::onRetryClicked }) - private suspend fun buildContentUiState(activityTypes: List): Content { + private suspend fun buildContentUiState(activityTypes: List, ): Content { return withContext(bgDispatcher) { // TODO malinjir replace the hardcoded header title val headerListItem = ListItemUiState.SectionHeader(UiStringText("Test")) @@ -71,7 +77,8 @@ class ActivityLogTypeFilterViewModel @Inject constructor( ListItemUiState.ActivityType( id = it.id, title = UiStringText(it.toString()), - onClick = { onItemClicked(it.id) } + onClick = { onItemClicked(it.id) }, + checked = initialSelection.contains(it.id) ) } Content( From 7fc943ca333f4155da9cec889dfa20f1492b7f57 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 27 Nov 2020 11:35:24 +0100 Subject: [PATCH 3/6] Fix NPE in tests --- .../activitylog/ActivityLogViewModelTest.kt | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt index 3045512efa45..fbcd468a1a8a 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt @@ -4,6 +4,7 @@ import androidx.arch.core.executor.testing.InstantTaskExecutorRule import androidx.lifecycle.MutableLiveData import com.nhaarman.mockitokotlin2.KArgumentCaptor import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.argumentCaptor import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never @@ -121,7 +122,7 @@ class ActivityLogViewModelTest { whenever(store.getRewindStatusForSite(site)).thenReturn(rewindStatusModel) whenever(rewindStatusService.rewindProgress).thenReturn(rewindProgress) whenever(rewindStatusService.rewindAvailable).thenReturn(rewindAvailable) - whenever(store.fetchActivities(any())).thenReturn(mock()) + whenever(store.fetchActivities(anyOrNull())).thenReturn(mock()) } @Test @@ -152,7 +153,7 @@ class ActivityLogViewModelTest { @Test fun onDataFetchedPostsDataAndChangesStatusIfCanLoadMore() = runBlocking { val canLoadMore = true - whenever(store.fetchActivities(any())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) viewModel.start(site) @@ -167,11 +168,12 @@ class ActivityLogViewModelTest { @Test fun onDataFetchedLoadsMoreDataIfCanLoadMore() = runBlocking { val canLoadMore = true - whenever(store.fetchActivities(any())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) viewModel.start(site) reset(store) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) viewModel.onScrolledToBottom() @@ -181,7 +183,7 @@ class ActivityLogViewModelTest { @Test fun onDataFetchedPostsDataAndChangesStatusIfCannotLoadMore() = runBlocking { val canLoadMore = false - whenever(store.fetchActivities(any())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) viewModel.start(site) @@ -197,7 +199,7 @@ class ActivityLogViewModelTest { fun onDataFetchedShowsFooterIfCannotLoadMoreAndIsFreeSite() = runBlocking { val canLoadMore = false whenever(site.hasFreePlan).thenReturn(true) - whenever(store.fetchActivities(any())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) viewModel.start(site) @@ -232,7 +234,7 @@ class ActivityLogViewModelTest { @Test fun onDataFetchedDoesNotLoadMoreDataIfCannotLoadMore() = runBlocking { val canLoadMore = false - whenever(store.fetchActivities(any())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(1, canLoadMore, FETCH_ACTIVITIES)) viewModel.start(site) @@ -240,14 +242,14 @@ class ActivityLogViewModelTest { viewModel.onScrolledToBottom() - verify(store, never()).fetchActivities(any()) + verify(store, never()).fetchActivities(anyOrNull()) } @Test fun onDataFetchedGoesToTopWhenSomeRowsAffected() = runBlocking { assertTrue(moveToTopEvents.isEmpty()) - whenever(store.fetchActivities(any())).thenReturn(OnActivityLogFetched(10, true, FETCH_ACTIVITIES)) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(10, true, FETCH_ACTIVITIES)) viewModel.start(site) @@ -258,7 +260,7 @@ class ActivityLogViewModelTest { fun onDataFetchedDoesNotLoadMoreDataIfNoRowsAffected() = runBlocking { val canLoadMore = true - whenever(store.fetchActivities(any())).thenReturn(OnActivityLogFetched(0, canLoadMore, FETCH_ACTIVITIES)) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(0, canLoadMore, FETCH_ACTIVITIES)) viewModel.start(site) @@ -268,7 +270,7 @@ class ActivityLogViewModelTest { @Test fun headerIsDisplayedForFirstItemOrWhenDifferentThenPrevious() = runBlocking { val canLoadMore = true - whenever(store.fetchActivities(any())).thenReturn(OnActivityLogFetched(3, canLoadMore, FETCH_ACTIVITIES)) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(3, canLoadMore, FETCH_ACTIVITIES)) viewModel.start(site) @@ -318,10 +320,11 @@ class ActivityLogViewModelTest { @Test fun loadsNextPageOnScrollToBottom() = runBlocking { - whenever(store.fetchActivities(any())).thenReturn(OnActivityLogFetched(10, true, FETCH_ACTIVITIES)) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(10, true, FETCH_ACTIVITIES)) viewModel.start(site) reset(store) + whenever(store.fetchActivities(anyOrNull())).thenReturn(OnActivityLogFetched(10, true, FETCH_ACTIVITIES)) viewModel.onScrolledToBottom() @@ -357,7 +360,7 @@ class ActivityLogViewModelTest { fun onActivityTypeFilterClickRemoteSiteIdIsPassed() { viewModel.onActivityTypeFilterClicked() - assertEquals(RemoteId(site.siteId), viewModel.showActivityTypeFilterDialog.value) + assertEquals(RemoteId(site.siteId), viewModel.showActivityTypeFilterDialog.value!!.siteId) } private suspend fun assertFetchEvents(canLoadMore: Boolean = false) { From e4cfe89853b0f3aaa8769fa6d3ef2a371c920c70 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 27 Nov 2020 11:36:02 +0100 Subject: [PATCH 4/6] Add tests for items initial selection --- .../filter/ActivityLogTypeFilterViewModelTest.kt | 16 ++++++++++++++-- .../activitylog/ActivityLogViewModelTest.kt | 10 ++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModelTest.kt index 55894d8697bc..cd3ce85ce800 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModelTest.kt @@ -175,6 +175,18 @@ class ActivityLogTypeFilterViewModelTest : BaseUnitTest() { ).isEmpty() } + @Test + fun `items are checked, when the user opens the screen with active activity type filter`() = test { + val uiStates = init().uiStates + val initialSelection = listOf(1,4) + + startVM(initialSelection = initialSelection) + + assertThat((uiStates.last() as Content).items.filterIsInstance(ActivityType::class.java) + .filter { it.checked }.map { it.id } + ).containsExactlyElementsOf(initialSelection) + } + private suspend fun init(successResponse: Boolean = true, activityTypeCount: Int = 5): Observers { val uiStates = mutableListOf() val dismissDialogEvents = mutableListOf() @@ -196,8 +208,8 @@ class ActivityLogTypeFilterViewModelTest : BaseUnitTest() { return Observers(uiStates, dismissDialogEvents) } - private fun startVM() { - viewModel.start(RemoteId(0L), parentViewModel) + private fun startVM(initialSelection: List = listOf()) { + viewModel.start(RemoteId(0L), parentViewModel, initialSelection) } private fun generateActivityTypes(count: Int): List { diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt index fbcd468a1a8a..6f7b03dc673e 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt @@ -363,6 +363,16 @@ class ActivityLogViewModelTest { assertEquals(RemoteId(site.siteId), viewModel.showActivityTypeFilterDialog.value!!.siteId) } + @Test + fun onActivityTypeFilterClickPreviouslySelectedTypesPassed() { + val selectedItems = listOf(1,4) + viewModel.onActivityTypesSelected(selectedItems) + + viewModel.onActivityTypeFilterClicked() + + assertEquals(selectedItems, viewModel.showActivityTypeFilterDialog.value!!.initialSelection) + } + private suspend fun assertFetchEvents(canLoadMore: Boolean = false) { verify(store).fetchActivities(fetchActivityLogCaptor.capture()) From 73341fe13c49890ddccd22266b327da60951ded3 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 27 Nov 2020 11:47:22 +0100 Subject: [PATCH 5/6] Remove extra comma --- .../activitylog/list/filter/ActivityLogTypeFilterViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModel.kt index 76383e4b8f50..040367871ea6 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModel.kt @@ -67,7 +67,7 @@ class ActivityLogTypeFilterViewModel @Inject constructor( private fun buildErrorUiState() = UiState.Error(Action(UiStringRes(R.string.retry)).apply { action = ::onRetryClicked }) - private suspend fun buildContentUiState(activityTypes: List, ): Content { + private suspend fun buildContentUiState(activityTypes: List): Content { return withContext(bgDispatcher) { // TODO malinjir replace the hardcoded header title val headerListItem = ListItemUiState.SectionHeader(UiStringText("Test")) From b1f34dde986ccae64737f2d291c02cc611a7fe41 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 27 Nov 2020 11:48:19 +0100 Subject: [PATCH 6/6] Fix lint --- .../filter/ActivityLogTypeFilterViewModelTest.kt | 2 +- .../activitylog/ActivityLogViewModelTest.kt | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModelTest.kt index cd3ce85ce800..835b06188400 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/activitylog/list/filter/ActivityLogTypeFilterViewModelTest.kt @@ -178,7 +178,7 @@ class ActivityLogTypeFilterViewModelTest : BaseUnitTest() { @Test fun `items are checked, when the user opens the screen with active activity type filter`() = test { val uiStates = init().uiStates - val initialSelection = listOf(1,4) + val initialSelection = listOf(1, 4) startVM(initialSelection = initialSelection) diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt index 6f7b03dc673e..3d4e4d94062d 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/activitylog/ActivityLogViewModelTest.kt @@ -72,7 +72,8 @@ class ActivityLogViewModelTest { Date(), true, null, - null) + null + ) val event = ActivityLogListItem.Event( "activityId", @@ -97,7 +98,7 @@ class ActivityLogViewModelTest { null, Date(), null - ) + ) @Before fun setUp() = runBlocking { @@ -365,7 +366,7 @@ class ActivityLogViewModelTest { @Test fun onActivityTypeFilterClickPreviouslySelectedTypesPassed() { - val selectedItems = listOf(1,4) + val selectedItems = listOf(1, 4) viewModel.onActivityTypesSelected(selectedItems) viewModel.onActivityTypeFilterClicked() @@ -387,8 +388,10 @@ class ActivityLogViewModelTest { birthday.set(1985, 8, 27) val list = mutableListOf() - val activity = ActivityLogModel("", "", null, "", "", "", - "", true, "", birthday.time) + val activity = ActivityLogModel( + "", "", null, "", "", "", + "", true, "", birthday.time + ) list.add(activity) list.add(activity.copy())