Skip to content
This repository was archived by the owner on Oct 4, 2023. It is now read-only.

Fix lineups not loading in Offline Mode (C-1945, C-1882, C-1803)#2697

Merged
nicoback2 merged 7 commits into
mainfrom
nkang--fix-collections-offline
Jan 27, 2023
Merged

Fix lineups not loading in Offline Mode (C-1945, C-1882, C-1803)#2697
nicoback2 merged 7 commits into
mainfrom
nkang--fix-collections-offline

Conversation

@nicoback2

@nicoback2 nicoback2 commented Jan 26, 2023

Copy link
Copy Markdown
Contributor

Description

After the perf improvements, the useReachabilityState callbacks were not being fired on every render anymore. This is a good thing, but it ended up revealing issues with how/when we were loading the lineup on the favorited tracks tab and all the favorited collection screens.

If you were offline and then loaded any of these screens, the lineup simply would not load. This is because the call to fetch local tracks was ONLY triggered when going from online -> offline. So this PR makes sure that we fetch local tracks on first render as well.

However this was not the only fix needed. We were also not updating our state.pages.collection (https://github.com/AudiusProject/audius-client/blob/main/packages/common/src/store/pages/collection/selectors.ts#L2) slice when navigating to a collection screen while offline, so the collection id passed to useOfflineCollectionLineup was always empty. I put in a quick fix for this but we should probably add logic to update that part of the store correctly when loading the offline lineup.

The last fix implemented here was that we were filtering out tracks needlessly from offline collection lineups because their downloaded tracks' data did not yet have the collection as a reason for download.

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

@pull-request-size pull-request-size Bot added size/M and removed size/S labels Jan 27, 2023
@nicoback2 nicoback2 changed the title Partially fix offline playlist tracks not loading on the collection screen Fix lineups not loading in Offline Mode Jan 27, 2023
@nicoback2 nicoback2 marked this pull request as ready for review January 27, 2023 00:31
hasReposted={has_current_user_reposted}
hasSaved={has_current_user_saved}
isAlbum={is_album}
collectionId={playlist_id}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the quick fix mentioned in the pr description

@nicoback2 nicoback2 changed the title Fix lineups not loading in Offline Mode Fix lineups not loading in Offline Mode (C-1945, C-1882, C-1803) Jan 27, 2023
@nicoback2 nicoback2 requested a review from a team January 27, 2023 00:36

@dylanjeffers dylanjeffers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wow super clean! Imo long term it would be great for us to now have to worry about the two cases from the fetching selecting perspective, maybe when we get the persist stuff going

(track) =>
track.offline?.reasons_for_download.some(
(reason) => reason.collection_id === collectionId.toString()
) || collectionTrackIds.has(track.track_id)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

reasoning: a track in a collection may be downloaded from the favorited tracks page, but not yet have the collection added as a second reason for download (since the download queue hasn't gotten there yet).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sadly this doesnt work the other way around- we only know to include a downloaded track in the favorited tracks tab if it has the favorited tracks collection as a reason for download. cant fix this until we have a way of keeping the full list of favorited track metadata before they are all downloaded - i.e. can do what we do here on line 156 but for the favorited tracks "collection".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great call. Sad we don't have a track list for favorites yet.

@amendelsohn amendelsohn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Amazing finds! Totally missed the collection page loading.

(track) =>
track.offline?.reasons_for_download.some(
(reason) => reason.collection_id === collectionId.toString()
) || collectionTrackIds.has(track.track_id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great call. Sad we don't have a track list for favorites yet.

store.dispatch(cacheActions.add(Kind.TRACKS, cacheTracks, false, true))

if (collectionId === DOWNLOAD_REASON_FAVORITES) {
// Reorder lineup tracks accorinding to favorite time

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💀

@nicoback2 nicoback2 merged commit 47b0ed1 into main Jan 27, 2023
@nicoback2 nicoback2 deleted the nkang--fix-collections-offline branch January 27, 2023 22:05
audius-infra pushed a commit that referenced this pull request Jan 28, 2023
[47b0ed1] Fix lineups not loading in Offline Mode (C-1945, C-1882, C-1803) (#2697) nicoback2
[da8637f] Add toggle for message notifs on web + mobile (#2700) Michelle Brier
[f909c82] Remove use of iso3 country code (#2719) Reed
[48ce610] Change location.in_eu to is_eu (#2718) Reed
[a0fa651] [DMs] Update DMs store, sagas and selectors to be optimistic (#2711) Marcus Pasell
[d62bc36] Fix popup menu on nav (#2717) Marcus Pasell
[5b5e5ae] [C-1969 C-1967] Don't re-favorite playlist on download (#2713) Dylan Jeffers
[1c02211] Port Input Component to Typescript - InputV2 (#2714) Marcus Pasell
[5e6a8b8] [DMs] Show the name of the user that reacted when hovering over a reaction (#2587) Marcus Pasell
[731539d] [C-1754] Fix modal popups not closing (#2691) Marcus Pasell
[9f53712] Fix incorrect type for follower group in profile (#2705) Marcus Pasell
[a664ba3] Update TextAreaV2 to respect initial height when non-growing (#2703) Marcus Pasell
[75e9243] [DMs] Fix plus button not appearing on own messages (#2712) Marcus Pasell
[f58e3fb] Change react-native-job-queue updateInterval to 500 ms (#2710) Sebastian Klingler
[6d174ed] Update PopupMenu to not have circular icons (#2704) Marcus Pasell
[613d992] [C-1891] Restart app on saga error, Refactor mobile toasts (#2696) Dylan Jeffers
[a1293c6] Fix profile picture crash (#2698) Raymond Jacobson
[3316525] Use 2-letter country codes in buy-audio location (#2695) Reed
[e773823] Improve offline mode react performance (#2694) Sebastian Klingler
[34c46c2] [PAY-878] Wrap getBalance in try/catch (#2692) Reed
[067f249] Fix button alignment in rewards tiles (#2693) Reed
[5b11226] Performance improvements for offline mode (#2690) Andrew Mendelsohn
[778640f] Revert "Make rc-staging fast-app variant (#2585)" (#2689) Raymond Jacobson
[6f24d1b] [C-1907, C-1928] Update chromecast icon in NowPlayingDrawer and track name length on track list items (#2688) Kyle Shanks
[ef58833] C-1932 Hide playlists in offline mode with no downloaded tracks (#2686) nicoback2
[2d5a7cd] Fix content alignment in rewards tiles (#2687) Reed
[16833c7] Remove ButtonWithArrow (#2682) Reed
[8d88e1d] [PAY-871] Extend reward buttons to full tile width (#2683) Reed
[82f5a9b] C-1908 Show error indicator on playlists if all track downloads failed (#2681) nicoback2
[70bb4de] Fix mobile error (#2685) nicoback2
[7d8b7aa] [DMs] Set max length of the composer to 10k (#2673) Marcus Pasell
[084f23d] Install react-native-tiktok with patch (#2659) nicoback2
[70598db] Add queue length checks when skipping using the track player (#2675) Kyle Shanks
[284b5c7] DMs: Add compose button (#2666) Marcus Pasell
[c72fbba] [DMs] Add tails to chat bubbles (#2653) Marcus Pasell
[f16eeb4] [DMs] Fix pagination for chat list and chat message list (#2652) Marcus Pasell
[fd84793] Fix rewards claim button for aggregate challenges (#2671) Reed
[2ecb9f5] Fix artwork for now-playing and playbar (#2674) Dylan Jeffers
[604566e] [C-1884] Improve top supporters fetching and loading (#2651) Dylan Jeffers
[c735486] Update SDK to 1.0.42 (#2668) Marcus Pasell
[2654459] [C-1854] Prevent crash on settings page (#2667) Sebastian Klingler
[df5e524] Listen to add succeeded (#2669) Saliou Diallo
[55c4b64] [PAY-862] Rewards UI Improvements Followups (#2662) Reed
[0ae57f7] [C-1894] [C-1904] Fix tiktok verification (#2665) Sebastian Klingler
[21fb1a6] Fix profile (#2664) Saliou Diallo
[8b8d350] Fix playlist tile click (#2663) Saliou Diallo
[0d6a949] [C-1885] Improve user socials fetching/loading (#2650) Dylan Jeffers
[9213f8f] [PAY-853] Premium content fixes on web (#2642) Saliou Diallo
[c204e1b] Memoize sorted challenges (#2649) Reed
[f7bfce0] [C-1874] Improve lineup loading animations, add fast-image (#2648) Dylan Jeffers
[b7bd8bb] [C-1669] Enable OFFLINE_MODE_ENABLED feature flag (#2658) Andrew Mendelsohn
[2601207] [C-1881] Allow removing downloads while offline (#2657) Andrew Mendelsohn
[3a5f98c] [C-1879] Download track when added to downloaded playlists (#2656) Andrew Mendelsohn
[d3de906] [C-1875] Fix empty favorited tracks lineup on reconnect (#2655) Andrew Mendelsohn
[1ca06c1] Add explicit sizing to image fetching (#2566) Sebastian Klingler
[66a56ea] [C-1876] Hide download indicator on deleted tracks (#2654) Sebastian Klingler
[f78d1eb] Allow fetching by permalink on client desktop (#2610) sabrina-kiam
@AudiusProject AudiusProject deleted a comment from linear Bot Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants