Do Not Send Digest Emails When Project Has No Workflow (#2858)

* Tests for digest emails when no activity

* Project collaborators digest emails are by default turned off

* Skip digest emails when project has no workflow

* Update CL

---------

Co-authored-by: Stuart Corbishley <[email protected]>

CHANGELOG.md

@@ -50,6 +50,8 @@ and this project adheres to
 
 ### Fixed
 
+- Do not send digest emails for projects with no workflows
+  [#2688](https://github.com/OpenFn/lightning/issues/2688)
 - Fixed navbar items alignment in the workflow builder
   [#2825](https://github.com/OpenFn/lightning/issues/2825)
 

lib/lightning/accounts/user_notifier.ex

@@ -339,6 +339,7 @@ defmodule Lightning.Accounts.UserNotifier do
           })
         )
       end)
+      |> String.trim()
 
     body = """
     Hi #{user.first_name},

lib/lightning/digest_email_worker.ex

@@ -51,32 +51,40 @@ defmodule Lightning.DigestEmailWorker do
       )
       |> Repo.all()
 
-    project_users
-    |> Enum.group_by(& &1.project.id)
-    |> Enum.each(fn {_project_id, project_users} ->
-      [%{project: project} | _other] = project_users
-
-      project_digest_data =
-        Workflows.get_workflows_for(project)
-        |> Enum.map(fn workflow ->
-          get_digest_data(workflow, start_date, end_date)
-        end)
-
-      Enum.each(project_users, fn pu ->
-        UserNotifier.deliver_project_digest(
-          project_digest_data,
-          %{
-            user: pu.user,
-            project: pu.project,
-            digest: digest,
-            start_date: start_date,
-            end_date: end_date
-          }
-        )
+    {notified_users, skipped_users} =
+      project_users
+      |> Enum.group_by(& &1.project.id)
+      |> Enum.reduce({[], []}, fn {_project_id, project_users},
+                                  {notified_acc, skipped_acc} ->
+        [%{project: project} | _other] = project_users
+        workflows = Workflows.get_workflows_for(project)
+
+        if length(workflows) > 0 do
+          project_digest_data =
+            Enum.map(workflows, fn workflow ->
+              get_digest_data(workflow, start_date, end_date)
+            end)
+
+          Enum.each(project_users, fn pu ->
+            UserNotifier.deliver_project_digest(
+              project_digest_data,
+              %{
+                user: pu.user,
+                project: pu.project,
+                digest: digest,
+                start_date: start_date,
+                end_date: end_date
+              }
+            )
+          end)
+
+          {notified_acc ++ project_users, skipped_acc}
+        else
+          {notified_acc, skipped_acc ++ project_users}
+        end
       end)
-    end)
 
-    {:ok, project_users}
+    {:ok, %{notified_users: notified_users, skipped_users: skipped_users}}
   end
 
   def digest_to_date(digest) do

lib/lightning/projects/project_user.ex

@@ -36,7 +36,7 @@ defmodule Lightning.Projects.ProjectUser do
     field :delete, :boolean, virtual: true
     field :failure_alert, :boolean, default: true
     field :role, RolesEnum
-    field :digest, DigestEnum, default: :weekly
+    field :digest, DigestEnum, default: :never
 
     timestamps()
   end

test/lightning/accounts/user_notifier_test.exs

@@ -271,8 +271,6 @@ defmodule Lightning.Accounts.UserNotifierTest do
         Click the link below to view this in the history page:
         #{UserNotifier.build_digest_url(workflow_c, start_date, end_date)}
 
-
-
         OpenFn
         """
       )
@@ -349,8 +347,6 @@ defmodule Lightning.Accounts.UserNotifierTest do
         Click the link below to view this in the history page:
         #{UserNotifier.build_digest_url(workflow_c, start_date, end_date)}
 
-
-
         OpenFn
         """
       )
@@ -429,13 +425,65 @@ defmodule Lightning.Accounts.UserNotifierTest do
         Click the link below to view this in the history page:
         #{UserNotifier.build_digest_url(workflow_c, start_date, end_date)}
 
-
-
         OpenFn
         """
       )
     end
 
+    test "digest emails with no activity" do
+      user = %User{email: "[email protected]", first_name: "Elias"}
+      project = %Project{name: "Real Project"}
+      workflow = Lightning.WorkflowsFixtures.workflow_fixture(name: "Workflow A")
+
+      data = [
+        %{
+          workflow: workflow,
+          successful_workorders: 0,
+          rerun_workorders: 0,
+          failed_workorders: 0
+        }
+      ]
+
+      for digest_type <- [:daily, :weekly, :monthly] do
+        period =
+          case digest_type do
+            :daily -> "today"
+            :weekly -> "this week"
+            :monthly -> "this month"
+          end
+
+        start_date = DigestEmailWorker.digest_to_date(digest_type)
+        end_date = Timex.now()
+
+        UserNotifier.deliver_project_digest(data, %{
+          user: user,
+          project: project,
+          digest: digest_type,
+          start_date: start_date,
+          end_date: end_date
+        })
+
+        assert_email_sent(
+          subject:
+            "#{String.capitalize("#{digest_type}")} digest for project Real Project",
+          to: Swoosh.Email.Recipient.format(user),
+          text_body: """
+          Hi Elias,
+
+          Here's your #{digest_type} project digest for "Real Project", covering activity from #{start_date |> Calendar.strftime("%a %B %d %Y at %H:%M %Z")} to #{end_date |> Calendar.strftime("%a %B %d %Y at %H:%M %Z")}.
+
+          Workflow A:
+          - 0 workorders correctly processed #{period}
+          - 0 work orders that failed, crashed or timed out
+          Click the link below to view this in the history page:
+          #{UserNotifier.build_digest_url(workflow, start_date, end_date)}
+
+          OpenFn
+          """
+        )
+      end
+    end
+
     test "Kafka trigger failure - alternate storage disabled" do
       stub(Lightning.MockConfig, :kafka_alternate_storage_enabled?, fn ->
         false

test/lightning/digest_email_worker_test.exs

@@ -1,11 +1,7 @@
 defmodule Lightning.DigestEmailWorkerTest do
   use Lightning.DataCase, async: true
 
-  alias Lightning.{
-    AccountsFixtures,
-    ProjectsFixtures,
-    DigestEmailWorker
-  }
+  alias Lightning.DigestEmailWorker
 
   import Lightning.Factories
 
@@ -21,63 +17,92 @@ defmodule Lightning.DigestEmailWorkerTest do
           ]
         )
 
-      assert project.project_users |> length() == 1
+      assert length(project.project_users) == 1
 
-      {:ok, notified_project_users} =
+      {:ok, result} =
         DigestEmailWorker.perform(%Oban.Job{
           args: %{"type" => "daily_project_digest"}
         })
 
-      assert notified_project_users |> length() == 0
+      assert length(result.notified_users) == 0
+      assert length(result.skipped_users) == 0
     end
 
     test "all project users of different project that have a digest of :daily, :weekly, and :monthly" do
-      user_1 = AccountsFixtures.user_fixture()
-      user_2 = AccountsFixtures.user_fixture()
-      user_3 = AccountsFixtures.user_fixture()
-
-      ProjectsFixtures.project_fixture(
-        project_users: [
-          %{user_id: user_1.id, digest: :daily},
-          %{user_id: user_2.id, digest: :weekly},
-          %{user_id: user_3.id, digest: :monthly}
-        ]
-      )
+      [user_1, user_2, user_3] = insert_list(3, :user)
 
-      ProjectsFixtures.project_fixture(
-        project_users: [
-          %{user_id: user_1.id, digest: :monthly},
-          %{user_id: user_2.id, digest: :daily},
-          %{user_id: user_3.id, digest: :daily}
-        ]
-      )
+      project_1 =
+        insert(:project,
+          project_users: [
+            %{user_id: user_1.id, digest: :daily},
+            %{user_id: user_2.id, digest: :weekly},
+            %{user_id: user_3.id, digest: :monthly}
+          ]
+        )
 
-      ProjectsFixtures.project_fixture(
-        project_users: [
-          %{user_id: user_1.id, digest: :weekly},
-          %{user_id: user_2.id, digest: :daily},
-          %{user_id: user_3.id, digest: :weekly}
-        ]
-      )
+      project_2 =
+        insert(:project,
+          project_users: [
+            %{user_id: user_1.id, digest: :monthly},
+            %{user_id: user_2.id, digest: :daily},
+            %{user_id: user_3.id, digest: :daily}
+          ]
+        )
 
-      {:ok, daily_project_users} =
+      project_3 =
+        insert(:project,
+          project_users: [
+            %{user_id: user_1.id, digest: :weekly},
+            %{user_id: user_2.id, digest: :daily},
+            %{user_id: user_3.id, digest: :weekly}
+          ]
+        )
+
+      insert(:simple_workflow, project: project_1)
+      insert(:simple_workflow, project: project_2)
+      insert(:simple_workflow, project: project_3)
+
+      {:ok, daily_result} =
         DigestEmailWorker.perform(%Oban.Job{
           args: %{"type" => "daily_project_digest"}
         })
 
-      {:ok, weekly_project_users} =
+      {:ok, weekly_result} =
         DigestEmailWorker.perform(%Oban.Job{
           args: %{"type" => "weekly_project_digest"}
         })
 
-      {:ok, monthly_project_users} =
+      {:ok, monthly_result} =
         DigestEmailWorker.perform(%Oban.Job{
           args: %{"type" => "monthly_project_digest"}
         })
 
-      assert daily_project_users |> length() == 4
-      assert weekly_project_users |> length() == 3
-      assert monthly_project_users |> length() == 2
+      assert length(daily_result.notified_users) == 4
+      assert length(weekly_result.notified_users) == 3
+      assert length(monthly_result.notified_users) == 2
+
+      assert length(daily_result.skipped_users) == 0
+      assert length(weekly_result.skipped_users) == 0
+      assert length(monthly_result.skipped_users) == 0
+    end
+
+    test "skips users when project has no workflows" do
+      user = insert(:user)
+
+      _project =
+        insert(:project,
+          project_users: [
+            %{user_id: user.id, digest: :daily}
+          ]
+        )
+
+      {:ok, result} =
+        DigestEmailWorker.perform(%Oban.Job{
+          args: %{"type" => "daily_project_digest"}
+        })
+
+      assert result.notified_users == []
+      assert length(result.skipped_users) == 1
     end
   end
 
@@ -118,6 +143,27 @@ defmodule Lightning.DigestEmailWorkerTest do
                  workflow: workflow_b
                }
     end
+
+    test "Gets project digest data with no activity for all digest periods" do
+      user = insert(:user)
+
+      project =
+        insert(:project, project_users: [%{user_id: user.id, digest: :daily}])
+
+      workflow = insert(:simple_workflow, project: project)
+
+      for period <- [:daily, :weekly, :monthly] do
+        start_date = DigestEmailWorker.digest_to_date(period)
+        end_date = Timex.now()
+
+        assert DigestEmailWorker.get_digest_data(workflow, start_date, end_date) ==
+                 %{
+                   failed_workorders: 0,
+                   successful_workorders: 0,
+                   workflow: workflow
+                 }
+      end
+    end
   end
 
   defp create_runs(

test/lightning/projects_test.exs

@@ -1471,6 +1471,17 @@ defmodule Lightning.ProjectsTest do
       assert {:ok, _} =
                Projects.invite_collaborators(project, collaborators, user)
     end
+
+    test "project collaborators digest emails are by default turned off" do
+      user = insert(:user)
+
+      project =
+        insert(:project, project_users: [%{user_id: user.id}])
+
+      assert project.project_users
+             |> Enum.map(& &1.digest)
+             |> Enum.all?(&(&1 == :never))
+    end
   end
 
   describe ".find_users_to_notify_of_trigger_failure/1" do