search

Automattic / pocket-casts-ios

Pocket Casts iOS app 🎧
Mozilla Public License 2.0
1.67k stars 134 forks source link

WatchOS app does not honour cellular download settings #1176

Open rviljoen opened 1 year ago

rviljoen commented 1 year ago

Description

If the watch app is configured to "Auto Download Up Next", it will ignore the "Only On Wifi" setting under "Auto Download". This results in using the cellular plan on the watch or the phone (in case of Wi-Fi only watch), which can quickly deplete a capped data plan.

I believe the problematic part of the code is here:

https://github.com/Automattic/pocket-casts-ios/blob/2fd9145ba5ff2cb6af9d90f140439a7cbaf9fac7/podcasts/DownloadManager.swift#L285-L289

For WatchOS, there is no check whether or not to use cellular, it simply sets it to foreground or background cellular session based on the application state.

This issue is related to, but not the same as, https://github.com/Automattic/pocket-casts-ios/issues/20

Step-by-step reproduction instructions

  1. Disable Wi-Fi on both iPhone and Apple Watch
  2. Enable the "Auto Download Up Next" setting under Apple Watch
  3. Enable the "Only on WiFi" setting under Auto Download
  4. Add an episode to the Up Next queue
  5. Click "Refresh Data" on watch
  6. Note that the new episode is added to the queue on the watch, and the download is initiated immediately despite having no Wi-Fi connection

Screenshots or screen recording

No response

Did you search for existing bug reports?

Device, Operating system, and Pocket Casts app version

iPhone 14 Pro, iOS 17.0.3, PC 7.50.0.0

rviljoen commented 11 months ago

I've commented out the WatchOS specific check and tested using the same logic (line 288) for both Phone and Watch. Seems to have resolved the issue, but I'll do some more testing. It would be interesting to find out if there was a reason for adding the WatchOS specific check. The blame history here on GH does not reveal anything (it was this way since first commit). @leandroalonso @emilylaguna I suspect you may have access to the commit history prior to this being open source. Maybe there is a clue in a previous SJ submission.

leandroalonso commented 11 months ago

@rviljoen This is the context I gathered for this change, PR had this description:

Created a cellular foreground url session. Use this session when starting downloads in the foreground

It fixed an issue which had this content:

Add background url session for watchOS Create a background url session config handle starting and ending wkurlbackgroundtask