From 37423e2bca9767b55e45718e0a786ffc7b37a9e3 Mon Sep 17 00:00:00 2001 From: Vincent Degove Date: Mon, 16 Dec 2024 16:47:15 +0100 Subject: [PATCH 1/8] Rename outdated data functions for clarity --- apps/transport/lib/transport/data_checker.ex | 14 +++++++------- .../transport/test/transport/data_checker_test.exs | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/transport/lib/transport/data_checker.ex b/apps/transport/lib/transport/data_checker.ex index 60c6fe663b..b11d48c31c 100644 --- a/apps/transport/lib/transport/data_checker.ex +++ b/apps/transport/lib/transport/data_checker.ex @@ -116,8 +116,8 @@ defmodule Transport.DataChecker do {delay, gtfs_datasets_expiring_on(date)} end |> Enum.reject(fn {_, records} -> Enum.empty?(records) end) - |> send_outdated_data_mail() - |> Enum.map(&send_outdated_data_notifications(&1, job_id)) + |> send_outdated_data_admin_mail() + |> Enum.map(&send_outdated_data_producer_notifications(&1, job_id)) end @spec gtfs_datasets_expiring_on(Date.t()) :: [{DB.Dataset.t(), [DB.Resource.t()]}] @@ -160,8 +160,8 @@ defmodule Transport.DataChecker do end) end - @spec send_outdated_data_notifications(delay_and_records(), integer()) :: delay_and_records() - def send_outdated_data_notifications({delay, records} = payload, job_id) do + @spec send_outdated_data_producer_notifications(delay_and_records(), integer()) :: delay_and_records() + def send_outdated_data_producer_notifications({delay, records} = payload, job_id) do Enum.each(records, fn {%DB.Dataset{} = dataset, resources} -> @expiration_reason |> DB.NotificationSubscription.subscriptions_for_reason_dataset_and_role(dataset, :producer) @@ -189,10 +189,10 @@ defmodule Transport.DataChecker do |> Enum.map_join(", ", fn %DB.Resource{title: title} -> title end) end - @spec send_outdated_data_mail([delay_and_records()]) :: [delay_and_records()] - defp send_outdated_data_mail([] = _records), do: [] + @spec send_outdated_data_admin_mail([delay_and_records()]) :: [delay_and_records()] + defp send_outdated_data_admin_mail([] = _records), do: [] - defp send_outdated_data_mail(records) do + defp send_outdated_data_admin_mail(records) do Transport.AdminNotifier.expiration(records) |> Transport.Mailer.deliver() diff --git a/apps/transport/test/transport/data_checker_test.exs b/apps/transport/test/transport/data_checker_test.exs index 44b9be7ced..87d3f1a9d3 100644 --- a/apps/transport/test/transport/data_checker_test.exs +++ b/apps/transport/test/transport/data_checker_test.exs @@ -290,7 +290,7 @@ defmodule Transport.DataCheckerTest do }) job_id = 42 - Transport.DataChecker.send_outdated_data_notifications({7, [{dataset, []}]}, job_id) + Transport.DataChecker.send_outdated_data_producer_notifications({7, [{dataset, []}]}, job_id) assert_email_sent( from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, From 27eb0b122ec02d1e8759000e7a9d5c95c6126c20 Mon Sep 17 00:00:00 2001 From: Vincent Degove Date: Mon, 16 Dec 2024 17:06:51 +0100 Subject: [PATCH 2/8] Move job to Oban --- .../transport/lib/jobs/outdated_data_notification_job.ex | 9 +++++++++ apps/transport/lib/transport/data_checker.ex | 6 +----- apps/transport/lib/transport/scheduler.ex | 2 -- apps/transport/test/transport/data_checker_test.exs | 4 ++-- config/runtime.exs | 3 ++- 5 files changed, 14 insertions(+), 10 deletions(-) create mode 100644 apps/transport/lib/jobs/outdated_data_notification_job.ex diff --git a/apps/transport/lib/jobs/outdated_data_notification_job.ex b/apps/transport/lib/jobs/outdated_data_notification_job.ex new file mode 100644 index 0000000000..8fbf9f21a6 --- /dev/null +++ b/apps/transport/lib/jobs/outdated_data_notification_job.ex @@ -0,0 +1,9 @@ +defmodule Transport.Jobs.OutdatedDataNotificationJob do + use Oban.Worker, max_attempts: 3, tags: ["notifications"] + + @impl Oban.Worker + + def perform(%Oban.Job{id: job_id}) do + Transport.DataChecker.outdated_data(job_id) + end +end diff --git a/apps/transport/lib/transport/data_checker.ex b/apps/transport/lib/transport/data_checker.ex index b11d48c31c..7b7786b481 100644 --- a/apps/transport/lib/transport/data_checker.ex +++ b/apps/transport/lib/transport/data_checker.ex @@ -106,11 +106,7 @@ defmodule Transport.DataChecker do ) end - def outdated_data do - # Generated as an integer rather than a UUID because `payload.job_id` - # for other notifications are %Oban.Job.id (bigint). - job_id = Enum.random(1..Integer.pow(2, 63)) - + def outdated_data(job_id) do for delay <- possible_delays(), date = Date.add(Date.utc_today(), delay) do {delay, gtfs_datasets_expiring_on(date)} diff --git a/apps/transport/lib/transport/scheduler.ex b/apps/transport/lib/transport/scheduler.ex index 3c5dcfc799..5dc8d56fe2 100644 --- a/apps/transport/lib/transport/scheduler.ex +++ b/apps/transport/lib/transport/scheduler.ex @@ -13,8 +13,6 @@ defmodule Transport.Scheduler do [ # Every day at 4am UTC {"0 4 * * *", {Transport.ImportData, :import_validate_all, []}}, - # Send email for outdated data - {"@daily", {Transport.DataChecker, :outdated_data, []}}, # Set inactive data {"@daily", {Transport.DataChecker, :inactive_data, []}}, # Watch for new comments on datasets diff --git a/apps/transport/test/transport/data_checker_test.exs b/apps/transport/test/transport/data_checker_test.exs index 87d3f1a9d3..9f8dd16e26 100644 --- a/apps/transport/test/transport/data_checker_test.exs +++ b/apps/transport/test/transport/data_checker_test.exs @@ -233,7 +233,7 @@ defmodule Transport.DataCheckerTest do dataset_id: dataset.id }) - Transport.DataChecker.outdated_data() + Transport.DataChecker.outdated_data(42) # a first mail to our team @@ -271,7 +271,7 @@ defmodule Transport.DataCheckerTest do end test "outdated_data job with nothing to send should not send email" do - Transport.DataChecker.outdated_data() + Transport.DataChecker.outdated_data(42) assert_no_email_sent() end end diff --git a/config/runtime.exs b/config/runtime.exs index 3c72728c3e..59102d4062 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -160,7 +160,8 @@ oban_prod_crontab = [ {"30 5 * * *", Transport.Jobs.ImportDatasetMonthlyMetricsJob}, {"45 5 * * *", Transport.Jobs.ImportResourceMonthlyMetricsJob}, {"0 8 * * *", Transport.Jobs.WarnUserInactivityJob}, - {"*/5 * * * *", Transport.Jobs.UpdateCounterCacheJob} + {"*/5 * * * *", Transport.Jobs.UpdateCounterCacheJob}, + {"0 0 * * *", Transport.Jobs.OutdatedDataNotificationJob} ] # Make sure that all modules exist From e042c0ff7d663df0276f90ac105d2ea13415baf5 Mon Sep 17 00:00:00 2001 From: Vincent Degove Date: Tue, 17 Dec 2024 10:52:21 +0100 Subject: [PATCH 3/8] Fix Oban job and do test it --- apps/transport/lib/jobs/outdated_data_notification_job.ex | 6 ++++++ apps/transport/lib/transport/data_checker.ex | 4 +++- apps/transport/test/transport/data_checker_test.exs | 5 +++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/apps/transport/lib/jobs/outdated_data_notification_job.ex b/apps/transport/lib/jobs/outdated_data_notification_job.ex index 8fbf9f21a6..b32918b372 100644 --- a/apps/transport/lib/jobs/outdated_data_notification_job.ex +++ b/apps/transport/lib/jobs/outdated_data_notification_job.ex @@ -1,9 +1,15 @@ defmodule Transport.Jobs.OutdatedDataNotificationJob do + @moduledoc """ + This module is in charge of sending notifications to both admins and users when data is outdated. + It is (currently) using the old DataChecker module, where there is also code for checking active/inactive datasets. + Behaviour of this job is tested in test/transport/data_checker_test.exs. + """ use Oban.Worker, max_attempts: 3, tags: ["notifications"] @impl Oban.Worker def perform(%Oban.Job{id: job_id}) do Transport.DataChecker.outdated_data(job_id) + :ok end end diff --git a/apps/transport/lib/transport/data_checker.ex b/apps/transport/lib/transport/data_checker.ex index 7b7786b481..e9f6e5815d 100644 --- a/apps/transport/lib/transport/data_checker.ex +++ b/apps/transport/lib/transport/data_checker.ex @@ -1,6 +1,8 @@ defmodule Transport.DataChecker do @moduledoc """ - Use to check data, and act about it, like send email + Use to check data for two things: + - Toggle in and of active status of datasets depending on status on data.gouv.fr + - Send notifications to producers and admins when data is outdated """ alias DB.{Dataset, Repo} import Ecto.Query diff --git a/apps/transport/test/transport/data_checker_test.exs b/apps/transport/test/transport/data_checker_test.exs index 9f8dd16e26..ab16de9a1a 100644 --- a/apps/transport/test/transport/data_checker_test.exs +++ b/apps/transport/test/transport/data_checker_test.exs @@ -3,6 +3,7 @@ defmodule Transport.DataCheckerTest do import Mox import DB.Factory import Swoosh.TestAssertions + use Oban.Testing, repo: DB.Repo doctest Transport.DataChecker, import: true @@ -233,7 +234,7 @@ defmodule Transport.DataCheckerTest do dataset_id: dataset.id }) - Transport.DataChecker.outdated_data(42) + assert :ok == perform_job(Transport.Jobs.OutdatedDataNotificationJob, %{}) # a first mail to our team @@ -271,7 +272,7 @@ defmodule Transport.DataCheckerTest do end test "outdated_data job with nothing to send should not send email" do - Transport.DataChecker.outdated_data(42) + assert :ok == perform_job(Transport.Jobs.OutdatedDataNotificationJob, %{}) assert_no_email_sent() end end From ef55051eb09467284792b667abc949d0f83bdfcc Mon Sep 17 00:00:00 2001 From: Vincent Degove Date: Tue, 17 Dec 2024 11:14:34 +0100 Subject: [PATCH 4/8] Clean some things in data_checker --- apps/transport/lib/transport/data_checker.ex | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/apps/transport/lib/transport/data_checker.ex b/apps/transport/lib/transport/data_checker.ex index e9f6e5815d..2693a197a0 100644 --- a/apps/transport/lib/transport/data_checker.ex +++ b/apps/transport/lib/transport/data_checker.ex @@ -158,8 +158,9 @@ defmodule Transport.DataChecker do end) end - @spec send_outdated_data_producer_notifications(delay_and_records(), integer()) :: delay_and_records() - def send_outdated_data_producer_notifications({delay, records} = payload, job_id) do + # A different email is sent to producers for every delay, containing in a single email all datasets expiring on this given delay + @spec send_outdated_data_producer_notifications(delay_and_records(), integer()) :: :ok + def send_outdated_data_producer_notifications({delay, records}, job_id) do Enum.each(records, fn {%DB.Dataset{} = dataset, resources} -> @expiration_reason |> DB.NotificationSubscription.subscriptions_for_reason_dataset_and_role(dataset, :producer) @@ -171,20 +172,6 @@ defmodule Transport.DataChecker do DB.Notification.insert!(dataset, subscription, %{delay: delay, job_id: job_id}) end) end) - - payload - end - - @doc """ - iex> resource_titles([%DB.Resource{title: "B"}]) - "B" - iex> resource_titles([%DB.Resource{title: "B"}, %DB.Resource{title: "A"}]) - "A, B" - """ - def resource_titles(resources) do - resources - |> Enum.sort_by(fn %DB.Resource{title: title} -> title end) - |> Enum.map_join(", ", fn %DB.Resource{title: title} -> title end) end @spec send_outdated_data_admin_mail([delay_and_records()]) :: [delay_and_records()] From 141f688bdaf99677beef95e2fd9557a4708393aa Mon Sep 17 00:00:00 2001 From: Vincent Degove Date: Tue, 17 Dec 2024 12:16:30 +0100 Subject: [PATCH 5/8] Migrate all new dataset notification code to job instead of datachecker --- .../lib/jobs/new_dataset_notifications_job.ex | 21 ++++++++- apps/transport/lib/transport/data_checker.ex | 20 --------- .../test/transport/data_checker_test.exs | 44 ------------------- .../new_dataset_notifications_job_test.exs | 33 +++++++++++--- 4 files changed, 47 insertions(+), 71 deletions(-) diff --git a/apps/transport/lib/jobs/new_dataset_notifications_job.ex b/apps/transport/lib/jobs/new_dataset_notifications_job.ex index f8be16d5b0..759f188371 100644 --- a/apps/transport/lib/jobs/new_dataset_notifications_job.ex +++ b/apps/transport/lib/jobs/new_dataset_notifications_job.ex @@ -4,10 +4,12 @@ defmodule Transport.Jobs.NewDatasetNotificationsJob do """ use Oban.Worker, max_attempts: 3, tags: ["notifications"] import Ecto.Query + @new_dataset_reason Transport.NotificationReason.reason(:new_dataset) @impl Oban.Worker - def perform(%Oban.Job{inserted_at: %DateTime{} = inserted_at}) do - inserted_at |> relevant_datasets() |> Transport.DataChecker.send_new_dataset_notifications() + + def perform(%Oban.Job{id: job_id, inserted_at: %DateTime{} = inserted_at}) do + inserted_at |> relevant_datasets() |> send_new_dataset_notifications(job_id) :ok end @@ -18,4 +20,19 @@ defmodule Transport.Jobs.NewDatasetNotificationsJob do |> where([dataset: d], d.inserted_at >= ^datetime_limit) |> DB.Repo.all() end + + @spec send_new_dataset_notifications([DB.Dataset.t()] | [], pos_integer()) :: no_return() | :ok + def send_new_dataset_notifications([], _job_id), do: :ok + + def send_new_dataset_notifications(datasets, job_id) do + @new_dataset_reason + |> DB.NotificationSubscription.subscriptions_for_reason_and_role(:reuser) + |> Enum.each(fn %DB.NotificationSubscription{contact: %DB.Contact{} = contact} = subscription -> + contact + |> Transport.UserNotifier.new_datasets(datasets) + |> Transport.Mailer.deliver() + + DB.Notification.insert!(subscription, %{dataset_ids: Enum.map(datasets, & &1.id), job_id: job_id}) + end) + end end diff --git a/apps/transport/lib/transport/data_checker.ex b/apps/transport/lib/transport/data_checker.ex index 2693a197a0..9724c70bf5 100644 --- a/apps/transport/lib/transport/data_checker.ex +++ b/apps/transport/lib/transport/data_checker.ex @@ -11,7 +11,6 @@ defmodule Transport.DataChecker do @type delay_and_records :: {integer(), [{DB.Dataset.t(), [DB.Resource.t()]}]} @type dataset_status :: :active | :inactive | :ignore | :no_producer | {:archived, DateTime.t()} @expiration_reason Transport.NotificationReason.reason(:expiration) - @new_dataset_reason Transport.NotificationReason.reason(:new_dataset) # If delay < 0, the resource is already expired @default_outdated_data_delays [-90, -60, -30, -45, -15, -7, -3, 0, 7, 14] @@ -139,25 +138,6 @@ defmodule Transport.DataChecker do |> Enum.sort() end - @spec send_new_dataset_notifications([Dataset.t()] | []) :: no_return() | :ok - def send_new_dataset_notifications([]), do: :ok - - def send_new_dataset_notifications(datasets) do - # Generated as an integer rather than a UUID because `payload.job_id` - # for other notifications are %Oban.Job.id (bigint). - job_id = Enum.random(1..Integer.pow(2, 63)) - - @new_dataset_reason - |> DB.NotificationSubscription.subscriptions_for_reason_and_role(:reuser) - |> Enum.each(fn %DB.NotificationSubscription{contact: %DB.Contact{} = contact} = subscription -> - contact - |> Transport.UserNotifier.new_datasets(datasets) - |> Transport.Mailer.deliver() - - DB.Notification.insert!(subscription, %{dataset_ids: Enum.map(datasets, & &1.id), job_id: job_id}) - end) - end - # A different email is sent to producers for every delay, containing in a single email all datasets expiring on this given delay @spec send_outdated_data_producer_notifications(delay_and_records(), integer()) :: :ok def send_outdated_data_producer_notifications({delay, records}, job_id) do diff --git a/apps/transport/test/transport/data_checker_test.exs b/apps/transport/test/transport/data_checker_test.exs index ab16de9a1a..8f58b73ba5 100644 --- a/apps/transport/test/transport/data_checker_test.exs +++ b/apps/transport/test/transport/data_checker_test.exs @@ -315,50 +315,6 @@ defmodule Transport.DataCheckerTest do DB.Notification |> DB.Repo.all() end - describe "send_new_dataset_notifications" do - test "no datasets" do - assert Transport.DataChecker.send_new_dataset_notifications([]) == :ok - end - - test "with datasets" do - %DB.Dataset{id: dataset_id} = dataset = insert(:dataset, type: "public-transit") - - %DB.Contact{id: contact_id, email: email} = contact = insert_contact() - - %DB.NotificationSubscription{id: ns_id} = - insert(:notification_subscription, %{ - reason: :new_dataset, - source: :user, - role: :reuser, - contact_id: contact_id - }) - - Transport.DataChecker.send_new_dataset_notifications([dataset]) - - assert_email_sent( - from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, - to: {DB.Contact.display_name(contact), email}, - subject: "Nouveaux jeux de données référencés", - text_body: nil, - html_body: - ~r|
  • #{dataset.custom_title} - \(Transport public collectif - horaires théoriques\)
    • | - ) - - assert [ - %DB.Notification{ - contact_id: ^contact_id, - email: ^email, - reason: :new_dataset, - role: :reuser, - dataset_id: nil, - payload: %{"dataset_ids" => [^dataset_id], "job_id" => _}, - notification_subscription_id: ^ns_id - } - ] = - DB.Notification |> DB.Repo.all() - end - end - describe "dataset_status" do test "active" do dataset = %DB.Dataset{datagouv_id: Ecto.UUID.generate()} diff --git a/apps/transport/test/transport/jobs/new_dataset_notifications_job_test.exs b/apps/transport/test/transport/jobs/new_dataset_notifications_job_test.exs index 9da324822c..17af65d9be 100644 --- a/apps/transport/test/transport/jobs/new_dataset_notifications_job_test.exs +++ b/apps/transport/test/transport/jobs/new_dataset_notifications_job_test.exs @@ -24,11 +24,10 @@ defmodule Transport.Test.Transport.Jobs.NewDatasetNotificationsJobTest do end test "perform" do - %DB.Dataset{id: dataset_id} = insert(:dataset, inserted_at: hours_ago(23), is_active: true) - %DB.Contact{id: contact_id, email: email} = contact = insert_contact() + {contact, contact_id, email, ns_id} = insert_contact_and_notification_subscription() - %DB.NotificationSubscription{id: ns_id} = - insert(:notification_subscription, %{reason: :new_dataset, source: :admin, role: :reuser, contact_id: contact_id}) + %DB.Dataset{id: dataset_id} = + dataset = insert(:dataset, inserted_at: hours_ago(23), is_active: true, type: "public-transit") assert :ok == perform_job(NewDatasetNotificationsJob, %{}, inserted_at: DateTime.utc_now()) @@ -37,7 +36,8 @@ defmodule Transport.Test.Transport.Jobs.NewDatasetNotificationsJobTest do to: {DB.Contact.display_name(contact), email}, subject: "Nouveaux jeux de données référencés", text_body: nil, - html_body: ~r|

    Bonjour,

    | + html_body: + ~r|
  • #{dataset.custom_title} - \(Transport public collectif - horaires théoriques\)
    • | ) # Logs have been saved @@ -46,6 +46,7 @@ defmodule Transport.Test.Transport.Jobs.NewDatasetNotificationsJobTest do contact_id: ^contact_id, email: ^email, reason: :new_dataset, + role: :reuser, dataset_id: nil, notification_subscription_id: ^ns_id, payload: %{"dataset_ids" => [^dataset_id]} @@ -54,7 +55,29 @@ defmodule Transport.Test.Transport.Jobs.NewDatasetNotificationsJobTest do DB.Notification |> DB.Repo.all() end + test "no datasets" do + insert_contact_and_notification_subscription() + + assert :ok == perform_job(NewDatasetNotificationsJob, %{}, inserted_at: DateTime.utc_now()) + + assert_no_email_sent() + end + defp hours_ago(hours) when hours > 0 do DateTime.utc_now() |> DateTime.add(-hours * 60 * 60, :second) end + + defp insert_contact_and_notification_subscription do + %DB.Contact{id: contact_id, email: email} = contact = insert_contact() + + %DB.NotificationSubscription{id: ns_id} = + insert(:notification_subscription, %{ + reason: :new_dataset, + source: :user, + role: :reuser, + contact_id: contact_id + }) + + {contact, contact_id, email, ns_id} + end end From d61bb631429e19e5917705419998288d78569d72 Mon Sep 17 00:00:00 2001 From: Vincent Degove Date: Tue, 17 Dec 2024 12:49:39 +0100 Subject: [PATCH 6/8] Move code from ImportData to OutdatedNotificationJob and merge two similar tests --- .../jobs/outdated_data_notification_job.ex | 68 ++++++- apps/transport/lib/transport/data_checker.ex | 65 +----- .../test/transport/data_checker_test.exs | 190 ------------------ .../outdated_data_notification_job_test.exs | 172 ++++++++++++++++ 4 files changed, 238 insertions(+), 257 deletions(-) create mode 100644 apps/transport/test/transport/jobs/outdated_data_notification_job_test.exs diff --git a/apps/transport/lib/jobs/outdated_data_notification_job.ex b/apps/transport/lib/jobs/outdated_data_notification_job.ex index b32918b372..e5321d1cba 100644 --- a/apps/transport/lib/jobs/outdated_data_notification_job.ex +++ b/apps/transport/lib/jobs/outdated_data_notification_job.ex @@ -1,15 +1,77 @@ defmodule Transport.Jobs.OutdatedDataNotificationJob do @moduledoc """ This module is in charge of sending notifications to both admins and users when data is outdated. - It is (currently) using the old DataChecker module, where there is also code for checking active/inactive datasets. - Behaviour of this job is tested in test/transport/data_checker_test.exs. """ + use Oban.Worker, max_attempts: 3, tags: ["notifications"] + import Ecto.Query + + @type delay_and_records :: {integer(), [{DB.Dataset.t(), [DB.Resource.t()]}]} + @expiration_reason Transport.NotificationReason.reason(:expiration) + # If delay < 0, the resource is already expired + @default_outdated_data_delays [-90, -60, -30, -45, -15, -7, -3, 0, 7, 14] @impl Oban.Worker def perform(%Oban.Job{id: job_id}) do - Transport.DataChecker.outdated_data(job_id) + outdated_data(job_id) :ok end + + def outdated_data(job_id) do + for delay <- possible_delays(), + date = Date.add(Date.utc_today(), delay) do + {delay, gtfs_datasets_expiring_on(date)} + end + |> Enum.reject(fn {_, records} -> Enum.empty?(records) end) + |> send_outdated_data_admin_mail() + |> Enum.map(&send_outdated_data_producer_notifications(&1, job_id)) + end + + @spec gtfs_datasets_expiring_on(Date.t()) :: [{DB.Dataset.t(), [DB.Resource.t()]}] + def gtfs_datasets_expiring_on(%Date{} = date) do + DB.Dataset.base_query() + |> DB.Dataset.join_from_dataset_to_metadata(Transport.Validators.GTFSTransport.validator_name()) + |> where( + [metadata: m, resource: r], + fragment("TO_DATE(?->>'end_date', 'YYYY-MM-DD')", m.metadata) == ^date and r.format == "GTFS" + ) + |> select([dataset: d, resource: r], {d, r}) + |> distinct(true) + |> DB.Repo.all() + |> Enum.group_by(fn {%DB.Dataset{} = d, _} -> d end, fn {_, %DB.Resource{} = r} -> r end) + |> Enum.to_list() + end + + def possible_delays do + @default_outdated_data_delays + |> Enum.uniq() + |> Enum.sort() + end + + # A different email is sent to producers for every delay, containing all datasets expiring on this given delay + @spec send_outdated_data_producer_notifications(delay_and_records(), integer()) :: :ok + def send_outdated_data_producer_notifications({delay, records}, job_id) do + Enum.each(records, fn {%DB.Dataset{} = dataset, resources} -> + @expiration_reason + |> DB.NotificationSubscription.subscriptions_for_reason_dataset_and_role(dataset, :producer) + |> Enum.each(fn %DB.NotificationSubscription{contact: %DB.Contact{} = contact} = subscription -> + contact + |> Transport.UserNotifier.expiration_producer(dataset, resources, delay) + |> Transport.Mailer.deliver() + + DB.Notification.insert!(dataset, subscription, %{delay: delay, job_id: job_id}) + end) + end) + end + + @spec send_outdated_data_admin_mail([delay_and_records()]) :: [delay_and_records()] + defp send_outdated_data_admin_mail([] = _records), do: [] + + defp send_outdated_data_admin_mail(records) do + Transport.AdminNotifier.expiration(records) + |> Transport.Mailer.deliver() + + records + end end diff --git a/apps/transport/lib/transport/data_checker.ex b/apps/transport/lib/transport/data_checker.ex index 9724c70bf5..6cdd2f3d1d 100644 --- a/apps/transport/lib/transport/data_checker.ex +++ b/apps/transport/lib/transport/data_checker.ex @@ -1,18 +1,12 @@ defmodule Transport.DataChecker do @moduledoc """ - Use to check data for two things: - - Toggle in and of active status of datasets depending on status on data.gouv.fr - - Send notifications to producers and admins when data is outdated + Use to check data for toggling on and off active status of datasets depending on status on data.gouv.fr """ alias DB.{Dataset, Repo} import Ecto.Query require Logger - @type delay_and_records :: {integer(), [{DB.Dataset.t(), [DB.Resource.t()]}]} @type dataset_status :: :active | :inactive | :ignore | :no_producer | {:archived, DateTime.t()} - @expiration_reason Transport.NotificationReason.reason(:expiration) - # If delay < 0, the resource is already expired - @default_outdated_data_delays [-90, -60, -30, -45, -15, -7, -3, 0, 7, 14] @doc """ This method is a scheduled job which does two things: @@ -107,63 +101,6 @@ defmodule Transport.DataChecker do ) end - def outdated_data(job_id) do - for delay <- possible_delays(), - date = Date.add(Date.utc_today(), delay) do - {delay, gtfs_datasets_expiring_on(date)} - end - |> Enum.reject(fn {_, records} -> Enum.empty?(records) end) - |> send_outdated_data_admin_mail() - |> Enum.map(&send_outdated_data_producer_notifications(&1, job_id)) - end - - @spec gtfs_datasets_expiring_on(Date.t()) :: [{DB.Dataset.t(), [DB.Resource.t()]}] - def gtfs_datasets_expiring_on(%Date{} = date) do - DB.Dataset.base_query() - |> DB.Dataset.join_from_dataset_to_metadata(Transport.Validators.GTFSTransport.validator_name()) - |> where( - [metadata: m, resource: r], - fragment("TO_DATE(?->>'end_date', 'YYYY-MM-DD')", m.metadata) == ^date and r.format == "GTFS" - ) - |> select([dataset: d, resource: r], {d, r}) - |> distinct(true) - |> DB.Repo.all() - |> Enum.group_by(fn {%DB.Dataset{} = d, _} -> d end, fn {_, %DB.Resource{} = r} -> r end) - |> Enum.to_list() - end - - def possible_delays do - @default_outdated_data_delays - |> Enum.uniq() - |> Enum.sort() - end - - # A different email is sent to producers for every delay, containing in a single email all datasets expiring on this given delay - @spec send_outdated_data_producer_notifications(delay_and_records(), integer()) :: :ok - def send_outdated_data_producer_notifications({delay, records}, job_id) do - Enum.each(records, fn {%DB.Dataset{} = dataset, resources} -> - @expiration_reason - |> DB.NotificationSubscription.subscriptions_for_reason_dataset_and_role(dataset, :producer) - |> Enum.each(fn %DB.NotificationSubscription{contact: %DB.Contact{} = contact} = subscription -> - contact - |> Transport.UserNotifier.expiration_producer(dataset, resources, delay) - |> Transport.Mailer.deliver() - - DB.Notification.insert!(dataset, subscription, %{delay: delay, job_id: job_id}) - end) - end) - end - - @spec send_outdated_data_admin_mail([delay_and_records()]) :: [delay_and_records()] - defp send_outdated_data_admin_mail([] = _records), do: [] - - defp send_outdated_data_admin_mail(records) do - Transport.AdminNotifier.expiration(records) - |> Transport.Mailer.deliver() - - records - end - # Do nothing if all lists are empty defp send_inactive_datasets_mail([] = _reactivated_datasets, [] = _inactive_datasets, [] = _archived_datasets), do: nil diff --git a/apps/transport/test/transport/data_checker_test.exs b/apps/transport/test/transport/data_checker_test.exs index 8f58b73ba5..c4b9372070 100644 --- a/apps/transport/test/transport/data_checker_test.exs +++ b/apps/transport/test/transport/data_checker_test.exs @@ -3,9 +3,6 @@ defmodule Transport.DataCheckerTest do import Mox import DB.Factory import Swoosh.TestAssertions - use Oban.Testing, repo: DB.Repo - - doctest Transport.DataChecker, import: true setup :verify_on_exit! @@ -128,193 +125,6 @@ defmodule Transport.DataCheckerTest do end end - test "gtfs_datasets_expiring_on" do - {today, tomorrow, yesterday} = {Date.utc_today(), Date.add(Date.utc_today(), 1), Date.add(Date.utc_today(), -1)} - assert [] == today |> Transport.DataChecker.gtfs_datasets_expiring_on() - - insert_fn = fn %Date{} = expiration_date, %DB.Dataset{} = dataset -> - multi_validation = - insert(:multi_validation, - validator: Transport.Validators.GTFSTransport.validator_name(), - resource_history: insert(:resource_history, resource: insert(:resource, dataset: dataset, format: "GTFS")) - ) - - insert(:resource_metadata, - multi_validation_id: multi_validation.id, - metadata: %{"end_date" => expiration_date} - ) - end - - # Ignores hidden or inactive datasets - insert_fn.(today, insert(:dataset, is_active: false)) - insert_fn.(today, insert(:dataset, is_active: true, is_hidden: true)) - - assert [] == today |> Transport.DataChecker.gtfs_datasets_expiring_on() - - # 2 GTFS resources expiring on the same day for a dataset - %DB.Dataset{id: dataset_id} = dataset = insert(:dataset, is_active: true) - insert_fn.(today, dataset) - insert_fn.(today, dataset) - - assert [ - {%DB.Dataset{id: ^dataset_id}, - [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} - ] = today |> Transport.DataChecker.gtfs_datasets_expiring_on() - - assert [] == tomorrow |> Transport.DataChecker.gtfs_datasets_expiring_on() - assert [] == yesterday |> Transport.DataChecker.gtfs_datasets_expiring_on() - - insert_fn.(tomorrow, dataset) - - assert [ - {%DB.Dataset{id: ^dataset_id}, - [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} - ] = today |> Transport.DataChecker.gtfs_datasets_expiring_on() - - assert [ - {%DB.Dataset{id: ^dataset_id}, [%DB.Resource{dataset_id: ^dataset_id}]} - ] = tomorrow |> Transport.DataChecker.gtfs_datasets_expiring_on() - - assert [] == yesterday |> Transport.DataChecker.gtfs_datasets_expiring_on() - - # Multiple datasets - %DB.Dataset{id: d2_id} = d2 = insert(:dataset, is_active: true) - insert_fn.(today, d2) - - assert [ - {%DB.Dataset{id: ^dataset_id}, - [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]}, - {%DB.Dataset{id: ^d2_id}, [%DB.Resource{dataset_id: ^d2_id}]} - ] = today |> Transport.DataChecker.gtfs_datasets_expiring_on() - end - - describe "outdated_data job" do - test "sends email to our team + relevant contact before expiry" do - %DB.Dataset{id: dataset_id} = - dataset = - insert(:dataset, is_active: true, custom_title: "Dataset custom title", custom_tags: ["loi-climat-resilience"]) - - assert DB.Dataset.climate_resilience_bill?(dataset) - # fake a resource expiring today - %DB.Resource{id: resource_id} = - resource = insert(:resource, dataset: dataset, format: "GTFS", title: resource_title = "Super GTFS") - - multi_validation = - insert(:multi_validation, - validator: Transport.Validators.GTFSTransport.validator_name(), - resource_history: insert(:resource_history, resource: resource) - ) - - insert(:resource_metadata, - multi_validation_id: multi_validation.id, - metadata: %{"end_date" => Date.utc_today()} - ) - - assert [{%DB.Dataset{id: ^dataset_id}, [%DB.Resource{id: ^resource_id}]}] = - Date.utc_today() |> Transport.DataChecker.gtfs_datasets_expiring_on() - - %DB.Contact{id: contact_id, email: email} = contact = insert_contact() - - insert(:notification_subscription, %{ - reason: :expiration, - source: :admin, - role: :producer, - contact_id: contact_id, - dataset_id: dataset.id - }) - - # Should be ignored, this subscription is for a reuser - %DB.Contact{id: reuser_id} = insert_contact() - - insert(:notification_subscription, %{ - reason: :expiration, - source: :user, - role: :reuser, - contact_id: reuser_id, - dataset_id: dataset.id - }) - - assert :ok == perform_job(Transport.Jobs.OutdatedDataNotificationJob, %{}) - - # a first mail to our team - - assert_email_sent(fn %Swoosh.Email{ - from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, - to: [{"", "contact@transport.data.gouv.fr"}], - subject: "Jeux de données arrivant à expiration", - text_body: nil, - html_body: body - } -> - assert body =~ ~r/Jeux de données périmant demain :/ - - assert body =~ - ~s|
  • #{dataset.custom_title} - ✅ notification automatique ⚖️🗺️ article 122
    • | - end) - - # a second mail to the email address in the notifications config - display_name = DB.Contact.display_name(contact) - - assert_email_sent(fn %Swoosh.Email{ - from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, - to: [{^display_name, ^email}], - subject: "Jeu de données arrivant à expiration", - html_body: html_body - } -> - refute html_body =~ "notification automatique" - refute html_body =~ "article 122" - - assert html_body =~ - ~s(Les données GTFS #{resource_title} associées au jeu de données #{dataset.custom_title} périment demain.) - - assert html_body =~ - ~s(remplaçant la ressource périmée par la nouvelle) - end) - end - - test "outdated_data job with nothing to send should not send email" do - assert :ok == perform_job(Transport.Jobs.OutdatedDataNotificationJob, %{}) - assert_no_email_sent() - end - end - - test "send_outdated_data_notifications" do - %{id: dataset_id} = dataset = insert(:dataset) - %DB.Contact{id: contact_id, email: email} = contact = insert_contact() - - %DB.NotificationSubscription{id: ns_id} = - insert(:notification_subscription, %{ - reason: :expiration, - source: :admin, - role: :producer, - contact_id: contact_id, - dataset_id: dataset.id - }) - - job_id = 42 - Transport.DataChecker.send_outdated_data_producer_notifications({7, [{dataset, []}]}, job_id) - - assert_email_sent( - from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, - to: {DB.Contact.display_name(contact), email}, - subject: "Jeu de données arrivant à expiration", - text_body: nil, - html_body: ~r/Bonjour/ - ) - - assert [ - %DB.Notification{ - contact_id: ^contact_id, - email: ^email, - reason: :expiration, - dataset_id: ^dataset_id, - notification_subscription_id: ^ns_id, - role: :producer, - payload: %{"delay" => 7, "job_id" => ^job_id} - } - ] = - DB.Notification |> DB.Repo.all() - end - describe "dataset_status" do test "active" do dataset = %DB.Dataset{datagouv_id: Ecto.UUID.generate()} diff --git a/apps/transport/test/transport/jobs/outdated_data_notification_job_test.exs b/apps/transport/test/transport/jobs/outdated_data_notification_job_test.exs new file mode 100644 index 0000000000..c83275757b --- /dev/null +++ b/apps/transport/test/transport/jobs/outdated_data_notification_job_test.exs @@ -0,0 +1,172 @@ +defmodule Transport.Test.Transport.Jobs.OutdatedDataNotificationJobTest do + use ExUnit.Case, async: true + import DB.Factory + import Swoosh.TestAssertions + use Oban.Testing, repo: DB.Repo + + setup do + Ecto.Adapters.SQL.Sandbox.checkout(DB.Repo) + end + + test "sends email to our team + relevant contact before expiry" do + %DB.Dataset{id: dataset_id} = + dataset = + insert(:dataset, is_active: true, custom_title: "Dataset custom title", custom_tags: ["loi-climat-resilience"]) + + assert DB.Dataset.climate_resilience_bill?(dataset) + # fake a resource expiring today + %DB.Resource{id: resource_id} = + resource = insert(:resource, dataset: dataset, format: "GTFS", title: resource_title = "Super GTFS") + + multi_validation = + insert(:multi_validation, + validator: Transport.Validators.GTFSTransport.validator_name(), + resource_history: insert(:resource_history, resource: resource) + ) + + insert(:resource_metadata, + multi_validation_id: multi_validation.id, + metadata: %{"end_date" => Date.utc_today()} + ) + + assert [{%DB.Dataset{id: ^dataset_id}, [%DB.Resource{id: ^resource_id}]}] = + Date.utc_today() |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + + %DB.Contact{id: contact_id, email: email} = contact = insert_contact() + + %DB.NotificationSubscription{id: ns_id} = + insert(:notification_subscription, %{ + reason: :expiration, + source: :admin, + role: :producer, + contact_id: contact_id, + dataset_id: dataset.id + }) + + # Should be ignored, this subscription is for a reuser + %DB.Contact{id: reuser_id} = insert_contact() + + insert(:notification_subscription, %{ + reason: :expiration, + source: :user, + role: :reuser, + contact_id: reuser_id, + dataset_id: dataset.id + }) + + assert :ok == perform_job(Transport.Jobs.OutdatedDataNotificationJob, %{}) + + # a first mail to our team + + assert_email_sent(fn %Swoosh.Email{ + from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, + to: [{"", "contact@transport.data.gouv.fr"}], + subject: "Jeux de données arrivant à expiration", + text_body: nil, + html_body: body + } -> + assert body =~ ~r/Jeux de données périmant demain :/ + + assert body =~ + ~s|
  • #{dataset.custom_title} - ✅ notification automatique ⚖️🗺️ article 122
    • | + end) + + # a second mail to the email address in the notifications config + display_name = DB.Contact.display_name(contact) + + assert_email_sent(fn %Swoosh.Email{ + from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, + to: [{^display_name, ^email}], + subject: "Jeu de données arrivant à expiration", + html_body: html_body + } -> + refute html_body =~ "notification automatique" + refute html_body =~ "article 122" + + assert html_body =~ + ~s(Les données GTFS #{resource_title} associées au jeu de données #{dataset.custom_title} périment demain.) + + assert html_body =~ + ~s(remplaçant la ressource périmée par la nouvelle) + end) + + # Logs are there + assert [ + %DB.Notification{ + contact_id: ^contact_id, + email: ^email, + reason: :expiration, + dataset_id: ^dataset_id, + notification_subscription_id: ^ns_id, + role: :producer, + payload: %{"delay" => 0, "job_id" => _job_id} + } + ] = + DB.Notification |> DB.Repo.all() + end + + test "outdated_data job with nothing to send should not send email" do + assert :ok == perform_job(Transport.Jobs.OutdatedDataNotificationJob, %{}) + assert_no_email_sent() + end + + test "gtfs_datasets_expiring_on" do + {today, tomorrow, yesterday} = {Date.utc_today(), Date.add(Date.utc_today(), 1), Date.add(Date.utc_today(), -1)} + assert [] == today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + + insert_fn = fn %Date{} = expiration_date, %DB.Dataset{} = dataset -> + multi_validation = + insert(:multi_validation, + validator: Transport.Validators.GTFSTransport.validator_name(), + resource_history: insert(:resource_history, resource: insert(:resource, dataset: dataset, format: "GTFS")) + ) + + insert(:resource_metadata, + multi_validation_id: multi_validation.id, + metadata: %{"end_date" => expiration_date} + ) + end + + # Ignores hidden or inactive datasets + insert_fn.(today, insert(:dataset, is_active: false)) + insert_fn.(today, insert(:dataset, is_active: true, is_hidden: true)) + + assert [] == today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + + # 2 GTFS resources expiring on the same day for a dataset + %DB.Dataset{id: dataset_id} = dataset = insert(:dataset, is_active: true) + insert_fn.(today, dataset) + insert_fn.(today, dataset) + + assert [ + {%DB.Dataset{id: ^dataset_id}, + [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} + ] = today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + + assert [] == tomorrow |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + assert [] == yesterday |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + + insert_fn.(tomorrow, dataset) + + assert [ + {%DB.Dataset{id: ^dataset_id}, + [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} + ] = today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + + assert [ + {%DB.Dataset{id: ^dataset_id}, [%DB.Resource{dataset_id: ^dataset_id}]} + ] = tomorrow |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + + assert [] == yesterday |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + + # Multiple datasets + %DB.Dataset{id: d2_id} = d2 = insert(:dataset, is_active: true) + insert_fn.(today, d2) + + assert [ + {%DB.Dataset{id: ^dataset_id}, + [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]}, + {%DB.Dataset{id: ^d2_id}, [%DB.Resource{dataset_id: ^d2_id}]} + ] = today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + end +end From d278d6c4387e516492c935a6b650c78138086e2a Mon Sep 17 00:00:00 2001 From: Vincent Degove Date: Tue, 17 Dec 2024 15:47:21 +0100 Subject: [PATCH 7/8] Rename OutdatedDataJob to ExpirationAdminProducerJob to show proximity with reuser job --- ...ration_admin_producer_notification_job.ex} | 6 +++-- .../lib/jobs/expiration_notification_job.ex | 2 ++ ..._admin_producer_notification_job_test.exs} | 26 +++++++++---------- config/runtime.exs | 6 ++--- 4 files changed, 22 insertions(+), 18 deletions(-) rename apps/transport/lib/jobs/{outdated_data_notification_job.ex => expiration_admin_producer_notification_job.ex} (90%) rename apps/transport/test/transport/jobs/{outdated_data_notification_job_test.exs => expiration_admin_producer_notification_job_test.exs} (81%) diff --git a/apps/transport/lib/jobs/outdated_data_notification_job.ex b/apps/transport/lib/jobs/expiration_admin_producer_notification_job.ex similarity index 90% rename from apps/transport/lib/jobs/outdated_data_notification_job.ex rename to apps/transport/lib/jobs/expiration_admin_producer_notification_job.ex index e5321d1cba..ad493add7b 100644 --- a/apps/transport/lib/jobs/outdated_data_notification_job.ex +++ b/apps/transport/lib/jobs/expiration_admin_producer_notification_job.ex @@ -1,6 +1,8 @@ -defmodule Transport.Jobs.OutdatedDataNotificationJob do +defmodule Transport.Jobs.ExpirationAdminProducerNotificationJob do @moduledoc """ - This module is in charge of sending notifications to both admins and users when data is outdated. + This module is in charge of sending notifications to admins and producers when data is outdated. + It is similar to `Transport.Jobs.ExpirationNotificationJob`, dedicated to reusers. + Both could be merged in the future. """ use Oban.Worker, max_attempts: 3, tags: ["notifications"] diff --git a/apps/transport/lib/jobs/expiration_notification_job.ex b/apps/transport/lib/jobs/expiration_notification_job.ex index e8cba3cf6f..09f4f0dfce 100644 --- a/apps/transport/lib/jobs/expiration_notification_job.ex +++ b/apps/transport/lib/jobs/expiration_notification_job.ex @@ -6,6 +6,8 @@ defmodule Transport.Jobs.ExpirationNotificationJob do It has 2 `perform/1` methods: - a dispatcher one in charge of identifying contacts we should get in touch with today - another in charge of building the daily digest for a specific contact (with only their favorited datasets) + + It is similar to `Transport.Jobs.ExpirationAdminProducerNotificationJob`, dedicated to producers and admins. """ use Oban.Worker, max_attempts: 3, diff --git a/apps/transport/test/transport/jobs/outdated_data_notification_job_test.exs b/apps/transport/test/transport/jobs/expiration_admin_producer_notification_job_test.exs similarity index 81% rename from apps/transport/test/transport/jobs/outdated_data_notification_job_test.exs rename to apps/transport/test/transport/jobs/expiration_admin_producer_notification_job_test.exs index c83275757b..c456bace18 100644 --- a/apps/transport/test/transport/jobs/outdated_data_notification_job_test.exs +++ b/apps/transport/test/transport/jobs/expiration_admin_producer_notification_job_test.exs @@ -1,4 +1,4 @@ -defmodule Transport.Test.Transport.Jobs.OutdatedDataNotificationJobTest do +defmodule Transport.Test.Transport.Jobs.ExpirationAdminProducerNotificationJobTest do use ExUnit.Case, async: true import DB.Factory import Swoosh.TestAssertions @@ -30,7 +30,7 @@ defmodule Transport.Test.Transport.Jobs.OutdatedDataNotificationJobTest do ) assert [{%DB.Dataset{id: ^dataset_id}, [%DB.Resource{id: ^resource_id}]}] = - Date.utc_today() |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + Date.utc_today() |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() %DB.Contact{id: contact_id, email: email} = contact = insert_contact() @@ -54,7 +54,7 @@ defmodule Transport.Test.Transport.Jobs.OutdatedDataNotificationJobTest do dataset_id: dataset.id }) - assert :ok == perform_job(Transport.Jobs.OutdatedDataNotificationJob, %{}) + assert :ok == perform_job(Transport.Jobs.ExpirationAdminProducerNotificationJob, %{}) # a first mail to our team @@ -106,13 +106,13 @@ defmodule Transport.Test.Transport.Jobs.OutdatedDataNotificationJobTest do end test "outdated_data job with nothing to send should not send email" do - assert :ok == perform_job(Transport.Jobs.OutdatedDataNotificationJob, %{}) + assert :ok == perform_job(Transport.Jobs.ExpirationAdminProducerNotificationJob, %{}) assert_no_email_sent() end test "gtfs_datasets_expiring_on" do {today, tomorrow, yesterday} = {Date.utc_today(), Date.add(Date.utc_today(), 1), Date.add(Date.utc_today(), -1)} - assert [] == today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + assert [] == today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() insert_fn = fn %Date{} = expiration_date, %DB.Dataset{} = dataset -> multi_validation = @@ -131,7 +131,7 @@ defmodule Transport.Test.Transport.Jobs.OutdatedDataNotificationJobTest do insert_fn.(today, insert(:dataset, is_active: false)) insert_fn.(today, insert(:dataset, is_active: true, is_hidden: true)) - assert [] == today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + assert [] == today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() # 2 GTFS resources expiring on the same day for a dataset %DB.Dataset{id: dataset_id} = dataset = insert(:dataset, is_active: true) @@ -141,23 +141,23 @@ defmodule Transport.Test.Transport.Jobs.OutdatedDataNotificationJobTest do assert [ {%DB.Dataset{id: ^dataset_id}, [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} - ] = today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + ] = today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - assert [] == tomorrow |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() - assert [] == yesterday |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + assert [] == tomorrow |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() + assert [] == yesterday |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() insert_fn.(tomorrow, dataset) assert [ {%DB.Dataset{id: ^dataset_id}, [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} - ] = today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + ] = today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() assert [ {%DB.Dataset{id: ^dataset_id}, [%DB.Resource{dataset_id: ^dataset_id}]} - ] = tomorrow |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + ] = tomorrow |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - assert [] == yesterday |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + assert [] == yesterday |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() # Multiple datasets %DB.Dataset{id: d2_id} = d2 = insert(:dataset, is_active: true) @@ -167,6 +167,6 @@ defmodule Transport.Test.Transport.Jobs.OutdatedDataNotificationJobTest do {%DB.Dataset{id: ^dataset_id}, [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]}, {%DB.Dataset{id: ^d2_id}, [%DB.Resource{dataset_id: ^d2_id}]} - ] = today |> Transport.Jobs.OutdatedDataNotificationJob.gtfs_datasets_expiring_on() + ] = today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() end end diff --git a/config/runtime.exs b/config/runtime.exs index 59102d4062..19a851a01c 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -134,7 +134,8 @@ oban_prod_crontab = [ {"0 6 * * 1-5", Transport.Jobs.NewDatagouvDatasetsJob, args: %{check_rules: true}}, {"5 6 * * 1-5", Transport.Jobs.NewDatagouvDatasetsJob}, {"0 6 * * *", Transport.Jobs.NewDatasetNotificationsJob}, - {"30 6 * * *", Transport.Jobs.ExpirationNotificationJob}, + {"30 6 * * *", Transport.Jobs.ExpirationAdminProducerNotificationJob}, + {"45 6 * * *", Transport.Jobs.ExpirationNotificationJob}, {"0 8 * * 1-5", Transport.Jobs.NewCommentsNotificationJob}, {"0 21 * * *", Transport.Jobs.DatasetHistoryDispatcherJob}, # Should be executed after all `DatasetHistoryJob` have been executed @@ -160,8 +161,7 @@ oban_prod_crontab = [ {"30 5 * * *", Transport.Jobs.ImportDatasetMonthlyMetricsJob}, {"45 5 * * *", Transport.Jobs.ImportResourceMonthlyMetricsJob}, {"0 8 * * *", Transport.Jobs.WarnUserInactivityJob}, - {"*/5 * * * *", Transport.Jobs.UpdateCounterCacheJob}, - {"0 0 * * *", Transport.Jobs.OutdatedDataNotificationJob} + {"*/5 * * * *", Transport.Jobs.UpdateCounterCacheJob} ] # Make sure that all modules exist From b092f4e83956eee0ffc268a53fc6a16f1a76a3ac Mon Sep 17 00:00:00 2001 From: Vincent Degove Date: Tue, 17 Dec 2024 17:17:15 +0100 Subject: [PATCH 8/8] merge expiration jobs into a single one --- ...iration_admin_producer_notification_job.ex | 79 -------- .../lib/jobs/expiration_notification_job.ex | 102 ++++++++-- ...n_admin_producer_notification_job_test.exs | 172 ----------------- .../jobs/expiration_notification_job_test.exs | 177 +++++++++++++++++- config/runtime.exs | 3 +- 5 files changed, 262 insertions(+), 271 deletions(-) delete mode 100644 apps/transport/lib/jobs/expiration_admin_producer_notification_job.ex delete mode 100644 apps/transport/test/transport/jobs/expiration_admin_producer_notification_job_test.exs diff --git a/apps/transport/lib/jobs/expiration_admin_producer_notification_job.ex b/apps/transport/lib/jobs/expiration_admin_producer_notification_job.ex deleted file mode 100644 index ad493add7b..0000000000 --- a/apps/transport/lib/jobs/expiration_admin_producer_notification_job.ex +++ /dev/null @@ -1,79 +0,0 @@ -defmodule Transport.Jobs.ExpirationAdminProducerNotificationJob do - @moduledoc """ - This module is in charge of sending notifications to admins and producers when data is outdated. - It is similar to `Transport.Jobs.ExpirationNotificationJob`, dedicated to reusers. - Both could be merged in the future. - """ - - use Oban.Worker, max_attempts: 3, tags: ["notifications"] - import Ecto.Query - - @type delay_and_records :: {integer(), [{DB.Dataset.t(), [DB.Resource.t()]}]} - @expiration_reason Transport.NotificationReason.reason(:expiration) - # If delay < 0, the resource is already expired - @default_outdated_data_delays [-90, -60, -30, -45, -15, -7, -3, 0, 7, 14] - - @impl Oban.Worker - - def perform(%Oban.Job{id: job_id}) do - outdated_data(job_id) - :ok - end - - def outdated_data(job_id) do - for delay <- possible_delays(), - date = Date.add(Date.utc_today(), delay) do - {delay, gtfs_datasets_expiring_on(date)} - end - |> Enum.reject(fn {_, records} -> Enum.empty?(records) end) - |> send_outdated_data_admin_mail() - |> Enum.map(&send_outdated_data_producer_notifications(&1, job_id)) - end - - @spec gtfs_datasets_expiring_on(Date.t()) :: [{DB.Dataset.t(), [DB.Resource.t()]}] - def gtfs_datasets_expiring_on(%Date{} = date) do - DB.Dataset.base_query() - |> DB.Dataset.join_from_dataset_to_metadata(Transport.Validators.GTFSTransport.validator_name()) - |> where( - [metadata: m, resource: r], - fragment("TO_DATE(?->>'end_date', 'YYYY-MM-DD')", m.metadata) == ^date and r.format == "GTFS" - ) - |> select([dataset: d, resource: r], {d, r}) - |> distinct(true) - |> DB.Repo.all() - |> Enum.group_by(fn {%DB.Dataset{} = d, _} -> d end, fn {_, %DB.Resource{} = r} -> r end) - |> Enum.to_list() - end - - def possible_delays do - @default_outdated_data_delays - |> Enum.uniq() - |> Enum.sort() - end - - # A different email is sent to producers for every delay, containing all datasets expiring on this given delay - @spec send_outdated_data_producer_notifications(delay_and_records(), integer()) :: :ok - def send_outdated_data_producer_notifications({delay, records}, job_id) do - Enum.each(records, fn {%DB.Dataset{} = dataset, resources} -> - @expiration_reason - |> DB.NotificationSubscription.subscriptions_for_reason_dataset_and_role(dataset, :producer) - |> Enum.each(fn %DB.NotificationSubscription{contact: %DB.Contact{} = contact} = subscription -> - contact - |> Transport.UserNotifier.expiration_producer(dataset, resources, delay) - |> Transport.Mailer.deliver() - - DB.Notification.insert!(dataset, subscription, %{delay: delay, job_id: job_id}) - end) - end) - end - - @spec send_outdated_data_admin_mail([delay_and_records()]) :: [delay_and_records()] - defp send_outdated_data_admin_mail([] = _records), do: [] - - defp send_outdated_data_admin_mail(records) do - Transport.AdminNotifier.expiration(records) - |> Transport.Mailer.deliver() - - records - end -end diff --git a/apps/transport/lib/jobs/expiration_notification_job.ex b/apps/transport/lib/jobs/expiration_notification_job.ex index 09f4f0dfce..75dcb27c94 100644 --- a/apps/transport/lib/jobs/expiration_notification_job.ex +++ b/apps/transport/lib/jobs/expiration_notification_job.ex @@ -3,11 +3,10 @@ defmodule Transport.Jobs.ExpirationNotificationJob do This job sends daily digests to reusers about the expiration of their favorited datasets. The expiration delays is the same for all reusers and cannot be customized for now. - It has 2 `perform/1` methods: + It has 3 `perform/1` methods: - a dispatcher one in charge of identifying contacts we should get in touch with today - another in charge of building the daily digest for a specific contact (with only their favorited datasets) - - It is similar to `Transport.Jobs.ExpirationAdminProducerNotificationJob`, dedicated to producers and admins. + - and one to send to admins and producers """ use Oban.Worker, max_attempts: 3, @@ -23,11 +22,23 @@ defmodule Transport.Jobs.ExpirationNotificationJob do import Ecto.Query @notification_reason Transport.NotificationReason.reason(:expiration) + @expiration_reason Transport.NotificationReason.reason(:expiration) + @type delay_and_records :: {integer(), [{DB.Dataset.t(), [DB.Resource.t()]}]} # If delay < 0, the resource is already expired - @default_outdated_data_delays [-30, -7, 0, 7, 14] + @default_outdated_data_delays_reuser [-30, -7, 0, 7, 14] + @default_outdated_data_delays [-90, -60, -30, -45, -15, -7, -3, 0, 7, 14] @impl Oban.Worker - def perform(%Oban.Job{id: job_id, args: %{"contact_id" => contact_id, "digest_date" => digest_date}}) do + + def perform(%Oban.Job{id: job_id, args: %{"role" => "admin_and_producer"}}) do + outdated_data(job_id) + :ok + end + + def perform(%Oban.Job{ + id: job_id, + args: %{"contact_id" => contact_id, "digest_date" => digest_date, "role" => "reuser"} + }) do contact = DB.Repo.get!(DB.Contact, contact_id) subscribed_dataset_ids = subscribed_dataset_ids_for_expiration(contact) @@ -56,10 +67,12 @@ defmodule Transport.Jobs.ExpirationNotificationJob do DB.Repo.transaction( fn -> + new(%{"role" => "admin_and_producer"}) |> Oban.insert() + dataset_ids |> contact_ids_subscribed_to_dataset_ids() |> Stream.chunk_every(100) - |> Stream.each(fn contact_ids -> insert_jobs(contact_ids, target_date) end) + |> Stream.each(fn contact_ids -> insert_reuser_jobs(contact_ids, target_date) end) |> Stream.run() end, timeout: :timer.seconds(60) @@ -138,12 +151,12 @@ defmodule Transport.Jobs.ExpirationNotificationJob do def delay_str(-1), do: "périmé depuis hier" def delay_str(d) when d <= -2, do: "périmés depuis #{-d} jours" - defp insert_jobs(contact_ids, %Date{} = target_date) do + defp insert_reuser_jobs(contact_ids, %Date{} = target_date) do # Oban caveat: can't use [insert_all/2](https://hexdocs.pm/oban/Oban.html#insert_all/2): # > Only the Smart Engine in Oban Pro supports bulk unique jobs and automatic batching. # > With the basic engine, you must use insert/3 for unique support. Enum.each(contact_ids, fn contact_id -> - %{"contact_id" => contact_id, "digest_date" => target_date} + %{"contact_id" => contact_id, "digest_date" => target_date, "role" => "reuser"} |> new() |> Oban.insert() end) @@ -172,9 +185,9 @@ defmodule Transport.Jobs.ExpirationNotificationJob do Transport.Cache.fetch( to_string(__MODULE__) <> ":gtfs_expiring_on_target_dates:#{reference_date}", fn -> - delays_and_dates = delays_and_dates(reference_date) - dates_and_delays = Map.new(delays_and_dates, fn {key, value} -> {value, key} end) - expiring_dates = Map.values(delays_and_dates) + delays_and_date_reuser = delays_and_date_reuser(reference_date) + dates_and_delays = Map.new(delays_and_date_reuser, fn {key, value} -> {value, key} end) + expiring_dates = Map.values(delays_and_date_reuser) DB.Dataset.base_query() |> DB.Dataset.join_from_dataset_to_metadata(Transport.Validators.GTFSTransport.validator_name()) @@ -201,7 +214,7 @@ defmodule Transport.Jobs.ExpirationNotificationJob do end @doc """ - iex> delays_and_dates(~D[2024-05-21]) + iex> delays_and_date_reuser(~D[2024-05-21]) %{ -30 => ~D[2024-04-21], -7 => ~D[2024-05-14], @@ -210,8 +223,67 @@ defmodule Transport.Jobs.ExpirationNotificationJob do 14 => ~D[2024-06-04] } """ - @spec delays_and_dates(Date.t()) :: %{delay() => Date.t()} - def delays_and_dates(%Date{} = date) do - Map.new(@default_outdated_data_delays, fn delay -> {delay, Date.add(date, delay)} end) + @spec delays_and_date_reuser(Date.t()) :: %{delay() => Date.t()} + def delays_and_date_reuser(%Date{} = date) do + Map.new(@default_outdated_data_delays_reuser, fn delay -> {delay, Date.add(date, delay)} end) + end + + ####  HERE CODE FROM ADMIN AND PRODUCER JOB #### + + def outdated_data(job_id) do + for delay <- possible_delays(), + date = Date.add(Date.utc_today(), delay) do + {delay, gtfs_datasets_expiring_on(date)} + end + |> Enum.reject(fn {_, records} -> Enum.empty?(records) end) + |> send_outdated_data_admin_mail() + |> Enum.map(&send_outdated_data_producer_notifications(&1, job_id)) + end + + @spec gtfs_datasets_expiring_on(Date.t()) :: [{DB.Dataset.t(), [DB.Resource.t()]}] + def gtfs_datasets_expiring_on(%Date{} = date) do + DB.Dataset.base_query() + |> DB.Dataset.join_from_dataset_to_metadata(Transport.Validators.GTFSTransport.validator_name()) + |> where( + [metadata: m, resource: r], + fragment("TO_DATE(?->>'end_date', 'YYYY-MM-DD')", m.metadata) == ^date and r.format == "GTFS" + ) + |> select([dataset: d, resource: r], {d, r}) + |> distinct(true) + |> DB.Repo.all() + |> Enum.group_by(fn {%DB.Dataset{} = d, _} -> d end, fn {_, %DB.Resource{} = r} -> r end) + |> Enum.to_list() + end + + def possible_delays do + @default_outdated_data_delays + |> Enum.uniq() + |> Enum.sort() + end + + # A different email is sent to producers for every delay, containing all datasets expiring on this given delay + @spec send_outdated_data_producer_notifications(delay_and_records(), integer()) :: :ok + def send_outdated_data_producer_notifications({delay, records}, job_id) do + Enum.each(records, fn {%DB.Dataset{} = dataset, resources} -> + @expiration_reason + |> DB.NotificationSubscription.subscriptions_for_reason_dataset_and_role(dataset, :producer) + |> Enum.each(fn %DB.NotificationSubscription{contact: %DB.Contact{} = contact} = subscription -> + contact + |> Transport.UserNotifier.expiration_producer(dataset, resources, delay) + |> Transport.Mailer.deliver() + + DB.Notification.insert!(dataset, subscription, %{delay: delay, job_id: job_id}) + end) + end) + end + + @spec send_outdated_data_admin_mail([delay_and_records()]) :: [delay_and_records()] + defp send_outdated_data_admin_mail([] = _records), do: [] + + defp send_outdated_data_admin_mail(records) do + Transport.AdminNotifier.expiration(records) + |> Transport.Mailer.deliver() + + records end end diff --git a/apps/transport/test/transport/jobs/expiration_admin_producer_notification_job_test.exs b/apps/transport/test/transport/jobs/expiration_admin_producer_notification_job_test.exs deleted file mode 100644 index c456bace18..0000000000 --- a/apps/transport/test/transport/jobs/expiration_admin_producer_notification_job_test.exs +++ /dev/null @@ -1,172 +0,0 @@ -defmodule Transport.Test.Transport.Jobs.ExpirationAdminProducerNotificationJobTest do - use ExUnit.Case, async: true - import DB.Factory - import Swoosh.TestAssertions - use Oban.Testing, repo: DB.Repo - - setup do - Ecto.Adapters.SQL.Sandbox.checkout(DB.Repo) - end - - test "sends email to our team + relevant contact before expiry" do - %DB.Dataset{id: dataset_id} = - dataset = - insert(:dataset, is_active: true, custom_title: "Dataset custom title", custom_tags: ["loi-climat-resilience"]) - - assert DB.Dataset.climate_resilience_bill?(dataset) - # fake a resource expiring today - %DB.Resource{id: resource_id} = - resource = insert(:resource, dataset: dataset, format: "GTFS", title: resource_title = "Super GTFS") - - multi_validation = - insert(:multi_validation, - validator: Transport.Validators.GTFSTransport.validator_name(), - resource_history: insert(:resource_history, resource: resource) - ) - - insert(:resource_metadata, - multi_validation_id: multi_validation.id, - metadata: %{"end_date" => Date.utc_today()} - ) - - assert [{%DB.Dataset{id: ^dataset_id}, [%DB.Resource{id: ^resource_id}]}] = - Date.utc_today() |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - - %DB.Contact{id: contact_id, email: email} = contact = insert_contact() - - %DB.NotificationSubscription{id: ns_id} = - insert(:notification_subscription, %{ - reason: :expiration, - source: :admin, - role: :producer, - contact_id: contact_id, - dataset_id: dataset.id - }) - - # Should be ignored, this subscription is for a reuser - %DB.Contact{id: reuser_id} = insert_contact() - - insert(:notification_subscription, %{ - reason: :expiration, - source: :user, - role: :reuser, - contact_id: reuser_id, - dataset_id: dataset.id - }) - - assert :ok == perform_job(Transport.Jobs.ExpirationAdminProducerNotificationJob, %{}) - - # a first mail to our team - - assert_email_sent(fn %Swoosh.Email{ - from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, - to: [{"", "contact@transport.data.gouv.fr"}], - subject: "Jeux de données arrivant à expiration", - text_body: nil, - html_body: body - } -> - assert body =~ ~r/Jeux de données périmant demain :/ - - assert body =~ - ~s|
  • #{dataset.custom_title} - ✅ notification automatique ⚖️🗺️ article 122
    • | - end) - - # a second mail to the email address in the notifications config - display_name = DB.Contact.display_name(contact) - - assert_email_sent(fn %Swoosh.Email{ - from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, - to: [{^display_name, ^email}], - subject: "Jeu de données arrivant à expiration", - html_body: html_body - } -> - refute html_body =~ "notification automatique" - refute html_body =~ "article 122" - - assert html_body =~ - ~s(Les données GTFS #{resource_title} associées au jeu de données #{dataset.custom_title} périment demain.) - - assert html_body =~ - ~s(remplaçant la ressource périmée par la nouvelle) - end) - - # Logs are there - assert [ - %DB.Notification{ - contact_id: ^contact_id, - email: ^email, - reason: :expiration, - dataset_id: ^dataset_id, - notification_subscription_id: ^ns_id, - role: :producer, - payload: %{"delay" => 0, "job_id" => _job_id} - } - ] = - DB.Notification |> DB.Repo.all() - end - - test "outdated_data job with nothing to send should not send email" do - assert :ok == perform_job(Transport.Jobs.ExpirationAdminProducerNotificationJob, %{}) - assert_no_email_sent() - end - - test "gtfs_datasets_expiring_on" do - {today, tomorrow, yesterday} = {Date.utc_today(), Date.add(Date.utc_today(), 1), Date.add(Date.utc_today(), -1)} - assert [] == today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - - insert_fn = fn %Date{} = expiration_date, %DB.Dataset{} = dataset -> - multi_validation = - insert(:multi_validation, - validator: Transport.Validators.GTFSTransport.validator_name(), - resource_history: insert(:resource_history, resource: insert(:resource, dataset: dataset, format: "GTFS")) - ) - - insert(:resource_metadata, - multi_validation_id: multi_validation.id, - metadata: %{"end_date" => expiration_date} - ) - end - - # Ignores hidden or inactive datasets - insert_fn.(today, insert(:dataset, is_active: false)) - insert_fn.(today, insert(:dataset, is_active: true, is_hidden: true)) - - assert [] == today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - - # 2 GTFS resources expiring on the same day for a dataset - %DB.Dataset{id: dataset_id} = dataset = insert(:dataset, is_active: true) - insert_fn.(today, dataset) - insert_fn.(today, dataset) - - assert [ - {%DB.Dataset{id: ^dataset_id}, - [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} - ] = today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - - assert [] == tomorrow |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - assert [] == yesterday |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - - insert_fn.(tomorrow, dataset) - - assert [ - {%DB.Dataset{id: ^dataset_id}, - [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} - ] = today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - - assert [ - {%DB.Dataset{id: ^dataset_id}, [%DB.Resource{dataset_id: ^dataset_id}]} - ] = tomorrow |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - - assert [] == yesterday |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - - # Multiple datasets - %DB.Dataset{id: d2_id} = d2 = insert(:dataset, is_active: true) - insert_fn.(today, d2) - - assert [ - {%DB.Dataset{id: ^dataset_id}, - [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]}, - {%DB.Dataset{id: ^d2_id}, [%DB.Resource{dataset_id: ^d2_id}]} - ] = today |> Transport.Jobs.ExpirationAdminProducerNotificationJob.gtfs_datasets_expiring_on() - end -end diff --git a/apps/transport/test/transport/jobs/expiration_notification_job_test.exs b/apps/transport/test/transport/jobs/expiration_notification_job_test.exs index 28c19f842c..47bfb825c0 100644 --- a/apps/transport/test/transport/jobs/expiration_notification_job_test.exs +++ b/apps/transport/test/transport/jobs/expiration_notification_job_test.exs @@ -29,7 +29,7 @@ defmodule Transport.Test.Transport.Jobs.ExpirationNotificationJobTest do %DB.Contact{id: c2_id} = c2 = insert_contact() # Subscriptions for `c1`: should not match because: - # - is a producer for a relevant expiration delay (+7) + # - is a producer for a relevant expiration delay (+7). Producers have a distinct job but for all producers together # - is a reuser but for an ignored expiration delay # - is a reuser for an irrelevant reason for a matching dataset insert(:notification_subscription, @@ -57,6 +57,7 @@ defmodule Transport.Test.Transport.Jobs.ExpirationNotificationJobTest do ) # Subscriptions for `c2`: matches for expiration today and for +7 + # Does a single email insert(:notification_subscription, contact_id: c2.id, dataset_id: d1.id, @@ -81,7 +82,13 @@ defmodule Transport.Test.Transport.Jobs.ExpirationNotificationJobTest do assert [ %Oban.Job{ worker: "Transport.Jobs.ExpirationNotificationJob", - args: %{"contact_id" => ^c2_id, "digest_date" => ^today_str}, + args: %{"contact_id" => ^c2_id, "digest_date" => ^today_str, "role" => "reuser"}, + conflict?: false, + state: "available" + }, + %Oban.Job{ + worker: "Transport.Jobs.ExpirationNotificationJob", + args: %{"role" => "admin_and_producer"}, conflict?: false, state: "available" } @@ -132,7 +139,7 @@ defmodule Transport.Test.Transport.Jobs.ExpirationNotificationJobTest do ExpirationNotificationJob.gtfs_expiring_on_target_dates(today) assert :ok == - perform_job(ExpirationNotificationJob, %{contact_id: contact.id, digest_date: today}, + perform_job(ExpirationNotificationJob, %{contact_id: contact.id, digest_date: today, role: "reuser"}, inserted_at: DateTime.utc_now() ) @@ -186,4 +193,168 @@ defmodule Transport.Test.Transport.Jobs.ExpirationNotificationJobTest do assert %Oban.Job{conflict?: false, unique: %{fields: [:args, :queue, :worker], period: 72_000}} = enqueue_job.() assert %Oban.Job{conflict?: true, unique: nil} = enqueue_job.() end + + describe "admins and producer notifications" do + test "sends email to our team + relevant contact before expiry" do + %DB.Dataset{id: dataset_id} = + dataset = + insert(:dataset, is_active: true, custom_title: "Dataset custom title", custom_tags: ["loi-climat-resilience"]) + + assert DB.Dataset.climate_resilience_bill?(dataset) + # fake a resource expiring today + %DB.Resource{id: resource_id} = + resource = insert(:resource, dataset: dataset, format: "GTFS", title: resource_title = "Super GTFS") + + multi_validation = + insert(:multi_validation, + validator: Transport.Validators.GTFSTransport.validator_name(), + resource_history: insert(:resource_history, resource: resource) + ) + + insert(:resource_metadata, + multi_validation_id: multi_validation.id, + metadata: %{"end_date" => Date.utc_today()} + ) + + assert [{%DB.Dataset{id: ^dataset_id}, [%DB.Resource{id: ^resource_id}]}] = + Date.utc_today() |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + + %DB.Contact{id: contact_id, email: email} = contact = insert_contact() + + %DB.NotificationSubscription{id: ns_id} = + insert(:notification_subscription, %{ + reason: :expiration, + source: :admin, + role: :producer, + contact_id: contact_id, + dataset_id: dataset.id + }) + + # Should be ignored, this subscription is for a reuser + %DB.Contact{id: reuser_id} = insert_contact() + + insert(:notification_subscription, %{ + reason: :expiration, + source: :user, + role: :reuser, + contact_id: reuser_id, + dataset_id: dataset.id + }) + + assert :ok == perform_job(Transport.Jobs.ExpirationNotificationJob, %{"role" => "admin_and_producer"}) + + # a first mail to our team + + assert_email_sent(fn %Swoosh.Email{ + from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, + to: [{"", "contact@transport.data.gouv.fr"}], + subject: "Jeux de données arrivant à expiration", + text_body: nil, + html_body: body + } -> + assert body =~ ~r/Jeux de données périmant demain :/ + + assert body =~ + ~s|
  • #{dataset.custom_title} - ✅ notification automatique ⚖️🗺️ article 122
    • | + end) + + # a second mail to the email address in the notifications config + display_name = DB.Contact.display_name(contact) + + assert_email_sent(fn %Swoosh.Email{ + from: {"transport.data.gouv.fr", "contact@transport.data.gouv.fr"}, + to: [{^display_name, ^email}], + subject: "Jeu de données arrivant à expiration", + html_body: html_body + } -> + refute html_body =~ "notification automatique" + refute html_body =~ "article 122" + + assert html_body =~ + ~s(Les données GTFS #{resource_title} associées au jeu de données #{dataset.custom_title} périment demain.) + + assert html_body =~ + ~s(remplaçant la ressource périmée par la nouvelle) + end) + + # Logs are there + assert [ + %DB.Notification{ + contact_id: ^contact_id, + email: ^email, + reason: :expiration, + dataset_id: ^dataset_id, + notification_subscription_id: ^ns_id, + role: :producer, + payload: %{"delay" => 0, "job_id" => _job_id} + } + ] = + DB.Notification |> DB.Repo.all() + end + + test "outdated_data job with nothing to send should not send email" do + assert :ok == perform_job(Transport.Jobs.ExpirationNotificationJob, %{"role" => "admin_and_producer"}) + assert_no_email_sent() + end + + test "gtfs_datasets_expiring_on" do + {today, tomorrow, yesterday} = {Date.utc_today(), Date.add(Date.utc_today(), 1), Date.add(Date.utc_today(), -1)} + assert [] == today |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + + insert_fn = fn %Date{} = expiration_date, %DB.Dataset{} = dataset -> + multi_validation = + insert(:multi_validation, + validator: Transport.Validators.GTFSTransport.validator_name(), + resource_history: insert(:resource_history, resource: insert(:resource, dataset: dataset, format: "GTFS")) + ) + + insert(:resource_metadata, + multi_validation_id: multi_validation.id, + metadata: %{"end_date" => expiration_date} + ) + end + + # Ignores hidden or inactive datasets + insert_fn.(today, insert(:dataset, is_active: false)) + insert_fn.(today, insert(:dataset, is_active: true, is_hidden: true)) + + assert [] == today |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + + # 2 GTFS resources expiring on the same day for a dataset + %DB.Dataset{id: dataset_id} = dataset = insert(:dataset, is_active: true) + insert_fn.(today, dataset) + insert_fn.(today, dataset) + + assert [ + {%DB.Dataset{id: ^dataset_id}, + [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} + ] = today |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + + assert [] == tomorrow |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + assert [] == yesterday |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + + insert_fn.(tomorrow, dataset) + + assert [ + {%DB.Dataset{id: ^dataset_id}, + [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]} + ] = today |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + + assert [ + {%DB.Dataset{id: ^dataset_id}, [%DB.Resource{dataset_id: ^dataset_id}]} + ] = tomorrow |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + + assert [] == yesterday |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + + # Multiple datasets + %DB.Dataset{id: d2_id} = d2 = insert(:dataset, is_active: true) + insert_fn.(today, d2) + + assert [ + {%DB.Dataset{id: ^dataset_id}, + [%DB.Resource{dataset_id: ^dataset_id}, %DB.Resource{dataset_id: ^dataset_id}]}, + {%DB.Dataset{id: ^d2_id}, [%DB.Resource{dataset_id: ^d2_id}]} + ] = today |> Transport.Jobs.ExpirationNotificationJob.gtfs_datasets_expiring_on() + end + end end diff --git a/config/runtime.exs b/config/runtime.exs index 19a851a01c..3c72728c3e 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -134,8 +134,7 @@ oban_prod_crontab = [ {"0 6 * * 1-5", Transport.Jobs.NewDatagouvDatasetsJob, args: %{check_rules: true}}, {"5 6 * * 1-5", Transport.Jobs.NewDatagouvDatasetsJob}, {"0 6 * * *", Transport.Jobs.NewDatasetNotificationsJob}, - {"30 6 * * *", Transport.Jobs.ExpirationAdminProducerNotificationJob}, - {"45 6 * * *", Transport.Jobs.ExpirationNotificationJob}, + {"30 6 * * *", Transport.Jobs.ExpirationNotificationJob}, {"0 8 * * 1-5", Transport.Jobs.NewCommentsNotificationJob}, {"0 21 * * *", Transport.Jobs.DatasetHistoryDispatcherJob}, # Should be executed after all `DatasetHistoryJob` have been executed