From cfdb4034d189d490b7a8379d0fe80ec2481cadcc Mon Sep 17 00:00:00 2001
From: An Phan <an@construct.sg>
Date: Tue, 5 Apr 2016 15:38:10 +0800
Subject: [PATCH] Refactor

---
 app/Models/File.php                           |  7 +-
 app/Models/Song.php                           |  6 +-
 resources/assets/js/services/playback.js      |  2 +-
 resources/assets/js/stores/album.js           | 36 ++++---
 resources/assets/js/stores/artist.js          | 94 ++++++++++++-------
 resources/assets/js/stores/favorite.js        | 36 ++++---
 resources/assets/js/stores/playlist.js        | 36 ++++++-
 resources/assets/js/stores/queue.js           |  3 +-
 resources/assets/js/stores/song.js            | 29 +++---
 resources/assets/js/stores/user.js            | 21 +++--
 resources/assets/js/tests/stores/albumTest.js |  6 +-
 .../assets/js/tests/stores/artistTest.js      |  6 +-
 12 files changed, 186 insertions(+), 96 deletions(-)

diff --git a/app/Models/File.php b/app/Models/File.php
index 2bd2ead6..5ee264bc 100644
--- a/app/Models/File.php
+++ b/app/Models/File.php
@@ -62,12 +62,7 @@ class File
      */
     public function __construct($path, $getID3 = null)
     {
-        if ($path instanceof SplFileInfo) {
-            $this->splFileInfo = $path;
-        } else {
-            $this->splFileInfo = new SplFileInfo($path);
-        }
-
+        $this->splFileInfo = $path instanceof SplFileInfo ? $path : new SplFileInfo($path);
         $this->setGetID3($getID3);
         $this->mtime = $this->splFileInfo->getMTime();
         $this->path = $this->splFileInfo->getPathname();
diff --git a/app/Models/Song.php b/app/Models/Song.php
index 2455e58d..dd3d460a 100644
--- a/app/Models/Song.php
+++ b/app/Models/Song.php
@@ -237,9 +237,9 @@ class Song extends Model
      */
     public function getLyricsAttribute($value)
     {
-        // We don't use nl2br() here, because the function actually preserve linebreaks -
-        // it just _appends_ a "<br />" after each of them. This would case our client
-        // implementation of br2nl fails with duplicated linebreaks.
+        // We don't use nl2br() here, because the function actually preserves linebreaks -
+        // it just _appends_ a "<br />" after each of them. This would cause our client
+        // implementation of br2nl to fail with duplicated linebreaks.
         return str_replace(["\r\n", "\r", "\n"], '<br />', $value);
     }
 }
diff --git a/resources/assets/js/services/playback.js b/resources/assets/js/services/playback.js
index 9735adb8..faaf802d 100644
--- a/resources/assets/js/services/playback.js
+++ b/resources/assets/js/services/playback.js
@@ -1,12 +1,12 @@
 import { shuffle } from 'lodash';
 import $ from 'jquery';
+import plyr from 'plyr';
 
 import queueStore from '../stores/queue';
 import songStore from '../stores/song';
 import artistStore from '../stores/artist';
 import preferenceStore from '../stores/preference';
 import config from '../config';
-import plyr from 'plyr';
 
 export default {
     app: null,
diff --git a/resources/assets/js/stores/album.js b/resources/assets/js/stores/album.js
index de3fd158..9bdbb2e5 100644
--- a/resources/assets/js/stores/album.js
+++ b/resources/assets/js/stores/album.js
@@ -29,7 +29,7 @@ export default {
      */
     init(artists) {
         // Traverse through the artists array and add their albums into our master album list.
-        this.state.albums = reduce(artists, (albums, artist) => {
+        this.all = reduce(artists, (albums, artist) => {
             // While we're doing so, for each album, we get its length
             // and keep a back reference to the artist too.
             each(artist.albums, album => {
@@ -40,7 +40,7 @@ export default {
         }, []);
 
         // Then we init the song store.
-        songStore.init(this.state.albums);
+        songStore.init(this.all);
     },
 
     setupAlbum(album, artist) {
@@ -61,6 +61,15 @@ export default {
         return this.state.albums;
     },
 
+    /**
+     * Set all albums.
+     *
+     * @param  {Array.<Object>} value
+     */
+    set all(value) {
+        this.state.albums = value;
+    },
+
     byId(id) {
         return find(this.all, { id });
     },
@@ -81,13 +90,18 @@ export default {
     },
 
     /**
-     * Appends a new album into the current collection.
+     * Add new album/albums into the current collection.
      *
-     * @param  {Object} album
+     * @param  {Array.<Object>|Object} albums
      */
-    append(album) {
-        this.state.albums.push(this.setupAlbum(album, album.artist));
-        album.playCount = reduce(album.songs, (count, song) => count + song.playCount, 0);
+    add(albums) {
+        albums = [].concat(albums);
+        each(albums, a => {
+            this.setupAlbum(a, a.artist)
+            a.playCount = reduce(a.songs, (count, song) => count + song.playCount, 0);
+        });
+
+        this.all = union(this.all, albums);
     },
 
     /**
@@ -101,7 +115,7 @@ export default {
 
         album.songs = union(album.songs ? album.songs : [], songs);
 
-        songs.forEach(song => {
+        each(songs, song => {
             song.album_id = album.id;
             song.album = album;
         });
@@ -140,10 +154,10 @@ export default {
      */
     remove(albums) {
         albums = [].concat(albums);
-        this.state.albums = difference(this.state.albums, albums);
+        this.all = difference(this.all, albums);
 
         // Remove from the artist as well
-        albums.forEach(album => {
+        each(albums, album => {
             artistStore.removeAlbumsFromArtist(album.artist, album);
         });
     },
@@ -157,7 +171,7 @@ export default {
      */
     getMostPlayed(n = 6) {
         // Only non-unknown albums with actually play count are applicable.
-        const applicable = filter(this.state.albums, album => {
+        const applicable = filter(this.all, album => {
             return album.playCount && album.id !== 1;
         });
 
diff --git a/resources/assets/js/stores/artist.js b/resources/assets/js/stores/artist.js
index fe8fb1d5..0ebb82a8 100644
--- a/resources/assets/js/stores/artist.js
+++ b/resources/assets/js/stores/artist.js
@@ -27,14 +27,14 @@ export default {
      * @param  {Array.<Object>} artists The array of artists we got from the server.
      */
     init(artists) {
-        this.state.artists = artists;
+        this.all = artists;
 
         // Traverse through artists array to get the cover and number of songs for each.
-        each(this.state.artists, artist => {
+        each(this.all, artist => {
             this.setupArtist(artist);
         });
 
-        albumStore.init(this.state.artists);
+        albumStore.init(this.all);
     },
 
     setupArtist(artist) {
@@ -46,29 +46,67 @@ export default {
         return artist;
     },
 
+    /**
+     * Get all artists.
+     *
+     * @return {Array.<Object>}
+     */
     get all() {
         return this.state.artists;
     },
 
+    /**
+     * Set all artists.
+     *
+     * @param  {Array.<Object>} value
+     */
+    set all(value) {
+        this.state.artists = value;
+    },
+
+    /**
+     * Get an artist object by its ID.
+     *
+     * @param  {Number} id
+     */
     byId(id) {
         return find(this.all, { id });
     },
 
     /**
-     * Appends a new artist into the current collection.
+     * Adds an artist/artists into the current collection.
      *
-     * @param  {Object} artist
+     * @param  {Array.<Object>|Object} artists
      */
-    append(artist) {
-        this.state.artists.push(this.setupArtist(artist));
+    add(artists) {
+        artists = [].concat(artists);
+        each(artists, a => this.setupArtist(a));
+
+        this.all = union(this.all, artists);
     },
 
+    /**
+     * Remove artist(s) from the store.
+     *
+     * @param  {Array.<Object>|Object} artists
+     */
+    remove(artists) {
+        this.all = difference(this.all, [].concat(artists));
+    },
+
+    /**
+     * Add album(s) into an artist.
+     *
+     * @param {Object} artist
+     * @param {Array.<Object>|Object} albums
+     *
+     */
     addAlbumsIntoArtist(artist, albums) {
         albums = [].concat(albums);
 
         artist.albums = union(artist.albums ? artist.albums : [], albums);
 
-        albums.forEach(album => {
+        each(albums, album => {
             album.artist_id = artist.id;
             album.artist = artist;
         });
@@ -98,15 +136,6 @@ export default {
         return !artist.albums.length;
     },
 
-    /**
-     * Remove artist(s) from the store.
-     *
-     * @param  {Array.<Object>|Object} artists
-     */
-    remove(artists) {
-        this.state.artists = difference(this.state.artists, [].concat(artists));
-    },
-
     /**
      * Get all songs performed by an artist.
      *
@@ -130,24 +159,21 @@ export default {
      * @return {String}
      */
     getImage(artist) {
-        // If the artist already has a proper image, just return it.
-        if (artist.image) {
-            return artist.image;
+        if (!artist.image) {
+            // Try to get an image from one of the albums.
+            artist.image = config.unknownCover;
+
+            artist.albums.every(album => {
+                // If there's a "real" cover, use it.
+                if (album.image !== config.unknownCover) {
+                    artist.image = album.cover;
+
+                    // I want to break free.
+                    return false;
+                }
+            });
         }
 
-        // Otherwise, we try to get an image from one of their albums.
-        artist.image = config.unknownCover;
-
-        artist.albums.every(album => {
-            // If there's a "real" cover, use it.
-            if (album.image !== config.unknownCover) {
-                artist.image = album.cover;
-
-                // I want to break free.
-                return false;
-            }
-        });
-
         return artist.image;
     },
 
@@ -160,7 +186,7 @@ export default {
      */
     getMostPlayed(n = 6) {
         // Only non-unknown artists with actually play count are applicable.
-        const applicable = filter(this.state.artists, artist => {
+        const applicable = filter(this.all, artist => {
             return artist.playCount && artist.id !== 1;
         });
 
diff --git a/resources/assets/js/stores/favorite.js b/resources/assets/js/stores/favorite.js
index 6dec545a..2386a18e 100644
--- a/resources/assets/js/stores/favorite.js
+++ b/resources/assets/js/stores/favorite.js
@@ -23,8 +23,13 @@ export default {
         return this.state.songs;
     },
 
-    clear() {
-        this.state.songs = [];
+    /**
+     * Set all favorite'd songs.
+     *
+     * @param  {Array.<Object>} value
+     */
+    set all(value) {
+        this.state.songs = value;
     },
 
     /**
@@ -53,21 +58,28 @@ export default {
     },
 
     /**
-     * Add a song into the store.
+     * Add a song/songs into the store.
      *
-     * @param {Object} song
+     * @param {Array.<Object>|Object} songs
      */
-    add(song) {
-        this.state.songs.push(song);
+    add(songs) {
+        this.all = union(this.all, [].concat(songs));
     },
 
     /**
-     * Remove a song from the store.
+     * Remove a song/songs from the store.
      *
-     * @param {Object} song
+     * @param {Array.<Object>|Object} songs
      */
-    remove(song) {
-        this.state.songs = difference(this.state.songs, [song]);
+    remove(songs) {
+        this.all = difference(this.all, [].concat(songs));
+    },
+
+    /**
+     * Remove all favorites.
+     */
+    clear() {
+        this.all = [];
     },
 
     /**
@@ -80,7 +92,7 @@ export default {
         // Don't wait for the HTTP response to update the status, just set them to Liked right away.
         // This may cause a minor problem if the request fails somehow, but do we care?
         each(songs, song => song.liked = true);
-        this.state.songs = union(this.state.songs, songs);
+        this.add(songs);
 
         http.post('interaction/batch/like', { songs: map(songs, 'id') }, () => {
             if (cb) {
@@ -99,7 +111,7 @@ export default {
         // Don't wait for the HTTP response to update the status, just set them to Unliked right away.
         // This may cause a minor problem if the request fails somehow, but do we care?
         each(songs, song => song.liked = false);
-        this.state.songs = difference(this.state.songs, songs);
+        this.remove(songs);
 
         http.post('interaction/batch/unlike', { songs: map(songs, 'id') }, () => {
             if (cb) {
diff --git a/resources/assets/js/stores/playlist.js b/resources/assets/js/stores/playlist.js
index f351b1d0..610a644c 100644
--- a/resources/assets/js/stores/playlist.js
+++ b/resources/assets/js/stores/playlist.js
@@ -19,9 +19,8 @@ export default {
     },
 
     init(playlists) {
-        this.state.playlists = playlists;
-
-        each(this.state.playlists, this.getSongs);
+        this.all = playlists;
+        each(this.all, this.getSongs);
     },
 
     /**
@@ -33,6 +32,15 @@ export default {
         return this.state.playlists;
     },
 
+    /**
+     * Set all playlists.
+     *
+     * @param  {Array.<Object>} value
+     */
+    set all(value) {
+        this.state.playlists = value;
+    },
+
     /**
      * Get all songs in a playlist.
      *
@@ -42,6 +50,24 @@ export default {
         return (playlist.songs = songStore.byIds(playlist.songs));
     },
 
+    /**
+     * Add a playlist/playlists into the store.
+     *
+     * @param {Array.<Object>|Object} playlists
+     */
+    add(playlists) {
+        this.all = union(this.all, [].concat(playlists));
+    },
+
+    /**
+     * Remove a playlist/playlists from the store.
+     *
+     * @param  {Array.<Object>|Object} playlist
+     */
+    remove(playlists) {
+        this.all = difference(this.all, [].concat(playlists));
+    },
+
     /**
      * Create a new playlist, optionally with its songs.
      *
@@ -61,7 +87,7 @@ export default {
             const playlist = response.data;
             playlist.songs = songs;
             this.getSongs(playlist);
-            this.state.playlists.push(playlist);
+            this.add(playlist);
 
             if (cb) {
                 cb();
@@ -79,7 +105,7 @@ export default {
         NProgress.start();
 
         http.delete(`playlist/${playlist.id}`, {}, () => {
-            this.state.playlists = without(this.state.playlists, playlist);
+            this.remove(playlist);
 
             if (cb) {
                 cb();
diff --git a/resources/assets/js/stores/queue.js b/resources/assets/js/stores/queue.js
index cd9a6c4d..b7e66b88 100644
--- a/resources/assets/js/stores/queue.js
+++ b/resources/assets/js/stores/queue.js
@@ -1,6 +1,7 @@
 import {
     head,
     last,
+    each,
     includes,
     union,
     difference,
@@ -139,7 +140,7 @@ export default {
     move(songs, target) {
         const $targetIndex = this.indexOf(target);
 
-        songs.forEach(song => {
+        each(songs, song => {
             this.all.splice(this.indexOf(song), 1);
             this.all.splice($targetIndex, 0, song);
         });
diff --git a/resources/assets/js/stores/song.js b/resources/assets/js/stores/song.js
index df166cb0..84b2dc56 100644
--- a/resources/assets/js/stores/song.js
+++ b/resources/assets/js/stores/song.js
@@ -1,5 +1,5 @@
 import Vue from 'vue';
-import { without, map, take, remove, orderBy } from 'lodash';
+import { without, map, take, remove, orderBy, each } from 'lodash';
 
 import http from '../services/http';
 import utils from '../services/utils';
@@ -39,9 +39,9 @@ export default {
      */
     init(albums) {
         // Iterate through the albums. With each, add its songs into our master song list.
-        this.state.songs = albums.reduce((songs, album) => {
+        this.all = albums.reduce((songs, album) => {
             // While doing so, we populate some other information into the songs as well.
-            album.songs.forEach(song => {
+            each(album.songs, song => {
                 song.fmtLength = utils.secondsToHis(song.length);
 
                 // Manually set these additional properties to be reactive
@@ -67,7 +67,7 @@ export default {
     initInteractions(interactions) {
         favoriteStore.clear();
 
-        interactions.forEach(interaction => {
+        each(interactions, interaction => {
             const song = this.byId(interaction.song_id);
 
             if (!song) {
@@ -112,6 +112,15 @@ export default {
         return this.state.songs;
     },
 
+    /**
+     * Set all songs.
+     *
+     * @param  {Array.<Object>} value
+     */
+    set all(value) {
+        this.state.songs = value;
+    },
+
     /**
      * Get a song by its ID.
      *
@@ -203,7 +212,7 @@ export default {
 
             // Convert the duration into i:s
             if (data.album_info && data.album_info.tracks) {
-                data.album_info.tracks.forEach(track => track.fmtLength = utils.secondsToHis(track.length));
+                each(data.album_info.tracks, track => track.fmtLength = utils.secondsToHis(track.length));
             }
 
             // If the album cover is not in a nice form, don't use it.
@@ -261,9 +270,7 @@ export default {
             data,
             songs: map(songs, 'id'),
         }, response => {
-            response.data.forEach(song => {
-               this.syncUpdatedSong(song);
-            });
+            each(response.data, song => this.syncUpdatedSong(song));
 
             if (successCb) {
                 successCb();
@@ -327,7 +334,7 @@ export default {
                 // - Add the new album into our collection
                 // - Add the song into it
                 albumStore.addSongsIntoAlbum(updatedSong.album, originalSong);
-                albumStore.append(updatedSong.album);
+                albumStore.add(updatedSong.album);
             }
 
             if (updatedSong.album.artist.id === originalArtistId) { // case 2.a
@@ -345,7 +352,7 @@ export default {
                     // (there's no "new artist with existing album" in our system).
                     // - Add the new artist into our collection
                     artistStore.addAlbumsIntoArtist(updatedSong.album.artist, updatedSong.album);
-                    artistStore.append(updatedSong.album.artist);
+                    artistStore.add(updatedSong.album.artist);
                 }
             }
 
@@ -395,7 +402,7 @@ export default {
      * @return {Array.<Object>}
      */
     getMostPlayed(n = 10) {
-        const songs = take(orderBy(this.state.songs, 'playCount', 'desc'), n);
+        const songs = take(orderBy(this.all, 'playCount', 'desc'), n);
 
         // Remove those with playCount=0
         remove(songs, song => !song.playCount);
diff --git a/resources/assets/js/stores/user.js b/resources/assets/js/stores/user.js
index eddca0f1..b7a8b807 100644
--- a/resources/assets/js/stores/user.js
+++ b/resources/assets/js/stores/user.js
@@ -21,11 +21,11 @@ export default {
      * @param {Object}          currentUser The current user.
      */
     init(users, currentUser) {
-        this.state.users = users;
-        this.state.current = currentUser;
+        this.all = users;
+        this.current = currentUser;
 
         // Set the avatar for each of the users…
-        each(this.state.users, this.setAvatar);
+        each(this.all, this.setAvatar);
 
         // …and the current user as well.
         this.setAvatar();
@@ -40,6 +40,15 @@ export default {
         return this.state.users;
     },
 
+    /**
+     * Set all users.
+     *
+     * @param  {Array.<Object>} value
+     */
+    set all(value) {
+        this.state.users = value;
+    },
+
     /**
      * Get a user by his ID
      *
@@ -48,7 +57,7 @@ export default {
      * @return {Object}
      */
     byId(id) {
-        return find(this.state.users, { id });
+        return find(this.all, { id });
     },
 
     /**
@@ -155,7 +164,7 @@ export default {
             const user = response.data;
 
             this.setAvatar(user);
-            this.state.users.unshift(user);
+            this.all.unshift(user);
 
             if (cb) {
                 cb();
@@ -195,7 +204,7 @@ export default {
         NProgress.start();
 
         http.delete(`user/${user.id}`, {}, () => {
-            this.state.users = without(this.state.users, user);
+            this.all = without(this.all, user);
 
             // Mama, just killed a man
             // Put a gun against his head
diff --git a/resources/assets/js/tests/stores/albumTest.js b/resources/assets/js/tests/stores/albumTest.js
index de4d3717..fcff9b1a 100644
--- a/resources/assets/js/tests/stores/albumTest.js
+++ b/resources/assets/js/tests/stores/albumTest.js
@@ -37,12 +37,12 @@ describe('stores/album', () => {
         });
     });
 
-    describe('#append', () => {
+    describe('#add', () => {
         beforeEach(() => {
-            albumStore.append(cloneDeep(singleAlbum));
+            albumStore.add(cloneDeep(singleAlbum));
         });
 
-        it('correctly appends a new album into the state', () => {
+        it('correctly adds a new album into the state', () => {
             last(albumStore.state.albums).id.should.equal(9999);
         });
 
diff --git a/resources/assets/js/tests/stores/artistTest.js b/resources/assets/js/tests/stores/artistTest.js
index 5602fcdb..baad7bd8 100644
--- a/resources/assets/js/tests/stores/artistTest.js
+++ b/resources/assets/js/tests/stores/artistTest.js
@@ -34,10 +34,10 @@ describe('stores/artist', () => {
         });
     });
 
-    describe('#append', () => {
-        beforeEach(() => artistStore.append(cloneDeep(singleArtist)));
+    describe('#add', () => {
+        beforeEach(() => artistStore.add(cloneDeep(singleArtist)));
 
-        it('correctly appends an artist', () => {
+        it('correctly adds an artist', () => {
             last(artistStore.state.artists).name.should.equal('John Cena');
         });
     });