Settings Sync: Set Modified Date on settings import #1616

bjtitus · 2024-04-12T02:38:41Z

Depends On	#1615

Set the Modified Date on all imported settings to 1 second after the epoch date.

This ensures that imported settings are sent to the server (any settings without a date are not).
By defaulting to 1 second after, the setting is stored by the server. A default epoch date is effectively a "null" value and will not replace the default value.

To test

Change the return value of shouldEnableSyncedSettings in FeatureFlag.swift to false
Build and run
Log in to an account which hasn't used synced settings
Change a few settings
Enable the newSettingsStorage and settingsSync feature flags
Restart the app
Pull to refresh or tap "Refresh Now" in profile
Ensure changed settings remain
Delete the app
Change shouldEnableSyncedSettings in FeatureFlag.swift back to its original implementation or set to true
Build and run
Log in to the same account from above
Verify that the changed settings are the same as when changed above

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.

# Conflicts: # podcasts/DataManager+Import.swift

dangermattic · 2024-04-12T02:39:06Z

	1 Warning
⚠️	This PR is assigned to the milestone 7.63. The due date for this milestone has already passed. Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

This ensures it doesn't match the server value, which would end up overwriting it.

SergioEstevao · 2024-04-12T14:05:55Z

podcasts/AppSettings+ImportUserDefaults.swift

-        self.update(\.$autoPlayEnabled, value: UserDefaults.standard.bool(forKey: Constants.UserDefaults.autoplay))
-        self.update(\.$notifications, value: UserDefaults.standard.bool(forKey: Constants.UserDefaults.pushEnabled))
-        self.update(\.$appBadge, value: Int32(UserDefaults.standard.integer(forKey: Constants.UserDefaults.appBadge)))
+        let date = Date(timeIntervalSince1970: 1)


Maybe Date.distantPast will be more clear here.

@SergioEstevao I don't think we can use that as per:

By defaulting to 1 second after, the setting is stored by the server. A default epoch date is effectively a "null" value and will not replace the default value.

This would probably result in the setting being discarded by the server.

So any reference date will be discarded is this what this means: A default epoch date ?

SergioEstevao · 2024-04-12T14:06:34Z

podcasts/DataManager+Import.swift

@@ -5,37 +5,38 @@ extension DataManager {
    func importPodcastSettings() {
        let podcasts = allPodcasts(includeUnsubscribed: true)

+        let date = Date(timeIntervalSince1970: 1)


Same as above: Date.distantPast

SergioEstevao

Working good, just a small comment about the way the date is set.

leandroalonso

@bjtitus this is really not working for me. I'm gonna send you a video in a DM.

MiSikora · 2024-04-15T09:09:14Z

podcasts/AppSettings+ImportUserDefaults.swift

-        self.update(\.$autoPlayEnabled, value: UserDefaults.standard.bool(forKey: Constants.UserDefaults.autoplay))
-        self.update(\.$notifications, value: UserDefaults.standard.bool(forKey: Constants.UserDefaults.pushEnabled))
-        self.update(\.$appBadge, value: Int32(UserDefaults.standard.integer(forKey: Constants.UserDefaults.appBadge)))
+        let date = Date(timeIntervalSince1970: 1)


Just double checking if this could be changed to 1 millisecond or should we adjust on Android to 1 second?

I'll get this updated today. I still need to double check the comparisons to make sure the precision is working.

MiSikora · 2024-04-15T09:10:54Z

podcasts/DataManager+Import.swift

@@ -5,40 +5,42 @@ extension DataManager {
    func importPodcastSettings() {
        let podcasts = allPodcasts(includeUnsubscribed: true)

+        let date = Date(timeIntervalSince1970: 1)


Same as #1616 (comment)

leandroalonso

@bjtitus the Up Next setting is now persisting correctly, but I still have issues with the Settings under Appearance.

bjtitus added 2 commits April 11, 2024 20:36

Add default modifiedAt date when importing

b9d897e

# Conflicts: # podcasts/DataManager+Import.swift

Add modifiedAt check to settings import unit tests

dceed67

bjtitus added the settings Issues related to user or podcast settings label Apr 12, 2024

bjtitus added this to the 7.62 milestone Apr 12, 2024

Change default import date to 1 second after epoch

b295a66

This ensures it doesn't match the server value, which would end up overwriting it.

bjtitus marked this pull request as ready for review April 12, 2024 02:57

bjtitus requested a review from a team as a code owner April 12, 2024 02:57

bjtitus requested review from leandroalonso and removed request for a team April 12, 2024 02:57

bjtitus mentioned this pull request Apr 12, 2024

Settings Sync: Fix Up Next settings #1615

Open

3 tasks

SergioEstevao requested review from SergioEstevao and removed request for leandroalonso April 12, 2024 14:04

SergioEstevao reviewed Apr 12, 2024

View reviewed changes

SergioEstevao self-requested a review April 12, 2024 14:46

SergioEstevao approved these changes Apr 12, 2024

View reviewed changes

leandroalonso requested changes Apr 12, 2024

View reviewed changes

bjtitus added 2 commits April 12, 2024 13:05

Update process settings on full sync

bcd0584

Set podcast syncStatus after importing

7b94def

bjtitus requested a review from leandroalonso April 12, 2024 20:06

MiSikora reviewed Apr 15, 2024

View reviewed changes

bjtitus added 2 commits April 15, 2024 09:40

Sync date extension on Date 1 millisecond after epoch

29bcd4e

Adjust unit tests to check for syncDefaultDate

8a8beff

leandroalonso requested changes Apr 15, 2024

View reviewed changes

bjtitus added 2 commits April 15, 2024 11:34

Switch default import date to 1 second after 1970

13cbfa0

Use proper default epoch date to match server

bd012d4

SergioEstevao modified the milestones: 7.62, 7.63 Apr 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Settings Sync: Set Modified Date on settings import #1616

Settings Sync: Set Modified Date on settings import #1616

bjtitus commented Apr 12, 2024 •

edited

dangermattic commented Apr 12, 2024 •

edited

SergioEstevao Apr 12, 2024

leandroalonso Apr 12, 2024

SergioEstevao Apr 15, 2024

SergioEstevao Apr 12, 2024

SergioEstevao left a comment

leandroalonso left a comment

MiSikora Apr 15, 2024

bjtitus Apr 15, 2024

MiSikora Apr 15, 2024 •

edited

leandroalonso left a comment

Settings Sync: Set Modified Date on settings import #1616

Are you sure you want to change the base?

Settings Sync: Set Modified Date on settings import #1616

Conversation

bjtitus commented Apr 12, 2024 • edited

To test

Checklist

dangermattic commented Apr 12, 2024 • edited

SergioEstevao Apr 12, 2024

Choose a reason for hiding this comment

leandroalonso Apr 12, 2024

Choose a reason for hiding this comment

SergioEstevao Apr 15, 2024

Choose a reason for hiding this comment

SergioEstevao Apr 12, 2024

Choose a reason for hiding this comment

SergioEstevao left a comment

Choose a reason for hiding this comment

leandroalonso left a comment

Choose a reason for hiding this comment

MiSikora Apr 15, 2024

Choose a reason for hiding this comment

bjtitus Apr 15, 2024

Choose a reason for hiding this comment

MiSikora Apr 15, 2024 • edited

Choose a reason for hiding this comment

leandroalonso left a comment

Choose a reason for hiding this comment

bjtitus commented Apr 12, 2024 •

edited

dangermattic commented Apr 12, 2024 •

edited

MiSikora Apr 15, 2024 •

edited