From e52e4d1588d4dab7f1487ade894e48a58a403b5d Mon Sep 17 00:00:00 2001 From: anshg1214 Date: Fri, 20 Sep 2024 17:36:08 +0000 Subject: [PATCH 1/6] feat: Cache Album Art Query --- frontend/js/src/common/listens/ListenCard.tsx | 78 +++++++++++++------ frontend/js/src/utils/utils.tsx | 25 ++++++ 2 files changed, 78 insertions(+), 25 deletions(-) diff --git a/frontend/js/src/common/listens/ListenCard.tsx b/frontend/js/src/common/listens/ListenCard.tsx index 5185161c41..91033eb6fa 100644 --- a/frontend/js/src/common/listens/ListenCard.tsx +++ b/frontend/js/src/common/listens/ListenCard.tsx @@ -28,9 +28,11 @@ import NiceModal from "@ebay/nice-modal-react"; import { faPlayCircle } from "@fortawesome/free-regular-svg-icons"; import { toast } from "react-toastify"; import { Link } from "react-router-dom"; +import { useQuery } from "@tanstack/react-query"; import { fullLocalizedDateFromTimestampOrISODate, getAlbumArtFromListenMetadata, + getAlbumArtFromListenMetadataKey, getArtistLink, getArtistMBIDs, getArtistName, @@ -93,10 +95,10 @@ export type ListenCardProps = { export type ListenCardState = { listen: Listen; isCurrentlyPlaying: boolean; - thumbnailSrc?: string; // Full URL to the CoverArtArchive thumbnail }; type ListenCardPropsWithDispatch = ListenCardProps & { + thumbnailSrc?: string; dispatch: (action: BrainzPlayerActionType, callback?: () => void) => void; }; @@ -117,7 +119,6 @@ export class ListenCard extends React.Component< async componentDidMount() { window.addEventListener("message", this.receiveBrainzPlayerMessage); - await this.getCoverArt(); } async componentDidUpdate( @@ -129,33 +130,12 @@ export class ListenCard extends React.Component< if (Boolean(listen) && !isEqual(listen, oldListen)) { this.setState({ listen }); } - if (!customThumbnail && Boolean(listen) && !isEqual(listen, oldListen)) { - await this.getCoverArt(); - } } componentWillUnmount() { window.removeEventListener("message", this.receiveBrainzPlayerMessage); } - async getCoverArt() { - const { spotifyAuth, APIService, userPreferences } = this.context; - if (userPreferences?.saveData === true) { - return; - } - const { listen } = this.state; - const albumArtSrc = await getAlbumArtFromListenMetadata( - listen, - spotifyAuth, - APIService - ); - if (albumArtSrc) { - this.setState({ thumbnailSrc: albumArtSrc }); - } else { - this.setState({ thumbnailSrc: undefined }); - } - } - playListen = () => { const { listen, isCurrentlyPlaying } = this.state; if (isCurrentlyPlaying) { @@ -277,9 +257,10 @@ export class ListenCard extends React.Component< additionalActions, listen: listenFromProps, dispatch: dispatchProp, + thumbnailSrc, ...otherProps } = this.props; - const { listen, isCurrentlyPlaying, thumbnailSrc } = this.state; + const { listen, isCurrentlyPlaying } = this.state; const { currentUser } = this.context; const isLoggedIn = !isEmpty(currentUser); @@ -761,5 +742,52 @@ export class ListenCard extends React.Component< export default function ListenCardWrapper(props: ListenCardProps) { const dispatch = useBrainzPlayerDispatch(); - return ; + const { spotifyAuth, APIService, userPreferences } = React.useContext( + GlobalAppContext + ); + const { listen, customThumbnail } = props; + + const albumArtQueryKey = getAlbumArtFromListenMetadataKey( + listen, + spotifyAuth + ); + + const albumArtDisabled = + customThumbnail || !listen || userPreferences?.saveData; + + const { data } = useQuery({ + queryKey: ["album-art", albumArtQueryKey, albumArtDisabled], + queryFn: async () => { + try { + if (albumArtDisabled) { + return { + data: undefined, + hasError: false, + errorMessage: "", + }; + } + const albumArtSrc = await getAlbumArtFromListenMetadata( + listen, + spotifyAuth, + APIService + ); + return { + data: albumArtSrc, + hasError: false, + errorMessage: "", + }; + } catch (error) { + return { + data: undefined, + hasError: true, + errorMessage: error.message, + }; + } + }, + }); + + const thumbnailSrc = data?.data; + return ( + + ); } diff --git a/frontend/js/src/utils/utils.tsx b/frontend/js/src/utils/utils.tsx index 3288c4f17b..45dc2b441e 100644 --- a/frontend/js/src/utils/utils.tsx +++ b/frontend/js/src/utils/utils.tsx @@ -839,6 +839,30 @@ const getAlbumArtFromSpotifyTrackID = async ( return undefined; }; +const getAlbumArtFromListenMetadataKey = ( + listen: BaseListenFormat, + spotifyUser?: SpotifyUser +): string | undefined => { + if ( + SpotifyPlayer.isListenFromThisService(listen) && + SpotifyPlayer.hasPermissions(spotifyUser) + ) { + return `spotify:${SpotifyPlayer.getSpotifyTrackIDFromListen(listen)}`; + } + if (YoutubePlayer.isListenFromThisService(listen)) { + return `youtube:${YoutubePlayer.getVideoIDFromListen(listen)}`; + } + const userSubmittedReleaseMBID = + listen.track_metadata?.release_mbid ?? + listen.track_metadata?.additional_info?.release_mbid; + const caaId = listen.track_metadata?.mbid_mapping?.caa_id; + const caaReleaseMbid = listen.track_metadata?.mbid_mapping?.caa_release_mbid; + + return `ca:${userSubmittedReleaseMBID ?? ""}:${caaId ?? ""}:${ + caaReleaseMbid ?? "" + }`; +}; + const getAlbumArtFromListenMetadata = async ( listen: BaseListenFormat, spotifyUser?: SpotifyUser, @@ -1110,6 +1134,7 @@ export { pinnedRecordingToListen, getAlbumArtFromReleaseMBID, getAlbumArtFromReleaseGroupMBID, + getAlbumArtFromListenMetadataKey, getAlbumArtFromListenMetadata, getAverageRGBOfImage, getAdditionalContent, From 2d95e92414a8bf26857cff2b9a53e2bf5d58ef8c Mon Sep 17 00:00:00 2001 From: anshg1214 Date: Fri, 20 Sep 2024 18:22:49 +0000 Subject: [PATCH 2/6] test: Add React Query Wrapper around Components --- .../common/brainzplayer/BrainzPlayer.test.tsx | 21 ++++-- .../tests/common/listens/ListenCard.test.tsx | 75 ++++++------------- .../tests/explore/huesound/HueSound.test.tsx | 10 ++- .../PersonalRecommendationsModal.test.tsx | 6 +- frontend/js/tests/playlists/Playlist.test.tsx | 14 +++- .../js/tests/recent/RecentListens.test.tsx | 5 +- frontend/js/tests/test-react-query.tsx | 16 ++++ .../js/tests/user/taste/UserFeedback.test.tsx | 19 ++++- 8 files changed, 102 insertions(+), 64 deletions(-) create mode 100644 frontend/js/tests/test-react-query.tsx diff --git a/frontend/js/tests/common/brainzplayer/BrainzPlayer.test.tsx b/frontend/js/tests/common/brainzplayer/BrainzPlayer.test.tsx index 88da2a54ec..363327cfb7 100644 --- a/frontend/js/tests/common/brainzplayer/BrainzPlayer.test.tsx +++ b/frontend/js/tests/common/brainzplayer/BrainzPlayer.test.tsx @@ -17,6 +17,7 @@ import { } from "../../../src/common/brainzplayer/BrainzPlayerContext"; import { renderWithProviders } from "../../test-utils/rtl-test-utils"; import { listenOrJSPFTrackToQueueItem } from "../../../src/common/brainzplayer/utils"; +import { ReactQueryWrapper } from "../../test-react-query"; // Font Awesome generates a random hash ID for each icon everytime. // Mocking Math.random() fixes this @@ -175,7 +176,9 @@ describe("BrainzPlayer", () => { ...GlobalContextMock.context, spotifyAuth: spotifyAccountWithPermissions, }, - {} + { + wrapper: ReactQueryWrapper, + } ); const playButton = screen.getByTestId("bp-play-button"); @@ -201,7 +204,9 @@ describe("BrainzPlayer", () => { ...GlobalContextMock.context, spotifyAuth: spotifyAccountWithPermissions, }, - {} + { + wrapper: ReactQueryWrapper, + } ); const queueList = screen.getByTestId("queue"); @@ -224,7 +229,9 @@ describe("BrainzPlayer", () => { ...GlobalContextMock.context, spotifyAuth: spotifyAccountWithPermissions, }, - {} + { + wrapper: ReactQueryWrapper, + } ); const playButton = screen.getByTestId("bp-play-button"); @@ -256,7 +263,9 @@ describe("BrainzPlayer", () => { ...GlobalContextMock.context, spotifyAuth: spotifyAccountWithPermissions, }, - {} + { + wrapper: ReactQueryWrapper, + } ); const playButton = screen.getByTestId("bp-play-button"); @@ -287,7 +296,9 @@ describe("BrainzPlayer", () => { ...GlobalContextMock.context, spotifyAuth: spotifyAccountWithPermissions, }, - {} + { + wrapper: ReactQueryWrapper, + } ); const playButton = screen.getByTestId("bp-play-button"); diff --git a/frontend/js/tests/common/listens/ListenCard.test.tsx b/frontend/js/tests/common/listens/ListenCard.test.tsx index 3f846c2442..1dd5b094cb 100644 --- a/frontend/js/tests/common/listens/ListenCard.test.tsx +++ b/frontend/js/tests/common/listens/ListenCard.test.tsx @@ -20,12 +20,23 @@ import { waitForComponentToPaint } from "../../test-utils"; import CBReviewModal from "../../../src/cb-review/CBReviewModal"; import RecordingFeedbackManager from "../../../src/utils/RecordingFeedbackManager"; import Card from "../../../src/components/Card"; +import { ReactQueryWrapper } from "../../test-react-query"; // Font Awesome generates a random hash ID for each icon everytime. // Mocking Math.random() fixes this // https://github.com/FortAwesome/react-fontawesome/issues/194#issuecomment-627235075 jest.spyOn(global.Math, "random").mockImplementation(() => 0); +function ListenCardWithWrappers(props: ListenCardProps) { + return ( + + + + + + ); +} + const listen: Listen = { listened_at: 0, playing_now: false, @@ -63,11 +74,7 @@ const globalProps: GlobalAppContextT = { describe("ListenCard", () => { it("renders correctly", () => { - const wrapper = mount( - - - - ); + const wrapper = mount(); const card = wrapper.find(Card); expect(card).toHaveLength(1); expect(card.getDOMNode()).toHaveClass("listen-card"); @@ -92,9 +99,7 @@ describe("ListenCard", () => { }; const wrapper = mount( - - - + ); @@ -105,11 +110,7 @@ describe("ListenCard", () => { it("should render timestamp using preciseTimestamp", () => { const preciseTimestamp = jest.spyOn(utils, "preciseTimestamp"); - const wrapper = mount( - - - - ); + const wrapper = mount(); expect(preciseTimestamp).toHaveBeenCalledTimes(1); }); @@ -136,9 +137,7 @@ describe("ListenCard", () => { user_name: "test", }; const wrapper = mount( - - - + ); expect( wrapper.find('[href="https://musicbrainz.org/recording/bar"]') @@ -157,11 +156,7 @@ describe("ListenCard", () => { }); it("should render a play button", () => { - const wrapper = mount( - - - - ); + const wrapper = mount(); const instance = wrapper .find(ListenCardClass) .instance() as ListenCardClass; @@ -171,11 +166,7 @@ describe("ListenCard", () => { }); it("should send an event to BrainzPlayer when playListen is called", async () => { - const wrapper = mount( - - - - ); + const wrapper = mount(); const instance = wrapper .find(ListenCardClass) .instance() as ListenCardClass; @@ -194,11 +185,7 @@ describe("ListenCard", () => { it("should do nothing when playListen is called on currently playing listen", async () => { const postMessageSpy = jest.spyOn(window, "postMessage"); - const wrapper = mount( - - - - ); + const wrapper = mount(); const instance = wrapper .find(ListenCardClass) .instance() as ListenCardClass; @@ -213,11 +200,7 @@ describe("ListenCard", () => { }); it("should render the formatted duration_ms if present in the listen metadata", () => { - const wrapper = mount( - - - - ); + const wrapper = mount(); const durationElement = wrapper.find('[title="Duration"]'); expect(durationElement).toBeDefined(); expect(durationElement.text()).toEqual("2:03"); @@ -230,9 +213,7 @@ describe("ListenCard", () => { ); set(listenWithDuration, "track_metadata.additional_info.duration", 142); const wrapper = mount( - - - + ); const durationElement = wrapper.find('[title="Duration"]'); expect(durationElement).toBeDefined(); @@ -243,9 +224,7 @@ describe("ListenCard", () => { it("calls API, and creates a new alert on success", async () => { const wrapper = mount( - - - + ); const instance = wrapper @@ -279,9 +258,7 @@ describe("ListenCard", () => { currentUser: { auth_token: undefined, name: "test" }, }} > - - - + ); const instance = wrapper @@ -302,9 +279,7 @@ describe("ListenCard", () => { it("calls handleError if error is returned", async () => { const wrapper = mount( - - - + ); const instance = wrapper @@ -336,9 +311,7 @@ describe("ListenCard", () => { const wrapper = mount( - - - + ); diff --git a/frontend/js/tests/explore/huesound/HueSound.test.tsx b/frontend/js/tests/explore/huesound/HueSound.test.tsx index b1c8df88a5..eb0095e143 100644 --- a/frontend/js/tests/explore/huesound/HueSound.test.tsx +++ b/frontend/js/tests/explore/huesound/HueSound.test.tsx @@ -11,6 +11,7 @@ import { textContentMatcher, } from "../../test-utils/rtl-test-utils"; import getExploreRoutes from "../../../src/explore/routes"; +import { ReactQueryWrapper } from "../../test-react-query"; const release: ColorReleaseItem = { artist_name: "Rorcal & Music For The Space", @@ -96,7 +97,14 @@ describe("HueSound", () => { messages.push(message) ); // With initial navigation, see initialEntries when we create memRouter - render(); + renderWithProviders( + , + {}, + { + wrapper: ReactQueryWrapper, + }, + false + ); await waitFor(async () => { const releaseButton = screen.getByRole("button"); diff --git a/frontend/js/tests/personal-recommendations/PersonalRecommendationsModal.test.tsx b/frontend/js/tests/personal-recommendations/PersonalRecommendationsModal.test.tsx index d9d0585998..1e6ca59aa6 100644 --- a/frontend/js/tests/personal-recommendations/PersonalRecommendationsModal.test.tsx +++ b/frontend/js/tests/personal-recommendations/PersonalRecommendationsModal.test.tsx @@ -12,6 +12,7 @@ import GlobalAppContext, { } from "../../src/utils/GlobalAppContext"; import { waitForComponentToPaint } from "../test-utils"; import RecordingFeedbackManager from "../../src/utils/RecordingFeedbackManager"; +import { ReactQueryWrapper } from "../test-react-query"; const listenToPersonallyRecommend: Listen = { listened_at: 1605927742, @@ -76,10 +77,13 @@ describe("PersonalRecommendationModal", () => { const wrapper = mount( + + + /> + ); diff --git a/frontend/js/tests/playlists/Playlist.test.tsx b/frontend/js/tests/playlists/Playlist.test.tsx index c30d388e6d..aec2453d28 100644 --- a/frontend/js/tests/playlists/Playlist.test.tsx +++ b/frontend/js/tests/playlists/Playlist.test.tsx @@ -8,6 +8,7 @@ import { renderWithProviders, textContentMatcher, } from "../test-utils/rtl-test-utils"; +import { ReactQueryWrapper } from "../test-react-query"; jest.mock("../../src/utils/SearchTrackOrMBID"); @@ -33,7 +34,14 @@ const router = createMemoryRouter( describe("PlaylistPage", () => { it("renders correctly", () => { - renderWithProviders(, {}, {}, false); + renderWithProviders( + , + {}, + { + wrapper: ReactQueryWrapper, + }, + false + ); screen.getByTestId("playlist"); const h1 = screen.getByRole("heading"); expect(h1).toHaveTextContent("1980s flashback jams"); @@ -62,7 +70,9 @@ describe("PlaylistPage", () => { renderWithProviders( , alternativeContextMock, - {}, + { + wrapper: ReactQueryWrapper, + }, false ); screen.getByText("Export to Spotify", { exact: false }); diff --git a/frontend/js/tests/recent/RecentListens.test.tsx b/frontend/js/tests/recent/RecentListens.test.tsx index 43ffc7171f..8f1cc696ef 100644 --- a/frontend/js/tests/recent/RecentListens.test.tsx +++ b/frontend/js/tests/recent/RecentListens.test.tsx @@ -21,6 +21,7 @@ import RecentListens, { import { waitForComponentToPaint } from "../test-utils"; import RecordingFeedbackManager from "../../src/utils/RecordingFeedbackManager"; import ListenCard from "../../src/common/listens/ListenCard"; +import { ReactQueryWrapper } from "../test-react-query"; // import Card from "../../src/components/Card"; // import BrainzPlayer from "../../src/brainzplayer/BrainzPlayer"; @@ -98,7 +99,9 @@ describe("Recentlistens", () => { const wrapper = mount( - + + + ); diff --git a/frontend/js/tests/test-react-query.tsx b/frontend/js/tests/test-react-query.tsx new file mode 100644 index 0000000000..cc608f47b8 --- /dev/null +++ b/frontend/js/tests/test-react-query.tsx @@ -0,0 +1,16 @@ +import * as React from "react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +export const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, +}); + +export function ReactQueryWrapper({ children }: any) { + return ( + {children} + ); +} diff --git a/frontend/js/tests/user/taste/UserFeedback.test.tsx b/frontend/js/tests/user/taste/UserFeedback.test.tsx index c89b01f97b..4e788553db 100644 --- a/frontend/js/tests/user/taste/UserFeedback.test.tsx +++ b/frontend/js/tests/user/taste/UserFeedback.test.tsx @@ -7,6 +7,7 @@ import UserFeedback, { } from "../../../src/user/taste/components/UserFeedback"; import * as userFeedbackProps from "../../__mocks__/userFeedbackProps.json"; import * as userFeedbackAPIResponse from "../../__mocks__/userFeedbackAPIResponse.json"; +import { ReactQueryWrapper } from "../../test-react-query"; const { totalCount, user, feedback } = userFeedbackProps; @@ -33,7 +34,11 @@ describe("UserFeedback", () => { }); it("renders ListenCard items for each feedback item", async () => { - render(); + render( + + + + ); const listensContainer = screen.getByTestId("userfeedback-listens"); expect(listensContainer).toBeInTheDocument(); expect(screen.getAllByTestId("listen")).toHaveLength(15); @@ -67,7 +72,11 @@ describe("UserFeedback", () => { }, ], }; - render(); + render( + + + + ); const listensContainer = screen.getByTestId("userfeedback-listens"); expect(listensContainer).toBeInTheDocument(); expect(screen.getAllByTestId("listen")).toHaveLength(1); @@ -75,7 +84,11 @@ describe("UserFeedback", () => { describe("getFeedbackItemsFromAPI", () => { it("sets the state and updates browser history", async () => { - render(); + render( + + + + ); fetchMock.mockResponseOnce(JSON.stringify(userFeedbackAPIResponse)); From d164dac8d0ee95faf6e2012b231d49ad22cbaad4 Mon Sep 17 00:00:00 2001 From: anshg1214 Date: Fri, 20 Sep 2024 18:45:49 +0000 Subject: [PATCH 3/6] fix: tests --- frontend/js/src/common/listens/ListenCard.tsx | 14 +++++++------- .../PersonalRecommendationsModal.test.tsx | 19 ++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/frontend/js/src/common/listens/ListenCard.tsx b/frontend/js/src/common/listens/ListenCard.tsx index 91033eb6fa..6ae6baa6fb 100644 --- a/frontend/js/src/common/listens/ListenCard.tsx +++ b/frontend/js/src/common/listens/ListenCard.tsx @@ -758,14 +758,14 @@ export default function ListenCardWrapper(props: ListenCardProps) { const { data } = useQuery({ queryKey: ["album-art", albumArtQueryKey, albumArtDisabled], queryFn: async () => { + if (albumArtDisabled) { + return { + data: undefined, + hasError: false, + errorMessage: "", + }; + } try { - if (albumArtDisabled) { - return { - data: undefined, - hasError: false, - errorMessage: "", - }; - } const albumArtSrc = await getAlbumArtFromListenMetadata( listen, spotifyAuth, diff --git a/frontend/js/tests/personal-recommendations/PersonalRecommendationsModal.test.tsx b/frontend/js/tests/personal-recommendations/PersonalRecommendationsModal.test.tsx index 1e6ca59aa6..ac4a0d00df 100644 --- a/frontend/js/tests/personal-recommendations/PersonalRecommendationsModal.test.tsx +++ b/frontend/js/tests/personal-recommendations/PersonalRecommendationsModal.test.tsx @@ -78,12 +78,11 @@ describe("PersonalRecommendationModal", () => { - - - + ); @@ -220,10 +219,12 @@ describe("PersonalRecommendationModal", () => { const wrapper = mount( - + + + ); From 15dad1f5e4aa28dc60a1255c3839029394febe17 Mon Sep 17 00:00:00 2001 From: anshg1214 Date: Sat, 21 Sep 2024 15:58:32 +0000 Subject: [PATCH 4/6] feat: Set cache ttl --- frontend/js/src/common/listens/ListenCard.tsx | 32 ++++++------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/frontend/js/src/common/listens/ListenCard.tsx b/frontend/js/src/common/listens/ListenCard.tsx index 6ae6baa6fb..2e76d6ebe6 100644 --- a/frontend/js/src/common/listens/ListenCard.tsx +++ b/frontend/js/src/common/listens/ListenCard.tsx @@ -747,46 +747,34 @@ export default function ListenCardWrapper(props: ListenCardProps) { ); const { listen, customThumbnail } = props; - const albumArtQueryKey = getAlbumArtFromListenMetadataKey( - listen, - spotifyAuth + const albumArtQueryKey = React.useMemo( + () => getAlbumArtFromListenMetadataKey(listen, spotifyAuth), + [listen, spotifyAuth] ); const albumArtDisabled = - customThumbnail || !listen || userPreferences?.saveData; + Boolean(customThumbnail) || !listen || userPreferences?.saveData; - const { data } = useQuery({ + const { data: thumbnailSrc } = useQuery({ queryKey: ["album-art", albumArtQueryKey, albumArtDisabled], queryFn: async () => { if (albumArtDisabled) { - return { - data: undefined, - hasError: false, - errorMessage: "", - }; + return undefined; } try { - const albumArtSrc = await getAlbumArtFromListenMetadata( + return await getAlbumArtFromListenMetadata( listen, spotifyAuth, APIService ); - return { - data: albumArtSrc, - hasError: false, - errorMessage: "", - }; } catch (error) { - return { - data: undefined, - hasError: true, - errorMessage: error.message, - }; + return undefined; } }, + staleTime: 1000 * 60 * 60 * 12, + gcTime: 1000 * 60 * 60 * 12, }); - const thumbnailSrc = data?.data; return ( ); From 763a273404221b3cfa104263c921d9e8575586bd Mon Sep 17 00:00:00 2001 From: anshg1214 Date: Sat, 21 Sep 2024 15:59:31 +0000 Subject: [PATCH 5/6] test: Add React Query Wrapper around Components --- .../tests/pins/PinnedRecordingCard.test.tsx | 101 +++++++++++------- frontend/js/tests/pins/UserPins.test.tsx | 47 +++++--- 2 files changed, 92 insertions(+), 56 deletions(-) diff --git a/frontend/js/tests/pins/PinnedRecordingCard.test.tsx b/frontend/js/tests/pins/PinnedRecordingCard.test.tsx index 0d320a16d5..4521fd21fd 100644 --- a/frontend/js/tests/pins/PinnedRecordingCard.test.tsx +++ b/frontend/js/tests/pins/PinnedRecordingCard.test.tsx @@ -14,12 +14,21 @@ import GlobalAppContext, { import { waitForComponentToPaint } from "../test-utils"; import RecordingFeedbackManager from "../../src/utils/RecordingFeedbackManager"; import ListenCard from "../../src/common/listens/ListenCard"; +import { ReactQueryWrapper } from "../test-react-query"; // Font Awesome generates a random hash ID for each icon everytime. // Mocking Math.random() fixes this // https://github.com/FortAwesome/react-fontawesome/issues/194#issuecomment-627235075 jest.spyOn(global.Math, "random").mockImplementation(() => 0); +function PinnedRecordingCardWithWrapper(props: PinnedRecordingCardProps) { + return ( + + + + ); +} + const user = { id: 1, name: "name", @@ -64,18 +73,17 @@ const props: PinnedRecordingCardProps = { describe("PinnedRecordingCard", () => { it("renders correctly", () => { - const wrapper = mount( - - ); + const wrapper = mount(); expect(wrapper.find(ListenCard)).toHaveLength(1); }); describe("determineIfCurrentlyPinned", () => { it("returns true when pinned_until > now", async () => { - const wrapper = mount( - + const componentWrapper = mount( + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; let isPlaying; await act(() => { @@ -85,12 +93,13 @@ describe("PinnedRecordingCard", () => { }); it("returns false when pinned_until < now", async () => { - const wrapper = mount( - ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; let isPlaying; await act(() => { isPlaying = instance.determineIfCurrentlyPinned(); @@ -101,12 +110,13 @@ describe("PinnedRecordingCard", () => { describe("unpinRecording", () => { it("calls API, updates currentlyPinned in state", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; const { APIService } = instance.context; const spy = jest.spyOn(APIService, "unpinRecording"); @@ -126,12 +136,15 @@ describe("PinnedRecordingCard", () => { }); it("does nothing if isCurrentUser is false", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; const { APIService } = instance.context; const spy = jest.spyOn(APIService, "unpinRecording"); @@ -148,17 +161,18 @@ describe("PinnedRecordingCard", () => { }); it("does nothing if CurrentUser.authtoken is not set", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; const { APIService } = instance.context; const spy = jest.spyOn(APIService, "unpinRecording"); @@ -175,12 +189,13 @@ describe("PinnedRecordingCard", () => { }); it("doesn't update currentlyPinned in state if status code is not 200", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; const { APIService } = instance.context; const spy = jest.spyOn(APIService, "unpinRecording"); @@ -197,12 +212,13 @@ describe("PinnedRecordingCard", () => { }); it("calls handleError if error is returned", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; instance.handleError = jest.fn(); const error = new Error("error"); @@ -226,12 +242,13 @@ describe("PinnedRecordingCard", () => { describe("deletePin", () => { it("calls API and updates isDeleted and currentlyPinned in state", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; const { APIService } = instance.context; const spy = jest.spyOn(APIService, "deletePin"); @@ -260,12 +277,15 @@ describe("PinnedRecordingCard", () => { }); it("does nothing if isCurrentUser is false", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; const { APIService } = instance.context; const spy = jest.spyOn(APIService, "deletePin"); @@ -282,17 +302,18 @@ describe("PinnedRecordingCard", () => { }); it("does nothing if CurrentUser.authtoken is not set", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; const { APIService } = instance.context; const spy = jest.spyOn(APIService, "deletePin"); @@ -309,12 +330,13 @@ describe("PinnedRecordingCard", () => { }); it("doesn't update currentlyPinned in state if status code is not 200", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; const { APIService } = instance.context; const spy = jest.spyOn(APIService, "deletePin"); @@ -331,12 +353,13 @@ describe("PinnedRecordingCard", () => { }); it("calls handleError if error is returned", async () => { - const wrapper = mount( + const componentWrapper = mount( - + ); - const instance = wrapper.instance(); + const wrapper = componentWrapper.find(PinnedRecordingCard); + const instance = wrapper.instance() as PinnedRecordingCard; instance.handleError = jest.fn(); const error = new Error("error"); diff --git a/frontend/js/tests/pins/UserPins.test.tsx b/frontend/js/tests/pins/UserPins.test.tsx index 5afc47af09..a087d49b72 100644 --- a/frontend/js/tests/pins/UserPins.test.tsx +++ b/frontend/js/tests/pins/UserPins.test.tsx @@ -16,12 +16,21 @@ import UserPins, { } from "../../src/user/taste/components/UserPins"; import PinnedRecordingCard from "../../src/user/components/PinnedRecordingCard"; import RecordingFeedbackManager from "../../src/utils/RecordingFeedbackManager"; +import { ReactQueryWrapper } from "../test-react-query"; // Font Awesome generates a random hash ID for each icon everytime. // Mocking Math.random() fixes this // https://github.com/FortAwesome/react-fontawesome/issues/194#issuecomment-627235075 jest.spyOn(global.Math, "random").mockImplementation(() => 0); +function UserPinsWithWrapper(props: UserPinsProps) { + return ( + + + + ); +} + // typescript doesn't recognise string literal values const props = { ...pinsPageProps, @@ -66,14 +75,14 @@ const mountOptions: { context: GlobalAppContextT } = { describe("UserPins", () => { it("renders correctly", () => { - const wrapper = mount(, mountOptions); + const wrapper = mount(, mountOptions); expect(wrapper.find("#pinned-recordings")).toHaveLength(1); expect(wrapper.getDOMNode()).toHaveTextContent("Lorde"); expect(wrapper.getDOMNode()).toHaveTextContent("400 Lux"); }); it("renders the correct number of pinned recordings", () => { - const wrapper = mount(, mountOptions); + const wrapper = mount(, mountOptions); const wrapperElement = wrapper.find("#pinned-recordings"); const pinnedRecordings = wrapperElement.find(PinnedRecordingCard); @@ -83,26 +92,28 @@ describe("UserPins", () => { describe("handleLoadMore", () => { describe("handleClickOlder", () => { it("does nothing if page >= maxPage", async () => { - const wrapper = mount(, mountOptions); - const instance = wrapper.instance(); + const wrapper = mount(, mountOptions); + const userPinWrapper = wrapper.find(UserPins); + const instance = userPinWrapper.instance() as UserPins; const spy = jest.fn().mockImplementation(() => {}); instance.getPinsFromAPI = spy; await act(() => { - wrapper.setState({ maxPage: 1 }); + userPinWrapper.setState({ maxPage: 1 }); }); await act(async () => { await instance.handleLoadMore(); }); - expect(wrapper.state("loading")).toBeFalsy(); + expect(userPinWrapper.state("loading")).toBeFalsy(); expect(spy).not.toHaveBeenCalled(); }); it("calls the API to get next page", async () => { - const wrapper = mount(, mountOptions); - const instance = wrapper.instance(); + const wrapper = mount(, mountOptions); + const userPinWrapper = wrapper.find(UserPins); + const instance = userPinWrapper.instance() as UserPins; const apiSpy = jest .fn() @@ -119,10 +130,10 @@ describe("UserPins", () => { expect(getPinsFromAPISpy).toHaveBeenCalledWith(2); expect(apiSpy).toHaveBeenCalledWith(props.user.name, 25, 25); - expect(wrapper.state("loading")).toBeFalsy(); - expect(wrapper.state("page")).toEqual(2); + expect(userPinWrapper.state("loading")).toBeFalsy(); + expect(userPinWrapper.state("page")).toEqual(2); // result should be combined previous pins and new pins - expect(wrapper.state("pins")).toEqual([ + expect(userPinWrapper.state("pins")).toEqual([ ...props.pins, ...APIPinsPageTwo.pinned_recordings, ]); @@ -132,21 +143,23 @@ describe("UserPins", () => { describe("removePinFromPinsList", () => { it("updates the listens state after removing particular pin", async () => { - const wrapper = mount(, mountOptions); - const instance = wrapper.instance(); + const wrapper = mount(, mountOptions); + const userPinWrapper = wrapper.find(UserPins); + const instance = userPinWrapper.instance() as UserPins; + await act(() => { - wrapper.setState({ pins: props.pins }); + userPinWrapper.setState({ pins: props.pins }); }); - expect(wrapper.state("pins")).toHaveLength(25); + expect(userPinWrapper.state("pins")).toHaveLength(25); const expectedNewFirstPin = props.pins[1]; await act(() => { instance.removePinFromPinsList(props.pins[0]); }); - expect(wrapper.state("pins")).toHaveLength(24); - expect(wrapper.state("pins")[0].recording_msid).toEqual( + expect(userPinWrapper.state("pins")).toHaveLength(24); + expect(userPinWrapper.state("pins")[0].recording_msid).toEqual( expectedNewFirstPin.recording_msid ); }); From 50225aba79127b15bd95b8f2b55c321a40309e4b Mon Sep 17 00:00:00 2001 From: anshg1214 Date: Mon, 23 Sep 2024 06:56:57 +0000 Subject: [PATCH 6/6] fix: Query client returning undefined --- frontend/js/src/common/listens/ListenCard.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frontend/js/src/common/listens/ListenCard.tsx b/frontend/js/src/common/listens/ListenCard.tsx index 2e76d6ebe6..caae54cf7b 100644 --- a/frontend/js/src/common/listens/ListenCard.tsx +++ b/frontend/js/src/common/listens/ListenCard.tsx @@ -759,16 +759,19 @@ export default function ListenCardWrapper(props: ListenCardProps) { queryKey: ["album-art", albumArtQueryKey, albumArtDisabled], queryFn: async () => { if (albumArtDisabled) { - return undefined; + return ""; } try { - return await getAlbumArtFromListenMetadata( + const albumArtURL = await getAlbumArtFromListenMetadata( listen, spotifyAuth, APIService ); + return albumArtURL ?? ""; } catch (error) { - return undefined; + // eslint-disable-next-line no-console + console.error("Error fetching album art", error); + return ""; } }, staleTime: 1000 * 60 * 60 * 12,