From dd1a00aad3794ba79dd1fd7b30eef9fa6b79b097 Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Fri, 13 Dec 2024 19:52:16 +0100 Subject: [PATCH 1/2] Fix multi-track matching modal I broke it by forgetting to rename a prop in #3062 which I didn't get a typescript error for. Took the opportunity to fix the type in LinkListens --- .../src/settings/link-listens/LinkListens.tsx | 17 ++++++------- .../MultiTrackMBIDMappingModal.tsx | 24 +++++++++---------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/frontend/js/src/settings/link-listens/LinkListens.tsx b/frontend/js/src/settings/link-listens/LinkListens.tsx index 0f6525f6e7..d8e328f3d4 100644 --- a/frontend/js/src/settings/link-listens/LinkListens.tsx +++ b/frontend/js/src/settings/link-listens/LinkListens.tsx @@ -11,7 +11,7 @@ import { Link, useLocation, useSearchParams } from "react-router-dom"; import { toast } from "react-toastify"; import { Helmet } from "react-helmet"; -import NiceModal from "@ebay/nice-modal-react"; +import NiceModal, { NiceModalHocProps } from "@ebay/nice-modal-react"; import { groupBy, isNil, isNull, pick, size, sortBy } from "lodash"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; @@ -26,6 +26,7 @@ import GlobalAppContext from "../../utils/GlobalAppContext"; import { getRecordingMSID } from "../../utils/utils"; import MultiTrackMBIDMappingModal, { MatchingTracksResults, + MultiTrackMBIDMappingModalProps, } from "./MultiTrackMBIDMappingModal"; import Accordion from "../../common/Accordion"; import { useBrainzPlayerDispatch } from "../../common/brainzplayer/BrainzPlayerContext"; @@ -258,13 +259,13 @@ export default function LinkListensPage() { style={{ padding: "0", height: "initial" }} type="button" onClick={(e) => { - NiceModal.show( - MultiTrackMBIDMappingModal, - { - unlinkedListens: group, - releaseName, - } - ).then((matchedTracks) => { + NiceModal.show< + MatchingTracksResults, + MultiTrackMBIDMappingModalProps & NiceModalHocProps + >(MultiTrackMBIDMappingModal, { + unlinkedListens: group, + releaseName, + }).then((matchedTracks) => { Object.entries(matchedTracks).forEach( ([recordingMsid, track]) => { // For deleting items from the BrainzPlayer queue, we need to use diff --git a/frontend/js/src/settings/link-listens/MultiTrackMBIDMappingModal.tsx b/frontend/js/src/settings/link-listens/MultiTrackMBIDMappingModal.tsx index 527b570b49..81533b68c4 100644 --- a/frontend/js/src/settings/link-listens/MultiTrackMBIDMappingModal.tsx +++ b/frontend/js/src/settings/link-listens/MultiTrackMBIDMappingModal.tsx @@ -32,14 +32,14 @@ export type MatchingTracksResults = { export type MultiTrackMBIDMappingModalProps = { releaseName: string | null; - missingData: Array; + unlinkedListens: Array; }; // https://lucene.apache.org/core/7_7_2/queryparser/org/apache/lucene/queryparser/classic/package-summary.html#Escaping_Special_Characters const lucineSpecialCharRegex = /[+\-!(){}[\]^"~*?:\\/]|(?:&{2})|(?:\|{2})/gm; export default NiceModal.create( - ({ missingData, releaseName }: MultiTrackMBIDMappingModalProps) => { + ({ unlinkedListens, releaseName }: MultiTrackMBIDMappingModalProps) => { const modal = useModal(); const { APIService, currentUser } = React.useContext(GlobalAppContext); const { lookupMBRelease, submitMBIDMapping } = APIService; @@ -267,15 +267,15 @@ export default NiceModal.create( keys: ["title", "artist-credit.name"], }); const newMatchingTracks: MatchingTracksResults = {}; - missingData.forEach((missingDataItem) => { - let stringToSearch = missingDataItem.recording_name; + unlinkedListens.forEach((unlinkedListensItem) => { + let stringToSearch = unlinkedListensItem.recording_name; if (includeArtistNameMatch) { - stringToSearch += ` ${missingDataItem.artist_name}`; + stringToSearch += ` ${unlinkedListensItem.artist_name}`; } const matches = fuzzysearch.search(stringToSearch); if (matches[0]) { // We have a match - newMatchingTracks[missingDataItem.recording_msid] = { + newMatchingTracks[unlinkedListensItem.recording_msid] = { ...matches[0].item, searchString: stringToSearch, }; @@ -285,7 +285,7 @@ export default NiceModal.create( } }); setMatchingTracks(newMatchingTracks); - }, [includeArtistNameMatch, missingData, potentialTracks]); + }, [includeArtistNameMatch, unlinkedListens, potentialTracks]); const removeItemFromMatches = (recordingMSID: string) => { setMatchingTracks((currentMatchingTracks) => @@ -293,14 +293,14 @@ export default NiceModal.create( ); }; - if (!missingData) { + if (!unlinkedListens) { return null; } const matchingTracksEntries = matchingTracks && Object.entries(matchingTracks); const hasMatches = Boolean(matchingTracksEntries?.length); const unmatchedItems = - missingData.filter((md) => !matchingTracks?.[md.recording_msid]) ?? []; + unlinkedListens.filter((md) => !matchingTracks?.[md.recording_msid]) ?? []; // We may need to escape or replace the Lucene search special characters // + - && || ! ( ) { } [ ] ^ " ~ * ? : \ / as described in @@ -309,20 +309,20 @@ export default NiceModal.create( lucineSpecialCharRegex, "\\$&" ); - const escapedArtistName = missingData[0]?.artist_name?.replace( + const escapedArtistName = unlinkedListens[0]?.artist_name?.replace( lucineSpecialCharRegex, "\\$&" ); let searchTerm = escapeSpecialCharacters ? escapedReleaseName : releaseName; if ( includeArtistNameSearch && - (missingData[0]?.artist_name || + (unlinkedListens[0]?.artist_name || (escapeSpecialCharacters && escapedArtistName)) ) { searchTerm += ` artist:(${ escapeSpecialCharacters ? escapedArtistName - : missingData[0]?.artist_name + : unlinkedListens[0]?.artist_name })`; } From d7402ba3928706a2fe1dfb11c41ddba8fe282e53 Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Fri, 13 Dec 2024 20:47:52 +0100 Subject: [PATCH 2/2] Improve modal typing in The type for NiceModal.show are not convenient to use, requiring importing and using the NiceModalHOC type in conjunction with the props we are passing to the modal. I couldn't find how to define a return type for the modal that we automatically be determined when passing the component to NiceModal.show. The suggest method in the forums involves using async/await instead of a promise chain to set the return type manually. Not ideal still but simpler. https://github.com/eBay/nice-modal-react/issues/127#issuecomment-1742452067 --- .../src/settings/link-listens/LinkListens.tsx | 77 +++++++++---------- 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/frontend/js/src/settings/link-listens/LinkListens.tsx b/frontend/js/src/settings/link-listens/LinkListens.tsx index d8e328f3d4..14a762b3e0 100644 --- a/frontend/js/src/settings/link-listens/LinkListens.tsx +++ b/frontend/js/src/settings/link-listens/LinkListens.tsx @@ -11,7 +11,7 @@ import { Link, useLocation, useSearchParams } from "react-router-dom"; import { toast } from "react-toastify"; import { Helmet } from "react-helmet"; -import NiceModal, { NiceModalHocProps } from "@ebay/nice-modal-react"; +import NiceModal from "@ebay/nice-modal-react"; import { groupBy, isNil, isNull, pick, size, sortBy } from "lodash"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; @@ -26,7 +26,6 @@ import GlobalAppContext from "../../utils/GlobalAppContext"; import { getRecordingMSID } from "../../utils/utils"; import MultiTrackMBIDMappingModal, { MatchingTracksResults, - MultiTrackMBIDMappingModalProps, } from "./MultiTrackMBIDMappingModal"; import Accordion from "../../common/Accordion"; import { useBrainzPlayerDispatch } from "../../common/brainzplayer/BrainzPlayerContext"; @@ -173,6 +172,40 @@ export default function LinkListensPage() { } }; + const openMultiTrackMappingModal = React.useCallback( + async (group: UnlinkedListens[], releaseName: string | null) => { + const matchedTracks: MatchingTracksResults = await NiceModal.show( + MultiTrackMBIDMappingModal, + { + unlinkedListens: group, + releaseName, + } + ); + // Remove successfully matched items from the page + setUnlinkedListens((prevValue) => + prevValue.filter((md) => !matchedTracks[md.recording_msid]) + ); + Object.entries(matchedTracks).forEach(([recordingMsid, track]) => { + // For deleting items from the BrainzPlayer queue, we need to use + // the metadata it was created from rather than the matched track metadata + const itemBeforeMatching = group.find( + ({ recording_msid }) => recordingMsid === recording_msid + ); + if (itemBeforeMatching) { + // Remove the listen from the BrainzPlayer queue + dispatch({ + type: "REMOVE_TRACK_FROM_AMBIENT_QUEUE", + data: { + track: unlinkedListenDataToListen(itemBeforeMatching, user), + index: -1, + }, + }); + } + }); + }, + [dispatch, user] + ); + // Effects React.useEffect(() => { // Set the ?page search param in URL on startup if not set, as well as @@ -258,44 +291,8 @@ export default function LinkListensPage() { className="btn btn-link btn-icon color-orange" style={{ padding: "0", height: "initial" }} type="button" - onClick={(e) => { - NiceModal.show< - MatchingTracksResults, - MultiTrackMBIDMappingModalProps & NiceModalHocProps - >(MultiTrackMBIDMappingModal, { - unlinkedListens: group, - releaseName, - }).then((matchedTracks) => { - Object.entries(matchedTracks).forEach( - ([recordingMsid, track]) => { - // For deleting items from the BrainzPlayer queue, we need to use - // the metadata it was created from rather than the matched track metadata - const itemBeforeMatching = group.find( - ({ recording_msid }) => - recordingMsid === recording_msid - ); - if (itemBeforeMatching) { - // Remove the listen from the BrainzPlayer queue - dispatch({ - type: "REMOVE_TRACK_FROM_AMBIENT_QUEUE", - data: { - track: unlinkedListenDataToListen( - itemBeforeMatching, - user - ), - index: -1, - }, - }); - } - } - ); - // Remove successfully matched items from the page - setUnlinkedListens((prevValue) => - prevValue.filter( - (md) => !matchedTracks[md.recording_msid] - ) - ); - }); + onClick={() => { + openMultiTrackMappingModal(group, releaseName); }} data-toggle="modal" data-target="#MultiTrackMBIDMappingModal"