Reputation: 75
I have two models that are related to each other. If a mongo document for the first model is successful deleted I have to delete its parent document.
An exception can be raised at the time of trying to execute the second deletion or not. With errors or without them I would like to concatenate the first and the second messages, but the scope of "message" variable inside the "findOneAndDelete" is different of its parent.
How to accomplish this requirement?
The code snippet below works fine with the exception of only the main message is delivered.
var id = req.params.id;
var valid = mongoose.Types.ObjectId.isValid(id);
if (valid) {
MyModel
.findByIdAndDelete({"_id": id})
.exec(function (err, result) {
var message = "Item has been removed successful.";
if (!result) {
message = "Error at deleting an about item.";
} else {
// finding and deleting related about image item
var query = { "id": id };
SecondModel
.findOneAndDelete(query).exec(function (err, result) {
// deletion is OK, but the concatenation does not work
// how to return this "message" scope to its parent?
message += " Image item has been removed successful.";
if (!result) {
message += " Error trying to remove about image item.";
}
});
}
// the concatenation should be returned here
responseUtilities.sendJsonResponse(res, err, {"message": message});
});
} else {
responseUtilities.sendJsonResponse(res, false, { "message": "Item id is not valid." });
}
Upvotes: 1
Views: 275
Reputation: 62676
Glad you found an answer. Here's a handful of structure and style ideas that will make this code easier to read (by you in the future or by a colleague) and make it easier to debug and extend. Start by building small, promise-returning functions for the two async operations.
function deleteAboutWithId(id) {
return new Promise((resolve, reject) => {
AboutModel.findByIdAndDelete({"_id": id}).exec(function (err, result) {
err ? reject(err) : resolve(result)
})
})
}
function deleteImageWithAboutId(id) {
return new Promise((resolve, reject) => {
AboutImageModel.findOneAndDelete({ aboutId: id }).exec(function (err, result) {
err ? reject(err) : resolve(result)
})
})
}
A nice benefit of these is that they can be unit tested, reused, generalized, etc. The other benefit is that using them makes your business logic is clear and concise. The code reads the way you'd describe the function to a colleague...
The public function:
const msgIdError = 'About item id is not valid.'
const msgItemSuccess = 'About item has been removed successful.'
const msgImageSuccess = ' Its image item has also been removed successful.'
const msgItemError = 'Error at deleting an about item.'
const msgImageError = ' Error trying to remove about image item.'
module.exports.delete = function (req, res) {
const idAbout = req.params.idAbout
if (!mongoose.Types.ObjectId.isValid(idAbout)) {
responseUtilities.sendJsonResponse(res, false, { "message": msgIdError })
}
let message = ''
deleteAboutWithId(idAbout).then(() => {
message += msgItemSuccess
return deleteImageWithAboutId(idAbout)
}).then(() => {
message += msgImageSuccess
responseUtilities.sendJsonResponse(res, err, { "message": message })
}).catch(err => {
// if message is empty, the first block threw the error
message += message.length ? msgImageError : msgItemError
responseUtilities.sendJsonResponse(res, err, { "message": message });
})
}
A couple quick notes: (1) it's nice to get the strings out of the way. Those might someday be kept in config, translated to other languages, etc. (2) It cleans up the code a little to check for invalid inputs at the top and bail out at the top if the inputs are invalid.
Upvotes: 1
Reputation: 38982
Query.exec
receives a callback that is processed later.
In your outer Query.exec
call when there is no results returned, you send a response. Otherwise, you perform another Query.exec
that processes another callback which returns no response.
A quick fix would be to send a response in the inner Query.exec
.
This is a sequence of operations is performed in a chain order and can be better coordinated using promises. For example,
const message = (prefix='') => msg => `${prefix} ${msg}`
const asyncChain = (outer, inner) => {
return outer
.then(message())
.then(outerResult => inner
.then(message(outerResult))
.catch(message(outerResult))
)
.catch(message())
.then(console.log);
}
asyncChain(Promise.resolve('outer1✅'), Promise.resolve('inner1✅'))
// outer1✅ inner1✅
asyncChain(Promise.resolve('outer2✅'), Promise.reject('inner2❌'))
// outer2✅ inner2❌
asyncChain(Promise.reject('outer3❌'), Promise.resolve('inner3✅'))
// outer3❌
Taking knowledge that Query.exec
returns a Promise
object, one can coordinate the query operations as follows:
MyModel.findByIdAndDelete({ _id: id })
.exec()
.then(function() {
return "Item has been removed successful.";
})
.then(function(deleteItemSuccessResult) {
return SecondModel.findOneAndDelete(query)
.exec()
.then(function() {
return (
deleteItemSuccessResult + " Image item has been removed successful."
);
})
.catch(function() {
return (
deleteItemSuccessResult + " Error trying to remove about image item."
);
});
})
.catch(function() {
return "Error trying to remove item";
})
.then(function(message) {
responseUtilities.sendJsonResponse(res, err, { message: message });
});
Upvotes: 0
Reputation: 75
I have found myself a solution to this requirement but I am not sure that the approach that I followed is the best way to do so. So, I have implemented a Promise and I call an outside function, then I am sure that it has finished. Thus, I can now concatenate the message. Maybe there may be a more elegant version of this solution. If anyone could review I wonder.
module.exports.delete = function (req, res) {
var idAbout = req.params.idAbout;
var valid = mongoose.Types.ObjectId.isValid(idAbout);
if (valid) {
var message;
AboutModel
.findByIdAndDelete({"_id": idAbout})
.exec(function (err, result) {
message = "About item has been removed successful.";
if (!result) {
message = "Error at deleting an about item.";
} else {
new Promise((resolve, reject) => {
deleteImage(idAbout);
resolve();
}).then(() => {
message += " Its image item has also been removed successful.";
responseUtilities.sendJsonResponse(res, err, { "message": message });
}).catch(
error => {
message += " Error trying to remove about image item. ";
message += error.message;
responseUtilities.sendJsonResponse(res, err, { "message": message });
}
);
}
});
} else {
responseUtilities.sendJsonResponse(res, false, { "message": "About item id is not valid." });
}
};
// outside function
function deleteImage(idAbout) {
var query = { "aboutId": idAbout };
AboutImageModel.findOneAndDelete(query).exec(function (err, result) {});
}
Upvotes: 0