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

Allow fetching by permalink on client desktop#2610

Merged
sabrina-kiam merged 17 commits into
mainfrom
sk-pr-clean
Jan 21, 2023
Merged

Allow fetching by permalink on client desktop#2610
sabrina-kiam merged 17 commits into
mainfrom
sk-pr-clean

Conversation

@sabrina-kiam

Copy link
Copy Markdown
Contributor

Description

This diff adds permalinks across the frontend. These changes should be backwards compatible except for the last one which is commented out. It covers:

  • Adding to the collections cache a map of permalinks to playlist id's
  • Adding to the collection store a permalink object which tracks the permalink
  • Updating audius api client to call the new route getPlaylistByPermalink
  • Adding a retrieveCollectionByPermalink function to retrieveCollections which gets called whenever we fetch a collection (if permalink exists)
  • Updated routing for matching a path to our new permalink path containing a slug and handle. This is commented out for now as this change is not backwards compatible.

What remains on client side:

  • Gating the routing and fetching behind a feature flag
  • Updating mobile stuff which Dylan is working on
  • Updating PLAYLIST_PAGE() routing calls to take in a handle and slug

Dragons

There are a lot of changes that could easily mess up playlists somehow so want to make sure we're extra thorough with QA-ing playlist behavior and just ensuring current behavior hasn't changed.

How Has This Been Tested?

Ran client against staging. I tested a lot of existing behavior to make sure nothing crashed:

  • creating a playlist
  • adding a song to playlist
  • fetching an uncached playlist
  • fetching a cached playlist

How will this change be monitored?

Feature Flags

Will want to add feature flags!

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/sk-pr-clean

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

awesome work on this!!!

'Permalink formatted incorrectly. Should follow /<handle>/playlist/<slug> format.'
)
}
const handle = splitPermalink[1]

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.

just a thought, you could do something fancy like

const [_audiusUrl, handle, _playlist, slug] = splitPermalink

or maybe even

const [, handle, , slug] = splitPermalink

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.

ooo fancy i like, will add

return {
...state,
permalinks: {
// @ts-ignore

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.

just want to double check we want to ts-ignore this?

Comment thread packages/common/src/store/pages/collection/reducer.ts Outdated
Comment thread packages/common/src/store/pages/collection/types.ts Outdated
Comment thread packages/web/src/common/store/cache/collections/sagas.js Outdated
Comment thread packages/web/src/pages/collection-page/CollectionPageProvider.tsx Outdated
this.props.fetchCollection(collectionId as number)

if (params?.collectionId) {
if (forceFetch || params?.collectionId !== this.state.playlistId) {

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.

ultra nit, params.collectionId since if branch ensures it's there few other cases down below too. personally, once ive established existance in an object, and its reffed multple times, i desturcture it,

if (params.?collectionId) {
  const {collectionId} = params
  ...

}
}

if (params?.permalink) {

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.

do we also need collectionId to be here?

return { collectionId, handle: null, collectionType: null, title: null }
}

// const playlistByPermalinkMatch = matchPath<{

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.

comments

@gitguardian

gitguardian Bot commented Jan 20, 2023

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id Secret Commit Filename
688750 Generic High Entropy Secret 0b7d0ae packages/mobile/.env.stage View secret
2416684 Generic High Entropy Secret 0b7d0ae packages/mobile/.env.stage View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/sk-pr-clean

@sabrina-kiam sabrina-kiam merged commit f78d1eb into main Jan 21, 2023
@sabrina-kiam sabrina-kiam deleted the sk-pr-clean branch January 21, 2023 22:15
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
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.

3 participants