On Automotive add the option to clear your data before logging in #978
Conversation
| */ | ||
| private fun isAutoDownloadingAllPodcasts(): Boolean { | ||
| return if (countPodcasts() == 0) false else podcastDao.countDownloadStatus(Podcast.AUTO_DOWNLOAD_NEW_EPISODES) == podcastDao.countSubscribed() | ||
| } |
There was a problem hiding this comment.
These methods weren't been used
| // if the episode is deleted from the database while playing catch the error and just return an empty state | ||
| .onErrorReturn { Optional.empty() } | ||
| .toObservable() | ||
| } |
There was a problem hiding this comment.
As part of clearing the data when the episode was deleted, it caused the Up Next and playback event monitoring to crash. This meant that after clearing the data it wasn't possible to play another episode. This change fixes that.
...s/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/user/UserManager.kt
Outdated
Show resolved
Hide resolved
| wasInitiatedByUser: Boolean, | ||
| ) { | ||
| // Sign out first to make sure no data changes get synced | ||
| signOut(playbackManager, wasInitiatedByUser = true) |
There was a problem hiding this comment.
| signOut(playbackManager, wasInitiatedByUser = true) | |
| signOut(playbackManager, wasInitiatedByUser = wasInitiatedByUser) |
Alternatively, if we want this to always be true, we could remove the wasInitiatedByUser parameter from being passed in signOutAndClearData.
| folderManager = folderManager, | ||
| searchHistoryManager = searchHistoryManager, | ||
| episodeManager = episodeManager, | ||
| wasInitiatedByUser = true |
There was a problem hiding this comment.
I wonder if wasInitiatedByUser should be false here. The user is initiating the action here, but they're not trying to sign out. One of the things this wasInitiatedByUser variable is used for is to identify in our analytics whether the user was attempting to sign out.
Admittedly this is a bit of a moot point at this time since we're not sending any analytics to Tracks from Android Automotive yet.
Also, if you change wasInitiatedByUser to be false, you may need to do something like add a call to settings.setFullySignedOut(false) to get past this if-check in the signOut method.
The settings.getFullySIgnedOut value is a relatively recent addition I did largely to avoid extra signout calls to analytics (we were making a call to signout every time a user who was not signed in started the app). Maybe you have some ideas for how to better accomplish that because it does seem to be creating some undesirable complexity here. 🤔
There was a problem hiding this comment.
I wonder if it would be easier to understand if we named the parameter something like userAttemptingToSignOut. 🤔
I was trying to reuse the same logic but I think I have made it more complicated. Thanks for picking this up, I have committed a fix to set wasInitiatedByUser to false.
There was a problem hiding this comment.
I wonder if it would be easier to understand if we named the parameter something like
userAttemptingToSignOut. 🤔
If that seems like a better name to you I'm certainly not opposed to switching to that.
There was a problem hiding this comment.
Just wanted to note explicitly that with this change none of this code is getting run when the user clears data on sign-in:
if (wasInitiatedByUser || !settings.getFullySignedOut()) {
LogBuffer.i(LogBuffer.TAG_BACKGROUND_TASKS, "Signing out")
subscriptionManager.clearCachedStatus()
syncManager.signOut {
settings.clearPlusPreferences()
GlobalScope.launch {
userEpisodeManager.removeCloudStatusFromFiles(playbackManager)
}
settings.setMarketingOptIn(false)
settings.setMarketingOptInNeedsSync(false)
settings.setEndOfYearModalHasBeenShown(false)
analyticsTracker.track(
AnalyticsEvent.USER_SIGNED_OUT,
mapOf(KEY_USER_INITIATED to wasInitiatedByUser)
)
analyticsTracker.flush()
analyticsTracker.clearAllData()
analyticsTracker.refreshMetadata()
}
}
I think that's probably fine because all those things seem to relate to accounts and obviously the user isn't signed-in in this scenario, but it's worth keeping in mind if we see any issues.
…/pocketcasts/repositories/user/UserManager.kt Co-authored-by: Matt Chowning <mchowning@gmail.com>
Description
For the Automotive app, we have been asked if we can add the option to clear your data before you can log in or create an account. If you have an Up Next, any podcasts or episodes then it will ask you, otherwise the dialog won't be shown to you.
Testing Instructions
Screenshots