Ahmad Alhaj-Karim
Ahmad Alhaj-Karim

Reputation: 194

Calling Subscribe inside Foreach Angular

I've a survey which contains list of questions, where each question contains list of multiple choice answer. I'm trying to push this to my backend, but it's pushing ONLY the first question of the survey according to my database. I made sure that the questions and the answers are inserted correctly and also i tested the mutation using Graphql

this._surveyService.addSurveyToInitiative(this._initiativeId, this.name, this.category, "TBD").subscribe(surveyData => {
     this.survey.questions.forEach(question => {
        this._surveyService.addQuestionToSurvey(surveyData.data.createSurvey.survey.id, question.question).subscribe(questionData=> {
          question.multipleChoiceAnswers.forEach(answer => {
            this._surveyService.addMultipleChoiceAnswerToQuestion(questionData.data.addQuestionToSurvey.newQuestion.id, answer).subscribe()
          })
        })
     })
   })
}

Upvotes: 0

Views: 626

Answers (2)

Mrk Sef
Mrk Sef

Reputation: 8022

From the first blush, the problem doesn't appear to be RxJS or subscribe. Something about your service calls is likely the culprit. That being said, it's hard to debug.

Nesting subscribes and/or calling subscribe in a loop is basically always an antipattern. You're mixing imperative and functional programming styles with abandon - that's not to say you can't make it work but that teasing apart what is going wrong (not to mention maintaining/extending this in the future) becomes more challenging.

First lets rewrite this more functionally:

this._surveyService.addSurveyToInitiative(
  this._initiativeId, 
  this.name, 
  this.category, 
  "TBD"
).pipe(
  map(surveyData => 
    this.survey.question.map(question =>
      this._surveyService.addQuestionToSurvey(
        surveyData.data.createSurvey.survey.id, 
        question.question
      ).pipe(
        map(questionData => ({ question, questionData }))
      )
    )
  ),
  mergeMap(serviceCalls => merge(...serviceCalls)),
  map(({ question, questionData }) => 
    question.multipleChoiceAnswers.map(answer => 
      this._surveyService.addMultipleChoiceAnswerToQuestion(
        questionData.data.addQuestionToSurvey.newQuestion.id, 
        answer
      )
    )
  ),
  mergeMap(serviceCalls => merge(...serviceCalls))
).subscribe();

Here, this line: mergeMap(serviceCalls => merge(...serviceCalls)) (Seen twice) is effectively how you subscribe to all your calls.

This should be much easier to debug. Now you can tap any part of your stream to see what's happening at that moment.

this._surveyService.addSurveyToInitiative(
  this._initiativeId, 
  this.name, 
  this.category, 
  "TBD"
).pipe(
  tap(surveyData => console.log("Processing surveryData: ", surveyData)),
  map(surveyData => 
    this.survey.question.map(question =>
      this._surveyService.addQuestionToSurvey(
        surveyData.data.createSurvey.survey.id, 
        question.question
      ).pipe(
        map(questionData => ({ question, questionData }))
      )
    )
  ),
  mergeMap(serviceCalls => merge(...serviceCalls)),
  tap(questionAndData => console.log("Processing questionAndData: ", questionAndData)),
  map(({ question, questionData }) => 
    question.multipleChoiceAnswers.map(answer => 
      this._surveyService.addMultipleChoiceAnswerToQuestion(
        questionData.data.addQuestionToSurvey.newQuestion.id, 
        answer
      )
    )
  ),
  mergeMap(serviceCalls => merge(...serviceCalls))
).subscribe(x => console.log("done processing final step, got reponse: ", x));

Now you can delay certain calls, or use concat(... instead of merge(... to get a feel for what your stream is doing.

Aside:

Here is the same thing, only a bit more concise (We combine our merge calls):

this._surveyService.addSurveyToInitiative(
  this._initiativeId, 
  this.name, 
  this.category, 
  "TBD"
).pipe(
  mergeMap(surveyData => 
    merge(...this.survey.question.map(question =>
      this._surveyService.addQuestionToSurvey(
        surveyData.data.createSurvey.survey.id, 
        question.question
      ).pipe(
        map(questionData => ({ question, questionData }))
      )
    ))
  ),
  mergeMap(({ question, questionData }) => 
    merge(...question.multipleChoiceAnswers.map(answer => 
      this._surveyService.addMultipleChoiceAnswerToQuestion(
        questionData.data.addQuestionToSurvey.newQuestion.id, 
        answer
      )
    ))
  )
).subscribe();

Update # 1

Are you sure

this.survey.question.map(question =>

is right? Seems like a strange naming convention. I would expect something more akin to

this.survey.questions.map(question =>

Upvotes: 1

Tombery
Tombery

Reputation: 296

You should not chain subscriptions. Also forEach is a final operation, which means that nothing is returned from it. You should use map instead.

A better way would be to use mergeMap to get something like follow:

this._surveyService.addSurveyToInitiative(this._initiativeId, this.name, this.category, "TBD")
    .pipe(
        map(surveyData => surveyData.data.createSurvey.survey.id),
        mergeMap(surveyId => this.survey.questions.map(question => this._surveyService.addQuestionToSurvey(surveyId, question.question))),
        map(questionData => questionData.data.addQuestionToSurvey.newQuestion.id),
        mergeMap(questionId => question.multipleChoiceAnswers.forEach(answer => this._surveyService.addMultipleChoiceAnswerToQuestion(questionId, answer)))
    ).subscribe();

Upvotes: 0

Related Questions