ResourceUnavailableNotificationJob : améliore le contenu (#3316)

* Notification ressource indisponible : adapte contenu * I can template * Comments * Proper English is better * Update expected string * Mise à jour contenu * Update content Co-authored-by: Vincent Degove <[email protected]> * Remove extra space --------- Co-authored-by: Vincent Degove <[email protected]>
etalab · Jul 18, 2023 · 69d5931 · 69d5931
1 parent d1f174b
commit 69d5931
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 42 deletions.
diff --git a/apps/transport/lib/db/resource.ex b/apps/transport/lib/db/resource.ex
@@ -275,29 +275,40 @@ defmodule DB.Resource do
  end
  end
 
+ @doc """
+ iex> hosted_on_datagouv?(%DB.Resource{url: "https://static.data.gouv.fr/file.zip"})
+ true
+ iex> hosted_on_datagouv?("https://static.data.gouv.fr/file.zip")
+ true
+ iex> hosted_on_datagouv?(%DB.Resource{url: "https://example.com/file.zip"})
+ false
+ """
+ @spec hosted_on_datagouv?(__MODULE__.t() | binary()) :: boolean()
+ def hosted_on_datagouv?(url) when is_binary(url), do: hosted_on_datagouv?(%__MODULE__{url: url})
+
+ def hosted_on_datagouv?(%__MODULE__{url: url}) do
+ host = url |> URI.parse() |> Map.fetch!(:host)
+ Enum.member?(Application.fetch_env!(:transport, :datagouv_static_hosts), host)
+ end
+
  defp needs_stable_url?(%__MODULE__{latest_url: nil}), do: false
 
- defp needs_stable_url?(%__MODULE__{url: url}) do
+ defp needs_stable_url?(%__MODULE__{url: url} = resource) do
  parsed_url = URI.parse(url)
 
- hosted_on_static_datagouv =
- Enum.member?(Application.fetch_env!(:transport, :datagouv_static_hosts), parsed_url.host)
-
  object_storage_regex =
  ~r{(https://.*\.blob\.core\.windows\.net)|(https://.*\.s3\..*\.amazonaws\.com)|(https://.*\.s3\..*\.scw\.cloud)|(https://.*\.cellar-c2\.services\.clever-cloud\.com)|(https://s3\..*\.cloud\.ovh\.net)}
 
  hosted_on_bison_fute = parsed_url.host == Application.fetch_env!(:transport, :bison_fute_host)
 
  cond do
  hosted_on_bison_fute -> is_link_to_folder?(parsed_url)
- hosted_on_static_datagouv -> true
+ hosted_on_datagouv?(resource) -> true
  String.match?(url, object_storage_regex) -> true
  true -> false
  end
  end
 
- defp needs_stable_url?(%__MODULE__{}), do: false
-
  defp is_link_to_folder?(%URI{path: path}) do
  path |> Path.basename() |> :filename.extension() == ""
  end

diff --git a/apps/transport/lib/jobs/resource_unavailable_notification_job.ex b/apps/transport/lib/jobs/resource_unavailable_notification_job.ex
@@ -31,24 +31,13 @@ defmodule Transport.Jobs.ResourceUnavailableNotificationJob do
  email,
  Application.get_env(:transport, :contact_email),
  "Ressources indisponibles dans le jeu de données #{dataset.custom_title}",
- """
- Bonjour,
-
- Les ressources #{Enum.map_join(unavailabilities, ", ", &resource_title/1)} dans votre jeu de données #{dataset_url(dataset)} ne sont plus disponibles au téléchargement depuis plus de #{@hours_consecutive_downtime}h.
-
- Ces erreurs empêchent la réutilisation de vos données.
-
- Nous vous invitons à corriger l'accès de vos données dès que possible.
-
- Nous restons disponible pour vous accompagner si besoin.
-
- Merci par avance pour votre action,
-
- À bientôt,
-
- L'équipe du PAN
- """,
- ""
+ "",
+ Phoenix.View.render_to_string(TransportWeb.EmailView, "resource_unavailable.html",
+ dataset: dataset,
+ hours_consecutive_downtime: @hours_consecutive_downtime,
+ deleted_recreated_on_datagouv: deleted_and_recreated_resource_hosted_on_datagouv(dataset, unavailabilities),
+ resource_titles: Enum.map_join(unavailabilities, ", ", &resource_title/1)
+ )
  )
 
  save_notification(dataset, email)
@@ -68,6 +57,39 @@ defmodule Transport.Jobs.ResourceUnavailableNotificationJob do
  |> MapSet.new()
  end
 
+ @doc """
+ Detects when the producer deleted and recreated just after a resource hosted on data.gouv.fr.
+ Best practice: upload a new version of the file, keep the same datagouv's resource.
+
+ Detected if:
+ - a resource hosted on datagouv is unavailable (ie it was deleted)
+ - call the API now and see that a resource hosted on datagouv has been created recently
+ """
+ def deleted_and_recreated_resource_hosted_on_datagouv(%DB.Dataset{} = dataset, unavailabilities) do
+ hosted_on_datagouv = Enum.any?(unavailabilities, &DB.Resource.hosted_on_datagouv?(&1.resource))
+ hosted_on_datagouv and created_resource_hosted_on_datagouv_recently?(dataset)
+ end
+
+ def created_resource_hosted_on_datagouv_recently?(%DB.Dataset{datagouv_id: datagouv_id}) do
+ case Datagouvfr.Client.Datasets.get(datagouv_id) do
+ {:ok, %{"resources" => resources}} ->
+ dt_limit = DateTime.utc_now() |> DateTime.add(-12, :hour)
+
+ Enum.any?(resources, fn %{"created_at" => created_at, "url" => url} ->
+ is_recent = created_at |> parse_datetime() |> DateTime.compare(dt_limit) == :gt
+ is_recent and DB.Resource.hosted_on_datagouv?(url)
+ end)
+
+ _ ->
+ false
+ end
+ end
+
+ defp parse_datetime(value) do
+ {:ok, datetime, 0} = DateTime.from_iso8601(value)
+ datetime
+ end
+
  def relevant_unavailabilities(%DateTime{} = inserted_at) do
  datetime_lower_limit = inserted_at |> DateTime.add(-@hours_consecutive_downtime * 60 - 30, :minute)
  datetime_upper_limit = inserted_at |> DateTime.add(-@hours_consecutive_downtime, :hour)
@@ -90,12 +112,6 @@ defmodule Transport.Jobs.ResourceUnavailableNotificationJob do
  |> MapSet.new()
  end
 
- defp dataset_url(%DB.Dataset{slug: slug, custom_title: custom_title}) do
- url = TransportWeb.Router.Helpers.dataset_url(TransportWeb.Endpoint, :details, slug)
-
- "#{custom_title} — #{url}"
- end
-
  defp resource_title(%DB.ResourceUnavailability{resource: %DB.Resource{title: title}}) do
  title
  end

diff --git a/apps/transport/lib/transport_web/templates/email/resource_unavailable.html.md b/apps/transport/lib/transport_web/templates/email/resource_unavailable.html.md
@@ -0,0 +1,22 @@
+Bonjour,
+
+Les ressources <%= @resource_titles %> dans votre jeu de données <%= link_for_dataset(@dataset, :heex) %> ne sont plus disponibles au téléchargement depuis plus de <%= @hours_consecutive_downtime %>h.
+
+<%= if @deleted_recreated_on_datagouv do %>
+Il semble que vous ayez supprimé et créé une nouvelle ressource. Lors de la mise à jour de vos données, remplacez plutôt le fichier au sein de la ressource existante. Retrouvez la procédure pas à pas [sur notre documentation](https://doc.transport.data.gouv.fr/producteurs/mettre-a-jour-des-donnees).
+
+Pour corriger le problème pour cette fois-ci :
+1. Mettez à jour l’ancienne ressource avec les nouvelles données en [suivant notre documentation](https://doc.transport.data.gouv.fr/producteurs/mettre-a-jour-des-donnees) ;
+2. Supprimez la ressource nouvellement créée qui sera alors en doublon.
+
+<% else %>
+Ces erreurs provoquent des difficultés pour les réutilisateurs. Nous vous invitons à corriger l’accès de vos données dès que possible.
+<% end %>
+
+Nous restons disponible pour vous accompagner si besoin.
+
+Merci par avance pour votre action,
+
+À bientôt,
+
+L’équipe du PAN
diff --git a/apps/transport/test/transport/jobs/resource_unavailable_notification_job_test.exs b/apps/transport/test/transport/jobs/resource_unavailable_notification_job_test.exs
@@ -49,9 +49,23 @@ defmodule Transport.Test.Transport.Jobs.ResourceUnavailableNotificationJobTest d
  %{id: gtfs_dataset_id} =
  gtfs_dataset = insert(:dataset, slug: Ecto.UUID.generate(), is_active: true, custom_title: "Dataset GTFS")
 
- resource_1 = insert(:resource, dataset: dataset, format: "geojson", title: "GeoJSON 1")
- resource_2 = insert(:resource, dataset: dataset, format: "geojson", title: "GeoJSON 2")
- resource_gtfs = insert(:resource, dataset: gtfs_dataset, format: "GTFS")
+ resource_1 =
+ insert(:resource,
+ dataset: dataset,
+ format: "geojson",
+ title: "GeoJSON 1",
+ url: "https://static.data.gouv.fr/file.geojson"
+ )
+
+ resource_2 =
+ insert(:resource,
+ dataset: dataset,
+ format: "geojson",
+ title: "GeoJSON 2",
+ url: "https://static.data.gouv.fr/other_file.geojson"
+ )
+
+ resource_gtfs = insert(:resource, dataset: gtfs_dataset, format: "GTFS", url: "https://example/file.zip")
 
  insert(:resource_unavailability,
  start: DateTime.add(DateTime.utc_now(), -6 * 60 - 29, :minute),
@@ -83,6 +97,8 @@ defmodule Transport.Test.Transport.Jobs.ResourceUnavailableNotificationJobTest d
  |> Ecto.Changeset.change(%{inserted_at: DateTime.utc_now() |> DateTime.add(-20, :day)})
  |> DB.Repo.update!()
 
+ setup_dataset_response(dataset, resource_1.url, DateTime.utc_now() |> DateTime.add(-6, :hour))
+
  %DB.Contact{id: already_sent_contact_id} = insert_contact(%{email: already_sent_email})
  %DB.Contact{id: foo_contact_id} = insert_contact(%{email: "[email protected]"})
 
@@ -118,12 +134,15 @@ defmodule Transport.Test.Transport.Jobs.ResourceUnavailableNotificationJobTest d
  "[email protected]" = _to,
  "[email protected]",
  subject,
- plain_text_body,
- "" = _html_part ->
+ _plain_text_body,
+ html_part ->
  assert subject == "Ressources indisponibles dans le jeu de données #{dataset.custom_title}"
 
- assert plain_text_body =~
- "Les ressources #{resource_1.title}, #{resource_2.title} dans votre jeu de données #{dataset.custom_title} — http://127.0.0.1:5100/datasets/#{dataset.slug} ne sont plus disponibles au téléchargement depuis plus de 6h."
+ assert html_part =~
+ ~s(Les ressources #{resource_1.title}, #{resource_2.title} dans votre jeu de données <a href="http://127.0.0.1:5100/datasets/#{dataset.slug}">#{dataset.custom_title}</a> ne sont plus disponibles au téléchargement depuis plus de 6h.)
+
+ assert html_part =~
+ "Il semble que vous ayez supprimé et créé une nouvelle ressource. Lors de la mise à jour de vos données, remplacez plutôt le fichier au sein de la ressource existante."
 
  :ok
  end)
@@ -134,12 +153,15 @@ defmodule Transport.Test.Transport.Jobs.ResourceUnavailableNotificationJobTest d
  "[email protected]" = _to,
  "[email protected]",
  subject,
- plain_text_body,
- "" = _html_part ->
+ _plain_text_body,
+ html_part ->
  assert subject == "Ressources indisponibles dans le jeu de données #{gtfs_dataset.custom_title}"
 
- assert plain_text_body =~
- "Les ressources #{resource_gtfs.title} dans votre jeu de données #{gtfs_dataset.custom_title} — http://127.0.0.1:5100/datasets/#{gtfs_dataset.slug} ne sont plus disponibles au téléchargement depuis plus de 6h."
+ assert html_part =~
+ ~s(Les ressources #{resource_gtfs.title} dans votre jeu de données <a href="http://127.0.0.1:5100/datasets/#{gtfs_dataset.slug}">#{gtfs_dataset.custom_title}</a> ne sont plus disponibles au téléchargement depuis plus de 6h.)
+
+ refute html_part =~ "Il semble que vous ayez supprimé et créé une nouvelle ressource."
+ assert html_part =~ "Ces erreurs provoquent des difficultés pour les réutilisateurs."
 
  :ok
  end)
@@ -165,4 +187,43 @@ defmodule Transport.Test.Transport.Jobs.ResourceUnavailableNotificationJobTest d
  )
  |> DB.Repo.exists?()
  end
+
+ describe "created_resource_hosted_on_datagouv_recently?" do
+ test "base case" do
+ dataset = %DB.Dataset{datagouv_id: Ecto.UUID.generate()}
+ file_url = "https://static.data.gouv.fr/file.zip"
+ assert DB.Resource.hosted_on_datagouv?(file_url)
+ setup_dataset_response(dataset, file_url, DateTime.utc_now() |> DateTime.add(-6, :hour))
+ assert ResourceUnavailableNotificationJob.created_resource_hosted_on_datagouv_recently?(dataset)
+ end
+
+ test "resource on datagouv has been created a long time ago" do
+ dataset = %DB.Dataset{datagouv_id: Ecto.UUID.generate()}
+ file_url = "https://static.data.gouv.fr/file.zip"
+ assert DB.Resource.hosted_on_datagouv?(file_url)
+ setup_dataset_response(dataset, file_url, DateTime.utc_now() |> DateTime.add(-24, :hour))
+ refute ResourceUnavailableNotificationJob.created_resource_hosted_on_datagouv_recently?(dataset)
+ end
+
+ test "recent resource is not hosted on datagouv" do
+ dataset = %DB.Dataset{datagouv_id: Ecto.UUID.generate()}
+ file_url = "https://example.com/file.zip"
+ refute DB.Resource.hosted_on_datagouv?(file_url)
+ setup_dataset_response(dataset, file_url, DateTime.utc_now() |> DateTime.add(-6, :hour))
+ refute ResourceUnavailableNotificationJob.created_resource_hosted_on_datagouv_recently?(dataset)
+ end
+ end
+
+ defp setup_dataset_response(%DB.Dataset{datagouv_id: datagouv_id}, resource_url, created_at) do
+ url = "https://demo.data.gouv.fr/api/1/datasets/#{datagouv_id}/"
+
+ Transport.HTTPoison.Mock
+ |> expect(:request, fn :get, ^url, "", [], [follow_redirect: true] ->
+ {:ok,
+ %HTTPoison.Response{
+ status_code: 200,
+ body: Jason.encode!(%{"resources" => [%{"url" => resource_url, "created_at" => created_at}]})
+ }}
+ end)
+ end
 end