From 57f1a933ec90d697b5df0197cd690f845680d050 Mon Sep 17 00:00:00 2001 From: Antoine Augusti Date: Thu, 26 Sep 2024 11:10:44 +0200 Subject: [PATCH] contact : ajout colonne creation_source (#4225) --- apps/transport/lib/db/contact.ex | 6 ++- .../jobs/import_dataset_contact_points_job.ex | 2 +- .../backoffice/contact_controller.ex | 2 +- .../controllers/session_controller.ex | 3 +- ...0926074311_contact_add_creation_source.exs | 44 +++++++++++++++++++ apps/transport/test/db/contact_test.exs | 17 ++++++- apps/transport/test/support/factory.ex | 3 +- ...import_dataset_contact_points_job_test.exs | 6 ++- .../backoffice/contact_controller_test.exs | 14 ++++-- ...ification_subscription_controller_test.exs | 21 +++------ .../controllers/session_controller_test.exs | 6 ++- 11 files changed, 93 insertions(+), 31 deletions(-) create mode 100644 apps/transport/priv/repo/migrations/20240926074311_contact_add_creation_source.exs diff --git a/apps/transport/lib/db/contact.ex b/apps/transport/lib/db/contact.ex index d15a9a0b9d..c56e6d5521 100644 --- a/apps/transport/lib/db/contact.ex +++ b/apps/transport/lib/db/contact.ex @@ -29,6 +29,7 @@ defmodule DB.Contact do field(:phone_number, DB.Encrypted.Binary) field(:secondary_phone_number, DB.Encrypted.Binary) field(:last_login_at, :utc_datetime_usec) + field(:creation_source, Ecto.Enum, values: [:"automation:import_contact_point", :admin, :datagouv_oauth_login]) timestamps(type: :utc_datetime_usec) @@ -115,11 +116,12 @@ defmodule DB.Contact do :phone_number, :secondary_phone_number, :datagouv_user_id, - :last_login_at + :last_login_at, + :creation_source ]) |> trim_fields([:first_name, :last_name, :organization, :job_title]) |> capitalize_fields([:first_name, :last_name]) - |> validate_required([:email]) + |> validate_required([:email, :creation_source]) |> validate_format(:email, ~r/@/) |> validate_names_or_mailing_list_title() |> cast_phone_numbers() diff --git a/apps/transport/lib/jobs/import_dataset_contact_points_job.ex b/apps/transport/lib/jobs/import_dataset_contact_points_job.ex index 97e296d318..aaeb235094 100644 --- a/apps/transport/lib/jobs/import_dataset_contact_points_job.ex +++ b/apps/transport/lib/jobs/import_dataset_contact_points_job.ex @@ -131,7 +131,7 @@ defmodule Transport.Jobs.ImportDatasetContactPointsJob do contact nil -> - Map.merge(guess_identity(name), %{email: email}) + Map.merge(guess_identity(name), %{email: email, creation_source: :"automation:import_contact_point"}) |> DB.Contact.insert!() end end diff --git a/apps/transport/lib/transport_web/controllers/backoffice/contact_controller.ex b/apps/transport/lib/transport_web/controllers/backoffice/contact_controller.ex index 61b56c7a6d..c0b8e9a82d 100644 --- a/apps/transport/lib/transport_web/controllers/backoffice/contact_controller.ex +++ b/apps/transport/lib/transport_web/controllers/backoffice/contact_controller.ex @@ -37,7 +37,7 @@ defmodule TransportWeb.Backoffice.ContactController do end defp existing_contact(%{"id" => id}) when id != "", do: DB.Repo.get!(DB.Contact, String.to_integer(id)) - defp existing_contact(%{}), do: %DB.Contact{} + defp existing_contact(%{}), do: %DB.Contact{creation_source: :admin} @spec edit(Plug.Conn.t(), map()) :: Plug.Conn.t() def edit(%Plug.Conn{} = conn, %{"id" => contact_id} = params) do diff --git a/apps/transport/lib/transport_web/controllers/session_controller.ex b/apps/transport/lib/transport_web/controllers/session_controller.ex index bfe9288d2a..5403930b61 100644 --- a/apps/transport/lib/transport_web/controllers/session_controller.ex +++ b/apps/transport/lib/transport_web/controllers/session_controller.ex @@ -126,7 +126,8 @@ defmodule TransportWeb.SessionController do datagouv_user_id: user_id, first_name: first_name, last_name: last_name, - email: email + email: email, + creation_source: :datagouv_oauth_login } |> DB.Contact.insert!() end diff --git a/apps/transport/priv/repo/migrations/20240926074311_contact_add_creation_source.exs b/apps/transport/priv/repo/migrations/20240926074311_contact_add_creation_source.exs new file mode 100644 index 0000000000..5a4ce2e1fb --- /dev/null +++ b/apps/transport/priv/repo/migrations/20240926074311_contact_add_creation_source.exs @@ -0,0 +1,44 @@ +defmodule DB.Repo.Migrations.ContactAddCreationSource do + use Ecto.Migration + + def change do + alter table(:contact) do + add(:creation_source, :string) + end + + execute( + """ + UPDATE contact + SET creation_source = 'automation:import_contact_point' + WHERE id in ( + SELECT distinct contact_id + FROM notification_subscription + WHERE source = 'automation:import_contact_point' + ) + """, + "" + ) + + execute( + """ + UPDATE contact + SET creation_source = 'admin' + WHERE creation_source IS NULL + AND (last_login_at IS NULL OR inserted_at::date = '2023-03-02') + """, + "" + ) + + execute( + """ + UPDATE contact + SET creation_source = 'datagouv_oauth_login' + WHERE creation_source IS NULL + """, + "" + ) + + # Make sure the column is not null + execute("ALTER TABLE contact ALTER COLUMN creation_source SET NOT NULL", "") + end +end diff --git a/apps/transport/test/db/contact_test.exs b/apps/transport/test/db/contact_test.exs index 4be6a1894b..3a592f7199 100644 --- a/apps/transport/test/db/contact_test.exs +++ b/apps/transport/test/db/contact_test.exs @@ -17,7 +17,8 @@ defmodule DB.ContactTest do job_title: "Chef SIG", organization: "Big Corp Inc", phone_number: "06 82 22 88 03", - secondary_phone_number: "+33 1 99 00 17 45" + secondary_phone_number: "+33 1 99 00 17 45", + creation_source: "admin" } |> DB.Contact.insert!() @@ -150,6 +151,17 @@ defmodule DB.ContactTest do DB.Contact.insert!(%{sample_contact_args() | email: "foo.BAR@example.fr"}) end + test "creation_source is required and is an enum" do + assert %Ecto.Changeset{valid?: false, errors: [creation_source: {"can't be blank", [validation: :required]}]} = + DB.Contact.changeset(%DB.Contact{}, %{sample_contact_args() | creation_source: nil}) + + assert %Ecto.Changeset{valid?: false, errors: [creation_source: {"is invalid", _}]} = + DB.Contact.changeset(%DB.Contact{}, %{sample_contact_args() | creation_source: "foobar"}) + + assert %Ecto.Changeset{valid?: true} = + DB.Contact.changeset(%DB.Contact{}, %{sample_contact_args() | creation_source: :admin}) + end + test "search" do search_fn = fn args -> args |> DB.Contact.search() |> order_by([contact: c], asc: c.id) |> DB.Repo.all() end assert search_fn.(%{}) == [] @@ -301,7 +313,8 @@ defmodule DB.ContactTest do email: "john#{Ecto.UUID.generate()}@example.fr", job_title: "Boss", organization: "Big Corp Inc", - phone_number: "06 82 22 88 03" + phone_number: "06 82 22 88 03", + creation_source: "admin" } end end diff --git a/apps/transport/test/support/factory.ex b/apps/transport/test/support/factory.ex index 896670828a..9d67e616e2 100644 --- a/apps/transport/test/support/factory.ex +++ b/apps/transport/test/support/factory.ex @@ -332,7 +332,8 @@ defmodule DB.Factory do email: "john#{Ecto.UUID.generate()}@example.fr", job_title: "Boss", organization: "Big Corp Inc", - phone_number: "06 82 22 88 03" + phone_number: "06 82 22 88 03", + creation_source: "admin" } |> Map.merge(args) |> DB.Contact.insert!() diff --git a/apps/transport/test/transport/jobs/import_dataset_contact_points_job_test.exs b/apps/transport/test/transport/jobs/import_dataset_contact_points_job_test.exs index 1893163d7a..e08430a8f3 100644 --- a/apps/transport/test/transport/jobs/import_dataset_contact_points_job_test.exs +++ b/apps/transport/test/transport/jobs/import_dataset_contact_points_job_test.exs @@ -139,7 +139,8 @@ defmodule Transport.Test.Transport.Jobs.ImportDatasetContactPointsJobTest do ImportDatasetContactPointsJob.import_contact_point(datagouv_id) - %DB.Contact{first_name: "John", email: ^email} = contact = DB.Repo.get_by(DB.Contact, last_name: "DOE") + %DB.Contact{first_name: "John", email: ^email, creation_source: :"automation:import_contact_point"} = + contact = DB.Repo.get_by(DB.Contact, last_name: "DOE") assert nil == DB.Repo.reload(previous_contact_point_ns) assert MapSet.new(@producer_reasons) == subscribed_reasons(dataset, contact) @@ -165,7 +166,8 @@ defmodule Transport.Test.Transport.Jobs.ImportDatasetContactPointsJobTest do assert :ok == perform_job(ImportDatasetContactPointsJob, %{}) - %DB.Contact{email: ^email, first_name: "John"} = created_contact = DB.Repo.get_by(DB.Contact, last_name: "DOE") + %DB.Contact{email: ^email, first_name: "John", creation_source: :"automation:import_contact_point"} = + created_contact = DB.Repo.get_by(DB.Contact, last_name: "DOE") assert MapSet.new(@producer_reasons) == subscribed_reasons(d1, contact_point) assert MapSet.new([]) == subscribed_reasons(d2, contact_point) diff --git a/apps/transport/test/transport_web/controllers/backoffice/contact_controller_test.exs b/apps/transport/test/transport_web/controllers/backoffice/contact_controller_test.exs index 99c5c547cb..9a3f054263 100644 --- a/apps/transport/test/transport_web/controllers/backoffice/contact_controller_test.exs +++ b/apps/transport/test/transport_web/controllers/backoffice/contact_controller_test.exs @@ -66,7 +66,8 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do assert [ {"li", [], ["first_name : You need to fill either first_name and last_name OR mailing_list_title"]}, - {"li", [], ["email : can't be blank"]} + {"li", [], ["email : can't be blank"]}, + {"li", [], ["creation_source : can't be blank"]} ] == Floki.find(doc, ".notification.error ul li") end end @@ -87,7 +88,13 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do assert redirected_to(conn, 302) == backoffice_contact_path(conn, :index) - assert %DB.Contact{first_name: "John", last_name: "Doe", email: "john@example.com", organization: "Corp Inc"} = + assert %DB.Contact{ + first_name: "John", + last_name: "Doe", + email: "john@example.com", + organization: "Corp Inc", + creation_source: :admin + } = DB.Repo.one!(DB.Contact) assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "Contact mis à jour" @@ -293,7 +300,8 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do email: "john#{Ecto.UUID.generate()}@example.fr", job_title: "Boss", organization: "Big Corp Inc", - phone_number: "06 82 22 88 03" + phone_number: "06 82 22 88 03", + creation_source: "admin" }, args ) diff --git a/apps/transport/test/transport_web/controllers/backoffice/notification_subscription_controller_test.exs b/apps/transport/test/transport_web/controllers/backoffice/notification_subscription_controller_test.exs index 26a2c04339..44ed5aa3db 100644 --- a/apps/transport/test/transport_web/controllers/backoffice/notification_subscription_controller_test.exs +++ b/apps/transport/test/transport_web/controllers/backoffice/notification_subscription_controller_test.exs @@ -11,7 +11,7 @@ defmodule TransportWeb.NotificationSubscriptionControllerTest do describe "create" do test "for reasons related to a dataset", %{conn: conn} do %DB.Dataset{id: dataset_id} = insert(:dataset) - %DB.Contact{id: contact_id} = DB.Contact.insert!(sample_contact_args()) + %DB.Contact{id: contact_id} = insert_contact() args = %{ "redirect_location" => "dataset", @@ -66,7 +66,7 @@ defmodule TransportWeb.NotificationSubscriptionControllerTest do test "for reasons not related to datasets", %{conn: conn} do %DB.Dataset{id: dataset_id} = insert(:dataset) - %DB.Contact{id: contact_id} = DB.Contact.insert!(sample_contact_args()) + %DB.Contact{id: contact_id} = insert_contact() # An existing subscription linked to a dataset insert(:notification_subscription, dataset_id: dataset_id, @@ -122,7 +122,7 @@ defmodule TransportWeb.NotificationSubscriptionControllerTest do test "delete", %{conn: conn} do %DB.Dataset{id: dataset_id} = insert(:dataset) - %DB.Contact{id: contact_id} = DB.Contact.insert!(sample_contact_args()) + %DB.Contact{id: contact_id} = insert_contact() %DB.NotificationSubscription{id: subscription_id} = insert(:notification_subscription, @@ -148,8 +148,8 @@ defmodule TransportWeb.NotificationSubscriptionControllerTest do test "delete_for_contact_and_dataset", %{conn: conn} do %DB.Dataset{id: dataset_id} = insert(:dataset) - %DB.Contact{id: contact_id} = DB.Contact.insert!(sample_contact_args()) - %DB.Contact{id: other_contact_id} = DB.Contact.insert!(sample_contact_args()) + %DB.Contact{id: contact_id} = insert_contact() + %DB.Contact{id: other_contact_id} = insert_contact() insert(:notification_subscription, contact_id: contact_id, @@ -196,15 +196,4 @@ defmodule TransportWeb.NotificationSubscriptionControllerTest do } ] = DB.NotificationSubscription |> DB.Repo.all() end - - defp sample_contact_args do - %{ - first_name: "John", - last_name: "Doe", - email: "john#{Ecto.UUID.generate()}@example.fr", - job_title: "Boss", - organization: "Big Corp Inc", - phone_number: "06 82 22 88 03" - } - end end diff --git a/apps/transport/test/transport_web/controllers/session_controller_test.exs b/apps/transport/test/transport_web/controllers/session_controller_test.exs index 2cec82d7d8..dc4432d59e 100644 --- a/apps/transport/test/transport_web/controllers/session_controller_test.exs +++ b/apps/transport/test/transport_web/controllers/session_controller_test.exs @@ -65,7 +65,8 @@ defmodule TransportWeb.SessionControllerTest do email: ^email, organization: ^organization_name, datagouv_user_id: ^datagouv_user_id, - last_login_at: last_login_at + last_login_at: last_login_at, + creation_source: :datagouv_oauth_login } ] = DB.Repo.all(DB.Contact) @@ -215,7 +216,8 @@ defmodule TransportWeb.SessionControllerTest do datagouv_user_id: ^datagouv_user_id, email: ^email, organization: ^org_name, - last_login_at: last_login_at + last_login_at: last_login_at, + creation_source: :datagouv_oauth_login } ] = DB.Contact |> DB.Repo.all()