diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderFragment.kt index 1ee55879d14b..708bc6b8500a 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderFragment.kt @@ -384,6 +384,6 @@ class ReaderFragment : Fragment(R.layout.reader_fragment_layout), MenuProvider, private fun clearFilter() { val viewModel = getSubFilterViewModel() ?: return - viewModel.setDefaultSubfilter() + viewModel.setDefaultSubfilter(isClearingFilter = true) } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostListFragment.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostListFragment.java index 9836f0d2a617..b20894355a08 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostListFragment.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostListFragment.java @@ -638,7 +638,6 @@ private void initSubFilterViewModel(@Nullable Bundle savedInstanceState) { visibleState.getCategories(), mUiHelpers.getTextOfUiString(requireContext(), visibleState.getTitle()) ); - mReaderTracker.track(Stat.READER_FILTER_SHEET_DISPLAYED); bottomSheet.show(getChildFragmentManager(), SUBFILTER_BOTTOM_SHEET_TAG); } else if (!uiState.isVisible() && bottomSheet != null) { bottomSheet.dismiss(); @@ -792,7 +791,7 @@ private void resumeFollowedTag() { mSubFilterViewModel.setSubfilterFromTag(newTag); } else if (isFollowingScreen() && !ReaderTagTable.tagExists(getCurrentTag())) { // user just removed a tag which was selected in the subfilter - mSubFilterViewModel.setDefaultSubfilter(); + mSubFilterViewModel.setDefaultSubfilter(false); } else { // otherwise, refresh posts to make sure any changes are reflected and auto-update // posts in the current tag if it's time @@ -828,7 +827,7 @@ private void resumeFollowedSite(Site currentSite) { refreshPosts(); } else { if (mIsFilterableScreen) { - mSubFilterViewModel.setDefaultSubfilter(); + mSubFilterViewModel.setDefaultSubfilter(false); } } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModel.kt index 9e6f9543523d..b1715a3a007a 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModel.kt @@ -193,13 +193,14 @@ class SubFilterViewModel @Inject constructor( ) } - fun setDefaultSubfilter() { + fun setDefaultSubfilter(isClearingFilter: Boolean) { readerTracker.track(Stat.READER_FILTER_SHEET_CLEARED) updateSubfilter( - SiteAll( + filter = SiteAll( onClickAction = ::onSubfilterClicked, - isSelected = true - ) + isSelected = true, + isClearingFilter = isClearingFilter, + ), ) } @@ -213,6 +214,11 @@ class SubFilterViewModel @Inject constructor( listOf(category) // TODO thomashortadev this should accept only a single category ) ) + val source = when(category) { + SubfilterCategory.SITES -> "blogs" + SubfilterCategory.TAGS -> "tags" + } + readerTracker.track(Stat.READER_FILTER_SHEET_DISPLAYED, source) } fun updateTagsAndSites() { @@ -286,7 +292,10 @@ class SubFilterViewModel @Inject constructor( } fun onSubfilterSelected(subfilterListItem: SubfilterListItem) { - readerTracker.track(Stat.READER_FILTER_SHEET_ITEM_SELECTED) + // We should not track subfilter selected if we're clearing a filter that is currently applied. + if (!subfilterListItem.isClearingFilter) { + readerTracker.track(Stat.READER_FILTER_SHEET_ITEM_SELECTED) + } changeSubfilter(subfilterListItem, true, mTagFragmentStartedWith) } @@ -340,7 +349,7 @@ class SubFilterViewModel @Inject constructor( ) ) - setDefaultSubfilter() + setDefaultSubfilter(false) } } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubfilterListItem.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubfilterListItem.kt index 0827c9cd121e..1e24f2c868d4 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubfilterListItem.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubfilterListItem.kt @@ -14,6 +14,7 @@ import org.wordpress.android.ui.utils.UiString.UiStringText sealed class SubfilterListItem(val type: ItemType, val isTrackedItem: Boolean = false) { open var isSelected: Boolean = false + open var isClearingFilter: Boolean = false open val onClickAction: ((filter: SubfilterListItem) -> Unit)? = null open val label: UiString? = null @@ -52,6 +53,7 @@ sealed class SubfilterListItem(val type: ItemType, val isTrackedItem: Boolean = @Suppress("DataClassShouldBeImmutable") data class SiteAll( override var isSelected: Boolean = false, + override var isClearingFilter: Boolean = false, override val onClickAction: (filter: SubfilterListItem) -> Unit ) : SubfilterListItem(SITE_ALL) { override val label: UiString = UiStringRes(R.string.reader_filter_cta) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModelTest.kt index 9e3565518c74..09c971abc5d1 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModelTest.kt @@ -16,6 +16,7 @@ import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.BaseUnitTest +import org.wordpress.android.analytics.AnalyticsTracker import org.wordpress.android.fluxc.model.AccountModel import org.wordpress.android.fluxc.store.AccountStore import org.wordpress.android.models.ReaderTag @@ -136,7 +137,7 @@ class SubFilterViewModelTest : BaseUnitTest() { @Test fun `view model doesn't start tracking subfiltered list if filter is a not tracked item`() { - viewModel.setDefaultSubfilter() + viewModel.setDefaultSubfilter(false) // this is done to focus this unit test only on effects of initSubfiltersTracking // usually it's not considered great to use this function but here it seemed @@ -171,7 +172,7 @@ class SubFilterViewModelTest : BaseUnitTest() { @Test fun `view model is able to set default subfilter`() { var item: SubfilterListItem? = null - viewModel.setDefaultSubfilter() + viewModel.setDefaultSubfilter(false) viewModel.currentSubFilter.observeForever { item = it } @@ -424,4 +425,24 @@ class SubFilterViewModelTest : BaseUnitTest() { assertThat(it.isFiltered).isEqualTo(true) } } + + @Test + fun `Should NOT track READER_FILTER_SHEET_ITEM_SELECTED if clearing filter when onSubfilterSelected is called`() { + val filter = SiteAll( + isClearingFilter = true, + onClickAction = {}, + ) + viewModel.onSubfilterSelected(filter) + verify(readerTracker, times(0)).track(AnalyticsTracker.Stat.READER_FILTER_SHEET_ITEM_SELECTED) + } + + @Test + fun `Should track READER_FILTER_SHEET_ITEM_SELECTED if NOT clearing filter when onSubfilterSelected is called`() { + val filter = SiteAll( + isClearingFilter = false, + onClickAction = {}, + ) + viewModel.onSubfilterSelected(filter) + verify(readerTracker).track(AnalyticsTracker.Stat.READER_FILTER_SHEET_ITEM_SELECTED) + } }