From 0867b142da2d5207ef53e682a5bf120ea3850e57 Mon Sep 17 00:00:00 2001 From: Charlie Humphreys Date: Thu, 27 Oct 2022 21:44:26 -0500 Subject: [PATCH 1/7] update getBrew usages to not fetch google brew during updates --- server/homebrew.api.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index 293e8f873..586466364 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -27,7 +27,7 @@ const getId = (req)=>{ return { id, googleId }; }; -const getBrew = (accessType)=>{ +const getBrew = (accessType, fetchGoogle = true)=>{ // Create middleware with the accessType passed in as part of the scope return async (req, res, next)=>{ // Get relevant IDs for the brew @@ -45,7 +45,7 @@ const getBrew = (accessType)=>{ stub = stub?.toObject(); // If there is a google id, try to find the google brew - if(googleId || stub?.googleId) { + if(fetchGoogle && (googleId || stub?.googleId)) { let googleError; const googleBrew = await GoogleActions.getGoogleBrew(googleId || stub?.googleId, id, accessType) .catch((err)=>{ @@ -327,8 +327,8 @@ const deleteBrew = async (req, res, next)=>{ }; router.post('/api', asyncHandler(newBrew)); -router.put('/api/:id', asyncHandler(getBrew('edit')), asyncHandler(updateBrew)); -router.put('/api/update/:id', asyncHandler(getBrew('edit')), asyncHandler(updateBrew)); +router.put('/api/:id', asyncHandler(getBrew('edit', false)), asyncHandler(updateBrew)); +router.put('/api/update/:id', asyncHandler(getBrew('edit', false)), asyncHandler(updateBrew)); router.delete('/api/:id', asyncHandler(deleteBrew)); router.get('/api/remove/:id', asyncHandler(deleteBrew)); From 2c6779bb1c893b910c47ed76c0d5b42b64786a4a Mon Sep 17 00:00:00 2001 From: Charlie Humphreys Date: Wed, 16 Nov 2022 22:28:00 -0600 Subject: [PATCH 2/7] adjust getBrew to check authorship, change update method to not perform a get --- server/homebrew.api.js | 44 +++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index 586466364..44b604bbc 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -43,6 +43,9 @@ const getBrew = (accessType, fetchGoogle = true)=>{ } }); stub = stub?.toObject(); + if(stub?.authors && !stub?.authors.includes(req.account.username)) { + throw 'Current logged in user does not have access to this brew.'; + } // If there is a google id, try to find the google brew if(fetchGoogle && (googleId || stub?.googleId)) { @@ -59,14 +62,14 @@ const getBrew = (accessType, fetchGoogle = true)=>{ } // If after all of that we still don't have a brew, throw an exception - if(!stub) { + if(!stub && fetchGoogle) { throw 'Brew not found in Homebrewery database or Google Drive'; } - if(typeof stub.tags === 'string') { + if(typeof stub?.tags === 'string') { stub.tags = []; } - req.brew = stub; + req.brew = stub || {}; next(); }; @@ -234,30 +237,31 @@ const updateBrew = async (req, res)=>{ if(req.account) { brew.authors = _.uniq(_.concat(brew.authors, req.account.username)); } - - // Fetch the brew from the database again (if it existed there to begin with), and assign the existing brew to it - brew = _.assign(await HomebrewModel.findOne({ _id: brew._id }), brew); - - if(!brew.markModified) { - // If it wasn't in the database, create a new db brew - brew = new HomebrewModel(brew); + // we need the tag type change in both getBrew and here to handle the case where we don't have a stub on which to perform the modification + if(typeof brew.tags === 'string') { + brew.tags = []; } - brew.markModified('authors'); - brew.markModified('systems'); - - // Save the database brew - const saved = await brew.save() - .catch((err)=>{ - console.error(err); - res.status(err.status || 500).send(err.message || 'Unable to save brew to Homebrewery database'); - }); + // define a function to catch our save errors + const saveError = (err)=>{ + console.error(err); + res.status(err.status || 500).send(err.message || 'Unable to save brew to Homebrewery database'); + }; + let saved; + if(!brew._id) { + // if the brew does not have a stub id, create and save it, then write the new value back to the brew. + saved = await new HomebrewModel(brew).save().catch(saveError); + brew = saved.toObject(); + } else { + // if the brew does have a stub id, update it using the stub id as the key. + saved = await HomebrewModel.updateOne({ _id: brew._id }, brew).catch(saveError); + } if(!saved) return; // Call and wait for afterSave to complete const after = await afterSave(); if(!after) return; - res.status(200).send(saved); + res.status(200).send(brew); }; const deleteGoogleBrew = async (account, id, editId, res)=>{ From f9711de634b6af758a8e18a003e2eab4058b63fb Mon Sep 17 00:00:00 2001 From: Charlie Humphreys Date: Wed, 16 Nov 2022 22:32:50 -0600 Subject: [PATCH 3/7] add elvis for the possibility that the save failed --- server/homebrew.api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index 44b604bbc..563926f01 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -251,7 +251,7 @@ const updateBrew = async (req, res)=>{ if(!brew._id) { // if the brew does not have a stub id, create and save it, then write the new value back to the brew. saved = await new HomebrewModel(brew).save().catch(saveError); - brew = saved.toObject(); + brew = saved?.toObject(); } else { // if the brew does have a stub id, update it using the stub id as the key. saved = await HomebrewModel.updateOne({ _id: brew._id }, brew).catch(saveError); From 2e305d56361398422a8044bf2ca642e6822ebf50 Mon Sep 17 00:00:00 2001 From: Charlie Humphreys Date: Wed, 16 Nov 2022 22:37:59 -0600 Subject: [PATCH 4/7] remove authorship piece from this PR --- server/homebrew.api.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index 563926f01..ec8e438fa 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -43,9 +43,6 @@ const getBrew = (accessType, fetchGoogle = true)=>{ } }); stub = stub?.toObject(); - if(stub?.authors && !stub?.authors.includes(req.account.username)) { - throw 'Current logged in user does not have access to this brew.'; - } // If there is a google id, try to find the google brew if(fetchGoogle && (googleId || stub?.googleId)) { From f26e3d6cd1a53f6075f3835f44204c2a340d7c62 Mon Sep 17 00:00:00 2001 From: Charlie Humphreys Date: Fri, 18 Nov 2022 17:53:47 -0600 Subject: [PATCH 5/7] remove tags from google brew fetch --- server/googleActions.js | 1 - server/homebrew.api.js | 4 ---- 2 files changed, 5 deletions(-) diff --git a/server/googleActions.js b/server/googleActions.js index 4ccf7a1dd..8a11a73ac 100644 --- a/server/googleActions.js +++ b/server/googleActions.js @@ -249,7 +249,6 @@ const GoogleActions = { text : file.data, description : obj.data.description, - tags : obj.data.properties.tags ? obj.data.properties.tags : '', systems : obj.data.properties.systems ? obj.data.properties.systems.split(',') : [], authors : [], published : obj.data.properties.published ? obj.data.properties.published == 'true' : false, diff --git a/server/homebrew.api.js b/server/homebrew.api.js index ec8e438fa..484ca8bb5 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -234,10 +234,6 @@ const updateBrew = async (req, res)=>{ if(req.account) { brew.authors = _.uniq(_.concat(brew.authors, req.account.username)); } - // we need the tag type change in both getBrew and here to handle the case where we don't have a stub on which to perform the modification - if(typeof brew.tags === 'string') { - brew.tags = []; - } // define a function to catch our save errors const saveError = (err)=>{ From fec1766e269925a06f5e3761c02307d2cd179653 Mon Sep 17 00:00:00 2001 From: Charlie Humphreys Date: Mon, 21 Nov 2022 16:21:36 -0600 Subject: [PATCH 6/7] switch fetchGoogle to stubOnly --- server/homebrew.api.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index 484ca8bb5..b46bbd1ab 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -27,7 +27,7 @@ const getId = (req)=>{ return { id, googleId }; }; -const getBrew = (accessType, fetchGoogle = true)=>{ +const getBrew = (accessType, stubOnly = false)=>{ // Create middleware with the accessType passed in as part of the scope return async (req, res, next)=>{ // Get relevant IDs for the brew @@ -45,7 +45,7 @@ const getBrew = (accessType, fetchGoogle = true)=>{ stub = stub?.toObject(); // If there is a google id, try to find the google brew - if(fetchGoogle && (googleId || stub?.googleId)) { + if(!stubOnly && (googleId || stub?.googleId)) { let googleError; const googleBrew = await GoogleActions.getGoogleBrew(googleId || stub?.googleId, id, accessType) .catch((err)=>{ @@ -59,7 +59,7 @@ const getBrew = (accessType, fetchGoogle = true)=>{ } // If after all of that we still don't have a brew, throw an exception - if(!stub && fetchGoogle) { + if(!stub && !stubOnly) { throw 'Brew not found in Homebrewery database or Google Drive'; } @@ -324,8 +324,8 @@ const deleteBrew = async (req, res, next)=>{ }; router.post('/api', asyncHandler(newBrew)); -router.put('/api/:id', asyncHandler(getBrew('edit', false)), asyncHandler(updateBrew)); -router.put('/api/update/:id', asyncHandler(getBrew('edit', false)), asyncHandler(updateBrew)); +router.put('/api/:id', asyncHandler(getBrew('edit', true)), asyncHandler(updateBrew)); +router.put('/api/update/:id', asyncHandler(getBrew('edit', true)), asyncHandler(updateBrew)); router.delete('/api/:id', asyncHandler(deleteBrew)); router.get('/api/remove/:id', asyncHandler(deleteBrew)); From 99019be152304f7c4ea49e119bb326ff9fb06ca7 Mon Sep 17 00:00:00 2001 From: Charlie Humphreys Date: Mon, 5 Dec 2022 21:00:44 -0600 Subject: [PATCH 7/7] switch updateOne to findOneAndUpdate and return the saved instead of the brew passed in --- server/homebrew.api.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/homebrew.api.js b/server/homebrew.api.js index b46bbd1ab..57b009b8c 100644 --- a/server/homebrew.api.js +++ b/server/homebrew.api.js @@ -247,14 +247,16 @@ const updateBrew = async (req, res)=>{ brew = saved?.toObject(); } else { // if the brew does have a stub id, update it using the stub id as the key. - saved = await HomebrewModel.updateOne({ _id: brew._id }, brew).catch(saveError); + saved = await HomebrewModel.findOneAndUpdate({ _id: brew._id }, brew, { + returnOriginal : false + }).catch(saveError); } if(!saved) return; // Call and wait for afterSave to complete const after = await afterSave(); if(!after) return; - res.status(200).send(brew); + res.status(200).send(saved); }; const deleteGoogleBrew = async (account, id, editId, res)=>{