From 1fc89d31f06422ee232d08d5858bc63c79429e20 Mon Sep 17 00:00:00 2001 From: Noam Bendelac Date: Wed, 19 May 2021 00:45:12 -0400 Subject: [PATCH 1/8] Refactor sit. and sep. chat event actions --- client/src/Playlist-Editor/Chat/ChatMessage.tsx | 2 +- client/src/shared/dbTypes.ts | 11 +++++------ src/routes/api/playlists.ts | 5 ++--- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/client/src/Playlist-Editor/Chat/ChatMessage.tsx b/client/src/Playlist-Editor/Chat/ChatMessage.tsx index 62d34f7..e5effeb 100644 --- a/client/src/Playlist-Editor/Chat/ChatMessage.tsx +++ b/client/src/Playlist-Editor/Chat/ChatMessage.tsx @@ -69,7 +69,7 @@ export const SituatedChatMessage = ({ {new Date(chatEvent.timestamp).toLocaleString()} - { chatEvent.action && + { chatEvent.action !== 'comment' &&

{ chatEvent.action === 'add' ? 'Added this track' diff --git a/client/src/shared/dbTypes.ts b/client/src/shared/dbTypes.ts index 9686aca..bdbf808 100644 --- a/client/src/shared/dbTypes.ts +++ b/client/src/shared/dbTypes.ts @@ -17,14 +17,14 @@ interface Event { /** - * A user leaves a message, possibly while adding/removing a track - * If action included, message can be empty string, otherwise message - * shouldn't be an empty string + * A user leaves a message, possibly while adding/removing the track + * If action is not 'comment', message can be empty string, otherwise message + * shouldn't be an empty string, though this is not currently enforced * This occurs in situated or hybrid mode */ export interface SituatedChatEvent extends Event { message: string, - action?: 'add' | 'remove', + action: 'comment' | 'add' | 'remove', } @@ -35,7 +35,6 @@ export interface SituatedChatEvent extends Event { * situated mode */ export interface SeparateChatAction extends Event { - type: 'action', action: 'add' | 'remove', trackId: string, } @@ -44,7 +43,7 @@ export interface SeparateChatAction extends Event { * Occurs in separate or hybrid mode */ export interface SeparateChatMessage extends Event { - type: 'message', + action: 'comment', message: string, } /** diff --git a/src/routes/api/playlists.ts b/src/routes/api/playlists.ts index 0728821..a0ce871 100644 --- a/src/routes/api/playlists.ts +++ b/src/routes/api/playlists.ts @@ -157,6 +157,7 @@ playlistIdRouter.post('/tracks/:trackId/chat/', { _id: playlistId }, { $push: { [`tracks.${dbTrackIndex}.chat`]: { + action: 'comment', message, timestamp: new Date(), userId: res.locals.userId, @@ -202,7 +203,6 @@ playlistIdRouter.put('/tracks/:trackId/removed', action: 'remove', } as SituatedChatEvent, chat: { - type: 'action', action: 'remove', trackId, timestamp: new Date(), @@ -255,7 +255,6 @@ playlistIdRouter.post('/tracks/', }] } as TrackObject, chat: { - type: 'action', action: 'add', trackId, timestamp: new Date(), @@ -289,7 +288,7 @@ playlistIdRouter.post('/chat/', { _id: playlistId }, { $push: { chat: { - type: 'message', + action: 'comment', message, timestamp: new Date(), userId: res.locals.userId, From 6cf18cf2274f555b339db866bdcd5d710762fca8 Mon Sep 17 00:00:00 2001 From: Noam Bendelac Date: Fri, 21 May 2021 03:35:58 -0400 Subject: [PATCH 2/8] Use safer type annotations for #88 --- .../Playlist-Editor/DraftAdditionSongRow.tsx | 5 ++-- client/src/Playlist-Editor/SavedSongRow.tsx | 5 ++-- client/src/util.ts | 13 ++++++++++ src/initializePlaylist.ts | 2 +- src/routes/api/index.ts | 11 ++++---- src/routes/api/playlists.ts | 25 ++++++++++--------- src/server.ts | 5 ++-- src/util.ts | 13 ++++++++++ 8 files changed, 55 insertions(+), 24 deletions(-) create mode 100644 client/src/util.ts create mode 100644 src/util.ts diff --git a/client/src/Playlist-Editor/DraftAdditionSongRow.tsx b/client/src/Playlist-Editor/DraftAdditionSongRow.tsx index e4f02b9..b4e80ea 100644 --- a/client/src/Playlist-Editor/DraftAdditionSongRow.tsx +++ b/client/src/Playlist-Editor/DraftAdditionSongRow.tsx @@ -12,6 +12,7 @@ import { PlaylistTrackObject, PostTrackRequest } from '../shared/apiTypes' import { handleApiError } from '../api' import { postWrapper } from '../fetchWrapper' import { useParams } from 'react-router' +import { asType } from '../util' /** @@ -42,10 +43,10 @@ export const DraftAdditionSongRow = ({ const onSubmit = async (message: string) => { const response = await postWrapper( `/api/playlists/${playlistId}/tracks`, - { + asType({ message, trackId: track.id - } as PostTrackRequest, + }), ) handleApiError(response) diff --git a/client/src/Playlist-Editor/SavedSongRow.tsx b/client/src/Playlist-Editor/SavedSongRow.tsx index 26f7e5a..5a0b0cb 100644 --- a/client/src/Playlist-Editor/SavedSongRow.tsx +++ b/client/src/Playlist-Editor/SavedSongRow.tsx @@ -11,6 +11,7 @@ import { PlaylistTrackObject, PostSituatedChatRequest, PutTrackRemovedRequest } import { useParams } from 'react-router' import { postWrapper } from '../fetchWrapper' import { handleApiError } from '../api' +import { asType } from '../util' @@ -63,12 +64,12 @@ export const SavedSongRow = ({ const response = removingThis ? await postWrapper( `/api/playlists/${playlistId}/tracks/${track.id}/removed/`, - { message } as PutTrackRemovedRequest, + asType({ message }), { method: 'PUT' } ) : await postWrapper( `/api/playlists/${playlistId}/tracks/${track.id}/chat/`, - { message } as PostSituatedChatRequest, + asType({ message }), ) handleApiError(response) diff --git a/client/src/util.ts b/client/src/util.ts new file mode 100644 index 0000000..dd3b46f --- /dev/null +++ b/client/src/util.ts @@ -0,0 +1,13 @@ + + +/** + * static type assertion + * makes the compiler check that the argument is of type T; get autocompletion + * hints in a much safer way than `as` type casting that gives proper errors + * for missing, mismatched, or extraneous properties + * source: https://github.com/microsoft/TypeScript/issues/7481 + */ +export const asType = (t: T): T => t + + + diff --git a/src/initializePlaylist.ts b/src/initializePlaylist.ts index 0e7b60e..c4e2ce4 100644 --- a/src/initializePlaylist.ts +++ b/src/initializePlaylist.ts @@ -19,7 +19,7 @@ export const initializePlaylist = async ( users: config.userIds, chat: [], tracks: spotifyPlaylist.tracks.items.map(initializeTrackObject), - }) as PlaylistDocument + }) } diff --git a/src/routes/api/index.ts b/src/routes/api/index.ts index 2cab409..fae3f57 100644 --- a/src/routes/api/index.ts +++ b/src/routes/api/index.ts @@ -8,6 +8,7 @@ import { } from '../../../client/src/shared/apiTypes' import { accessTokenCache, refreshTokenCache } from '../../userCache' import { spotifyApi } from '../../ownerAccount' +import { asType } from '../../util' @@ -52,7 +53,7 @@ apiRouter.get('/refresh_token', async (req, res, next) => { accessTokenCache.set(data.body.access_token, userId) res.cookie('access_token', data.body.access_token, { maxAge: data.body.expires_in * 1000 }) - res.json({ expires_in: data.body.expires_in } as GetRefreshTokenResponse) + res.json(asType({ expires_in: data.body.expires_in })) }) @@ -78,7 +79,7 @@ apiRouter.use((req, res: Res, next) => { * json body if not logged in */ apiRouter.get('/login', (req, res: Res, next) => { - res.json({ userId: res.locals.userId } as GetLoginResponse) + res.json(asType({ userId: res.locals.userId })) }) @@ -97,7 +98,7 @@ apiRouter.get('/search', async (req, res) => { const data = await spotifyApi.searchTracks(q) - res.json(data.body as GetTrackSearchResponse) + res.json(asType(data.body)) }) @@ -140,13 +141,13 @@ apiRouter.get('/users/:id', async (req, res) => { /** * catch server errors */ -apiRouter.use(((err, req, res, next) => { +apiRouter.use(asType((err, req, res, next) => { console.error(`ERROR at ${req.method} ${req.originalUrl}:`) console.error(err) // always respond with json; never send err (might leak server info) res.status(err.status ?? 500) .json({}) // could use `err.client ?? {}` -}) as express.ErrorRequestHandler) +})) /** diff --git a/src/routes/api/playlists.ts b/src/routes/api/playlists.ts index a0ce871..a3005cf 100644 --- a/src/routes/api/playlists.ts +++ b/src/routes/api/playlists.ts @@ -15,6 +15,7 @@ import { } from '../../../client/src/shared/apiTypes' import { spotifyApi } from '../../ownerAccount' import { playlistsDB, usersDB } from '../../db' +import { asType } from '../../util' @@ -156,12 +157,12 @@ playlistIdRouter.post('/tracks/:trackId/chat/', await playlistsDB.update( { _id: playlistId }, { $push: { [`tracks.${dbTrackIndex}.chat`]: - { + asType({ action: 'comment', message, timestamp: new Date(), userId: res.locals.userId, - } as SituatedChatEvent + }) } } ) @@ -196,18 +197,18 @@ playlistIdRouter.put('/tracks/:trackId/removed', { _id: playlistId }, { $push: { - [`tracks.${dbTrackIndex}.chat`]: { + [`tracks.${dbTrackIndex}.chat`]: asType({ message, timestamp: new Date(), userId: res.locals.userId, action: 'remove', - } as SituatedChatEvent, - chat: { + }), + chat: asType({ action: 'remove', trackId, timestamp: new Date(), userId: res.locals.userId, - } as SeparateChatAction, + }), }, $set: { [`tracks.${dbTrackIndex}.removed`]: true @@ -243,7 +244,7 @@ playlistIdRouter.post('/tracks/', { _id: playlistId }, { $push: { - tracks: { + tracks: asType({ id: trackId, removed: false, addedBy: res.locals.userId, @@ -253,13 +254,13 @@ playlistIdRouter.post('/tracks/', userId: res.locals.userId, action: 'add', }] - } as TrackObject, - chat: { + }), + chat: asType({ action: 'add', trackId, timestamp: new Date(), userId: res.locals.userId, - } as SeparateChatAction + }) } } ) @@ -287,12 +288,12 @@ playlistIdRouter.post('/chat/', await playlistsDB.update( { _id: playlistId }, { $push: { chat: - { + asType({ action: 'comment', message, timestamp: new Date(), userId: res.locals.userId, - } as SeparateChatMessage + }) } } ) diff --git a/src/server.ts b/src/server.ts index f57e658..c8b410a 100644 --- a/src/server.ts +++ b/src/server.ts @@ -64,6 +64,7 @@ import { apiRouter } from './routes/api' app.use('/api', apiRouter) import { adminRouter } from './routes/admin' +import { asType } from './util' app.use('/admin', adminRouter) @@ -112,11 +113,11 @@ app.get('/*', (req, res) => { /** * Error handler */ -app.use(((err, req, res, next) => { +app.use(asType((err, req, res, next) => { console.error(`ERROR at ${req.method} ${req.originalUrl}:`) console.error(err) res.sendStatus(err.status ?? 500) -}) as express.ErrorRequestHandler) +})) app.listen(PORT, () => { diff --git a/src/util.ts b/src/util.ts new file mode 100644 index 0000000..dd3b46f --- /dev/null +++ b/src/util.ts @@ -0,0 +1,13 @@ + + +/** + * static type assertion + * makes the compiler check that the argument is of type T; get autocompletion + * hints in a much safer way than `as` type casting that gives proper errors + * for missing, mismatched, or extraneous properties + * source: https://github.com/microsoft/TypeScript/issues/7481 + */ +export const asType = (t: T): T => t + + + From 7ef96f054f99e52a62fe33a107f0d99615263b11 Mon Sep 17 00:00:00 2001 From: Noam Bendelac Date: Sat, 22 May 2021 01:51:48 -0400 Subject: [PATCH 3/8] Add re-add to /:trackId/removed, removedTracks arr Add 're-add' action to situated and separate chat, add removedTracks array to playlist document, and make PUT `.../:trackId/removed` support re-adding removed tracks. --- .../Playlist-Editor/DraftAdditionSongRow.tsx | 1 - client/src/Playlist-Editor/SavedSongRow.tsx | 5 +- client/src/shared/apiTypes.ts | 3 +- client/src/shared/dbTypes.ts | 21 +++- src/initializePlaylist.ts | 2 +- src/routes/api/playlists.ts | 101 +++++++++++++----- 6 files changed, 95 insertions(+), 38 deletions(-) diff --git a/client/src/Playlist-Editor/DraftAdditionSongRow.tsx b/client/src/Playlist-Editor/DraftAdditionSongRow.tsx index b4e80ea..af36dbd 100644 --- a/client/src/Playlist-Editor/DraftAdditionSongRow.tsx +++ b/client/src/Playlist-Editor/DraftAdditionSongRow.tsx @@ -29,7 +29,6 @@ export const DraftAdditionSongRow = ({ const track: PlaylistTrackObject = { ...trackData, chat: [], - removed: false, addedBy: 'You', // supposed to be an id, idk whether to use user's id } diff --git a/client/src/Playlist-Editor/SavedSongRow.tsx b/client/src/Playlist-Editor/SavedSongRow.tsx index 5a0b0cb..79f27c3 100644 --- a/client/src/Playlist-Editor/SavedSongRow.tsx +++ b/client/src/Playlist-Editor/SavedSongRow.tsx @@ -64,7 +64,10 @@ export const SavedSongRow = ({ const response = removingThis ? await postWrapper( `/api/playlists/${playlistId}/tracks/${track.id}/removed/`, - asType({ message }), + asType({ + remove: true, + message + }), { method: 'PUT' } ) : await postWrapper( diff --git a/client/src/shared/apiTypes.ts b/client/src/shared/apiTypes.ts index 61a26e9..6bf7ca1 100644 --- a/client/src/shared/apiTypes.ts +++ b/client/src/shared/apiTypes.ts @@ -33,7 +33,8 @@ export interface PostSituatedChatRequest { } export interface PutTrackRemovedRequest { - message: string + remove: boolean, + message: string, } export interface PostTrackRequest { diff --git a/client/src/shared/dbTypes.ts b/client/src/shared/dbTypes.ts index bdbf808..649fd25 100644 --- a/client/src/shared/dbTypes.ts +++ b/client/src/shared/dbTypes.ts @@ -24,7 +24,7 @@ interface Event { */ export interface SituatedChatEvent extends Event { message: string, - action: 'comment' | 'add' | 'remove', + action: 'comment' | 'add' | 'remove' | 're-add', } @@ -35,7 +35,7 @@ export interface SituatedChatEvent extends Event { * situated mode */ export interface SeparateChatAction extends Event { - action: 'add' | 'remove', + action: 'add' | 'remove' | 're-add', trackId: string, } /** @@ -53,8 +53,7 @@ export type SeparateChatEvent = SeparateChatAction | SeparateChatMessage /** - * track in playlist; keeps track of whether it has been removed or not (or - * readded) + * track in playlist that has either not been removed, or has been readded * Chat can be empty array if no messages/actions yet or if in separate mode */ export interface TrackObject { @@ -66,7 +65,18 @@ export interface TrackObject { // always be the adder in spotify's data: addedBy: string, chat: SituatedChatEvent[], // situated - removed: boolean, +} + +/** + * Similar to TrackObject, but keeps track of who removed it + * TODO extend Pick + * like api's PlaylistTrackObject (and refactor api to have just needed data) + */ +export interface RemovedTrackObject { + id: string, + // user id of remover + removedBy: string, + chat: SituatedChatEvent[], // situated } /** @@ -76,6 +86,7 @@ export interface TrackObject { export interface PlaylistDocument extends Document { users: string[], // many-to-many tracks: TrackObject[], + removedTracks: RemovedTrackObject[], chat: SeparateChatEvent[], // separate chatMode: 'situated' | 'separate' | 'hybrid', } diff --git a/src/initializePlaylist.ts b/src/initializePlaylist.ts index c4e2ce4..382f2a1 100644 --- a/src/initializePlaylist.ts +++ b/src/initializePlaylist.ts @@ -19,6 +19,7 @@ export const initializePlaylist = async ( users: config.userIds, chat: [], tracks: spotifyPlaylist.tracks.items.map(initializeTrackObject), + removedTracks: [], }) } @@ -32,7 +33,6 @@ const initializeTrackObject = (spotifyTrack: SpotifyApi.PlaylistTrackObject): Tr return { id: spotifyTrack.track.id, chat: [], - removed: false, addedBy: spotifyTrack.added_by.id, // TODO will always be wrong, wont-fix? } } diff --git a/src/routes/api/playlists.ts b/src/routes/api/playlists.ts index a3005cf..246776f 100644 --- a/src/routes/api/playlists.ts +++ b/src/routes/api/playlists.ts @@ -7,6 +7,7 @@ import { SeparateChatMessage, SeparateChatAction, PlaylistDocument, + RemovedTrackObject, } from '../../../client/src/shared/dbTypes' import { GetPlaylistIdResponse, PostSituatedChatRequest, PutTrackRemovedRequest, @@ -117,7 +118,6 @@ playlistIdRouter.get('/', owner: spotifyPlaylist.owner, followers: spotifyPlaylist.followers, tracks: res.locals.dbPlaylist.tracks - .filter(dbTrack => !dbTrack.removed) .map(dbTrack => { const spotifyTrack = spotifyPlaylist.tracks.items .find(spotifyTrack => spotifyTrack.track.id === dbTrack.id) @@ -174,7 +174,7 @@ playlistIdRouter.post('/tracks/:trackId/chat/', ) /** - * Remove existing song in playlist + * Remove existing track in playlist or re-add after previously removing * Removal is not a DELETE; it's gone from spotify, but our * backend remembers the chat history * Body should include a message, but it can be an empty string @@ -182,38 +182,82 @@ playlistIdRouter.post('/tracks/:trackId/chat/', playlistIdRouter.put('/tracks/:trackId/removed', async (req, res: Res, next) => { try { - const { message } = req.body as PutTrackRemovedRequest + const { message, remove } = req.body as PutTrackRemovedRequest const { playlistId, trackId } = req.params - console.log({message, playlistId, trackId}) + console.log({message, remove, playlistId, trackId}) - await spotifyApi.removeTracksFromPlaylist( - playlistId, [{ uri: `spotify:track:${trackId}` }] - ) + // TODO illegal state checks? e.g. is track present in the correct list + // (removed vs not removed). also json body checks? - const dbTrackIndex = res.locals.dbPlaylist.tracks.findIndex( - track => track.id === trackId + if (remove) { + await spotifyApi.removeTracksFromPlaylist( + playlistId, [{ uri: `spotify:track:${trackId}` }] + ) + } else { + await spotifyApi.addTracksToPlaylist( + playlistId, [`spotify:track:${trackId}`] + ) + } + + + // we are now going to mutate dbPlaylist and update the whole db document + // because nedb doesn't have support for the specific array operations we + // need (also js doesn't have great immutable array methods so we use + // mutation) + + const dbPlaylist = res.locals.dbPlaylist + + // list we're moving the track from + const listFrom = remove ? dbPlaylist.tracks : dbPlaylist.removedTracks + + const dbTrackIndex = listFrom.findIndex( + (track: TrackObject | RemovedTrackObject) => track.id === trackId ) + // TODO check track is present in listFrom + + // remove and get track object + const [trackObject] = listFrom.splice(dbTrackIndex) + + // add chat messages + trackObject.chat.push(asType({ + message, + timestamp: new Date(), + userId: res.locals.userId, + action: remove ? 'remove' : 're-add', + })) + dbPlaylist.chat.push(asType({ + action: remove ? 'remove' : 're-add', + timestamp: new Date(), + userId: res.locals.userId, + trackId: trackId, + })) + + if (remove) { + // convert track to removed track type + const removedTrackObject: RemovedTrackObject = { + id: trackObject.id, + chat: trackObject.chat, + removedBy: res.locals.userId, + // TODO add spotify data like name, album, etc + } + // add removed track to start of "removed" list, will show up at the top + // of the removed section of the table on the frontend + dbPlaylist.removedTracks.unshift(removedTrackObject) + } else { + // convert + const newTrackObject: TrackObject = { + id: trackObject.id, + chat: trackObject.chat, + addedBy: res.locals.userId, + } + // add track to end of list to reflect position in spotify playlist + dbPlaylist.tracks.push(newTrackObject) + } + + // replace the whole document with the new object; _id is unchanged await playlistsDB.update( { _id: playlistId }, - { - $push: { - [`tracks.${dbTrackIndex}.chat`]: asType({ - message, - timestamp: new Date(), - userId: res.locals.userId, - action: 'remove', - }), - chat: asType({ - action: 'remove', - trackId, - timestamp: new Date(), - userId: res.locals.userId, - }), - }, - $set: { - [`tracks.${dbTrackIndex}.removed`]: true - } - } + dbPlaylist ) res.status(200).json({}) @@ -246,7 +290,6 @@ playlistIdRouter.post('/tracks/', $push: { tracks: asType({ id: trackId, - removed: false, addedBy: res.locals.userId, chat: [{ message, From babb24106d6932fa1e7c705eec636db0ce7644dd Mon Sep 17 00:00:00 2001 From: Noam Bendelac Date: Sat, 22 May 2021 02:59:48 -0400 Subject: [PATCH 4/8] Add some backend checks for POST `.../tracks/` TODO add other checks for other endpoints like `.../:trackId/removed`? --- src/routes/api/playlists.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/routes/api/playlists.ts b/src/routes/api/playlists.ts index 246776f..9f5905c 100644 --- a/src/routes/api/playlists.ts +++ b/src/routes/api/playlists.ts @@ -178,6 +178,8 @@ playlistIdRouter.post('/tracks/:trackId/chat/', * Removal is not a DELETE; it's gone from spotify, but our * backend remembers the chat history * Body should include a message, but it can be an empty string + * Body should include a boolean 'remove' that is true if the track is already + * present, or false if the track is already removed */ playlistIdRouter.put('/tracks/:trackId/removed', async (req, res: Res, next) => { @@ -272,14 +274,24 @@ playlistIdRouter.put('/tracks/:trackId/removed', * Add song to playlist * Body should include trackId * Body should include a message, but it can be an empty string + * responds with error 422 if track already exists in `tracks` or + * `removedTracks` */ playlistIdRouter.post('/tracks/', - async (req, res: Res, next) => { + async (req, res: Res, next) => { try { const { message, trackId } = req.body as PostTrackRequest const { playlistId } = req.params console.log({message, playlistId, trackId}) + // check if track already exists + if ( + (res.locals.dbPlaylist.tracks.find(track => track.id === trackId) !== undefined) || + (res.locals.dbPlaylist.removedTracks.find(track => track.id === trackId) !== undefined) + ) { + return next({ status: 422 }) + } + await spotifyApi.addTracksToPlaylist( playlistId, [`spotify:track:${trackId}`] ) From 394add2b34f51de6b3e5b0fcf07ff46b2ae72311 Mon Sep 17 00:00:00 2001 From: Noam Bendelac Date: Sun, 23 May 2021 02:52:42 -0400 Subject: [PATCH 5/8] Refactor search & playlist tracks API #70 #71 Refactor api types for track search and tracks within `.../playlists/:id` to have just the data needed by the frontend, and move some of the processing from the frontend in several places to the backend in one place. These were done together so the draft addition track's data can be mocked in `SearchResults.tsx` `addButtonOnClick` callback. This will now allow spotify data on removed tracks to be saved when they are removed for #87. --- .../Playlist-Editor/DraftAdditionSongRow.tsx | 6 +-- client/src/Playlist-Editor/PlaylistInfo.tsx | 25 +++--------- client/src/Playlist-Editor/SavedSongRow.tsx | 5 +-- client/src/Playlist-Editor/SearchPanel.tsx | 17 ++++----- client/src/Playlist-Editor/SearchResults.tsx | 38 ++++++++----------- client/src/shared/apiTypes.ts | 31 ++++++++------- src/routes/api/index.ts | 15 ++++++-- src/routes/api/playlists.ts | 18 +++++---- 8 files changed, 72 insertions(+), 83 deletions(-) diff --git a/client/src/Playlist-Editor/DraftAdditionSongRow.tsx b/client/src/Playlist-Editor/DraftAdditionSongRow.tsx index af36dbd..f936a7f 100644 --- a/client/src/Playlist-Editor/DraftAdditionSongRow.tsx +++ b/client/src/Playlist-Editor/DraftAdditionSongRow.tsx @@ -32,8 +32,6 @@ export const DraftAdditionSongRow = ({ addedBy: 'You', // supposed to be an id, idk whether to use user's id } - const artistNames = track.artists.map(artist => artist.name).join(', ') - const { setModificationState, loadPlaylist } = useContext(playlistContext) const { id: playlistId } = useParams() @@ -80,8 +78,8 @@ export const DraftAdditionSongRow = ({

{track.name}
-
{artistNames}
-
{track.album.name}
+
{track.artists}
+
{track.album}
{track.addedBy}
} diff --git a/client/src/Playlist-Editor/SavedSongRow.tsx b/client/src/Playlist-Editor/SavedSongRow.tsx index 79f27c3..2024ec0 100644 --- a/client/src/Playlist-Editor/SavedSongRow.tsx +++ b/client/src/Playlist-Editor/SavedSongRow.tsx @@ -28,7 +28,6 @@ export const SavedSongRow = ({ addedByUsers: Record, }) => { - const artistNames = track.artists.map(artist => artist.name).join(', ') const addedByUser = addedByUsers[track.addedBy] const { @@ -128,8 +127,8 @@ export const SavedSongRow = ({ }
{track.name}
-
{artistNames}
-
{track.album.name}
+
{track.artists}
+
{track.album}
{addedByUser.display_name}
{/* only provide the remove button as an option if no other track is diff --git a/client/src/Playlist-Editor/SearchPanel.tsx b/client/src/Playlist-Editor/SearchPanel.tsx index 03c3098..24af841 100644 --- a/client/src/Playlist-Editor/SearchPanel.tsx +++ b/client/src/Playlist-Editor/SearchPanel.tsx @@ -1,16 +1,15 @@ import React, { useState, useEffect, CSSProperties } from 'react' import { useDebounceCallback } from '@react-hook/debounce' -import { useResource, fetchWrapper, Resource } from '../fetchWrapper' +import { useResource, fetchWrapper } from '../fetchWrapper' import { SearchResults } from "./SearchResults" import { classes, colors } from "../styles" import { handleApiError } from '../api' +import { GetTrackSearchResponse } from '../shared/apiTypes' -type SongResults = SpotifyApi.TrackSearchResponse - -export const useSongSearch = (query: string): Resource => { - const [resource, setter] = useResource(null) +export const useSongSearch = (query: string) => { + const [resource, setter] = useResource(null) useEffect(() => { if (query !== '') { @@ -18,7 +17,7 @@ export const useSongSearch = (query: string): Resource => { setter({ loading: true, }) - const response = await fetchWrapper(`/api/search?q=${query}`) + const response = await fetchWrapper(`/api/search?q=${query}`) handleApiError(response) setter({ loading: false, @@ -82,7 +81,7 @@ export const SearchPanel = ({ const [query, setQuery] = useState('') - const { data: result } = useSongSearch(query) + const searchResults = useSongSearch(query) const searchTabStyle = { ...style, @@ -112,10 +111,10 @@ export const SearchPanel = ({ placeholder="Search to add track..." /> - { result && + { searchResults.data && } diff --git a/client/src/Playlist-Editor/SearchResults.tsx b/client/src/Playlist-Editor/SearchResults.tsx index e036f71..22a3111 100644 --- a/client/src/Playlist-Editor/SearchResults.tsx +++ b/client/src/Playlist-Editor/SearchResults.tsx @@ -6,26 +6,25 @@ import { faPlusCircle } from '@fortawesome/free-solid-svg-icons' import { useHover } from '../useHover' import { playlistContext } from './playlistContext' import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' +import { GetTrackSearchItem, GetTrackSearchResponse } from '../shared/apiTypes' +const itemNotFirstStyle = { + marginTop: '1.0rem', +} export const SearchResults = ({ - data, + searchResults, style, }: { - data: SpotifyApi.TrackSearchResponse, + searchResults: GetTrackSearchResponse, style?: CSSProperties, }) => { - const items = data.tracks.items - - const itemNotFirstStyle = { - marginTop: '1.0rem', - } return - {items.map((item, index) => + {searchResults.map((track, index) => @@ -69,29 +68,22 @@ const addButtonStyle = { } as const const SearchItem = ({ - item, + track, style, }: { - item: SpotifyApi.TrackObjectFull, + track: GetTrackSearchItem, style: CSSProperties, }) => { - const { name, artists, album } = item - const image = album.images[2] - const artistNames = artists.map(artist => artist.name).join(', ') const { modificationState, setModificationState } = useContext(playlistContext) const [addButtonIsHovered, addButtonHoverProps, setAddButtonIsHovered] = useHover() const addButtonOnClick = () => { + const { id, album, artists, name } = track setModificationState({ userAction: 'add', - trackData: { - id: item.id, - album: item.album, - artists: item.artists, - name: item.name, - } + trackData: { id, album, artists, name }, }) setAddButtonIsHovered(false) // otherwise, stays hovered if addition is cancelled } @@ -106,10 +98,10 @@ const SearchItem = ({ } return
- {`Album: + {`Album:
-
{name}
-
{artistNames}
+
{track.name}
+
{track.artists}
{ modificationState.userAction === 'view' &&
- { track.chat.map((chatEvent, index) => + { chat.map((chatEvent, index) => ) }
diff --git a/client/src/Playlist-Editor/DraftAdditionSongRow.tsx b/client/src/Playlist-Editor/DraftAdditionSongRow.tsx index f936a7f..0a413fd 100644 --- a/client/src/Playlist-Editor/DraftAdditionSongRow.tsx +++ b/client/src/Playlist-Editor/DraftAdditionSongRow.tsx @@ -92,7 +92,7 @@ export const DraftAdditionSongRow = ({
{ @@ -78,6 +79,7 @@ const usePlaylistData = (playlistId: string) => { +const padding = '2.0rem' const searchTabStyle = { flex: 0.2, @@ -95,12 +97,23 @@ const playlistTableStyle = { } const tHeadStyle = { background: colors.grayscale.gray, - padding: '2.0rem 2.0rem 0', + padding: `${padding} ${padding} 0`, position: 'sticky', top: 0, } as const const songsStyle = { - padding: '0 2.0rem 2.0rem', + padding: `0 ${padding}`, +} +const removedHeaderStyle = { + padding: `4.0rem ${padding} 0`, +} +const removedHeadingStyle = { + ...classes.text, + ...classes.bold, + fontSize: '2.4rem', +} +const bottomSpaceStyle: CSSProperties = { + minHeight: '5.0rem', } const separateChatStyle = { flex: 1, @@ -157,6 +170,18 @@ export const PlaylistEditor = ({ } + { playlist.data.removedTracks.length && <> +
+

Removed Tracks

+ +
+
+ { playlist.data.removedTracks.map(track => + + )} +
+ } +
}
{ false && { } +export const RemovedTracksHeader = () => { + return
+
+
+ Title +
+
+ Artist +
+
+ Album +
+
+ Removed by +
+
+
+} + diff --git a/client/src/Playlist-Editor/RemovedTrackRow.tsx b/client/src/Playlist-Editor/RemovedTrackRow.tsx new file mode 100644 index 0000000..1b8977a --- /dev/null +++ b/client/src/Playlist-Editor/RemovedTrackRow.tsx @@ -0,0 +1,145 @@ + +import React, { useContext, useState } from 'react' +import { useParams } from 'react-router' +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' +import { faChevronCircleDown, faChevronCircleUp, faPlusCircle } from '@fortawesome/free-solid-svg-icons' +import { handleApiError } from '../api' +import { postWrapper } from '../fetchWrapper' +import { PostSituatedChatRequest, PutTrackRemovedRequest } from '../shared/apiTypes' +import { RemovedTrackObject } from '../shared/dbTypes' +import { classes, colors } from '../styles' +import { useHover } from '../useHover' +import { asType } from '../util' +import { playlistContext } from './playlistContext' +import * as styles from './playlistTableRowStyles' +import { SituatedChat } from './Chat/SituatedChat' + + +export const RemovedTrackRow = ({ track }: { track: RemovedTrackObject }) => { + + const { + modificationState, setModificationState, loadPlaylist + } = useContext(playlistContext) + + const { id: playlistId } = useParams() + + // true if user is attempting to re-add *this* track; else the user is just + // viewing or commenting on this track + const reAddingThis = modificationState.userAction === "re-add" + && modificationState.trackId === track.id + + // situated chat expand/collapse button + const [chatExpanded, setChatExpanded] = useState(false) + + // button hover states + const [expandButtonIsHovered, expandButtonHoverProps] = useHover() + const [reAddButtonIsHovered, reAddButtonHoverProps, setReAddButtonIsHovered] = useHover() + + + // on click the right-most "re-add" button + const reAddButtonOnClick = () => { + setModificationState({ + userAction: 're-add', + trackId: track.id, + }) + setReAddButtonIsHovered(false) // otherwise, stays hovered if cancelled + } + + // on submit the chat form + const onSubmitChat = async (message: string) => { + const response = reAddingThis + ? await postWrapper( + `/api/playlists/${playlistId}/tracks/${track.id}/removed/`, + asType({ + remove: false, + message + }), + { method: 'PUT' } + ) + : await postWrapper( + `/api/playlists/${playlistId}/tracks/${track.id}/chat/`, + asType({ message }), + ) + handleApiError(response) + + if (!response.error) { + // just resets modification state to 'view' + setModificationState({ userAction: 'view' }) + // reload playlist to get updated tracks/chats + loadPlaylist() + // inform caller of success; this clears the textarea + return true + } + return false + } + // on cancel the chat form + const onCancelChat = () => { + if (reAddingThis) { + setModificationState({ userAction: 'view' }) + } else { + setChatExpanded(false) + } + } + + + + const expandButtonStyle = { + ...styles.rightButtonStyle, + background: colors.translucentWhite(expandButtonIsHovered ? 0.3 : 0.15), + } + const rightButtonStyle = { + ...styles.rightButtonStyle, + background: colors.translucentWhite(reAddButtonIsHovered ? 0.3 : 0.15), + } + + return
+
+
+ {/* only show expand/collapse button if not currently re-adding this track */} + { !reAddingThis && + + } +
+
{track.name}
+
{track.artists}
+
{track.album}
+
{track.removedBy}
+
+ {/* only provide the re-add button as an option if no other track is + currently selected for modification */} + { modificationState.userAction === "view" && + + } +
+
+ {/* only show chat if this track is selected for re-adding or chat is expanded */} + { (reAddingThis || chatExpanded) && + + } +
+} + diff --git a/client/src/Playlist-Editor/SavedSongRow.tsx b/client/src/Playlist-Editor/SavedSongRow.tsx index 2024ec0..1b6b60c 100644 --- a/client/src/Playlist-Editor/SavedSongRow.tsx +++ b/client/src/Playlist-Editor/SavedSongRow.tsx @@ -147,7 +147,7 @@ export const SavedSongRow = ({ {/* only show chat if this track is selected for removal or chat is expanded */} { (removingThis || chatExpanded) && Date: Sun, 23 May 2021 22:18:00 -0400 Subject: [PATCH 8/8] Fix re-add bug from Array.splice When there were multiple removed tracks and one of them was re-added, the other removed tracks disappeared because I assumed that if I leave off the second parameter to splice it would default to 1. (This behavior likely varied depending on the position of the re-added track within the removed tracks array, so maybe it didn't always happen). --- src/routes/api/playlists.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/api/playlists.ts b/src/routes/api/playlists.ts index 1c8cd29..eae62ed 100644 --- a/src/routes/api/playlists.ts +++ b/src/routes/api/playlists.ts @@ -225,7 +225,7 @@ playlistIdRouter.put('/tracks/:trackId/removed', // TODO check track is present in listFrom // remove and get track object - const [trackObject] = listFrom.splice(dbTrackIndex) + const [trackObject] = listFrom.splice(dbTrackIndex, 1) // add chat messages trackObject.chat.push(asType({