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

Fix: Disabled global Auto download new episodes setting overrides podcast specifc one #3342

Merged
merged 4 commits into from
Dec 11, 2024

Conversation

mebarbosa
Copy link
Contributor

@mebarbosa mebarbosa commented Dec 9, 2024

Description

  • Auto download screen was updating podcast auto download setting when this screen was opened

Fixes #3338

Testing Instructions

Screenshots or Screencast

Screen_recording_20241209_130121.webm

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
  • ~ I have updated (or requested that someone edit) [the spreadsheet]~(https://docs.google.com/spreadsheets/d/107jqrutZhU0fVZJ19SBqxxVKbV2NWSdQC9MFYdLiAxc/edit?usp=sharing) to reflect any new or changed analytics.

@mebarbosa mebarbosa added this to the 7.80 milestone Dec 9, 2024
@mebarbosa mebarbosa requested a review from a team as a code owner December 9, 2024 16:19
@mebarbosa mebarbosa requested review from MiSikora and removed request for a team December 9, 2024 16:19
Comment on lines 419 to 427
viewModel.countPodcastsAutoDownloading()
.map { it > 0 }
.subscribeBy(
onError = { Timber.e(it) },
onSuccess = { on ->
onNewEpisodesToggleChange(on.toAutoDownloadStatus())
newEpisodesPreference?.isChecked = on
},
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 9, 2024

📲 You can test the changes from this Pull Request in 📱 Mobile by scanning the QR code below to install the corresponding build.
App Name 📱 Mobile
Build TypedebugProd
Commit71faf58
Direct Downloadpocketcasts-app-prototype-build-pr3342-71faf58.apk
📲 You can test the changes from this Pull Request in 🚗 Automotive by scanning the QR code below to install the corresponding build.
App Name 🚗 Automotive
Build TypedebugProd
Commit71faf58
Direct Downloadpocketcasts-automotive-prototype-build-pr3342-71faf58.apk
📲 You can test the changes from this Pull Request in ⌚ Wear by scanning the QR code below to install the corresponding build.
App Name ⌚ Wear
Build TypedebugProd
Commit71faf58
Direct Downloadpocketcasts-wear-prototype-build-pr3342-71faf58.apk

Comment on lines +130 to 136
fun countPodcastsAutoDownloading(): Single<Int> = podcastManager.countDownloadStatusRxSingle(Podcast.AUTO_DOWNLOAD_NEW_EPISODES)
.subscribeOn(Schedulers.io())

fun countPodcasts(): Single<Int> = podcastManager.countSubscribedRxSingle()
.observeOn(AndroidSchedulers.mainThread())
.subscribeOn(Schedulers.io())
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minor refactor. Moved from fragment to viewmodel

Copy link
Contributor

@MiSikora MiSikora left a comment

Choose a reason for hiding this comment

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

App crashes when I execute this test:

  1. Navigate to the Profile tab.
  2. Open Settings.
  3. Go to the Auto Download section.
  4. Ensure the New Episodes toggle is off.
  5. Navigate to the settings of any followed podcast.
  6. Disable the Auto Download setting.
  7. Tap on the Podcasts tab.
  8. Select any podcast.
  9. Open its settings.
  10. Enable the Auto Download setting.
  11. Navigate to the Profile tab.
  12. 💥
java.lang.IllegalStateException: Cannot call this method while RecyclerView is computing a layout or scrolling androidx.recyclerview.widget.RecyclerView{626f1d9 VFED.V... ......ID 0,0-1080,1915 #7f0a04b4 app:id/recycler_view}, adapter:androidx.preference.PreferenceGroupAdapter@82d989f, layout:androidx.recyclerview.widget.LinearLayoutManager@5703faa, context:dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper@83664c3
	at androidx.recyclerview.widget.RecyclerView.assertNotInLayoutOrScroll(RecyclerView.java:3482)
	at androidx.recyclerview.widget.RecyclerView$RecyclerViewDataObserver.onItemRangeChanged(RecyclerView.java:6071)
	at androidx.recyclerview.widget.RecyclerView$AdapterDataObservable.notifyItemRangeChanged(RecyclerView.java:13219)
	at androidx.recyclerview.widget.RecyclerView$Adapter.notifyItemChanged(RecyclerView.java:8136)
	at androidx.preference.PreferenceGroupAdapter.onPreferenceChange(PreferenceGroupAdapter.java:354)
	at androidx.preference.Preference.notifyChanged(Preference.java:1294)
	at androidx.preference.TwoStatePreference.setChecked(TwoStatePreference.java:99)
	at au.com.shiftyjelly.pocketcasts.settings.AutoDownloadSettingsFragment.setupNewEpisodesToggleStatusCheck$lambda$33(AutoDownloadSettingsFragment.kt:425)
	at au.com.shiftyjelly.pocketcasts.settings.AutoDownloadSettingsFragment.$r8$lambda$U6792c7lbcG7qUaEN7BLPQFH6fM(Unknown Source:0)
	at au.com.shiftyjelly.pocketcasts.settings.AutoDownloadSettingsFragment$$ExternalSyntheticLambda11.invoke(D8$$SyntheticClass:0)
	at io.reactivex.rxkotlin.SubscribersKt$sam$io_reactivex_functions_Consumer$0.accept(Unknown Source:2)
	at io.reactivex.internal.observers.ConsumerSingleObserver.onSuccess(ConsumerSingleObserver.java:62)
	at io.reactivex.internal.operators.single.SingleMap$MapSingleObserver.onSuccess(SingleMap.java:64)
	at io.reactivex.internal.operators.single.SingleSubscribeOn$SubscribeOnObserver.onSuccess(SingleSubscribeOn.java:68)
	at io.reactivex.internal.operators.single.SingleFromCallable.subscribeActual(SingleFromCallable.java:56)
	at io.reactivex.Single.subscribe(Single.java:3666)
	at io.reactivex.internal.operators.single.SingleSubscribeOn$SubscribeOnObserver.run(SingleSubscribeOn.java:89)
	at io.reactivex.Scheduler$DisposeTask.run(Scheduler.java:608)
	at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:66)
	at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:57)
	at java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:307)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:644)
	at java.lang.Thread.run(Thread.java:1012)

@mebarbosa mebarbosa requested a review from MiSikora December 10, 2024 17:51
@mebarbosa
Copy link
Contributor Author

@MiSikora I've fixed this issue and tested using a real device this time. For some reason, I could not reproduce this issue you found using the emulator, which does not make any sense.

Copy link
Contributor

@MiSikora MiSikora left a comment

Choose a reason for hiding this comment

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

The change looks good. Would you mind adding a bug fix entry to the changelog?

For some reason, I could not reproduce this issue you found using the emulator, which does not make any sense.

I don't understand it as well. It crashes when I test it on an emulator when I test with the previous commit.

@mebarbosa
Copy link
Contributor Author

The change looks good. Would you mind adding a bug fix entry to the changelog?

For some reason, I could not reproduce this issue you found using the emulator, which does not make any sense.

I don't understand it as well. It crashes when I test it on an emulator when I test with the previous commit.

@MiSikora I bet I am have some super weird cash problem in my Android Studio. It's not the first time this happens. When you described these steps, I followed it and I could not reproduce, I repeated again and could not reproduce either. So I moved to other task that I had to use the real emulator. After that, I tried to reproduce this issue with the real device that I was using and finally I could reproduce the issue

@mebarbosa mebarbosa enabled auto-merge (squash) December 11, 2024 10:20
@mebarbosa mebarbosa merged commit ccfa7c4 into main Dec 11, 2024
16 checks passed
@mebarbosa mebarbosa deleted the task/fix-autodownload-disabling branch December 11, 2024 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disabled global Auto download new episodes setting overrides podcast specifc one
3 participants