Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watch sync improvements #2425

Merged
merged 6 commits into from
Nov 15, 2024
Merged

Conversation

SergioEstevao
Copy link
Contributor

| 📘 Part of: # |
|:---:|

Fixes #

Optimize sync process to be faster and reduce the maximum number of episodes synced on a partial refresh.

To test

  1. Start the app on the watch simulator connected to the phone using the Pocket Casts Watch scheme
  2. Add some episodes to UpNext on the phone
  3. Check on the phone source on the watch if they show correctly
  4. Move around the episode on the phone ( Mark as Played, Archive, Remove)
  5. Check if on the watch things show correctly

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.

@SergioEstevao SergioEstevao added the [Area] watchOS Issus related to watchOS label Nov 14, 2024
@SergioEstevao SergioEstevao added this to the 7.77 ❄️ milestone Nov 14, 2024
@SergioEstevao SergioEstevao requested a review from a team as a code owner November 14, 2024 20:59
@SergioEstevao SergioEstevao requested review from danielebogo and removed request for a team November 14, 2024 20:59
}

let ids = episodes.map({"\($0.id)"})
try db.executeUpdate("UPDATE \(DataManager.episodeTableName) SET playingStatusModified = 0, playedUpToModified = 0, durationModified = 0, keepEpisodeModified = 0, archivedModified = 0 WHERE id IN (\(ids.joined(separator: ",")))", values: nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we feature flag this change? I know we've had performance issues with filters when doing queries with long lists so I don't know if that might be an issue here. Maybe less likely because those were issues on the main thread.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here: c62407f

Copy link
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I played with few episodes and it seems to work. Restarting the app kept all the changes in place. I didn't notice any slow down for now but my db is not that big.
I left few comments

@@ -728,16 +728,15 @@ class EpisodeDataManager {
}

func markAllSynced(episodes: [Episode], dbQueue: FMDatabaseQueue) {
if episodes.count == 0 { return }
guard !episodes.isEmpty else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe if episodes.isEmpty is more clear than double negative

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done here: 4f456c8

}

let ids = episodes.map({"\($0.id)"})
try db.executeUpdate("UPDATE \(DataManager.episodeTableName) SET playingStatusModified = 0, playedUpToModified = 0, durationModified = 0, keepEpisodeModified = 0, archivedModified = 0 WHERE id IN (\(ids.joined(separator: ",")))", values: nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@SergioEstevao SergioEstevao changed the title Fix watch sync improvements Watch sync improvements Nov 15, 2024
@SergioEstevao
Copy link
Contributor Author

@danielebogo and @bjtitus ready for another look

# Conflicts:
#	Modules/Utils/Sources/PocketCastsUtils/Feature Flags/FeatureFlag.swift
Copy link
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@SergioEstevao SergioEstevao merged commit a1218ea into release/7.77 Nov 15, 2024
4 of 6 checks passed
@SergioEstevao SergioEstevao deleted the fix/watch_sync_improvements branch November 15, 2024 15:10
@pachlava pachlava added the [Type] Enhancement Improve an existing feature. label Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Area] watchOS Issus related to watchOS [Type] Enhancement Improve an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants