From a2b35ba8a38e374a6267abd0a3e635a47a8f8163 Mon Sep 17 00:00:00 2001
From: Peter Deltchev <peter@deltchev.com>
Date: Sat, 23 Jan 2016 04:20:18 -0800
Subject: [PATCH] #56: Fixed duplicate tracks in playlists.

---
 app/Commands/AddTrackToPlaylistCommand.php    | 21 +++-
 .../Api/Web/PlaylistsController.php           | 11 ++-
 ..._062640_EnforceUniqueTracksInPlaylists.php | 99 +++++++++++++++++++
 public/templates/tracks/show.html             |  6 +-
 .../scripts/app/controllers/track.coffee      |  9 +-
 5 files changed, 136 insertions(+), 10 deletions(-)
 create mode 100644 database/migrations/2016_01_23_062640_EnforceUniqueTracksInPlaylists.php

diff --git a/app/Commands/AddTrackToPlaylistCommand.php b/app/Commands/AddTrackToPlaylistCommand.php
index 499fcf74..b6d80e7c 100644
--- a/app/Commands/AddTrackToPlaylistCommand.php
+++ b/app/Commands/AddTrackToPlaylistCommand.php
@@ -22,12 +22,16 @@ namespace Poniverse\Ponyfm\Commands;
 
 use Poniverse\Ponyfm\Models\Playlist;
 use Poniverse\Ponyfm\Models\Track;
-use Illuminate\Support\Facades\Auth;
-use Illuminate\Support\Facades\DB;
+use Auth;
+use DB;
+use Validator;
 
 class AddTrackToPlaylistCommand extends CommandBase
 {
+    /** @var Track */
     private $_track;
+
+    /** @var Playlist */
     private $_playlist;
 
     function __construct($playlistId, $trackId)
@@ -52,11 +56,22 @@ class AddTrackToPlaylistCommand extends CommandBase
      */
     public function execute()
     {
+        // check if this track is already in the playlist
+        $validator = Validator::make(
+            ['track_id' => $this->_track->id],
+            ['track_id' => "unique:playlist_track,track_id,null,id,playlist_id,{$this->_playlist->id}",]
+        );
+
+        if ($validator->fails()) {
+            return CommandResponse::fail($validator);
+        }
+
+
         $songIndex = $this->_playlist->tracks()->count() + 1;
         $this->_playlist->tracks()->attach($this->_track, ['position' => $songIndex]);
         $this->_playlist->touch();
 
-        Playlist::whereId($this->_playlist->id)->update([
+        Playlist::where('id', $this->_playlist->id)->update([
             'track_count' => DB::raw('(SELECT COUNT(id) FROM playlist_track WHERE playlist_id = ' . $this->_playlist->id . ')')
         ]);
 
diff --git a/app/Http/Controllers/Api/Web/PlaylistsController.php b/app/Http/Controllers/Api/Web/PlaylistsController.php
index c0ee2c0b..113162c3 100644
--- a/app/Http/Controllers/Api/Web/PlaylistsController.php
+++ b/app/Http/Controllers/Api/Web/PlaylistsController.php
@@ -169,8 +169,12 @@ class PlaylistsController extends ApiControllerBase
 
     public function getOwned()
     {
-        $query = Playlist::summary()->with('pins', 'tracks', 'tracks.cover')->where('user_id',
-            \Auth::user()->id)->orderBy('title', 'asc')->get();
+        $query = Playlist::summary()
+            ->with('pins', 'tracks', 'tracks.cover')
+            ->where('user_id', Auth::user()->id)
+            ->orderBy('title', 'asc')
+            ->get();
+
         $playlists = [];
         foreach ($query as $playlist) {
             $playlists[] = [
@@ -185,7 +189,8 @@ class PlaylistsController extends ApiControllerBase
                     'normal' => $playlist->getCoverUrl(Image::NORMAL)
                 ],
                 'is_pinned' => $playlist->hasPinFor(Auth::user()->id),
-                'is_public' => $playlist->is_public == 1
+                'is_public' => $playlist->is_public == 1,
+                'track_ids' => $playlist->tracks->pluck('id')
             ];
         }
 
diff --git a/database/migrations/2016_01_23_062640_EnforceUniqueTracksInPlaylists.php b/database/migrations/2016_01_23_062640_EnforceUniqueTracksInPlaylists.php
new file mode 100644
index 00000000..f33ae14f
--- /dev/null
+++ b/database/migrations/2016_01_23_062640_EnforceUniqueTracksInPlaylists.php
@@ -0,0 +1,99 @@
+<?php
+
+/**
+ * Pony.fm - A community for pony fan music.
+ * Copyright (C) 2016 Peter Deltchev
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Database\Migrations\Migration;
+use Poniverse\Ponyfm\Models\Playlist;
+
+class EnforceUniqueTracksInPlaylists extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        DB::transaction(function(){
+            $playlistIds = DB::table('playlists')->lists('id');
+
+            foreach ($playlistIds as $playlistId) {
+                /** @var Playlist $playlist */
+
+                // Deletes all of a playlist's entries of a
+                // duplicate track except for the first one.
+                $ids = DB::select(
+                    DB::raw(
+<<<EOF
+SELECT id,position FROM playlist_track
+WHERE playlist_id = ?
+AND track_id IN
+(
+    SELECT track_id FROM
+    (
+        SELECT track_id,COUNT(*) as count FROM playlist_track
+        WHERE playlist_id = ?
+        GROUP BY track_id
+    ) as duplicateTracks
+    WHERE count > 1
+)
+ORDER BY position ASC
+LIMIT 1,18446744073709551615
+EOF
+                    )
+                , [$playlistId, $playlistId]);
+                $ids = collect($ids)->pluck('id');
+
+                DB::table('playlist_track')
+                    ->whereIn('id', $ids)
+                    ->delete();
+
+                // Using this instead of $model->fresh(); because that
+                // doesn't deal with soft-deleted models.
+                $playlist = Playlist::with('tracks')->withTrashed()->find($playlistId);
+
+                $position = 1;
+                foreach($playlist->tracks as $track) {
+                    $track->pivot->position = $position;
+                    $track->pivot->save();
+                    $position++;
+                }
+            }
+        });
+
+        Schema::table('playlist_track', function(Blueprint $table) {
+            $table->unique(['playlist_id', 'track_id']);
+        });
+
+        Artisan::call('refresh-cache');
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('playlist_track', function (Blueprint $table) {
+            $table->dropUnique('playlist_track_playlist_id_track_id_unique');
+        });
+    }
+}
diff --git a/public/templates/tracks/show.html b/public/templates/tracks/show.html
index b84c380f..fc931f06 100644
--- a/public/templates/tracks/show.html
+++ b/public/templates/tracks/show.html
@@ -25,7 +25,11 @@
             </a>
             <ul class="dropdown-menu">
                 <li bindonce ng-repeat="playlist in playlists">
-                    <a ng-class="{disabled: playlist.message, 'btn-success': playlist.message}" bo-href="playlist.url" pfm-eat-click ng-click="addToPlaylist(playlist); $event.stopPropagation()">
+                    <a ng-class="{disabled: playlist.message, 'btn-success': playlist.message}"
+                       bo-href="playlist.url"
+                       pfm-eat-click
+                       ng-click="addToPlaylist(playlist); $event.stopPropagation()"
+                    >
                         <span ng-hide="playlist.message" bo-text="playlist.title"></span>
                         <span ng-show="playlist.message">{{playlist.message}}</span>
                     </a>
diff --git a/resources/assets/scripts/app/controllers/track.coffee b/resources/assets/scripts/app/controllers/track.coffee
index 9393df1f..1b00ef11 100644
--- a/resources/assets/scripts/app/controllers/track.coffee
+++ b/resources/assets/scripts/app/controllers/track.coffee
@@ -24,8 +24,9 @@ angular.module('ponyfm').controller "track", [
     '$scope', '$rootScope', 'tracks', '$state', 'playlists', 'auth', 'favourites', '$dialog', 'download-cached', '$window', '$timeout'
     ($scope, $rootScope, tracks, $state, playlists, auth, favourites, $dialog, cachedTrack, $window, $timeout) ->
         track = null
+        trackId = parseInt($state.params.id)
 
-        tracks.fetch($state.params.id).done (trackResponse) ->
+        tracks.fetch(trackId).done (trackResponse) ->
             $scope.track = trackResponse.track
             track = trackResponse.track
             $rootScope.description = "Listen to #{track.title} by #{track.user.name} on the largest pony music site"
@@ -33,8 +34,10 @@ angular.module('ponyfm').controller "track", [
         $scope.playlists = []
 
         if auth.data.isLogged
-            playlists.refreshOwned().done (lists) ->
-                $scope.playlists.push list for list in lists
+            playlists.refreshOwned().done (playlists) ->
+                for playlist in playlists
+                    if trackId not in playlist.track_ids
+                        $scope.playlists.push playlist
 
         $scope.favouriteWorking = false