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

[C-2042] Fix empty collection on deep link#2914

Merged
sliptype merged 2 commits into
mainfrom
sk-c-2042-empty-collection-on-deeplink
Feb 22, 2023
Merged

[C-2042] Fix empty collection on deep link#2914
sliptype merged 2 commits into
mainfrom
sk-c-2042-empty-collection-on-deeplink

Conversation

@sliptype

Copy link
Copy Markdown
Contributor

Description

  • Fix empty collection lineup upon deep linking due not waiting for the reset action to complete before triggering the fetch
  • Sequence reset and fetch actions by putting into a new saga

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.

@sliptype sliptype requested review from a team and amendelsohn February 22, 2023 21:53

@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.

Very nice! Feels better to have the timing more explicitly controlled.

yield put(fetchCollection(action.collectionId))
}

yield take(collectionActions.fetchCollectionSucceeded)

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.

Nice! Good benefit of switching to a saga

}
dispatch(tracksActions.fetchLineupMetadatas(0, 200, false, undefined))
}, [dispatch, collectionUid, userUid, collectionId])
dispatch(resetAndFetchCollection(collectionId))

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.

this doesn't match the name in the actions file resetAndFetchCollectionTracks

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.

Updated that right at the end, fixed!

// Reset collection so that lineup is not shared between separate instances
// of the CollectionScreen
yield put(collectionActions.resetCollection(collectionUid, userUid))
yield take(tracksActions.reset().type)

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.

why do we have to do this one differently than the others like collectionActions.fetchCollectionSucceeded

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 stumped me for a bit. I guess by default take calls the function if it's an action creator and uses the type value from the returned action. Well our lineup action creators use the this context to add a prefix, which was undefined when called by take implicitly

https://github.com/AudiusProject/audius-client/blob/main/packages/common/src/store/lineup/actions.ts#L208

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.

AHHHH cool. I remember seeing that on the lineup actions. Good to know!

@sliptype sliptype merged commit 4050a55 into main Feb 22, 2023
@sliptype sliptype deleted the sk-c-2042-empty-collection-on-deeplink branch February 22, 2023 22:16
audius-infra pushed a commit that referenced this pull request Feb 25, 2023
[ef02ac1] Fix podfile, specify ruby-version (#2924) Dylan Jeffers
[c925cfe] Fix track play button (#2925) Michael Piazza
[6e3338b] [C-2201] Improve card list skeleton count (#2916) Dylan Jeffers
[c696519] [C-2207] Fix scrubber timestamp sync issues (#2912) Dylan Jeffers
[f9f5066] [C-2202] Sync legacy search history (#2915) Dylan Jeffers
[1c50935] [C-2193] Fix webview audio players on ios (#2922) Raymond Jacobson
[dd649d5] [C-1922] Fix mobile share encoding (#2918) Dylan Jeffers
[eb6b89e] Fix collectibles route (#2917) Dylan Jeffers
[efaa539] Fix lineup showing empty tile before fetch (#2911) Dylan Jeffers
[c77076a] comma (#2920) Marcus Pasell
[6041a40] [PAY-943] - Filter out download options for gated track (#2919) Saliou Diallo
[3b2ff6d] [DMs] RadioButton component and InboxSettingsModal (#2884) Marcus Pasell
[709dde5] [PAY-890] Add locked content modal (#2893) Saliou Diallo
[3d8912f] [C-1856] Upgrade react-native to 0.71 (#2644) Dylan Jeffers
[e863756] [C-1931] Refactor collection header for private track logic (#2676) Dylan Jeffers
[1f8358b] [C-2171] Fix remove download performance (#2903) Dylan Jeffers
[4050a55] [C-2042] Fix empty collection on deep link (#2914) Sebastian Klingler
[03a31ee] [C-2164] add retry and requeue logic to download track jobs (#2899) Andrew Mendelsohn
[3bf27f7] [C-2186] Fix spacing for longs handles/names (#2908) Raymond Jacobson
[a89b3bb] Fix image loading issues w Share to X (#2909) nicoback2
[902159c] [C-2130] Fix race cond in lineup init (#2905) Raymond Jacobson
[7beb57d] [C-2200] Fix profile tabs loading/empty states (#2906) Dylan Jeffers
[6beaa2d] [C-2198] Fix runtime error due to missing drawer data (#2904) Dylan Jeffers
[2b44b26] [C-2197] Fix image prefetch to work with fast-image (#2902) Dylan Jeffers
[5a8069f] Permission copy change (#2901) nicoback2
[d427d3e] Fix TikTok share permission for iOS (#2897) nicoback2
[44c665d] [C-2195] Fix tiktok share on android (#2900) Sebastian Klingler
[0235af9] [C-2178] Complete w/ tiktok qa fixes (#2896) Sebastian Klingler
[dfe4622] Add repost of a repost metadata to reposting tracks and collections (#2847) sabrina-kiam
[c2f6ad7] Minor updates to mobile chats (#2891) Reed
[0e70d27] [C-2169] Fix track-player hot reload (#2890) Dylan Jeffers
[a07ce78] Rename TikTok video feature flag (#2894) nicoback2
[3da2b87] Relay to both POA and ACDC (#2895) Isaac Solo
[a02fee7] [C-2176] TikTok sdk improvements (#2892) Sebastian Klingler
@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.

2 participants