From 2b959f7803b2ee63cd21ac925e1e389009c7c624 Mon Sep 17 00:00:00 2001 From: James Barnsley Date: Fri, 17 Nov 2017 08:32:12 +1300 Subject: [PATCH 1/4] Enforcing valid URIs --- src/js/components/Modal/EditRadioModal.js | 83 +++++++++++++++++------ src/js/services/pusher/middleware.js | 7 +- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/src/js/components/Modal/EditRadioModal.js b/src/js/components/Modal/EditRadioModal.js index 84260fb7..d8109b7b 100755 --- a/src/js/components/Modal/EditRadioModal.js +++ b/src/js/components/Modal/EditRadioModal.js @@ -26,15 +26,43 @@ export default class EditRadioModal extends React.Component{ } handleStart(e){ - e.preventDefault() - this.props.pusherActions.startRadio(this.state.seeds) - this.props.uiActions.closeModal() + e.preventDefault(); + + var valid_seeds = true; + var seeds = this.mapSeeds(); + for (var i = 0; i < seeds.length; i++){ + if (seeds[i].unresolved !== undefined){ + valid_seeds = false; + continue; + } + } + + if (valid_seeds){ + this.props.pusherActions.startRadio(this.state.seeds); + this.props.uiActions.closeModal(); + } else { + this.setState({error_message: "Remove all invalid seed URIs"}); + } } handleUpdate(e){ - e.preventDefault() - this.props.pusherActions.updateRadio(this.state.seeds) - this.props.uiActions.closeModal() + e.preventDefault(); + + var valid_seeds = true; + var seeds = this.mapSeeds(); + for (var i = 0; i < seeds.length; i++){ + if (seeds[i].unresolved !== undefined){ + valid_seeds = false; + continue; + } + } + + if (valid_seeds){ + this.props.pusherActions.updateRadio(this.state.seeds); + this.props.uiActions.closeModal(); + } else { + this.setState({error_message: "Remove all invalid seed URIs"}); + } } handleStop(e){ @@ -45,20 +73,31 @@ export default class EditRadioModal extends React.Component{ addSeed(){ if (this.state.uri == ''){ - this.setState({error_message: 'Cannot be empty'}) - return null + this.setState({error_message: 'Cannot be empty'}); + return null; } - var seeds = Object.assign([],this.state.seeds) + var seeds = Object.assign([], this.state.seeds) var uris = this.state.uri.split(',') for (var i = 0; i < uris.length; i++){ if (seeds.indexOf(uris[i]) > -1){ - this.setState({error_message: 'URI already added'}) + this.setState({error_message: 'URI already added'}); } else { - seeds.push(uris[i]) - this.setState({error_message: null}) - } + seeds.push(uris[i]); + this.setState({error_message: null}); + } + + // Resolve + switch (helpers.uriType(uris[i])){ + case 'track': + this.props.spotifyActions.getTrack(uris[i]); + break; + + case 'artist': + this.props.spotifyActions.getArtist(uris[i]); + break; + } } // commit to state @@ -75,10 +114,10 @@ export default class EditRadioModal extends React.Component{ seeds.push(this.state.seeds[i]) } } - this.setState({seeds: seeds}) + this.setState({seeds: seeds}); } - renderSeeds(){ + mapSeeds(){ var seeds = [] if (this.state.seeds){ @@ -87,20 +126,18 @@ export default class EditRadioModal extends React.Component{ if (uri){ if (helpers.uriType(uri) == 'artist'){ if (this.props.artists && this.props.artists.hasOwnProperty(uri)){ - seeds.push(this.props.artists[uri]) + seeds.push(this.props.artists[uri]); } else { seeds.push({ - type: 'artist', unresolved: true, uri: uri }) } } else if (helpers.uriType(uri) == 'track'){ if (this.props.tracks && this.props.tracks.hasOwnProperty(uri)){ - seeds.push(this.props.tracks[uri]) + seeds.push(this.props.tracks[uri]); } else { seeds.push({ - type: 'track', unresolved: true, uri: uri }) @@ -110,6 +147,12 @@ export default class EditRadioModal extends React.Component{ } } + return seeds; + } + + renderSeeds(){ + var seeds = this.mapSeeds(); + if (seeds.length > 0){ return (
@@ -119,7 +162,7 @@ export default class EditRadioModal extends React.Component{ return (
{seed.unresolved ? {seed.uri} : {seed.name} } -  ({seed.type}) + {!seed.unresolved ?  ({seed.type}) : null} diff --git a/src/js/services/pusher/middleware.js b/src/js/services/pusher/middleware.js index a2ea9fa2..fd75fa3c 100755 --- a/src/js/services/pusher/middleware.js +++ b/src/js/services/pusher/middleware.js @@ -373,12 +373,13 @@ const PusherMiddleware = (function(){ request(store, 'change_radio', data) .then( response => { + store.dispatch(uiActions.processFinished('PUSHER_RADIO_PROCESS')); if (response.status == 0){ - store.dispatch(uiActions.createNotification(response.message, 'bad')) + store.dispatch(uiActions.createNotification(response.message, 'bad')); } - store.dispatch(uiActions.processFinished('PUSHER_RADIO_PROCESS')) }, - error => { + error => { + store.dispatch(uiActions.processFinished('PUSHER_RADIO_PROCESS')); store.dispatch(coreActions.handleException( 'Could not change radio', error From 67ba8420141cdf2798e8378a88f3c1c96d2617ce Mon Sep 17 00:00:00 2001 From: James Barnsley Date: Tue, 21 Nov 2017 08:07:00 +1300 Subject: [PATCH 2/4] Licence button --- src/js/views/Settings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/views/Settings.js b/src/js/views/Settings.js index 3ee2b728..1a650d28 100755 --- a/src/js/views/Settings.js +++ b/src/js/views/Settings.js @@ -466,7 +466,7 @@ class Settings extends React.Component {  GitHub    - Creative Commons License +  Licence
From 62e50e89b766b768149f5f598b03982663251b0e Mon Sep 17 00:00:00 2001 From: James Barnsley Date: Tue, 21 Nov 2017 11:27:34 +1300 Subject: [PATCH 3/4] 30s timeout for pusher; No broadcasting after metadata flush; Extra error logging during radio functions --- mopidy_iris/core.py | 106 ++++++++++++++------------- mopidy_iris/frontend.py | 4 +- src/js/services/pusher/middleware.js | 2 +- 3 files changed, 57 insertions(+), 55 deletions(-) diff --git a/mopidy_iris/core.py b/mopidy_iris/core.py index a13e4db1..f4133e43 100755 --- a/mopidy_iris/core.py +++ b/mopidy_iris/core.py @@ -372,39 +372,53 @@ class IrisCore(object): # We only want to play the first batch added = self.core.tracklist.add(uris = uris[0:3]) + if (not added.get()): + logger.error("No recommendations added to queue") + + self.radio['enabled'] = 0; + error = { + 'message': 'No recommendations added to queue', + 'radio': self.radio + } + if (callback): + callback(False, error) + else: + return error + # Save results (minus first batch) for later use self.radio['results'] = uris[3:] - if added.get(): - if starting: - self.core.playback.play() - self.broadcast( - data={ - 'type': 'radio_started', - 'radio': self.radio - } - ) - else: - self.broadcast( - data={ - 'type': 'radio_changed', - 'radio': self.radio - } - ) + if starting: + self.core.playback.play() + self.broadcast( + data={ + 'type': 'radio_started', + 'radio': self.radio + } + ) + else: + self.broadcast( + data={ + 'type': 'radio_changed', + 'radio': self.radio + } + ) - self.get_radio(callback=callback) - return + self.get_radio(callback=callback) + return - # failed fetching/adding tracks, so no-go - self.radio['enabled'] = 0; - error = { - 'message': 'Could not start radio', - 'radio': self.radio - } - if (callback): - callback(False, error) + # Failed fetching/adding tracks, so no-go else: - return error + logger.error("No recommendations returned by Spotify") + self.radio['enabled'] = 0; + error = { + 'message': 'Could not start radio', + 'radio': self.radio + } + if (callback): + callback(False, error) + else: + return error def stop_radio(self, *args, **kwargs): @@ -440,13 +454,10 @@ class IrisCore(object): def load_more_tracks(self, *args, **kwargs): - # this is crude, but it means we don't need to handle expired tokens - # TODO: address this when it's clear what Jodal and the team want to do with Pyspotify - self.refresh_spotify_token() - try: - token = self.spotify_token - token = token['access_token'] + self.get_spotify_token() + spotify_token = self.spotify_token + access_token = spotify_token['access_token'] except: error = 'IrisFrontend: access_token missing or invalid' logger.error(error) @@ -460,7 +471,7 @@ class IrisCore(object): url = url+'&limit=50' req = urllib2.Request(url) - req.add_header('Authorization', 'Bearer '+self.spotify_token['access_token']) + req.add_header('Authorization', 'Bearer '+access_token) response = urllib2.urlopen(req, timeout=30).read() response_dict = json.loads(response) @@ -554,21 +565,6 @@ class IrisCore(object): self.queue_metadata = cleaned_queue_metadata - self.broadcast( - data={ - 'type': 'queue_metadata_changed', - 'queue_metadata': self.queue_metadata - } - ) - - response = { - 'message': 'Cleaned queue metadata' - } - if (callback): - callback(response) - else: - return response - ## # Spotify authentication @@ -580,6 +576,11 @@ class IrisCore(object): def get_spotify_token(self, *args, **kwargs): callback = kwargs.get('callback', False) + + # Expired, so go get a new one + if (not self.spotify_token or self.spotify_token['expires_at'] <= time.time()): + self.refresh_spotify_token() + response = { 'spotify_token': self.spotify_token } @@ -606,6 +607,10 @@ class IrisCore(object): request = tornado.httpclient.HTTPRequest(url, method='POST', body=urllib.urlencode(data)) response = http_client.fetch(request) + token = json.loads(response.body) + token['expires_at'] = time.time() + token['expires_in'] + self.spotify_token = token + self.broadcast( data={ 'type': 'spotify_token_changed', @@ -613,12 +618,9 @@ class IrisCore(object): } ) - token = json.loads(response.body) - self.spotify_token = token response = { 'spotify_token': token } - if (callback): callback(response) else: diff --git a/mopidy_iris/frontend.py b/mopidy_iris/frontend.py index 1d35f2e0..6e4e0b90 100755 --- a/mopidy_iris/frontend.py +++ b/mopidy_iris/frontend.py @@ -19,9 +19,9 @@ class IrisFrontend(pykka.ThreadingActor, CoreListener): def on_start(self): logger.info('Starting Iris '+mem.iris.version) - def track_playback_ended( self, tl_track, time_position ): + def track_playback_ended(self, tl_track, time_position): mem.iris.check_for_radio_update() - def tracklist_changed( self ): + def tracklist_changed(self): mem.iris.clean_queue_metadata() \ No newline at end of file diff --git a/src/js/services/pusher/middleware.js b/src/js/services/pusher/middleware.js index fd75fa3c..b001f4fa 100755 --- a/src/js/services/pusher/middleware.js +++ b/src/js/services/pusher/middleware.js @@ -83,7 +83,7 @@ const PusherMiddleware = (function(){ store.dispatch(uiActions.stopLoading(request_id)); reject({message: "Request timed out", method: method, data: data}); }, - 5000 // 30000 + 30000 ); // add query to our deferred responses From e331afcbc905d4bcc4a3c4a23bc18543c34e6c66 Mon Sep 17 00:00:00 2001 From: James Barnsley Date: Tue, 21 Nov 2017 18:55:26 +1300 Subject: [PATCH 4/4] Padding on list wrapper; Track action zones --- src/js/components/List.js | 16 +++++++++------- src/js/components/Track.js | 4 +++- src/scss/components/_lists.scss | 25 +++++++++++++++++-------- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/js/components/List.js b/src/js/components/List.js index a4eeb98a..672a1a55 100755 --- a/src/js/components/List.js +++ b/src/js/components/List.js @@ -39,12 +39,14 @@ class List extends React.Component{ return (
- { - this.props.columns.map((col, col_index) => { - var className = 'col '+col.name.replace('.','_') - return
{ col.label ? col.label : col.name }
- }) - } +
+ { + this.props.columns.map((col, col_index) => { + var className = 'col '+col.name.replace('.','_') + return
{ col.label ? col.label : col.name }
+ }) + } +
) } @@ -54,7 +56,7 @@ class List extends React.Component{ var value = row for (var i = 0; i < key.length; i++){ - if (typeof(value[key[i]]) === 'undefined'){ + if (value[key[i]] === undefined){ return - } else if (typeof(value[key[i]]) === 'string' && value[key[i]].replace(' ','') == ''){ return - diff --git a/src/js/components/Track.js b/src/js/components/Track.js index 021426f7..6b250463 100755 --- a/src/js/components/Track.js +++ b/src/js/components/Track.js @@ -262,7 +262,9 @@ export default class Track extends React.Component{ return (
{track_actions} - {track_columns} +
+ {track_columns} +
) } else { diff --git a/src/scss/components/_lists.scss b/src/scss/components/_lists.scss index 5aa41371..25e1764a 100755 --- a/src/scss/components/_lists.scss +++ b/src/scss/components/_lists.scss @@ -1,7 +1,6 @@ .list { .list-item { - @include clearfix; -webkit-touch-callout: none; -webkit-user-select: none; @@ -12,13 +11,17 @@ display: block; position: relative; - padding: 14px 30px 13px 10px; margin: 0 -10px -1px -10px; cursor: pointer; border-radius: 3px; border-bottom: 1px solid rgba(255,255,255,0.05); border-top: 1px solid rgba(255,255,255,0.05); + .liner { + @include clearfix; + padding: 14px 30px 13px 10px; + } + &.selected { background: rgba(255,255,255,0.08) !important; @@ -229,11 +232,11 @@ .list-item { &.can-sort { - padding-left: 70px !important; + padding-left: 60px !important; } &:not(.can-sort){ - padding-left: 42px !important; + padding-left: 35px !important; } .select-zone { @@ -247,8 +250,9 @@ .fa { position: absolute; - top: 27px; + top: 50%; left: 17px; + margin-top: -3px; pointer-events: none; color: $white; z-index: 1; @@ -262,8 +266,9 @@ width: 14px; height: 14px; position: absolute; - top: 23px; + top: 50%; left: 14px; + margin-top: -7px; } } @@ -279,8 +284,9 @@ .fa { position: absolute; - top: 24px; + top: 50%; left: 6px; + margin-top: -6px; pointer-events: none; } } @@ -290,7 +296,10 @@ @include responsive($bp_medium){ .list-item { - padding: 12px !important; + + .liner { + padding: 12px !important; + } .source { position: static;