From fba0a1a54e1f90ad3f762e61ca9f69fe0aed8e22 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Thu, 30 Apr 2026 14:32:00 -0500 Subject: [PATCH] refactor: route Sites and Monitoring queries through Query modules Wire up Towerops.Sites.SiteQuery and Towerops.Monitoring.CheckQuery, which were created in the recent context split but had no callers. Sites: 5 callsites now compose via SiteQuery.for_organization/roots/ order_by_display. Monitoring: 6 callsites compose via CheckQuery.for_organization/ for_device/of_type/with_enabled/order_by_name. --- lib/towerops/monitoring.ex | 73 +++++++++++++++----------------------- lib/towerops/sites.ex | 62 ++++++++++++++------------------ 2 files changed, 56 insertions(+), 79 deletions(-) diff --git a/lib/towerops/monitoring.ex b/lib/towerops/monitoring.ex index d6557f36..4e34d508 100644 --- a/lib/towerops/monitoring.ex +++ b/lib/towerops/monitoring.ex @@ -10,6 +10,7 @@ defmodule Towerops.Monitoring do alias Towerops.Agents.AgentAssignment alias Towerops.Devices.Device alias Towerops.Monitoring.Check + alias Towerops.Monitoring.CheckQuery alias Towerops.Monitoring.CheckResult alias Towerops.Monitoring.MonitoringCheck alias Towerops.Repo @@ -21,12 +22,9 @@ defmodule Towerops.Monitoring do Returns the list of checks for an organization. """ def list_checks(organization_id, opts \\ []) do - query = - from c in Check, - where: c.organization_id == ^organization_id, - order_by: [asc: c.name] - - query + organization_id + |> CheckQuery.for_organization() + |> CheckQuery.order_by_name() |> maybe_filter_by_device(opts[:device_id]) |> maybe_filter_by_check_type(opts[:check_type]) |> maybe_filter_by_enabled(opts[:enabled]) @@ -34,22 +32,13 @@ defmodule Towerops.Monitoring do end defp maybe_filter_by_device(query, nil), do: query - - defp maybe_filter_by_device(query, device_id) do - from c in query, where: c.device_id == ^device_id - end + defp maybe_filter_by_device(query, device_id), do: CheckQuery.for_device(query, device_id) defp maybe_filter_by_check_type(query, nil), do: query - - defp maybe_filter_by_check_type(query, check_type) do - from c in query, where: c.check_type == ^check_type - end + defp maybe_filter_by_check_type(query, check_type), do: CheckQuery.of_type(query, check_type) defp maybe_filter_by_enabled(query, nil), do: query - - defp maybe_filter_by_enabled(query, enabled) do - from c in query, where: c.enabled == ^enabled - end + defp maybe_filter_by_enabled(query, enabled), do: CheckQuery.with_enabled(query, enabled) @doc """ Returns the list of service checks for devices assigned to a specific agent. @@ -302,8 +291,8 @@ defmodule Towerops.Monitoring do """ def get_device_check_data(device_id, since) do check_ids = - Check - |> where(device_id: ^device_id) + device_id + |> CheckQuery.for_device() |> select([c], c.id) |> Repo.all() @@ -465,11 +454,10 @@ defmodule Towerops.Monitoring do def stop_device_checks(device_id) do # Get all check IDs for this device check_ids = - Repo.all( - from c in Check, - where: c.device_id == ^device_id, - select: c.id - ) + device_id + |> CheckQuery.for_device() + |> select([c], c.id) + |> Repo.all() # Cancel all scheduled jobs for these checks Enum.each(check_ids, fn check_id -> @@ -492,9 +480,10 @@ defmodule Towerops.Monitoring do Checks can be re-enabled when SNMP is re-enabled. """ def disable_device_checks(device_id) do - Repo.update_all(from(c in Check, where: c.device_id == ^device_id, where: c.enabled == true), - set: [enabled: false, updated_at: DateTime.utc_now()] - ) + device_id + |> CheckQuery.for_device() + |> CheckQuery.with_enabled(true) + |> Repo.update_all(set: [enabled: false, updated_at: DateTime.utc_now()]) # Also cancel any scheduled jobs stop_device_checks(device_id) @@ -574,14 +563,12 @@ defmodule Towerops.Monitoring do """ def ensure_default_ping_check(%Device{} = device) do existing = - Repo.one( - from(c in Check, - where: c.device_id == ^device.id, - where: c.check_type == "ping", - where: c.source_type == "auto_discovery", - limit: 1 - ) - ) + device.id + |> CheckQuery.for_device() + |> CheckQuery.of_type("ping") + |> where([c], c.source_type == "auto_discovery") + |> limit(1) + |> Repo.one() case existing do nil -> @@ -627,14 +614,12 @@ defmodule Towerops.Monitoring do source_id = Map.fetch!(attrs, :source_id) existing = - Repo.one( - from(c in Check, - where: c.device_id == ^device_id, - where: c.check_type == ^check_type, - where: c.source_id == ^source_id, - limit: 1 - ) - ) + device_id + |> CheckQuery.for_device() + |> CheckQuery.of_type(check_type) + |> where([c], c.source_id == ^source_id) + |> limit(1) + |> Repo.one() case existing do nil -> diff --git a/lib/towerops/sites.ex b/lib/towerops/sites.ex index 731adce3..98b51097 100644 --- a/lib/towerops/sites.ex +++ b/lib/towerops/sites.ex @@ -10,6 +10,7 @@ defmodule Towerops.Sites do alias Towerops.Devices.Device alias Towerops.Repo alias Towerops.Sites.Site + alias Towerops.Sites.SiteQuery @site_preloads [:parent_site, :child_sites, :device] @@ -27,13 +28,11 @@ defmodule Towerops.Sites do """ @spec list_organization_sites(String.t()) :: [Site.t()] def list_organization_sites(organization_id) do - Repo.all( - from(s in Site, - where: s.organization_id == ^organization_id, - order_by: [asc: s.display_order, asc: s.name], - preload: [:parent_site] - ) - ) + organization_id + |> SiteQuery.for_organization() + |> SiteQuery.order_by_display() + |> preload(:parent_site) + |> Repo.all() end @doc "Search sites by name for an organization. Returns up to 10 results." @@ -53,10 +52,9 @@ defmodule Towerops.Sites do """ @spec count_organization_sites(String.t()) :: integer() def count_organization_sites(organization_id) do - Repo.aggregate( - from(s in Site, where: s.organization_id == ^organization_id), - :count - ) + organization_id + |> SiteQuery.for_organization() + |> Repo.aggregate(:count) end @doc """ @@ -65,13 +63,11 @@ defmodule Towerops.Sites do """ @spec list_root_sites(String.t()) :: [Site.t()] def list_root_sites(organization_id) do - Repo.all( - from(s in Site, - where: s.organization_id == ^organization_id, - where: is_nil(s.parent_site_id), - order_by: [asc: s.display_order, asc: s.name] - ) - ) + organization_id + |> SiteQuery.for_organization() + |> SiteQuery.roots() + |> SiteQuery.order_by_display() + |> Repo.all() end @doc """ @@ -114,13 +110,11 @@ defmodule Towerops.Sites do """ @spec get_organization_site!(String.t(), String.t()) :: Site.t() def get_organization_site!(organization_id, site_id) do - Repo.one!( - from(s in Site, - where: s.id == ^site_id, - where: s.organization_id == ^organization_id, - preload: ^@site_preloads - ) - ) + organization_id + |> SiteQuery.for_organization() + |> where([s], s.id == ^site_id) + |> preload(^@site_preloads) + |> Repo.one!() end @doc """ @@ -274,12 +268,11 @@ defmodule Towerops.Sites do # Get all sites excluding the one being moved, in current order other_sites = - Repo.all( - from(s in Site, - where: s.organization_id == ^organization_id and s.id != ^site_id, - order_by: [asc: s.display_order, asc: s.name] - ) - ) + organization_id + |> SiteQuery.for_organization() + |> where([s], s.id != ^site_id) + |> SiteQuery.order_by_display() + |> Repo.all() # Insert at new position (1-based index, convert to 0-based for List.insert_at) new_order = List.insert_at(other_sites, new_position - 1, site) @@ -317,9 +310,8 @@ defmodule Towerops.Sites do Clears display_order field for all sites, allowing them to fall back to alphabetical sorting. """ def reset_site_order(organization_id) do - Repo.update_all( - from(s in Site, where: s.organization_id == ^organization_id), - set: [display_order: nil, updated_at: Towerops.Time.now()] - ) + organization_id + |> SiteQuery.for_organization() + |> Repo.update_all(set: [display_order: nil, updated_at: Towerops.Time.now()]) end end