From a73c30801053414685751c508a99ce66329438ec Mon Sep 17 00:00:00 2001 From: Antoine Augusti Date: Tue, 13 Aug 2024 12:17:53 +0200 Subject: [PATCH] =?UTF-8?q?R=C3=A9sultats=20de=20recherche=20:=20uniformis?= =?UTF-8?q?e=20titre=20de=20page=20(#4119)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Résultats de recherche : uniformise titre de page * HTTP-only version --- apps/transport/lib/db/region.ex | 2 + .../controllers/dataset_controller.ex | 47 ++++++++++++++----- .../controllers/page_controller.ex | 4 +- .../controllers/dataset_controller_test.exs | 37 +++++++++++++++ 4 files changed, 77 insertions(+), 13 deletions(-) diff --git a/apps/transport/lib/db/region.ex b/apps/transport/lib/db/region.ex index 5a8b8a6140..c2529b1ca8 100644 --- a/apps/transport/lib/db/region.ex +++ b/apps/transport/lib/db/region.ex @@ -20,4 +20,6 @@ defmodule DB.Region do has_many(:departements, Departement, foreign_key: :region_insee, references: :insee) has_one(:datasets, Dataset) end + + def national, do: DB.Repo.get_by!(DB.Region, nom: "National") end diff --git a/apps/transport/lib/transport_web/controllers/dataset_controller.ex b/apps/transport/lib/transport_web/controllers/dataset_controller.ex index 8fefac2069..bb793392fa 100644 --- a/apps/transport/lib/transport_web/controllers/dataset_controller.ex +++ b/apps/transport/lib/transport_web/controllers/dataset_controller.ex @@ -463,13 +463,22 @@ defmodule TransportWeb.DatasetController do end end - defp put_page_title(conn, %{"region" => id}), - do: + defp put_page_title(conn, %{"region" => region_id} = params) do + national_region = DB.Region.national() + + # For "region = (National) + modes[]=bus", which correspond to + # long distance coaches (Flixbus, BlaBlaBus etc.) we don't want + # to put the region name but instead "Long distance coaches" + if region_id == to_string(national_region.id) and Map.has_key?(params, "modes") do + put_page_title(conn, Map.delete(params, "region")) + else assign( conn, :page_title, - %{type: dgettext("page-shortlist", "region"), name: get_name(Region, id)} + %{type: dgettext("page-shortlist", "region"), name: get_name(Region, region_id)} ) + end + end defp put_page_title(conn, %{"insee_commune" => insee}) do name = Repo.get_by!(Commune, insee: insee).nom @@ -489,15 +498,31 @@ defmodule TransportWeb.DatasetController do %{type: "AOM", name: get_name(AOM, id)} ) - defp put_page_title(conn, %{"type" => t} = f) when map_size(f) == 1, - do: - assign( - conn, - :page_title, - %{type: dgettext("page-shortlist", "category"), name: Dataset.type_to_str(t)} - ) + defp put_page_title(%Plug.Conn{query_params: query_params} = conn, _) do + TransportWeb.PageController.home_tiles(conn) + # Allows to match `?type=foo&filter=has_realtime` otherwise + # `?type=foo` would match and we would not consider + # other options. + |> Enum.sort_by(&String.length(&1.link), :desc) + |> Enum.find(&tile_matches_query?(&1, MapSet.new(query_params))) + |> case do + %TransportWeb.PageController.Tile{title: title} -> + assign( + conn, + :page_title, + %{type: dgettext("page-shortlist", "category"), name: title} + ) - defp put_page_title(conn, _), do: conn + _ -> + conn + end + end + + defp tile_matches_query?(%TransportWeb.PageController.Tile{link: link}, %MapSet{} = query_params) do + tile_query = link |> URI.new!() |> Map.fetch!(:query) |> Plug.Conn.Query.decode() + + MapSet.subset?(MapSet.new(tile_query), query_params) + end defp put_dataset_heart_values(%Plug.Conn{assigns: %{current_user: current_user}} = conn, datasets) do if is_nil(current_user) do diff --git a/apps/transport/lib/transport_web/controllers/page_controller.ex b/apps/transport/lib/transport_web/controllers/page_controller.ex index 0e0fd65fa5..1f5034150a 100644 --- a/apps/transport/lib/transport_web/controllers/page_controller.ex +++ b/apps/transport/lib/transport_web/controllers/page_controller.ex @@ -235,7 +235,7 @@ defmodule TransportWeb.PageController do defstruct [:link, :icon, :title, :count, :type, :documentation_url] end - defp home_tiles(conn) do + def home_tiles(conn) do counts = home_index_stats() [ @@ -247,7 +247,7 @@ defmodule TransportWeb.PageController do count: Keyword.fetch!(counts, :count_public_transport_has_realtime) }, %Tile{ - # 14 is the region « national » We defined coaches as buses not bound to a region or AOM + # 14 is the region « National » We defined coaches as buses not bound to a region or AOM link: dataset_path(conn, :by_region, 14, "modes[]": "bus"), icon: icon_type_path("long-distance-coach"), title: dgettext("page-index", "Long distance coach"), diff --git a/apps/transport/test/transport_web/controllers/dataset_controller_test.exs b/apps/transport/test/transport_web/controllers/dataset_controller_test.exs index 91347d6492..b4fd06e5cf 100644 --- a/apps/transport/test/transport_web/controllers/dataset_controller_test.exs +++ b/apps/transport/test/transport_web/controllers/dataset_controller_test.exs @@ -779,6 +779,43 @@ defmodule TransportWeb.DatasetControllerTest do assert content =~ "Ce jeu de données est masqué" end + test "dataset-page-title", %{conn: conn} do + [ + {%{"type" => "public-transit"}, "Transport public collectif - horaires théoriques"}, + {%{"type" => "public-transit", "filter" => "has_realtime"}, "Transport public collectif - horaires temps réel"}, + {%{"modes" => ["rail"]}, "Transport ferroviaire"}, + {%{"custom_tag" => "paris2024"}, "JOP Paris 2024"} + ] + |> Enum.each(fn {params, expected_title} -> + title = + conn + |> get(dataset_path(conn, :index, params)) + |> html_response(200) + |> dataset_page_title() + + assert title == expected_title + end) + end + + test "dataset page title for long distance coaches", %{conn: conn} do + national_region = DB.Repo.get_by!(DB.Region, nom: "National") + + title = + conn + |> get(dataset_path(conn, :by_region, national_region.id, %{"modes" => ["bus"]})) + |> html_response(200) + |> dataset_page_title() + + assert title == "Autocars longue distance" + end + + defp dataset_page_title(content) do + content + |> Floki.parse_document!() + |> Floki.find(".dataset-page-title h1") + |> Floki.text() + end + defp mock_empty_history_resources do Transport.History.Fetcher.Mock |> expect(:history_resources, fn _, options ->