Migrate all new dataset notification code to job instead of datachecker

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

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

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", "[email protected]"},
-        to: {DB.Contact.display_name(contact), email},
-        subject: "Nouveaux jeux de données référencés",
-        text_body: nil,
-        html_body:
-          ~r|<li><a href="http://127.0.0.1:5100/datasets/#{dataset.slug}">#{dataset.custom_title}</a> - \(Transport public collectif - horaires théoriques\)</li>|
-      )
-
-      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()}

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|<p>Bonjour,</p>|
+      html_body:
+        ~r|<li><a href="http://127.0.0.1:5100/datasets/#{dataset.slug}">#{dataset.custom_title}</a> - \(Transport public collectif - horaires théoriques\)</li>|
     )
 
     # 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