diff --git a/listenbrainz/webserver/static/js/src/RecentListens.tsx b/listenbrainz/webserver/static/js/src/RecentListens.tsx index 033c4a14e5..53f02c843f 100644 --- a/listenbrainz/webserver/static/js/src/RecentListens.tsx +++ b/listenbrainz/webserver/static/js/src/RecentListens.tsx @@ -51,8 +51,6 @@ export interface RecentListensState { previousListenTs?: number; saveUrl: string; recordingFeedbackMap: RecordingFeedbackMap; - currRecPage?: number; - totalRecPages: number; } export default class RecentListens extends React.Component< @@ -68,16 +66,11 @@ export default class RecentListens extends React.Component< private expectedListensPerPage = 25; - private expectedRecommendationsPerPage = 25; - constructor(props: RecentListensProps) { super(props); this.state = { alerts: [], - listens: - props.mode === "cf_recs" - ? props.listens?.slice(0, this.expectedRecommendationsPerPage) || [] - : props.listens || [], + listens: props.listens || [], mode: props.mode, followList: props.followList || [], playingNowByUser: {}, @@ -90,10 +83,6 @@ export default class RecentListens extends React.Component< previousListenTs: props.listens?.[0]?.listened_at, direction: "down", recordingFeedbackMap: {}, - currRecPage: 1, - totalRecPages: props.listens - ? Math.ceil(props.listens.length / this.expectedRecommendationsPerPage) - : 0, }; this.APIService = new APIService( @@ -609,91 +598,6 @@ export default class RecentListens extends React.Component< } }; - handleClickPreviousRecommendations = () => { - const { listens } = this.props; - const { currRecPage } = this.state; - - if (currRecPage && currRecPage > 1) { - this.setState({ loading: true }); - const offset = (currRecPage - 1) * this.expectedRecommendationsPerPage; - this.setState( - { - listens: - listens?.slice( - offset - this.expectedRecommendationsPerPage, - offset - ) || [], - currRecPage: currRecPage - 1, - }, - this.afterRecommendationsDisplay - ); - window.history.pushState(null, "", `?page=${currRecPage}`); - } - }; - - handleClickNextRecommendations = () => { - const { listens } = this.props; - const { currRecPage, totalRecPages } = this.state; - - if (currRecPage && currRecPage < totalRecPages) { - this.setState({ loading: true }); - const offset = currRecPage * this.expectedRecommendationsPerPage; - this.setState( - { - listens: - listens?.slice( - offset, - offset + this.expectedRecommendationsPerPage - ) || [], - currRecPage: currRecPage + 1, - }, - this.afterRecommendationsDisplay - ); - window.history.pushState(null, "", `?page=${currRecPage}`); - } - }; - - recommendationPaginationControl = () => { - const { currRecPage, totalRecPages } = this.state; - return ( - - ); - }; - afterListensFetch() { this.checkListensRange(); if (this.listensTable?.current) { @@ -702,13 +606,6 @@ export default class RecentListens extends React.Component< this.setState({ loading: false }); } - afterRecommendationsDisplay() { - if (this.listensTable?.current) { - this.listensTable.current.scrollIntoView({ behavior: "smooth" }); - } - this.setState({ loading: false }); - } - render() { const { alerts, @@ -914,8 +811,6 @@ export default class RecentListens extends React.Component< )} - - {mode === "cf_recs" && this.recommendationPaginationControl()} )}
diff --git a/listenbrainz/webserver/static/js/src/RecentListensControls.test.tsx b/listenbrainz/webserver/static/js/src/RecentListensControls.test.tsx index 924836af7d..84de33289c 100644 --- a/listenbrainz/webserver/static/js/src/RecentListensControls.test.tsx +++ b/listenbrainz/webserver/static/js/src/RecentListensControls.test.tsx @@ -246,76 +246,3 @@ describe("removeListenFromListenList", () => { expect(wrapper.state("listens")).toMatchObject([]); }); }); - -describe("handleClickPreviousRecommendations", () => { - it("don't do anything if already on first page", async () => { - const extraProps = { ...props, mode: "cf_recs" as ListensListMode }; - const wrapper = shallow(); - const instance = wrapper.instance(); - instance.afterRecommendationsDisplay = jest.fn(); - - await instance.handleClickPreviousRecommendations(); - - expect(wrapper.state("currRecPage")).toEqual(1); - expect(wrapper.state("totalRecPages")).toEqual(3); - expect(wrapper.state("listens").length).toEqual(25); - expect(instance.afterRecommendationsDisplay).toHaveBeenCalledTimes(0); - }); - - it("go to the previous page if not on first page", async () => { - const extraProps = { ...props, mode: "cf_recs" as ListensListMode }; - const wrapper = shallow(); - const instance = wrapper.instance(); - instance.afterRecommendationsDisplay = jest.fn(); - - wrapper.setState({ - currRecPage: 3, - listens: recentListensProps.listens.slice(50, 52), - }); - - await instance.handleClickPreviousRecommendations(); - - expect(wrapper.state("currRecPage")).toEqual(2); - expect(wrapper.state("listens").length).toEqual(25); - expect(instance.afterRecommendationsDisplay).toHaveBeenCalledTimes(1); - }); -}); - -describe("handleClickNextRecommendations", () => { - it("don't do anything if already on last page", async () => { - const extraProps = { ...props, mode: "cf_recs" as ListensListMode }; - const wrapper = shallow(); - const instance = wrapper.instance(); - instance.afterRecommendationsDisplay = jest.fn(); - - wrapper.setState({ - currRecPage: 3, - listens: recentListensProps.listens.slice(50, 52), - }); - - await instance.handleClickNextRecommendations(); - - expect(wrapper.state("currRecPage")).toEqual(3); - expect(wrapper.state("totalRecPages")).toEqual(3); - expect(wrapper.state("listens").length).toEqual(2); - expect(instance.afterRecommendationsDisplay).toHaveBeenCalledTimes(0); - }); - - it("go to the next page if not on last page", async () => { - const extraProps = { ...props, mode: "cf_recs" as ListensListMode }; - const wrapper = shallow(); - const instance = wrapper.instance(); - instance.afterRecommendationsDisplay = jest.fn(); - - wrapper.setState({ - currRecPage: 2, - listens: recentListensProps.listens.slice(25, 50), - }); - - await instance.handleClickNextRecommendations(); - - expect(wrapper.state("currRecPage")).toEqual(3); - expect(wrapper.state("listens").length).toEqual(2); - expect(instance.afterRecommendationsDisplay).toHaveBeenCalledTimes(1); - }); -}); diff --git a/listenbrainz/webserver/static/js/src/listens/ListenCard.test.tsx b/listenbrainz/webserver/static/js/src/listens/ListenCard.test.tsx index 6abe178a78..52482d13cb 100644 --- a/listenbrainz/webserver/static/js/src/listens/ListenCard.test.tsx +++ b/listenbrainz/webserver/static/js/src/listens/ListenCard.test.tsx @@ -21,7 +21,6 @@ const listen: Listen = { recording_msid: "bar", }, }, - score: 2.3, }; const props: ListenCardProps = { @@ -60,14 +59,6 @@ describe("ListenCard", () => { expect(wrapper).toMatchSnapshot(); }); - it("renders correctly for mode = 'cf_recs'", () => { - const wrapper = mount( - - ); - - expect(wrapper).toMatchSnapshot(); - }); - it("renders correctly for playing_now listen", () => { const playingNowListen: Listen = { playing_now: true, ...listen }; const wrapper = mount( diff --git a/listenbrainz/webserver/static/js/src/listens/ListenCard.tsx b/listenbrainz/webserver/static/js/src/listens/ListenCard.tsx index 652361f39a..603bf7f6b7 100644 --- a/listenbrainz/webserver/static/js/src/listens/ListenCard.tsx +++ b/listenbrainz/webserver/static/js/src/listens/ListenCard.tsx @@ -142,66 +142,6 @@ export default class ListenCard extends React.Component< ); }; - handleListenedAt = () => { - const { mode, listen } = this.props; - if (mode === "cf_recs") { - return ( - - score: {listen.score ? listen.score.toString() : "Undefined"} - - ); - } - return ( - - {listen.listened_at_iso - ? timeago.ago(listen.listened_at_iso) - : timeago.ago(listen.listened_at * 1000)} - - ); - }; - - handleListenedAtMobileView = () => { - const { mode, listen } = this.props; - if (mode === "cf_recs") { - return ( - - score: {listen.score ? listen.score.toString() : "Undefined"} -   —   - - ); - } - return ( - - {` - ${ - listen.listened_at_iso - ? timeago.ago(listen.listened_at_iso, true) - : timeago.ago(listen.listened_at * 1000, true) - } - `} - ago —   - - ); - }; - render() { const { listen, mode, className, isCurrentUser } = this.props; const { feedback, isDeleted } = this.state; @@ -215,9 +155,7 @@ export default class ListenCard extends React.Component< >
Playing now ) : ( - this.handleListenedAt() + + {listen.listened_at_iso + ? timeago.ago(listen.listened_at_iso) + : timeago.ago(listen.listened_at * 1000)} + )}
@@ -265,7 +213,22 @@ export default class ListenCard extends React.Component< now — ) : ( - this.handleListenedAtMobileView() + + {` + ${ + listen.listened_at_iso + ? timeago.ago(listen.listened_at_iso, true) + : timeago.ago(listen.listened_at * 1000, true) + } + `} + ago —   + )} {getArtistLink(listen)} @@ -276,9 +239,7 @@ export default class ListenCard extends React.Component<
) : ( - mode !== "cf_recs" && ( -
- {!listen?.playing_now && ( - <> - this.submitFeedback(feedback === 1 ? 0 : 1)} - className={`${feedback === 1 ? " loved" : ""}`} - /> +
+ {!listen?.playing_now && ( + <> + this.submitFeedback(feedback === 1 ? 0 : 1)} + className={`${feedback === 1 ? " loved" : ""}`} + /> + this.submitFeedback(feedback === -1 ? 0 : -1)} + className={`${feedback === -1 ? " hated" : ""}`} + /> + +
    - this.submitFeedback(feedback === -1 ? 0 : -1) - } - className={`${feedback === -1 ? " hated" : ""}`} - /> - -
      - -
    - - )} -
- ) + + + )} +
)}
diff --git a/listenbrainz/webserver/static/js/src/listens/__snapshots__/ListenCard.test.tsx.snap b/listenbrainz/webserver/static/js/src/listens/__snapshots__/ListenCard.test.tsx.snap index 313b1b1100..514e78b389 100644 --- a/listenbrainz/webserver/static/js/src/listens/__snapshots__/ListenCard.test.tsx.snap +++ b/listenbrainz/webserver/static/js/src/listens/__snapshots__/ListenCard.test.tsx.snap @@ -1,73 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`ListenCard renders correctly for mode = 'cf_recs' 1`] = ` - - -
-
- - -
-
-
- - -`; - exports[`ListenCard renders correctly for mode = 'follow ' 1`] = ` ; -declare type ListensListMode = "listens" | "follow" | "recent" | "cf_recs"; +declare type ListensListMode = "listens" | "follow" | "recent"; declare type ListenFeedBack = 1 | 0 | -1; diff --git a/listenbrainz/webserver/templates/recommendations_cf_recording/similar_artist.html b/listenbrainz/webserver/templates/recommendations_cf_recording/similar_artist.html index d9918cc8d6..cf724b5155 100644 --- a/listenbrainz/webserver/templates/recommendations_cf_recording/similar_artist.html +++ b/listenbrainz/webserver/templates/recommendations_cf_recording/similar_artist.html @@ -19,5 +19,5 @@

Error

{% block scripts %} {{ super() }} - + {% endblock %} diff --git a/listenbrainz/webserver/templates/recommendations_cf_recording/top_artist.html b/listenbrainz/webserver/templates/recommendations_cf_recording/top_artist.html index c20bd659d1..1d937ba00d 100644 --- a/listenbrainz/webserver/templates/recommendations_cf_recording/top_artist.html +++ b/listenbrainz/webserver/templates/recommendations_cf_recording/top_artist.html @@ -19,5 +19,5 @@

Error

{% block scripts %} {{ super() }} - + {% endblock %} diff --git a/listenbrainz/webserver/views/recommendations_cf_recording.py b/listenbrainz/webserver/views/recommendations_cf_recording.py index 8800a3981a..aef7bc3b4a 100644 --- a/listenbrainz/webserver/views/recommendations_cf_recording.py +++ b/listenbrainz/webserver/views/recommendations_cf_recording.py @@ -84,8 +84,8 @@ def _get_template(active_section, user): "We are working on it. Check back later." ) - listens = _get_listens_from_recording_mbid(result) - if not listens: + recommendations = _get_playable_recommendations_list(result) + if not recommendations: current_app.logger.error('The API returned an empty response for {} recommendations.\nData: {}' .format(active_section, result)) return render_template( @@ -116,8 +116,7 @@ def _get_template(active_section, user): "spotify": spotify_data, "api_url": current_app.config["API_URL"], "web_sockets_server_url": current_app.config['WEBSOCKETS_SERVER_URL'], - "listens": listens, - "mode": "cf_recs" + "recommendations": recommendations } return render_template( @@ -129,14 +128,15 @@ def _get_template(active_section, user): ) -def _get_listens_from_recording_mbid(mbids_and_ratings_list): - """ Get listens from recording mbid using labs.listenbrainz.api. +def _get_playable_recommendations_list(mbids_and_ratings_list): + """ Get artist, track etc info from recording mbid using labs.listenbrainz.api + so that they can be played using BrainzPlayer. Refer to webserver/static/js/src/BrainzPlayer.tsx Args: mbids_and_ratings_list: Contains recording mbid and corresponding score. Returns: - listens: list of listens of the format + recommendations: list of recommendations of the format { 'listened_at' : 0, 'track_metadata' : { @@ -147,18 +147,12 @@ def _get_listens_from_recording_mbid(mbids_and_ratings_list): 'recording_mbid' : "181c4177-f33a-441d-b15d-910acaf18b07", 'artist_mbids' : "181c4177-f33a-441d-b15d-910acaf18b07" } - }, - 'score': 0.123 + } } - """ data = [] - mbids_and_ratings = {} - for r in mbids_and_ratings_list: data.append({'[recording_mbid]': r['recording_mbid']}) - # get score corresponding to recording mbid. - mbids_and_ratings[r['recording_mbid']] = r['score'] r = requests.post(SERVER_URL, json=data) if r.status_code != 200: @@ -174,24 +168,20 @@ def _get_listens_from_recording_mbid(mbids_and_ratings_list): except Exception as err: raise InternalServerError(str(err)) - listens = [] + recommendations = [] for row in rows: - if mbids_and_ratings.get(row['recording_mbid']) is not None: - listens.append({ - 'listened_at': 0, - 'track_metadata': { - 'artist_name': row['artist_credit_name'], - 'track_name': row['recording_name'], - 'release_name': row.get('release_name', ""), - 'additional_info': { - 'recording_mbid': row['recording_mbid'], - 'artist_mbids': row['[artist_credit_mbids]'] - } - }, - 'score': mbids_and_ratings[row['recording_mbid']] - }) - - listens = sorted(listens, key=lambda x: x['score'], reverse=True) + recommendations.append({ + 'listened_at': 0, + 'track_metadata': { + 'artist_name': row['artist_credit_name'], + 'track_name': row['recording_name'], + 'release_name': row.get('release_name', ""), + 'additional_info': { + 'recording_mbid': row['recording_mbid'], + 'artist_mbids': row['[artist_credit_mbids]'] + } + } + }) - return listens + return recommendations diff --git a/listenbrainz/webserver/views/test/test_recommendations_cf_recording.py b/listenbrainz/webserver/views/test/test_recommendations_cf_recording.py index ea29af8538..214523a7a2 100644 --- a/listenbrainz/webserver/views/test/test_recommendations_cf_recording.py +++ b/listenbrainz/webserver/views/test/test_recommendations_cf_recording.py @@ -142,8 +142,8 @@ def test_get_template_missing_rec_similar_artist(self): self.assert_context('user', user) @patch('listenbrainz.webserver.views.recommendations_cf_recording.db_recommendations_cf_recording.get_user_recommendation') - @patch('listenbrainz.webserver.views.recommendations_cf_recording._get_listens_from_recording_mbid') - def test_get_template_empty_repsonce_top_artist(self, mock_get_listens, mock_get_rec): + @patch('listenbrainz.webserver.views.recommendations_cf_recording._get_playable_recommendations_list') + def test_get_template_empty_repsonce_top_artist(self, mock_get_recommendations, mock_get_rec): user = _get_user('vansika_1') mock_get_rec.return_value = UserRecommendationsData(**{ @@ -156,7 +156,7 @@ def test_get_template_empty_repsonce_top_artist(self, mock_get_listens, mock_get 'created': datetime.utcnow(), 'user_id': 1 }) - mock_get_listens.return_value = [] + mock_get_recommendations.return_value = [] recommendations_cf_recording._get_template(active_section='top_artist', user=user) self.assertTemplateUsed('recommendations_cf_recording/top_artist.html') @@ -166,8 +166,8 @@ def test_get_template_empty_repsonce_top_artist(self, mock_get_listens, mock_get self.assert_context('error_msg', error_msg) @patch('listenbrainz.webserver.views.recommendations_cf_recording.db_recommendations_cf_recording.get_user_recommendation') - @patch('listenbrainz.webserver.views.recommendations_cf_recording._get_listens_from_recording_mbid') - def test_get_template_empty_repsonce_similar_artist(self, mock_get_listens, mock_get_rec): + @patch('listenbrainz.webserver.views.recommendations_cf_recording._get_playable_recommendations_list') + def test_get_template_empty_repsonce_similar_artist(self, mock_get_recommendations, mock_get_rec): user = _get_user('vansika_1') mock_get_rec.return_value = UserRecommendationsData(**{ @@ -180,7 +180,7 @@ def test_get_template_empty_repsonce_similar_artist(self, mock_get_listens, mock 'created': datetime.utcnow(), 'user_id': 1 }) - mock_get_listens.return_value = [] + mock_get_recommendations.return_value = [] recommendations_cf_recording._get_template(active_section='similar_artist', user=user) self.assertTemplateUsed('recommendations_cf_recording/similar_artist.html') @@ -192,8 +192,8 @@ def test_get_template_empty_repsonce_similar_artist(self, mock_get_listens, mock @patch('listenbrainz.webserver.views.recommendations_cf_recording.spotify.get_user_dict') @patch('listenbrainz.webserver.views.recommendations_cf_recording.current_user') @patch('listenbrainz.webserver.views.recommendations_cf_recording.db_recommendations_cf_recording.get_user_recommendation') - @patch('listenbrainz.webserver.views.recommendations_cf_recording._get_listens_from_recording_mbid') - def test_get_template(self, mock_get_listens, mock_get_rec, mock_curr_user, mock_spotify_dict): + @patch('listenbrainz.webserver.views.recommendations_cf_recording._get_playable_recommendations_list') + def test_get_template(self, mock_get_recommendations, mock_get_rec, mock_curr_user, mock_spotify_dict): # active_section = 'top_artist' user = _get_user('vansika_1') created = datetime.utcnow() @@ -209,7 +209,7 @@ def test_get_template(self, mock_get_listens, mock_get_rec, mock_curr_user, mock 'user_id': 1 }) - listens = [{ + recommendations = [{ 'listened_at': 0, 'track_metadata': { 'artist_name': "Ultravox", @@ -219,10 +219,9 @@ def test_get_template(self, mock_get_listens, mock_get_rec, mock_curr_user, mock 'recording_mbid': "af5a56f4-1f83-4681-b319-70a734d0d047", 'artist_mbids': ["6a70b322-9aa9-41b3-9dce-824733633a1c"] } - }, - 'score': 0.756 + } }] - mock_get_listens.return_value = listens + mock_get_recommendations.return_value = recommendations spotify_dict = {'user': 10} mock_spotify_dict.return_value = spotify_dict @@ -233,7 +232,7 @@ def test_get_template(self, mock_get_listens, mock_get_rec, mock_curr_user, mock recommendations_cf_recording._get_template(active_section='top_artist', user=user) mock_get_rec.assert_called_with(user.id) - mock_get_listens.assert_called_once() + mock_get_recommendations.assert_called_once() mock_spotify_dict.assert_called_with(10) self.assertTemplateUsed('recommendations_cf_recording/top_artist.html') self.assert_context('active_section', 'top_artist') @@ -253,8 +252,7 @@ def test_get_template(self, mock_get_listens, mock_get_rec, mock_curr_user, mock "spotify": spotify_dict, "api_url": current_app.config["API_URL"], "web_sockets_server_url": current_app.config['WEBSOCKETS_SERVER_URL'], - "listens": listens, - "mode": "cf_recs" + "recommendations": recommendations } received_props = ujson.loads(self.get_context_variable('props')) self.assertEqual(expected_props, received_props) @@ -280,7 +278,7 @@ def test_get_template(self, mock_get_listens, mock_get_rec, mock_curr_user, mock @patch('listenbrainz.webserver.views.recommendations_cf_recording.requests') - def test_get_listens_from_recording_mbid(self, mock_requests): + def test_get_playable_recommendations_list(self, mock_requests): mbids_and_ratings = [ { 'recording_mbid': "03f1b16a-af43-4cd7-b22c-d2991bf011a3", @@ -320,43 +318,41 @@ def test_get_listens_from_recording_mbid(self, mock_requests): mock_requests.post().text = ujson.dumps(text) mock_requests.post().status_code = 200 - received_listens = recommendations_cf_recording._get_listens_from_recording_mbid(mbids_and_ratings) + received_recommendations = recommendations_cf_recording._get_playable_recommendations_list(mbids_and_ratings) mock_requests.post.assert_called_with(self.server_url, json=data) - expected_listens = [ + expected_recommendations = [ { 'listened_at': 0, 'track_metadata': { 'artist_name': 'Tame Impala', - 'track_name': 'Sun’s Coming Up', + 'track_name': 'One More Hour', 'release_name': '', 'additional_info': { - 'recording_mbid': '2c8412f0-9353-48a2-aedb-1ad8dac9498f', - 'artist_mbids': ['63aa26c3-d59b-4da4-84ac-716b54f1ef4d'] + 'recording_mbid': '03f1b16a-af43-4cd7-b22c-d2991bf011a3', + 'artist_mbids': ['63aa26c3-d59b-4da4-84ac-716b54f1ef4d'] } - }, - 'score': 9.0 + } }, { 'listened_at': 0, 'track_metadata': { 'artist_name': 'Tame Impala', - 'track_name': 'One More Hour', + 'track_name': 'Sun’s Coming Up', 'release_name': '', 'additional_info': { - 'recording_mbid': '03f1b16a-af43-4cd7-b22c-d2991bf011a3', - 'artist_mbids': ['63aa26c3-d59b-4da4-84ac-716b54f1ef4d'] + 'recording_mbid': '2c8412f0-9353-48a2-aedb-1ad8dac9498f', + 'artist_mbids': ['63aa26c3-d59b-4da4-84ac-716b54f1ef4d'] } - }, - 'score': 6.88 + } } ] - self.assertEqual(expected_listens, received_listens) + self.assertEqual(expected_recommendations, received_recommendations) mock_requests.post().status_code = 400 with self.assertRaises(BadRequest): - recommendations_cf_recording._get_listens_from_recording_mbid(mbids_and_ratings) + recommendations_cf_recording._get_playable_recommendations_list(mbids_and_ratings) mock_requests.post().status_code = 304 with self.assertRaises(InternalServerError): - recommendations_cf_recording._get_listens_from_recording_mbid(mbids_and_ratings) + recommendations_cf_recording._get_playable_recommendations_list(mbids_and_ratings)