From 0f9266027fb224f24ba08a8562f500d9564d6473 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sat, 3 Jan 2026 12:56:40 -0600 Subject: [PATCH] fix dialyzer things --- lib/towerops/accounts.ex | 25 +++--- lib/towerops/alerts.ex | 2 +- lib/towerops/monitoring/equipment_monitor.ex | 85 ++++++++++--------- lib/towerops/monitoring/supervisor.ex | 15 ++-- lib/towerops/organizations.ex | 62 ++++++++------ lib/towerops/organizations/invitation.ex | 28 ++++-- lib/towerops/release.ex | 6 +- .../controllers/user_session_controller.ex | 13 +-- .../controllers/user_settings_controller.ex | 11 +-- lib/towerops_web/live/alert_live/index.ex | 11 +-- lib/towerops_web/live/dashboard_live.ex | 9 +- lib/towerops_web/live/equipment_live/show.ex | 7 +- test/towerops/alerts/alert_notifier_test.exs | 19 +++-- 13 files changed, 167 insertions(+), 126 deletions(-) diff --git a/lib/towerops/accounts.ex b/lib/towerops/accounts.ex index 143c5ed7..ffdb7087 100644 --- a/lib/towerops/accounts.ex +++ b/lib/towerops/accounts.ex @@ -93,21 +93,24 @@ defmodule Towerops.Accounts do {:error, %Ecto.Changeset{}} """ + @dialyzer {:nowarn_function, register_user_with_organization: 1} def register_user_with_organization(attrs) do user_changeset = User.email_changeset(%User{}, attrs) - Ecto.Multi.new() - |> Ecto.Multi.insert(:user, user_changeset) - |> Ecto.Multi.run(:organization, fn _repo, %{user: user} -> - org_name = attrs["organization_name"] || "Personal" + multi = Ecto.Multi.new() + multi = Ecto.Multi.insert(multi, :user, user_changeset) - Towerops.Organizations.create_organization( - %{name: org_name}, - user.id - ) - end) - |> Repo.transaction() - |> case do + multi = + Ecto.Multi.run(multi, :organization, fn _repo, %{user: user} -> + org_name = attrs["organization_name"] || "Personal" + + Towerops.Organizations.create_organization( + %{name: org_name}, + user.id + ) + end) + + case Repo.transaction(multi) do {:ok, %{user: user}} -> {:ok, user} {:error, :user, changeset, _} -> {:error, changeset} {:error, :organization, changeset, _} -> {:error, changeset} diff --git a/lib/towerops/alerts.ex b/lib/towerops/alerts.ex index f43e514b..db9343b0 100644 --- a/lib/towerops/alerts.ex +++ b/lib/towerops/alerts.ex @@ -60,7 +60,7 @@ defmodule Towerops.Alerts do where: s.organization_id == ^organization_id, order_by: [desc: a.triggered_at], limit: ^limit, - preload: [equipment: {e, site: s}, acknowledged_by: :acknowledged_by] + preload: [equipment: {e, site: s}, acknowledged_by: []] ) ) end diff --git a/lib/towerops/monitoring/equipment_monitor.ex b/lib/towerops/monitoring/equipment_monitor.ex index 75986bbc..b5f019fa 100644 --- a/lib/towerops/monitoring/equipment_monitor.ex +++ b/lib/towerops/monitoring/equipment_monitor.ex @@ -52,13 +52,13 @@ defmodule Towerops.Monitoring.EquipmentMonitor do @impl true def handle_info(:check_equipment, state) do - perform_check(state.equipment_id) + _ = perform_check(state.equipment_id) {:noreply, state} end @impl true def handle_cast(:check_now, state) do - perform_check(state.equipment_id) + _ = perform_check(state.equipment_id) {:noreply, state} end @@ -88,19 +88,21 @@ defmodule Towerops.Monitoring.EquipmentMonitor do new_status = if status == :success, do: :up, else: :down old_status = equipment.status - Equipment.update_equipment_status(equipment, new_status) + _ = Equipment.update_equipment_status(equipment, new_status) # Create alerts if status changed - if old_status != new_status do - handle_status_change(equipment_id, old_status, new_status) - end + _ = + if old_status != new_status do + handle_status_change(equipment_id, old_status, new_status) + end # Broadcast status change via PubSub - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "equipment:#{equipment_id}", - {:equipment_status_changed, equipment_id, new_status, response_time} - ) + _ = + Phoenix.PubSub.broadcast( + Towerops.PubSub, + "equipment:#{equipment_id}", + {:equipment_status_changed, equipment_id, new_status, response_time} + ) # Only schedule next check if monitoring is enabled if equipment.monitoring_enabled do @@ -124,18 +126,20 @@ defmodule Towerops.Monitoring.EquipmentMonitor do }) # Send email notification in background (not in test environment) - if !test_env?() do - Task.start(fn -> - Alerts.send_alert_notification(alert) - end) - end + _ = + if !test_env?() do + Task.start(fn -> + Alerts.send_alert_notification(alert) + end) + end # Broadcast alert - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "alerts:new", - {:new_alert, equipment_id, :equipment_down} - ) + _ = + Phoenix.PubSub.broadcast( + Towerops.PubSub, + "alerts:new", + {:new_alert, equipment_id, :equipment_down} + ) end {_, :up} -> @@ -149,31 +153,30 @@ defmodule Towerops.Monitoring.EquipmentMonitor do }) # Send email notification in background (not in test environment) - if !test_env?() do - Task.start(fn -> - Alerts.send_alert_notification(alert) - end) - end + _ = + if !test_env?() do + Task.start(fn -> + Alerts.send_alert_notification(alert) + end) + end # Resolve any active equipment_down alerts - case Alerts.get_active_alert(equipment_id, :equipment_down) do - nil -> - :ok + _ = + case Alerts.get_active_alert(equipment_id, :equipment_down) do + nil -> + :ok - alert -> - Alerts.resolve_alert(alert) - end + alert -> + Alerts.resolve_alert(alert) + end # Broadcast recovery - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "alerts:resolved", - {:alert_resolved, equipment_id, :equipment_down} - ) - - {_, :unknown} -> - # Status is unknown - no action needed - :ok + _ = + Phoenix.PubSub.broadcast( + Towerops.PubSub, + "alerts:resolved", + {:alert_resolved, equipment_id, :equipment_down} + ) end end diff --git a/lib/towerops/monitoring/supervisor.ex b/lib/towerops/monitoring/supervisor.ex index a59fdd57..cd505baf 100644 --- a/lib/towerops/monitoring/supervisor.ex +++ b/lib/towerops/monitoring/supervisor.ex @@ -21,13 +21,14 @@ defmodule Towerops.Monitoring.Supervisor do # Start all monitors after supervisor is initialized (but not in test mode) # In test mode, the Repo uses Sandbox pool which we can detect - if !test_mode?() do - Task.start(fn -> - # Wait briefly for the supervisor tree to be fully initialized - Process.sleep(100) - start_all_monitors() - end) - end + _ = + if !test_mode?() do + Task.start(fn -> + # Wait briefly for the supervisor tree to be fully initialized + Process.sleep(100) + start_all_monitors() + end) + end Supervisor.init(children, strategy: :one_for_one) end diff --git a/lib/towerops/organizations.ex b/lib/towerops/organizations.ex index 21edbadb..09d58b47 100644 --- a/lib/towerops/organizations.ex +++ b/lib/towerops/organizations.ex @@ -43,18 +43,21 @@ defmodule Towerops.Organizations do @doc """ Creates an organization and adds the creator as owner. """ + @dialyzer {:nowarn_function, create_organization: 2} def create_organization(attrs, user_id) do - Ecto.Multi.new() - |> Ecto.Multi.insert(:organization, Organization.changeset(%Organization{}, attrs)) - |> Ecto.Multi.insert(:membership, fn %{organization: organization} -> - Membership.changeset(%Membership{}, %{ - organization_id: organization.id, - user_id: user_id, - role: :owner - }) - end) - |> Repo.transaction() - |> case do + multi = Ecto.Multi.new() + multi = Ecto.Multi.insert(multi, :organization, Organization.changeset(%Organization{}, attrs)) + + multi = + Ecto.Multi.insert(multi, :membership, fn %{organization: organization} -> + Membership.changeset(%Membership{}, %{ + organization_id: organization.id, + user_id: user_id, + role: :owner + }) + end) + + case Repo.transaction(multi) do {:ok, %{organization: organization}} -> {:ok, organization} {:error, :organization, changeset, _} -> {:error, changeset} {:error, :membership, changeset, _} -> {:error, changeset} @@ -201,23 +204,28 @@ defmodule Towerops.Organizations do @doc """ Accepts an invitation and creates a membership. """ + @dialyzer {:nowarn_function, accept_invitation: 2} def accept_invitation(invitation, user_id) do - Ecto.Multi.new() - |> Ecto.Multi.update(:invitation, fn _ -> - Ecto.Changeset.change(invitation, %{ - accepted_at: DateTime.truncate(DateTime.utc_now(), :second), - accepted_by_id: user_id - }) - end) - |> Ecto.Multi.insert(:membership, fn _ -> - Membership.changeset(%Membership{}, %{ - organization_id: invitation.organization_id, - user_id: user_id, - role: invitation.role - }) - end) - |> Repo.transaction() - |> case do + multi = Ecto.Multi.new() + + multi = + Ecto.Multi.update(multi, :invitation, fn _ -> + Ecto.Changeset.change(invitation, %{ + accepted_at: DateTime.truncate(DateTime.utc_now(), :second), + accepted_by_id: user_id + }) + end) + + multi = + Ecto.Multi.insert(multi, :membership, fn _ -> + Membership.changeset(%Membership{}, %{ + organization_id: invitation.organization_id, + user_id: user_id, + role: invitation.role + }) + end) + + case Repo.transaction(multi) do {:ok, %{membership: membership}} -> {:ok, membership} {:error, _failed_operation, changeset, _changes_so_far} -> {:error, changeset} end diff --git a/lib/towerops/organizations/invitation.ex b/lib/towerops/organizations/invitation.ex index 45413b29..03b6ca06 100644 --- a/lib/towerops/organizations/invitation.ex +++ b/lib/towerops/organizations/invitation.ex @@ -25,7 +25,15 @@ defmodule Towerops.Organizations.Invitation do @doc false def changeset(invitation, attrs) do invitation - |> cast(attrs, [:email, :role, :organization_id, :invited_by_id]) + |> cast(attrs, [ + :email, + :role, + :organization_id, + :invited_by_id, + :token, + :expires_at, + :accepted_at + ]) |> validate_required([:email, :role, :organization_id, :invited_by_id]) |> validate_format(:email, ~r/^[^\s]+@[^\s]+\.[^\s]+$/) |> generate_token() @@ -37,12 +45,22 @@ defmodule Towerops.Organizations.Invitation do end defp generate_token(changeset) do - token = 32 |> :crypto.strong_rand_bytes() |> Base.url_encode64(padding: false) - put_change(changeset, :token, token) + # Only generate token if not already provided + if get_field(changeset, :token) do + changeset + else + token = 32 |> :crypto.strong_rand_bytes() |> Base.url_encode64(padding: false) + put_change(changeset, :token, token) + end end defp set_expires_at(changeset) do - expires_at = DateTime.utc_now() |> DateTime.add(7, :day) |> DateTime.truncate(:second) - put_change(changeset, :expires_at, expires_at) + # Only set expires_at if not already provided + if get_field(changeset, :expires_at) do + changeset + else + expires_at = DateTime.utc_now() |> DateTime.add(7, :day) |> DateTime.truncate(:second) + put_change(changeset, :expires_at, expires_at) + end end end diff --git a/lib/towerops/release.ex b/lib/towerops/release.ex index abc5d6a7..8b4b20ad 100644 --- a/lib/towerops/release.ex +++ b/lib/towerops/release.ex @@ -6,7 +6,7 @@ defmodule Towerops.Release do @app :towerops def migrate do - load_app() + _ = load_app() for repo <- repos() do {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :up, all: true)) @@ -14,7 +14,7 @@ defmodule Towerops.Release do end def rollback(repo, version) do - load_app() + _ = load_app() {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :down, to: version)) end @@ -24,7 +24,7 @@ defmodule Towerops.Release do defp load_app do # Many platforms require SSL when connecting to the database - Application.ensure_all_started(:ssl) + _ = Application.ensure_all_started(:ssl) Application.ensure_loaded(@app) end end diff --git a/lib/towerops_web/controllers/user_session_controller.ex b/lib/towerops_web/controllers/user_session_controller.ex index e3901164..7a8967ce 100644 --- a/lib/towerops_web/controllers/user_session_controller.ex +++ b/lib/towerops_web/controllers/user_session_controller.ex @@ -50,12 +50,13 @@ defmodule ToweropsWeb.UserSessionController do # magic link request def create(conn, %{"user" => %{"email" => email}}) do - if user = Accounts.get_user_by_email(email) do - Accounts.deliver_login_instructions( - user, - &url(~p"/users/log-in/#{&1}") - ) - end + _ = + if user = Accounts.get_user_by_email(email) do + Accounts.deliver_login_instructions( + user, + &url(~p"/users/log-in/#{&1}") + ) + end info = "If your email is in our system, you will receive instructions for logging in shortly." diff --git a/lib/towerops_web/controllers/user_settings_controller.ex b/lib/towerops_web/controllers/user_settings_controller.ex index a9fab650..206fe706 100644 --- a/lib/towerops_web/controllers/user_settings_controller.ex +++ b/lib/towerops_web/controllers/user_settings_controller.ex @@ -19,11 +19,12 @@ defmodule ToweropsWeb.UserSettingsController do case Accounts.change_user_email(user, user_params) do %{valid?: true} = changeset -> - Accounts.deliver_user_update_email_instructions( - Ecto.Changeset.apply_action!(changeset, :insert), - user.email, - &url(~p"/users/settings/confirm-email/#{&1}") - ) + _ = + Accounts.deliver_user_update_email_instructions( + Ecto.Changeset.apply_action!(changeset, :insert), + user.email, + &url(~p"/users/settings/confirm-email/#{&1}") + ) conn |> put_flash( diff --git a/lib/towerops_web/live/alert_live/index.ex b/lib/towerops_web/live/alert_live/index.ex index b3478490..ef857421 100644 --- a/lib/towerops_web/live/alert_live/index.ex +++ b/lib/towerops_web/live/alert_live/index.ex @@ -9,10 +9,11 @@ defmodule ToweropsWeb.AlertLive.Index do organization = socket.assigns.current_organization # Subscribe to alert events - if connected?(socket) do - Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:new") - Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:resolved") - end + _ = + if connected?(socket) do + _ = Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:new") + Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:resolved") + end {:ok, socket @@ -34,7 +35,7 @@ defmodule ToweropsWeb.AlertLive.Index do @impl true def handle_event("acknowledge", %{"id" => id}, socket) do alert = Alerts.get_alert!(id) - user_id = socket.assigns.current_user.id + user_id = socket.assigns.current_scope.user.id case Alerts.acknowledge_alert(alert, user_id) do {:ok, _alert} -> diff --git a/lib/towerops_web/live/dashboard_live.ex b/lib/towerops_web/live/dashboard_live.ex index b39e7910..414c01b1 100644 --- a/lib/towerops_web/live/dashboard_live.ex +++ b/lib/towerops_web/live/dashboard_live.ex @@ -11,10 +11,11 @@ defmodule ToweropsWeb.DashboardLive do organization = socket.assigns.current_organization # Subscribe to real-time alert updates - if connected?(socket) do - Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:new") - Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:resolved") - end + _ = + if connected?(socket) do + _ = Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:new") + Phoenix.PubSub.subscribe(Towerops.PubSub, "alerts:resolved") + end {:ok, socket diff --git a/lib/towerops_web/live/equipment_live/show.ex b/lib/towerops_web/live/equipment_live/show.ex index 62e6b931..830de067 100644 --- a/lib/towerops_web/live/equipment_live/show.ex +++ b/lib/towerops_web/live/equipment_live/show.ex @@ -16,9 +16,10 @@ defmodule ToweropsWeb.EquipmentLive.Show do checks = Monitoring.list_equipment_checks(id, 10) # Subscribe to status updates for this equipment - if connected?(socket) do - Phoenix.PubSub.subscribe(Towerops.PubSub, "equipment:#{id}") - end + _ = + if connected?(socket) do + Phoenix.PubSub.subscribe(Towerops.PubSub, "equipment:#{id}") + end {:noreply, socket diff --git a/test/towerops/alerts/alert_notifier_test.exs b/test/towerops/alerts/alert_notifier_test.exs index dce3fc59..e2132a76 100644 --- a/test/towerops/alerts/alert_notifier_test.exs +++ b/test/towerops/alerts/alert_notifier_test.exs @@ -42,6 +42,11 @@ defmodule Towerops.Alerts.AlertNotifierTest do site_id: site.id }) + # Consume confirmation emails from user creation + assert_email_sent(subject: "Confirmation instructions") + assert_email_sent(subject: "Confirmation instructions") + assert_email_sent(subject: "Confirmation instructions") + %{owner: owner, admin: admin, member: member, organization: organization, equipment: equipment} end @@ -66,9 +71,8 @@ defmodule Towerops.Alerts.AlertNotifierTest do assert length(results) == 2 # Verify emails were sent to owner and admin - assert_email_sent(subject: "[#{organization.name}] Equipment Down: #{equipment.name}") - assert_email_sent(to: owner.email) - assert_email_sent(to: admin.email) + assert_email_sent(subject: "[#{organization.name}] Equipment Down: #{equipment.name}", to: owner.email) + assert_email_sent(subject: "[#{organization.name}] Equipment Down: #{equipment.name}", to: admin.email) end test "sends equipment_up alert to owners and admins", %{ @@ -91,9 +95,8 @@ defmodule Towerops.Alerts.AlertNotifierTest do assert length(results) == 2 # Verify emails were sent - assert_email_sent(subject: "[#{organization.name}] Equipment Recovered: #{equipment.name}") - assert_email_sent(to: owner.email) - assert_email_sent(to: admin.email) + assert_email_sent(subject: "[#{organization.name}] Equipment Recovered: #{equipment.name}", to: owner.email) + assert_email_sent(subject: "[#{organization.name}] Equipment Recovered: #{equipment.name}", to: admin.email) end test "equipment_down alert includes correct information", %{ @@ -112,7 +115,7 @@ defmodule Towerops.Alerts.AlertNotifierTest do {:ok, _results} = AlertNotifier.deliver_alert_notification(alert) assert_email_sent(fn email -> - email.to == [{nil, owner.email}] && + email.to == [{"", owner.email}] && email.subject =~ "Equipment Down" && email.text_body =~ organization.name && email.text_body =~ equipment.name && @@ -138,7 +141,7 @@ defmodule Towerops.Alerts.AlertNotifierTest do {:ok, _results} = AlertNotifier.deliver_alert_notification(alert) assert_email_sent(fn email -> - email.to == [{nil, owner.email}] && + email.to == [{"", owner.email}] && email.subject =~ "Equipment Recovered" && email.text_body =~ organization.name && email.text_body =~ equipment.name &&