From 02e7da098304e790b4f277c562f0f4568b5feae7 Mon Sep 17 00:00:00 2001 From: Vincent Degove Date: Mon, 11 Dec 2023 16:51:44 +0100 Subject: [PATCH] =?UTF-8?q?R=C3=A9pare=20la=20page=20AOMs=20(#3654)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * dirty fix * still working with a lot of dbg * Revert "dirty fix" This reverts commit 62ef8fb8b13341d3abafcce55120478b29223f8e. * it works but… * remove comments * refactor splitting in subfuctions * add tests * Apply suggestions from code review Co-authored-by: Antoine Augusti * add tests * change functions * add specs * use a DB.Resource builtin function --------- Co-authored-by: Antoine Augusti --- .../lib/transport_web/controllers/aoms.ex | 86 ++++++++++++++----- .../controllers/aoms_controller_test.exs | 29 +++++++ 2 files changed, 95 insertions(+), 20 deletions(-) diff --git a/apps/transport/lib/transport_web/controllers/aoms.ex b/apps/transport/lib/transport_web/controllers/aoms.ex index fe09134d2f..1589f6856f 100644 --- a/apps/transport/lib/transport_web/controllers/aoms.ex +++ b/apps/transport/lib/transport_web/controllers/aoms.ex @@ -17,12 +17,22 @@ defmodule TransportWeb.AOMSController do :population ] + @type dataset :: %{ + required(:aom_id) => integer(), + required(:dataset_id) => integer(), + required(:end_date) => Date.t(), + required(:has_realtime) => boolean() + } + + @type aom_id :: integer() + @type dataset_id :: integer() + @spec index(Plug.Conn.t(), map()) :: Plug.Conn.t() def index(conn, _params), do: render(conn, "index.html", aoms: aoms()) @spec prepare_aom({AOM.t(), binary()}, list(), list()) :: map() - defp prepare_aom({aom, nom_commune}, datasets, aggregated_datasets) do - all_datasets = datasets ++ aggregated_datasets + defp prepare_aom({aom, nom_commune}, gtfs_datasets, aggregated_datasets) do + all_datasets = gtfs_datasets ++ aggregated_datasets datasets_up_to_date = all_datasets @@ -56,7 +66,37 @@ defmodule TransportWeb.AOMSController do @spec aoms :: [map()] def aoms do - datasets = + # Let’s fetch all GTFS datasets. + # This doesn’t include GTFS-Flex, although they are in public-transit and have a GTFS format on resource. + {gtfs_datasets_by_aom_id, gtfs_dataset_by_dataset_id} = gtfs_datasets() + + # Some AOM data is present in aggregated datasets: the region publishes on behalf of the AOM. + # In this case, there are at least 2 legal owners on the dataset + aggregated_datasets_by_aom_id = aggregated_datasets(gtfs_dataset_by_dataset_id) + + aoms_and_commune_principale = aom_and_commune_principale() + + aoms_and_commune_principale + |> Enum.map(fn {aom, nom_commune} -> + prepare_aom( + {aom, nom_commune}, + Map.get(gtfs_datasets_by_aom_id, aom.id, []), + Map.get(aggregated_datasets_by_aom_id, aom.id, []) + ) + end) + end + + @spec csv_content() :: binary() + defp csv_content do + aoms() + |> CSV.encode(headers: @csv_headers) + |> Enum.to_list() + |> to_string + end + + @spec gtfs_datasets() :: {%{required(aom_id) => [dataset]}, %{required(dataset_id) => dataset}} + defp gtfs_datasets do + gtfs_datasets = Dataset.base_query() |> Dataset.join_from_dataset_to_metadata(Transport.Validators.GTFSTransport.validator_name()) |> join(:left, [dataset: d], aom in AOM, on: d.aom_id == aom.id, as: :aom) @@ -72,10 +112,14 @@ defmodule TransportWeb.AOMSController do ) |> Repo.all() - datasets_by_aom_id = datasets |> Enum.group_by(& &1.aom_id) - datasets_by_dataset_id = datasets |> Enum.group_by(& &1.dataset_id) + gtfs_datasets_by_aom_id = gtfs_datasets |> Enum.group_by(& &1.aom_id) + gtfs_dataset_by_dataset_id = gtfs_datasets |> Map.new(&{&1.dataset_id, &1}) + {gtfs_datasets_by_aom_id, gtfs_dataset_by_dataset_id} + end - aggregated_datasets = + @spec aggregated_datasets(%{required(dataset_id) => dataset}) :: %{required(aom_id) => [dataset]} + defp aggregated_datasets(gtfs_dataset_by_dataset_id) do + aggregated_datasets_in_db = AOM |> join(:inner, [aom], d in assoc(aom, :legal_owners_dataset), as: :legal_owners_dataset) |> where( @@ -83,32 +127,34 @@ defmodule TransportWeb.AOMSController do d.id in subquery( Dataset.base_query() |> join(:inner, [dataset: d], aom in assoc(d, :legal_owners_aom), as: :aom) + |> DB.Resource.join_dataset_with_resource() |> group_by([dataset: d], d.id) |> having([aom: a], count(a.id) >= 2) + |> where([resource: r], r.format in ["GTFS", "NeTEx"]) |> select([dataset: d], d.id) ) ) |> select([aom, legal_owners_dataset: d], %{aom_id: aom.id, dataset_id: d.id}) |> Repo.all() - |> Enum.group_by(& &1.aom_id, fn %{dataset_id: dataset_id} -> - datasets_by_dataset_id |> Map.fetch!(dataset_id) |> hd() - end) + aggregated_datasets_in_db + |> Enum.group_by(& &1.aom_id, fn %{dataset_id: dataset_id} -> + Map.get( + # Let’s enrich aggregated datasets with the GTFS dataset metadata if we have it + gtfs_dataset_by_dataset_id, + dataset_id, + # In case of a NeTEx or GTFS-Flex dataset, we don’t have the end_date + # We could have the realtime info by redoing the SQL query behing aggregated_datasets_in_db + %{dataset_id: dataset_id, end_date: nil, has_realtime: false} + ) + end) + end + + defp aom_and_commune_principale do AOM |> join(:left, [aom], c in Commune, on: aom.insee_commune_principale == c.insee) |> preload([:region]) |> select([aom, commune], {aom, commune.nom}) |> Repo.all() - |> Enum.map(fn {aom, nom_commune} -> - prepare_aom({aom, nom_commune}, Map.get(datasets_by_aom_id, aom.id, []), Map.get(aggregated_datasets, aom.id, [])) - end) - end - - @spec csv_content() :: binary() - defp csv_content do - aoms() - |> CSV.encode(headers: @csv_headers) - |> Enum.to_list() - |> to_string end end diff --git a/apps/transport/test/transport_web/controllers/aoms_controller_test.exs b/apps/transport/test/transport_web/controllers/aoms_controller_test.exs index cde8892b2c..05af949f38 100644 --- a/apps/transport/test/transport_web/controllers/aoms_controller_test.exs +++ b/apps/transport/test/transport_web/controllers/aoms_controller_test.exs @@ -48,6 +48,35 @@ defmodule TransportWeb.AOMsControllerTest do } = TransportWeb.AOMSController.aoms() |> Enum.find(fn r -> r.nom == aom.nom end) end + test "Having an aggregated dataset without GTFS associated validations works" do + %DB.AOM{nom: nom_aom} = aom = insert(:aom, nom: "Super AOM 76") + aom2 = insert(:aom) + + dataset = + insert(:dataset, + region: insert(:region), + aom: nil, + legal_owners_aom: [aom, aom2], + is_active: true, + type: "public-transit", + has_realtime: true + ) + + assert is_nil(dataset.aom_id) + + insert(:resource, title: "GTFS-flex TAD", dataset: dataset, format: "GTFS") + + assert %{ + nom: ^nom_aom, + published: true, + in_aggregate: true, + # The controller only gets up to date information for GTFS datasets + up_to_date: false, + # This is a quirk that could be corrected with some effort + has_realtime: false + } = TransportWeb.AOMSController.aoms() |> Enum.find(fn r -> r.nom == aom.nom end) + end + test "displays AOM information with datasets" do aom = insert(:aom, nom: "aom") # insert 2 datasets, one is outdated