Simplify regexp and improve tests

elamperti · Sep 9, 2024 · 05e67ca · 05e67ca
1 parent 0bd3a89
commit 05e67ca
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 23 deletions.
diff --git a/src/domains/scrobbleSong/SongForm.test.ts b/src/domains/scrobbleSong/SongForm.test.ts
@@ -1,14 +1,15 @@
 import { extractArtistTitle } from './SongForm';
 
 describe('`extractArtistTitle` helper', () => {
-  describe('`splitArtistTitleFromText` function', () => {
+  describe('from pasted text', () => {
     it('extracts artist and title from text', () => {
       const text = 'Artist - Title';
       const result = extractArtistTitle(text);
       expect(result).toEqual({ artist: 'Artist', title: 'Title' });
     });
 
-    it('extracts artist and title from text in reverse order', () => {
+    it('extracts artist and title reversed', () => {
+      // This is the case where the user pastes into the title field
       const text = 'Title - Artist';
       const result = extractArtistTitle(text, true);
       expect(result).toEqual({ artist: 'Artist', title: 'Title' });
@@ -39,35 +40,65 @@ describe('`extractArtistTitle` helper', () => {
     });
   });
 
-  describe('`parseLastFmUrl` function', () => {
-    it('extracts artist and title from Last.fm URL', () => {
+  describe('from Last.fm URLs', () => {
+    it('extracts artist and title', () => {
       const url = 'https://www.last.fm/music/Artist/_/Title';
       const result = extractArtistTitle(url);
       expect(result).toEqual({ artist: 'Artist', title: 'Title' });
     });
 
-    it('extracts artist and title from Last.fm URL with country code', () => {
+    it('supports a partial URL', () => {
+      const url = 'last.fm/music/Artist/_/Title';
+      const result = extractArtistTitle(url);
+      expect(result).toEqual({ artist: 'Artist', title: 'Title' });
+    });
+
+    it('skips country code', () => {
       const url = 'https://www.last.fm/pt/music/Artist/_/Title';
       const result = extractArtistTitle(url);
       expect(result).toEqual({ artist: 'Artist', title: 'Title' });
     });
 
-    it('extracts artist and title from Last.fm URL with encoded characters', () => {
+    it('skips country code in albums', () => {
+      const url = 'https://www.last.fm/fr/music/Artist/Album/';
+      const result = extractArtistTitle(url);
+      expect(result).toEqual(null);
+    });
+
+    it('extracts the album name if present', () => {
+      const url = 'https://www.last.fm/music/Artist/Album/Title';
+      const result = extractArtistTitle(url);
+      expect(result).toEqual({ artist: 'Artist', album: 'Album', title: 'Title' });
+    });
+
+    it('handles encoded characters', () => {
       const url = 'https://www.last.fm/music/Artist%20Name/_/Title%20Name';
       const result = extractArtistTitle(url);
       expect(result).toEqual({ artist: 'Artist Name', title: 'Title Name' });
     });
 
-    it('extracts artist and title from Last.fm URL with encoded characters and special characters', () => {
+    it('extracts artist and title with encoded characters and special characters', () => {
       const url = 'https://www.last.fm/music/Artist%20Name/_/Title%20Name%20(Feat.%20Artist%202)';
       const result = extractArtistTitle(url);
       expect(result).toEqual({ artist: 'Artist Name', title: 'Title Name (Feat. Artist 2)' });
     });
 
-    it('extracts artist and title from Last.fm URL with encoded characters, special characters and multiple dashes', () => {
-      const url = 'https://www.last.fm/music/Artist%20Name/_/Title%20Name%20(Feat.%20Artist%202)%20-%20Remix';
+    it.skip('supports a double-encoded plus sign', () => {
+      // We shouldn't get cases like this, but I happened to find this one. No use in fixing it for now.
+      const url = 'https://www.last.fm/es/music/Florence+%252B+the+Machine/_/You%27ve+Got+the+Love';
+      const result = extractArtistTitle(url);
+      expect(result).toEqual({ artist: 'Florence + the Machine', title: "You've Got the Love" });
+    });
+
+    it('handles question marks properly', () => {
+      const url =
+        'https://www.last.fm/es/music/Man%C3%A1/%C2%BFD%C3%B3nde+jugar%C3%A1n+los+ni%C3%B1os%3F/%C2%BFD%C3%B3nde+jugar%C3%A1n+los+ni%C3%B1os%3F';
       const result = extractArtistTitle(url);
-      expect(result).toEqual({ artist: 'Artist Name', title: 'Title Name (Feat. Artist 2) - Remix' });
+      expect(result).toEqual({
+        artist: 'Maná',
+        album: '¿Dónde jugarán los niños?',
+        title: '¿Dónde jugarán los niños?',
+      });
     });
 
     it('returns null if URL does not match pattern', () => {

diff --git a/src/domains/scrobbleSong/SongForm.tsx b/src/domains/scrobbleSong/SongForm.tsx
@@ -30,7 +30,13 @@ const Tooltip = lazyWithPreload(() => import('components/Tooltip'));
 const reAutoPasteSplitting = / - | ?[–—] ?/;
 const controlOrder = ['artist', 'title', 'album']; // Used for arrow navigation
 
-export function splitArtistTitleFromText(text: string, reverse: boolean) {
+type SongMatch = {
+  artist: string;
+  title: string;
+  album?: string;
+} | null;
+
+function splitArtistTitleFromText(text: string, reverse: boolean): SongMatch {
   if (reAutoPasteSplitting.test(text)) {
     const result = text.split(reAutoPasteSplitting, 2);
 
@@ -44,26 +50,23 @@ export function splitArtistTitleFromText(text: string, reverse: boolean) {
   return null;
 }
 
-const parseLastFmUrl = (url: string) => {
-  const regex = /^https?:\/\/(www\.)?last\.fm(\/[a-zA-Z]{2})?\/music\/([^/]+)\/_\/([^/]+)$/;
-  const match = url.match(regex);
+const reLastfmURL = /last\.fm(?:\/[a-zA-Z]{2})?\/music\/([^/]+)\/([^/]+?)\/([^/]+)/;
+function parseLastFmUrl(url: string): SongMatch {
+  const match = url.match(reLastfmURL);
 
   if (!match) {
     return null;
   }
 
-  const artist = decodeURIComponent(match[3].replace(/\+/g, ' '));
-  const title = decodeURIComponent(match[4].replace(/\+/g, ' '));
-
   return {
-    artist,
-    title,
+    artist: decodeURIComponent(match[1].replace(/\+/g, ' ')),
+    album: match[2] !== '_' ? decodeURIComponent(match[2].replace(/\+/g, ' ')) : undefined,
+    title: decodeURIComponent(match[3].replace(/\+/g, ' ')),
   };
-};
+}
 
-export function extractArtistTitle(text: string, reverse = false) {
-  const parsedText = parseLastFmUrl(text);
-  return parsedText ?? splitArtistTitleFromText(text, reverse);
+export function extractArtistTitle(text: string, reverse = false): SongMatch {
+  return parseLastFmUrl(text) ?? splitArtistTitleFromText(text, reverse);
 }
 
 export function SongForm() {
@@ -155,6 +158,9 @@ export function SongForm() {
 
       setArtist(splittedValues.artist);
       setTitle(splittedValues.title);
+      if (splittedValues.album) {
+        setAlbum(splittedValues.album);
+      }
 
       const undoPaste = () => {
         cloneDataFromScrobble(prevState);