refactor(Sortable Songs In Playlists): revisited efficiency of solution

Signed-off-by: Jonathan <theflametrooper@gmail.com>

backend/logic/actions/playlists.js

@@ -46,15 +46,14 @@ CacheModule.runJob("SUB", {
 });
 
 CacheModule.runJob("SUB", {
-	channel: "playlist.repositionSongs",
+	channel: "playlist.repositionSong",
 	cb: res => {
-		WSModule.runJob("SOCKETS_FROM_USER", { userId: res.userId }, this).then(sockets =>
+		const { userId, playlistId, song } = res;
+
+		WSModule.runJob("SOCKETS_FROM_USER", { userId }, this).then(sockets =>
 			sockets.forEach(socket =>
-				socket.dispatch("event:playlist.songs.repositioned", {
-					data: {
-						playlistId: res.playlistId,
-						songsBeingChanged: res.songsBeingChanged
-					}
+				socket.dispatch("event:playlist.song.repositioned", {
+					data: { playlistId, song }
 				})
 			)
 		);
@@ -640,49 +639,6 @@ export default {
 		);
 	},
 
-	/**
-	 * Obtains basic metadata of a playlist in order to format an activity
-	 *
-	 * @param {object} session - the session object automatically added by the websocket
-	 * @param {string} playlistId - the playlist id
-	 * @param {Function} cb - callback
-	 */
-	getPlaylistForActivity(session, playlistId, cb) {
-		async.waterfall(
-			[
-				next => {
-					PlaylistsModule.runJob("GET_PLAYLIST", { playlistId }, this)
-						.then(playlist => {
-							next(null, playlist);
-						})
-						.catch(next);
-				}
-			],
-			async (err, playlist) => {
-				if (err) {
-					err = await UtilsModule.runJob("GET_ERROR", { error: err }, this);
-					this.log(
-						"ERROR",
-						"PLAYLISTS_GET_PLAYLIST_FOR_ACTIVITY",
-						`Failed to obtain metadata of playlist ${playlistId} for activity formatting. "${err}"`
-					);
-					return cb({ status: "error", message: err });
-				}
-				this.log(
-					"SUCCESS",
-					"PLAYLISTS_GET_PLAYLIST_FOR_ACTIVITY",
-					`Obtained metadata of playlist ${playlistId} for activity formatting successfully.`
-				);
-				return cb({
-					status: "success",
-					data: {
-						title: playlist.displayName
-					}
-				});
-			}
-		);
-	},
-
 	/**
 	 * Gets a playlist from station id
 	 *
@@ -742,67 +698,6 @@ export default {
 		);
 	},
 
-	// TODO Remove this
-	/**
-	 * Updates a private playlist
-	 *
-	 * @param {object} session - the session object automatically added by the websocket
-	 * @param {string} playlistId - the id of the playlist we are updating
-	 * @param {object} playlist - the new private playlist object
-	 * @param {Function} cb - gets called with the result
-	 */
-	update: isLoginRequired(async function update(session, playlistId, playlist, cb) {
-		const playlistModel = await DBModule.runJob(
-			"GET_MODEL",
-			{
-				modelName: "playlist"
-			},
-			this
-		);
-		async.waterfall(
-			[
-				next => {
-					playlistModel.updateOne(
-						{ _id: playlistId, createdBy: session.userId },
-						playlist,
-						{ runValidators: true },
-						next
-					);
-				},
-
-				(res, next) => {
-					PlaylistsModule.runJob("UPDATE_PLAYLIST", { playlistId }, this)
-						.then(playlist => {
-							next(null, playlist);
-						})
-						.catch(next);
-				}
-			],
-			async (err, playlist) => {
-				if (err) {
-					err = await UtilsModule.runJob("GET_ERROR", { error: err }, this);
-					this.log(
-						"ERROR",
-						"PLAYLIST_UPDATE",
-						`Updating private playlist "${playlistId}" failed for user "${session.userId}". "${err}"`
-					);
-					return cb({ status: "error", message: err });
-				}
-
-				this.log(
-					"SUCCESS",
-					"PLAYLIST_UPDATE",
-					`Successfully updated private playlist "${playlistId}" for user "${session.userId}".`
-				);
-
-				return cb({
-					status: "success",
-					data: { playlist }
-				});
-			}
-		);
-	}),
-
 	/**
 	 * Shuffles songs in a private playlist
 	 *
@@ -865,37 +760,44 @@ export default {
 	}),
 
 	/**
-	 * Changes the order of song(s) in a private playlist
+	 * Changes the order (position) of a song in a playlist
 	 *
 	 * @param {object} session - the session object automatically added by the websocket
 	 * @param {string} playlistId - the id of the playlist we are targeting
-	 * @param {Array} songsBeingChanged - the songs to be repositioned, each element contains "youtubeId" and "position" properties
+	 * @param {object} song - the song to be repositioned
+	 * @param {string} song.youtubeId - the youtube id of the song being repositioned
+	 * @param {string} song.newIndex - the new position of the song in the playlist
+	 * @param {...any} song.args - any other elements that would be included with a song item in a playlist
 	 * @param {Function} cb - gets called with the result
 	 */
-	repositionSongs: isLoginRequired(async function repositionSongs(session, playlistId, songsBeingChanged, cb) {
+	repositionSong: isLoginRequired(async function repositionSong(session, playlistId, song, cb) {
 		const playlistModel = await DBModule.runJob("GET_MODEL", { modelName: "playlist" }, this);
 
 		async.waterfall(
 			[
-				// update playlist object with each song's new position
-				next =>
-					async.each(
-						songsBeingChanged,
-						(song, nextSong) =>
-							playlistModel.updateOne(
-								{ _id: playlistId, "songs.youtubeId": song.youtubeId },
-								{
-									$set: {
-										"songs.$.position": song.position
-									}
-								},
-								err => {
-									if (err) return next(err);
-									return nextSong();
-								}
-							),
+				next => {
+					if (!playlistId) return next("Please provide a playlist.");
+					if (!song || !song.youtubeId) return next("You must provide a song to reposition.");
+					return next();
+				},
+
+				// remove song from playlist
+				next => {
+					playlistModel.updateOne(
+						{ _id: playlistId },
+						{ $pull: { songs: { youtubeId: song.youtubeId } } },
 						next
-					),
+					);
+				},
+
+				// add song back to playlist (in new position)
+				(res, next) => {
+					playlistModel.updateOne(
+						{ _id: playlistId },
+						{ $push: { songs: { $each: [song], $position: song.newIndex } } },
+						err => next(err)
+					);
+				},
 
 				// update the cache with the new songs positioning
 				next => {
@@ -910,8 +812,8 @@ export default {
 
 					this.log(
 						"ERROR",
-						"PLAYLIST_REPOSITION_SONGS",
-						`Repositioning songs for private playlist "${playlistId}" failed for user "${session.userId}". "${err}"`
+						"PLAYLIST_REPOSITION_SONG",
+						`Repositioning song ${song.youtubeId}  for private playlist "${playlistId}" failed for user "${session.userId}". "${err}"`
 					);
 
 					return cb({ status: "error", message: err });
@@ -919,107 +821,22 @@ export default {
 
 				this.log(
 					"SUCCESS",
-					"PLAYLIST_REPOSITION_SONGS",
-					`Successfully repositioned songs for private playlist "${playlistId}" for user "${session.userId}".`
+					"PLAYLIST_REPOSITION_SONG",
+					`Successfully repositioned song ${song.youtubeId} for private playlist "${playlistId}" for user "${session.userId}".`
 				);
 
 				CacheModule.runJob("PUB", {
-					channel: "playlist.repositionSongs",
+					channel: "playlist.repositionSong",
 					value: {
 						userId: session.userId,
 						playlistId,
-						songsBeingChanged
+						song
 					}
 				});
 
 				return cb({
 					status: "success",
-					message: "Order of songs successfully updated"
-				});
-			}
-		);
-	}),
-
-	/**
-	 * Moves a song to the bottom of the list in a private playlist
-	 *
-	 * @param {object} session - the session object automatically added by the websocket
-	 * @param {string} playlistId - the id of the playlist we are moving the song to the bottom from
-	 * @param {string} youtubeId - the youtube id of the song we are moving to the bottom of the list
-	 * @param {Function} cb - gets called with the result
-	 */
-	moveSongToBottom: isLoginRequired(async function moveSongToBottom(session, playlistId, youtubeId, cb) {
-		async.waterfall(
-			[
-				next => {
-					PlaylistsModule.runJob("GET_PLAYLIST", { playlistId }, this)
-						.then(playlist => next(null, playlist))
-						.catch(next);
-				},
-
-				(playlist, next) => {
-					if (!playlist || playlist.createdBy !== session.userId) return next("Playlist not found");
-					if (!playlist.isUserModifiable) return next("Playlist cannot be modified.");
-
-					// sort array by position
-					playlist.songs.sort((a, b) => a.position - b.position);
-
-					// find index of youtubeId
-					playlist.songs.forEach((song, index) => {
-						// reorder array (simulates what would be done with a drag and drop interface)
-						if (song.youtubeId === youtubeId)
-							playlist.songs.splice(playlist.songs.length, 0, playlist.songs.splice(index, 1)[0]);
-					});
-
-					const songsBeingChanged = [];
-
-					playlist.songs.forEach((song, index) => {
-						// check if position needs updated based on index
-						if (song.position !== index + 1)
-							songsBeingChanged.push({
-								youtubeId: song.youtubeId,
-								position: index + 1
-							});
-					});
-
-					// update position property on songs that need to be changed
-					return WSModule.runJob(
-						"RUN_ACTION2",
-						{
-							session,
-							namespace: "playlists",
-							action: "repositionSongs",
-							args: [playlistId, songsBeingChanged]
-						},
-						this
-					)
-						.then(res => {
-							if (res.status === "success") return next();
-							return next("Unable to reposition song in playlist.");
-						})
-						.catch(next);
-				}
-			],
-			async err => {
-				if (err) {
-					err = await UtilsModule.runJob("GET_ERROR", { error: err }, this);
-					this.log(
-						"ERROR",
-						"PLAYLIST_MOVE_SONG_TO_BOTTOM",
-						`Moving song "${youtubeId}" to the bottom for private playlist "${playlistId}" failed for user "${session.userId}". "${err}"`
-					);
-					return cb({ status: "error", message: err });
-				}
-
-				this.log(
-					"SUCCESS",
-					"PLAYLIST_MOVE_SONG_TO_BOTTOM",
-					`Successfully moved song "${youtubeId}" to the bottom for private playlist "${playlistId}" for user "${session.userId}".`
-				);
-
-				return cb({
-					status: "success",
-					message: "Order of songs successfully updated"
+					message: "Successfully repositioned song"
 				});
 			}
 		);
@@ -1311,59 +1128,14 @@ export default {
 			[
 				next => {
 					if (!youtubeId || typeof youtubeId !== "string") return next("Invalid song id.");
-					if (!playlistId) return next("Invalid playlist id.");
+					if (!playlistId || typeof youtubeId !== "string") return next("Invalid playlist id.");
 					return next();
 				},
 
-				next => {
-					PlaylistsModule.runJob("GET_PLAYLIST", { playlistId }, this)
-						.then(playlist => next(null, playlist))
-						.catch(next);
-				},
-
-				(playlist, next) => {
-					if (!playlist || playlist.createdBy !== session.userId) return next("Playlist not found");
-
-					// sort array by position
-					playlist.songs.sort((a, b) => a.position - b.position);
-
-					// find index of youtubeId
-					playlist.songs.forEach((song, ind) => {
-						// remove song from array
-						if (song.youtubeId === youtubeId) playlist.songs.splice(ind, 1);
-					});
-
-					const songsBeingChanged = [];
-
-					playlist.songs.forEach((song, index) => {
-						// check if position needs updated based on index
-						if (song.position !== index + 1)
-							songsBeingChanged.push({
-								youtubeId: song.youtubeId,
-								position: index + 1
-							});
-					});
-
-					// update position property on songs that need to be changed
-					return WSModule.runJob(
-						"RUN_ACTION2",
-						{
-							session,
-							namespace: "playlists",
-							action: "repositionSongs",
-							args: [playlistId, songsBeingChanged]
-						},
-						this
-					)
-						.then(res => {
-							if (res.status === "success") return next();
-							return next("Unable to reposition song in playlist.");
-						})
-						.catch(next);
-				},
-
+				// remove song from playlist
 				next => playlistModel.updateOne({ _id: playlistId }, { $pull: { songs: { youtubeId } } }, next),
 
+				// update cache representation of the playlist
 				(res, next) => {
 					PlaylistsModule.runJob("UPDATE_PLAYLIST", { playlistId }, this)
 						.then(playlist => next(null, playlist))

backend/logic/db/index.js

@@ -8,7 +8,7 @@ import CoreClass from "../../core";
 const REQUIRED_DOCUMENT_VERSIONS = {
 	activity: 2,
 	news: 2,
-	playlist: 3,
+	playlist: 4,
 	punishment: 1,
 	queueSong: 1,
 	report: 2,

backend/logic/db/schemas/playlist.js

@@ -11,8 +11,7 @@ export default {
 			duration: { type: Number },
 			thumbnail: { type: String, required: false },
 			artists: { type: Array, required: false },
-			status: { type: String },
-			position: { type: Number }
+			status: { type: String }
 		}
 	],
 	createdBy: { type: String, required: true },
@@ -20,5 +19,5 @@ export default {
 	createdFor: { type: String },
 	privacy: { type: String, enum: ["public", "private"], default: "private" },
 	type: { type: String, enum: ["user", "genre", "station"], required: true },
-	documentVersion: { type: Number, default: 3, required: true }
+	documentVersion: { type: Number, default: 4, required: true }
 };

frontend/src/components/Queue.vue

@@ -222,9 +222,9 @@ export default {
 				this.station._id,
 				youtubeId,
 				res => {
-					if (res.status === "success") {
+					if (res.status === "success")
 						new Toast("Successfully removed song from the queue.");
-					} else new Toast(res.message);
+					else new Toast(res.message);
 				}
 			);
 		},

frontend/src/components/modals/EditPlaylist.vue

@@ -240,7 +240,7 @@
 							v-bind="dragOptions"
 							@start="drag = true"
 							@end="drag = false"
-							@change="updateSongPositioning"
+							@change="repositionSong"
 						>
 							<transition-group
 								type="transition"
@@ -457,9 +457,7 @@ export default {
 				if (this.playlist._id === res.data.playlistId)
 					this.playlist.songs.push(res.data.song);
 			},
-			{
-				modal: "editPlaylist"
-			}
+			{ modal: "editPlaylist" }
 		);
 
 		this.socket.on(
@@ -482,9 +480,7 @@ export default {
 					});
 				}
 			},
-			{
-				modal: "editPlaylist"
-			}
+			{ modal: "editPlaylist" }
 		);
 
 		this.socket.on(
@@ -493,39 +489,32 @@ export default {
 				if (this.playlist._id === res.data.playlistId)
 					this.playlist.displayName = res.data.displayName;
 			},
-			{
-				modal: "editPlaylist"
-			}
+			{ modal: "editPlaylist" }
 		);
 
 		this.socket.on(
-			"event:playlist.songs.repositioned",
+			"event:playlist.song.repositioned",
 			res => {
 				if (this.playlist._id === res.data.playlistId) {
-					// for each song that has a new position
-					res.data.songsBeingChanged.forEach(changedSong => {
-						this.playlist.songs.forEach((song, index) => {
-							// find song locally
-							if (song.youtubeId === changedSong.youtubeId) {
-								// change song position attribute
-								this.playlist.songs[index].position =
-									changedSong.position;
-
-								// reposition in array if needed
-								if (index !== changedSong.position - 1)
-									this.playlist.songs.splice(
-										changedSong.position - 1,
-										0,
-										this.playlist.songs.splice(index, 1)[0]
-									);
-							}
-						});
-					});
+					const { song, playlistId } = res.data;
+
+					if (this.playlist._id === playlistId) {
+						if (
+							this.playlist.songs[song.newIndex] &&
+							this.playlist.songs[song.newIndex].youtubeId ===
+								song.youtubeId
+						)
+							return;
+
+						this.playlist.songs.splice(
+							song.newIndex,
+							0,
+							this.playlist.songs.splice(song.oldIndex, 1)[0]
+						);
+					}
 				}
 			},
-			{
-				modal: "editPlaylist"
-			}
+			{ modal: "editPlaylist" }
 		);
 	},
 	methods: {
@@ -584,28 +573,45 @@ export default {
 		isAdmin() {
 			return this.userRole === "admin";
 		},
-		updateSongPositioning({ moved }) {
+		repositionSong({ moved }) {
 			if (!moved) return; // we only need to update when song is moved
 
-			const songsBeingChanged = [];
-
-			this.playlist.songs.forEach((song, index) => {
-				if (song.position !== index + 1)
-					songsBeingChanged.push({
-						youtubeId: song.youtubeId,
-						position: index + 1
-					});
-			});
-
 			this.socket.dispatch(
-				"playlists.repositionSongs",
+				"playlists.repositionSong",
 				this.playlist._id,
-				songsBeingChanged,
+				{
+					...moved.element,
+					oldIndex: moved.oldIndex,
+					newIndex: moved.newIndex
+				},
 				res => {
-					new Toast(res.message);
+					if (res.status !== "success")
+						this.repositionSongInList({
+							...moved.element,
+							newIndex: moved.oldIndex,
+							oldIndex: moved.newIndex
+						});
 				}
 			);
 		},
+		moveSongToTop(song, index) {
+			this.repositionSongInPlaylist({
+				moved: {
+					element: song,
+					oldIndex: index,
+					newIndex: 0
+				}
+			});
+		},
+		moveSongToBottom(song, index) {
+			this.repositionSongInPlaylist({
+				moved: {
+					element: song,
+					oldIndex: index,
+					newIndex: this.songsList.length
+				}
+			});
+		},
 		totalLength() {
 			let length = 0;
 			this.playlist.songs.forEach(song => {
@@ -641,25 +647,24 @@ export default {
 			);
 		},
 		removeSongFromPlaylist(id) {
-			if (this.playlist.displayName === "Liked Songs") {
-				this.socket.dispatch("songs.unlike", id, res => {
+			if (this.playlist.displayName === "Liked Songs")
+				return this.socket.dispatch("songs.unlike", id, res => {
 					new Toast(res.message);
 				});
-			}
-			if (this.playlist.displayName === "Disliked Songs") {
-				this.socket.dispatch("songs.undislike", id, res => {
+
+			if (this.playlist.displayName === "Disliked Songs")
+				return this.socket.dispatch("songs.undislike", id, res => {
 					new Toast(res.message);
 				});
-			} else {
-				this.socket.dispatch(
-					"playlists.removeSongFromPlaylist",
-					id,
-					this.playlist._id,
-					res => {
-						new Toast(res.message);
-					}
-				);
-			}
+
+			return this.socket.dispatch(
+				"playlists.removeSongFromPlaylist",
+				id,
+				this.playlist._id,
+				res => {
+					new Toast(res.message);
+				}
+			);
 		},
 		renamePlaylist() {
 			const { displayName } = this.playlist;
@@ -717,24 +722,6 @@ export default {
 					() => new Toast("Failed to export and download playlist.")
 				);
 		},
-		moveSongToTop(index) {
-			this.playlist.songs.splice(
-				0,
-				0,
-				this.playlist.songs.splice(index, 1)[0]
-			);
-
-			this.updateSongPositioning({ moved: {} });
-		},
-		moveSongToBottom(index) {
-			this.playlist.songs.splice(
-				this.playlist.songs.length,
-				0,
-				this.playlist.songs.splice(index, 1)[0]
-			);
-
-			this.updateSongPositioning({ moved: {} });
-		},
 		updatePrivacy() {
 			const { privacy } = this.playlist;
 			if (privacy === "public" || privacy === "private") {

frontend/src/mixins/SortablePlaylists.vue

@@ -55,6 +55,7 @@ export default {
 		this.socket.on(
 			"event:playlist.song.added",
 			res => {
+				console.log("added");
 				this.playlists.forEach((playlist, index) => {
 					if (playlist._id === res.data.playlistId) {
 						this.playlists[index].songs.push(res.data.song);