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

Settings Sync - Make podcast display settings into UserSettings #1256

Merged

Conversation

mchowning
Copy link
Contributor

Description

This converts the podcasts display settings to be UserSettings.

In addition, I made some minor changes to how we track whether a setting needs to be synced.

Testing Instructions

1. Verify settings work

  1. On a fresh install of the app
  2. Subscribe to some podcasts
  3. go to the podcasts screen and tap the three dot overflow menu
  4. verify the following defaults:
    1. Sort by: Date added
    2. Layout: large grid
    3. badges: off
  5. On the podcasts tab, change the sort order, layout, and badges setting
  6. Kill the app
  7. Restart the app
  8. verify that the settings persisted and are reflected in the UI

2. Check for sync regressions

Because I did some minor refactoring to how syncing is done, it would be good to retest that syncing setting to an account still works:

  1. Fresh install the app
  2. Log into an account
  3. Change the skip forward and skip backward settings to new values
  4. Sync the app
  5. uninstall the app
  6. reinstall the app and log into the account
  7. Once the account has synced, verify that the updated setting values are shown

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • Any jetpack compose components I added or changed are covered by compose previews

@mchowning mchowning requested a review from a team as a code owner August 8, 2023 20:34
Copy link
Member

@geekygecko geekygecko left a comment

Choose a reason for hiding this comment

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

Using the types looks excellent. The changes worked well for me.

@mchowning mchowning merged commit 2c8cbb1 into feature/sync-settings-refactor Aug 9, 2023
@mchowning mchowning deleted the update/refactor-settings_grid branch August 9, 2023 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants