Evan G
Evan G

Reputation: 13

Async Promise chaining with a loop in Node.js & MongoDB

I recently imported my Database from Sql Server to MongoDB, converting to a full MERN stack. I have a titles_table, a skills_table, and a title_skills_table. The title_skills_table is a 'join' table that holds the _ids of the skills and title tables as some skills can fall under multiple titles.

I'm trying to call all title_skills associated with a particular title, which I've been able to do. What I can't figure out is how to use that title_skills array response to find all the skills by skill_id in the skills_table.

I've tried async waterfall, async chaining, among other things, but it's either just returning an empty error message or breaking the server.

I can do the two calls individually and get a response from each, it's joining them in a looping chain that seems to kill me.

Here's what I currently have:

const router = require('express').Router();
const Skill = require('../models/Skill');
const TitleSkills = require('../models/TitleSkills');

//Get A Skill by Skill Id
router.get("/skill/:id", async (req, res) => {
  try {
    const skill = await Skill.findOne({skill_id: req.params.id}).populate('SkillCategory')
    res.json(skill);
  } catch (err) {
    res.json({ message: err });
  }
});

//Get TitleSkills by TitleId *by itself
// router.get("/title/:id", async (req, res) => {
//   try {
//     const titleSkills = await TitleSkills.find({title_id: req.params.id})
//     res.json(titleSkills)
//   } catch (err) {
//     res.json({ message: err });
//   }
// });

//Get a skill by skill id and through it in an array
const getSkillsByskillId = (id) => new Promise((resolve, reject) => {
  const skillsArr = []
  router.get(`/skill/${id}`)
    .then((result) => resolve(skillsArr.push(result.data)))
    return(skillsArr)
    .catch(error => reject(error))
});

//Get all TitleSkills by TitleId and loop each TitleSkill through Skills Table
router.get("/title/:id", async (req, res) => {
  try {
    const titleSkills = await TitleSkills.find({title_id: req.params.id})
      .then(titleSkills.forEach((ts) => {
       getSkillsByskillId(ts.skill_id)
      }))
    res.json(titleSkills)
  } catch (err) {
    res.json({ message: err });
  }
});

module.exports = router;

#saveme

Upvotes: 1

Views: 319

Answers (3)

terrymorse
terrymorse

Reputation: 7086

This promise chain is ill-formed:

//Get a skill by skill id and through it in an array
const getSkillsByskillId = (id) => new Promise((resolve, reject) => {
  const skillsArr = [];
  router.get(`/skill/${id}`)
    .then((result) => resolve(skillsArr.push(result.data)))
  return (skillsArr)
    .catch(error => reject(error)); // <- never executed
});

That return (skillsArr) executes immediately after router.get is called, returning an empty array.

This modified code returns a promise that resolves to skillsArr (there's no need to include a catch block that doesn't do anything besides reject):

//Get a skill by skill id and return it in an array
const getSkillsByskillId = (id) => {
  const skillsArr = [];
  return router.get(`/skill/${id}`)
    .then((result) => {
      skillsArr.push(result.data);
      return skillsArr;
    });
};

Also, this call to getSkillsByskillId ignores the resolve:

    getSkillsByskillId(ts.skill_id)
   }))

This probably ought to be something like this:

getSkillsByskillId(ts.skill_id)
  .then(skillsArr => doSomething() );

Finally, this block is combining await and Promise chain unnecessarily.

const titleSkills = await TitleSkills.find({title_id: req.params.id})
  .then(titleSkills.forEach((ts) => {
    getSkillsByskillId(ts.skill_id);
  }))

This is equivalent:

const titleSkills = await TitleSkills.find({title_id: req.params.id});
for (let ts of titleSkills) {
  const skillsArr = await getSkillsByskillId(ts.skill_id);
  // do something with skillsArr
});

Upvotes: 1

danh
danh

Reputation: 62686

This is equivalent to the OP function of the same name...

const getSkillsByskillId = async (id) => {
  const result = await Skill.findOne({skill_id: req.params.id}).populate('SkillCategory')
  return result.data;  // removed OP's extra wrap of the array
}

Call it from the route, by mapping over it, collecting promises, and running them together with Promise.all()...

router.get("/title/:id", async (req, res) => {
  try {
    const skills = await TitleSkills.find({title_id: req.params.id})
    let promises = skills.map(ts => getSkillsByskillId(ts.skill_id));
    let titleSkills = await Promise.all(promises);
    res.json(titleSkills)
  } catch (err) {
    res.json({ message: err });
  }
});

Upvotes: 1

Evan G
Evan G

Reputation: 13

//Get A skill by skill_id
const getSkillsByskillId = async (id) => {
  const result = await Skill.findOne({skill_id: id}).populate('SkillCategory')
  return result;
}


//Get all TitleSkills by TitleId and then Map each TitleSkill to Skills_Table
router.get("/title/:id", async (req, res) => {
  try {
    const titleSkills = await TitleSkills.find({title_id: req.params.id})
    let promises = titleSkills.map(ts => getSkillsByskillId(ts.skill_id));
    let skills = await Promise.all(promises);
    res.json(skills)
  } catch (err) {
    res.json({ message: err });
  }
});

Upvotes: 0

Related Questions