refactor: apply FP patterns — with-chains + multi-clause over if/case

- accounts.ex: convert get_user_by_email_and_password if→with-chain
  with explicit User struct match and else→nil. Flatten delete_account
  nested if (password check + sole-owner check) into single with-chain
  with named else clauses, eliminating do_delete_account indirection.

- organizations.ex: flatten update_member_role nested if/case into
  with-chain — parse_role guard → membership fetch → owner check,
  each with explicit else branch. Prevents variable shadowing by using
  current_role for the membership pattern match.

- capacity.ex: extract calculate_throughput_from_stats inner if into
  multi-clause compute_throughput_from_pairs/1 — empty list clause
  returns zero_throughput(), non-empty clause handles the computation.
This commit is contained in:
Graham McIntire 2026-07-01 18:04:36 -05:00
parent e77a848f2f
commit bdb0dd8c38
3 changed files with 46 additions and 53 deletions

View file

@ -56,8 +56,12 @@ defmodule Towerops.Accounts do
""" """
@spec get_user_by_email_and_password(String.t(), String.t()) :: User.t() | nil @spec get_user_by_email_and_password(String.t(), String.t()) :: User.t() | nil
def get_user_by_email_and_password(email, password) when is_binary(email) and is_binary(password) do def get_user_by_email_and_password(email, password) when is_binary(email) and is_binary(password) do
user = Repo.get_by(User, email: email) with %User{} = user <- Repo.get_by(User, email: email),
if User.valid_password?(user, password), do: user true <- User.valid_password?(user, password) do
user
else
_ -> nil
end
end end
@doc """ @doc """
@ -476,21 +480,12 @@ defmodule Towerops.Accounts do
| {:error, :invalid_password} | {:error, :invalid_password}
| {:error, :sole_owner, [String.t()]} | {:error, :sole_owner, [String.t()]}
def delete_account(%User{} = user, password) when is_binary(password) do def delete_account(%User{} = user, password) when is_binary(password) do
if User.valid_password?(user, password) do with true <- User.valid_password?(user, password),
do_delete_account(user) [] <- sole_owner_organizations(user.id) do
else
{:error, :invalid_password}
end
end
defp do_delete_account(user) do
sole_owner_orgs = sole_owner_organizations(user.id)
if sole_owner_orgs == [] do
delete_account_transaction(user) delete_account_transaction(user)
else else
org_names = Enum.map(sole_owner_orgs, & &1.name) false -> {:error, :invalid_password}
{:error, :sole_owner, org_names} orgs when is_list(orgs) -> {:error, :sole_owner, Enum.map(orgs, & &1.name)}
end end
end end

View file

@ -211,9 +211,12 @@ defmodule Towerops.Capacity do
# Filter out zero-bps pairs (from clamped negative deltas / anomalies) # Filter out zero-bps pairs (from clamped negative deltas / anomalies)
valid_pairs = Enum.reject(pairs, fn {in_bps, out_bps} -> in_bps == 0.0 and out_bps == 0.0 end) valid_pairs = Enum.reject(pairs, fn {in_bps, out_bps} -> in_bps == 0.0 and out_bps == 0.0 end)
if Enum.empty?(valid_pairs) do compute_throughput_from_pairs(valid_pairs)
zero_throughput() end
else
defp compute_throughput_from_pairs([]), do: zero_throughput()
defp compute_throughput_from_pairs(valid_pairs) do
in_values = Enum.map(valid_pairs, &elem(&1, 0)) in_values = Enum.map(valid_pairs, &elem(&1, 0))
out_values = Enum.map(valid_pairs, &elem(&1, 1)) out_values = Enum.map(valid_pairs, &elem(&1, 1))
@ -233,7 +236,6 @@ defmodule Towerops.Capacity do
peak_bps: Float.round(max(peak_in, peak_out), 2) peak_bps: Float.round(max(peak_in, peak_out), 2)
} }
end end
end
# 400 Gbps — values above this are counter anomalies, not real traffic # 400 Gbps — values above this are counter anomalies, not real traffic
@max_reasonable_bps 400_000_000_000 @max_reasonable_bps 400_000_000_000

View file

@ -264,30 +264,26 @@ defmodule Towerops.Organizations do
Updates a member's role. Owners cannot have their role changed. Updates a member's role. Owners cannot have their role changed.
""" """
def update_member_role(organization_id, user_id, new_role) do def update_member_role(organization_id, user_id, new_role) do
role_atom = parse_role(new_role) with new_role when new_role != :invalid <- parse_role(new_role),
%Membership{} = membership <- get_membership(organization_id, user_id),
if role_atom == :invalid do %{role: current_role} when current_role != :owner <- membership do
changeset = membership
|> Membership.role_update_changeset(new_role)
|> Repo.update()
else
:invalid ->
{:error,
%Membership{} %Membership{}
|> Ecto.Changeset.change() |> Ecto.Changeset.change()
|> Ecto.Changeset.add_error(:role, "is invalid") |> Ecto.Changeset.add_error(:role, "is invalid")}
{:error, changeset}
else
case get_membership(organization_id, user_id) do
%Membership{role: :owner} -> %Membership{role: :owner} ->
{:error, :cannot_change_owner_role} {:error, :cannot_change_owner_role}
%Membership{} = membership ->
membership
|> Membership.role_update_changeset(role_atom)
|> Repo.update()
nil -> nil ->
{:error, :not_found} {:error, :not_found}
end end
end end
end
@doc """ @doc """
Lists users who should receive alert notifications for an organization. Lists users who should receive alert notifications for an organization.