From f4ee82fa2cf17ded0bd7ebdd2897f3e9da0f3453 Mon Sep 17 00:00:00 2001
From: Sander Vocke <sandervocke@gmail.com>
Date: Thu, 24 Sep 2020 07:33:35 +0200
Subject: [PATCH] Fix PUT endpoints.

---
 .../components/common/SubmitChangesButton.tsx |  1 +
 client/src/components/windows/SongWindow.tsx  | 27 +++++-
 client/src/lib/saveChanges.tsx                | 15 ++++
 .../endpoints/ModifyAlbumEndpointHandler.ts   | 60 ++++++++------
 .../endpoints/ModifyArtistEndpointHandler.ts  | 41 +++++----
 server/endpoints/ModifySongEndpointHandler.ts | 83 ++++++++++---------
 6 files changed, 141 insertions(+), 86 deletions(-)
 create mode 100644 client/src/lib/saveChanges.tsx

diff --git a/client/src/components/common/SubmitChangesButton.tsx b/client/src/components/common/SubmitChangesButton.tsx
index a6a794b..51ad1c5 100644
--- a/client/src/components/common/SubmitChangesButton.tsx
+++ b/client/src/components/common/SubmitChangesButton.tsx
@@ -4,6 +4,7 @@ import { Box, Button } from '@material-ui/core';
 export default function SubmitChangesButton(props: any) {
     return <Box>
         <Button
+            {...props}
             variant="contained" color="secondary"
         >
             Save Changes
diff --git a/client/src/components/windows/SongWindow.tsx b/client/src/components/windows/SongWindow.tsx
index 6c3aa2f..891f9e0 100644
--- a/client/src/components/windows/SongWindow.tsx
+++ b/client/src/components/windows/SongWindow.tsx
@@ -1,5 +1,5 @@
 import React, { useEffect, useState } from 'react';
-import { Box, Typography, IconButton, Button } from '@material-ui/core';
+import { Box, Typography, IconButton, Button, CircularProgress } from '@material-ui/core';
 import AudiotrackIcon from '@material-ui/icons/Audiotrack';
 import PersonIcon from '@material-ui/icons/Person';
 import AlbumIcon from '@material-ui/icons/Album';
@@ -10,6 +10,7 @@ import { AlbumMetadata } from './AlbumWindow';
 import StoreLinkIcon, { whichStore } from '../common/StoreLinkIcon';
 import EditableText from '../common/EditableText';
 import SubmitChangesButton from '../common/SubmitChangesButton';
+import saveSongChanges from '../../lib/saveChanges';
 
 export type SongMetadata = serverApi.SongDetails;
 export type SongMetadataChanges = serverApi.ModifySongRequest;
@@ -23,6 +24,7 @@ export interface SongWindowState extends WindowState {
 export enum SongWindowStateActions {
     SetMetadata = "SetMetadata",
     SetPendingChanges = "SetPendingChanges",
+    Reload = "Reload",
 }
 
 export function SongWindowReducer(state: SongWindowState, action: any) {
@@ -31,6 +33,12 @@ export function SongWindowReducer(state: SongWindowState, action: any) {
             return { ...state, metadata: action.value }
         case SongWindowStateActions.SetPendingChanges:
             return { ...state, pendingChanges: action.value }
+        case SongWindowStateActions.Reload:
+            return {
+                songId: state.songId,
+                metadata: null,
+                pendingChanges: null,
+            }
         default:
             throw new Error("Unimplemented SongWindow state update.")
     }
@@ -100,7 +108,7 @@ export default function SongWindow(props: IProps) {
         onChangeEditingValue={(v: string | null) => setEditingTitle(v)}
         onChangeChangedValue={(v: string | null) => {
             let newVal: any = { ...pendingChanges };
-            if(v) { newVal.title = v }
+            if (v) { newVal.title = v }
             else { delete newVal.title }
             props.dispatch({
                 type: SongWindowStateActions.SetPendingChanges,
@@ -134,8 +142,21 @@ export default function SongWindow(props: IProps) {
         </a>
     });
 
+    const [applying, setApplying] = useState(false);
     const maybeSubmitButton = pendingChanges && Object.keys(pendingChanges).length > 0 &&
-        <SubmitChangesButton/>
+        <Box>
+            <SubmitChangesButton onClick={() => {
+                setApplying(true);
+                saveSongChanges(props.state.songId, pendingChanges || {})
+                    .then(() => {
+                        setApplying(false);
+                        props.dispatch({
+                            type: SongWindowStateActions.Reload
+                        })
+                    })
+            }} />
+            {applying && <CircularProgress />}
+        </Box>
 
     return <Box width="100%" justifyContent="center" display="flex" flexWrap="wrap">
         <Box
diff --git a/client/src/lib/saveChanges.tsx b/client/src/lib/saveChanges.tsx
new file mode 100644
index 0000000..8411be4
--- /dev/null
+++ b/client/src/lib/saveChanges.tsx
@@ -0,0 +1,15 @@
+import * as serverApi from '../api';
+
+export default async function saveSongChanges(id: number, change: serverApi.ModifySongRequest) {
+    const requestOpts = {
+        method: 'PUT',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify(change),
+    };
+
+    const endpoint = serverApi.ModifySongEndpoint.replace(":id", id.toString());
+    const response = await fetch((process.env.REACT_APP_BACKEND || "") + endpoint, requestOpts)
+    if(!response.ok) {
+        throw new Error("Failed to save song changes: " + response.statusText);
+    }
+}
\ No newline at end of file
diff --git a/server/endpoints/ModifyAlbumEndpointHandler.ts b/server/endpoints/ModifyAlbumEndpointHandler.ts
index 4d2ae29..97f24e8 100644
--- a/server/endpoints/ModifyAlbumEndpointHandler.ts
+++ b/server/endpoints/ModifyAlbumEndpointHandler.ts
@@ -16,34 +16,40 @@ export const ModifyAlbumEndpointHandler: EndpointHandler = async (req: any, res:
 
     await knex.transaction(async (trx) => {
         try {
+
+            // Start retrieving the album itself.
+            const album = await trx.select('id')
+                .from('albums')
+                .where({ id: req.params.id })
+                .then((r: any) => (r && r[0]) ? r[0]['id'] : undefined);
+
+            const newAlbum = {
+                ...album,
+                ...reqObject
+            };
+
             // Start retrieving artists.
-            const artistIdsPromise = reqObject.artistIds ?
+            const artistIdsPromise = newAlbum.artistIds ?
                 trx.select('artistId')
                     .from('artists_albums')
-                    .whereIn('id', reqObject.artistIds)
+                    .whereIn('id', newAlbum.artistIds)
                     .then((as: any) => as.map((a: any) => a['artistId'])) :
-                (async () => { return [] })();
+                (async () => { return undefined })();
 
             // Start retrieving tags.
-            const tagIdsPromise = reqObject.tagIds ?
+            const tagIdsPromise = newAlbum.tagIds ?
                 trx.select('id')
                     .from('albums_tags')
-                    .whereIn('id', reqObject.tagIds)
+                    .whereIn('id', newAlbum.tagIds)
                     .then((ts: any) => ts.map((t: any) => t['tagId'])) :
-                (async () => { return [] })();
-
-            // Start retrieving the album itself.
-            const albumPromise = trx.select('id')
-                .from('albums')
-                .where({ id: req.params.id })
-                .then((r: any) => (r && r[0]) ? r[0]['id'] : undefined)
+                (async () => { return undefined })();
 
             // Wait for the requests to finish.
-            var [album, artists, tags] = await Promise.all([albumPromise, artistIdsPromise, tagIdsPromise]);;
+            var [artists, tags] = await Promise.all([artistIdsPromise, tagIdsPromise]);;
 
             // Check that we found all objects we need.
-            if ((reqObject.artistIds && artists.length !== reqObject.artistIds.length) ||
-                (reqObject.tagIds && tags.length !== reqObject.tagIds.length) ||
+            if ((newAlbum.artistIds && artists.length !== newAlbum.artistIds.length) ||
+                (newAlbum.tagIds && tags.length !== newAlbum.tagIds.length) ||
                 !album) {
                 const e: EndpointError = {
                     internalMessage: 'Not all albums and/or artists and/or tags exist for ModifyAlbum request: ' + JSON.stringify(req.body),
@@ -56,27 +62,27 @@ export const ModifyAlbumEndpointHandler: EndpointHandler = async (req: any, res:
             const modifyAlbumPromise = trx('albums')
                 .where({ 'id': req.params.id })
                 .update({
-                    name: reqObject.name,
-                    storeLinks: JSON.stringify(reqObject.storeLinks || []),
+                    name: newAlbum.name,
+                    storeLinks: JSON.stringify(newAlbum.storeLinks || []),
                 })
 
             // Remove unlinked artists.
             // TODO: test this!
-            const removeUnlinkedArtists = trx('artists_albums')
+            const removeUnlinkedArtists = artists ? trx('artists_albums')
                 .where({ 'albumId': req.params.id })
-                .whereNotIn('artistId', reqObject.artistIds || [])
-                .delete();
+                .whereNotIn('artistId', newAlbum.artistIds || [])
+                .delete() : undefined;
 
             // Remove unlinked tags.
             // TODO: test this!
-            const removeUnlinkedTags = trx('albums_tags')
+            const removeUnlinkedTags = tags ? trx('albums_tags')
                 .where({ 'albumId': req.params.id })
-                .whereNotIn('tagId', reqObject.tagIds || [])
-                .delete();
+                .whereNotIn('tagId', newAlbum.tagIds || [])
+                .delete() : undefined;
 
             // Link new artists.
             // TODO: test this!
-            const addArtists = trx('artists_albums')
+            const addArtists = artists ? trx('artists_albums')
                 .where({ 'albumId': req.params.id })
                 .then((as: any) => as.map((a: any) => a['artistId']))
                 .then((doneArtistIds: number[]) => {
@@ -97,11 +103,11 @@ export const ModifyAlbumEndpointHandler: EndpointHandler = async (req: any, res:
                             trx('artists_albums').insert(obj)
                         )
                     );
-                })
+                }) : undefined;
 
             // Link new tags.
             // TODO: test this!
-            const addTags = trx('albums_tags')
+            const addTags = tags ? trx('albums_tags')
                 .where({ 'albumId': req.params.id })
                 .then((ts: any) => ts.map((t: any) => t['tagId']))
                 .then((doneTagIds: number[]) => {
@@ -122,7 +128,7 @@ export const ModifyAlbumEndpointHandler: EndpointHandler = async (req: any, res:
                             trx('albums_tags').insert(obj)
                         )
                     );
-                })
+                }) : undefined;
 
             // Wait for all operations to finish.
             await Promise.all([
diff --git a/server/endpoints/ModifyArtistEndpointHandler.ts b/server/endpoints/ModifyArtistEndpointHandler.ts
index 165415b..6741952 100644
--- a/server/endpoints/ModifyArtistEndpointHandler.ts
+++ b/server/endpoints/ModifyArtistEndpointHandler.ts
@@ -17,25 +17,30 @@ export const ModifyArtistEndpointHandler: EndpointHandler = async (req: any, res
         try {
             const artistId = parseInt(req.params.id);
 
-            // Start retrieving tags.
-            const tagIdsPromise = reqObject.tagIds ?
-                trx.select('id')
-                    .from('artists_tags')
-                    .whereIn('id', reqObject.tagIds)
-                    .then((ts: any) => ts.map((t: any) => t['tagId'])) :
-                (async () => { return [] })();
-
             // Start retrieving the artist itself.
-            const artistPromise = trx.select('id')
+            const artist = await trx.select('id')
                 .from('artists')
                 .where({ id: artistId })
                 .then((r: any) => (r && r[0]) ? r[0]['id'] : undefined)
 
+            const newArtist = {
+                ...artist,
+                ...reqObject
+            }
+
+            // Start retrieving tags.
+            const tagIdsPromise = newArtist.tagIds ?
+                trx.select('id')
+                    .from('artists_tags')
+                    .whereIn('id', newArtist.tagIds)
+                    .then((ts: any) => ts.map((t: any) => t['tagId'])) :
+                (async () => { return undefined })();
+
             // Wait for the requests to finish.
-            var [artist, tags] = await Promise.all([artistPromise, tagIdsPromise]);;
+            var [tags] = await Promise.all([tagIdsPromise]);;
 
             // Check that we found all objects we need.
-            if ((reqObject.tagIds && tags.length !== reqObject.tagIds.length) ||
+            if ((newArtist.tagIds && tags.length !== newArtist.tagIds.length) ||
                 !artist) {
                 const e: EndpointError = {
                     internalMessage: 'Not all artists and/or tags exist for ModifyArtist request: ' + JSON.stringify(req.body),
@@ -48,22 +53,22 @@ export const ModifyArtistEndpointHandler: EndpointHandler = async (req: any, res
             const modifyArtistPromise = trx('artists')
                 .where({ 'id': artistId })
                 .update({
-                    name: reqObject.name,
-                    storeLinks: JSON.stringify(reqObject.storeLinks || []),
+                    name: newArtist.name,
+                    storeLinks: JSON.stringify(newArtist.storeLinks || []),
                 })
 
             // Remove unlinked tags.
             // TODO: test this!
-            const removeUnlinkedTags = reqObject.tagIds ?
+            const removeUnlinkedTags = tags ?
                 trx('artists_tags')
                     .where({ 'artistId': artistId })
-                    .whereNotIn('tagId', reqObject.tagIds || [])
+                    .whereNotIn('tagId', newArtist.tagIds || [])
                     .delete() :
-                (async () => undefined)();
+                undefined;
 
             // Link new tags.
             // TODO: test this!
-            const addTags = trx('artists_tags')
+            const addTags = tags ? trx('artists_tags')
                 .where({ 'artistId': artistId })
                 .then((ts: any) => ts.map((t: any) => t['tagId']))
                 .then((doneTagIds: number[]) => {
@@ -84,7 +89,7 @@ export const ModifyArtistEndpointHandler: EndpointHandler = async (req: any, res
                             trx('artists_tags').insert(obj)
                         )
                     );
-                });
+                }) : undefined;
 
             // Wait for all operations to finish.
             await Promise.all([
diff --git a/server/endpoints/ModifySongEndpointHandler.ts b/server/endpoints/ModifySongEndpointHandler.ts
index 980f62a..6b31ef4 100644
--- a/server/endpoints/ModifySongEndpointHandler.ts
+++ b/server/endpoints/ModifySongEndpointHandler.ts
@@ -16,44 +16,51 @@ export const ModifySongEndpointHandler: EndpointHandler = async (req: any, res:
 
     await knex.transaction(async (trx) => {
         try {
+            // Retrieve the song to be modified itself.
+            const song = await trx.select('id')
+                .from('songs')
+                .where({ id: req.params.id })
+                .then((r: any) => (r && r[0]) ? r[0]['id'] : undefined)
+
+            // Construct the new object from the original plus the
+            // changes included in the PUT.
+            const newSong = {
+                ...song,
+                ...reqObject,
+            };
+
             // Start retrieving artists.
-            const artistIdsPromise = reqObject.artistIds ?
+            const artistIdsPromise = newSong.artistIds ?
                 trx.select('artistId')
                     .from('songs_artists')
-                    .whereIn('id', reqObject.artistIds)
+                    .whereIn('id', newSong.artistIds)
                     .then((as: any) => as.map((a: any) => a['artistId'])) :
-                (async () => { return [] })();
+                (async () => { return undefined })();
 
             // Start retrieving tags.
-            const tagIdsPromise = reqObject.tagIds ?
+            const tagIdsPromise = newSong.tagIds ?
                 trx.select('id')
                     .from('songs_tags')
-                    .whereIn('id', reqObject.tagIds)
+                    .whereIn('id', newSong.tagIds)
                     .then((ts: any) => ts.map((t: any) => t['tagId'])) :
-                (async () => { return [] })();
+                (async () => { return undefined })();
 
             // Start retrieving albums.
-            const albumIdsPromise = reqObject.albumIds ?
+            const albumIdsPromise = newSong.albumIds ?
                 trx.select('id')
                     .from('songs_albums')
-                    .whereIn('id', reqObject.albumIds)
+                    .whereIn('id', newSong.albumIds)
                     .then((as: any) => as.map((a: any) => a['albumId'])) :
-                (async () => { return [] })();
-
-            // Start retrieving the song itself.
-            const songPromise = trx.select('id')
-                .from('songs')
-                .where({ id: req.params.id })
-                .then((r: any) => (r && r[0]) ? r[0]['id'] : undefined)
+                (async () => { return undefined })();
 
             // Wait for the requests to finish.
-            var [song, artists, tags, albums] =
-                await Promise.all([songPromise, artistIdsPromise, tagIdsPromise, albumIdsPromise]);;
+            var [artists, tags, albums] =
+                await Promise.all([artistIdsPromise, tagIdsPromise, albumIdsPromise]);;
 
             // Check that we found all objects we need.
-            if ((reqObject.artistIds && artists.length !== reqObject.artistIds.length) ||
-                (reqObject.tagIds && tags.length !== reqObject.tagIds.length) ||
-                (reqObject.albumIds && albums.length !== reqObject.albumIds.length) ||
+            if ((newSong.artistIds && artists.length !== newSong.artistIds.length) ||
+                (newSong.tagIds && tags.length !== newSong.tagIds.length) ||
+                (newSong.albumIds && albums.length !== newSong.albumIds.length) ||
                 !song) {
                 const e: EndpointError = {
                     internalMessage: 'Not all albums and/or artists and/or tags exist for ModifySong request: ' + JSON.stringify(req.body),
@@ -66,34 +73,34 @@ export const ModifySongEndpointHandler: EndpointHandler = async (req: any, res:
             const modifySongPromise = trx('songs')
                 .where({ 'id': req.params.id })
                 .update({
-                    title: reqObject.title,
-                    storeLinks: JSON.stringify(reqObject.storeLinks || []),
+                    title: newSong.title,
+                    storeLinks: JSON.stringify(newSong.storeLinks || []),
                 })
 
             // Remove unlinked artists.
             // TODO: test this!
-            const removeUnlinkedArtists = trx('artists_songs')
+            const removeUnlinkedArtists = artists ? trx('songs_artists')
                 .where({ 'songId': req.params.id })
-                .whereNotIn('artistId', reqObject.artistIds || [])
-                .delete();
+                .whereNotIn('artistId', newSong.artistIds || [])
+                .delete() : undefined;
 
             // Remove unlinked tags.
             // TODO: test this!
-            const removeUnlinkedTags = trx('songs_tags')
+            const removeUnlinkedTags = tags ? trx('songs_tags')
                 .where({ 'songId': req.params.id })
-                .whereNotIn('tagId', reqObject.tagIds || [])
-                .delete();
+                .whereNotIn('tagId', newSong.tagIds || [])
+                .delete() : undefined;
 
             // Remove unlinked albums.
             // TODO: test this!
-            const removeUnlinkedAlbums = trx('songs_albums')
+            const removeUnlinkedAlbums = albums ? trx('songs_albums')
                 .where({ 'songId': req.params.id })
-                .whereNotIn('albumId', reqObject.albumIds || [])
-                .delete();
+                .whereNotIn('albumId', newSong.albumIds || [])
+                .delete() : undefined;
 
             // Link new artists.
             // TODO: test this!
-            const addArtists = trx('artists_songs')
+            const addArtists = artists ? trx('songs_artists')
                 .where({ 'songId': req.params.id })
                 .then((as: any) => as.map((a: any) => a['artistId']))
                 .then((doneArtistIds: number[]) => {
@@ -111,14 +118,14 @@ export const ModifySongEndpointHandler: EndpointHandler = async (req: any, res:
                     // Link them
                     return Promise.all(
                         insertObjects.map((obj: any) =>
-                            trx('artists_songs').insert(obj)
+                            trx('songs_artists').insert(obj)
                         )
                     );
-                })
+                }) : undefined;
 
             // Link new tags.
             // TODO: test this!
-            const addTags = trx('songs_tags')
+            const addTags = tags ? trx('songs_tags')
                 .where({ 'songId': req.params.id })
                 .then((ts: any) => ts.map((t: any) => t['tagId']))
                 .then((doneTagIds: number[]) => {
@@ -139,11 +146,11 @@ export const ModifySongEndpointHandler: EndpointHandler = async (req: any, res:
                             trx('songs_tags').insert(obj)
                         )
                     );
-                })
+                }) : undefined;
 
             // Link new albums.
             // TODO: test this!
-            const addAlbums = trx('songs_albums')
+            const addAlbums = albums ? trx('songs_albums')
                 .where({ 'albumId': req.params.id })
                 .then((as: any) => as.map((a: any) => a['albumId']))
                 .then((doneAlbumIds: number[]) => {
@@ -164,7 +171,7 @@ export const ModifySongEndpointHandler: EndpointHandler = async (req: any, res:
                             trx('songs_albums').insert(obj)
                         )
                     );
-                })
+                }) : undefined;
 
             // Wait for all operations to finish.
             await Promise.all([