Mise à jour vers Phoenix 1.7 (#3696)

* Upgrade to Phoenix 1.7

* Verified routes

* Use verified routes

* Fork scrivener_html to transportdatagouvfr

* Add support for removed deps

---------

Co-authored-by: Thibaut Barrère <thibaut.barrere@gmail.com>

apps/gbfs/mix.exs

@@ -11,7 +11,6 @@ defmodule GBFS.MixProject do
       lockfile: "../../mix.lock",
       elixir: "~> 1.8",
       elixirc_paths: elixirc_paths(Mix.env()),
-      compilers: [:phoenix] ++ Mix.compilers(),
       start_permanent: Mix.env() == :prod,
       deps: deps(),
       test_coverage: [tool: ExCoveralls],
@@ -38,7 +37,7 @@ defmodule GBFS.MixProject do
     [
       {:cachex, "~> 3.5"},
       {:httpoison, "~> 2.1"},
-      {:phoenix, "~> 1.6.2"},
+      {:phoenix, "~> 1.7.0"},
       {:sweet_xml, ">= 0.0.0"},
       {:jason, ">= 0.0.0"},
       {:cors_plug, "~> 3.0"},

apps/shared/mix.exs

@@ -46,7 +46,7 @@ defmodule Shared.MixProject do
       {:finch, "~> 0.8"},
       # Required for the ConditionalJSONEncoder shared component, but
       # there is probably a way to avoid that?
-      {:phoenix, "~> 1.6.2"},
+      {:phoenix, "~> 1.7.0"},
       # The global app config references Sentry.LoggerBackend. We add it in "shared"
       # as an implicit dependency, to ensure `Sentry.LoggerBackend` is always defined,
       # otherwise running tests for an individual umbrella sub-app will raise error.

apps/transport/client/yarn.lock

@@ -4041,7 +4041,7 @@ path-type@^4.0.0:
   integrity sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw==
 
 "phoenix@file:../../../deps/phoenix":
-  version "1.6.16"
+  version "1.7.10"
 
 "phoenix_html@file:../../../deps/phoenix_html":
   version "3.3.3"

apps/transport/lib/transport_web.ex

@@ -16,6 +16,8 @@ defmodule TransportWeb do
   below. Instead, define any helper function in modules
   and import those modules here.
   """
+  def static_paths,
+    do: ~w(js css fonts images data favicon.ico documents BingSiteAuth.xml google5be4b09db1274976.html demo_rt.html)
 
   def controller do
     quote do
@@ -26,6 +28,8 @@ defmodule TransportWeb do
       import TransportWeb.PaginationHelpers
       alias TransportWeb.ErrorView
       import Phoenix.LiveView.Controller
+
+      unquote(verified_routes())
     end
   end
 
@@ -54,6 +58,12 @@ defmodule TransportWeb do
     end
   end
 
+  def view_helpers do
+    quote do
+      unquote(verified_routes())
+    end
+  end
+
   def router do
     quote do
       use Phoenix.Router
@@ -78,6 +88,15 @@ defmodule TransportWeb do
     end
   end
 
+  def verified_routes do
+    quote do
+      use Phoenix.VerifiedRoutes,
+        endpoint: TransportWeb.Endpoint,
+        router: TransportWeb.Router,
+        statics: TransportWeb.static_paths()
+    end
+  end
+
   @doc """
   When used, dispatch to the appropriate controller/view/etc.
   """

apps/transport/lib/transport_web/endpoint.ex

@@ -22,7 +22,7 @@ defmodule TransportWeb.Endpoint do
     at: "/",
     from: :transport,
     gzip: Mix.env() == :prod,
-    only: ~w(js css fonts images data favicon.ico documents BingSiteAuth.xml google5be4b09db1274976.html demo_rt.html)
+    only: TransportWeb.static_paths()
   )
 
   # Code reloading can be explicitly enabled under the

apps/transport/lib/transport_web/router.ex

@@ -12,6 +12,7 @@ defmodule TransportWeb.Router do
     plug(:canonical_host)
     plug(:accepts, ["html"])
     plug(:fetch_session)
+    plug(:fetch_flash)
     plug(:fetch_live_flash)
     plug(:protect_from_forgery)
     plug(TransportWeb.Plugs.PutLocale)

apps/transport/lib/transport_web/templates/layout/_breaking_news.html.heex

@@ -1,7 +1,7 @@
-<div :if={get_flash(@conn, :breaking_news_info)} class="notification">
-  <%= @conn |> get_flash(:breaking_news_info) |> markdown_to_safe_html!() %>
+<div :if={Phoenix.Flash.get(@flash, :breaking_news_info)} class="notification">
+  <%= @flash |> Phoenix.Flash.get(:breaking_news_info) |> markdown_to_safe_html!() %>
 </div>
 
-<div :if={get_flash(@conn, :breaking_news_error)} class="notification message--error">
-  <%= @conn |> get_flash(:breaking_news_error) |> markdown_to_safe_html!() %>
+<div :if={Phoenix.Flash.get(@flash, :breaking_news_error)} class="notification message--error">
+  <%= @flash |> Phoenix.Flash.get(:breaking_news_error) |> markdown_to_safe_html!() %>
 </div>

apps/transport/lib/transport_web/templates/layout/app.html.heex

@@ -64,16 +64,16 @@
     <main class="layout-main">
       <%= if has_flash(@conn) do %>
         <%= render(LayoutView, "_breaking_news.html", assigns) %>
-        <%= if get_flash(@conn, :info) do %>
-          <p class="notification"><%= get_flash(@conn, :info) %></p>
+        <%= if Phoenix.Flash.get(@flash, :info) do %>
+          <p class="notification"><%= Phoenix.Flash.get(@flash, :info) %></p>
         <% end %>
-        <%= if get_flash(@conn, :errors) do %>
-          <%= for error <- get_flash(@conn, :errors) do %>
+        <%= if Phoenix.Flash.get(@flash, :errors) do %>
+          <%= for error <- Phoenix.Flash.get(@flash, :errors) do %>
             <p class="notification message--error"><%= error %></p>
           <% end %>
         <% end %>
-        <%= if get_flash(@conn, :error) do %>
-          <p class="notification message--error"><%= get_flash(@conn, :error) %></p>
+        <%= if Phoenix.Flash.get(@flash, :error) do %>
+          <p class="notification message--error"><%= Phoenix.Flash.get(@flash, :error) %></p>
         <% end %>
       <% end %>
 

apps/transport/lib/transport_web/views/layout_view.ex

@@ -8,11 +8,7 @@ defmodule TransportWeb.LayoutView do
     Controller.current_path(conn)
   end
 
-  def has_flash(conn) do
-    # it's not nice to depend on some internal state, but it does not seems to have a better way
-    # to check if the `put_flash` function has been called, and it's important for error pages
-    not is_nil(conn.private[:phoenix_flash])
-  end
+  def has_flash(%Plug.Conn{} = conn), do: not Enum.empty?(conn.assigns.flash)
 
   def add_locale_to_url(conn, locale) do
     query_params = conn.query_params |> Map.put("locale", locale) |> Plug.Conn.Query.encode()

apps/transport/mix.exs

@@ -60,12 +60,12 @@ defmodule Transport.Mixfile do
       {:earmark, "~> 1.4"},
       {:gettext, "~> 0.11"},
       {:httpoison, "~> 2.1"},
-      {:phoenix, "~> 1.6.2"},
+      {:phoenix, "~> 1.7.0"},
       {:phoenix_html, "~> 3.1"},
       # Compilation issue for this dependency, see https://github.com/etalab/transport-site/issues/3499
       {:phoenix_markdown, git: "https://github.com/pzingg/phoenix_markdown.git", ref: "b2e5ff67c9ce9160d7ef1f66d0c859dfa6284a53"},
       # Careful with the upgrade: https://github.com/etalab/transport-site/issues/3433
-      {:phoenix_live_view, "~> 0.18.0"},
+      {:phoenix_live_view, "~> 0.18.18"},
       {:html_sanitize_ex, "~> 1.4"},
       {:floki, ">= 0.0.0", only: :test},
       {:plug_cowboy, "~> 2.3"},
@@ -74,8 +74,10 @@ defmodule Transport.Mixfile do
       {:timex, "~> 3.7"},
       {:sentry, "~> 10.1"},
       {:scrivener, "~> 2.5"},
-      # Compilation issue for this dependency, see https://github.com/etalab/transport-site/issues/3499
-      {:scrivener_html, git: "https://github.com/etalab/scrivener_html.git", ref: "f0245703abf7d0ce2b48a0f7e96997def7649e5f"},
+      # `scrivener_html` seems to be unmaintained!
+      # - Compilation issue for this dependency, see https://github.com/etalab/transport-site/issues/3499
+      # - was not updated to support Phoenix 1.7
+      {:scrivener_html, git: "https://github.com/transportdatagouvfr/scrivener_html.git", ref: "d6ac5ac4c0c94fc871a42817817b6d5c7b5d6b0c"},
       {:scrivener_list, "~>2.0"},
       {:jason, "~> 1.1"},
       {:open_api_spex, "~> 3.8"},

apps/transport/test/support/conn_case.ex

@@ -31,6 +31,8 @@ defmodule TransportWeb.ConnCase do
 
       # The default endpoint for testing
       @endpoint TransportWeb.Endpoint
+
+      use TransportWeb, :verified_routes
     end
   end
 

apps/transport/test/transport_web/controllers/api/gtfs_stops_controller_test.exs

@@ -7,15 +7,15 @@ defmodule TransportWeb.API.GTFSStopsControllerTest do
   end
 
   test "GET /api/gtfs-stops without parameters", %{conn: conn} do
-    conn = conn |> get("/api/gtfs-stops")
+    conn = conn |> get(~p"/api/gtfs-stops")
     json = json_response(conn, 422)
     assert json["error"] == "incorrect parameters"
   end
 
   test "GET /api/gtfs-stops with full map parameters", %{conn: conn} do
     conn =
       conn
-      |> get("/api/gtfs-stops", %{
+      |> get(~p"/api/gtfs-stops", %{
         "south" => "48.8",
         "east" => "2.4",
         "west" => "2.2",
@@ -36,7 +36,7 @@ defmodule TransportWeb.API.GTFSStopsControllerTest do
 
     conn =
       conn
-      |> get("/api/gtfs-stops", %{
+      |> get(~p"/api/gtfs-stops", %{
         "south" => "43.5326204268101",
         "east" => "22.6318359375",
         "west" => "-18.764648437500004",
@@ -55,7 +55,7 @@ defmodule TransportWeb.API.GTFSStopsControllerTest do
 
     conn =
       conn
-      |> get("/api/gtfs-stops", %{
+      |> get(~p"/api/gtfs-stops", %{
         "south" => "48.8",
         "east" => "2.4",
         "west" => "2.2",

apps/transport/test/transport_web/controllers/backoffice/backoffice_controller_test.exs

@@ -47,7 +47,7 @@ defmodule TransportWeb.BackofficeControllerTest do
     target_uri = URI.parse(redirected_to(conn, 302))
     assert target_uri.path == "/login/explanation"
     assert target_uri.query == URI.encode_query(redirect_path: "/backoffice")
-    assert get_flash(conn, :info) =~ "Vous devez être préalablement connecté"
+    assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "Vous devez être préalablement connecté"
   end
 
   test "Check that you are an admin", %{conn: conn} do
@@ -57,7 +57,9 @@ defmodule TransportWeb.BackofficeControllerTest do
       |> get(backoffice_page_path(conn, :index))
 
     assert redirected_to(conn, 302) =~ "/login/explanation"
-    assert get_flash(conn, :error) =~ "You need to be a member of the transport.data.gouv.fr team."
+
+    assert Phoenix.Flash.get(conn.assigns.flash, :error) =~
+             "You need to be a member of the transport.data.gouv.fr team."
   end
 
   test "Show 'add new dataset' form", %{conn: conn} do
@@ -80,7 +82,7 @@ defmodule TransportWeb.BackofficeControllerTest do
     assert redirected_to(conn, 302) == backoffice_page_path(conn, :index)
     assert Resource |> Repo.all() |> length() == 0
 
-    assert get_flash(conn, :error) ==
+    assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
              "%{region: [\"Vous devez remplir soit une région soit une AOM soit utiliser les zones data.gouv\"]}"
 
     assert [] == all_enqueued()
@@ -103,7 +105,7 @@ defmodule TransportWeb.BackofficeControllerTest do
     assert redirected_to(conn, 302) == backoffice_page_path(conn, :index)
     assert Resource |> Repo.all() |> length() == 0
 
-    assert get_flash(conn, :error) ==
+    assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
              "%{region: [\"Vous devez remplir soit une région soit une AOM soit utiliser les zones data.gouv\"]}"
 
     assert [] == all_enqueued()
@@ -174,7 +176,7 @@ defmodule TransportWeb.BackofficeControllerTest do
     assert redirected_to(conn, 302) == backoffice_page_path(conn, :index)
     assert Resource |> where([r], not r.is_community_resource) |> Repo.all() |> length() == 1
     assert Resource |> where([r], r.is_community_resource) |> Repo.all() |> length() == 2
-    assert ["Dataset ajouté" | _] = get_flash(conn, :info)
+    assert ["Dataset ajouté" | _] = Phoenix.Flash.get(conn.assigns.flash, :info)
   end
 
   test "Add a dataset linked to aom", %{conn: conn} do
@@ -216,7 +218,7 @@ defmodule TransportWeb.BackofficeControllerTest do
 
     assert Resource |> where([r], not r.is_community_resource) |> Repo.all() |> length() == 1
     assert Resource |> where([r], r.is_community_resource) |> Repo.all() |> length() == 2
-    assert ["Dataset ajouté" | _] = get_flash(conn, :info)
+    assert ["Dataset ajouté" | _] = Phoenix.Flash.get(conn.assigns.flash, :info)
   end
 
   test "Add a dataset linked to cities", %{conn: conn} do
@@ -238,7 +240,7 @@ defmodule TransportWeb.BackofficeControllerTest do
 
     assert redirected_to(conn, 302) == backoffice_page_path(conn, :index)
     assert Resource |> Repo.all() |> length() == 1
-    assert ["Dataset ajouté" | _] = get_flash(conn, :info)
+    assert ["Dataset ajouté" | _] = Phoenix.Flash.get(conn.assigns.flash, :info)
   end
 
   test "Add a dataset linked to cities and to the country", %{conn: conn} do
@@ -264,7 +266,7 @@ defmodule TransportWeb.BackofficeControllerTest do
     assert logs =~ "Vous devez remplir soit une région soit une AOM"
     assert redirected_to(conn, 302) == backoffice_page_path(conn, :index)
     assert Resource |> Repo.all() |> length() == 0
-    flash = get_flash(conn, :error)
+    flash = Phoenix.Flash.get(conn.assigns.flash, :error)
 
     assert flash =~
              "Vous devez remplir soit une région soit une AOM soit utiliser les zones data.gouv"
@@ -293,7 +295,7 @@ defmodule TransportWeb.BackofficeControllerTest do
     # is empty (but not null since it comes from a form)
     assert redirected_to(conn, 302) == backoffice_page_path(conn, :index)
     assert Resource |> Repo.all() |> length() == 1
-    assert ["Dataset ajouté" | _] = conn |> get_flash(:info)
+    assert ["Dataset ajouté" | _] = Phoenix.Flash.get(conn.assigns.flash, :info)
     %Resource{id: resource_id} = Resource |> Repo.one!()
 
     assert [
@@ -330,7 +332,7 @@ defmodule TransportWeb.BackofficeControllerTest do
     # It should not be possible to link a dataset to either a region and to the whole country
     assert redirected_to(conn, 302) == backoffice_page_path(conn, :index)
     assert Resource |> Repo.all() |> length() == 0
-    flash = get_flash(conn, :error)
+    flash = Phoenix.Flash.get(conn.assigns.flash, :error)
     assert logs =~ "Un jeu de données ne peut pas être à la fois régional et national"
     assert flash =~ "Un jeu de données ne peut pas être à la fois régional et national"
   end

apps/transport/test/transport_web/controllers/backoffice/broken_urls_controller_test.exs

@@ -12,7 +12,7 @@ defmodule TransportWeb.Backoffice.BrokenUrlsControllerTest do
     target_uri = URI.parse(redirected_to(conn, 302))
     assert target_uri.path == "/login/explanation"
     assert target_uri.query == URI.encode_query(redirect_path: request_path)
-    assert get_flash(conn, :info) =~ "Vous devez être préalablement connecté"
+    assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "Vous devez être préalablement connecté"
   end
 
   test "detects a broken URL", %{conn: conn} do

apps/transport/test/transport_web/controllers/backoffice/contact_controller_test.exs

@@ -12,7 +12,7 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do
     target_uri = URI.parse(redirected_to(conn, 302))
     assert target_uri.path == "/login/explanation"
     assert target_uri.query == URI.encode_query(redirect_path: request_path)
-    assert get_flash(conn, :info) =~ "Vous devez être préalablement connecté"
+    assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "Vous devez être préalablement connecté"
   end
 
   describe "index" do
@@ -90,7 +90,7 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do
       assert %DB.Contact{first_name: "John", last_name: "Doe", email: "john@example.com", organization: "Corp Inc"} =
                DB.Repo.one!(DB.Contact)
 
-      assert get_flash(conn, :info) =~ "Contact mis à jour"
+      assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "Contact mis à jour"
     end
 
     test "redirects when there are errors", %{conn: conn} do
@@ -137,7 +137,7 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do
         |> post(backoffice_contact_path(conn, :create, %{"contact" => args}))
 
       assert redirected_to(conn, 302) == backoffice_contact_path(conn, :index)
-      assert get_flash(conn, :info) =~ "Contact mis à jour"
+      assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "Contact mis à jour"
       assert %DB.Contact{last_name: ^new_last_name} = DB.Repo.reload!(contact)
     end
 
@@ -151,7 +151,7 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do
         |> post(backoffice_contact_path(conn, :create, %{"contact" => %{"id" => contact.id, "email" => other_email}}))
 
       assert redirected_to(conn, 302) == backoffice_contact_path(conn, :index)
-      assert get_flash(conn, :error) =~ "Un contact existe déjà avec cette adresse e-mail"
+      assert Phoenix.Flash.get(conn.assigns.flash, :error) =~ "Un contact existe déjà avec cette adresse e-mail"
       assert %DB.Contact{email: ^email} = DB.Repo.reload!(contact)
     end
 
@@ -196,7 +196,7 @@ defmodule TransportWeb.Backoffice.ContactControllerTest do
       |> post(backoffice_contact_path(conn, :delete, contact.id))
 
     assert redirected_to(conn, 302) == backoffice_contact_path(conn, :index)
-    assert get_flash(conn, :info) =~ "Le contact a été supprimé"
+    assert Phoenix.Flash.get(conn.assigns.flash, :info) =~ "Le contact a été supprimé"
     assert is_nil(DB.Repo.reload(contact))
   end