From 0920dd28c453a7de46fa9f176c4943bb61524411 Mon Sep 17 00:00:00 2001
From: Antoine Augusti <antoine.augusti@transport.data.gouv.fr>
Date: Tue, 25 Jul 2023 19:33:07 +0200
Subject: [PATCH] =?UTF-8?q?Am=C3=A9lioration=20de=20la=20recherche=20de=20?=
 =?UTF-8?q?contacts=20dans=20le=20backoffice=20(#3348)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Amélioration de la recherche de contacts dans le backoffice

* Add test

* Improve unaccent

* refactor

* Ignore Credo

* PR comments

---------

Co-authored-by: Thibaut Barrère <thibaut.barrere@gmail.com>
---
 apps/transport/lib/db/contact.ex              | 16 ++++++++++++-
 .../backoffice/contact_controller.ex          | 23 +++++++++----------
 .../backoffice/contact/index.html.heex        | 10 ++++----
 .../backoffice/page/form_dataset.html.heex    |  7 +++++-
 .../views/backoffice/contact_view.ex          |  1 +
 .../views/backoffice/page_view.ex             | 14 +++++++++++
 apps/transport/test/db/contact_test.exs       |  7 ++++--
 .../backoffice/contact_controller_test.exs    | 10 ++++----
 .../views/backoffice/page_view_test.exs       |  4 ++++
 9 files changed, 68 insertions(+), 24 deletions(-)
 create mode 100644 apps/transport/test/transport_web/views/backoffice/page_view_test.exs

diff --git a/apps/transport/lib/db/contact.ex b/apps/transport/lib/db/contact.ex
index 4ecebb9a84..83aafeec2e 100644
--- a/apps/transport/lib/db/contact.ex
+++ b/apps/transport/lib/db/contact.ex
@@ -36,8 +36,11 @@ defmodule DB.Contact do
   def base_query, do: from(c in __MODULE__, as: :contact)
 
   def search(%{"q" => q}) do
+    ilike_search = "%#{safe_like_pattern(q)}%"
+
     base_query()
-    |> where([contact: c], c.organization == ^q)
+    |> where([contact: c], ilike(c.organization, ^ilike_search))
+    |> or_where([contact: c], fragment("unaccent(?) ilike unaccent(?)", c.last_name, ^ilike_search))
     |> or_where(
       [contact: c],
       fragment(
@@ -52,6 +55,17 @@ defmodule DB.Contact do
 
   def search(%{}), do: base_query()
 
+  @doc """
+  Make sure a string that will be passed to `like` or `ilike` is safe.
+
+  See https://elixirforum.com/t/secure-ecto-like-queries/31265
+  iex> safe_like_pattern("I love %like_injections%\\!")
+  "I love likeinjections!"
+  """
+  def safe_like_pattern(value) do
+    String.replace(value, ["\\", "%", "_"], "")
+  end
+
   def insert!(%{} = fields), do: %__MODULE__{} |> changeset(fields) |> DB.Repo.insert!()
 
   @doc """
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 5a87dcea9a..a3e301237e 100644
--- a/apps/transport/lib/transport_web/controllers/backoffice/contact_controller.ex
+++ b/apps/transport/lib/transport_web/controllers/backoffice/contact_controller.ex
@@ -57,8 +57,8 @@ defmodule TransportWeb.Backoffice.ContactController do
 
   defp render_form(%Plug.Conn{assigns: assigns} = conn) do
     conn
-    |> assign(:existing_organizations, existing_organizations())
-    |> assign(:existing_job_titles, existing_job_titles())
+    |> assign(:existing_organizations, contact_values_for_field(:organization))
+    |> assign(:existing_job_titles, contact_values_for_field(:job_title))
     |> assign(:datasets_datalist, datasets_datalist())
     |> assign(:notification_subscriptions, notification_subscriptions_for_contact(Map.get(assigns, :contact_id)))
     |> render("form.html")
@@ -73,22 +73,21 @@ defmodule TransportWeb.Backoffice.ContactController do
 
     conn
     |> assign(:contacts, paginated_contacts)
-    |> assign(:existing_organizations, existing_organizations())
+    |> assign(:search_datalist, search_datalist())
     |> render("index.html")
   end
 
-  defp existing_organizations do
-    DB.Contact.base_query()
-    |> select([contact: c], c.organization)
-    |> order_by([contact: c], asc: c.organization)
-    |> distinct(true)
-    |> DB.Repo.all()
+  defp search_datalist do
+    :organization
+    |> contact_values_for_field()
+    |> Enum.concat(contact_values_for_field(:last_name))
+    |> Enum.sort()
   end
 
-  defp existing_job_titles do
+  defp contact_values_for_field(field) when is_atom(field) do
     DB.Contact.base_query()
-    |> select([contact: c], c.job_title)
-    |> order_by([contact: c], asc: c.job_title)
+    |> select([contact: c], field(c, ^field))
+    |> order_by([contact: c], asc: ^field)
     |> distinct(true)
     |> DB.Repo.all()
   end
diff --git a/apps/transport/lib/transport_web/templates/backoffice/contact/index.html.heex b/apps/transport/lib/transport_web/templates/backoffice/contact/index.html.heex
index d49c417bc7..abd2e33369 100644
--- a/apps/transport/lib/transport_web/templates/backoffice/contact/index.html.heex
+++ b/apps/transport/lib/transport_web/templates/backoffice/contact/index.html.heex
@@ -5,12 +5,14 @@
     <%= search_input(f, :q,
       id: "backoffice_search",
       value: assigns[:q] || "",
-      list: "existing_organizations",
+      list: "search_datalist",
       placeholder: dgettext("contact", "Find contacts")
     ) %>
-    <datalist id="existing_organizations">
-      <%= for org <- @existing_organizations do %>
-        <option value={org} />
+    <datalist id="search_datalist">
+      <%= for value <- @search_datalist do %>
+        <% unaccent_value = unaccent(value) %>
+        <option value={value} />
+        <option :if={value != unaccent_value} value={value}><%= unaccent_value %></option>
       <% end %>
     </datalist>
     <button type="submit" class="button backoffice_search_button"><i class="fa fa-search"></i></button>
diff --git a/apps/transport/lib/transport_web/templates/backoffice/page/form_dataset.html.heex b/apps/transport/lib/transport_web/templates/backoffice/page/form_dataset.html.heex
index 6838d71817..acba766718 100644
--- a/apps/transport/lib/transport_web/templates/backoffice/page/form_dataset.html.heex
+++ b/apps/transport/lib/transport_web/templates/backoffice/page/form_dataset.html.heex
@@ -90,7 +90,12 @@
         <% end %>
         <datalist id="contacts_datalist">
           <%= for contact <- @contacts_datalist do %>
-            <option value={contact.id}><%= DB.Contact.display_name(contact) %> (<%= contact.organization %>)</option>
+            <% display_name = DB.Contact.display_name(contact) %>
+            <% unaccent_display_name = unaccent(display_name) %>
+            <option value={contact.id}><%= display_name %> (<%= contact.organization %>)</option>
+            <option :if={display_name != unaccent_display_name} value={contact.id}>
+              <%= unaccent_display_name %> (<%= contact.organization %>)
+            </option>
           <% end %>
         </datalist>
         <%= label f, :reasons, class: "pt-12" do %>
diff --git a/apps/transport/lib/transport_web/views/backoffice/contact_view.ex b/apps/transport/lib/transport_web/views/backoffice/contact_view.ex
index 021bc1a8af..26528dd40d 100644
--- a/apps/transport/lib/transport_web/views/backoffice/contact_view.ex
+++ b/apps/transport/lib/transport_web/views/backoffice/contact_view.ex
@@ -2,6 +2,7 @@ defmodule TransportWeb.Backoffice.ContactView do
   use TransportWeb, :view
   import Shared.DateTimeDisplay, only: [format_datetime_to_paris: 2]
   import TransportWeb.BreadCrumbs, only: [breadcrumbs: 1]
+  import TransportWeb.Backoffice.PageView, only: [unaccent: 1]
   alias TransportWeb.PaginationHelpers
 
   def pagination_links(conn, contacts) do
diff --git a/apps/transport/lib/transport_web/views/backoffice/page_view.ex b/apps/transport/lib/transport_web/views/backoffice/page_view.ex
index f713e1e3ac..f27bb30cfb 100644
--- a/apps/transport/lib/transport_web/views/backoffice/page_view.ex
+++ b/apps/transport/lib/transport_web/views/backoffice/page_view.ex
@@ -64,6 +64,20 @@ defmodule TransportWeb.Backoffice.PageView do
     end
   end
 
+  @doc """
+  Replaces accented letters by their regular versions.
+
+  From https://stackoverflow.com/a/68724296
+
+  iex> unaccent("Et Ça sera sa moitié.")
+  "Et Ca sera sa moitie."
+  """
+  def unaccent(value) do
+    ~R<\p{Mn}>u
+    |> Regex.replace(value |> :unicode.characters_to_nfd_binary(), "")
+    |> :unicode.characters_to_nfc_binary()
+  end
+
   def notification_subscription_contact(%DB.NotificationSubscription{contact: %DB.Contact{} = contact}) do
     "#{DB.Contact.display_name(contact)} — #{contact.job_title} (#{contact.organization})"
   end
diff --git a/apps/transport/test/db/contact_test.exs b/apps/transport/test/db/contact_test.exs
index 54b74b9d95..7adfe109eb 100644
--- a/apps/transport/test/db/contact_test.exs
+++ b/apps/transport/test/db/contact_test.exs
@@ -151,13 +151,14 @@ defmodule DB.ContactTest do
   end
 
   test "search" do
-    search_fn = fn args -> args |> DB.Contact.search() |> DB.Repo.all() end
+    search_fn = fn args -> args |> DB.Contact.search() |> order_by([contact: c], asc: c.id) |> DB.Repo.all() end
     assert search_fn.(%{}) == []
 
     DB.Contact.insert!(%{sample_contact_args() | last_name: "Doe", organization: "Big Corp Inc"})
     DB.Contact.insert!(%{sample_contact_args() | last_name: "Bar", organization: "Big Corp Inc"})
     DB.Contact.insert!(%{sample_contact_args() | last_name: "Baz", organization: "Foo Bar"})
     DB.Contact.insert!(%{sample_contact_args() | first_name: "Marina", last_name: "Loiseau", organization: "CNRS"})
+    DB.Contact.insert!(%{sample_contact_args() | first_name: "Fabrice", last_name: "Mélo", organization: "CNRS"})
 
     DB.Contact.insert!(%{
       sample_contact_args()
@@ -169,10 +170,12 @@ defmodule DB.ContactTest do
 
     assert [%DB.Contact{last_name: "Doe"}] = search_fn.(%{"q" => "DOE"})
     assert [%DB.Contact{last_name: "Doe"}] = search_fn.(%{"q" => "doe"})
-    assert [%DB.Contact{last_name: "Bar"}] = search_fn.(%{"q" => "bar"})
+    assert [%DB.Contact{last_name: "Bar"}, %DB.Contact{organization: "Foo Bar"}] = search_fn.(%{"q" => "bar"})
     assert [%DB.Contact{organization: "Foo Bar"}] = search_fn.(%{"q" => "Foo Bar"})
     assert [%DB.Contact{mailing_list_title: "Service SIG"}] = search_fn.(%{"q" => "SIG"})
     assert [%DB.Contact{first_name: "Marina"}] = search_fn.(%{"q" => "marina"})
+    assert [%DB.Contact{last_name: "Mélo"}] = search_fn.(%{"q" => "mel"})
+    assert [%DB.Contact{last_name: "Mélo"}] = search_fn.(%{"q" => "mél"})
   end
 
   test "organisations" do
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 04024b2de6..c5e562e598 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
@@ -25,8 +25,9 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do
         |> get(backoffice_contact_path(conn, :index))
         |> html_response(200)
 
-      assert content =~ "Foo"
-      assert content =~ "Bar"
+      table_content = content |> Floki.parse_document!() |> Floki.find("table") |> Floki.text()
+      assert table_content =~ "Foo"
+      assert table_content =~ "Bar"
 
       content =
         conn
@@ -34,8 +35,9 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do
         |> get(backoffice_contact_path(conn, :index, %{"q" => "foo"}))
         |> html_response(200)
 
-      assert content =~ "Foo"
-      refute content =~ "Bar"
+      table_content = content |> Floki.parse_document!() |> Floki.find("table") |> Floki.text()
+      assert table_content =~ "Foo"
+      refute table_content =~ "Bar"
     end
   end
 
diff --git a/apps/transport/test/transport_web/views/backoffice/page_view_test.exs b/apps/transport/test/transport_web/views/backoffice/page_view_test.exs
new file mode 100644
index 0000000000..fd150ed68a
--- /dev/null
+++ b/apps/transport/test/transport_web/views/backoffice/page_view_test.exs
@@ -0,0 +1,4 @@
+defmodule TransportWeb.Backoffice.PageViewTest do
+  use ExUnit.Case, async: true
+  doctest TransportWeb.Backoffice.PageView, import: true
+end