Skip to content

Commit

Permalink
BrainzPlayer: Automatically detect listen datasource and use youtubeI…
Browse files Browse the repository at this point in the history
…D if available (#882)

* BrainzPlayer: Automatically detect listen datasource and use youtubeId if available

With tests.

* Add a TODO

Co-authored-by: Param Singh <[email protected]>
  • Loading branch information
MonkeyDo and paramsingh authored Jun 5, 2020
1 parent e39c2f2 commit 239b305
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 10 deletions.
78 changes: 78 additions & 0 deletions listenbrainz/webserver/static/js/src/BrainzPlayer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,84 @@ describe("BrainzPlayer", () => {
expect(instance.dataSources[0].current).toBeInstanceOf(YoutubePlayer);
});

it("selects Youtube as source when listen has a youtube URL", () => {
const mockProps = {
...props,
spotifyUser: {
access_token: "haveyouseenthefnords",
permission: "streaming user-read-email user-read-private" as SpotifyPermission,
},
};
const wrapper = mount<BrainzPlayer>(<BrainzPlayer {...mockProps} />);
const instance = wrapper.instance();
// Normally, Spotify datasource is selected by default
expect(instance.dataSources).toHaveLength(2);
expect(instance.dataSources[1].current).toBeInstanceOf(YoutubePlayer);
const youtubeListen: Listen = {
listened_at: 0,
track_metadata: {
artist_name: "Moondog",
track_name: "Bird's Lament",
additional_info: {
origin_url: "https://www.youtube.com/watch?v=RW8SBwGNcF8",
},
},
};
// if origin_url is a youtube link, it should play it with YoutubePlayer instead of Spotify
instance.playListen(youtubeListen);
expect(instance.state.currentDataSourceIndex).toEqual(1);
});

it("selects Spotify as source when listen has listening_from = spotify", () => {
const mockProps = {
...props,
spotifyUser: {
access_token: "haveyouseenthefnords",
permission: "streaming user-read-email user-read-private" as SpotifyPermission,
},
};
const wrapper = mount<BrainzPlayer>(<BrainzPlayer {...mockProps} />);
const instance = wrapper.instance();
const spotifyListen: Listen = {
listened_at: 0,
track_metadata: {
artist_name: "Moondog",
track_name: "Bird's Lament",
additional_info: {
listening_from: "spotify",
},
},
};
// Try to play on youtube directly (index 1), should use spotify instead (index 0)
instance.playListen(spotifyListen, 1);
expect(instance.state.currentDataSourceIndex).toEqual(0);
});

it("selects Spotify as source when listen has a spotify_id", () => {
const mockProps = {
...props,
spotifyUser: {
access_token: "haveyouseenthefnords",
permission: "streaming user-read-email user-read-private" as SpotifyPermission,
},
};
const wrapper = mount<BrainzPlayer>(<BrainzPlayer {...mockProps} />);
const instance = wrapper.instance();
const spotifyListen: Listen = {
listened_at: 0,
track_metadata: {
artist_name: "Moondog",
track_name: "Bird's Lament",
additional_info: {
spotify_id: "doesn't matter in this test as long as it's here",
},
},
};
// Try to play on youtube directly (index 1), should use spotify instead (index 0)
instance.playListen(spotifyListen, 1);
expect(instance.state.currentDataSourceIndex).toEqual(0);
});

describe("isCurrentListen", () => {
it("returns true if currentListen and passed listen is same", () => {
const wrapper = shallow<BrainzPlayer>(<BrainzPlayer {...props} />);
Expand Down
44 changes: 42 additions & 2 deletions listenbrainz/webserver/static/js/src/BrainzPlayer.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import * as React from "react";
import { isEqual as _isEqual, get as _get } from "lodash";
import {
isEqual as _isEqual,
isString as _isString,
get as _get,
} from "lodash";
import * as _ from "lodash";
import PlaybackControls from "./PlaybackControls";
import APIService from "./APIService";
Expand Down Expand Up @@ -186,12 +190,48 @@ export default class BrainzPlayer extends React.Component<
};

playListen = (listen: Listen, datasourceIndex: number = 0): void => {
let selectedDatasourceIndex = datasourceIndex;
const { onCurrentListenChange } = this.props;
onCurrentListenChange(listen);
if (this.firstRun) {
this.firstRun = false;
}
this.setState({ currentDataSourceIndex: datasourceIndex }, () => {

/** If available, retreive the service the listen was listened with */
// TODO: this should be moved to a different function eventually when
// the logic gets too much for one function
const listeningFrom = _get(
listen,
"track_metadata.additional_info.listening_from"
);
const originURL = _get(listen, "track_metadata.additional_info.origin_url");

/** Spotify */
if (
listeningFrom === "spotify" ||
_get(listen, "track_metadata.additional_info.spotify_id")
) {
selectedDatasourceIndex = this.dataSources.findIndex(
(ds) => ds.current instanceof SpotifyPlayer
);
}

/** Youtube */
if (
listeningFrom === "youtube" ||
/youtube\.com\/watch\?/.test(originURL)
) {
selectedDatasourceIndex = this.dataSources.findIndex(
(ds) => ds.current instanceof YoutubePlayer
);
}

/** If no matching datasource was found, revert to the default bahaviour */
if (selectedDatasourceIndex === -1) {
selectedDatasourceIndex = datasourceIndex;
}

this.setState({ currentDataSourceIndex: selectedDatasourceIndex }, () => {
const { currentDataSourceIndex } = this.state;
const dataSource =
this.dataSources[currentDataSourceIndex] &&
Expand Down
25 changes: 25 additions & 0 deletions listenbrainz/webserver/static/js/src/SpotifyPlayer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,31 @@ describe("SpotifyPlayer", () => {
expect(wrapper).toMatchSnapshot();
});

it("should play from spotify_id if it exists on the listen", () => {
const spotifyListen: Listen = {
listened_at: 0,
track_metadata: {
artist_name: "Moondog",
track_name: "Bird's Lament",
additional_info: {
spotify_id: "https://open.spotify.com/track/surprise!",
},
},
};
const wrapper = shallow<SpotifyPlayer>(<SpotifyPlayer {...props} />);
const instance = wrapper.instance();

instance.playSpotifyURI = jest.fn();
instance.searchAndPlayTrack = jest.fn();
// play listen should extract the spotify track ID
instance.playListen(spotifyListen);
expect(instance.playSpotifyURI).toHaveBeenCalledTimes(1);
expect(instance.playSpotifyURI).toHaveBeenCalledWith(
"spotify:track:surprise!"
);
expect(instance.searchAndPlayTrack).not.toHaveBeenCalled();
});

describe("checkSpotifyToken", () => {
it("calls onInvalidateDataSource (via handleAccountError) if no access token or no permission", () => {
const onInvalidateDataSource = jest.fn();
Expand Down
20 changes: 20 additions & 0 deletions listenbrainz/webserver/static/js/src/YoutubePlayer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,24 @@ describe("YoutubePlayer", () => {
expect(instance.props.onPlayerPausedChange).toHaveBeenCalledTimes(1);
expect(instance.props.onPlayerPausedChange).toHaveBeenCalledWith(false);
});

it("should play from youtube URL if present on the listen", () => {
const wrapper = shallow<YoutubePlayer>(<YoutubePlayer {...props} />);
const instance = wrapper.instance();
const playTrackById = jest.fn();
instance.playTrackById = playTrackById;
const youtubeListen: Listen = {
listened_at: 0,
track_metadata: {
artist_name: "Moondog",
track_name: "Bird's Lament",
additional_info: {
origin_url: "https://www.youtube.com/watch?v=RW8SBwGNcF8",
},
},
};
instance.playListen(youtubeListen);
expect(playTrackById).toHaveBeenCalledTimes(1);
expect(playTrackById).toHaveBeenCalledWith("RW8SBwGNcF8");
});
});
25 changes: 17 additions & 8 deletions listenbrainz/webserver/static/js/src/YoutubePlayer.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import * as React from "react";
import YouTube, { Options } from "react-youtube";
import { isEqual as _isEqual, get as _get, isNil as _isNil } from "lodash";
import {
isEqual as _isEqual,
get as _get,
isNil as _isNil,
isString as _isString,
} from "lodash";
import { DataSourceType, DataSourceProps } from "./BrainzPlayer";

type YoutubePlayerState = {
Expand Down Expand Up @@ -104,13 +109,17 @@ export default class YoutubePlayer
if (!show) {
return;
}
const youtubeURI = _get(
listen,
"track_metadata.additional_info.youtube_id"
);

if (youtubeURI) {
this.playTrackById(youtubeURI);
let youtubeId = _get(listen, "track_metadata.additional_info.youtube_id");
const originURL = _get(listen, "track_metadata.additional_info.origin_url");
if (!youtubeId && _isString(originURL) && originURL.length) {
const parsedURL = new URL(originURL);
const { hostname, searchParams } = parsedURL;
if (/youtube\.com/.test(hostname)) {
youtubeId = searchParams.get("v");
}
}
if (youtubeId) {
this.playTrackById(youtubeId);
} else {
this.searchAndPlayTrack(listen);
}
Expand Down
1 change: 1 addition & 0 deletions listenbrainz/webserver/static/js/src/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface AdditionalInfo {
spotify_artist_ids?: Array<string> | null;
spotify_id?: string | null;
youtube_id?: string | null;
origin_url?: string | null;
tags?: Array<string> | null;
track_mbid?: string | null;
tracknumber?: number | null;
Expand Down

0 comments on commit 239b305

Please sign in to comment.