0

I'm trying to do upsert inside forEach becase the request is an array of objects using Sequelize.upsert method. I do this:

async createInfo (req, res){
   newsData = req.body.news,
   newsList = [];

    newsData.forEach(async values => {
        var news = {};
        news.inserted_id = values.inserted_id;
        if(news.inserted_id == null){
            do {
                news.inserted_id = crypto.getRandom(5);
                var check = await InstitutionNews.count({where: {inserted_id: news.inserted_id}});
            } while (check > 0);
        }
        News.upsert({
            institution_id: institution_id,
            inserted_id: news.inserted_id,
            news_date: values.news_date,
            news_title: values.news_title,
            description: values.news_description,
            created_by: created_by
        }).then(ResNews => {
            news.news_date = values.news_date;
            news.news_title = values.news_title;
            news.description = values.news_description;
            newsList.push(news);
        })
    })

    console.log("TEST")
}

but the process stop at the then(). It didn't execute the next code like the console.log.

Is there any way to execute next line code after the then(). I need then() because I wanna push the news object into newsList array. Because I need newsList as the if else conditional to do the next process.

Thanks.

Blackjack
  • 1,016
  • 1
  • 20
  • 51
  • 1
    You're saying you need to check `newsList` on each iteration to determine whether you should continue? Not sure I completely understand – goto Aug 18 '19 at 17:23
  • I believe your `console.log` is logging immediately because your foreach has an async iterator. – Maddy Blacklisted Aug 18 '19 at 17:25
  • @goto1 no, I check the `newsList` out the forEach, so I do the check after the forEach has finish. – Blackjack Aug 18 '19 at 17:27
  • Or, do you need to do other work **after** `forEach` based on the results that you've got? – goto Aug 18 '19 at 17:27
  • @Blackjack ok, so you need to wait for `forEach` to complete before `console.log` gets executed, and I getting this right? – goto Aug 18 '19 at 17:28
  • @goto1 yes, you're right. I do the console after `forEach` complete – Blackjack Aug 18 '19 at 17:29
  • @MaddyBlacklisted Yes it logging immediately if I remove the `then()`, but right now it didn't. Because I use `then()`, and the code stop execute at `then()` – Blackjack Aug 18 '19 at 17:30
  • No, you don't need to use `then`. You should rather use `await` to wait for the result of the `News.upsert` call. – Bergi Aug 18 '19 at 17:52
  • I don't think that's the real issue. The problem is that he wants to do something with `newsList` **after** `forEach` finishes executing and all async calls are "done" – goto Aug 18 '19 at 17:56
  • @Bergi I need to use then because I wanna add the object to the array if the `upsert` is success – Blackjack Aug 18 '19 at 18:17
  • @goto1 Yes, That's right – Blackjack Aug 18 '19 at 18:17
  • @Blackjack No, you should try to achieve that with `await` instead of `then`, to avoid mixing promise syntax styles. Btw, you probably shouldn't even `push` to a `newsList`, you should just `return` the `news` item so that the `Promise.all` (as outlined in the duplicate or in goto1's answer) does fulfill with the array you are looking for. – Bergi Aug 18 '19 at 18:21
  • 1
    @Bergi that's a good point, I've edited my answer to show your approach – goto Aug 18 '19 at 18:39
  • @Bergi I didn't mean I should using `then`, what I mean I need the process inside `then`. That's my bad, I didn't explain well in the question. Sorry. – Blackjack Aug 18 '19 at 19:22
  • @Bergi thankyou for the detail explanation. – Blackjack Aug 18 '19 at 19:24
  • @goto1 thanks, that's exactly how it should look like :-) – Bergi Aug 18 '19 at 19:30

2 Answers2

2

Since it sounds like you need to wait for forEach to complete before you do another step, so I'd suggest using something like Promise.all:

async function createInfo(req, res) {
  const newsData = req.body.news
  const newsList = []

  try {
    await Promise.all(
      newsData.map(
        async (values) => {
          const news = {}
          news.inserted_id = values.inserted_id

          // ...

          News.upsert({...})
            .then(ResNews => {
              // ...
              newsList.push(news)
            })
        }
      )
    )
    console.log('newsList', newList)
    // do other work
  } catch (error) {
    // handle errors appropriately
  }
}

This way, you're creating an array of promises and waiting for all of them to resolve/finish.

Your current approach with forEach won't work in this case since it won't wait for each asynchronous call to finish before executing the "next" step. Since Promise.all returns a single Promise that you can then "wait" for to resolve before continuing with your next step.

Here's a simple example that somewhat does what you're trying to do:

async function createInfo() {
  const newsData = [1, 2, 3, 4, 5]
  const newsList = []

  await Promise.all(
    newsData.map(
      async (values) => {
        const temp = await Promise.resolve('temp')
        console.log('first async call inside values of', values)

        Promise.resolve('resolved')
          .then((result) => {
            newsList.push(`resolved with ${values}`)
          })
      }
    )
  )
  console.log('newsList after')
  console.log(newsList)
}

createInfo()

EDIT

Here's an alternative solution as rightly pointed by @Bergi in the comments:

async function createInfo(req, res) {
  const newsData = req.body.news

  try {
    const newList = await Promise.all(
      newsData.map(
        async (values) => {
          const news = {}

          news.inserted_id = values.inserted_id

          // ...

          // since it doesn't look like you're using any
          // data that you'd get back from the `News.upsert` call, wait for
          // it to finish and just simply return your `news` object

          await News.upsert({...})

          news.news_date = values.news_date
          news.news_title = values.news_title
          news.description = values.news_description

          return news
        }
      )
    )
    console.log('newsList', newList)
    // do other work
  } catch (error) {
    // handle errors appropriately
  }
}
goto
  • 4,336
  • 15
  • 20
  • Thankyou, yours work well for me. – Blackjack Aug 18 '19 at 19:17
  • Btw what the variable `temp` do? and in `Promise.resolve('resolved')`, where does the `resolved` come? – Blackjack Aug 18 '19 at 19:20
  • It doesn't really do anything in this example. This was just to show you a "similar" setup (in terms of asynchronous calls) to what you have without using your actual example. So you can imagine `temp` in this case would be the `check` variable that you create and then `Promise.resolve('resolved')` would be the result of the `News.upsert` call. Since both calls are promises, I used `Promise.resolve` to mimic a similar behavior – goto Aug 18 '19 at 19:25
0

use await instead of then like

    await News.upsert({
        institution_id: institution_id,
        inserted_id: news.inserted_id,
        news_date: values.news_date,
        news_title: values.news_title,
        description: values.news_description,
        created_by: created_by
    });
    news.news_date = values.news_date;
    news.news_title = values.news_title;
    news.description = values.news_description;
    newsList.push(news);
Maddy Blacklisted
  • 1,190
  • 1
  • 7
  • 17