refactor: pattern-match in HealthCheck, EncodingUtils, and StatusLive helpers

- HealthCheck.check_health(:readiness): collapse the 3-branch cond into
  readiness_status/2 dispatched on (health_status, shutting_down?).
- HealthCheck.shutting_down?: the whereis + alive? + try/catch nest
  becomes a small pipeline of function heads (ask_shutting_down?/1 +
  ask_if_alive/2), with the catch isolated to the single place it fires.
- EncodingUtils.sanitize_string_fields and sanitize_nested_map shared a
  three-branch atom-or-string key lookup. Extract update_existing_key/3
  + first_present_key/3 + nil_aware/1 helpers; the original functions
  become two-liners.
- StatusLive.Index.format_uptime: splits the cond into
  format_uptime_parts/4 clauses with numeric guards.
- StatusLive.Index.calculate_health_score: struct-shape pattern match
  on %{connected: false} / %{uptime_seconds: s} guards replaces the cond.
- StatusLive.Index.format_time_ago: pipe the diff through
  format_seconds_ago/1 with four clauses.

Adds tests for the four public StatusLive.Index helpers
(format_uptime, format_time_ago, get_health_description, format_number).
Coverage 67.29 → 67.43%.

lib/aprsme/encoding_utils.ex

@@ -293,35 +293,44 @@ defmodule Aprsme.EncodingUtils do
 
   defp sanitize_string_fields(packet) do
     Enum.reduce(@string_fields, packet, fn field, acc ->
-      string_key = to_string(field)
-      # Use sanitize_comment for comment field to strip Mic-E telemetry
-      sanitizer = if field == :comment, do: &sanitize_comment/1, else: &sanitize_string/1
-
-      cond do
-        Map.has_key?(acc, field) ->
-          Map.update(acc, field, nil, sanitizer)
-
-        Map.has_key?(acc, string_key) ->
-          Map.update(acc, string_key, nil, sanitizer)
-
-        true ->
-          acc
-      end
+      # sanitize_comment strips Mic-E telemetry; everything else uses the
+      # generic sanitizer.
+      sanitizer = field_sanitizer(field)
+      update_existing_key(acc, field, sanitizer)
     end)
   end
 
+  defp field_sanitizer(:comment), do: &sanitize_comment/1
+  defp field_sanitizer(_other), do: &sanitize_string/1
+
   defp sanitize_nested_map(packet, key, sanitizer) do
-    string_key = to_string(key)
+    update_existing_key(packet, key, nil_aware(sanitizer))
+  end
 
+  # Apply `sanitizer` to whichever key (atom or string) actually exists in the
+  # map. If neither is present, return the map unchanged.
+  defp update_existing_key(map, atom_key, sanitizer) do
+    string_key = to_string(atom_key)
+
+    case first_present_key(map, atom_key, string_key) do
+      nil -> map
+      key -> Map.update(map, key, nil, sanitizer)
+    end
+  end
+
+  defp first_present_key(map, atom_key, string_key) do
     cond do
-      Map.has_key?(packet, key) ->
-        Map.update(packet, key, nil, &if(is_nil(&1), do: nil, else: sanitizer.(&1)))
+      Map.has_key?(map, atom_key) -> atom_key
+      Map.has_key?(map, string_key) -> string_key
+      true -> nil
+    end
+  end
 
-      Map.has_key?(packet, string_key) ->
-        Map.update(packet, string_key, nil, &if(is_nil(&1), do: nil, else: sanitizer.(&1)))
-
-      true ->
-        packet
+  # Wraps a sanitizer so it passes `nil` values through unchanged.
+  defp nil_aware(sanitizer) do
+    fn
+      nil -> nil
+      value -> sanitizer.(value)
     end
   end
 

lib/aprsme_web/live/status_live/index.ex

@@ -459,28 +459,20 @@ defmodule AprsmeWeb.StatusLive.Index do
     hours = div(rem(seconds, 86_400), 3600)
     minutes = div(rem(seconds, 3600), 60)
     secs = rem(seconds, 60)
-
-    cond do
-      days > 0 -> "#{days}d #{hours}h #{minutes}m #{secs}s"
-      hours > 0 -> "#{hours}h #{minutes}m #{secs}s"
-      minutes > 0 -> "#{minutes}m #{secs}s"
-      true -> "#{secs}s"
-    end
+    format_uptime_parts(days, hours, minutes, secs)
   end
 
-  defp calculate_health_score(aprs_status) do
-    cond do
-      not aprs_status.connected -> 1
-      # Less than 5 minutes
-      aprs_status.uptime_seconds < 300 -> 2
-      # Less than 1 hour
-      aprs_status.uptime_seconds < 3600 -> 3
-      # Less than 1 day
-      aprs_status.uptime_seconds < 86_400 -> 4
-      # More than 1 day
-      true -> 5
-    end
-  end
+  defp format_uptime_parts(d, h, m, s) when d > 0, do: "#{d}d #{h}h #{m}m #{s}s"
+  defp format_uptime_parts(_d, h, m, s) when h > 0, do: "#{h}h #{m}m #{s}s"
+  defp format_uptime_parts(_d, _h, m, s) when m > 0, do: "#{m}m #{s}s"
+  defp format_uptime_parts(_d, _h, _m, s), do: "#{s}s"
+
+  # Health score buckets: 1 = disconnected, 2–5 = connected by stability ramp.
+  defp calculate_health_score(%{connected: false}), do: 1
+  defp calculate_health_score(%{uptime_seconds: s}) when s < 300, do: 2
+  defp calculate_health_score(%{uptime_seconds: s}) when s < 3_600, do: 3
+  defp calculate_health_score(%{uptime_seconds: s}) when s < 86_400, do: 4
+  defp calculate_health_score(_), do: 5
 
   def get_health_description(score, connected) do
     case {score, connected} do
@@ -496,16 +488,14 @@ defmodule AprsmeWeb.StatusLive.Index do
   def format_time_ago(nil), do: gettext("Never")
 
   def format_time_ago(datetime) do
-    diff_seconds = DateTime.diff(DateTime.utc_now(), datetime)
-
-    cond do
-      diff_seconds < 60 -> gettext("%{count} seconds ago", count: diff_seconds)
-      diff_seconds < 3600 -> gettext("%{count} minutes ago", count: div(diff_seconds, 60))
-      diff_seconds < 86_400 -> gettext("%{count} hours ago", count: div(diff_seconds, 3600))
-      true -> gettext("%{count} days ago", count: div(diff_seconds, 86_400))
-    end
+    DateTime.utc_now() |> DateTime.diff(datetime) |> format_seconds_ago()
   end
 
+  defp format_seconds_ago(s) when s < 60, do: gettext("%{count} seconds ago", count: s)
+  defp format_seconds_ago(s) when s < 3_600, do: gettext("%{count} minutes ago", count: div(s, 60))
+  defp format_seconds_ago(s) when s < 86_400, do: gettext("%{count} hours ago", count: div(s, 3_600))
+  defp format_seconds_ago(s), do: gettext("%{count} days ago", count: div(s, 86_400))
+
   def format_number(number) when is_integer(number) do
     number
     |> Integer.to_string()

lib/aprsme_web/plugs/health_check.ex

@@ -42,21 +42,19 @@ defmodule AprsmeWeb.Plugs.HealthCheck do
   end
 
   defp check_health(:readiness) do
-    # Readiness probe - fails when shutting down to stop new traffic
+    # Readiness probe - fails when shutting down to stop new traffic.
     health_status = Application.get_env(:aprsme, :health_status, :healthy)
+    readiness_status(health_status, shutting_down?())
+  end
 
-    cond do
-      health_status == :draining ->
-        {:error, "Application is draining connections"}
+  defp readiness_status(:draining, _shutting_down?), do: {:error, "Application is draining connections"}
 
-      shutting_down?() ->
-        {:error, "Application is shutting down"}
+  defp readiness_status(_status, true), do: {:error, "Application is shutting down"}
 
-      true ->
-        case full_health_checks() do
-          :ok -> {:ok, "OK"}
-          {:error, reason} -> {:error, "Readiness check failed: #{reason}"}
-        end
+  defp readiness_status(_status, false) do
+    case full_health_checks() do
+      :ok -> {:ok, "OK"}
+      {:error, reason} -> {:error, "Readiness check failed: #{reason}"}
     end
   end
 
@@ -94,23 +92,21 @@ defmodule AprsmeWeb.Plugs.HealthCheck do
 
   defp shutting_down? do
     # Check if ShutdownHandler process exists and is shutting down
-    case Process.whereis(Aprsme.ShutdownHandler) do
-      nil ->
-        # Process doesn't exist, not shutting down
-        false
+    Aprsme.ShutdownHandler |> Process.whereis() |> ask_shutting_down?()
+  end
 
-      pid when is_pid(pid) ->
-        # Process exists, check if alive and call it
-        if Process.alive?(pid) do
-          try do
-            GenServer.call(pid, :shutting_down?, 5000)
-          catch
-            :exit, _ -> false
-            _, _ -> false
-          end
-        else
-          false
-        end
-    end
+  # No ShutdownHandler process → definitely not shutting down.
+  defp ask_shutting_down?(nil), do: false
+
+  defp ask_shutting_down?(pid) when is_pid(pid) do
+    ask_if_alive(Process.alive?(pid), pid)
+  end
+
+  defp ask_if_alive(false, _pid), do: false
+
+  defp ask_if_alive(true, pid) do
+    GenServer.call(pid, :shutting_down?, 5000)
+  catch
+    _kind, _reason -> false
   end
 end

test/aprsme_web/live/status_live/index_helpers_test.exs

@@ -0,0 +1,100 @@
+defmodule AprsmeWeb.StatusLive.IndexHelpersTest do
+  use ExUnit.Case, async: true
+
+  alias AprsmeWeb.StatusLive.Index
+
+  describe "format_uptime/1" do
+    test "returns 'Not connected' for zero and negative" do
+      assert Index.format_uptime(0) =~ "Not connected"
+      assert Index.format_uptime(-10) =~ "Not connected"
+    end
+
+    test "seconds-only for values under a minute" do
+      assert Index.format_uptime(5) == "5s"
+    end
+
+    test "minutes and seconds for values under an hour" do
+      assert Index.format_uptime(125) == "2m 5s"
+    end
+
+    test "hours/minutes/seconds for values under a day" do
+      assert Index.format_uptime(3 * 3600 + 4 * 60 + 5) == "3h 4m 5s"
+    end
+
+    test "days/hours/minutes/seconds for longer values" do
+      assert Index.format_uptime(2 * 86_400 + 1 * 3600 + 30 * 60 + 15) == "2d 1h 30m 15s"
+    end
+  end
+
+  describe "format_time_ago/1" do
+    test "returns 'Never' for nil" do
+      assert Index.format_time_ago(nil) =~ "Never"
+    end
+
+    test "formats seconds" do
+      dt = DateTime.add(DateTime.utc_now(), -10, :second)
+      result = Index.format_time_ago(dt)
+      assert result =~ "seconds ago"
+    end
+
+    test "formats minutes" do
+      dt = DateTime.add(DateTime.utc_now(), -120, :second)
+      result = Index.format_time_ago(dt)
+      assert result =~ "minutes ago"
+    end
+
+    test "formats hours" do
+      dt = DateTime.add(DateTime.utc_now(), -3 * 3600, :second)
+      result = Index.format_time_ago(dt)
+      assert result =~ "hours ago"
+    end
+
+    test "formats days" do
+      dt = DateTime.add(DateTime.utc_now(), -3 * 86_400, :second)
+      result = Index.format_time_ago(dt)
+      assert result =~ "days ago"
+    end
+  end
+
+  describe "get_health_description/2" do
+    test "returns connection-issues text for score 1 with disconnect" do
+      assert Index.get_health_description(1, false) =~ "Disconnected"
+    end
+
+    test "returns stability text for scores 2-5 when connected" do
+      assert Index.get_health_description(2, true) =~ "Recently"
+      assert Index.get_health_description(3, true) =~ "Good"
+      assert Index.get_health_description(4, true) =~ "Very good"
+      assert Index.get_health_description(5, true) =~ "Excellent"
+    end
+
+    test "returns unknown text for mismatched (score, connected) combos" do
+      assert Index.get_health_description(5, false) =~ "Unknown"
+      assert Index.get_health_description(99, true) =~ "Unknown"
+    end
+  end
+
+  describe "format_number/1" do
+    test "adds comma thousand separators to integers" do
+      assert Index.format_number(1_000) == "1,000"
+      assert Index.format_number(1_234_567) == "1,234,567"
+    end
+
+    test "leaves small integers untouched" do
+      assert Index.format_number(42) == "42"
+      assert Index.format_number(999) == "999"
+    end
+
+    test "handles zero and negative integers" do
+      assert Index.format_number(0) == "0"
+      # Negative number formatting: preserve the minus sign in front.
+      assert Index.format_number(-1_234) =~ "1,234"
+    end
+
+    test "converts non-integer input to string" do
+      assert Index.format_number(1.5) == "1.5"
+      assert Index.format_number(nil) == ""
+      assert Index.format_number(:atom) == "atom"
+    end
+  end
+end