Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,22 @@ public static Question withAskTimeAccountQId(final Question question,
created, question.category, question.dependencyResponse);
}

public static Question withAccountQId(final Question question, final Long accountQuestionId) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest withAccountQuestionId instead of withAccountQId, we're not saving that many characters and it's more readable.

return new Question(question.id,
accountQuestionId,
question.text,
question.lang,
question.type,
question.frequency,
question.askTime,
question.dependency,
question.parentId,
question.askLocalDate,
question.choiceList,
question.accountInfo,
question.accountCreationDate,
question.category,
question.dependencyResponse);
}

}
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package com.hello.suripu.core.processors;

import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.hello.suripu.core.db.QuestionResponseDAO;
import com.hello.suripu.core.db.QuestionResponseReadDAO;
import com.hello.suripu.core.db.util.MatcherPatternsDB;
import com.hello.suripu.core.models.Account;
import com.hello.suripu.core.models.AccountQuestion;
import com.hello.suripu.core.models.Question;
import com.hello.suripu.core.models.Questions.QuestionCategory;
Expand All @@ -12,10 +15,12 @@
import com.hello.suripu.core.util.QuestionUtils;
import org.apache.commons.collections.ListUtils;
import org.joda.time.DateTime;
import org.skife.jdbi.v2.exceptions.UnableToExecuteStatementException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.List;
import java.util.regex.Matcher;

import static com.google.common.base.Preconditions.checkNotNull;

Expand Down Expand Up @@ -106,30 +111,62 @@ public List<Question> getSurveyQuestions(final Long accountId, final DateTime to
return Lists.newArrayList();
}

//Returns and saves 1st available question.
//Outputs 1st available question
final List<Question> outputSurveyQuestions = availableQuestions.subList(0, 1);
final DateTime expiration = todayLocal.plusDays(1);
return processQuestions(outputSurveyQuestions, accountId, todayLocal, expiration);

}

/*
Saves question to accountQuestions if not already in database
Return question with the accountQuestions Id from database
*/
private List<Question> processQuestions(final List<Question> questions, final Long accountId, final DateTime todayLocal, final DateTime expireDate) {

final List<Question> processedQuestions = Lists.newArrayList();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this failing before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it should not have failed before. In current prod version, there is a check if question already exists in db prior to attempting saveQuestions to prevent throwing duplicate exception. The below change is just in case.

//Check if database already has question with unique index on (account_id, question_id, created_local_utc_ts)
final Boolean savedQuestion = savedAccountQuestion(accountId, availableQuestions.get(0), todayLocal);
if (savedQuestion) {
return availableQuestions.subList(0, 1);
final Optional<AccountQuestion> savedQuestion = getSavedAccountQuestionOptional(accountId, questions.get(0), todayLocal);
if (savedQuestion.isPresent()) {
final Long savedAccountQId = savedQuestion.get().id;
final Question processedQuestion = Question.withAccountQId(questions.get(0), savedAccountQId);
processedQuestions.add(processedQuestion);
}
else { //Save question and reform with unique index
final Long accountQId = saveQuestion(accountId, questions.get(0), todayLocal, expireDate);
final Question processedQuestion = Question.withAccountQId(questions.get(0), accountQId);
processedQuestions.add(processedQuestion);
}

saveQuestion(accountId, availableQuestions.subList(0, 1).get(0), todayLocal, expiration);
return availableQuestions.subList(0, 1);
return processedQuestions;
}

/*
Insert questions
*/

private void saveQuestion(final Long accountId, final Question question, final DateTime todayLocal, final DateTime expireDate) {
LOGGER.debug("action=saved_question processor=question_survey account_id={} question_id={} today_local={} expire_date={}", accountId, question.id, todayLocal, expireDate);
this.questionResponseDAO.insertAccountQuestion(accountId, question.id, todayLocal, expireDate);
private Long saveQuestion(final Long accountId, final Question question, final DateTime todayLocal, final DateTime expireDate) {
try {
LOGGER.debug("action=saving_question processor=question_survey account_id={} question_id={} today_local={} expire_date={}", accountId, question.id, todayLocal, expireDate);
return this.questionResponseDAO.insertAccountQuestion(accountId, question.id, todayLocal, expireDate);

} catch (UnableToExecuteStatementException exception) {
final Matcher matcher = MatcherPatternsDB.PG_UNIQ_PATTERN.matcher(exception.getMessage());
if (matcher.find()) {
LOGGER.debug("action=not_saved_question reason=unable_to_execute_sql processor=question_survey account_id={} question_id={}", accountId, question.id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad debug message. PG_UNIQ_PATTERN means constraint violation, so debug message should indicate that it's a duplicate.
You should log if it's not a duplicate.

}
}
return 0L;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returning 0L ?????

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

private Boolean savedAccountQuestion(final Long accountId, final Question question, final DateTime created) {
final List<AccountQuestion> questions = questionResponseReadDAO.getAskedQuestionByQuestionIdCreatedDate(accountId, question.id, created);
return !questions.isEmpty();
/*
Get question from database. For purpose of 1. determine if question already exists in db and 2. if true, extract db's accountQuestions Id
*/
private Optional<AccountQuestion> getSavedAccountQuestionOptional(final Long accountId, final Question question, final DateTime created) {
final List<AccountQuestion> accountQuestions = questionResponseReadDAO.getAskedQuestionByQuestionIdCreatedDate(accountId, question.id, created);
if (accountQuestions.isEmpty()) {
return Optional.absent();
}
return Optional.of(accountQuestions.get(0));
}
}