I created an API that integrate database responses in a promise flow, but I think the interpretation of the code is complex and I believe that async / await approach could improve both understanding and the code itself.
The API is built in NodeJS using mongoose 5.6.1 and express 4.17.1.
Can you help me in improve this?
Below is the API that I want to improve:
/** New employee */
router.post('/', (req, res) => {
let { idCompany, name, departament } = req.body;
let _id = mongoose.Types.ObjectId(); // Generating new MongoDB _ID
let employeeCreated;
const promise1 = new Promise((resolve, reject) => {
// Querying by document '$oid'
Companies.findOne({ _id: idCompany }, (err, company) => {
// Error returned
if (err) reject({ error: "Invalid request, something went wrong!" });
// Invalid data received
if (!company) reject({ error: "Unauthorized action!" });
// Everything OK
resolve(company);
});
})
.then(company => {
if(company) {
const promise2 = new Promise((resolve, reject) => {
Employees.create({ _id, idCompany, name, departament }, (err, employee) => {
// Error returned
if (err) reject({ error: "Invalid request, something went wrong!", err });
// Everything OK
employeeCreated = employee;
resolve(company);
});
})
return promise2;
}else reject({ error: "Company not found!" });
})
.then(company => {
let { name: companyName, address, email, tel, employees } = company;
employees.push(_id);
const promise3 = new Promise((resolve, reject) => {
Companies.findByIdAndUpdate(
{ _id: idCompany },
{ $set: { _id: idCompany, name: companyName, address, email, tel, employees } }, // spotlight
{ new: true },
(err, company) => {
// Something wrong happens
if (err) reject({ success: false, error: "Can't update company!" });
// Everything OK
resolve(company);
}
);
});
return promise3;
});
promise1
.then(() => res.json({ success: true, employeeCreated }))
.catch(err => res.status(400).json({ error: "Invalid request, something went wrong!", err }));
});
Regards.
One key to using promises with mongoose, is using the exec
method:
Your code could then look something like this (not tested):
router.post('/', async (req, res) => {
try {
const { idCompany, name, departament } = req.body;
const _id = mongoose.Types.ObjectId();
const company = await Companies.findOne({ _id: idCompany }).exec();
const employeeCreated = await Employees.create({ _id, idCompany, name, departament });
const { name: companyName, address, email, tel, employees } = company;
employees.push(_id);
await Companies.findByIdAndUpdate(
{ _id: idCompany },
{ $set: { _id: idCompany, name: companyName, address, email, tel, employees } }, // spotlight
{ new: true }).exec();
res.json({ success: true, employeeCreated });
} catch(err) {
res.status(400).json({ error: "Invalid request, something went wrong!", err });
}
});
You could throw some specific custom errors in the try
block if you find that necessary.
Thanks, it worked perfectly, I was not aware of the
exec
method.