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.

Sign up for GitHub

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 offline collections not loading if started offline C-3187 #6245

Merged
merged 3 commits into from
Oct 6, 2023
Merged

Fix offline collections not loading if started offline C-3187 #6245

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f21ee6
fix offline collections
nicoback Oct 5, 2023
6e6dceb
pr review
nicoback Oct 6, 2023
a97b364
type fix
nicoback Oct 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 23 additions & 26 deletions packages/mobile/src/screens/favorites-screen/useCollectionsScreenData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,39 +117,36 @@ export const useCollectionsScreenData = ({
return []
}

return fetchedCollectionIds.filter((collectionId) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before the library work, fetchedCollectionIds was always populated even when offline bc we were using the collections value in the account object. Now, fetchedCollectionIds always comes from API (since we can't rely on the account object collections, since they only include the user's Favorites), so it'll be empty if we start the session offline. So to fix the issue we can just gather the downloaded collection IDS from the offlineCollectionStatus data in the store.

Copy link
Contributor

Choose a reason for hiding this comment

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

Love this change and makes a ton of sense

const collection = getCollection(state, { id: collectionId })
if (collection == null) {
console.error(
`Unexpected missing fetched collection: ${collectionId}`
)
return false
}

if (!isReachable) {
const offlineCollectionsStatus = getOfflineCollectionsStatus(state)
const offlineCollectionsStatus = getOfflineCollectionsStatus(state)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to remember which parts of state here might be best to select from. When we do the initial step where we load the offline data into the store, what do we use to determine which are avail? Is it just a crawl through the directory or do we use the status like you are using here? I wonder if there is a way to add a selector to kinda say "get downloaded collections"?

Copy link
Contributor

Choose a reason for hiding this comment

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

We definitely could roll this into a complex selector per the pattern we adopted during offline mode.

const offlineCollectionIds = Object.keys(offlineCollectionsStatus).filter(
(k) => offlineCollectionsStatus[k] === OfflineDownloadStatus.SUCCESS
)
return offlineCollectionIds
.map((stringId) => Number(stringId))
.filter((collectionId) => {
const collection = getCollection(state, { id: collectionId })
if (collection == null) {
console.error(
`Unexpected missing fetched collection: ${collectionId}`
)
return false
}
const trackIds =
collection.playlist_contents.track_ids.map(
(trackData) => trackData.track
) ?? []
const collectionDownloadStatus =
offlineCollectionsStatus[collection.playlist_id]
// Don't show a playlist in Offline Mode if it has at least one track but none of the tracks have been downloaded yet OR if it is not marked for download
return (
Boolean(collectionDownloadStatus) &&
collectionDownloadStatus !== OfflineDownloadStatus.INACTIVE &&
(trackIds.length === 0 ||
trackIds.some((t) => {
return (
offlineTracksStatus &&
offlineTracksStatus[t.toString()] ===
OfflineDownloadStatus.SUCCESS
)
}))
trackIds.length === 0 ||
trackIds.some((t) => {
return (
offlineTracksStatus &&
offlineTracksStatus[t.toString()] ===
OfflineDownloadStatus.SUCCESS
)
})
)
}
return true
})
})
},
[
fetchedCollectionIds,
Expand Down