Affichage des responsables de la donnée (#4230)

* Remove unused get_covered_area function * Display legal owners in dataset GUI * transform into links * Regroup spatial coverage information in side column * Add legal owners to API * Improve schema * mix format on schema * Remove clauses that can’t be reached * remove forgotten puts debug * Use DB.Dataset Co-authored-by: Antoine Augusti <[email protected]> * Better warning for legal responsibility of the data * Add tests for dataset#details * Improve view functions * mix format * mix format yet again --------- Co-authored-by: Antoine Augusti <[email protected]>
etalab · Oct 28, 2024 · 1be5a21 · 1be5a21
1 parent c610ba8
commit 1be5a21
Show file tree

Hide file tree

Showing 11 changed files with 207 additions and 80 deletions.
diff --git a/apps/transport/lib/db/dataset.ex b/apps/transport/lib/db/dataset.ex
@@ -677,7 +677,14 @@ defmodule DB.Dataset do
   def get_by_slug(slug) do
     preload_without_validations()
     |> where(slug: ^slug)
-    |> preload([:region, :aom, :communes, resources: [:resources_related, :dataset]])
+    |> preload([
+      :aom,
+      :communes,
+      :region,
+      :legal_owners_aom,
+      :legal_owners_region,
+      resources: [:resources_related, :dataset]
+    ])
     |> Repo.one()
     |> case do
       nil -> {:error, "Dataset with slug #{slug} not found"}
@@ -745,47 +752,6 @@ defmodule DB.Dataset do
     end
   end
 
-  @spec get_covered_area_names(__MODULE__.t()) :: binary | [any]
-  def get_covered_area_names(%__MODULE__{aom_id: aom_id}) when not is_nil(aom_id) do
-    get_covered_area_names(
-      "select string_agg(nom, ', ' ORDER BY nom) from commune group by aom_res_id having aom_res_id = (select composition_res_id from aom where id = $1)",
-      aom_id
-    )
-  end
-
-  def get_covered_area_names(%__MODULE__{region_id: region_id}) when not is_nil(region_id) do
-    get_covered_area_names(
-      "select string_agg(distinct(departement), ', ') from aom where region_id = $1",
-      region_id
-    )
-  end
-
-  def get_covered_area_names(%__MODULE__{communes: communes}) when length(communes) != 0 do
-    communes
-    |> Enum.map(fn c -> c.nom end)
-    # credo:disable-for-next-line
-    |> Enum.join(", ")
-  end
-
-  def get_covered_area_names(_), do: "National"
-
-  @spec get_covered_area_names(binary, binary()) :: [binary()]
-  def get_covered_area_names(query, id) do
-    query
-    |> Repo.query([id])
-    |> case do
-      {:ok, %{rows: [names | _]}} ->
-        Enum.reject(names, &(&1 == nil))
-
-      {:ok, %{rows: []}} ->
-        ""
-
-      {:error, error} ->
-        Logger.error(error)
-        ""
-    end
-  end
-
   @spec official_resources(__MODULE__.t()) :: list(Resource.t())
   def official_resources(%__MODULE__{resources: resources}),
     do: resources |> Stream.reject(&DB.Resource.community_resource?/1) |> Enum.to_list()

diff --git a/apps/transport/lib/transport_web/api/controllers/datasets_controller.ex b/apps/transport/lib/transport_web/api/controllers/datasets_controller.ex
@@ -79,7 +79,7 @@ defmodule TransportWeb.API.DatasetController do
   def by_id(%Plug.Conn{} = conn, %{"id" => datagouv_id}) do
     dataset =
       Dataset
-      |> preload([:resources, :aom, :region, :communes])
+      |> preload([:resources, :aom, :region, :communes, :legal_owners_aom, :legal_owners_region])
       |> Repo.get_by(datagouv_id: datagouv_id)
 
     if is_nil(dataset) do
@@ -167,9 +167,10 @@ defmodule TransportWeb.API.DatasetController do
       "updated" => Helpers.last_updated(Dataset.official_resources(dataset)),
       "resources" => Enum.map(dataset.resources, &transform_resource/1),
       "community_resources" => Enum.map(Dataset.community_resources(dataset), &transform_resource/1),
-      # DEPRECATED, only there for retrocompatibility, use covered_area instead
+      # DEPRECATED, only there for retrocompatibility, use covered_area and legal owners instead
       "aom" => transform_aom(dataset.aom),
       "covered_area" => covered_area(dataset),
+      "legal_owners" => legal_owners(dataset),
       "type" => dataset.type,
       "licence" => dataset.licence,
       "publisher" => get_publisher(dataset)
@@ -298,8 +299,8 @@ defmodule TransportWeb.API.DatasetController do
   defp covered_area(%Dataset{region: %{id: 14}}),
     do: %{"type" => "country", "name" => "France", "country" => %{"name" => "France"}}
 
-  defp covered_area(%Dataset{region: %{nom: nom}}),
-    do: %{"type" => "region", "name" => nom, "region" => %{"name" => nom}}
+  defp covered_area(%Dataset{region: %{nom: nom, insee: insee}}),
+    do: %{"type" => "region", "name" => nom, "region" => %{"name" => nom, "insee" => insee}}
 
   defp covered_area(%Dataset{communes: [_ | _] = c, associated_territory_name: nom}),
     do: %{"type" => "cities", "name" => nom, "cities" => transform_cities(c)}
@@ -313,6 +314,24 @@ defmodule TransportWeb.API.DatasetController do
     |> Enum.map(fn c -> %{"name" => c.nom, "insee" => c.insee} end)
   end
 
+  defp legal_owners(dataset) do
+    %{
+      "aoms" => legal_owners_aom(dataset.legal_owners_aom),
+      "regions" => legal_owners_region(dataset.legal_owners_region),
+      "company" => dataset.legal_owner_company_siren
+    }
+  end
+
+  defp legal_owners_aom(aoms) do
+    aoms
+    |> Enum.map(fn aom -> %{"name" => aom.nom, "siren" => aom.siren} end)
+  end
+
+  defp legal_owners_region(regions) do
+    regions
+    |> Enum.map(fn region -> %{"name" => region.nom, "insee" => region.insee} end)
+  end
+
   defp prepare_datasets_index_data(%Plug.Conn{} = conn) do
     datasets_with_gtfs_metadata =
       DB.Dataset.base_query()
@@ -355,7 +374,7 @@ defmodule TransportWeb.API.DatasetController do
 
     %{}
     |> Dataset.list_datasets()
-    |> preload([:resources, :aom, :region, :communes])
+    |> preload([:resources, :aom, :region, :communes, :legal_owners_aom, :legal_owners_region])
     |> Repo.all()
     |> Enum.map(fn dataset ->
       enriched_dataset = Map.get(existing_ids, dataset.id)

diff --git a/apps/transport/lib/transport_web/api/schemas.ex b/apps/transport/lib/transport_web/api/schemas.ex
@@ -293,6 +293,57 @@ defmodule TransportWeb.API.Schemas do
     })
   end
 
+  defmodule AOM do
+    @moduledoc false
+    require OpenApiSpex
+
+    OpenApiSpex.schema(%{
+      title: "AOM",
+      description: "AOM object, as used in covered area and legal owners",
+      type: :object,
+      required: [:name, :siren],
+      properties: %{
+        name: %Schema{type: :string},
+        siren: %Schema{type: :string}
+      },
+      additionalProperties: false
+    })
+  end
+
+  defmodule Region do
+    @moduledoc false
+    require OpenApiSpex
+
+    OpenApiSpex.schema(%{
+      title: "Region",
+      description: "Region object",
+      type: :object,
+      required: [:name, :insee],
+      properties: %{
+        name: %Schema{type: :string},
+        insee: %Schema{type: :string}
+      },
+      additionalProperties: false
+    })
+  end
+
+  defmodule City do
+    @moduledoc false
+    require OpenApiSpex
+
+    OpenApiSpex.schema(%{
+      title: "City",
+      description: "City object",
+      type: :object,
+      required: [:name, :insee],
+      properties: %{
+        name: %Schema{type: :string},
+        insee: %Schema{type: :string}
+      },
+      additionalProperties: false
+    })
+  end
+
   defmodule CoveredArea.Country do
     @moduledoc false
     require OpenApiSpex
@@ -337,15 +388,7 @@ defmodule TransportWeb.API.Schemas do
       properties: %{
         name: %Schema{type: :string},
         type: %Schema{type: :string, enum: ["aom"], required: true},
-        aom: %Schema{
-          type: :object,
-          required: [:name, :siren],
-          properties: %{
-            name: %Schema{type: :string},
-            siren: %Schema{type: :string}
-          },
-          additionalProperties: false
-        }
+        aom: AOM.schema()
       },
       additionalProperties: false
     })
@@ -368,15 +411,7 @@ defmodule TransportWeb.API.Schemas do
         type: %Schema{type: :string, enum: ["cities"], required: true},
         cities: %Schema{
           type: :array,
-          items: %Schema{
-            type: :object,
-            required: [:name, :insee],
-            properties: %{
-              name: %Schema{type: :string},
-              insee: %Schema{type: :string}
-            },
-            additionalProperties: false
-          }
+          items: City.schema()
         }
       },
       additionalProperties: false
@@ -398,14 +433,7 @@ defmodule TransportWeb.API.Schemas do
       properties: %{
         name: %Schema{type: :string},
         type: %Schema{type: :string, enum: ["region"], required: true},
-        region: %Schema{
-          type: :object,
-          required: [:name],
-          properties: %{
-            name: %Schema{type: :string}
-          },
-          additionalProperties: false
-        }
+        region: Region.schema()
       },
       additionalProperties: false
     })
@@ -428,6 +456,28 @@ defmodule TransportWeb.API.Schemas do
     })
   end
 
+  defmodule LegalOwners do
+    @moduledoc false
+    require OpenApiSpex
+
+    OpenApiSpex.schema(%{
+      title: "LegalOwners",
+      type: :object,
+      properties: %{
+        aoms: %Schema{
+          type: :array,
+          items: AOM.schema()
+        },
+        regions: %Schema{
+          type: :array,
+          items: Region.schema()
+        },
+        company: %Schema{type: :string, nullable: true}
+      },
+      additionalProperties: false
+    })
+  end
+
   defmodule GeoJSONResponse do
     @moduledoc false
     require OpenApiSpex
@@ -749,7 +799,8 @@ defmodule TransportWeb.API.Schemas do
           description: "All the community resources (published by the community) associated with the dataset",
           items: CommunityResource
         },
-        covered_area: CoveredArea.schema()
+        covered_area: CoveredArea.schema(),
+        legal_owners: LegalOwners.schema()
       }
 
       if details do

diff --git a/apps/transport/lib/transport_web/templates/dataset/details.html.heex b/apps/transport/lib/transport_web/templates/dataset/details.html.heex
@@ -247,17 +247,24 @@
       <div class="dataset__logo">
         <%= img_tag(DB.Dataset.full_logo(@dataset), alt: @dataset.custom_title) %>
       </div>
-      <div>
-        <i class="icon fa fa-map-marker-alt" /><%= Dataset.get_territory_or_nil(@dataset) %>
-      </div>
       <div class="pt-12">
         <span class="dataset-metas-info-title"><%= dgettext("page-dataset-details", "Data published by") %></span>
-        <div class="dataset-type-info">
+        <div>
           <b>
             <%= link(@dataset.organization, to: dataset_path(@conn, :index, organization_id: @dataset.organization_id)) %>
           </b>
         </div>
       </div>
+      <%= if Enum.count(@dataset.legal_owners_aom) > 0 || Enum.count(@dataset.legal_owners_region) > 0 do %>
+        <div class="pt-12">
+          <span class="dataset-metas-info-title">
+            <%= dgettext("page-dataset-details", "Data under the responsibility of") %>
+          </span>
+          <div>
+            <%= legal_owners_links(@conn, @dataset) %>
+          </div>
+        </div>
+      <% end %>
       <div class="pt-12">
         <span class="dataset-metas-info-title"><%= dgettext("page-dataset-details", "Data type") %></span>
         <%= render("_dataset_type.html",
@@ -303,6 +310,9 @@
       </div>
       <div class="pt-12">
         <span class="dataset-metas-info-title"><%= dgettext("page-dataset-details", "Covered area") %></span>
+        <div>
+          <%= DB.Dataset.get_territory_or_nil(@dataset) %>
+        </div>
         <div id="dataset-covered-area-map"></div>
       </div>
     </div>

diff --git a/apps/transport/lib/transport_web/views/dataset_view.ex b/apps/transport/lib/transport_web/views/dataset_view.ex
@@ -104,6 +104,7 @@ defmodule TransportWeb.DatasetView do
   def region_link(conn, %{nom: nom, count: count, id: id}) do
     url =
       case id do
+        # This is for the "All" region
         nil -> dataset_path(conn, :index)
         _ -> dataset_path(conn, :by_region, id)
       end
@@ -119,6 +120,24 @@ defmodule TransportWeb.DatasetView do
     end
   end
 
+  def legal_owners_links(conn, %DB.Dataset{legal_owners_aom: legal_owners_aom, legal_owners_region: legal_owners_region}) do
+    legal_owners_region
+    |> Enum.sort_by(& &1.nom)
+    |> Enum.concat(legal_owners_aom |> Enum.sort_by(& &1.nom))
+    |> Enum.map_join(", ", fn owner ->
+      conn |> legal_owner_link(owner) |> safe_to_string()
+    end)
+    |> raw()
+  end
+
+  def legal_owner_link(conn, %DB.Region{nom: nom, id: id}) do
+    link(nom, to: dataset_path(conn, :by_region, id))
+  end
+
+  def legal_owner_link(conn, %DB.AOM{nom: nom, id: id}) do
+    link(nom, to: dataset_path(conn, :by_aom, id))
+  end
+
   def type_link(conn, %{type: type, msg: msg, count: count}) do
     full_url =
       case type do

diff --git a/apps/transport/priv/gettext/en/LC_MESSAGES/page-dataset-details.po b/apps/transport/priv/gettext/en/LC_MESSAGES/page-dataset-details.po
@@ -713,3 +713,7 @@ msgstr ""
 #, elixir-autogen, elixir-format
 msgid "The producer requires authentication to access the data. Consequently, some features on transport.data.gouv.fr, such as data availability, validations, and metadata, are unavailable for this dataset. Please follow the producer's instructions to gain access to the data."
 msgstr ""
+
+#, elixir-autogen, elixir-format
+msgid "Data under the responsibility of"
+msgstr ""
diff --git a/apps/transport/priv/gettext/fr/LC_MESSAGES/page-dataset-details.po b/apps/transport/priv/gettext/fr/LC_MESSAGES/page-dataset-details.po
@@ -713,3 +713,7 @@ msgstr "Gérez les services liés à ce jeu de données"
 #, elixir-autogen, elixir-format
 msgid "The producer requires authentication to access the data. Consequently, some features on transport.data.gouv.fr, such as data availability, validations, and metadata, are unavailable for this dataset. Please follow the producer's instructions to gain access to the data."
 msgstr "Le producteur requiert une authentification pour accéder aux données. En conséquence, certaines fonctionnalités de transport.data.gouv.fr, comme la disponibilité des données, les rapports de validations et les métadonnées sont indisponibles pour ce jeu de données. Veuillez suivre les instructions du producteur pour obtenir l'accès aux données."
+
+#, elixir-autogen, elixir-format
+msgid "Data under the responsibility of"
+msgstr "Données sous la responsabilité de"
diff --git a/apps/transport/priv/gettext/page-dataset-details.pot b/apps/transport/priv/gettext/page-dataset-details.pot
@@ -713,3 +713,7 @@ msgstr ""
 #, elixir-autogen, elixir-format
 msgid "The producer requires authentication to access the data. Consequently, some features on transport.data.gouv.fr, such as data availability, validations, and metadata, are unavailable for this dataset. Please follow the producer's instructions to gain access to the data."
 msgstr ""
+
+#, elixir-autogen, elixir-format
+msgid "Data under the responsibility of"
+msgstr ""
diff --git a/apps/transport/test/support/database_case.ex b/apps/transport/test/support/database_case.ex
@@ -31,7 +31,7 @@ defmodule TransportWeb.DatabaseCase do
           Sandbox.mode(Repo, {:shared, self()})
         end
 
-        Repo.insert(%Region{nom: "Pays de la Loire"})
+        Repo.insert(%Region{nom: "Pays de la Loire", insee: "52"})
         Repo.insert(%Region{nom: "Auvergne-Rhône-Alpes"})
         Repo.insert(%Region{nom: "Île-de-France"})