Skip to content

Commit

Permalink
Filter upgrade notifications on users with active clusters (#1231)
Browse files Browse the repository at this point in the history
  • Loading branch information
michaeljguarino authored Aug 31, 2023
1 parent be6ec31 commit b0c7e09
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 23 deletions.
10 changes: 9 additions & 1 deletion apps/core/lib/core/schema/notification.ex
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
defmodule Core.Schema.Notification do
use Piazza.Ecto.Schema
alias Core.Schema.{User, Incident, IncidentMessage, Repository}
alias Core.Schema.{User, Incident, IncidentMessage, Repository, Cluster}

defenum Type, message: 0, incident_update: 1, mention: 2, locked: 3, pending: 4

Expand All @@ -20,6 +20,14 @@ defmodule Core.Schema.Notification do
timestamps()
end

def active_cluster(query \\ __MODULE__) do
from(n in query,
left_join: c in Cluster,
on: c.owner_id == n.user_id,
where: not is_nil(c.id)
)
end

def expired(query \\ __MODULE__) do
expires_at = Timex.now() |> Timex.shift(@expiry)
from(n in query, where: n.inserted_at <= ^expires_at)
Expand Down
6 changes: 6 additions & 0 deletions apps/core/lib/core/services/clusters.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ defmodule Core.Services.Clusters do
@spec get_cluster_by_owner(binary) :: Cluster.t | nil
def get_cluster_by_owner(user_id), do: Core.Repo.get_by(Cluster, owner_id: user_id)

@spec has_cluster?(User.t) :: boolean
def has_cluster?(%User{id: user_id}) do
Cluster.for_user(user_id)
|> Core.Repo.exists?()
end

@doc """
Determines if a user has access to the cluster referenced by `id`
"""
Expand Down
1 change: 1 addition & 0 deletions apps/cron/lib/cron/digest/locked.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ defmodule Cron.Digest.Locked do
Notification.for_type(:locked)
|> Notification.ordered(asc: :user_id)
|> Notification.preloaded([:repository])
|> Notification.active_cluster()
|> Core.Repo.all()
|> grouped(stages: 3)
|> Flow.map(fn {user, repos} -> Email.Builder.Digest.locked(user, repos) end)
Expand Down
1 change: 1 addition & 0 deletions apps/cron/lib/cron/digest/pending.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ defmodule Cron.Digest.Pending do
Notification.for_type(:pending)
|> Notification.ordered(asc: :user_id)
|> Notification.preloaded([:repository])
|> Notification.active_cluster()
|> Core.Repo.all()
|> grouped(stages: 3)
|> Flow.map(fn {user, repos} -> Email.Builder.Digest.pending(user, repos) end)
Expand Down
3 changes: 2 additions & 1 deletion apps/cron/test/cron/digest/locked_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ defmodule Cron.Digest.LockedTest do

describe "#run/0" do
test "it will prune old, unused upgrade queues" do
[user1, user2] = insert_list(2, :user)
[user1, user2] = users = insert_list(2, :user)
for u <- users, do: insert(:cluster, owner: u)
notifs = for _ <- 1..3, do: insert(:notification, user: user1, repository: insert(:repository), type: :locked)
insert_list(2, :notification, user: user1, type: :pending)
notifs2 = for _ <- 1..2, do: insert(:notification, user: user2, repository: insert(:repository), type: :locked)
Expand Down
3 changes: 2 additions & 1 deletion apps/cron/test/cron/digest/pending_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ defmodule Cron.Digest.PendingTest do

describe "#run/0" do
test "it will prune old, unused upgrade queues" do
[user1, user2] = insert_list(2, :user)
[user1, user2] = users = insert_list(2, :user)
for u <- users, do: insert(:cluster, owner: u)
notifs = for _ <- 1..3, do: insert(:notification, user: user1, repository: insert(:repository), type: :pending)
insert_list(2, :notification, user: user1, type: :locked)
notifs2 = for _ <- 1..2, do: insert(:notification, user: user2, repository: insert(:repository), type: :pending)
Expand Down
10 changes: 9 additions & 1 deletion apps/email/lib/email/builder/base.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,19 @@ defmodule Email.Builder.Base do
use Bamboo.Phoenix, view: EmailWeb.EmailView
import Email.Builder.Base, only: [
base_email: 0,
expand_service_account: 1
expand_service_account: 1,
active_cluster: 2
]
end
end

def active_cluster(%User{} = user, fun) when is_function(fun) do
case Core.Services.Clusters.has_cluster?(user) do
true -> fun.()
false -> :ok
end
end

def base_email() do
new_email()
|> from("Plural.sh<[email protected]>")
Expand Down
25 changes: 14 additions & 11 deletions apps/email/lib/email/builder/locked_installation.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@ defmodule Email.Builder.LockedInstallation do

def email(inst) do
%{installation: %{user: user}} = inst = Core.Repo.preload(inst, [:version, installation: [:user, :repository]])
repo = inst.installation.repository
base_email()
|> to(expand_service_account(user))
|> subject("Manual changes need to be applied for your #{repo.name} installation")
|> assign(:type, inst_type(inst))
|> assign(:user, user)
|> assign(:installation, inst)
|> assign(:deps, inst.version.dependencies)
|> assign(:instructions, instructions(inst.version))
|> assign(:repo, repo)
|> render(:locked_installation)

active_cluster(user, fn ->
repo = inst.installation.repository
base_email()
|> to(expand_service_account(user))
|> subject("Manual changes need to be applied for your #{repo.name} installation")
|> assign(:type, inst_type(inst))
|> assign(:user, user)
|> assign(:installation, inst)
|> assign(:deps, inst.version.dependencies)
|> assign(:instructions, instructions(inst.version))
|> assign(:repo, repo)
|> render(:locked_installation)
end)
end

defp instructions(%Version{dependencies: %Dependencies{instructions: %Dependencies.ChangeInstructions{} = instructions}}),
Expand Down
14 changes: 8 additions & 6 deletions apps/email/lib/email/builder/pending_promotion.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ defmodule Email.Builder.PendingPromotion do
])
repo = find_repo(update)

base_email()
|> to(expand_service_account(user))
|> subject("You have an upgrade pending promotion for #{repo.name}")
|> assign(:user, user)
|> assign(:repo, repo)
|> render(:pending_promotion)
active_cluster(user, fn ->
base_email()
|> to(expand_service_account(user))
|> subject("You have an upgrade pending promotion for #{repo.name}")
|> assign(:user, user)
|> assign(:repo, repo)
|> render(:pending_promotion)
end)
end

defp find_repo(%{terraform_installation: %{installation: %{repository: repo}}}), do: repo
Expand Down
2 changes: 2 additions & 0 deletions apps/email/test/email/builder/locked_installation_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ defmodule Email.Builder.LockedInstallationTest do
policy: build(:impersonation_policy, user: service_account),
group: insert(:group, account: service_account.account)
)
insert(:cluster, owner: service_account)
insert(:group_member, group: group, user: user)

inst = insert(:installation, user: service_account)
Expand All @@ -30,6 +31,7 @@ defmodule Email.Builder.LockedInstallationTest do
policy: build(:impersonation_policy, user: service_account),
group: insert(:group, account: service_account.account)
)
insert(:cluster, owner: service_account)
insert(:group_member, group: group, user: user)

inst = insert(:installation, user: service_account)
Expand Down
26 changes: 24 additions & 2 deletions apps/email/test/email/deliverable/versions_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,55 @@ defmodule Email.Deliverable.VersionsTest do
alias Email.PubSub.Consumer

describe "InstallationLocked" do
test "it can send reset emails" do
test "it can send lock emails" do
ci = insert(:chart_installation,
version: build(:version,
dependencies: %{instructions: %{script: "blach", instructions: nil}}
)
)
insert(:cluster, owner: ci.installation.user)

event = %PubSub.InstallationLocked{item: ci}
Consumer.handle_event(event)
{:ok, _} = Consumer.handle_event(event)

assert_delivered_email Email.Builder.LockedInstallation.email(ci)
end

test "it will ignore if there is no cluster" do
ci = insert(:chart_installation,
version: build(:version,
dependencies: %{instructions: %{script: "blach", instructions: nil}}
)
)

event = %PubSub.InstallationLocked{item: ci}
:ok = Consumer.handle_event(event)
end
end

describe "DeferredUpdateCreated" do
test "it can send promotion emails for pending updates" do
ci_inst = insert(:chart_installation)
ci = insert(:deferred_update, pending: true, chart_installation: ci_inst, version: ci_inst.version)
insert(:cluster, owner: ci.user)

event = %PubSub.DeferredUpdateCreated{item: ci}
{:ok, _} = Consumer.handle_event(event)

assert_delivered_email Email.Builder.PendingPromotion.email(ci)
end

test "it will ignore if the user no longer has a cluster" do
ci_inst = insert(:chart_installation)
ci = insert(:deferred_update, pending: true, chart_installation: ci_inst, version: ci_inst.version)

event = %PubSub.DeferredUpdateCreated{item: ci}
:ok = Consumer.handle_event(event)
end

test "it ignores for non pending updates" do
ci = insert(:deferred_update, pending: false)
insert(:cluster, owner: ci.user)

event = %PubSub.DeferredUpdateCreated{item: ci}
:ok = Consumer.handle_event(event)
Expand Down

0 comments on commit b0c7e09

Please sign in to comment.