feat(test): add and fix playlistStore tests

resources/assets/js/__tests__/factory/index.ts

@@ -3,6 +3,7 @@ import artistFactory, { states as artistStates } from '@/__tests__/factory/artis
 import songFactory, { states as songStates } from '@/__tests__/factory/songFactory'
 import albumFactory, { states as albumStates } from './albumFactory'
 import smartPlaylistRuleFactory from '@/__tests__/factory/smartPlaylistRuleFactory'
+import smartPlaylistRuleGroupFactory from '@/__tests__/factory/smartPlaylistRuleGroupFactory'
 import playlistFactory, { states as playlistStates } from '@/__tests__/factory/playlistFactory'
 import userFactory, { states as userStates } from '@/__tests__/factory/userFactory'
 import albumTrackFactory from '@/__tests__/factory/albumTrackFactory'
@@ -10,7 +11,7 @@ import albumInfoFactory from '@/__tests__/factory/albumInfoFactory'
 import artistInfoFactory from '@/__tests__/factory/artistInfoFactory'
 import youTubeVideoFactory from '@/__tests__/factory/youTubeVideoFactory'
 
-factory
+export default factory
   .define('artist', faker => artistFactory(faker), artistStates)
   .define('artist-info', faker => artistInfoFactory(faker))
   .define('album', faker => albumFactory(faker), albumStates)
@@ -19,7 +20,6 @@ factory
   .define('song', faker => songFactory(faker), songStates)
   .define('video', faker => youTubeVideoFactory(faker))
   .define('smart-playlist-rule', faker => smartPlaylistRuleFactory(faker))
+  .define('smart-playlist-rule-group', faker => smartPlaylistRuleGroupFactory(faker))
   .define('playlist', faker => playlistFactory(faker), playlistStates)
   .define('user', faker => userFactory(faker), userStates)
-
-export default factory

resources/assets/js/__tests__/factory/smartPlaylistRuleGroupFactory.ts

@@ -0,0 +1,7 @@
+import { Faker } from '@faker-js/faker'
+import factory from 'factoria'
+
+export default (faker: Faker): SmartPlaylistRuleGroup => ({
+  id: faker.datatype.number(),
+  rules: factory<SmartPlaylistRule[]>('smart-playlist-rule', 3)
+})

resources/assets/js/components/playlist/PlaylistNameEditor.vue

@@ -1,6 +1,6 @@
 <template>
   <input
-    v-model="mutatedPlaylist.name"
+    v-model="name"
     v-koel-focus
     data-testid="inline-playlist-name-input"
     name="name"
@@ -15,44 +15,42 @@
 <script lang="ts" setup>
 import { reactive, ref, toRefs } from 'vue'
 import { playlistStore } from '@/stores'
-import { alerts } from '@/utils'
+import { alerts, logger } from '@/utils'
 
 const props = defineProps<{ playlist: Playlist }>()
 const { playlist } = toRefs(props)
 
-const updating = ref(false)
+let updating = false
 
-const mutatedPlaylist = reactive<Playlist>(Object.assign({}, playlist.value))
+const mutablePlaylist = reactive<Playlist>(Object.assign({}, playlist.value))
+const name = ref(mutablePlaylist.name)
 
 const emit = defineEmits(['updated', 'cancelled'])
 
 const update = async () => {
-  mutatedPlaylist.name = mutatedPlaylist.name.trim()
-
-  if (!mutatedPlaylist.name) {
-    cancel()
-    return
-  }
-
-  if (mutatedPlaylist.name === playlist.value.name) {
+  if (!name.value || name.value === playlist.value.name) {
     cancel()
     return
   }
 
   // prevent duplicate updating from Enter and Blur
-  if (updating.value) {
+  if (updating) {
     return
   }
 
-  updating.value = true
+  updating = true
 
-  await playlistStore.update(mutatedPlaylist)
-  alerts.success(`Playlist "${mutatedPlaylist.name}" updated.`)
-  emit('updated', mutatedPlaylist)
+  try {
+    await playlistStore.update(mutablePlaylist, { name: name.value })
+    alerts.success(`Playlist "${name}" updated.`)
+    emit('updated', name)
+  } catch (error) {
+    alerts.error('Something went wrong. Please try again.')
+    logger.error(error)
+  } finally {
+    updating = false
+  }
 }
 
-const cancel = () => {
-  mutatedPlaylist.name = playlist.value.name
-  emit('cancelled')
-}
+const cancel = () => emit('cancelled')
 </script>

resources/assets/js/components/playlist/PlaylistSidebarItem.vue

@@ -117,8 +117,8 @@ const openContextMenu = async (event: MouseEvent) => {
 
 const cancelEditing = () => (editing.value = false)
 
-const onPlaylistNameUpdated = (mutatedPlaylist: Playlist) => {
-  playlist.value.name = mutatedPlaylist.name
+const onPlaylistNameUpdated = (name: string) => {
+  playlist.value.name = name
   editing.value = false
 }
 

resources/assets/js/components/playlist/smart-playlist/SmartPlaylistEditForm.vue

@@ -11,13 +11,13 @@
           <div class="form-row">
             <label>
               Name
-              <input v-model="mutatedPlaylist.name" v-koel-focus name="name" required type="text">
+              <input v-model="mutablePlaylist.name" v-koel-focus name="name" required type="text">
             </label>
           </div>
 
           <div class="form-row rules">
             <RuleGroup
-              v-for="(group, index) in mutatedPlaylist.rules"
+              v-for="(group, index) in mutablePlaylist.rules"
               :key="group.id"
               :group="group"
               :isFirstGroup="index === 0"
@@ -43,18 +43,18 @@ import { faPlus } from '@fortawesome/free-solid-svg-icons'
 import { computed, reactive, watch } from 'vue'
 import { cloneDeep, isEqual } from 'lodash'
 import { playlistStore } from '@/stores'
-import { alerts, eventBus, requireInjection } from '@/utils'
+import { alerts, eventBus, logger, requireInjection } from '@/utils'
 import { useSmartPlaylistForm } from '@/components/playlist/smart-playlist/useSmartPlaylistForm'
 import { PlaylistKey } from '@/symbols'
 
 const [playlist] = requireInjection(PlaylistKey)
 
-let mutatedPlaylist: Playlist
+let mutablePlaylist: Playlist
 
-watch(playlist, () => (mutatedPlaylist = reactive(cloneDeep(playlist.value))), { immediate: true })
+watch(playlist, () => (mutablePlaylist = reactive(cloneDeep(playlist.value))), { immediate: true })
 
 const isPristine = computed(() => {
-  return isEqual(mutatedPlaylist.rules, playlist.value.rules) && mutatedPlaylist.name.trim() === playlist.value.name
+  return isEqual(mutablePlaylist.rules, playlist.value.rules) && mutablePlaylist.name.trim() === playlist.value.name
 })
 
 const {
@@ -66,7 +66,7 @@ const {
   loading,
   addGroup,
   onGroupChanged
-} = useSmartPlaylistForm(mutatedPlaylist.rules)
+} = useSmartPlaylistForm(mutablePlaylist.rules)
 
 const emit = defineEmits(['close'])
 const close = () => emit('close')
@@ -82,12 +82,22 @@ const maybeClose = () => {
 
 const submit = async () => {
   loading.value = true
-  mutatedPlaylist.rules = collectedRuleGroups.value
-  await playlistStore.update(mutatedPlaylist)
-  Object.assign(playlist.value, mutatedPlaylist)
-  loading.value = false
-  alerts.success(`Playlist "${playlist.value.name}" updated.`)
-  eventBus.emit('SMART_PLAYLIST_UPDATED', playlist.value)
-  close()
+  mutablePlaylist.rules = collectedRuleGroups.value
+
+  try {
+    await playlistStore.update(playlist.value, {
+      name: mutablePlaylist.name,
+      rules: mutablePlaylist.rules
+    })
+
+    alerts.success(`Playlist "${playlist.value.name}" updated.`)
+    eventBus.emit('SMART_PLAYLIST_UPDATED', playlist.value)
+    close()
+  } catch (error) {
+    alerts.error('Something went wrong. Please try again.')
+    logger.error(error)
+  } finally {
+    loading.value = false
+  }
 }
 </script>

resources/assets/js/stores/playlistStore.spec.ts

@@ -2,6 +2,7 @@ import UnitTestCase from '@/__tests__/UnitTestCase'
 import factory from '@/__tests__/factory'
 import { playlistStore } from '@/stores'
 import { expect, it } from 'vitest'
+import { Cache, httpService } from '@/services'
 
 const ruleGroups: SmartPlaylistRuleGroup[] = [
   {
@@ -70,12 +71,102 @@ new class extends UnitTestCase {
     it('sets up a smart playlist with properly unserialized rules', () => {
       const playlist = factory<Playlist>('playlist', {
         is_smart: true,
-        rules: serializedRuleGroups as unknown as SmartPlaylistRuleGroup[]
+        rules: serializedRuleGroups as SmartPlaylistRuleGroup[]
       })
 
       playlistStore.setupSmartPlaylist(playlist)
 
       expect(playlist.rules).toEqual(ruleGroups)
     })
+
+    it('stores a playlist', async () => {
+      const songs = factory<Song[]>('song', 3)
+      const playlist = factory<Playlist>('playlist')
+      const postMock = this.mock(httpService, 'post').mockResolvedValue(playlist)
+      const serializeMock = this.mock(playlistStore, 'serializeSmartPlaylistRulesForStorage', null)
+
+      await playlistStore.store('New Playlist', songs, [])
+
+      expect(postMock).toHaveBeenCalledWith('playlist', {
+        name: 'New Playlist',
+        songs: songs.map(song => song.id),
+        rules: null
+      })
+
+      expect(serializeMock).toHaveBeenCalledWith([])
+      expect(playlistStore.state.playlists).toHaveLength(1)
+      expect(playlistStore.state.playlists[0]).toEqual(playlist)
+    })
+
+    it('deletes a playlist', async () => {
+      const playlist = factory<Playlist>('playlist', { id: 12 })
+      const deleteMock = this.mock(httpService, 'delete')
+      playlistStore.state.playlists = [factory<Playlist>('playlist'), playlist]
+
+      await playlistStore.delete(playlist)
+
+      expect(deleteMock).toHaveBeenCalledWith('playlists/12')
+      expect(playlistStore.state.playlists).toHaveLength(1)
+      expect(playlistStore.byId(playlist.id)).toBeUndefined()
+    })
+
+    it('adds songs to a playlist', async () => {
+      const playlist = factory<Playlist>('playlist', { id: 12 })
+      const songs = factory<Song[]>('song', 3)
+      const postMock = this.mock(httpService, 'post').mockResolvedValue(playlist)
+      const invalidateMock = this.mock(Cache, 'invalidate')
+
+      await playlistStore.addSongs(playlist, songs)
+
+      expect(postMock).toHaveBeenCalledWith('playlists/12/songs', { songs: songs.map(song => song.id) })
+      expect(invalidateMock).toHaveBeenCalledWith(['playlist.songs', 12])
+    })
+
+    it('removes songs from a playlist', async () => {
+      const playlist = factory<Playlist>('playlist', { id: 12 })
+      const songs = factory<Song[]>('song', 3)
+      const deleteMock = this.mock(httpService, 'delete').mockResolvedValue(playlist)
+      const invalidateMock = this.mock(Cache, 'invalidate')
+
+      await playlistStore.removeSongs(playlist, songs)
+
+      expect(deleteMock).toHaveBeenCalledWith('playlists/12/songs', { songs: songs.map(song => song.id) })
+      expect(invalidateMock).toHaveBeenCalledWith(['playlist.songs', 12])
+    })
+
+    it('does not modify a smart playlist content', async () => {
+      const playlist = factory.states('smart')<Playlist>('playlist')
+      const postMock = this.mock(httpService, 'post')
+
+      await playlistStore.addSongs(playlist, factory<Song[]>('song', 3))
+      expect(postMock).not.toHaveBeenCalled()
+
+      await playlistStore.removeSongs(playlist, factory<Song[]>('song', 3))
+      expect(postMock).not.toHaveBeenCalled()
+    })
+
+    it('updates a standard playlist', async () => {
+      const playlist = factory<Playlist>('playlist', { id: 12 })
+      const putMock = this.mock(httpService, 'put').mockResolvedValue(playlist)
+
+      await playlistStore.update(playlist, { name: 'Foo' })
+
+      expect(putMock).toHaveBeenCalledWith('playlists/12', { name: 'Foo', rules: null })
+      expect(playlist.name).toBe('Foo')
+    })
+
+    it('updates a smart playlist', async () => {
+      const playlist = factory.states('smart')<Playlist>('playlist', { id: 12 })
+      const rules = factory<SmartPlaylistRuleGroup[]>('smart-playlist-rule-group', 2)
+      const serializeMock = this.mock(playlistStore, 'serializeSmartPlaylistRulesForStorage', ['Whatever'])
+      const putMock = this.mock(httpService, 'put').mockResolvedValue(playlist)
+      const invalidateMock = this.mock(Cache, 'invalidate')
+
+      await playlistStore.update(playlist, { name: 'Foo', rules })
+
+      expect(serializeMock).toHaveBeenCalledWith(rules)
+      expect(putMock).toHaveBeenCalledWith('playlists/12', { name: 'Foo', rules: ['Whatever'] })
+      expect(invalidateMock).toHaveBeenCalledWith(['playlist.songs', 12])
+    })
   }
 }

resources/assets/js/stores/playlistStore.ts

@@ -1,6 +1,5 @@
 import { differenceBy, orderBy } from 'lodash'
 import { reactive } from 'vue'
-
 import { logger } from '@/utils'
 import { Cache, httpService } from '@/services'
 import models from '@/config/smart-playlist/models'
@@ -13,11 +12,7 @@ export const playlistStore = {
 
   init (playlists: Playlist[]) {
     this.state.playlists = this.sort(playlists)
-    this.state.playlists.forEach(playlist => this.setupPlaylist(playlist))
-  },
-
-  setupPlaylist (playlist: Playlist) {
-    playlist.is_smart && this.setupSmartPlaylist(playlist)
+    this.state.playlists.forEach(playlist => playlist.is_smart && this.setupSmartPlaylist(playlist))
   },
 
   /**
@@ -43,10 +38,12 @@ export const playlistStore = {
   },
 
   async store (name: string, songs: Song[] = [], rules: SmartPlaylistRuleGroup[] = []) {
-    const songIds = songs.map(song => song.id)
-    const serializedRules = this.serializeSmartPlaylistRulesForStorage(rules)
+    const playlist = await httpService.post<Playlist>('playlist', {
+      name,
+      songs: songs.map(song => song.id),
+      rules: this.serializeSmartPlaylistRulesForStorage(rules)
+    })
 
-    const playlist = await httpService.post<Playlist>('playlist', { name, songs: songIds, rules: serializedRules })
     this.state.playlists.push(playlist)
     this.state.playlists = this.sort(this.state.playlists)
 
@@ -80,11 +77,14 @@ export const playlistStore = {
     return playlist
   },
 
-  async update (playlist: Playlist) {
-    const serializedRules = this.serializeSmartPlaylistRulesForStorage(playlist.rules)
-    await httpService.put(`playlists/${playlist.id}`, { name: playlist.name, rules: serializedRules })
+  async update (playlist: Playlist, data: Partial<Pick<Playlist, 'name' | 'rules'>>) {
+    await httpService.put(`playlists/${playlist.id}`, {
+      name: data.name,
+      rules: this.serializeSmartPlaylistRulesForStorage(data.rules || [])
+    })
 
     playlist.is_smart && Cache.invalidate(['playlist.songs', playlist.id])
+    Object.assign(playlist, data)
 
     return playlist
   },