End of Year: sync Listening History #483

leandroalonso · 2022-11-04T14:49:30Z

📘 Project: #376

If we identify the user doesn't have the entire 2022 listening history, we sync this with the server adding the missing episodes.

This is a case that might happen for a very active user that switched devices during this year — given we only sync the latest 100 items.

How do we identify if a user needs a sync?

~~We look at their listening history. If they have an episode from last year, we assume that their 2022 listening history is complete.~~

A much more bulletproof strategy would be to call the endpoint to check the number of items for the user, and sync if needed, just once. But I'm not sure what would be the impact of such a call on the backend (@Ferdev curious to hear your thoughts about it).

We request the number of items the server has. If the returned number is smaller than what we have locally, we sync the necessary items.

To test

You'll need an account with more than 100 items. Please ping me and I can provide one.

Enable the endOfYear flag in FeatureFlag.swift
Do a clean run of the app in Staging mode (Schemes > pocketcasts > change "Build Configuration" to Staging)
Login with an account with more than 100 items
Check your listening history, you should see just ~40 items
Put a breakpoint on EndOfYearStoriesBuilder.swift line 39
Check your EoY stories
✅ The hitpoint should be hit, proceed with execution
✅ You should see the spinner for a few seconds
✅ The stories should appear
Dismiss the stories
Check your Listening History
✅ You should have more than 100 items there now (the oldest episode is an archived episode called "39. Mother"
Check your stories again
✅ The breakpoint should not be hit (2022 sync happens just once)

Checklist

I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
I have considered adding unit tests for my changes.
I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

To grab missing episodes

… than 5 mins There are a lot of podcasts episodes with less than 30 minutes and we don't want to leave these users out

The only rule will look at is if there's any podcast from more than a year ago

leandroalonso · 2022-11-04T20:32:30Z

Modules/DataModel/Sources/PocketCastsDataModel/Private/Managers/EndOfYearDataManager.swift

@@ -20,7 +20,7 @@ class EndOfYearDataManager {
                let query = """
                            SELECT playedUpTo from \(DataManager.episodeTableName)
                            WHERE
-                            playedUpTo > 1800 AND
+                            playedUpTo > 300 AND


If a user had listened to multiple episodes with less than 30 minutes they were not eligible, I changed that to just 5 minutes.

leandroalonso · 2022-11-04T20:33:20Z

Modules/Server/Sources/PocketCastsServer/Public/Sync/SyncYearListeningHistoryTask.swift

+
+/// Helper that checks for podcast existence
+/// It caches database requests
+class PodcastExistHelper {


I created this to avoid calling the database over and over to check if an episode exists.

Ferdev · 2022-11-07T08:14:44Z

Hey @leandroalonso!

A much more bulletproof strategy would be to call the endpoint to check the number of items for the user, and sync if needed, just once. But I'm not sure what would be the impact of such a call on the backend (@Ferdev curious to hear your thoughts about it)

I don't think it would be a problem if we just call it once. The database query is pretty simple and the whole request doesn't execute any crazy calculations, so we should be fine if we just run it one time 👍 .

emilylaguna

Heya! This worked well in my testing. I left some comments, let me know what you think.

emilylaguna · 2022-11-07T17:26:33Z

Modules/Server/Sources/PocketCastsServer/Public/Sync/SyncYearListeningHistoryTask.swift

+        var dataToSync = Api_YearHistoryRequest()
+        dataToSync.deviceTime = TimeFormatter.currentUTCTimeInMillis()
+        dataToSync.version = apiVersion
+        dataToSync.year = 2022


What do you think about allowing this to be injected?

Done in 9001df4

emilylaguna · 2022-11-07T17:35:41Z

Modules/Server/Sources/PocketCastsServer/Public/Sync/SyncYearListeningHistoryTask.swift

+        dispatchGroup.wait()
+
+        // Sync episode status for the retrieved podcasts' episodes
+        let uniqueUuidsToUpdate = Array(Set(podcastsToUpdate))


Instead of converting the array -> set -> array. What do you think about just changing podcastsToUpdate to a Set?

var podcastsToUpdate: Set<String> = []

Done in fecb93b

emilylaguna · 2022-11-07T17:39:13Z

Modules/Server/Sources/PocketCastsServer/Public/Sync/SyncYearListeningHistoryTask.swift

+    func markAsExistent(uuid: String) {
+        checkedUuidsThatExist.append(uuid)
+    }


This doesn't seem to be used, should we remove it?

Done in 194ddec

emilylaguna · 2022-11-07T17:40:02Z

Modules/Server/Sources/PocketCastsServer/Public/Sync/SyncYearListeningHistoryTask.swift

+
+/// Helper that checks for podcast existence
+/// It caches database requests
+class PodcastExistHelper {


Nit:

Suggested change

class PodcastExistHelper {

class PodcastExistsHelper {

Done in 21f03cb

emilylaguna · 2022-11-07T17:45:43Z

Modules/Server/Sources/PocketCastsServer/Public/Sync/SyncYearListeningHistoryTask.swift

+        do {
+            let data = try dataToSync.serializedData()
+            let (response, httpStatus) = postToServer(url: url, token: token, data: data)
+            if let response = response, httpStatus == ServerConstants.HttpConstants.ok {


Nit:

Suggested change

if let response = response, httpStatus == ServerConstants.HttpConstants.ok {

if let response, httpStatus == ServerConstants.HttpConstants.ok {

Done in fdd751b

leandroalonso added 5 commits November 4, 2022 09:21

Update proto with EoY Listening History

10e75e8

Add a method to return the number of episodes we have for 2022

7d662f2

Expose number of episodes

db3ea50

Add SyncYearListeningHistory

2968d29

To grab missing episodes

Pass the sync method in the constructor

e58af03

leandroalonso added the [Project] End of Year 2023 End of Year project label Nov 4, 2022

leandroalonso added this to the Future milestone Nov 4, 2022

leandroalonso added 9 commits November 4, 2022 14:08

User is eligible for end of year if any episode was listened for more…

302e2b2

… than 5 mins There are a lot of podcasts episodes with less than 30 minutes and we don't want to leave these users out

Add a method to add the podcast and episode

b00dbce

Update all the missing episodes

1917700

Add a flag to store if the whole 2022 listening history has been synced

aab2aad

Sync history only if it hasn't been done before and strictly necessary

b511065

Remove comment

8c50454

Rename file to keep consistency

0e54b2f

Remove the count of items to define full history or not

e8cd0d0

The only rule will look at is if there's any podcast from more than a year ago

Update the episodes in parallel, making it way quicker

617274c

leandroalonso commented Nov 4, 2022

View reviewed changes

leandroalonso marked this pull request as ready for review November 4, 2022 20:33

leandroalonso requested a review from a team as a code owner November 4, 2022 20:34

leandroalonso mentioned this pull request Nov 4, 2022

Project: End of Year #376

Closed

36 tasks

leandroalonso added 2 commits November 4, 2022 17:52

Fix tests

fd24a19

Fix unit tests

6f55edf

leandroalonso added 4 commits November 7, 2022 08:58

Check once, and once only, if history is synced

f7e4311

Merge branch 'trunk' into task/376-sync-listening-history

532cd8d

Remove unnecessary async block

10cf185

Fix tests and remove an out-dated one

b56861a

emilylaguna suggested changes Nov 7, 2022

View reviewed changes

Added year to the constructor

9001df4

leandroalonso added 4 commits November 7, 2022 15:19

Use Set instead of array and conversion to Set

fecb93b

Remove unused method

194ddec

Rename PodcastExistHelper to PodcastExistsHelper

21f03cb

Use the new let syntax

fdd751b

leandroalonso requested a review from emilylaguna November 7, 2022 18:26

emilylaguna approved these changes Nov 7, 2022

View reviewed changes

leandroalonso merged commit 8d8bf5d into trunk Nov 7, 2022

leandroalonso deleted the task/376-sync-listening-history branch November 7, 2022 19:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

End of Year: sync Listening History #483

End of Year: sync Listening History #483

leandroalonso commented Nov 4, 2022 •

edited

Loading

leandroalonso Nov 4, 2022

leandroalonso Nov 4, 2022

Ferdev commented Nov 7, 2022

emilylaguna left a comment

emilylaguna Nov 7, 2022

leandroalonso Nov 7, 2022

emilylaguna Nov 7, 2022

leandroalonso Nov 7, 2022

emilylaguna Nov 7, 2022

leandroalonso Nov 7, 2022

emilylaguna Nov 7, 2022

leandroalonso Nov 7, 2022

emilylaguna Nov 7, 2022

leandroalonso Nov 7, 2022

	if let response = response, httpStatus == ServerConstants.HttpConstants.ok {
	if let response, httpStatus == ServerConstants.HttpConstants.ok {

End of Year: sync Listening History #483

End of Year: sync Listening History #483

Conversation

leandroalonso commented Nov 4, 2022 • edited Loading

To test

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ferdev commented Nov 7, 2022

emilylaguna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leandroalonso commented Nov 4, 2022 •

edited

Loading