diff --git a/REFACTOR_SUMMARY.md b/REFACTOR_SUMMARY.md new file mode 100644 index 00000000..1c29f73a --- /dev/null +++ b/REFACTOR_SUMMARY.md @@ -0,0 +1,101 @@ +# DRY Refactoring Summary - FINAL + +## Completed: 15 out of 20 findings (75%) + +### ✅ All High-Impact Findings Complete (5/5) + +1. **#1: Merged duplicate credential_source_atom functions** + - Consolidated two near-identical functions using pattern matching + - Eliminated 7 lines of duplication + +2. **#2: Extracted propagate_credential_change/3 helper** + - Refactored 6 near-identical functions to use common helper + - Eliminated ~100 lines of duplicated code + +3. **#6: Merged get_fallback_agent_token variants** + - Used pattern matching instead of cond + - One function delegates to the other + +4. **#10: Extracted do_resolve_alert and do_acknowledge_alert helpers** + - Shared changeset + update logic for alert operations + - Each public function focuses on side effects + +5. **#11: Introduced Towerops.Time.now() utility** + - Replaced 199 instances across 74 files + - Single source of truth for timestamps + +### ✅ Medium/Low Impact Complete (10/15) + +6. **#9: Simplified normalize_attrs_alert_type to delegate** +7. **#12: Removed sanitize_like/1 wrappers** (5 files) +8. **#17: Module attributes for repeated preloads** (@device_preloads, @site_preloads) +9. **#14: Pattern matching in AccessControl verify_* functions** +10. **#15: Simplified mikrotik_use_ssl default logic** +11. **#7: Extracted maybe_preselect_site/3 helper** +12. **#8: Extracted find_org_agent/2 helper** +13. **#16: Pattern matching in should_trigger_discovery?** +14. **#18: maybe_filter_* helpers in list_organization_devices** +15. **#5: Simplified determine_effective_agent_id** + +### ⏭️ Deferred for Future PR (2/5) + +**#3: Repeated form LiveView pattern** (4 files) +- Complex: Requires behavior module or macro abstraction +- Impact: Would eliminate handle_params/validate/save boilerplate +- Recommendation: Separate PR with comprehensive testing + +**#4: Repeated access control error handling** (6+ files) +- Complex: AccessControl needs socket-ready response helpers +- Impact: Would eliminate repeated flash + error handling blocks +- Recommendation: Separate focused PR + +### ⏭️ Skipped - Minimal Benefit (3/5) + +**#13: handle_params tab/filter pattern** +- Reason: Pattern already clear (`Map.get(params, key, default)`) + +**#19: Collapse severity_color + severity_badge_class** +- Reason: `severity_color` used independently for text colors + +**#20: Consolidate time-formatting helpers** +- Reason: Different semantics (seconds vs minutes), context-appropriate + +## Final Impact + +### Code Changes +- **Files changed**: 90+ +- **Commits**: 16 +- **Net code reduction**: ~300+ lines eliminated through deduplication +- **New utilities added**: Towerops.Time module + +### Quality Improvements +- ✅ All high-impact duplication eliminated +- ✅ More idiomatic Elixir (pattern matching over cond/if) +- ✅ Better composability (query filter helpers) +- ✅ Single source of truth (Time.now, preload lists, credential helpers) +- ✅ Reduced maintenance burden + +### Test Coverage +- All existing tests pass +- No behavioral changes - pure refactoring +- Compilation warnings unchanged (only pre-existing proto warnings) + +## Remaining Work for Future PRs + +1. **Form LiveView Pattern (#3)** - Medium Priority + - Create reusable behavior or macro for form LiveViews + - Apply across schedule_live, escalation_policy_live, maintenance_live, site_live + - Estimated effort: 2-3 hours with comprehensive testing + +2. **Access Control Helpers (#4)** - Low Priority + - Add flash/navigate helpers to AccessControl module + - Update 6+ LiveView files to use new helpers + - Estimated effort: 1-2 hours + +## Recommendations + +✅ **Merge this PR** - High-impact work complete, code quality significantly improved + +🚀 **Future optimization** - Tackle #3 and #4 when refactoring LiveView patterns more broadly + +📊 **Metrics** - 75% of findings addressed, 100% of high-impact duplication eliminated diff --git a/dry.md b/dry.md new file mode 100644 index 00000000..da91c30c --- /dev/null +++ b/dry.md @@ -0,0 +1,564 @@ +# Code Review: DRY & Idiomatic Elixir Findings + +This document captures concrete duplication and non-idiomatic patterns found across the codebase. Each finding includes file paths and line numbers where applicable. + +--- + +## 1. ✅ DONE - Duplicated `credential_source_atom` / `safe_source_to_atom` + +**Files:** `lib/towerops/devices.ex` lines 408–412 and 887–893 + +Two private functions in the same module do the exact same thing — convert a credential source string to an atom — with slightly different names and one extra `nil` clause: + +```elixir +# line 408 +defp safe_source_to_atom("device"), do: :device +defp safe_source_to_atom("site"), do: :site +defp safe_source_to_atom("organization"), do: :organization +defp safe_source_to_atom(nil), do: :organization +defp safe_source_to_atom(_unknown), do: :organization + +# line 887 +defp credential_source_atom(value) do + case value do + "device" -> :device + "site" -> :site + "organization" -> :organization + _ -> :organization + end +end +``` + +These should be merged into a single private function using pattern-matching clauses (the idiomatic form): + +```elixir +defp credential_source_atom("device"), do: :device +defp credential_source_atom("site"), do: :site +defp credential_source_atom(_), do: :organization +``` + +--- + +## 2. ✅ DONE - Six Near-Identical `propagate_*` Functions in `devices.ex` + +**File:** `lib/towerops/devices.ex` + +The following six public functions share an identical structure — build a query, fetch device IDs, compute `now`, call `Repo.update_all`, then broadcast per-device: + +- `propagate_site_mikrotik_change/2` (~line 459) +- `propagate_organization_mikrotik_change/2` (~line 492) +- `propagate_site_snmpv3_change/2` (~line 563) +- `propagate_organization_snmpv3_change/2` (~line 591) +- `propagate_site_community_change/2` (~line 903) +- `propagate_organization_community_change/2` (~line 930) + +The only differences are the query filter field (`site_id` vs `organization_id`), the credential source string (`"site"` vs `"organization"`), the fields being updated, and the broadcast event atom. + +Extract a private helper: + +```elixir +defp propagate_credential_change(device_query, attrs, broadcast_event) do + device_ids = Repo.all(from(d in device_query, select: d.id)) + now = DateTime.truncate(DateTime.utc_now(), :second) + updates = attrs |> Map.new() |> Map.put(:updated_at, now) |> Map.to_list() + + Repo.update_all(device_query, set: updates) + + Enum.each(device_ids, fn device_id -> + Phoenix.PubSub.broadcast( + Towerops.PubSub, + "device:#{device_id}:assignments", + {:assignments_changed, broadcast_event} + ) + end) + + :ok +end +``` + +The six public functions then become thin wrappers that build the query and delegate. + +--- + +## 3. ⏭️ DEFERRED - Repeated Form LiveView Pattern (handle_params + validate + save) + +**Reason**: Complex refactoring requiring behavior module or macro abstraction across 4 LiveView files. Would benefit from separate PR with comprehensive testing. + +**Files:** + +- `lib/towerops_web/live/schedule_live/form.ex` +- `lib/towerops_web/live/escalation_policy_live/form.ex` +- `lib/towerops_web/live/maintenance_live/form.ex` +- `lib/towerops_web/live/site_live/form.ex` + +All four follow the exact same skeleton: + +```elixir +def handle_params(params, _url, socket) do + case socket.assigns.live_action do + :new -> # assign empty struct + changeset + :edit -> # load resource + changeset + end +end + +def handle_event("validate", %{"resource" => params}, socket) do + changeset = socket.assigns.resource |> Schema.changeset(params) |> Map.put(:action, :validate) + {:noreply, assign(socket, :form, to_form(changeset))} +end + +def handle_event("save", %{"resource" => params}, socket) do + case socket.assigns.live_action do + :new -> save_resource(socket, :create, params) + :edit -> save_resource(socket, :update, params) + end +end + +defp save_resource(socket, :create, params) do + case Context.create_resource(params) do + {:ok, resource} -> {:noreply, socket |> put_flash(:info, "...") |> push_navigate(...)} + {:error, changeset} -> {:noreply, assign(socket, :form, to_form(changeset))} + end +end + +defp save_resource(socket, :update, params) do + case Context.update_resource(socket.assigns.resource, params) do + {:ok, resource} -> {:noreply, socket |> put_flash(:info, "...") |> push_navigate(...)} + {:error, changeset} -> {:noreply, assign(socket, :form, to_form(changeset))} + end +end +``` + +The `save_resource/3` `:create` and `:update` clauses are also structurally identical — only the context function and flash message differ. The `case socket.assigns.live_action` dispatch in `handle_event("save")` is also repeated verbatim in all four modules. + +--- + +## 4. ⏭️ DEFERRED - Repeated Access Control Error Handling in LiveViews + +**Reason**: Would require AccessControl helper to return socket-ready responses. Impacts 6+ LiveView files. Better as separate focused PR. + +**Files:** + +- `lib/towerops_web/live/alert_live/index.ex` (~lines 53–80) +- `lib/towerops_web/live/device_live/form.ex` (~lines 105–115) +- `lib/towerops_web/live/device_live/show.ex` (~lines 61–75, 817–825, 1018–1025) +- `lib/towerops_web/live/config_timeline_live.ex` (~line 24) +- `lib/towerops_web/live/dashboard_live.ex` (~line 66) +- `lib/towerops_web/live/device_live/index.ex` (~lines 146–166) + +Every call to `AccessControl.verify_*_access/2` is followed by the same two error clauses: + +```elixir +{:error, :not_found} -> + {:noreply, put_flash(socket, :error, t_equipment("... not found"))} + +{:error, :unauthorized} -> + {:noreply, put_flash(socket, :error, t_equipment("You don't have access to this ..."))} +``` + +`AccessControl` already centralises the lookup logic. A companion helper in the same module (or a small addition to `AccessControl`) could handle the flash + navigate response, reducing each call site to a single `with` or `case` without the repeated error branches. + +--- + +## 5. ✅ DONE - `determine_effective_agent_id` Duplicates `Agents.get_effective_agent_token/1` + +**File:** `lib/towerops_web/live/device_live/form.ex` (~lines 390–415) + +`determine_effective_agent_id/2` walks device → site → org to find an agent token. This is exactly what `Agents.get_effective_agent_token/1` and `Agents.get_effective_agent_token_with_source/1` already do in `lib/towerops/agents.ex`. The LiveView version operates on form params (maps) rather than structs, but the logic is the same. Similarly, `using_cloud_poller?/2` (~line 418) repeats the same three-level walk. + +These should be consolidated — either by passing a lightweight struct to the existing `Agents` functions, or by extracting a shared helper that accepts either form params or a struct. + +--- + +## 6. ✅ DONE - `get_fallback_agent_token/1` and `get_fallback_agent_token_with_source/1` Are Near-Identical + +**File:** `lib/towerops/agents.ex` + +Both private functions walk the same site → org → global chain. The only difference is that one returns just the token ID and the other returns `{token_id, source}`. This is a classic case for a single function that always returns `{id, source}`, with the caller discarding the source when not needed: + +```elixir +defp resolve_fallback_agent(%{site: %{agent_token_id: id}} = _device) when not is_nil(id), + do: {id, :site} + +defp resolve_fallback_agent(%{site: %{organization: %{default_agent_token_id: id}}} = _device) + when not is_nil(id), + do: {id, :organization} + +defp resolve_fallback_agent(%{organization: %{default_agent_token_id: id}} = _device) + when not is_nil(id), + do: {id, :organization} + +defp resolve_fallback_agent(_device) do + case Towerops.Settings.get_global_default_cloud_poller() do + nil -> {nil, :none} + id -> {id, :global} + end +end +``` + +`get_effective_agent_token/1` then becomes `elem(get_effective_agent_token_with_source(device), 0)`. + +--- + +## 7. ✅ DONE - `cond` Instead of Pattern-Matching Function Clauses + +**File:** `lib/towerops_web/live/device_live/form.ex` (~lines 72–82) + +```elixir +equipment_attrs = + cond do + socket.assigns.preselected_site_id -> + Map.put(equipment_attrs, :site_id, socket.assigns.preselected_site_id) + + length(socket.assigns.available_sites) == 1 -> + site = List.first(socket.assigns.available_sites) + Map.put(equipment_attrs, :site_id, site.id) + + true -> + equipment_attrs + end +``` + +This is better expressed as a private function with pattern-matching clauses: + +```elixir +defp maybe_preselect_site(attrs, preselected_id, _sites) when not is_nil(preselected_id), + do: Map.put(attrs, :site_id, preselected_id) + +defp maybe_preselect_site(attrs, nil, [site]), + do: Map.put(attrs, :site_id, site.id) + +defp maybe_preselect_site(attrs, nil, _sites), do: attrs +``` + +**File:** `lib/towerops/agents.ex` — `get_fallback_agent_token/1` and `get_fallback_agent_token_with_source/1` both use `cond` with `match?/2` guards. These should be pattern-matching function clauses (see finding #6 above). + +**File:** `lib/towerops/devices.ex` — `should_trigger_discovery?/4` (~line 1090) uses `cond` with boolean conditions that are better expressed as pattern-matching clauses or a series of guard-based function heads. + +--- + +## 8. ✅ DONE - `if` for Optional Value Lookup (Use `case` or Pattern Matching) + +**File:** `lib/towerops_web/live/site_live/form.ex` (~lines 30–36 and 52–58) + +```elixir +org_agent = + if socket.assigns.organization.default_agent_token_id do + Enum.find(socket.assigns.available_agents, fn a -> + a.id == socket.assigns.organization.default_agent_token_id + end) + end +``` + +This duplicates the same block in both `apply_action/3` clauses. Extract to a private function and use `case` or pattern matching: + +```elixir +defp find_org_agent(nil, _agents), do: nil +defp find_org_agent(agent_id, agents), do: Enum.find(agents, &(&1.id == agent_id)) +``` + +Then call `find_org_agent(organization.default_agent_token_id, available_agents)` in both clauses. + +--- + +## 9. ✅ DONE - `normalize_alert_type` Defined Twice in `alerts.ex` + +**File:** `lib/towerops/alerts.ex` + +```elixir +# Used for attrs map normalization +defp normalize_attrs_alert_type(%{alert_type: alert_type} = attrs) when is_atom(alert_type) do + %{attrs | alert_type: to_string(alert_type)} +end +defp normalize_attrs_alert_type(attrs), do: attrs + +# Used for query normalization +defp normalize_alert_type(alert_type) when is_atom(alert_type), do: to_string(alert_type) +defp normalize_alert_type(alert_type) when is_binary(alert_type), do: alert_type +``` + +The second function (`normalize_alert_type/1`) is a subset of the first. The atom-to-string conversion is the same in both. The attrs version can delegate: + +```elixir +defp normalize_attrs_alert_type(%{alert_type: type} = attrs) when is_atom(type), + do: %{attrs | alert_type: normalize_alert_type(type)} +defp normalize_attrs_alert_type(attrs), do: attrs +``` + +--- + +## 10. ✅ DONE - `resolve_alert` and `resolve_alert_silent` Share Identical Changeset Logic + +**File:** `lib/towerops/alerts.ex` + +Both functions build the same changeset (`%{resolved_at: DateTime.truncate(...)}`) and call `Repo.update`. The only difference is the post-update side effects. Extract the update: + +```elixir +defp do_resolve_alert(alert) do + alert + |> Alert.changeset(%{resolved_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Repo.update() +end +``` + +Then `resolve_alert/1` and `resolve_alert_silent/1` call `do_resolve_alert/1` and handle their respective side effects. + +The same applies to `acknowledge_alert/2` and `acknowledge_alert_silent/1`. + +--- + +## 11. ✅ DONE - `DateTime.truncate(DateTime.utc_now(), :second)` Repeated ~30+ Times + +**Files:** `lib/towerops/devices.ex`, `lib/towerops/alerts.ex`, `lib/towerops/sites.ex`, `lib/towerops/organizations.ex`, `lib/towerops/agents.ex`, `lib/towerops_web/channels/agent_channel.ex`, and many more. + +This expression appears over 30 times across the codebase. A single module-level helper (or a function in `Towerops.Repo` or a shared utility module) would eliminate the repetition: + +```elixir +defp now(), do: DateTime.truncate(DateTime.utc_now(), :second) +``` + +Or, since it's needed across many modules, a public helper in e.g. `Towerops.Time`: + +```elixir +defmodule Towerops.Time do + def now(), do: DateTime.truncate(DateTime.utc_now(), :second) +end +``` + +--- + +## 12. ✅ DONE - `sanitize_like/1` Delegated Identically in Both `sites.ex` and `devices.ex` + +**Files:** `lib/towerops/sites.ex` (last line) and `lib/towerops/devices.ex` (last line) + +Both modules define: + +```elixir +defp sanitize_like(query), do: Towerops.QueryHelpers.sanitize_like(query) +``` + +This private wrapper adds no value — callers can just call `Towerops.QueryHelpers.sanitize_like/1` directly, or `import Towerops.QueryHelpers, only: [sanitize_like: 1]` at the top of each module. + +--- + +## 13. ⏭️ SKIPPED - `handle_params` Tab/Filter Pattern Repeated Across Index LiveViews + +**Reason**: Pattern is already clear and simple (`Map.get(params, "filter", default)`). Helper function would add minimal value. + +**Files:** + +- `lib/towerops_web/live/alert_live/index.ex` (~lines 38–44) +- `lib/towerops_web/live/maintenance_live/index.ex` (~lines 17–23) + +Both follow: + +```elixir +def handle_params(params, _url, socket) do + filter = Map.get(params, "filter", "default") + {:noreply, socket |> assign(:filter, filter) |> load_data(...)} +end +``` + +This is a minor but consistent pattern. A shared helper `assign_from_params(socket, params, key, default)` would make the intent clearer and reduce boilerplate. + +--- + +## 14. ✅ DONE - `verify_device_access` / `verify_site_access` in `AccessControl` Use `if` Instead of Pattern Matching + +**File:** `lib/towerops_web/live/helpers/access_control.ex` (~lines 34–43, 66–75) + +```elixir +def verify_device_access(device_id, organization_id) do + case Devices.get_device(device_id) do + nil -> {:error, :not_found} + device -> + if device.organization_id == organization_id do + {:ok, device} + else + {:error, :unauthorized} + end + end +end +``` + +The inner `if` should be a `case` or pattern match. More idiomatically, the `case` + `if` can collapse into a single `case` with a guard, or use `with`: + +```elixir +def verify_device_access(device_id, organization_id) do + with %Device{organization_id: ^organization_id} = device <- Devices.get_device(device_id) do + {:ok, device} + else + nil -> {:error, :not_found} + %Device{} -> {:error, :unauthorized} + end +end +``` + +The same applies to `verify_site_access/2`. `verify_alert_access/2` has a more complex preload but the same structural issue. + +--- + +## 15. ✅ DONE - `resolve_mikrotik_config` Uses `if` for a Nil-Default + +**File:** `lib/towerops/devices.ex` (~line 448) + +```elixir +use_ssl: if(device.mikrotik_use_ssl == nil, do: true, else: device.mikrotik_use_ssl), +``` + +This is better expressed as: + +```elixir +use_ssl: device.mikrotik_use_ssl != false, +``` + +or with a pattern-matching helper: + +```elixir +defp mikrotik_use_ssl?(nil), do: true +defp mikrotik_use_ssl?(val), do: val +``` + +--- + +## 16. ✅ DONE - `handle_snmp_changes` and `should_trigger_discovery?` Use `cond` for Boolean Logic + +**File:** `lib/towerops/devices.ex` (~lines 1080–1110) + +`should_trigger_discovery?/4` uses a `cond` with four boolean branches, all of which check `device.snmp_enabled` first. This is better expressed as pattern-matching function clauses: + +```elixir +defp should_trigger_discovery?(%{snmp_enabled: true}, false, _old_version, _old_port), do: true +defp should_trigger_discovery?(%{snmp_enabled: true, snmp_version: v}, _old_snmp, v, _), do: false +defp should_trigger_discovery?(%{snmp_enabled: true, snmp_version: _}, _, _, _), do: true +defp should_trigger_discovery?(%{snmp_enabled: true, snmp_port: p}, _, _, p), do: false +defp should_trigger_discovery?(%{snmp_enabled: true}, _, _, _), do: true +defp should_trigger_discovery?(_, _, _, _), do: false +``` + +Similarly, `handle_monitoring_changes/2` and `handle_snmp_changes/4` use `cond` where function clauses with pattern matching on the boolean pairs would be cleaner. + +--- + +## 17. ✅ DONE - `get_device` and `get_device!` Repeat the Same Preload + +**File:** `lib/towerops/devices.ex` (~lines 330–345) + +```elixir +def get_device(id) do + DeviceSchema + |> Repo.get(id) + |> case do + nil -> nil + device -> Repo.preload(device, [:organization, site: :organization]) + end +end + +def get_device!(id) do + DeviceSchema + |> Repo.get!(id) + |> Repo.preload([:organization, site: :organization]) +end +``` + +The preload list `[:organization, site: :organization]` is duplicated. Extract it as a module attribute or a private function: + +```elixir +@device_preloads [:organization, site: :organization] + +def get_device(id) do + case Repo.get(DeviceSchema, id) do + nil -> nil + device -> Repo.preload(device, @device_preloads) + end +end + +def get_device!(id), do: DeviceSchema |> Repo.get!(id) |> Repo.preload(@device_preloads) +``` + +The same pattern applies to `get_site/1` and `get_site!/1` in `lib/towerops/sites.ex` (both preload `[:parent_site, :child_sites, :device]`). + +--- + +## 18. ✅ DONE - `list_organization_devices` Uses `if` for Query Composition + +**File:** `lib/towerops/devices.ex` (~lines 44–65) + +```elixir +query = + if site_id = filters["site_id"] do + where(query, [e], e.site_id == ^site_id) + else + query + end + +query = + if status = filters["status"] do + where(query, [e], e.status == ^status) + else + query + end +``` + +This is the idiomatic Ecto filter-building pattern, but the `if/else query` form is verbose. The more idiomatic approach uses `Enum.reduce` over the filters or a pipeline of `maybe_filter_*` helpers: + +```elixir +defp maybe_filter_site(query, nil), do: query +defp maybe_filter_site(query, site_id), do: where(query, [e], e.site_id == ^site_id) + +defp maybe_filter_status(query, nil), do: query +defp maybe_filter_status(query, status), do: where(query, [e], e.status == ^status) +``` + +--- + +## 19. ⏭️ SKIPPED - `alert_live/index.ex` — `severity_color/1` and `severity_badge_class/1` Are Two Functions That Should Be One + +**Reason**: `severity_color/1` is used independently in template for text colors. Cannot collapse without breaking functionality. + +**File:** `lib/towerops_web/live/alert_live/index.ex` (~lines 220–250) + +`severity_color/1` returns a string like `"red"`, and `severity_badge_class/1` immediately maps that string to a CSS class. The intermediate string representation is only used to feed `severity_badge_class/1`. Collapse them: + +```elixir +defp severity_badge_class(alert) do + cond do + alert.resolved_at -> "bg-gray-300 dark:bg-gray-600" + alert.alert_type == "device_down" and is_nil(alert.resolved_at) -> + age_minutes = DateTime.diff(DateTime.utc_now(), alert.triggered_at, :minute) + cond do + age_minutes > 60 -> "bg-red-500" + age_minutes > 15 -> "bg-orange-500" + true -> "bg-yellow-500" + end + true -> "bg-green-500" + end +end +``` + +Unless `severity_color/1` is used independently in the template (in which case the indirection is intentional), the two-step conversion is unnecessary. + +--- + +## 20. ⏭️ SKIPPED - `age_text/1`, `duration_text/1`, and `time_ago/1` Repeat Minute-Bucketing Logic + +**Reason**: Functions have different semantics (seconds vs minutes, "just now" vs exact seconds) and serve different contexts. Duplication is acceptable for clarity. + +**Files:** `lib/towerops_web/live/alert_live/index.ex` and `lib/towerops_web/live/device_live/helpers/formatters.ex` + +`age_text/1` and `duration_text/1` in `alert_live/index.ex` both compute `minutes` from a `DateTime.diff` and then bucket into `<1m / Xm / Xh / Xd`. The `Formatters` module already has `time_ago/1`. These should be consolidated in `Formatters` and called from the template. + +--- + +## Summary of Priorities + +**High impact (most duplication removed):** + +1. Merge `safe_source_to_atom` / `credential_source_atom` into one function (finding #1) +2. Extract `propagate_credential_change/3` helper (finding #2) +3. Merge `get_fallback_agent_token` variants (finding #6) +4. Extract `do_resolve_alert` / `do_acknowledge_alert` (finding #10) +5. Introduce `Towerops.Time.now/0` or a module-level `now/0` (finding #11) + +**Medium impact (idiomatic improvements):** 6. Replace `if` with `with` in `AccessControl.verify_*_access` (finding #14) 7. Replace `cond` with pattern-matching clauses in `device_live/form.ex`, `agents.ex`, `devices.ex` (findings #7, #16) 8. Extract `maybe_preselect_site/3` and `find_org_agent/2` (findings #7, #8) 9. Use `@device_preloads` module attribute for repeated preload lists (finding #17) 10. Use `maybe_filter_*` helpers in `list_organization_devices` (finding #18) + +**Low impact (minor cleanup):** 11. Remove `sanitize_like/1` private wrappers (finding #12) 12. Collapse `severity_color` + `severity_badge_class` (finding #19) 13. Consolidate time-formatting helpers into `Formatters` (finding #20) 14. Simplify `normalize_attrs_alert_type` to delegate to `normalize_alert_type` (finding #9) diff --git a/lib/mix/tasks/geoip.import.ex b/lib/mix/tasks/geoip.import.ex index cb99ab6f..ff53aca6 100644 --- a/lib/mix/tasks/geoip.import.ex +++ b/lib/mix/tasks/geoip.import.ex @@ -257,7 +257,7 @@ defmodule Mix.Tasks.Geoip.Import do |> Stream.reject(&is_nil/1) |> Stream.chunk_every(@batch_size) |> Enum.reduce({0, MapSet.new()}, fn batch, {count, geoname_ids} -> - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = Enum.map(batch, fn location -> @@ -322,7 +322,7 @@ defmodule Mix.Tasks.Geoip.Import do |> Stream.filter(fn block -> MapSet.member?(valid_geoname_ids, block.geoname_id) end) |> Stream.chunk_every(@batch_size) |> Enum.reduce(0, fn batch, count -> - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = Enum.map(batch, fn block -> diff --git a/lib/towerops/accounts.ex b/lib/towerops/accounts.ex index 649b59a5..3dd4f254 100644 --- a/lib/towerops/accounts.ex +++ b/lib/towerops/accounts.ex @@ -734,7 +734,7 @@ defmodule Towerops.Accounts do {:ok, %User{last_sudo_at: ~U[2026-02-01 12:00:00Z]}} """ def grant_sudo_mode(%User{} = user) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() with {:ok, updated_user} <- user diff --git a/lib/towerops/agents.ex b/lib/towerops/agents.ex index 36df2c53..e42bf70b 100644 --- a/lib/towerops/agents.ex +++ b/lib/towerops/agents.ex @@ -186,7 +186,7 @@ defmodule Towerops.Agents do @spec update_agent_token_heartbeat(Ecto.UUID.t(), String.t() | nil, map() | nil) :: {non_neg_integer(), nil | [term()]} def update_agent_token_heartbeat(agent_token_id, ip_address, metadata \\ nil) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Only update metadata if explicitly provided (not nil and not empty) updates = @@ -210,7 +210,7 @@ defmodule Towerops.Agents do defp dismiss_agent_offline_insights(agent_token_id) do alias Towerops.Preseem.Insight - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {count, _} = Insight @@ -851,23 +851,8 @@ defmodule Towerops.Agents do # Checks site, organization, and global default in sequence defp get_fallback_agent_token(device) do - cond do - # 2. Site-level agent - match?(%{agent_token_id: id} when not is_nil(id), device.site) -> - device.site.agent_token_id - - # 3. Organization default via site - match?(%{organization: %{default_agent_token_id: id}} when not is_nil(id), device.site) -> - device.site.organization.default_agent_token_id - - # 3b. Organization default directly (device has no site) - match?(%{default_agent_token_id: id} when not is_nil(id), device.organization) -> - device.organization.default_agent_token_id - - # 4. Global default - true -> - Towerops.Settings.get_global_default_cloud_poller() - end + {agent_token_id, _source} = get_fallback_agent_token_with_source(device) + agent_token_id end @doc """ @@ -902,30 +887,22 @@ defmodule Towerops.Agents do end end - # Checks site, organization, and global default in sequence with source tracking - defp get_fallback_agent_token_with_source(device) do - cond do - # 2. Site-level agent - match?(%{agent_token_id: id} when not is_nil(id), device.site) -> - {device.site.agent_token_id, :site} + # Site-level agent (highest priority fallback) + defp get_fallback_agent_token_with_source(%{site: %{agent_token_id: id}} = _device) when not is_nil(id), do: {id, :site} - # 3. Organization default via site - match?(%{organization: %{default_agent_token_id: id}} when not is_nil(id), device.site) -> - {device.site.organization.default_agent_token_id, :organization} + # Organization default via site + defp get_fallback_agent_token_with_source(%{site: %{organization: %{default_agent_token_id: id}}}) when not is_nil(id), + do: {id, :organization} - # 3b. Organization default directly (device has no site) - match?(%{default_agent_token_id: id} when not is_nil(id), device.organization) -> - {device.organization.default_agent_token_id, :organization} + # Organization default directly (device has no site or site has no org reference) + defp get_fallback_agent_token_with_source(%{organization: %{default_agent_token_id: id}}) when not is_nil(id), + do: {id, :organization} - # 4. Global default - true -> - case Towerops.Settings.get_global_default_cloud_poller() do - agent_token_id when not is_nil(agent_token_id) -> - {agent_token_id, :global} - - _ -> - {nil, :none} - end + # Global default or none + defp get_fallback_agent_token_with_source(_device) do + case Towerops.Settings.get_global_default_cloud_poller() do + id when not is_nil(id) -> {id, :global} + _ -> {nil, :none} end end diff --git a/lib/towerops/alerts.ex b/lib/towerops/alerts.ex index 4c57952c..1c6ea8d5 100644 --- a/lib/towerops/alerts.ex +++ b/lib/towerops/alerts.ex @@ -55,8 +55,8 @@ defmodule Towerops.Alerts do defp enrich_with_organization_id(attrs), do: attrs # Normalize alert_type in attrs map for create/update operations - defp normalize_attrs_alert_type(%{alert_type: alert_type} = attrs) when is_atom(alert_type) do - %{attrs | alert_type: to_string(alert_type)} + defp normalize_attrs_alert_type(%{alert_type: type} = attrs) when is_atom(type) do + %{attrs | alert_type: normalize_alert_type(type)} end defp normalize_attrs_alert_type(attrs), do: attrs @@ -281,15 +281,7 @@ defmodule Towerops.Alerts do Acknowledges an alert. """ def acknowledge_alert(alert, user_id) do - result = - alert - |> Alert.changeset(%{ - acknowledged_at: DateTime.truncate(DateTime.utc_now(), :second), - acknowledged_by_id: user_id - }) - |> Repo.update() - - case result do + case do_acknowledge_alert(alert, user_id) do {:ok, updated_alert} -> AlertNotificationWorker.enqueue_acknowledge(updated_alert.id) Subscriptions.publish_alert_event(updated_alert, "acknowledged") @@ -304,12 +296,7 @@ defmodule Towerops.Alerts do Resolves an alert. """ def resolve_alert(alert) do - result = - alert - |> Alert.changeset(%{resolved_at: DateTime.truncate(DateTime.utc_now(), :second)}) - |> Repo.update() - - case result do + case do_resolve_alert(alert) do {:ok, updated_alert} -> AlertNotificationWorker.enqueue_resolve(updated_alert.id) broadcast_alert_change(updated_alert) @@ -325,12 +312,7 @@ defmodule Towerops.Alerts do Resolves an alert without notifying PagerDuty (used when PagerDuty is the source). """ def resolve_alert_silent(alert) do - result = - alert - |> Alert.changeset(%{resolved_at: DateTime.truncate(DateTime.utc_now(), :second)}) - |> Repo.update() - - case result do + case do_resolve_alert(alert) do {:ok, updated_alert} -> broadcast_alert_change(updated_alert) {:ok, updated_alert} @@ -344,8 +326,23 @@ defmodule Towerops.Alerts do Acknowledges an alert without notifying PagerDuty (used when PagerDuty is the source). """ def acknowledge_alert_silent(alert) do + do_acknowledge_alert(alert, nil) + end + + # Performs the database update for resolving an alert + defp do_resolve_alert(alert) do alert - |> Alert.changeset(%{acknowledged_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Alert.changeset(%{resolved_at: Towerops.Time.now()}) + |> Repo.update() + end + + # Performs the database update for acknowledging an alert + defp do_acknowledge_alert(alert, user_id) do + attrs = %{acknowledged_at: Towerops.Time.now()} + attrs = if user_id, do: Map.put(attrs, :acknowledged_by_id, user_id), else: attrs + + alert + |> Alert.changeset(attrs) |> Repo.update() end @@ -406,7 +403,7 @@ defmodule Towerops.Alerts do Resolves all active alerts for a check. """ def resolve_check_alerts(check_id) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() Repo.update_all(from(a in Alert, where: a.check_id == ^check_id, where: is_nil(a.resolved_at)), set: [resolved_at: now] diff --git a/lib/towerops/alerts/notification_rate_limiter.ex b/lib/towerops/alerts/notification_rate_limiter.ex index 18e64e97..60bd8f26 100644 --- a/lib/towerops/alerts/notification_rate_limiter.ex +++ b/lib/towerops/alerts/notification_rate_limiter.ex @@ -89,7 +89,7 @@ defmodule Towerops.Alerts.NotificationRateLimiter do digest |> NotificationDigest.changeset(%{ digest_sent: true, - digest_sent_at: DateTime.truncate(DateTime.utc_now(), :second) + digest_sent_at: Towerops.Time.now() }) |> Repo.update() end diff --git a/lib/towerops/alerts/site_correlation.ex b/lib/towerops/alerts/site_correlation.ex index 073e2287..8dbeb522 100644 --- a/lib/towerops/alerts/site_correlation.ex +++ b/lib/towerops/alerts/site_correlation.ex @@ -200,7 +200,7 @@ defmodule Towerops.Alerts.SiteCorrelation do end defp resolve_outage(outage) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() outage |> SiteOutage.changeset(%{resolved_at: now}) diff --git a/lib/towerops/alerts/storm_detector.ex b/lib/towerops/alerts/storm_detector.ex index c9bc1443..257a2891 100644 --- a/lib/towerops/alerts/storm_detector.ex +++ b/lib/towerops/alerts/storm_detector.ex @@ -241,7 +241,7 @@ defmodule Towerops.Alerts.StormDetector do device_count = length(events) devices = Enum.map(events, & &1.device) device_names = devices |> Enum.map(& &1.name) |> Enum.sort() - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Check maintenance window for the site (single check instead of N) if Maintenance.site_in_maintenance?(site_id) do diff --git a/lib/towerops/api_tokens.ex b/lib/towerops/api_tokens.ex index 93589565..0fa99ec9 100644 --- a/lib/towerops/api_tokens.ex +++ b/lib/towerops/api_tokens.ex @@ -184,7 +184,7 @@ defmodule Towerops.ApiTokens do @spec update_last_used(ApiToken.t()) :: :ok defp update_last_used(token) do - timestamp = DateTime.truncate(DateTime.utc_now(), :second) + timestamp = Towerops.Time.now() # In test mode, update synchronously to avoid ownership issues # In production, use Task.start to avoid blocking the request diff --git a/lib/towerops/devices.ex b/lib/towerops/devices.ex index 1dc174bc..5edd1d7c 100644 --- a/lib/towerops/devices.ex +++ b/lib/towerops/devices.ex @@ -17,6 +17,8 @@ defmodule Towerops.Devices do alias Towerops.Workers.DeviceMonitorWorker alias Towerops.Workers.DiscoveryWorker + @device_preloads [:organization, site: :organization] + @doc """ Returns the list of devices for a site. Ordered by custom display_order (if set), then alphabetically by name. @@ -41,29 +43,15 @@ defmodule Towerops.Devices do """ @spec list_organization_devices(String.t(), map()) :: [DeviceSchema.t()] def list_organization_devices(organization_id, filters \\ %{}) do - query = - from(e in DeviceSchema, - left_join: s in assoc(e, :site), - where: e.organization_id == ^organization_id, - order_by: [asc: e.display_order, asc: e.name], - preload: [site: s] - ) - - query = - if site_id = filters["site_id"] do - where(query, [e], e.site_id == ^site_id) - else - query - end - - query = - if status = filters["status"] do - where(query, [e], e.status == ^status) - else - query - end - - Repo.all(query) + from(e in DeviceSchema, + left_join: s in assoc(e, :site), + where: e.organization_id == ^organization_id, + order_by: [asc: e.display_order, asc: e.name], + preload: [site: s] + ) + |> maybe_filter_site(filters["site_id"]) + |> maybe_filter_status(filters["status"]) + |> Repo.all() end @doc """ @@ -144,7 +132,7 @@ defmodule Towerops.Devices do """ @spec search_devices(String.t(), String.t()) :: [DeviceSchema.t()] def search_devices(organization_id, query) do - search_term = "%#{sanitize_like(query)}%" + search_term = "%#{Towerops.QueryHelpers.sanitize_like(query)}%" DeviceSchema |> where(organization_id: ^organization_id) @@ -290,11 +278,9 @@ defmodule Towerops.Devices do Gets a single device by ID, returns nil if not found. """ def get_device(id) do - DeviceSchema - |> Repo.get(id) - |> case do + case Repo.get(DeviceSchema, id) do nil -> nil - device -> Repo.preload(device, [:organization, site: :organization]) + device -> Repo.preload(device, @device_preloads) end end @@ -304,7 +290,7 @@ defmodule Towerops.Devices do def get_device!(id) do DeviceSchema |> Repo.get!(id) - |> Repo.preload([:organization, site: :organization]) + |> Repo.preload(@device_preloads) end @doc """ @@ -400,16 +386,16 @@ defmodule Towerops.Devices do community: device.snmp_community, port: device.snmp_port || 161, transport: device.snmp_transport || "udp", - source: safe_source_to_atom(device.snmp_community_source) + source: credential_source_atom(device.snmp_community_source) } end # Safely convert credential source to atom with whitelist to prevent atom exhaustion - defp safe_source_to_atom("device"), do: :device - defp safe_source_to_atom("site"), do: :site - defp safe_source_to_atom("organization"), do: :organization - defp safe_source_to_atom(nil), do: :organization - defp safe_source_to_atom(_unknown), do: :organization + defp credential_source_atom("device"), do: :device + defp credential_source_atom("site"), do: :site + defp credential_source_atom("organization"), do: :organization + defp credential_source_atom(nil), do: :organization + defp credential_source_atom(_), do: :organization @doc """ Gets MikroTik API configuration for a device with hierarchical fallback. @@ -444,7 +430,7 @@ defmodule Towerops.Devices do password: device.mikrotik_password, port: device.mikrotik_port || 8729, ssh_port: device.mikrotik_ssh_port || 22, - use_ssl: if(device.mikrotik_use_ssl == nil, do: true, else: device.mikrotik_use_ssl), + use_ssl: device.mikrotik_use_ssl != false, enabled: device.mikrotik_enabled || false, source: credential_source_atom(device.mikrotik_credential_source) } @@ -457,29 +443,13 @@ defmodule Towerops.Devices do Updates username, password, port, use_ssl, and enabled fields. """ def propagate_site_mikrotik_change(site_id, attrs) do - # Find all devices in this site that inherit credentials from site device_query = from(d in DeviceSchema, where: d.site_id == ^site_id, where: d.mikrotik_credential_source == "site" ) - device_ids = Repo.all(from(d in device_query, select: d.id)) - now = DateTime.truncate(DateTime.utc_now(), :second) - updates = attrs |> Map.new() |> Map.put(:updated_at, now) |> Map.to_list() - - Repo.update_all(device_query, set: updates) - - Enum.each(device_ids, fn device_id -> - # Broadcast assignment change to trigger agent job list refresh - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "device:#{device_id}:assignments", - {:assignments_changed, :mikrotik_updated} - ) - end) - - :ok + propagate_credential_change(device_query, attrs, :mikrotik_updated) end @doc """ @@ -490,30 +460,13 @@ defmodule Towerops.Devices do Updates username, password, port, use_ssl, and enabled fields. """ def propagate_organization_mikrotik_change(organization_id, attrs) do - # With denormalized credentials, update all devices that inherit from the organization - # (devices with mikrotik_credential_source = "organization") device_query = from(d in DeviceSchema, where: d.organization_id == ^organization_id, where: d.mikrotik_credential_source == "organization" ) - device_ids = Repo.all(from(d in device_query, select: d.id)) - now = DateTime.truncate(DateTime.utc_now(), :second) - updates = attrs |> Map.new() |> Map.put(:updated_at, now) |> Map.to_list() - - Repo.update_all(device_query, set: updates) - - Enum.each(device_ids, fn device_id -> - # Broadcast assignment change to trigger agent job list refresh - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "device:#{device_id}:assignments", - {:assignments_changed, :mikrotik_updated} - ) - end) - - :ok + propagate_credential_change(device_query, attrs, :mikrotik_updated) end @doc """ @@ -567,21 +520,7 @@ defmodule Towerops.Devices do where: d.snmpv3_credential_source == "site" ) - device_ids = Repo.all(from(d in device_query, select: d.id)) - now = DateTime.truncate(DateTime.utc_now(), :second) - updates = attrs |> Map.new() |> Map.put(:updated_at, now) |> Map.to_list() - - Repo.update_all(device_query, set: updates) - - Enum.each(device_ids, fn device_id -> - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "device:#{device_id}:assignments", - {:assignments_changed, :snmpv3_updated} - ) - end) - - :ok + propagate_credential_change(device_query, attrs, :snmpv3_updated) end @doc """ @@ -589,29 +528,13 @@ defmodule Towerops.Devices do in sites that have no site-level credentials. """ def propagate_organization_snmpv3_change(organization_id, attrs) do - # With denormalized credentials, update all devices that inherit from the organization - # (devices with snmpv3_credential_source = "organization") device_query = from(d in DeviceSchema, where: d.organization_id == ^organization_id, where: d.snmpv3_credential_source == "organization" ) - device_ids = Repo.all(from(d in device_query, select: d.id)) - now = DateTime.truncate(DateTime.utc_now(), :second) - updates = attrs |> Map.new() |> Map.put(:updated_at, now) |> Map.to_list() - - Repo.update_all(device_query, set: updates) - - Enum.each(device_ids, fn device_id -> - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "device:#{device_id}:assignments", - {:assignments_changed, :snmpv3_updated} - ) - end) - - :ok + propagate_credential_change(device_query, attrs, :snmpv3_updated) end @doc """ @@ -884,13 +807,29 @@ defmodule Towerops.Devices do defp present?(value) when is_binary(value), do: String.trim(value) != "" defp present?(_), do: false - defp credential_source_atom(value) do - case value do - "device" -> :device - "site" -> :site - "organization" -> :organization - _ -> :organization - end + # Generic credential propagation helper used by all propagate_* functions + defp propagate_credential_change(device_query, attrs, broadcast_event) do + device_ids = Repo.all(from(d in device_query, select: d.id)) + now = Towerops.Time.now() + + # Normalize attrs to keyword list and add updated_at + updates = + attrs + |> Map.new() + |> Map.put(:updated_at, now) + |> Map.to_list() + + Repo.update_all(device_query, set: updates) + + Enum.each(device_ids, fn device_id -> + Phoenix.PubSub.broadcast( + Towerops.PubSub, + "device:#{device_id}:assignments", + {:assignments_changed, broadcast_event} + ) + end) + + :ok end @doc """ @@ -901,25 +840,10 @@ defmodule Towerops.Devices do the resolved value cached and ready for agent communication. """ def propagate_site_community_change(site_id, new_community) do - # Find all devices in this site that inherit community from site device_query = from(d in DeviceSchema, where: d.site_id == ^site_id, where: d.snmp_community_source == "site") - device_ids = Repo.all(from(d in device_query, select: d.id)) - now = DateTime.truncate(DateTime.utc_now(), :second) - - Repo.update_all(device_query, set: [snmp_community: new_community, updated_at: now]) - - Enum.each(device_ids, fn device_id -> - # Broadcast assignment change to trigger agent job list refresh - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "device:#{device_id}:assignments", - {:assignments_changed, :community_updated} - ) - end) - - :ok + propagate_credential_change(device_query, %{snmp_community: new_community}, :community_updated) end @doc """ @@ -928,29 +852,13 @@ defmodule Towerops.Devices do and site.snmp_community is nil). """ def propagate_organization_community_change(organization_id, new_community) do - # With denormalized credentials, update all devices that inherit from the organization - # (devices with snmp_community_source = "organization") device_query = from(d in DeviceSchema, where: d.organization_id == ^organization_id, where: d.snmp_community_source == "organization" ) - device_ids = Repo.all(from(d in device_query, select: d.id)) - now = DateTime.truncate(DateTime.utc_now(), :second) - - Repo.update_all(device_query, set: [snmp_community: new_community, updated_at: now]) - - Enum.each(device_ids, fn device_id -> - # Broadcast assignment change to trigger agent job list refresh - Phoenix.PubSub.broadcast( - Towerops.PubSub, - "device:#{device_id}:assignments", - {:assignments_changed, :community_updated} - ) - end) - - :ok + propagate_credential_change(device_query, %{snmp_community: new_community}, :community_updated) end @doc """ @@ -1007,7 +915,7 @@ defmodule Towerops.Devices do Updates the status of device. """ def update_device_status(%DeviceSchema{} = device, new_status) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() status_changed = device.status != new_status changes = %{ @@ -1043,7 +951,7 @@ defmodule Towerops.Devices do Also auto-dismisses any active "device_poll_gap" insights when polling resumes. """ def update_snmp_poll_time(%DeviceSchema{} = device) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() result = device @@ -1064,7 +972,7 @@ defmodule Towerops.Devices do defp dismiss_poll_gap_insights(device_id) do alias Towerops.Preseem.Insight - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {count, _} = Insight @@ -1118,25 +1026,26 @@ defmodule Towerops.Devices do end end - defp should_trigger_discovery?(device, old_snmp, old_snmp_version, old_snmp_port) do - cond do - # SNMP was just enabled - device.snmp_enabled && !old_snmp -> - true + # SNMP was just enabled + defp should_trigger_discovery?(%{snmp_enabled: true}, false, _old_version, _old_port), do: true - # SNMP version changed while enabled - device.snmp_enabled && device.snmp_version != old_snmp_version -> - true + # SNMP version changed while enabled + defp should_trigger_discovery?(%{snmp_enabled: true, snmp_version: v}, _old_snmp, old_v, _old_port) when v != old_v, + do: true - # SNMP port changed while enabled - device.snmp_enabled && device.snmp_port != old_snmp_port -> - true + # SNMP port changed while enabled + defp should_trigger_discovery?(%{snmp_enabled: true, snmp_port: p}, _old_snmp, _old_version, old_p) when p != old_p, + do: true - # Otherwise no discovery needed - true -> - false - end - end + # Otherwise no discovery needed + defp should_trigger_discovery?(_device, _old_snmp, _old_version, _old_port), do: false + + # Query filter helpers + defp maybe_filter_site(query, nil), do: query + defp maybe_filter_site(query, site_id), do: where(query, [e], e.site_id == ^site_id) + + defp maybe_filter_status(query, nil), do: query + defp maybe_filter_status(query, status), do: where(query, [e], e.status == ^status) ## Events @@ -1196,7 +1105,7 @@ defmodule Towerops.Devices do |> Enum.unzip() device_ids = Enum.map(device_ids, &Ecto.UUID.dump!/1) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() query = from(d in DeviceSchema, @@ -1227,7 +1136,7 @@ defmodule Towerops.Devices do join: s in assoc(d, :site), where: s.organization_id == ^organization_id ), - set: [display_order: nil, updated_at: DateTime.truncate(DateTime.utc_now(), :second)] + set: [display_order: nil, updated_at: Towerops.Time.now()] ) end @@ -1238,7 +1147,4 @@ defmodule Towerops.Devices do {event, organization_id} ) end - - # Sanitize user input for LIKE queries to prevent wildcard injection - defp sanitize_like(query), do: Towerops.QueryHelpers.sanitize_like(query) end diff --git a/lib/towerops/gaiia.ex b/lib/towerops/gaiia.ex index 42736de4..2dcf1faa 100644 --- a/lib/towerops/gaiia.ex +++ b/lib/towerops/gaiia.ex @@ -117,7 +117,7 @@ defmodule Towerops.Gaiia do end def search_inventory_items(organization_id, query) do - search = "%#{sanitize_like(query)}%" + search = "%#{Towerops.QueryHelpers.sanitize_like(query)}%" InventoryItem |> where(organization_id: ^organization_id) @@ -128,7 +128,7 @@ defmodule Towerops.Gaiia do end def search_network_sites(organization_id, query) do - search = "%#{sanitize_like(query)}%" + search = "%#{Towerops.QueryHelpers.sanitize_like(query)}%" NetworkSite |> where(organization_id: ^organization_id) @@ -200,7 +200,7 @@ defmodule Towerops.Gaiia do def suggest_site_matches(_organization_id, %NetworkSite{name: ""}), do: [] def suggest_site_matches(organization_id, %NetworkSite{name: gaiia_name}) do - search_term = "%#{sanitize_like(gaiia_name)}%" + search_term = "%#{Towerops.QueryHelpers.sanitize_like(gaiia_name)}%" # Find sites where either name contains the other (bidirectional) Site @@ -589,7 +589,4 @@ defmodule Towerops.Gaiia do Map.put(entry, :last_seen, last_seen) end) end - - # Sanitize user input for LIKE queries to prevent wildcard injection - defp sanitize_like(query), do: Towerops.QueryHelpers.sanitize_like(query) end diff --git a/lib/towerops/gaiia/impact_analysis.ex b/lib/towerops/gaiia/impact_analysis.ex index 1d2d06f5..96fddf43 100644 --- a/lib/towerops/gaiia/impact_analysis.ex +++ b/lib/towerops/gaiia/impact_analysis.ex @@ -33,7 +33,7 @@ defmodule Towerops.Gaiia.ImpactAnalysis do site_level: site_level, total_subscribers: total_subscribers, total_mrr: total_mrr, - analyzed_at: DateTime.truncate(DateTime.utc_now(), :second) + analyzed_at: Towerops.Time.now() } end diff --git a/lib/towerops/gaiia/reconciliation.ex b/lib/towerops/gaiia/reconciliation.ex index eb21f0c2..6bf926a3 100644 --- a/lib/towerops/gaiia/reconciliation.ex +++ b/lib/towerops/gaiia/reconciliation.ex @@ -55,7 +55,7 @@ defmodule Towerops.Gaiia.Reconciliation do mismatch_count: length(data_mismatches), missing_mapping_count: length(missing_mappings) }, - reconciled_at: DateTime.truncate(DateTime.utc_now(), :second) + reconciled_at: Towerops.Time.now() } end diff --git a/lib/towerops/gaiia/subscriber_matching.ex b/lib/towerops/gaiia/subscriber_matching.ex index c40d4a28..b8eb13cc 100644 --- a/lib/towerops/gaiia/subscriber_matching.ex +++ b/lib/towerops/gaiia/subscriber_matching.ex @@ -62,7 +62,7 @@ defmodule Towerops.Gaiia.SubscriberMatching do end defp do_refresh(organization_id) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Load all inventory items with assigned accounts items = load_assigned_items(organization_id) @@ -130,7 +130,7 @@ defmodule Towerops.Gaiia.SubscriberMatching do end defp do_refresh_for_device(device) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() org_id = device.organization_id lookups = build_device_lookups(device) diff --git a/lib/towerops/integrations.ex b/lib/towerops/integrations.ex index 08595cf2..50f80b70 100644 --- a/lib/towerops/integrations.ex +++ b/lib/towerops/integrations.ex @@ -41,7 +41,7 @@ defmodule Towerops.Integrations do integration |> Integration.changeset(%{ last_sync_status: status, - last_synced_at: DateTime.truncate(DateTime.utc_now(), :second), + last_synced_at: Towerops.Time.now(), last_sync_message: message }) |> Repo.update() diff --git a/lib/towerops/mobile_sessions/mobile_session.ex b/lib/towerops/mobile_sessions/mobile_session.ex index 266ebe43..6570af0c 100644 --- a/lib/towerops/mobile_sessions/mobile_session.ex +++ b/lib/towerops/mobile_sessions/mobile_session.ex @@ -64,7 +64,7 @@ defmodule Towerops.MobileSessions.MobileSession do Updates the last_used_at timestamp for a session. """ def touch_changeset(mobile_session) do - last_used_at = DateTime.truncate(DateTime.utc_now(), :second) + last_used_at = Towerops.Time.now() change(mobile_session, last_used_at: last_used_at) end @@ -86,7 +86,7 @@ defmodule Towerops.MobileSessions.MobileSession do end defp put_timestamps(changeset) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() default_expiration = now |> DateTime.add(90, :day) |> DateTime.truncate(:second) changeset diff --git a/lib/towerops/mobile_sessions/qr_login_token.ex b/lib/towerops/mobile_sessions/qr_login_token.ex index 81fd0958..46a1c33a 100644 --- a/lib/towerops/mobile_sessions/qr_login_token.ex +++ b/lib/towerops/mobile_sessions/qr_login_token.ex @@ -42,7 +42,7 @@ defmodule Towerops.MobileSessions.QRLoginToken do Marks a QR login token as completed. """ def complete_changeset(qr_login_token, mobile_session_id) do - completed_at = DateTime.truncate(DateTime.utc_now(), :second) + completed_at = Towerops.Time.now() change(qr_login_token, completed_at: completed_at, mobile_session_id: mobile_session_id) end diff --git a/lib/towerops/on_call/escalation.ex b/lib/towerops/on_call/escalation.ex index 9e90ed3f..fccb9cef 100644 --- a/lib/towerops/on_call/escalation.ex +++ b/lib/towerops/on_call/escalation.ex @@ -35,7 +35,7 @@ defmodule Towerops.OnCall.Escalation do {:error, :no_policy} policy -> - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() attrs = %{ alert_id: alert.id, @@ -59,7 +59,7 @@ defmodule Towerops.OnCall.Escalation do Acknowledges an incident, stopping further escalation. """ def acknowledge_incident(incident_id, user_id) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() Incident |> Repo.get!(incident_id) @@ -75,7 +75,7 @@ defmodule Towerops.OnCall.Escalation do Resolves an incident. """ def resolve_incident(incident_id, user_id) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() Incident |> Repo.get!(incident_id) @@ -158,7 +158,7 @@ defmodule Towerops.OnCall.Escalation do end defp log_notification(incident, user) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() %Notification{} |> Notification.changeset(%{ diff --git a/lib/towerops/organizations.ex b/lib/towerops/organizations.ex index 24fc3666..27f8c41f 100644 --- a/lib/towerops/organizations.ex +++ b/lib/towerops/organizations.ex @@ -448,7 +448,7 @@ defmodule Towerops.Organizations do multi = Ecto.Multi.update(multi, :invitation, fn _ -> Ecto.Changeset.change(invitation, %{ - accepted_at: DateTime.truncate(DateTime.utc_now(), :second), + accepted_at: Towerops.Time.now(), accepted_by_id: user_id }) end) @@ -492,7 +492,7 @@ defmodule Towerops.Organizations do set: [ snmp_version: organization.snmp_version, snmp_community: organization.snmp_community, - updated_at: DateTime.truncate(DateTime.utc_now(), :second) + updated_at: Towerops.Time.now() ] ) end @@ -517,7 +517,7 @@ defmodule Towerops.Organizations do Repo.delete_all(from(a in AgentAssignment, where: a.device_id in ^device_ids)) # Create new assignments - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() assignments = Enum.map(device_ids, fn device_id -> @@ -551,7 +551,7 @@ defmodule Towerops.Organizations do from(d in Device, where: d.organization_id == ^organization_id), set: [ site_id: nil, - updated_at: DateTime.truncate(DateTime.utc_now(), :second) + updated_at: Towerops.Time.now() ] ) diff --git a/lib/towerops/preseem/baseline.ex b/lib/towerops/preseem/baseline.ex index ac47496f..f3f65891 100644 --- a/lib/towerops/preseem/baseline.ex +++ b/lib/towerops/preseem/baseline.ex @@ -15,7 +15,7 @@ defmodule Towerops.Preseem.Baseline do @doc "Compute baselines for all matched APs in an organization." def compute_baselines(organization_id) do cutoff = DateTime.add(DateTime.utc_now(), -@baseline_days * 86_400, :second) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() matched_aps = AccessPoint diff --git a/lib/towerops/preseem/fleet_intelligence.ex b/lib/towerops/preseem/fleet_intelligence.ex index 02b94d24..6d80b57c 100644 --- a/lib/towerops/preseem/fleet_intelligence.ex +++ b/lib/towerops/preseem/fleet_intelligence.ex @@ -13,7 +13,7 @@ defmodule Towerops.Preseem.FleetIntelligence do def compute_fleet_profiles(organization_id) do require Logger - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() matched_aps = AccessPoint diff --git a/lib/towerops/preseem/insights.ex b/lib/towerops/preseem/insights.ex index e7f933b8..5d17d416 100644 --- a/lib/towerops/preseem/insights.ex +++ b/lib/towerops/preseem/insights.ex @@ -82,7 +82,7 @@ defmodule Towerops.Preseem.Insights do insight |> Insight.changeset(%{ status: "dismissed", - dismissed_at: DateTime.truncate(DateTime.utc_now(), :second) + dismissed_at: Towerops.Time.now() }) |> Repo.update() end @@ -90,7 +90,7 @@ defmodule Towerops.Preseem.Insights do @doc "Bulk dismiss insights by IDs." def dismiss_insights(insight_ids) when is_list(insight_ids) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {count, _} = Insight diff --git a/lib/towerops/preseem/sync.ex b/lib/towerops/preseem/sync.ex index 04482306..8d62470c 100644 --- a/lib/towerops/preseem/sync.ex +++ b/lib/towerops/preseem/sync.ex @@ -79,7 +79,7 @@ defmodule Towerops.Preseem.Sync do recorded_at = case metrics_data["recorded_at"] do - nil -> DateTime.truncate(DateTime.utc_now(), :second) + nil -> Towerops.Time.now() dt_string -> parse_datetime(dt_string) end @@ -154,7 +154,7 @@ defmodule Towerops.Preseem.Sync do records_synced: records_synced, errors: errors, duration_ms: duration_ms, - inserted_at: DateTime.truncate(DateTime.utc_now(), :second) + inserted_at: Towerops.Time.now() }) |> Repo.insert() end @@ -162,7 +162,7 @@ defmodule Towerops.Preseem.Sync do defp parse_datetime(dt_string) when is_binary(dt_string) do case DateTime.from_iso8601(dt_string) do {:ok, dt, _offset} -> DateTime.truncate(dt, :second) - _ -> DateTime.truncate(DateTime.utc_now(), :second) + _ -> Towerops.Time.now() end end diff --git a/lib/towerops/reports.ex b/lib/towerops/reports.ex index 780a91f0..9b4129f7 100644 --- a/lib/towerops/reports.ex +++ b/lib/towerops/reports.ex @@ -180,7 +180,7 @@ defmodule Towerops.Reports do """ def mark_run(%Report{} = report, status) do update_report(report, %{ - last_run_at: DateTime.truncate(DateTime.utc_now(), :second), + last_run_at: Towerops.Time.now(), last_run_status: status }) end diff --git a/lib/towerops/sites.ex b/lib/towerops/sites.ex index d374b58f..b97caebc 100644 --- a/lib/towerops/sites.ex +++ b/lib/towerops/sites.ex @@ -11,6 +11,8 @@ defmodule Towerops.Sites do alias Towerops.Repo alias Towerops.Sites.Site + @site_preloads [:parent_site, :child_sites, :device] + @doc """ Lists all sites across all organizations that have geographic coordinates set. Used by the weather sync worker to fetch weather data. @@ -36,7 +38,7 @@ defmodule Towerops.Sites do @doc "Search sites by name for an organization. Returns up to 10 results." def search_sites(organization_id, query) do - search_term = "%#{sanitize_like(query)}%" + search_term = "%#{Towerops.QueryHelpers.sanitize_like(query)}%" Site |> where(organization_id: ^organization_id) @@ -93,7 +95,7 @@ defmodule Towerops.Sites do def get_site!(id) do Site |> Repo.get!(id) - |> Repo.preload([:parent_site, :child_sites, :device]) + |> Repo.preload(@site_preloads) end @doc """ @@ -103,7 +105,7 @@ defmodule Towerops.Sites do def get_site(id) do case Repo.get(Site, id) do nil -> nil - site -> Repo.preload(site, [:parent_site, :child_sites, :device]) + site -> Repo.preload(site, @site_preloads) end end @@ -116,7 +118,7 @@ defmodule Towerops.Sites do from(s in Site, where: s.id == ^site_id, where: s.organization_id == ^organization_id, - preload: [:parent_site, :child_sites, :device] + preload: ^@site_preloads ) ) end @@ -211,7 +213,7 @@ defmodule Towerops.Sites do set: [ snmp_version: site.snmp_version, snmp_community: site.snmp_community, - updated_at: DateTime.truncate(DateTime.utc_now(), :second) + updated_at: Towerops.Time.now() ] ) end @@ -235,7 +237,7 @@ defmodule Towerops.Sites do Repo.delete_all(from(a in AgentAssignment, where: a.device_id in ^device_ids)) # Create new assignments - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() assignments = Enum.map(device_ids, fn device_id -> @@ -289,7 +291,7 @@ defmodule Towerops.Sites do |> Enum.unzip() site_ids = Enum.map(site_ids, &Ecto.UUID.dump!/1) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() query = from(s in Site, @@ -317,10 +319,7 @@ defmodule Towerops.Sites do 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: DateTime.truncate(DateTime.utc_now(), :second)] + set: [display_order: nil, updated_at: Towerops.Time.now()] ) end - - # Sanitize user input for LIKE queries to prevent wildcard injection - defp sanitize_like(query), do: Towerops.QueryHelpers.sanitize_like(query) end diff --git a/lib/towerops/snmp.ex b/lib/towerops/snmp.ex index d1397c3d..6ca3a965 100644 --- a/lib/towerops/snmp.ex +++ b/lib/towerops/snmp.ex @@ -612,7 +612,7 @@ defmodule Towerops.Snmp do def create_sensor_readings_batch([]), do: {0, nil} def create_sensor_readings_batch(entries) when is_list(entries) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() rows = Enum.map(entries, fn entry -> @@ -643,7 +643,7 @@ defmodule Towerops.Snmp do def create_interface_stats_batch([]), do: {0, nil} def create_interface_stats_batch(entries) when is_list(entries) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() rows = Enum.map(entries, fn entry -> @@ -759,7 +759,7 @@ defmodule Towerops.Snmp do from(e in DeviceSchema, join: s in assoc(e, :site), where: s.organization_id == ^organization_id, - where: fragment("LOWER(?) LIKE ?", e.name, ^"%#{sanitize_like(normalized_name)}%"), + where: fragment("LOWER(?) LIKE ?", e.name, ^"%#{Towerops.QueryHelpers.sanitize_like(normalized_name)}%"), select: e, limit: 1 ) @@ -1674,7 +1674,7 @@ defmodule Towerops.Snmp do def create_processor_readings_batch([]), do: {0, nil} def create_processor_readings_batch(entries) when is_list(entries) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() rows = Enum.map(entries, fn entry -> @@ -1803,7 +1803,7 @@ defmodule Towerops.Snmp do def create_storage_readings_batch([]), do: {0, nil} def create_storage_readings_batch(entries) when is_list(entries) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() rows = Enum.map(entries, fn entry -> @@ -2048,7 +2048,7 @@ defmodule Towerops.Snmp do def create_transceiver_readings_batch([]), do: {0, nil} def create_transceiver_readings_batch(entries) when is_list(entries) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() rows = Enum.map(entries, fn entry -> @@ -2073,7 +2073,7 @@ defmodule Towerops.Snmp do def create_entity_physical_readings_batch([]), do: {0, nil} def create_entity_physical_readings_batch(entries) when is_list(entries) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() rows = Enum.map(entries, fn entry -> @@ -2184,7 +2184,7 @@ defmodule Towerops.Snmp do def create_mempool_readings_batch([]), do: {0, nil} def create_mempool_readings_batch(entries) when is_list(entries) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() rows = Enum.map(entries, fn entry -> @@ -2747,8 +2747,6 @@ defmodule Towerops.Snmp do "#{a}.#{b}.#{c}.#{d}" end - defp sanitize_like(str), do: Towerops.QueryHelpers.sanitize_like(str) - # Wireless Client queries @doc """ @@ -2841,7 +2839,7 @@ defmodule Towerops.Snmp do def create_wireless_client_readings_batch([]), do: {0, nil} def create_wireless_client_readings_batch(entries) when is_list(entries) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() rows = Enum.map(entries, fn entry -> diff --git a/lib/towerops/snmp/arp_discovery.ex b/lib/towerops/snmp/arp_discovery.ex index bb922baa..66fec0b2 100644 --- a/lib/towerops/snmp/arp_discovery.ex +++ b/lib/towerops/snmp/arp_discovery.ex @@ -43,7 +43,7 @@ defmodule Towerops.Snmp.ArpDiscovery do mac_address: format_mac_address(mac), if_index: if_index, entry_type: entry_type_to_string(entry_type), - last_seen_at: DateTime.truncate(DateTime.utc_now(), :second) + last_seen_at: Towerops.Time.now() } end) |> Enum.reject(&(is_nil(&1.ip_address) or is_nil(&1.mac_address))) diff --git a/lib/towerops/snmp/discovery.ex b/lib/towerops/snmp/discovery.ex index 3265adbe..a691d630 100644 --- a/lib/towerops/snmp/discovery.ex +++ b/lib/towerops/snmp/discovery.ex @@ -1643,7 +1643,7 @@ defmodule Towerops.Snmp.Discovery do {:ok, DeviceSchema.t()} | {:error, Ecto.Changeset.t()} defp update_device_discovery_time(device) do device - |> Ecto.Changeset.change(last_discovery_at: DateTime.truncate(DateTime.utc_now(), :second)) + |> Ecto.Changeset.change(last_discovery_at: Towerops.Time.now()) |> Repo.update() end diff --git a/lib/towerops/snmp/mac_discovery.ex b/lib/towerops/snmp/mac_discovery.ex index 606be6ed..5567a2ee 100644 --- a/lib/towerops/snmp/mac_discovery.ex +++ b/lib/towerops/snmp/mac_discovery.ex @@ -64,7 +64,7 @@ defmodule Towerops.Snmp.MacDiscovery do port_index: port_index, vlan_id: vlan_id, entry_status: fdb_status_to_string(status), - last_seen_at: DateTime.truncate(DateTime.utc_now(), :second) + last_seen_at: Towerops.Time.now() } end) |> Enum.reject(&is_nil(&1.mac_address)) @@ -94,7 +94,7 @@ defmodule Towerops.Snmp.MacDiscovery do port_index: port_index, vlan_id: nil, entry_status: fdb_status_to_string(status), - last_seen_at: DateTime.truncate(DateTime.utc_now(), :second) + last_seen_at: Towerops.Time.now() } end) |> Enum.reject(&(is_nil(&1.mac_address) or is_nil(&1.port_index))) diff --git a/lib/towerops/snmp/neighbor_discovery.ex b/lib/towerops/snmp/neighbor_discovery.ex index 368f14c9..2cc4fc30 100644 --- a/lib/towerops/snmp/neighbor_discovery.ex +++ b/lib/towerops/snmp/neighbor_discovery.ex @@ -222,7 +222,7 @@ defmodule Towerops.Snmp.NeighborDiscovery do remote_port_description: Sanitizer.sanitize_string(neighbor_map[:remote_port_description]), remote_address: Sanitizer.sanitize_string(neighbor_map[:remote_address]), remote_capabilities: neighbor_map[:remote_capabilities] || [], - last_discovered_at: DateTime.truncate(DateTime.utc_now(), :second) + last_discovered_at: Towerops.Time.now() } end end diff --git a/lib/towerops/snmp/wireless_client_discovery.ex b/lib/towerops/snmp/wireless_client_discovery.ex index 246864f7..ff292ef2 100644 --- a/lib/towerops/snmp/wireless_client_discovery.ex +++ b/lib/towerops/snmp/wireless_client_discovery.ex @@ -75,7 +75,7 @@ defmodule Towerops.Snmp.WirelessClientDiscovery do # --- Cambium ePMP --- defp discover_epmp_clients(client_opts) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() with {:ok, macs} <- Client.walk(client_opts, "#{@epmp_base}.1"), {:ok, ips} <- walk_or_empty(client_opts, "#{@epmp_base}.10"), @@ -129,7 +129,7 @@ defmodule Towerops.Snmp.WirelessClientDiscovery do # --- Ubiquiti airOS --- defp discover_ubnt_clients(client_opts) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() with {:ok, macs} <- Client.walk(client_opts, "#{@ubnt_base}.1"), {:ok, ips} <- walk_or_empty(client_opts, "#{@ubnt_base}.10"), @@ -172,7 +172,7 @@ defmodule Towerops.Snmp.WirelessClientDiscovery do # --- MikroTik RouterOS --- defp discover_mikrotik_clients(client_opts) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() with {:ok, macs} <- Client.walk(client_opts, "#{@mikrotik_base}.1"), {:ok, signals} <- walk_or_empty(client_opts, "#{@mikrotik_base}.3"), diff --git a/lib/towerops/status_pages.ex b/lib/towerops/status_pages.ex index 6021e652..cdd23fbb 100644 --- a/lib/towerops/status_pages.ex +++ b/lib/towerops/status_pages.ex @@ -97,7 +97,7 @@ defmodule Towerops.StatusPages do def resolve_incident(%StatusIncident{} = incident) do update_incident(incident, %{ status: "resolved", - resolved_at: DateTime.truncate(DateTime.utc_now(), :second) + resolved_at: Towerops.Time.now() }) end diff --git a/lib/towerops/time.ex b/lib/towerops/time.ex new file mode 100644 index 00000000..e34b07e0 --- /dev/null +++ b/lib/towerops/time.ex @@ -0,0 +1,20 @@ +defmodule Towerops.Time do + @moduledoc """ + Time utility functions used across the application. + """ + + @doc """ + Returns the current UTC time truncated to seconds. + + This is the standard timestamp format used throughout the application + for consistency with database timestamps and to avoid microsecond precision issues. + + ## Examples + + iex> Towerops.Time.now() + ~U[2024-03-15 10:30:45Z] + """ + def now do + DateTime.truncate(DateTime.utc_now(), :second) + end +end diff --git a/lib/towerops/trace.ex b/lib/towerops/trace.ex index c33c72d5..dd413b18 100644 --- a/lib/towerops/trace.ex +++ b/lib/towerops/trace.ex @@ -35,7 +35,7 @@ defmodule Towerops.Trace do if String.length(query) < 2 do [] else - pattern = "%#{sanitize_like(query)}%" + pattern = "%#{Towerops.QueryHelpers.sanitize_like(query)}%" accounts = search_accounts(organization_id, pattern) inventory = search_inventory(organization_id, pattern) @@ -452,6 +452,4 @@ defmodule Towerops.Trace do end defp format_address(_), do: nil - - defp sanitize_like(query), do: Towerops.QueryHelpers.sanitize_like(query) end diff --git a/lib/towerops/uisp/config_snapshot.ex b/lib/towerops/uisp/config_snapshot.ex index 5c14d039..092f9890 100644 --- a/lib/towerops/uisp/config_snapshot.ex +++ b/lib/towerops/uisp/config_snapshot.ex @@ -57,7 +57,7 @@ defmodule Towerops.Uisp.ConfigSnapshot do config_size_bytes: byte_size(config_json), compressed_size_bytes: byte_size(compressed), source: "uisp", - snapshot_at: DateTime.truncate(DateTime.utc_now(), :second) + snapshot_at: Towerops.Time.now() } case insert_snapshot(attrs) do @@ -164,7 +164,7 @@ defmodule Towerops.Uisp.ConfigSnapshot do |> Repo.insert_all([ Map.merge(attrs, %{ id: Ecto.UUID.generate(), - inserted_at: DateTime.truncate(DateTime.utc_now(), :second) + inserted_at: Towerops.Time.now() }) ]) |> case do diff --git a/lib/towerops/uisp/sync.ex b/lib/towerops/uisp/sync.ex index 0fa76d40..9ddf24a9 100644 --- a/lib/towerops/uisp/sync.ex +++ b/lib/towerops/uisp/sync.ex @@ -92,7 +92,7 @@ defmodule Towerops.Uisp.Sync do records_synced: records_synced, errors: errors, duration_ms: duration_ms, - inserted_at: DateTime.truncate(DateTime.utc_now(), :second) + inserted_at: Towerops.Time.now() }) |> Repo.insert() end diff --git a/lib/towerops/workers/device_monitor_worker.ex b/lib/towerops/workers/device_monitor_worker.ex index 66282609..7992a90e 100644 --- a/lib/towerops/workers/device_monitor_worker.ex +++ b/lib/towerops/workers/device_monitor_worker.ex @@ -130,7 +130,7 @@ defmodule Towerops.Workers.DeviceMonitorWorker do defp perform_check(device) do check_result = check_device_health(device) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {status, response_time_ms} = case check_result do @@ -175,7 +175,7 @@ defmodule Towerops.Workers.DeviceMonitorWorker do end defp handle_status_change(device, old_status, new_status) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() case {old_status, new_status} do {_, :down} -> diff --git a/lib/towerops/workers/device_poller_worker.ex b/lib/towerops/workers/device_poller_worker.ex index c932c572..f6bce53c 100644 --- a/lib/towerops/workers/device_poller_worker.ex +++ b/lib/towerops/workers/device_poller_worker.ex @@ -168,7 +168,7 @@ defmodule Towerops.Workers.DevicePollerWorker do Devices.update_snmp_poll_time(device) client_opts = build_client_opts(device) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Run all polling operations in parallel using async tasks task_names = [ @@ -416,7 +416,7 @@ defmodule Towerops.Workers.DevicePollerWorker do defp process_wireless_clients(_device, []), do: :ok defp process_wireless_clients(device, clients) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() cutoff = DateTime.add(now, -5, :minute) Snmp.delete_stale_and_upsert_wireless_clients(device.id, device.organization_id, clients, cutoff) diff --git a/lib/towerops/workers/gaiia_insight_worker.ex b/lib/towerops/workers/gaiia_insight_worker.ex index e9d30f5f..ca94f0a2 100644 --- a/lib/towerops/workers/gaiia_insight_worker.ex +++ b/lib/towerops/workers/gaiia_insight_worker.ex @@ -44,7 +44,7 @@ defmodule Towerops.Workers.GaiiaInsightWorker do alias Towerops.Preseem.Insight alias Towerops.Repo - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {count, _} = Insight diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index 6dbaf1c8..9ea5405a 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -1526,7 +1526,7 @@ defmodule ToweropsWeb.AgentChannel do :ok <- verify_device_organization(device, organization_id) do # Use server time to avoid clock skew issues # Agent timestamps can be unreliable due to clock drift - checked_at = DateTime.truncate(DateTime.utc_now(), :second) + checked_at = Towerops.Time.now() # Keep status as string for storage, convert to atom for device status update status_string = check.status @@ -1583,7 +1583,7 @@ defmodule ToweropsWeb.AgentChannel do end defp record_and_process_check_result(check, result, organization_id, agent_token_id) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() case Monitoring.create_check_result(%{ check_id: check.id, @@ -1699,7 +1699,7 @@ defmodule ToweropsWeb.AgentChannel do defp handle_agent_status_change(device, _old_status, :down) do if !Alerts.has_active_alert?(device.id, :device_down) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() message = if device.snmp_enabled, @@ -1722,7 +1722,7 @@ defmodule ToweropsWeb.AgentChannel do end defp handle_agent_status_change(device, _old_status, :up) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() message = if device.snmp_enabled, @@ -1752,7 +1752,7 @@ defmodule ToweropsWeb.AgentChannel do # Resolve any stuck alerts for a device that's currently up # This handles edge cases where device is UP but alerts didn't resolve (e.g., due to bugs) defp resolve_any_stuck_down_alerts(device) do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Resolve stuck device_down alerts case Alerts.get_active_alert(device.id, :device_down) do @@ -1957,7 +1957,7 @@ defmodule ToweropsWeb.AgentChannel do snmp_device = device.snmp_device oid_values = normalize_oid_keys(result.oid_values) # Use server time to avoid clock drift issues between agents - timestamp = DateTime.truncate(DateTime.utc_now(), :second) + timestamp = Towerops.Time.now() Logger.info( "Processing polling result for #{device.name}: #{map_size(oid_values)} OIDs, #{length(snmp_device.sensors)} sensors, #{length(snmp_device.interfaces)} interfaces" diff --git a/lib/towerops_web/controllers/api/v1/geoip_controller.ex b/lib/towerops_web/controllers/api/v1/geoip_controller.ex index 31dc2ff1..2a70a463 100644 --- a/lib/towerops_web/controllers/api/v1/geoip_controller.ex +++ b/lib/towerops_web/controllers/api/v1/geoip_controller.ex @@ -107,7 +107,7 @@ defmodule ToweropsWeb.Api.V1.GeoipController do Repo.delete_all(Location) end - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = Enum.map(data, fn location -> @@ -158,7 +158,7 @@ defmodule ToweropsWeb.Api.V1.GeoipController do Repo.delete_all(Block) end - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = Enum.map(data, fn block -> diff --git a/lib/towerops_web/live/device_live/form.ex b/lib/towerops_web/live/device_live/form.ex index 7cb9f904..d8f6d6f8 100644 --- a/lib/towerops_web/live/device_live/form.ex +++ b/lib/towerops_web/live/device_live/form.ex @@ -75,20 +75,11 @@ defmodule ToweropsWeb.DeviceLive.Form do |> merge_bool_param(:snmp_enabled, params["snmp_enabled"]) equipment_attrs = - cond do - # Use preselected site from URL params if available - socket.assigns.preselected_site_id -> - Map.put(equipment_attrs, :site_id, socket.assigns.preselected_site_id) - - # Auto-select if there's only one site - length(socket.assigns.available_sites) == 1 -> - site = List.first(socket.assigns.available_sites) - Map.put(equipment_attrs, :site_id, site.id) - - # Otherwise, leave it empty for user to select - true -> - equipment_attrs - end + maybe_preselect_site( + equipment_attrs, + socket.assigns.preselected_site_id, + socket.assigns.available_sites + ) changeset = Devices.change_device(%DeviceSchema{}, equipment_attrs) @@ -691,29 +682,31 @@ defmodule ToweropsWeb.DeviceLive.Form do # Determine effective agent ID considering inheritance chain defp determine_effective_agent_id(device_params, assigns) do - # Check device-level agent - form_agent_id = device_params["agent_token_id"] - - if form_agent_id && form_agent_id != "" do - form_agent_id + with nil <- get_device_agent(device_params), + nil <- get_site_agent(device_params, assigns), + nil <- get_org_agent(assigns) do + nil else - # Check site-level agent - site_id = device_params["site_id"] - site = site_id && Enum.find(assigns.available_sites, &(&1.id == site_id)) - site_agent_id = site && site.agent_token_id - - if site_agent_id do - site_agent_id - else - # Check organization-level agent - org = assigns.organization - org_agent_id = org && org.default_agent_token_id - - org_agent_id - end + agent_id when is_binary(agent_id) and agent_id != "" -> agent_id + _ -> nil end end + defp get_device_agent(%{"agent_token_id" => id}) when is_binary(id) and id != "", do: id + defp get_device_agent(_), do: nil + + defp get_site_agent(%{"site_id" => site_id}, %{available_sites: sites}) when is_binary(site_id) do + case Enum.find(sites, &(&1.id == site_id)) do + %{agent_token_id: id} when not is_nil(id) -> id + _ -> nil + end + end + + defp get_site_agent(_, _), do: nil + + defp get_org_agent(%{organization: %{default_agent_token_id: id}}) when not is_nil(id), do: id + defp get_org_agent(_), do: nil + defp send_credential_test_to_agent(agent_token_id, device_map, test_id) do require Logger @@ -842,6 +835,14 @@ defmodule ToweropsWeb.DeviceLive.Form do defp non_routable_ipv4_range?(100, b) when b >= 64 and b <= 127, do: true defp non_routable_ipv4_range?(_a, _b), do: false + # Auto-select site if preselected or if there's only one option + defp maybe_preselect_site(attrs, preselected_id, _sites) when not is_nil(preselected_id), + do: Map.put(attrs, :site_id, preselected_id) + + defp maybe_preselect_site(attrs, nil, [site]), do: Map.put(attrs, :site_id, site.id) + + defp maybe_preselect_site(attrs, nil, _sites), do: attrs + @impl true def terminate(_reason, socket) do # Unsubscribe from credential test topic if we're still subscribed diff --git a/lib/towerops_web/live/helpers/access_control.ex b/lib/towerops_web/live/helpers/access_control.ex index c0b32307..e8c220c3 100644 --- a/lib/towerops_web/live/helpers/access_control.ex +++ b/lib/towerops_web/live/helpers/access_control.ex @@ -8,8 +8,10 @@ defmodule ToweropsWeb.Live.Helpers.AccessControl do alias Towerops.Alerts.Alert alias Towerops.Devices + alias Towerops.Devices.Device alias Towerops.Repo alias Towerops.Sites + alias Towerops.Sites.Site @doc """ Verifies that a device belongs to the specified organization. @@ -28,18 +30,12 @@ defmodule ToweropsWeb.Live.Helpers.AccessControl do {:error, :not_found} """ @spec verify_device_access(binary(), binary()) :: - {:ok, Devices.Device.t()} | {:error, :not_found | :unauthorized} + {:ok, Device.t()} | {:error, :not_found | :unauthorized} def verify_device_access(device_id, organization_id) do case Devices.get_device(device_id) do - nil -> - {:error, :not_found} - - device -> - if device.organization_id == organization_id do - {:ok, device} - else - {:error, :unauthorized} - end + %Device{organization_id: ^organization_id} = device -> {:ok, device} + nil -> {:error, :not_found} + %Device{} -> {:error, :unauthorized} end end @@ -60,18 +56,12 @@ defmodule ToweropsWeb.Live.Helpers.AccessControl do {:error, :not_found} """ @spec verify_site_access(binary(), binary()) :: - {:ok, Sites.Site.t()} | {:error, :not_found | :unauthorized} + {:ok, Site.t()} | {:error, :not_found | :unauthorized} def verify_site_access(site_id, organization_id) do case Sites.get_site(site_id) do - nil -> - {:error, :not_found} - - site -> - if site.organization_id == organization_id do - {:ok, site} - else - {:error, :unauthorized} - end + %Site{organization_id: ^organization_id} = site -> {:ok, site} + nil -> {:error, :not_found} + %Site{} -> {:error, :unauthorized} end end @@ -95,18 +85,13 @@ defmodule ToweropsWeb.Live.Helpers.AccessControl do @spec verify_alert_access(binary(), binary()) :: {:ok, Alert.t()} | {:error, :not_found | :unauthorized} def verify_alert_access(alert_id, organization_id) do - case Repo.get(Alert, alert_id) do - nil -> - {:error, :not_found} - - alert -> - alert = Repo.preload(alert, [:acknowledged_by, device: [site: :organization]]) - - if alert.device.site.organization_id == organization_id do - {:ok, alert} - else - {:error, :unauthorized} - end + with %Alert{} = alert <- Repo.get(Alert, alert_id), + alert = Repo.preload(alert, [:acknowledged_by, device: [site: :organization]]), + %Alert{device: %{site: %{organization_id: ^organization_id}}} <- alert do + {:ok, alert} + else + nil -> {:error, :not_found} + %Alert{} -> {:error, :unauthorized} end end end diff --git a/lib/towerops_web/live/site_live/form.ex b/lib/towerops_web/live/site_live/form.ex index 28c7898f..b4912265 100644 --- a/lib/towerops_web/live/site_live/form.ex +++ b/lib/towerops_web/live/site_live/form.ex @@ -28,11 +28,10 @@ defmodule ToweropsWeb.SiteLive.Form do # Get inherited agent info from organization org_agent = - if socket.assigns.organization.default_agent_token_id do - Enum.find(socket.assigns.available_agents, fn a -> - a.id == socket.assigns.organization.default_agent_token_id - end) - end + find_org_agent( + socket.assigns.organization.default_agent_token_id, + socket.assigns.available_agents + ) socket |> assign(:page_title, t("New Site")) @@ -51,11 +50,10 @@ defmodule ToweropsWeb.SiteLive.Form do # Get inherited agent info from organization org_agent = - if socket.assigns.organization.default_agent_token_id do - Enum.find(socket.assigns.available_agents, fn a -> - a.id == socket.assigns.organization.default_agent_token_id - end) - end + find_org_agent( + socket.assigns.organization.default_agent_token_id, + socket.assigns.available_agents + ) socket |> assign(:page_title, t("Edit Site")) @@ -184,4 +182,8 @@ defmodule ToweropsWeb.SiteLive.Form do {:noreply, assign(socket, :form, to_form(changeset))} end end + + # Find organization's default agent in the available agents list + defp find_org_agent(nil, _agents), do: nil + defp find_org_agent(agent_id, agents), do: Enum.find(agents, &(&1.id == agent_id)) end diff --git a/priv/repo/seeds_e2e.exs b/priv/repo/seeds_e2e.exs index e6d16ab5..da66043d 100644 --- a/priv/repo/seeds_e2e.exs +++ b/priv/repo/seeds_e2e.exs @@ -31,7 +31,7 @@ end # Confirm the user's email directly (bypass token flow for seeding) {:ok, user} = user - |> Ecto.Changeset.change(%{confirmed_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{confirmed_at: Towerops.Time.now()}) |> Repo.update() # Enable TOTP for the user diff --git a/test/mix/tasks/geoip.import_test.exs b/test/mix/tasks/geoip.import_test.exs index 28233444..61ed6368 100644 --- a/test/mix/tasks/geoip.import_test.exs +++ b/test/mix/tasks/geoip.import_test.exs @@ -299,8 +299,8 @@ defmodule Mix.Tasks.Geoip.ImportTest do subdivision_2_name: nil, latitude: nil, longitude: nil, - inserted_at: DateTime.truncate(DateTime.utc_now(), :second), - updated_at: DateTime.truncate(DateTime.utc_now(), :second) + inserted_at: Towerops.Time.now(), + updated_at: Towerops.Time.now() } attrs = Map.merge(defaults, Map.new(attrs)) diff --git a/test/support/fixtures/snmp_fixtures.ex b/test/support/fixtures/snmp_fixtures.ex index 41d8120a..bec4bde5 100644 --- a/test/support/fixtures/snmp_fixtures.ex +++ b/test/support/fixtures/snmp_fixtures.ex @@ -103,7 +103,7 @@ defmodule Towerops.SnmpFixtures do tx_rate: 100_000, rx_rate: 50_000, uptime_seconds: 3600, - last_seen_at: DateTime.truncate(DateTime.utc_now(), :second), + last_seen_at: Towerops.Time.now(), metadata: %{} } @@ -145,7 +145,7 @@ defmodule Towerops.SnmpFixtures do mac_address: default_mac, ip_address: "10.0.#{:rand.uniform(254)}.#{:rand.uniform(254)}", interface: "ether1", - last_seen_at: DateTime.truncate(DateTime.utc_now(), :second) + last_seen_at: Towerops.Time.now() } merged_attrs = @@ -321,7 +321,7 @@ defmodule Towerops.SnmpFixtures do # Default reading attributes default_attrs = %{ transceiver_id: transceiver.id, - measured_at: DateTime.truncate(DateTime.utc_now(), :second) + measured_at: Towerops.Time.now() } # Merge with provided attrs diff --git a/test/towerops/accounts/user_consent_test.exs b/test/towerops/accounts/user_consent_test.exs index 92215275..6c4446ab 100644 --- a/test/towerops/accounts/user_consent_test.exs +++ b/test/towerops/accounts/user_consent_test.exs @@ -238,7 +238,7 @@ defmodule Towerops.Accounts.UserConsentTest do end test "successfully revokes consent", %{consent: consent} do - revoked_at = DateTime.truncate(DateTime.utc_now(), :second) + revoked_at = Towerops.Time.now() changeset = UserConsent.revoke_changeset(consent, %{revoked_at: revoked_at}) assert {:ok, updated_consent} = Repo.update(changeset) diff --git a/test/towerops/agents/cloud_poller_test.exs b/test/towerops/agents/cloud_poller_test.exs index 62b2daf6..f900ef2b 100644 --- a/test/towerops/agents/cloud_poller_test.exs +++ b/test/towerops/agents/cloud_poller_test.exs @@ -13,7 +13,7 @@ defmodule Towerops.Agents.CloudPollerTest do {:ok, cloud_poller, _token} = Agents.create_cloud_poller("Cloud 1") cloud_poller - |> Ecto.Changeset.change(%{last_seen_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{last_seen_at: Towerops.Time.now()}) |> Repo.update!() pollers = Agents.list_online_cloud_pollers() @@ -39,7 +39,7 @@ defmodule Towerops.Agents.CloudPollerTest do cloud_poller |> Ecto.Changeset.change(%{ - last_seen_at: DateTime.truncate(DateTime.utc_now(), :second), + last_seen_at: Towerops.Time.now(), enabled: false }) |> Repo.update!() @@ -53,7 +53,7 @@ defmodule Towerops.Agents.CloudPollerTest do {:ok, agent, _token} = Agents.create_agent_token(org.id, "Regular Agent") agent - |> Ecto.Changeset.change(%{last_seen_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{last_seen_at: Towerops.Time.now()}) |> Repo.update!() assert Agents.list_online_cloud_pollers() == [] diff --git a/test/towerops/agents/stats_test.exs b/test/towerops/agents/stats_test.exs index 58bddf19..8c50a5d2 100644 --- a/test/towerops/agents/stats_test.exs +++ b/test/towerops/agents/stats_test.exs @@ -33,7 +33,7 @@ defmodule Towerops.Agents.StatsTest do # Touch last_seen_at to make it online agent_token |> Ecto.Changeset.change(%{ - last_seen_at: DateTime.truncate(DateTime.utc_now(), :second), + last_seen_at: Towerops.Time.now(), metadata: %{"version" => "0.1.0", "hostname" => "test-host"} }) |> Repo.update!() @@ -90,7 +90,7 @@ defmodule Towerops.Agents.StatsTest do {:ok, _assignment} = Agents.assign_device_to_agent(agent_token.id, device.id) agent_token - |> Ecto.Changeset.change(%{last_seen_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{last_seen_at: Towerops.Time.now()}) |> Repo.update!() health = Stats.get_organization_agent_health(org.id) @@ -157,7 +157,7 @@ defmodule Towerops.Agents.StatsTest do {:ok, agent_token, _token} = Agents.create_agent_token(org.id, "Online Agent") agent_token - |> Ecto.Changeset.change(%{last_seen_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{last_seen_at: Towerops.Time.now()}) |> Repo.update!() offline = Stats.get_offline_agents(org.id) @@ -506,7 +506,7 @@ defmodule Towerops.Agents.StatsTest do }) # Create checks from agent1 with avg 50ms (bulk insert for speed) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() checks_agent1 = for _ <- 1..10 do diff --git a/test/towerops/devices_test.exs b/test/towerops/devices_test.exs index c8948ba0..17f624d4 100644 --- a/test/towerops/devices_test.exs +++ b/test/towerops/devices_test.exs @@ -685,7 +685,7 @@ defmodule Towerops.EquipmentTest do severity: "info", message: "Interface came online", metadata: %{"interface" => "eth0"}, - occurred_at: DateTime.truncate(DateTime.utc_now(), :second) + occurred_at: Towerops.Time.now() } assert {:ok, %Event{} = event} = Devices.create_event(attrs) diff --git a/test/towerops/equipment/event_logger_test.exs b/test/towerops/equipment/event_logger_test.exs index 36ee8db6..621ac439 100644 --- a/test/towerops/equipment/event_logger_test.exs +++ b/test/towerops/equipment/event_logger_test.exs @@ -49,7 +49,7 @@ defmodule Towerops.Devices.EventLoggerTest do old_speed: nil, new_speed: 1_000_000_000 }, - occurred_at: DateTime.truncate(DateTime.utc_now(), :second) + occurred_at: Towerops.Time.now() } # Broadcast event via PubSub to org-scoped topic @@ -82,7 +82,7 @@ defmodule Towerops.Devices.EventLoggerTest do end test "EventLogger handles multiple events in sequence", %{device: device, organization: organization} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Broadcast multiple events for i <- 1..3 do diff --git a/test/towerops/gaiia/site_aggregation_test.exs b/test/towerops/gaiia/site_aggregation_test.exs index 55ebf0a6..d972e749 100644 --- a/test/towerops/gaiia/site_aggregation_test.exs +++ b/test/towerops/gaiia/site_aggregation_test.exs @@ -375,7 +375,7 @@ defmodule Towerops.Gaiia.SiteAggregationTest do device = device_fixture(%{organization_id: org.id, site: towerops_site}) # Create a device_subscriber_link connecting the device to the account - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() Repo.insert!(%DeviceSubscriberLink{ organization_id: org.id, diff --git a/test/towerops/gaiia_test.exs b/test/towerops/gaiia_test.exs index 388c7e8a..36a1db57 100644 --- a/test/towerops/gaiia_test.exs +++ b/test/towerops/gaiia_test.exs @@ -194,7 +194,7 @@ defmodule Towerops.GaiiaTest do mrr_amount: Decimal.new("50.00") }) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Same account linked to AP via wireless_mac (high confidence) Repo.insert!(%Gaiia.DeviceSubscriberLink{ @@ -251,7 +251,7 @@ defmodule Towerops.GaiiaTest do mrr_amount: Decimal.new("75.00") }) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Account only linked to router via ARP (no AP match) Repo.insert!(%Gaiia.DeviceSubscriberLink{ @@ -292,7 +292,7 @@ defmodule Towerops.GaiiaTest do mrr_amount: Decimal.new("60.00") }) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Same account linked to both devices via arp_ip (same method) Repo.insert!(%Gaiia.DeviceSubscriberLink{ @@ -344,7 +344,7 @@ defmodule Towerops.GaiiaTest do mrr_amount: Decimal.new("50.00") }) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Link via CGNAT IP — should be excluded from per-device counts Repo.insert!(%Gaiia.DeviceSubscriberLink{ @@ -383,7 +383,7 @@ defmodule Towerops.GaiiaTest do mrr_amount: Decimal.new("80.00") }) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() Repo.insert!(%Gaiia.DeviceSubscriberLink{ organization_id: org.id, @@ -432,7 +432,7 @@ defmodule Towerops.GaiiaTest do mrr_amount: Decimal.new("50.00") }) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Link to router via LAN IP Repo.insert!(%Gaiia.DeviceSubscriberLink{ @@ -496,7 +496,7 @@ defmodule Towerops.GaiiaTest do mrr_amount: Decimal.new("60.00") }) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # acct1 on AP-1 and router (AP wins via wireless_mac) Repo.insert!(%Gaiia.DeviceSubscriberLink{ diff --git a/test/towerops/monitoring/monitoring_check_test.exs b/test/towerops/monitoring/monitoring_check_test.exs index 219bf138..35b5bf3d 100644 --- a/test/towerops/monitoring/monitoring_check_test.exs +++ b/test/towerops/monitoring/monitoring_check_test.exs @@ -51,7 +51,7 @@ defmodule Towerops.Monitoring.MonitoringCheckTest do describe "Monitoring.create_monitoring_check/1" do test "inserts a monitoring check record" do {device, agent} = create_device_with_agent() - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() assert {:ok, check} = Monitoring.create_monitoring_check(%{ @@ -71,7 +71,7 @@ defmodule Towerops.Monitoring.MonitoringCheckTest do test "inserts without agent_token_id" do {device, _agent} = create_device_with_agent() - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() assert {:ok, check} = Monitoring.create_monitoring_check(%{ diff --git a/test/towerops/monitoring_test.exs b/test/towerops/monitoring_test.exs index c9a13b1b..55075262 100644 --- a/test/towerops/monitoring_test.exs +++ b/test/towerops/monitoring_test.exs @@ -370,7 +370,7 @@ defmodule Towerops.MonitoringTest do attrs = %{ check_id: check.id, organization_id: org.id, - checked_at: DateTime.truncate(DateTime.utc_now(), :second), + checked_at: Towerops.Time.now(), status: 0, output: "HTTP 200 OK", response_time_ms: 42.5, @@ -390,7 +390,7 @@ defmodule Towerops.MonitoringTest do describe "get_latest_check_result/1" do test "returns the most recent result", %{organization: org} do {:ok, check} = Monitoring.create_check(valid_check_attrs(org.id)) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {:ok, _old} = Monitoring.create_check_result(%{ @@ -420,7 +420,7 @@ defmodule Towerops.MonitoringTest do describe "get_check_results/2" do test "returns results within time range", %{organization: org} do {:ok, check} = Monitoring.create_check(valid_check_attrs(org.id)) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {:ok, in_range} = Monitoring.create_check_result(%{ @@ -450,7 +450,7 @@ defmodule Towerops.MonitoringTest do test "respects limit", %{organization: org} do {:ok, check} = Monitoring.create_check(valid_check_attrs(org.id)) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() for i <- 1..5 do Monitoring.create_check_result(%{ @@ -521,7 +521,7 @@ defmodule Towerops.MonitoringTest do describe "get_device_health_summary/1" do test "returns health summary for devices with checks", %{organization: org, device: device} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {:ok, _check} = Monitoring.create_check(valid_check_attrs(org.id, %{device_id: device.id})) @@ -573,7 +573,7 @@ defmodule Towerops.MonitoringTest do device_id: device.id, status: "up", response_time_ms: 12.5, - checked_at: DateTime.truncate(DateTime.utc_now(), :second) + checked_at: Towerops.Time.now() } assert {:ok, mc} = Monitoring.create_monitoring_check(attrs) @@ -588,7 +588,7 @@ defmodule Towerops.MonitoringTest do describe "get_device_latest_response_times/1" do test "returns latest response time per device", %{device: device} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() Monitoring.create_monitoring_check(%{ device_id: device.id, diff --git a/test/towerops/organizations/invitation_test.exs b/test/towerops/organizations/invitation_test.exs index 39faeee6..43c8af83 100644 --- a/test/towerops/organizations/invitation_test.exs +++ b/test/towerops/organizations/invitation_test.exs @@ -191,7 +191,7 @@ defmodule Towerops.Organizations.InvitationTest do end test "allows accepted_at to be set" do - accepted_at = DateTime.truncate(DateTime.utc_now(), :second) + accepted_at = Towerops.Time.now() attrs = %{ email: "test@example.com", diff --git a/test/towerops/organizations_test.exs b/test/towerops/organizations_test.exs index 5b18b94a..71b2e77c 100644 --- a/test/towerops/organizations_test.exs +++ b/test/towerops/organizations_test.exs @@ -248,7 +248,7 @@ defmodule Towerops.OrganizationsTest do invited_by_id: owner.id, token: "test-token-#{System.unique_integer()}", expires_at: DateTime.truncate(DateTime.add(DateTime.utc_now(), 7, :day), :second), - accepted_at: DateTime.truncate(DateTime.utc_now(), :second) + accepted_at: Towerops.Time.now() }) invitations = Organizations.list_pending_invitations(organization.id) @@ -325,7 +325,7 @@ defmodule Towerops.OrganizationsTest do invited_by_id: owner.id, token: token, expires_at: DateTime.truncate(DateTime.add(DateTime.utc_now(), 7, :day), :second), - accepted_at: DateTime.truncate(DateTime.utc_now(), :second) + accepted_at: Towerops.Time.now() }) assert Organizations.get_invitation_by_token(token) == nil diff --git a/test/towerops/preseem/device_baseline_test.exs b/test/towerops/preseem/device_baseline_test.exs index df7e1438..fb10f96e 100644 --- a/test/towerops/preseem/device_baseline_test.exs +++ b/test/towerops/preseem/device_baseline_test.exs @@ -30,7 +30,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do p5: 72.0, p95: 95.3, sample_count: 336, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -43,7 +43,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do period: "busy", mean: 85.5, sample_count: 100, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -57,7 +57,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do period: "busy", mean: 85.5, sample_count: 100, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -71,7 +71,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do metric_name: "qoe_score", mean: 85.5, sample_count: 100, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -85,7 +85,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do metric_name: "qoe_score", period: "busy", sample_count: 100, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -99,7 +99,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do metric_name: "qoe_score", period: "busy", mean: 85.5, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -128,7 +128,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do period: "invalid_period", mean: 85.5, sample_count: 100, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -144,7 +144,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do period: period, mean: 85.5, sample_count: 100, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -159,7 +159,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do period: "busy", mean: 85.5, sample_count: 100, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -178,7 +178,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do period: "busy", mean: 85.5, sample_count: 100, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = DeviceBaseline.changeset(%DeviceBaseline{}, attrs) @@ -187,7 +187,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do end test "persists and reloads from database", %{access_point: ap} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() attrs = %{ preseem_access_point_id: ap.id, @@ -216,7 +216,7 @@ defmodule Towerops.Preseem.DeviceBaselineTest do end test "enforces unique constraint on [access_point_id, metric_name, period]", %{access_point: ap} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() attrs = %{ preseem_access_point_id: ap.id, diff --git a/test/towerops/preseem/fleet_profile_test.exs b/test/towerops/preseem/fleet_profile_test.exs index 4c8a6749..22334317 100644 --- a/test/towerops/preseem/fleet_profile_test.exs +++ b/test/towerops/preseem/fleet_profile_test.exs @@ -26,7 +26,7 @@ defmodule Towerops.Preseem.FleetProfileTest do capacity_ceiling: 35, avg_busy_hours: 6.2, performance_data: %{"p50_qoe" => 84.0, "p10_qoe" => 65.2}, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = FleetProfile.changeset(%FleetProfile{}, attrs) @@ -38,7 +38,7 @@ defmodule Towerops.Preseem.FleetProfileTest do manufacturer: "Cambium", model: "ePMP 3000", device_count: 25, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = FleetProfile.changeset(%FleetProfile{}, attrs) @@ -51,7 +51,7 @@ defmodule Towerops.Preseem.FleetProfileTest do organization_id: Ecto.UUID.generate(), model: "ePMP 3000", device_count: 25, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = FleetProfile.changeset(%FleetProfile{}, attrs) @@ -64,7 +64,7 @@ defmodule Towerops.Preseem.FleetProfileTest do organization_id: Ecto.UUID.generate(), manufacturer: "Cambium", device_count: 25, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = FleetProfile.changeset(%FleetProfile{}, attrs) @@ -77,7 +77,7 @@ defmodule Towerops.Preseem.FleetProfileTest do organization_id: Ecto.UUID.generate(), manufacturer: "Cambium", model: "ePMP 3000", - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = FleetProfile.changeset(%FleetProfile{}, attrs) @@ -99,7 +99,7 @@ defmodule Towerops.Preseem.FleetProfileTest do end test "persists and reloads from database", %{organization: org} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() attrs = %{ organization_id: org.id, @@ -151,7 +151,7 @@ defmodule Towerops.Preseem.FleetProfileTest do model: "ePMP 3000", device_count: 15, performance_data: performance_data, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } {:ok, profile} = %FleetProfile{} |> FleetProfile.changeset(attrs) |> Repo.insert() @@ -166,7 +166,7 @@ defmodule Towerops.Preseem.FleetProfileTest do manufacturer: "Cambium", model: "ePMP 3000", device_count: 25, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() } changeset = FleetProfile.changeset(%FleetProfile{}, attrs) diff --git a/test/towerops/preseem/insights_test.exs b/test/towerops/preseem/insights_test.exs index 434be0e7..fd7a0a3d 100644 --- a/test/towerops/preseem/insights_test.exs +++ b/test/towerops/preseem/insights_test.exs @@ -547,7 +547,7 @@ defmodule Towerops.Preseem.InsightsTest do metric_name: metric_name, period: period, sample_count: 336, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() }, attrs ) @@ -581,7 +581,7 @@ defmodule Towerops.Preseem.InsightsTest do manufacturer: model |> String.split(" ") |> hd(), model: model, device_count: 10, - computed_at: DateTime.truncate(DateTime.utc_now(), :second) + computed_at: Towerops.Time.now() }, attrs ) diff --git a/test/towerops/preseem/subscriber_metric_test.exs b/test/towerops/preseem/subscriber_metric_test.exs index 44813ec3..67801280 100644 --- a/test/towerops/preseem/subscriber_metric_test.exs +++ b/test/towerops/preseem/subscriber_metric_test.exs @@ -29,7 +29,7 @@ defmodule Towerops.Preseem.SubscriberMetricTest do avg_throughput: 45.8, p95_latency: 28.3, subscriber_count: 42, - recorded_at: DateTime.truncate(DateTime.utc_now(), :second) + recorded_at: Towerops.Time.now() } changeset = SubscriberMetric.changeset(%SubscriberMetric{}, attrs) @@ -37,7 +37,7 @@ defmodule Towerops.Preseem.SubscriberMetricTest do end test "requires preseem_access_point_id" do - attrs = %{recorded_at: DateTime.truncate(DateTime.utc_now(), :second)} + attrs = %{recorded_at: Towerops.Time.now()} changeset = SubscriberMetric.changeset(%SubscriberMetric{}, attrs) refute changeset.valid? assert "can't be blank" in errors_on(changeset).preseem_access_point_id @@ -56,7 +56,7 @@ defmodule Towerops.Preseem.SubscriberMetricTest do avg_latency: 12.5, p95_latency: 28.3, subscriber_count: 42, - recorded_at: DateTime.truncate(DateTime.utc_now(), :second) + recorded_at: Towerops.Time.now() } {:ok, metric} = %SubscriberMetric{} |> SubscriberMetric.changeset(attrs) |> Repo.insert() diff --git a/test/towerops/preseem/sync_log_test.exs b/test/towerops/preseem/sync_log_test.exs index 31692682..29a8d572 100644 --- a/test/towerops/preseem/sync_log_test.exs +++ b/test/towerops/preseem/sync_log_test.exs @@ -22,7 +22,7 @@ defmodule Towerops.Preseem.SyncLogTest do status: "success", records_synced: 42, duration_ms: 1500, - inserted_at: DateTime.truncate(DateTime.utc_now(), :second) + inserted_at: Towerops.Time.now() } changeset = SyncLog.changeset(%SyncLog{}, attrs) @@ -30,7 +30,7 @@ defmodule Towerops.Preseem.SyncLogTest do end test "requires organization_id and integration_id" do - attrs = %{status: "success", inserted_at: DateTime.truncate(DateTime.utc_now(), :second)} + attrs = %{status: "success", inserted_at: Towerops.Time.now()} changeset = SyncLog.changeset(%SyncLog{}, attrs) refute changeset.valid? assert "can't be blank" in errors_on(changeset).organization_id @@ -41,7 +41,7 @@ defmodule Towerops.Preseem.SyncLogTest do attrs = %{ organization_id: Ecto.UUID.generate(), integration_id: Ecto.UUID.generate(), - inserted_at: DateTime.truncate(DateTime.utc_now(), :second) + inserted_at: Towerops.Time.now() } changeset = SyncLog.changeset(%SyncLog{}, attrs) @@ -53,7 +53,7 @@ defmodule Towerops.Preseem.SyncLogTest do base = %{ organization_id: Ecto.UUID.generate(), integration_id: Ecto.UUID.generate(), - inserted_at: DateTime.truncate(DateTime.utc_now(), :second) + inserted_at: Towerops.Time.now() } for status <- ~w(success partial failed) do @@ -73,7 +73,7 @@ defmodule Towerops.Preseem.SyncLogTest do integration_id: integration.id, status: "failed", errors: errors, - inserted_at: DateTime.truncate(DateTime.utc_now(), :second) + inserted_at: Towerops.Time.now() } {:ok, log} = %SyncLog{} |> SyncLog.changeset(attrs) |> Repo.insert() diff --git a/test/towerops/preseem_test.exs b/test/towerops/preseem_test.exs index 3598c36b..8615866f 100644 --- a/test/towerops/preseem_test.exs +++ b/test/towerops/preseem_test.exs @@ -33,7 +33,7 @@ defmodule Towerops.PreseemTest do defp insert_metric!(ap, attrs) do default = %{ preseem_access_point_id: ap.id, - recorded_at: DateTime.truncate(DateTime.utc_now(), :second) + recorded_at: Towerops.Time.now() } {:ok, metric} = @@ -122,7 +122,7 @@ defmodule Towerops.PreseemTest do describe "list_subscriber_metrics/2" do test "returns metrics ordered by recorded_at desc", %{organization: org} do ap = insert_access_point!(org) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() old = DateTime.add(now, -3600, :second) insert_metric!(ap, %{recorded_at: old, avg_latency: 10.0}) @@ -135,7 +135,7 @@ defmodule Towerops.PreseemTest do test "respects limit option", %{organization: org} do ap = insert_access_point!(org) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() for i <- 1..5 do insert_metric!(ap, %{ diff --git a/test/towerops/rf_links_test.exs b/test/towerops/rf_links_test.exs index ad0b92b6..8caa6ba4 100644 --- a/test/towerops/rf_links_test.exs +++ b/test/towerops/rf_links_test.exs @@ -115,7 +115,7 @@ defmodule Towerops.RfLinksTest do base = %{ device_id: device.id, organization_id: org.id, - last_seen_at: DateTime.truncate(DateTime.utc_now(), :second) + last_seen_at: Towerops.Time.now() } {:ok, client} = diff --git a/test/towerops/snmp/batch_insert_test.exs b/test/towerops/snmp/batch_insert_test.exs index 7035f9d6..151a85ac 100644 --- a/test/towerops/snmp/batch_insert_test.exs +++ b/test/towerops/snmp/batch_insert_test.exs @@ -139,7 +139,7 @@ defmodule Towerops.Snmp.BatchInsertTest do describe "create_sensor_readings_batch/1" do test "inserts multiple sensor readings", %{sensor: sensor, sensor2: sensor2} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{sensor_id: sensor.id, value: 25.5, status: "ok", checked_at: now}, @@ -169,7 +169,7 @@ defmodule Towerops.Snmp.BatchInsertTest do end test "generates unique UUIDs for each entry", %{sensor: sensor} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{sensor_id: sensor.id, value: 10.0, status: "ok", checked_at: now}, @@ -184,7 +184,7 @@ defmodule Towerops.Snmp.BatchInsertTest do end test "handles optional state_descr field", %{sensor: sensor} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{ @@ -203,7 +203,7 @@ defmodule Towerops.Snmp.BatchInsertTest do end test "handles entries with nil value", %{sensor: sensor} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{sensor_id: sensor.id, value: nil, status: "error", checked_at: now} @@ -219,7 +219,7 @@ defmodule Towerops.Snmp.BatchInsertTest do describe "create_interface_stats_batch/1" do test "inserts multiple interface stats", %{interface: interface, interface2: interface2} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{ @@ -273,7 +273,7 @@ defmodule Towerops.Snmp.BatchInsertTest do end test "generates unique UUIDs for each entry", %{interface: interface} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{interface_id: interface.id, if_in_octets: 100, if_out_octets: 200, checked_at: now}, @@ -288,7 +288,7 @@ defmodule Towerops.Snmp.BatchInsertTest do end test "handles entries with nil counter values", %{interface: interface} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{interface_id: interface.id, if_in_octets: nil, if_out_octets: nil, checked_at: now} @@ -304,7 +304,7 @@ defmodule Towerops.Snmp.BatchInsertTest do describe "create_processor_readings_batch/1" do test "inserts multiple processor readings", %{processor: processor, processor2: processor2} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{processor_id: processor.id, load_percent: 45.0, status: "ok", checked_at: now}, @@ -334,7 +334,7 @@ defmodule Towerops.Snmp.BatchInsertTest do end test "generates unique UUIDs for each entry", %{processor: processor} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{processor_id: processor.id, load_percent: 10.0, status: "ok", checked_at: now}, @@ -349,7 +349,7 @@ defmodule Towerops.Snmp.BatchInsertTest do end test "handles entries with nil load_percent", %{processor: processor} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{processor_id: processor.id, load_percent: nil, status: "error", checked_at: now} @@ -365,7 +365,7 @@ defmodule Towerops.Snmp.BatchInsertTest do describe "create_storage_readings_batch/1" do test "inserts multiple storage readings", %{storage: storage, storage2: storage2} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{ @@ -409,7 +409,7 @@ defmodule Towerops.Snmp.BatchInsertTest do end test "generates unique UUIDs for each entry", %{storage: storage} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{ @@ -436,7 +436,7 @@ defmodule Towerops.Snmp.BatchInsertTest do end test "handles entries with nil optional fields", %{storage: storage} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{ diff --git a/test/towerops/snmp/entity_physical_reading_test.exs b/test/towerops/snmp/entity_physical_reading_test.exs index dbe82a08..e55a7114 100644 --- a/test/towerops/snmp/entity_physical_reading_test.exs +++ b/test/towerops/snmp/entity_physical_reading_test.exs @@ -139,7 +139,7 @@ defmodule Towerops.Snmp.EntityPhysicalReadingTest do }) |> Repo.insert() - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() attrs = %{ entity_physical_id: entity.id, diff --git a/test/towerops/snmp/mac_address_test.exs b/test/towerops/snmp/mac_address_test.exs index 4cbff26c..e2d5019e 100644 --- a/test/towerops/snmp/mac_address_test.exs +++ b/test/towerops/snmp/mac_address_test.exs @@ -116,7 +116,7 @@ defmodule Towerops.Snmp.MacAddressTest do end test "enforces unique constraint on device_id, mac_address, vlan_id", %{device: device} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {:ok, _} = %MacAddress{} @@ -142,7 +142,7 @@ defmodule Towerops.Snmp.MacAddressTest do end test "allows same mac_address on different VLANs", %{device: device} do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {:ok, _} = %MacAddress{} diff --git a/test/towerops/snmp/memory_pool_test.exs b/test/towerops/snmp/memory_pool_test.exs index 4de42585..82b3ba32 100644 --- a/test/towerops/snmp/memory_pool_test.exs +++ b/test/towerops/snmp/memory_pool_test.exs @@ -135,7 +135,7 @@ defmodule Towerops.Snmp.MemoryPoolTest do describe "unique constraint" do test "prevents duplicate pool_index on same device", %{snmp_device: snmp_device} do # First pool - use Repo.insert_all for speed - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() pool1 = %{ id: Ecto.UUID.generate(), diff --git a/test/towerops/snmp_test.exs b/test/towerops/snmp_test.exs index ee2839b2..dd986cbb 100644 --- a/test/towerops/snmp_test.exs +++ b/test/towerops/snmp_test.exs @@ -1172,7 +1172,7 @@ defmodule Towerops.SnmpTest do }) |> Repo.insert!() - new_time = DateTime.truncate(DateTime.utc_now(), :second) + new_time = Towerops.Time.now() attrs = %{ device_id: device.id, @@ -1805,7 +1805,7 @@ defmodule Towerops.SnmpTest do }) |> Repo.insert!() - new_time = DateTime.truncate(DateTime.utc_now(), :second) + new_time = Towerops.Time.now() attrs = %{ device_id: device.id, @@ -2628,7 +2628,7 @@ defmodule Towerops.SnmpTest do |> Interface.changeset(%{snmp_device_id: snmp_device.id, if_index: 1, if_name: "eth0"}) |> Repo.insert!() - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() cutoff = DateTime.add(now, -5, :minute) # Create a stale ARP entry @@ -2671,7 +2671,7 @@ defmodule Towerops.SnmpTest do |> Interface.changeset(%{snmp_device_id: snmp_device.id, if_index: 1, if_name: "eth0"}) |> Repo.insert!() - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() cutoff = DateTime.add(now, -5, :minute) # Create a stale MAC entry @@ -2941,7 +2941,7 @@ defmodule Towerops.SnmpTest do tx_rate: 100_000, rx_rate: 50_000, uptime_seconds: 3600, - last_seen_at: DateTime.truncate(DateTime.utc_now(), :second) + last_seen_at: Towerops.Time.now() }) |> Repo.insert!() @@ -2953,7 +2953,7 @@ defmodule Towerops.SnmpTest do organization: organization, wireless_client: wireless_client } do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{ @@ -3025,7 +3025,7 @@ defmodule Towerops.SnmpTest do organization: organization, wireless_client: wireless_client } do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{ @@ -3051,7 +3051,7 @@ defmodule Towerops.SnmpTest do organization: organization, wireless_client: wireless_client } do - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() entries = [ %{ diff --git a/test/towerops/status_pages_test.exs b/test/towerops/status_pages_test.exs index c5746cb0..c570824c 100644 --- a/test/towerops/status_pages_test.exs +++ b/test/towerops/status_pages_test.exs @@ -85,7 +85,7 @@ defmodule Towerops.StatusPagesTest do title: "Network Outage", severity: "major", status: "investigating", - started_at: DateTime.truncate(DateTime.utc_now(), :second), + started_at: Towerops.Time.now(), status_page_config_id: config.id }) diff --git a/test/towerops/workers/agent_latency_evaluator_test.exs b/test/towerops/workers/agent_latency_evaluator_test.exs index 62309eba..973e3d4d 100644 --- a/test/towerops/workers/agent_latency_evaluator_test.exs +++ b/test/towerops/workers/agent_latency_evaluator_test.exs @@ -237,7 +237,7 @@ defmodule Towerops.Workers.AgentLatencyEvaluatorTest do snmp_version: "2c" }) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() for_result = for _ <- 1..10 do diff --git a/test/towerops/workers/cloud_latency_probe_worker_test.exs b/test/towerops/workers/cloud_latency_probe_worker_test.exs index bcda1f40..6d7c190b 100644 --- a/test/towerops/workers/cloud_latency_probe_worker_test.exs +++ b/test/towerops/workers/cloud_latency_probe_worker_test.exs @@ -16,7 +16,7 @@ defmodule Towerops.Workers.CloudLatencyProbeWorkerTest do {:ok, cloud_poller, _token} = Agents.create_cloud_poller("Cloud 1") cloud_poller - |> Ecto.Changeset.change(%{last_seen_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{last_seen_at: Towerops.Time.now()}) |> Repo.update!() {:ok, site} = Sites.create_site(%{name: "Test Site", organization_id: org.id}) @@ -68,7 +68,7 @@ defmodule Towerops.Workers.CloudLatencyProbeWorkerTest do {:ok, cloud_poller, _token} = Agents.create_cloud_poller("Cloud 1") cloud_poller - |> Ecto.Changeset.change(%{last_seen_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{last_seen_at: Towerops.Time.now()}) |> Repo.update!() Phoenix.PubSub.subscribe(Towerops.PubSub, "agent:#{cloud_poller.id}:latency_probe") @@ -86,7 +86,7 @@ defmodule Towerops.Workers.CloudLatencyProbeWorkerTest do {:ok, local_agent, _token} = Agents.create_agent_token(org.id, "Local Agent") cloud_poller - |> Ecto.Changeset.change(%{last_seen_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{last_seen_at: Towerops.Time.now()}) |> Repo.update!() {:ok, site} = Sites.create_site(%{name: "Test Site", organization_id: org.id}) diff --git a/test/towerops/workers/data_retention_worker_test.exs b/test/towerops/workers/data_retention_worker_test.exs index 3f41c081..454c8fc2 100644 --- a/test/towerops/workers/data_retention_worker_test.exs +++ b/test/towerops/workers/data_retention_worker_test.exs @@ -16,7 +16,7 @@ defmodule Towerops.Workers.DataRetentionWorkerTest do {:ok, org} = Organizations.update_organization(org, %{data_retention_days: 30}) device = device_fixture(%{organization_id: org.id}) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() old_time = DateTime.add(now, -31, :day) recent_time = DateTime.add(now, -10, :day) @@ -62,7 +62,7 @@ defmodule Towerops.Workers.DataRetentionWorkerTest do org = organization_fixture(user.id) {:ok, org} = Organizations.update_organization(org, %{data_retention_days: 30}) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() old_time = DateTime.add(now, -31, :day) recent_time = DateTime.add(now, -10, :day) @@ -123,7 +123,7 @@ defmodule Towerops.Workers.DataRetentionWorkerTest do {:ok, org2} = Organizations.update_organization(org2, %{data_retention_days: 90}) device2 = device_fixture(%{organization_id: org2.id}) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # 75 days old: past org1's 60-day retention, within org2's 90-day retention mid_time = DateTime.add(now, -75, :day) @@ -179,7 +179,7 @@ defmodule Towerops.Workers.DataRetentionWorkerTest do # Default 365-day retention device = device_fixture(%{organization_id: org.id}) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() recent_time = DateTime.add(now, -100, :day) recent_id = Ecto.UUID.generate() @@ -215,7 +215,7 @@ defmodule Towerops.Workers.DataRetentionWorkerTest do {:ok, org} = Organizations.update_organization(org, %{data_retention_days: 30}) device = device_fixture(%{organization_id: org.id}) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() old_time = DateTime.add(now, -31, :day) Repo.insert_all("monitoring_checks", [ @@ -247,7 +247,7 @@ defmodule Towerops.Workers.DataRetentionWorkerTest do {:ok, org} = Organizations.update_organization(org, %{data_retention_days: 30}) device = device_fixture(%{organization_id: org.id}) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() old_time = DateTime.add(now, -31, :day) resolved_id = Ecto.UUID.generate() diff --git a/test/towerops/workers/preseem_sync_worker_test.exs b/test/towerops/workers/preseem_sync_worker_test.exs index 2ae440b0..ac0fa67b 100644 --- a/test/towerops/workers/preseem_sync_worker_test.exs +++ b/test/towerops/workers/preseem_sync_worker_test.exs @@ -52,7 +52,7 @@ defmodule Towerops.Workers.PreseemSyncWorkerTest do test "skips integrations synced recently", %{org: org} do _integration = integration_fixture(org.id, %{ - last_synced_at: DateTime.truncate(DateTime.utc_now(), :second), + last_synced_at: Towerops.Time.now(), last_sync_status: "success" }) diff --git a/test/towerops_web/controllers/api/v1/alerts_controller_test.exs b/test/towerops_web/controllers/api/v1/alerts_controller_test.exs index 28ff049c..c29b51f6 100644 --- a/test/towerops_web/controllers/api/v1/alerts_controller_test.exs +++ b/test/towerops_web/controllers/api/v1/alerts_controller_test.exs @@ -36,7 +36,7 @@ defmodule ToweropsWeb.Api.V1.AlertsControllerTest do device_id: device.id, alert_type: "ping_down", message: "Device unreachable", - triggered_at: DateTime.truncate(DateTime.utc_now(), :second) + triggered_at: Towerops.Time.now() } {:ok, alert} = Alerts.create_alert(Map.merge(default_attrs, attrs)) diff --git a/test/towerops_web/controllers/api/v1/pagerduty_webhook_controller_test.exs b/test/towerops_web/controllers/api/v1/pagerduty_webhook_controller_test.exs index c9ce65e3..63ffbdd6 100644 --- a/test/towerops_web/controllers/api/v1/pagerduty_webhook_controller_test.exs +++ b/test/towerops_web/controllers/api/v1/pagerduty_webhook_controller_test.exs @@ -164,7 +164,7 @@ defmodule ToweropsWeb.Api.V1.PagerdutyWebhookControllerTest do title: "Test Alert", severity: 1, alert_type: "device_down", - triggered_at: DateTime.truncate(DateTime.utc_now(), :second), + triggered_at: Towerops.Time.now(), device_id: device.id, organization_id: org.id }) @@ -224,7 +224,7 @@ defmodule ToweropsWeb.Api.V1.PagerdutyWebhookControllerTest do title: "Test Alert", severity: 2, alert_type: "device_down", - triggered_at: DateTime.truncate(DateTime.utc_now(), :second), + triggered_at: Towerops.Time.now(), device_id: device.id, organization_id: org.id }) diff --git a/test/towerops_web/controllers/invitation_controller_test.exs b/test/towerops_web/controllers/invitation_controller_test.exs index 3b53f937..41ceb812 100644 --- a/test/towerops_web/controllers/invitation_controller_test.exs +++ b/test/towerops_web/controllers/invitation_controller_test.exs @@ -83,7 +83,7 @@ defmodule ToweropsWeb.InvitationControllerTest do # Mark as accepted invitation - |> Ecto.Changeset.change(%{accepted_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{accepted_at: Towerops.Time.now()}) |> Towerops.Repo.update!() conn = get(conn, ~p"/invitations/#{invitation.token}") diff --git a/test/towerops_web/graphql/resolvers/time_series_test.exs b/test/towerops_web/graphql/resolvers/time_series_test.exs index e598c9aa..7c7801fb 100644 --- a/test/towerops_web/graphql/resolvers/time_series_test.exs +++ b/test/towerops_web/graphql/resolvers/time_series_test.exs @@ -150,7 +150,7 @@ defmodule ToweropsWeb.GraphQL.Resolvers.TimeSeriesTest do }) |> Repo.insert!() - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {:ok, _} = Snmp.create_sensor_reading(%{ @@ -292,7 +292,7 @@ defmodule ToweropsWeb.GraphQL.Resolvers.TimeSeriesTest do }) |> Repo.insert!() - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() # Two data points 60 seconds apart: # Stat 1: 1000 bytes in, 2000 bytes out @@ -359,7 +359,7 @@ defmodule ToweropsWeb.GraphQL.Resolvers.TimeSeriesTest do }) |> Repo.insert!() - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {:ok, _} = Snmp.create_interface_stat(%{ @@ -453,7 +453,7 @@ defmodule ToweropsWeb.GraphQL.Resolvers.TimeSeriesTest do config: %{"host" => "10.0.0.1"} }) - now = DateTime.truncate(DateTime.utc_now(), :second) + now = Towerops.Time.now() {:ok, _} = Monitoring.create_check_result(%{ diff --git a/test/towerops_web/live/agent_live/index_test.exs b/test/towerops_web/live/agent_live/index_test.exs index 05256fa1..6427a2f8 100644 --- a/test/towerops_web/live/agent_live/index_test.exs +++ b/test/towerops_web/live/agent_live/index_test.exs @@ -171,7 +171,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do # Update the agent to have a recent last_seen_at agent_token - |> Ecto.Changeset.change(%{last_seen_at: DateTime.truncate(DateTime.utc_now(), :second)}) + |> Ecto.Changeset.change(%{last_seen_at: Towerops.Time.now()}) |> Towerops.Repo.update!() conn = diff --git a/test/towerops_web/live/org/preseem_insights_live_test.exs b/test/towerops_web/live/org/preseem_insights_live_test.exs index 7ea224e1..b4a5d961 100644 --- a/test/towerops_web/live/org/preseem_insights_live_test.exs +++ b/test/towerops_web/live/org/preseem_insights_live_test.exs @@ -162,7 +162,7 @@ defmodule ToweropsWeb.Org.PreseemInsightsLiveTest do insert_insight!(org, ap, %{ title: "Dismissed One", status: "dismissed", - dismissed_at: DateTime.truncate(DateTime.utc_now(), :second) + dismissed_at: Towerops.Time.now() }) {:ok, _view, html} =