From 296f1b32edd610b21c805af16acfd01444a5e662 Mon Sep 17 00:00:00 2001 From: develric Date: Fri, 14 Feb 2020 23:28:59 +0100 Subject: [PATCH 01/13] Marking Sites and Tags as tracked items. --- .../android/ui/reader/subfilter/SubfilterListItem.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 5330555b9002..ee8ada553299 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 @@ -12,7 +12,7 @@ import org.wordpress.android.ui.utils.UiString import org.wordpress.android.ui.utils.UiString.UiStringRes import org.wordpress.android.ui.utils.UiString.UiStringText -sealed class SubfilterListItem(val type: ItemType) { +sealed class SubfilterListItem(val type: ItemType, val isTrackedItem: Boolean = false) { open var isSelected: Boolean = false open val onClickAction: ((filter: SubfilterListItem) -> Unit)? = null open val label: UiString? = null @@ -60,7 +60,7 @@ sealed class SubfilterListItem(val type: ItemType) { override var isSelected: Boolean = false, override val onClickAction: (filter: SubfilterListItem) -> Unit, val blog: ReaderBlog - ) : SubfilterListItem(SITE) { + ) : SubfilterListItem(SITE, true) { override val label: UiString = if (blog.name.isNotEmpty()) { UiStringText(blog.name) } else { @@ -72,7 +72,7 @@ sealed class SubfilterListItem(val type: ItemType) { override var isSelected: Boolean = false, override val onClickAction: (filter: SubfilterListItem) -> Unit, val tag: ReaderTag - ) : SubfilterListItem(TAG) { + ) : SubfilterListItem(TAG, true) { override val label: UiString = UiStringText(tag.tagTitle) } } From 4002dd94b33774d1d3ddc8a2ebe22245308d87a3 Mon Sep 17 00:00:00 2001 From: develric Date: Fri, 14 Feb 2020 23:29:43 +0100 Subject: [PATCH 02/13] Adding isRunning function to ReaderTracker. --- .../wordpress/android/ui/reader/tracker/ReaderTracker.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTracker.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTracker.kt index 112cf133698a..413d620da887 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTracker.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTracker.kt @@ -11,7 +11,7 @@ import javax.inject.Singleton @Singleton @MainThread class ReaderTracker @Inject constructor(private val dateProvider: DateProvider) { - // TODO: evaluate during IA extensions to use something like Dispatchers.Main.Immediate in the fun(s) + // TODO: evaluate to use something like Dispatchers.Main.Immediate in the fun(s) // to sync the access to trackers; so to remove the @MainThread and make the // usage of this class more transparent to its users private val trackers = mutableMapOf() @@ -45,6 +45,12 @@ class ReaderTracker @Inject constructor(private val dateProvider: DateProvider) } } + fun isRunning(type: ReaderTrackerType): Boolean { + return trackers[type]?.let { trackerInto -> + trackerInto.startDate != null + } ?: false + } + fun getAnalyticsData(): Map { return trackers.entries.associate { it.key.propertyName to it.value.accumulatedTime } } From 2d9f0bc906fb1d420e95ddf5d9232670a3e451f0 Mon Sep 17 00:00:00 2001 From: develric Date: Fri, 14 Feb 2020 23:30:21 +0100 Subject: [PATCH 03/13] Adding dedicated tracker to trace time spent in content filtered with reader subfilter. --- .../wordpress/android/ui/reader/tracker/ReaderTrackerInfo.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTrackerInfo.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTrackerInfo.kt index 5602abdb6f2b..85d54bdb8143 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTrackerInfo.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTrackerInfo.kt @@ -10,5 +10,6 @@ data class ReaderTrackerInfo( enum class ReaderTrackerType constructor(val propertyName: String) { MAIN_READER("time_in_main_reader"), FILTERED_LIST("time_in_reader_filtered_list"), - PAGED_POST("time_in_reader_paged_post") + PAGED_POST("time_in_reader_paged_post"), + SUBFILTERED_LIST("time_in_subfiltered_list") } From a56fa426b199a032c8402d3d15ef408e3f9d089e Mon Sep 17 00:00:00 2001 From: develric Date: Fri, 14 Feb 2020 23:33:08 +0100 Subject: [PATCH 04/13] Moving tracking into the View Model and adding tracking for subfiltered content. --- .../ui/reader/ReaderPostListFragment.java | 22 +++--- .../viewmodels/ReaderPostListViewModel.kt | 78 ++++++++++++++++++- 2 files changed, 84 insertions(+), 16 deletions(-) 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 9f91a1864ec8..c470fff61887 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 @@ -108,6 +108,7 @@ 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.subfilter.SubfilterListItem.Site; +import org.wordpress.android.ui.reader.subfilter.SubfilterListItem.SiteAll; import org.wordpress.android.ui.reader.tracker.ReaderTracker; import org.wordpress.android.ui.reader.tracker.ReaderTrackerType; import org.wordpress.android.ui.reader.utils.ReaderUtils; @@ -224,7 +225,6 @@ public class ReaderPostListFragment extends Fragment @Inject ImageManager mImageManager; @Inject QuickStartStore mQuickStartStore; @Inject UiHelpers mUiHelpers; - @Inject ReaderTracker mReaderTracker; private enum ActionableEmptyViewButtonType { DISCOVER, @@ -405,7 +405,7 @@ public void onCreate(Bundle savedInstanceState) { BuildConfig.INFORMATION_ARCHITECTURE_AVAILABLE && mIsTopLevel, mRecyclerView ) || ReaderUtils.isDefaultTag(mCurrentTag)) && getPostListType() != ReaderPostListType.SEARCH_RESULTS) { - mViewModel.onSubfilterChanged(subfilterListItem, true); + mViewModel.onSubfilterChanged(subfilterListItem); } }); @@ -522,23 +522,19 @@ private void changeReaderMode(ReaderModeInfo readerModeInfo, boolean onlyOnChang @Override public void onPause() { super.onPause(); - AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count [mIsTopLevel = " + mIsTopLevel + "]"); - mReaderTracker.stop( - mIsTopLevel ? ReaderTrackerType.MAIN_READER : ReaderTrackerType.FILTERED_LIST - ); + if (mBookmarksSavedLocallyDialog != null) { mBookmarksSavedLocallyDialog.dismiss(); } mWasPaused = true; + + mViewModel.onFragmentPause(mIsTopLevel); } @Override public void onResume() { super.onResume(); - AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > START Count [mIsTopLevel = " + mIsTopLevel + "]"); - mReaderTracker.start( - mIsTopLevel ? ReaderTrackerType.MAIN_READER : ReaderTrackerType.FILTERED_LIST - ); + checkPostAdapter(); if (mWasPaused) { @@ -580,6 +576,8 @@ public void onResume() { AppLog.w(T.READER, "Discover tag not found; ReaderTagTable returned null"); } } + + mViewModel.onFragmentResume(mIsTopLevel, ReaderUtils.isFollowing(getCurrentTag(), mIsTopLevel, mRecyclerView)); } /* @@ -1094,7 +1092,7 @@ public boolean onMenuItemActionCollapse(MenuItem item) { BuildConfig.INFORMATION_ARCHITECTURE_AVAILABLE && mIsTopLevel, mRecyclerView) ) { - mViewModel.onSubfilterChanged(mViewModel.getCurrentSubfilterValue(), false); + mViewModel.manageSubfilter(); } else { // return to the followed tag that was showing prior to searching resetPostAdapter(ReaderPostListType.TAG_FOLLOWED); @@ -2004,7 +2002,7 @@ && getPostAdapter().isCurrentTag(tag)) { if (BuildConfig.INFORMATION_ARCHITECTURE_AVAILABLE && manageSubfilter) { if (mCurrentTag.isFollowedSites()) { - mViewModel.onSubfilterChanged(mViewModel.getCurrentSubfilterValue(), false); + mViewModel.manageSubfilter(); } else { changeReaderMode(new ReaderModeInfo( tag, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt index b78f4bc80285..a06c35dcb0cf 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt @@ -28,6 +28,8 @@ import org.wordpress.android.ui.reader.subfilter.SubfilterListItem.Site import org.wordpress.android.ui.reader.subfilter.SubfilterListItem.SiteAll import org.wordpress.android.ui.reader.subfilter.SubfilterListItem.Tag import org.wordpress.android.ui.reader.subfilter.SubfilterListItemMapper +import org.wordpress.android.ui.reader.tracker.ReaderTracker +import org.wordpress.android.ui.reader.tracker.ReaderTrackerType import org.wordpress.android.ui.reader.utils.ReaderUtils import org.wordpress.android.util.AppLog import org.wordpress.android.util.AppLog.T @@ -46,7 +48,8 @@ class ReaderPostListViewModel @Inject constructor( @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, private val appPrefsWrapper: AppPrefsWrapper, private val subfilterListItemMapper: SubfilterListItemMapper, - private val eventBusWrapper: EventBusWrapper + private val eventBusWrapper: EventBusWrapper, + private val readerTracker: ReaderTracker ) : ScopedViewModel(bgDispatcher) { private val newsItemSource = newsManager.newsItemSource() private val _newsItemSourceMediator = MediatorLiveData() @@ -104,7 +107,7 @@ class ReaderPostListViewModel @Inject constructor( newsManager.pull() updateSubfilter(getCurrentSubfilterValue()) - _shouldShowSubFilters.value = shouldShowSubfilter + changeSubfiltersVisibility(shouldShowSubfilter) } _shouldCollapseToolbar.value = collapseToolbar @@ -197,7 +200,24 @@ class ReaderPostListViewModel @Inject constructor( updateSubfilter(filter) } - fun changeSubfiltersVisibility(show: Boolean) = _shouldShowSubFilters.postValue(show) + fun changeSubfiltersVisibility(show: Boolean) { + if (show) { + if (getCurrentSubfilterValue().isTrackedItem && + !readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + AppLog.d(T.READER,"TRACK READER ReaderPostListFragment > START Count SUBFILTERED_LIST") + readerTracker.start(ReaderTrackerType.SUBFILTERED_LIST) + } else if (!getCurrentSubfilterValue().isTrackedItem && + readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count SUBFILTERED_LIST") + readerTracker.stop(ReaderTrackerType.SUBFILTERED_LIST) + } + } else if (readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)){ + AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count SUBFILTERED_LIST") + readerTracker.stop(ReaderTrackerType.SUBFILTERED_LIST) + } + + _shouldShowSubFilters.postValue(show) + } fun getCurrentSubfilterValue(): SubfilterListItem { return if (!BuildConfig.INFORMATION_ARCHITECTURE_AVAILABLE) { @@ -244,10 +264,19 @@ class ReaderPostListViewModel @Inject constructor( _changeBottomSheetVisibility.value = Event(false) } - fun onSubfilterChanged( + private fun changeSubfilter( subfilterListItem: SubfilterListItem, requestNewerPosts: Boolean ) { + if (subfilterListItem.isTrackedItem && + !readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > START Count SUBFILTERED_LIST") + readerTracker.start(ReaderTrackerType.SUBFILTERED_LIST) + } else if (!subfilterListItem.isTrackedItem && readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count SUBFILTERED_LIST") + readerTracker.stop(ReaderTrackerType.SUBFILTERED_LIST) + } + when (subfilterListItem.type) { SubfilterListItem.ItemType.SECTION_TITLE, SubfilterListItem.ItemType.DIVIDER -> { @@ -295,6 +324,14 @@ class ReaderPostListViewModel @Inject constructor( isFirstLoad = false } + fun onSubfilterChanged(subfilterListItem: SubfilterListItem) { + changeSubfilter(subfilterListItem, true) + } + + fun manageSubfilter() { + changeSubfilter(getCurrentSubfilterValue(), false) + } + fun onSearchMenuCollapse(collapse: Boolean) { _shouldCollapseToolbar.value = collapse } @@ -314,6 +351,39 @@ class ReaderPostListViewModel @Inject constructor( _startSubsActivity.postValue(Event(selectedTabIndex)) } + fun onFragmentResume(isTopLevelFragment: Boolean, isFollowingTag: Boolean) { + AppLog.d( + T.READER, + "TRACK READER ReaderPostListFragment > START Count [mIsTopLevel = $isTopLevelFragment]" + ) + readerTracker.start( + if (isTopLevelFragment) ReaderTrackerType.MAIN_READER else ReaderTrackerType.FILTERED_LIST + ) + + if (BuildConfig.INFORMATION_ARCHITECTURE_AVAILABLE && isTopLevelFragment) { + if (isFollowingTag && getCurrentSubfilterValue().isTrackedItem) { + AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > START Count SUBFILTERED_LIST") + readerTracker.start(ReaderTrackerType.SUBFILTERED_LIST) + } + } + } + + fun onFragmentPause(isTopLevelFragment: Boolean) { + AppLog.d( + T.READER, + "TRACK READER ReaderPostListFragment > STOP Count [mIsTopLevel = $isTopLevelFragment]" + ) + readerTracker.stop( + if (isTopLevelFragment) ReaderTrackerType.MAIN_READER else ReaderTrackerType.FILTERED_LIST + ) + + if (BuildConfig.INFORMATION_ARCHITECTURE_AVAILABLE && + isTopLevelFragment && readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count SUBFILTERED_LIST") + readerTracker.stop(ReaderTrackerType.SUBFILTERED_LIST) + } + } + private fun updateSubfilter(filter: SubfilterListItem) { _currentSubFilter.postValue(filter) val json = subfilterListItemMapper.toJson(filter) From 582085e78788cbcd0b8fb1605518c2192833820c Mon Sep 17 00:00:00 2001 From: develric Date: Fri, 14 Feb 2020 23:46:36 +0100 Subject: [PATCH 05/13] Fixing Lint. --- .../wordpress/android/ui/reader/ReaderPostListFragment.java | 3 --- .../android/ui/reader/viewmodels/ReaderPostListViewModel.kt | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) 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 c470fff61887..aea05ee81881 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 @@ -108,9 +108,6 @@ 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.subfilter.SubfilterListItem.Site; -import org.wordpress.android.ui.reader.subfilter.SubfilterListItem.SiteAll; -import org.wordpress.android.ui.reader.tracker.ReaderTracker; -import org.wordpress.android.ui.reader.tracker.ReaderTrackerType; import org.wordpress.android.ui.reader.utils.ReaderUtils; import org.wordpress.android.ui.reader.viewmodels.ReaderModeInfo; import org.wordpress.android.ui.reader.viewmodels.ReaderPostListViewModel; diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt index a06c35dcb0cf..1695496d446b 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt @@ -204,14 +204,14 @@ class ReaderPostListViewModel @Inject constructor( if (show) { if (getCurrentSubfilterValue().isTrackedItem && !readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { - AppLog.d(T.READER,"TRACK READER ReaderPostListFragment > START Count SUBFILTERED_LIST") + AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > START Count SUBFILTERED_LIST") readerTracker.start(ReaderTrackerType.SUBFILTERED_LIST) } else if (!getCurrentSubfilterValue().isTrackedItem && readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count SUBFILTERED_LIST") readerTracker.stop(ReaderTrackerType.SUBFILTERED_LIST) } - } else if (readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)){ + } else if (readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count SUBFILTERED_LIST") readerTracker.stop(ReaderTrackerType.SUBFILTERED_LIST) } From 0e5a12875abad59e5959b646b2a3e9dd3530d0f1 Mon Sep 17 00:00:00 2001 From: develric Date: Sat, 15 Feb 2020 00:54:00 +0100 Subject: [PATCH 06/13] Fixing Unit Testing. --- .../android/ui/reader/ReaderTrackerTest.kt | 60 +++++++++++++++++-- .../reader/ReaderPostListViewModelTest.kt | 5 +- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/ReaderTrackerTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/ReaderTrackerTest.kt index 635a1114d3a8..93b5f08b0f51 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/reader/ReaderTrackerTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/reader/ReaderTrackerTest.kt @@ -11,6 +11,7 @@ import org.wordpress.android.ui.reader.tracker.ReaderTracker import org.wordpress.android.ui.reader.tracker.ReaderTrackerType.FILTERED_LIST import org.wordpress.android.ui.reader.tracker.ReaderTrackerType.MAIN_READER import org.wordpress.android.ui.reader.tracker.ReaderTrackerType.PAGED_POST +import org.wordpress.android.ui.reader.tracker.ReaderTrackerType.SUBFILTERED_LIST import org.wordpress.android.ui.reader.utils.DateProvider import java.util.Calendar import java.util.Date @@ -32,7 +33,8 @@ class ReaderTrackerTest { val expected = mapOf( "time_in_main_reader" to 0, "time_in_reader_filtered_list" to 0, - "time_in_reader_paged_post" to 0 + "time_in_reader_paged_post" to 0, + "time_in_subfiltered_list" to 0 ) assertThat(tracker.getAnalyticsData()).isEqualTo(expected) @@ -48,6 +50,7 @@ class ReaderTrackerTest { tracker.start(MAIN_READER) tracker.start(FILTERED_LIST) tracker.start(PAGED_POST) + tracker.start(SUBFILTERED_LIST) whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, Int.MAX_VALUE - 1)) tracker.stop(MAIN_READER) @@ -58,10 +61,15 @@ class ReaderTrackerTest { whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, Int.MAX_VALUE - 3)) tracker.stop(PAGED_POST) + whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, Int.MAX_VALUE - 4)) + tracker.stop(SUBFILTERED_LIST) + + val expected = mapOf( "time_in_main_reader" to Int.MAX_VALUE - 1, "time_in_reader_filtered_list" to Int.MAX_VALUE - 2, - "time_in_reader_paged_post" to Int.MAX_VALUE - 3 + "time_in_reader_paged_post" to Int.MAX_VALUE - 3, + "time_in_subfiltered_list" to Int.MAX_VALUE - 4 ) assertThat(tracker.getAnalyticsData()).isEqualTo(expected) } @@ -80,6 +88,7 @@ class ReaderTrackerTest { tracker.start(MAIN_READER) tracker.start(FILTERED_LIST) tracker.start(PAGED_POST) + tracker.start(SUBFILTERED_LIST) whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, 1)) tracker.stop(MAIN_READER) @@ -89,12 +98,16 @@ class ReaderTrackerTest { whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, 3)) tracker.stop(PAGED_POST) + + whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, 4)) + tracker.stop(SUBFILTERED_LIST) } val expected = mapOf( "time_in_main_reader" to (1 * numRep), "time_in_reader_filtered_list" to (2 * numRep), - "time_in_reader_paged_post" to (3 * numRep) + "time_in_reader_paged_post" to (3 * numRep), + "time_in_subfiltered_list" to (4 * numRep) ) assertThat(tracker.getAnalyticsData()).isEqualTo(expected) } @@ -113,6 +126,7 @@ class ReaderTrackerTest { tracker.start(MAIN_READER) tracker.start(FILTERED_LIST) tracker.start(PAGED_POST) + tracker.start(SUBFILTERED_LIST) whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, 1)) tracker.stop(MAIN_READER) @@ -122,25 +136,61 @@ class ReaderTrackerTest { whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, 3)) tracker.stop(PAGED_POST) + + whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, 4)) + tracker.stop(SUBFILTERED_LIST) } var expected = mapOf( "time_in_main_reader" to (1 * numRep), "time_in_reader_filtered_list" to (2 * numRep), - "time_in_reader_paged_post" to (3 * numRep) + "time_in_reader_paged_post" to (3 * numRep), + "time_in_subfiltered_list" to (4 * numRep) ) assertThat(tracker.getAnalyticsData()).isEqualTo(expected) expected = mapOf( "time_in_main_reader" to 0, "time_in_reader_filtered_list" to 0, - "time_in_reader_paged_post" to 0 + "time_in_reader_paged_post" to 0, + "time_in_subfiltered_list" to 0 ) tracker.setupTrackers() assertThat(tracker.getAnalyticsData()).isEqualTo(expected) } + @Test + fun `tracker is not running if not started`() { + tracker.setupTrackers() + + assertThat(tracker.isRunning(MAIN_READER)).isEqualTo(false) + } + + + @Test + fun `tracker is running after start`() { + tracker.setupTrackers() + val startPoint = Date() + whenever(dateProvider.getCurrentDate()).thenReturn(startPoint) + + tracker.start(MAIN_READER) + + assertThat(tracker.isRunning(MAIN_READER)).isEqualTo(true) + } + + @Test + fun `tracker is not running after stop`() { + tracker.setupTrackers() + val startPoint = Date() + whenever(dateProvider.getCurrentDate()).thenReturn(startPoint) + + tracker.start(MAIN_READER) + tracker.stop(MAIN_READER) + + assertThat(tracker.isRunning(MAIN_READER)).isEqualTo(false) + } + private fun addToDate(date: Date, seconds: Int): Date { val calendar = Calendar.getInstance() calendar.time = date diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/reader/ReaderPostListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/reader/ReaderPostListViewModelTest.kt index 6f5b6b89a5c3..cf63a5ad5357 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/reader/ReaderPostListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/reader/ReaderPostListViewModelTest.kt @@ -34,6 +34,7 @@ import org.wordpress.android.ui.reader.subfilter.SubfilterListItem import org.wordpress.android.ui.reader.subfilter.SubfilterListItem.SiteAll import org.wordpress.android.ui.reader.subfilter.SubfilterListItem.Tag import org.wordpress.android.ui.reader.subfilter.SubfilterListItemMapper +import org.wordpress.android.ui.reader.tracker.ReaderTracker import org.wordpress.android.ui.reader.viewmodels.ReaderPostListViewModel import org.wordpress.android.util.EventBusWrapper @@ -58,6 +59,7 @@ class ReaderPostListViewModelTest { @Mock private lateinit var appPrefsWrapper: AppPrefsWrapper @Mock private lateinit var subfilterListItemMapper: SubfilterListItemMapper @Mock private lateinit var eventBusWrapper: EventBusWrapper + @Mock private lateinit var readerTracker: ReaderTracker private lateinit var viewModel: ReaderPostListViewModel private val liveData = MutableLiveData() @@ -82,7 +84,8 @@ class ReaderPostListViewModelTest { TEST_DISPATCHER, appPrefsWrapper, subfilterListItemMapper, - eventBusWrapper + eventBusWrapper, + readerTracker ) val observable = viewModel.getNewsDataSource() observable.observeForever(observer) From 032b7acbdafb530d922c3643018e2a03f877ba33 Mon Sep 17 00:00:00 2001 From: develric Date: Sat, 15 Feb 2020 01:37:01 +0100 Subject: [PATCH 07/13] Additional lint fixing. --- .../java/org/wordpress/android/ui/reader/ReaderTrackerTest.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/ReaderTrackerTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/ReaderTrackerTest.kt index 93b5f08b0f51..3d030114907c 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/reader/ReaderTrackerTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/reader/ReaderTrackerTest.kt @@ -64,7 +64,6 @@ class ReaderTrackerTest { whenever(dateProvider.getCurrentDate()).thenReturn(addToDate(startPoint, Int.MAX_VALUE - 4)) tracker.stop(SUBFILTERED_LIST) - val expected = mapOf( "time_in_main_reader" to Int.MAX_VALUE - 1, "time_in_reader_filtered_list" to Int.MAX_VALUE - 2, @@ -167,7 +166,6 @@ class ReaderTrackerTest { assertThat(tracker.isRunning(MAIN_READER)).isEqualTo(false) } - @Test fun `tracker is running after start`() { tracker.setupTrackers() From 7369d1a3224faf9d1555984dff7d8d46eba10d1b Mon Sep 17 00:00:00 2001 From: develric Date: Wed, 19 Feb 2020 11:02:12 +0100 Subject: [PATCH 08/13] Correcting type and simplifying isRunning function. --- .../android/ui/reader/tracker/ReaderTracker.kt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTracker.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTracker.kt index 413d620da887..de2364a2ff98 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTracker.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/tracker/ReaderTracker.kt @@ -35,9 +35,9 @@ class ReaderTracker @Inject constructor(private val dateProvider: DateProvider) } fun stop(type: ReaderTrackerType) { - trackers[type]?.let { trackerInto -> - trackerInto.startDate?.let { startDate -> - val accumulatedTime = trackerInto.accumulatedTime + + trackers[type]?.let { trackerInfo -> + trackerInfo.startDate?.let { startDate -> + val accumulatedTime = trackerInfo.accumulatedTime + DateTimeUtils.secondsBetween(dateProvider.getCurrentDate(), startDate) // let reset the startDate to null trackers[type] = ReaderTrackerInfo(accumulatedTime = accumulatedTime) @@ -46,9 +46,7 @@ class ReaderTracker @Inject constructor(private val dateProvider: DateProvider) } fun isRunning(type: ReaderTrackerType): Boolean { - return trackers[type]?.let { trackerInto -> - trackerInto.startDate != null - } ?: false + return trackers[type]?.startDate != null } fun getAnalyticsData(): Map { From 56f01bceb31845b0c9a545c26dedf0a39fd75fc7 Mon Sep 17 00:00:00 2001 From: develric Date: Wed, 19 Feb 2020 11:16:49 +0100 Subject: [PATCH 09/13] Improving readability of checked conditions. --- .../viewmodels/ReaderPostListViewModel.kt | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt index 1695496d446b..e46420c10f40 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt @@ -201,17 +201,18 @@ class ReaderPostListViewModel @Inject constructor( } fun changeSubfiltersVisibility(show: Boolean) { + val isCurrentSubfilterTracked = getCurrentSubfilterValue().isTrackedItem + val isSubfilterListTrackerRunning = readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST) + if (show) { - if (getCurrentSubfilterValue().isTrackedItem && - !readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + if (isCurrentSubfilterTracked && !isSubfilterListTrackerRunning) { AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > START Count SUBFILTERED_LIST") readerTracker.start(ReaderTrackerType.SUBFILTERED_LIST) - } else if (!getCurrentSubfilterValue().isTrackedItem && - readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + } else if (!isCurrentSubfilterTracked && isSubfilterListTrackerRunning) { AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count SUBFILTERED_LIST") readerTracker.stop(ReaderTrackerType.SUBFILTERED_LIST) } - } else if (readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + } else if (isSubfilterListTrackerRunning) { AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count SUBFILTERED_LIST") readerTracker.stop(ReaderTrackerType.SUBFILTERED_LIST) } @@ -268,11 +269,13 @@ class ReaderPostListViewModel @Inject constructor( subfilterListItem: SubfilterListItem, requestNewerPosts: Boolean ) { - if (subfilterListItem.isTrackedItem && - !readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + val isSubfilterItemTracked = subfilterListItem.isTrackedItem + val isSubfilterListTrackerRunning = readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST) + + if (isSubfilterItemTracked && !isSubfilterListTrackerRunning) { AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > START Count SUBFILTERED_LIST") readerTracker.start(ReaderTrackerType.SUBFILTERED_LIST) - } else if (!subfilterListItem.isTrackedItem && readerTracker.isRunning(ReaderTrackerType.SUBFILTERED_LIST)) { + } else if (!isSubfilterItemTracked && isSubfilterListTrackerRunning) { AppLog.d(T.READER, "TRACK READER ReaderPostListFragment > STOP Count SUBFILTERED_LIST") readerTracker.stop(ReaderTrackerType.SUBFILTERED_LIST) } From 09f76fb894bbe9e6dc16f762f0d44e01d16c4ae8 Mon Sep 17 00:00:00 2001 From: develric Date: Wed, 19 Feb 2020 11:22:53 +0100 Subject: [PATCH 10/13] Renaming method from onSubfilterChanged to onSubfilterSelected. --- .../org/wordpress/android/ui/reader/ReaderPostListFragment.java | 2 +- .../android/ui/reader/viewmodels/ReaderPostListViewModel.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 aea05ee81881..8afe6c267561 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 @@ -402,7 +402,7 @@ public void onCreate(Bundle savedInstanceState) { BuildConfig.INFORMATION_ARCHITECTURE_AVAILABLE && mIsTopLevel, mRecyclerView ) || ReaderUtils.isDefaultTag(mCurrentTag)) && getPostListType() != ReaderPostListType.SEARCH_RESULTS) { - mViewModel.onSubfilterChanged(subfilterListItem); + mViewModel.onSubfilterSelected(subfilterListItem); } }); diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt index e46420c10f40..3e57f5cfd9b9 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt @@ -327,7 +327,7 @@ class ReaderPostListViewModel @Inject constructor( isFirstLoad = false } - fun onSubfilterChanged(subfilterListItem: SubfilterListItem) { + fun onSubfilterSelected(subfilterListItem: SubfilterListItem) { changeSubfilter(subfilterListItem, true) } From a2aea53df5c76409f454e3a7f06c035b9fda5bdd Mon Sep 17 00:00:00 2001 From: develric Date: Wed, 19 Feb 2020 12:07:55 +0100 Subject: [PATCH 11/13] Fixing method that was changed. --- .../org/wordpress/android/ui/reader/ReaderPostListFragment.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 863087e83855..321d42ae9e7c 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 @@ -606,7 +606,7 @@ && isCurrentTagManagedInFollowingTab() showEmptyView(); } - mViewModel.onFragmentResume(mIsTopLevel, ReaderUtils.isFollowing(getCurrentTag(), mIsTopLevel, mRecyclerView)); + mViewModel.onFragmentResume(mIsTopLevel, isCurrentTagManagedInFollowingTab()); } /* From 9202fcd5c7007a8f8e26f42b43a8679d95f9dd8c Mon Sep 17 00:00:00 2001 From: develric Date: Wed, 19 Feb 2020 12:22:59 +0100 Subject: [PATCH 12/13] Fixing Lint. --- .../android/viewmodel/reader/ReaderPostListViewModelTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/reader/ReaderPostListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/reader/ReaderPostListViewModelTest.kt index 093764e0cde5..0c3f2a0c8bbe 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/reader/ReaderPostListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/reader/ReaderPostListViewModelTest.kt @@ -94,7 +94,7 @@ class ReaderPostListViewModelTest { subfilterListItemMapper, eventBusWrapper, accountStore, - readerTracker + readerTracker ) val observable = viewModel.getNewsDataSource() observable.observeForever(observer) From ac146f1ff94197769160de6644e4c2a3d16de2c6 Mon Sep 17 00:00:00 2001 From: develric Date: Wed, 19 Feb 2020 12:32:41 +0100 Subject: [PATCH 13/13] Renaming method from manageSubfilter to onSubfilterReselected. --- .../wordpress/android/ui/reader/ReaderPostListFragment.java | 4 ++-- .../android/ui/reader/viewmodels/ReaderPostListViewModel.kt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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 321d42ae9e7c..e24bdbe3c7e0 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 @@ -1145,7 +1145,7 @@ public boolean onMenuItemActionCollapse(MenuItem item) { if (BuildConfig.INFORMATION_ARCHITECTURE_AVAILABLE && mIsTopLevel) { if (isCurrentTagManagedInFollowingTab()) { - mViewModel.manageSubfilter(); + mViewModel.onSubfilterReselected(); } else { // return to the followed tag that was showing prior to searching resetPostAdapter(ReaderPostListType.TAG_FOLLOWED); @@ -2188,7 +2188,7 @@ && getPostAdapter().isCurrentTag(tag)) { if (BuildConfig.INFORMATION_ARCHITECTURE_AVAILABLE && manageSubfilter) { if (mCurrentTag.isFollowedSites() || mCurrentTag.isDefaultInMemoryTag()) { - mViewModel.manageSubfilter(); + mViewModel.onSubfilterReselected(); } else { changeReaderMode(new ReaderModeInfo( tag, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt index edba266f6aac..d709b395bad1 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderPostListViewModel.kt @@ -336,7 +336,7 @@ class ReaderPostListViewModel @Inject constructor( changeSubfilter(subfilterListItem, true) } - fun manageSubfilter() { + fun onSubfilterReselected() { changeSubfilter(getCurrentSubfilterValue(), false) }