Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1149e9e0cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard hasProAccess else { return false } | ||
| guard showWidgetRefreshButton else { return false } | ||
| guard channel?.channelName != YouTubeChannel.preview.channelName else { return false } | ||
| return latestUpload != nil |
There was a problem hiding this comment.
Keep refresh button available when upload data is missing
The refresh control is hidden whenever latestUpload is nil, which means a transient API failure (or any decode/network error path that leaves the channel present but upload absent) removes the user's only manual recovery path. In that state the widget shows placeholder content but cannot be refreshed on demand, so users must wait for the next scheduled timeline reload instead of retrying immediately.
Useful? React with 👍 / 👎.
| let uploadsPlaylistId = try await getUploadsPlaylistId(for: channelIdWithoutSpaces) | ||
| let latestVideoId = try await getLatestUploadVideoId(for: uploadsPlaylistId) | ||
| let latestUpload = try await getLatestUploadVideoDetails(for: latestVideoId) | ||
|
|
||
| try latestUploadStorage?.setObject(latestUpload, forKey: channelIdWithoutSpaces) | ||
| return latestUpload |
There was a problem hiding this comment.
📝 Info: 3 sequential API calls per widget refresh for latest upload
Each latest upload widget refresh makes 3 sequential YouTube API calls: getUploadsPlaylistId (channels?part=contentDetails), getLatestUploadVideoId (playlistItems), and getLatestUploadVideoDetails (videos) — plus an image download. The 120-second cache (SubscriberWidget/Services/YouTubeService.swift:20-21) mitigates this during rapid refreshes, but each cold fetch consumes significant API quota. The uploads playlist ID for a channel is stable and could be cached with a much longer TTL to reduce calls from 3 to 2 on most refreshes.
Was this helpful? React with 👍 or 👎 to provide feedback.
| func getSnapshot( | ||
| for configuration: SelectChannelIntent, | ||
| in context: Context, | ||
| completion: @escaping (LatestUploadEntry) -> Void | ||
| ) { | ||
| Task { | ||
| let channelStorageService = ChannelStorageService() | ||
|
|
||
| if configuration.channel == nil { | ||
| let channels = channelStorageService.getChannels() | ||
| guard let firstChannel = channels.first else { | ||
| completion(LatestUploadEntry(channel: nil, latestUpload: nil)) | ||
| return | ||
| } | ||
|
|
||
| completion(await fetchLatestUpload(for: firstChannel)) | ||
| } else { | ||
| completion( | ||
| await fetchLatestUpload( | ||
| for: configuration.channel ?? YouTubeChannelParam.global, | ||
| channelStorageService: channelStorageService | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🚩 getSnapshot makes network calls without checking context.isPreview
The getSnapshot method at SubscriberCount/LatestUploadTimelineProvider.swift:45-70 always makes network calls to fetch the latest upload, even when called from the widget gallery (where context.isPreview is true). WidgetKit best practices recommend returning quickly with placeholder data in the preview context. The existing SubWidgetIntentTimelineProvider has a similar pattern (it fetches images in the snapshot), but the new provider is heavier because it makes 3 API calls + image download. This could cause the widget to appear blank or slow in the widget gallery if the network is slow.
Was this helpful? React with 👍 or 👎 to provide feedback.
Uh oh!
There was an error while loading. Please reload this page.