Duplicate survey (#2354)

* Fix unrelated typing warning

Was laying there since #1842

* Refactor API preparing to receive more survey opts

Will need this to duplicate a survey.

See #2351

* Add endpoint to duplicate surveys

I still have to check if it works with already-running surveys, and
panel surveys and other configurations.

See #2351

* Expose survey duplicate feature from UI

See #2351

* Prevent duplicating Panel Surveys

See #2351

* Test duplication of running surveys

See #2351

* Test duplicate surveys don't have exit status

See #2351

* Fix: SurveyCanceller tests avoid the Supervisor

The tests were unnecessarily coupled to the Supervisor. We still have
lots of improvements to do, but this should make the tests pass.

See #2337
See #2147

Co-authored-by: Ana Pérez Ghiglia <aghiglia@manas.tech>

* Fix: add Flow types to new action

See #2351

* Don't over-engineer the double-click prevention

* Test copying standard quex

* Fix running survey duplication test

* Comment how quota_buckets_definitions work

* Extract common survey creation behaviour

See #2351
See #2354

---------

Co-authored-by: Ana Pérez Ghiglia <aghiglia@manas.tech>

assets/js/actions/panelSurvey.js

@@ -7,7 +7,7 @@ export const RECEIVE = "RECEIVE_PANEL_SURVEY"
 
 export const createPanelSurvey =
   (projectId: number, folderId?: number) => (dispatch: Function, getState: () => Store) =>
-    api.createSurvey(projectId, folderId, true).then((response) => {
+    api.createSurvey(projectId, folderId, { generatesPanelSurvey: true } ).then((response) => {
       const firstWave = response.result
       dispatch(fetch(projectId, firstWave.id))
       dispatch(receive(firstWave))

assets/js/actions/survey.js

@@ -68,6 +68,15 @@ export const fetchSurvey =
       })
   }
 
+export const duplicateSurvey = (survey: Survey) => (dispatch: Function) => {
+  return api.duplicateSurvey(survey.projectId, survey.id).then((response) => {
+    const newSurvey = response.entities.surveys[response.result]
+    dispatch(fetch(newSurvey.projectId, newSurvey.id))
+    dispatch(receive(newSurvey))
+    return newSurvey
+  })
+}
+
 export const fetchSurveyStats = (projectId: number, id: number) => (dispatch: Function) => {
   api.fetchSurveyStats(projectId, id).then((stats) => dispatch(receiveSurveyStats(id, stats)))
 }

assets/js/api.js

@@ -207,11 +207,13 @@ export const renameFolder = (projectId, folderId, name) => {
   return apiPostJSON(`projects/${projectId}/folders/${folderId}/set_name`, folderSchema, { name })
 }
 
-export const createSurvey = (projectId, folderId, generatesPanelSurvey = false) => {
+export const createSurvey = (projectId, folderId, survey) => {
   let folderPath = folderId ? `/folders/${folderId}` : ""
-  return apiPostJSON(`projects/${projectId}${folderPath}/surveys`, surveySchema, {
-    survey: { generatesPanelSurvey },
-  })
+  return apiPostJSON(`projects/${projectId}${folderPath}/surveys`, surveySchema, { survey })
+}
+
+export const duplicateSurvey = (projectId, surveyId) => {
+  return apiPostJSON(`projects/${projectId}/surveys/${surveyId}/duplicate`, surveySchema)
 }
 
 export const deleteSurvey = (projectId, survey) => {

assets/js/components/surveys/RespondentsContainer.jsx

@@ -43,5 +43,5 @@ export const RespondentsContainer = translate()(({ children, t, incentivesEnable
 RespondentsContainer.propTypes = {
   t: PropTypes.func,
   children: PropTypes.node,
-  incentiveDisabled: PropTypes.boolean,
+  incentivesEnabled: PropTypes.bool,
 }

assets/js/components/surveys/SurveyCard.jsx

@@ -23,6 +23,8 @@ class _SurveyCard extends Component<any> {
     readOnly: boolean,
   }
 
+  duplicatingSurvey: boolean
+
   constructor(props) {
     super(props)
 
@@ -36,10 +38,24 @@ class _SurveyCard extends Component<any> {
     if (survey.state != "not_ready") {
       fetchRespondentsStats(survey.projectId, survey.id)(dispatch)
     }
+
+    this.duplicatingSurvey = false
   }
 
   changeFolder = (folderId) => this.setState({ folderId })
 
+  duplicateSurvey = () => {
+    // Prevent duplicating multiple surveys at once
+    if (this.duplicatingSurvey) return
+    this.duplicatingSurvey = true
+
+    const { dispatch, survey } = this.props
+    dispatch(surveyActions.duplicateSurvey(survey)).then((newSurvey) => {
+      this.duplicatingSurvey = false
+      window.location = routes.survey(newSurvey.projectId, newSurvey.id)
+    })
+  }
+
   askMoveSurvey = () => {
     const moveSurveyConfirmationModal: ConfirmationModal = this.refs.moveSurveyConfirmationModal
     const { survey } = this.props
@@ -106,7 +122,9 @@ class _SurveyCard extends Component<any> {
   render() {
     const { survey, t, dispatch } = this.props
 
-    let actions = []
+    let actions = [
+      { name: t("Duplicate"), icon: "content_copy", func: this.duplicateSurvey }
+    ]
     if (this.movable())
       actions.push({ name: t("Move to"), icon: "folder", func: this.askMoveSurvey })
     if (this.deletable())

lib/ask/runtime/survey_canceller.ex

@@ -21,7 +21,8 @@ defmodule Ask.Runtime.SurveyCanceller do
 
   @impl true
   def init(survey_id) do
-    :timer.send_after(1000, :cancel)
+    # FIXME: We only care about the :cancel being async, so we use the smallest timeout possible - but this smells bad
+    :timer.send_after(1, :cancel)
     Logger.info("Starting canceller for survey #{survey_id}")
     {:ok, survey_id}
   end

lib/ask_web/controllers/survey_controller.ex

@@ -7,6 +7,7 @@ defmodule AskWeb.SurveyController do
     Survey,
     Logger,
     ActivityLog,
+    QuotaBucket,
     RetriesHistogram,
     ScheduleError
   }
@@ -78,6 +79,46 @@ defmodule AskWeb.SurveyController do
       |> build_assoc(:surveys)
       |> Survey.changeset(props)
 
+    insert_survey_and_show_it(changeset, project, conn)
+  end
+
+  def duplicate(conn, %{"project_id" => project_id, "survey_id" => source_survey_id}) do
+    project = load_project_for_change(conn, project_id)
+
+    source_survey =
+      project
+      |> load_standalone_survey(source_survey_id)
+      |> Repo.preload([:quota_buckets, :questionnaires])
+
+    props = %{
+      "project_id" => project_id,
+      "folder_id" => source_survey.folder_id,
+      "name" => "#{source_survey.name || "Untitled survey"} (duplicate)",
+      "description" => source_survey.description,
+      "comparisons" => source_survey.comparisons,
+      "questionnaire_ids" => editable_questionnaire_ids(source_survey.questionnaires),
+      "mode" => source_survey.mode,
+      "schedule" => source_survey.schedule,
+      "fallback_delay" => source_survey.fallback_delay,
+      "ivr_retry_configuration" => source_survey.ivr_retry_configuration,
+      "mobileweb_retry_configuration" => source_survey.mobileweb_retry_configuration,
+      "sms_retry_configuration" => source_survey.sms_retry_configuration,
+      "cutoff" => source_survey.cutoff,
+      "quota_vars" => source_survey.quota_vars,
+      "count_partial_results" => source_survey.count_partial_results,
+    }
+
+    changeset =
+      project
+      |> build_assoc(:surveys)
+      |> Survey.changeset(props)
+      |> update_questionnaires(props)
+      |> put_assoc(:quota_buckets, quota_buckets_definitions(source_survey.quota_buckets))
+
+    insert_survey_and_show_it(changeset, project, conn)
+  end
+
+  defp insert_survey_and_show_it(changeset, project, conn) do
     multi =
       Multi.new()
       |> Multi.insert(:survey, changeset)
@@ -94,11 +135,11 @@ defmodule AskWeb.SurveyController do
           survey
           |> Repo.preload([:quota_buckets])
           |> Repo.preload(:questionnaires)
-          |> Survey.with_links(user_level(project_id, current_user(conn).id))
+          |> Survey.with_links(user_level(project.id, current_user(conn).id))
 
         conn
         |> put_status(:created)
-        |> put_resp_header("location", project_survey_path(conn, :show, project_id, survey))
+        |> put_resp_header("location", project_survey_path(conn, :show, project.id, survey))
         |> render("show.json", survey: survey)
 
       {:error, _, changeset, _} ->
@@ -111,6 +152,20 @@ defmodule AskWeb.SurveyController do
     end
   end
 
+  defp editable_questionnaire_ids(questionnaires), do:
+    Enum.map(questionnaires,
+      fn %{id: questionnaire_id, snapshot_of: original_questionnaire_id} -> original_questionnaire_id || questionnaire_id end
+    )
+
+  # Duplicate the buckets without their counts and quotas
+  # This is because duplicate surveys will have different respondent groups,
+  # implying different quotas - and no current counts
+  defp quota_buckets_definitions(quota_buckets), do:
+    Enum.map(quota_buckets, fn %{condition: condition} ->
+      %QuotaBucket{condition: condition}
+    end
+    )
+
   def show(conn, %{"project_id" => project_id, "id" => id}) do
     survey =
       conn
@@ -479,4 +534,15 @@ defmodule AskWeb.SurveyController do
     |> assoc(:surveys)
     |> Repo.get!(survey_id)
   end
+
+  defp load_standalone_survey(project, survey_id) do
+    load_survey(project, survey_id)
+    |> validate_standalone_survey
+  end
+
+  # TODO: this seems to clash with the definition in `Survey.belongs_to_panel_survey?`
+  defp validate_standalone_survey(%{generates_panel_survey: true}), do: raise ConflictError
+  defp validate_standalone_survey(survey = %{panel_survey_id: nil}), do: survey
+  defp validate_standalone_survey(%{panel_survey_id: _id}), do: raise ConflictError
+
 end

lib/ask_web/router.ex

@@ -102,6 +102,7 @@ defmodule AskWeb.Router do
           post "/launch", SurveyController, :launch
           post "/stop", SurveyController, :stop
           post "/config", SurveyController, :config
+          post "/duplicate", SurveyController, :duplicate
 
           put "/update_locked_status", SurveyController, :update_locked_status,
             as: :update_locked_status

locales/template/translation.json

@@ -148,6 +148,7 @@
   "Drop your CSV file here, or click to browse": "",
   "Drop your MP3, WAV, M4A, ACC or MP4 file here, or click to browse": "",
   "Drop your file to start uploading": "",
+  "Duplicate": "",
   "Duplicate questionnaire": "",
   "EDIT QUOTAS": "",
   "Edited <i>{{surveyName}}</i> survey": "",

test/ask/runtime/survey_canceller_supervisor_test.exs

@@ -13,10 +13,24 @@ defmodule Ask.Runtime.SurveyCancellerSupervisorTest do
       assert [] = processes
     end
 
-    test "supervisor starts when surveys to cancel" do
-      build(:survey, state: :cancelling) |> Repo.insert!()
+    test "supervisor starts canceller process for single survey to cancel" do
+      %{id: survey_id} = build(:survey, state: :cancelling) |> Repo.insert!()
       {:ok, {_, processes}} = SurveyCancellerSupervisor.init([])
-      assert Enum.count(processes) > 0
+      assert Enum.count(processes) == 1
+      %{ id: canceller_id } = processes |> hd
+      assert survey_canceller_name(survey_id) == canceller_id
+    end
+
+    test "supervisor starts canceller process for multilpe surveys to cancel" do
+      %{id: survey_id_1} = build(:survey, state: :cancelling) |> Repo.insert!()
+      %{id: survey_id_2} = build(:survey, state: :cancelling) |> Repo.insert!()
+      {:ok, {_, processes}} = SurveyCancellerSupervisor.init([])
+      assert Enum.count(processes) == 2
+      canceller_ids = Enum.map(processes, fn %{ id: canceller_id } -> canceller_id end) |> MapSet.new
+      expected_ids = [ survey_canceller_name(survey_id_1), survey_canceller_name(survey_id_2) ] |> MapSet.new
+      assert canceller_ids == expected_ids
     end
   end
+
+  defp survey_canceller_name(survey_id), do: :"SurveyCanceller_#{survey_id}"
 end

test/ask/survey_canceller_test.exs

@@ -7,7 +7,7 @@ defmodule Ask.SurveyCancellerTest do
   use Ask.DummySteps
 
   alias Ask.{Survey, RespondentGroup, Channel, TestChannel, RespondentGroupChannel}
-  alias Ask.Runtime.{Flow, Session, SurveyCancellerSupervisor}
+  alias Ask.Runtime.{Flow, Session, SurveyCanceller, SurveyCancellerSupervisor}
   alias Ask.Runtime.SessionModeProvider
 
   setup %{conn: conn} do
@@ -21,6 +21,34 @@ defmodule Ask.SurveyCancellerTest do
     {:ok, conn: conn, user: user}
   end
 
+  describe "canceller" do
+    test "sends a cancel message to itself on init" do
+      project = insert(:project)
+      %{id: survey_id} = insert(:survey, project: project)
+      {:ok, canceller_pid } = SurveyCanceller.start_link(survey_id)
+      :erlang.trace(canceller_pid, true, [:receive])
+      assert_receive {:trace, ^canceller_pid, :receive, :cancel}, 2_000
+    end
+
+    test "marks the survey as terminated" do
+      project = insert(:project)
+      %{id: survey_id} = insert(:survey, project: project)
+
+      cancel_survey(survey_id)
+
+      survey = Repo.get(Survey, survey_id)
+
+      assert survey.state == :terminated
+    end
+  end
+
+  defp cancel_survey(survey_id) do
+    {:ok, canceller_pid } = SurveyCanceller.start_link(survey_id)
+    ref = Process.monitor(canceller_pid)
+    assert_receive {:DOWN, ^ref, :process, ^canceller_pid, :normal }, 2_000
+    :ok
+  end
+
   describe "stops surveys as if the application were starting" do
     test "survey canceller does not have pending surveys to cancel" do
       start_survey_canceller_supervisor()
@@ -68,9 +96,7 @@ defmodule Ask.SurveyCancellerTest do
       |> Ask.Respondent.changeset(%{session: session})
       |> Repo.update!()
 
-      start_survey_canceller_supervisor()
-
-      wait_all_cancellations(survey_1)
+      cancel_survey(survey_1.id)
 
       survey = Repo.get(Survey, survey_1.id)
       assert Survey.cancelled?(survey)
@@ -122,10 +148,8 @@ defmodule Ask.SurveyCancellerTest do
       |> Ask.Respondent.changeset(%{session: session})
       |> Repo.update!()
 
-      start_survey_canceller_supervisor()
-
-      wait_all_cancellations(survey_1)
-      wait_all_cancellations(survey_2)
+      cancel_survey(survey_1.id)
+      cancel_survey(survey_2.id)
 
       survey = Repo.get(Survey, survey_1.id)
       survey_2 = Repo.get(Survey, survey_2.id)
@@ -144,6 +168,7 @@ defmodule Ask.SurveyCancellerTest do
       assert_receive [:cancel_message, ^test_channel, %{}]
     end
 
+    @tag :skip # FIXME: this is testing the supervisor launches a new canceller - it isn't a canceller test
     test "stops multiple surveys from canceller and from controller simultaneously", %{
       conn: conn,
       user: user