Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap in a transaction operations a respondents list #2376

Merged
merged 2 commits into from
Dec 6, 2024
Merged
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
4 changes: 4 additions & 0 deletions assets/js/actions/respondentGroups.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ const handleRespondentGroupUpload = (dispatch, promise, groupId = null) => {
} else {
dispatch(receiveInvalids(value))
}
},
() => {
// Reject with original error
Promise.reject(e)
Copy link
Member

Choose a reason for hiding this comment

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

The error we get doing this is JSON.parse: unexpected character at line 1 column 1 of the JSON data, which isn't really useful for the user. Without this block, we get an equally useless Unhandled promise rejection error.

I'd keep this, anyways, so we're closer to something useful. We should eventually change the endpoint to reply a JSON error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my tests the JSON error didn't bubble up high enough, it logged on the browser console but no feedback to the user. The outer error did show the popup with not much information. It could have been replaced by new Error("some message"), still not very friendly.

})
}
)
Expand Down
123 changes: 72 additions & 51 deletions lib/ask/runtime/respondent_group_action.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,34 @@ defmodule Ask.Runtime.RespondentGroupAction do

respondents_count = phone_numbers |> length

respondent_group =
%RespondentGroup{
name: name,
survey_id: survey.id,
sample: sample,
respondents_count: respondents_count
}
|> Repo.insert!()
|> Repo.preload(:respondent_group_channels)

insert_respondents(phone_numbers, respondent_group)

survey
|> Repo.preload([:questionnaires])
|> Repo.preload([:quota_buckets])
|> Repo.preload(respondent_groups: [respondent_group_channels: :channel])
|> Changeset.change()
|> Survey.update_state()
|> Repo.update!()
Repo.transaction(fn ->
respondent_group =
%RespondentGroup{
name: name,
survey_id: survey.id,
sample: sample,
respondents_count: respondents_count
}
|> Repo.insert!()
|> Repo.preload(:respondent_group_channels)

insert_respondents(phone_numbers, respondent_group)

survey
|> Repo.preload([:questionnaires])
|> Repo.preload([:quota_buckets])
|> Repo.preload(respondent_groups: [respondent_group_channels: :channel])
|> Changeset.change()
|> Survey.update_state()
|> Repo.update!()

respondent_group
respondent_group
end)
|> case do
{:ok, respondent_group} ->
respondent_group
err -> err
end
end

def add_respondents(respondent_group, loaded_entries, file_name, conn) do
Expand All @@ -54,51 +61,65 @@ defmodule Ask.Runtime.RespondentGroupAction do

loaded_entries = clean_entries(loaded_entries, phone_numbers)

insert_respondents(phone_numbers, respondent_group)
Repo.transaction(fn ->
insert_respondents(phone_numbers, respondent_group)

respondents_count = Enum.count(phone_numbers)
respondents_count = Enum.count(phone_numbers)

if Survey.launched?(survey) and respondents_count > 0 do
ActivityLog.add_respondents(survey.project, conn, survey, %{
file_name: file_name,
respondents_count: respondents_count
})
|> Repo.insert!()
end
if Survey.launched?(survey) and respondents_count > 0 do
ActivityLog.add_respondents(survey.project, conn, survey, %{
file_name: file_name,
respondents_count: respondents_count
})
|> Repo.insert!()
end

new_count = respondent_group.respondents_count + length(phone_numbers)
new_count = respondent_group.respondents_count + length(phone_numbers)

new_sample = merge_sample(respondent_group.sample, loaded_entries)
new_sample = merge_sample(respondent_group.sample, loaded_entries)

respondent_group
|> RespondentGroup.changeset(%{"respondents_count" => new_count, "sample" => new_sample})
|> Repo.update!()
respondent_group
|> RespondentGroup.changeset(%{"respondents_count" => new_count, "sample" => new_sample})
|> Repo.update!()
end)
|> case do
{:ok, respondent_group} ->
respondent_group
err -> err
end
end

def replace_respondents(respondent_group, loaded_entries) do
respondent_group = Repo.preload(respondent_group, :survey)
phone_numbers = map_phone_numbers_from_loaded_entries(loaded_entries)

# First delete existing respondents from that group
Repo.delete_all(
from(r in Respondent,
where: r.respondent_group_id == ^respondent_group.id
Repo.transaction(fn ->
# First delete existing respondents from that group
Repo.delete_all(
from(r in Respondent,
where: r.respondent_group_id == ^respondent_group.id
)
)
)

# Then create respondents from the CSV file
insert_respondents(phone_numbers, respondent_group)
# Then create respondents from the CSV file
insert_respondents(phone_numbers, respondent_group)

sample = take_sample(loaded_entries)
respondents_count = phone_numbers |> length
sample = take_sample(loaded_entries)
respondents_count = phone_numbers |> length

respondent_group
|> RespondentGroup.changeset(%{
"sample" => sample,
"respondents_count" => respondents_count
})
|> Repo.update!()
|> Repo.preload(:respondent_group_channels)
respondent_group
|> RespondentGroup.changeset(%{
"sample" => sample,
"respondents_count" => respondents_count
})
|> Repo.update!()
|> Repo.preload(:respondent_group_channels)
end)
|> case do
{:ok, respondent_group} ->
respondent_group
err -> err
end
end

defp clean_entries(loaded_entries, phone_numbers) do
Expand Down Expand Up @@ -175,7 +196,7 @@ defmodule Ask.Runtime.RespondentGroupAction do
end)
end

def insert_respondents(phone_numbers, respondent_group) do
defp insert_respondents(phone_numbers, respondent_group) do
respondent_group = Repo.preload(respondent_group, survey: :project)

map_respondent = fn phone_number ->
Expand Down