Refactor scrobble

app/Http/Controllers/API/ScrobbleController.php

@@ -2,12 +2,21 @@
 
 namespace App\Http\Controllers\API;
 
+use App\Models\Album;
 use App\Models\Song;
+use App\Services\LastfmService;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
 
 class ScrobbleController extends Controller
 {
+    private $lastfmService;
+
+    public function __construct(LastfmService $lastfmService)
+    {
+        $this->lastfmService = $lastfmService;
+    }
+
     /**
      * Create a Last.fm scrobble entry for a song.
      *
@@ -19,6 +28,20 @@ class ScrobbleController extends Controller
      */
     public function store(Request $request, Song $song, $timestamp)
     {
-        return response()->json($song->scrobble($request->user(), $timestamp));
+        if ($song->artist->is_unknown) {
+            return response()->json();
+        }
+
+        if (!$request->user()->connectedToLastfm()) {
+            return response()->json();
+        }
+
+        return response()->json($this->lastfmService->scrobble(
+            $song->artist->name,
+            $song->title,
+            $timestamp,
+            $song->album->name === Album::UNKNOWN_NAME ? '' : $song->album->name,
+            $request->user()->lastfm_session_key
+        ));
     }
 }

app/Models/Song.php

@@ -90,35 +90,6 @@ class Song extends Model
         return $this->belongsToMany(Playlist::class);
     }
 
-    /**
-     * Scrobble the song using Last.fm service.
-     *
-     * @param User   $user
-     * @param string $timestamp The UNIX timestamp in which the song started playing.
-     *
-     * @return mixed
-     */
-    public function scrobble(User $user, $timestamp)
-    {
-        // Don't scrobble the unknown guys. No one knows them.
-        if ($this->artist->is_unknown) {
-            return false;
-        }
-
-        // If the current user hasn't connected to Last.fm, don't do shit.
-        if (!$user->connectedToLastfm()) {
-            return false;
-        }
-
-        return Lastfm::scrobble(
-            $this->artist->name,
-            $this->title,
-            $timestamp,
-            $this->album->name === Album::UNKNOWN_NAME ? '' : $this->album->name,
-            $user->lastfm_session_key
-        );
-    }
-
     /**
      * Get a Song record using its path.
      *

tests/Feature/ScrobbleTest.php

@@ -4,6 +4,7 @@ namespace Tests\Feature;
 
 use App\Models\Song;
 use App\Services\LastfmService;
+use Exception;
 use Illuminate\Foundation\Testing\WithoutMiddleware;
 use Mockery as m;
 
@@ -11,14 +12,10 @@ class ScrobbleTest extends TestCase
 {
     use WithoutMiddleware;
 
-    protected function tearDown()
-    {
-        m::close();
-        parent::tearDown();
-    }
-
-    /** @test */
-    public function a_song_can_be_scrobbled_via_lastfm()
+    /**
+     * @throws Exception
+     */
+    public function testLastfmScrobble()
     {
         $this->withoutEvents();
         $this->createSampleMediaSet();

tests/Feature/SongTest.php

@@ -7,20 +7,25 @@ use App\Models\Album;
 use App\Models\Artist;
 use App\Models\Song;
 use App\Models\User;
+use Exception;
 use Illuminate\Foundation\Testing\WithoutMiddleware;
 
 class SongTest extends TestCase
 {
     use WithoutMiddleware;
 
+    /**
+     * @throws Exception
+     */
     public function setUp()
     {
         parent::setUp();
+
         $this->createSampleMediaSet();
     }
 
     /**
-     * @throws \Exception
+     * @throws Exception
      */
     public function testSingleUpdateAllInfoNoCompilation()
     {
@@ -302,7 +307,7 @@ class SongTest extends TestCase
     }
 
     /**
-     * @throws \Exception
+     * @throws Exception
      */
     public function testDeletingByChunk()
     {

tests/Integration/Models/SongTest.php

@@ -41,50 +41,6 @@ class SongTest extends TestCase
         $this->assertEquals($mockedURL, $url);
     }
 
-    /** @test */
-    public function it_scrobbles_if_the_user_is_connected_to_lastfm()
-    {
-        // Given there's a song
-        /** @var Song $song */
-        $song = factory(Song::class)->create();
-
-        // And a user who's connected to lastfm
-        /** @var User $user */
-        $user = factory(User::class)->create();
-        $user->setPreference('lastfm_session_key', 'foo');
-
-        // When I call the scrobble method
-        $time = time();
-        Lastfm::shouldReceive('scrobble')
-            ->once()
-            ->with($song->artist->name, $song->title, $time, $song->album->name, 'foo');
-
-        $song->scrobble($user, $time);
-
-        // Then I see the song is scrobbled
-    }
-
-    /** @test */
-    public function it_does_not_scrobble_if_the_user_is_not_connected_to_lastfm()
-    {
-        Lastfm::shouldReceive('scrobble')->times(0);
-
-        // Given there's a song
-        /** @var Song $song */
-        $song = factory(Song::class)->create();
-
-        // And a user who is not connected to lastfm
-        /** @var User $user */
-        $user = factory(User::class)->create();
-        $user->setPreference('lastfm_session_key', false);
-
-        // When I call the scrobble method
-
-        $song->scrobble($user, time());
-
-        // Then the song shouldn't be scrobbled
-    }
-
     /** @test */
     public function it_can_be_retrieved_using_its_path()
     {