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 - convert remaining settings to UserSettings #1271

Conversation

mchowning
Copy link
Contributor

@mchowning mchowning commented Aug 11, 2023

Description

This makes the automotive settings for subscribing to played podcasts, showing played podcasts, and the setting for the last played list into UserSettings.

In addition, this fixes a bug whereby auto play on automotive was not updating the autoPlay user setting. It was just updating the persisted setting directly, which meant the UserSetting's flow was not updated and would not reflect the new value until the app was restarted. I introduced this bug when I refactored the auto play setting to be a UserSetting (#1213), so it never got released.

Testing Instructions

1. Subscribing to played podcasts

On the automotive app, ensure that toggling the subscribe to played podcast setting

  • is defaulted to true
  • controls whether played podcasts get automatically subscribed to
  • is persisted between restarts of the app

2. Showing played podcasts

On the automotive app, ensure that togglign the show played episodes setting

  • is defaulted to false
  • controls whether played episodes are shown when viewing a podcast
  • is persisted between restarts of the app

3. Auto play (implicitly testing the last-played-list setting)

On both the automotive and phone apps, ensure that when playing a podcast episode with remaining unplayed episodes, completing that episode will:

  • if the auto play setting is on, continue playing episodes from that podcast
  • if the auto play setting is off, stop playing episodes from that podcast

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

When refactoring autoplay to be a UserSetting, I failed to make sure we
were directly updating the UserSetting here. As a result the
SwitchPreference was updating the persisted setting directly, which
meant that the UserSetting's flow never got updated.
@mchowning mchowning requested a review from a team as a code owner August 11, 2023 02:37
@geekygecko geekygecko merged commit b469e40 into feature/sync-settings-refactor Aug 15, 2023
@geekygecko geekygecko deleted the update/user-settings_additional-items branch August 15, 2023 07:53
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