Fix broken query + add test for metrics filtering

lib/nerves_hub/devices.ex

@@ -161,36 +161,20 @@ defmodule NervesHub.Devices do
 
   def filter_on_metric(query, key, value, operator) do
     query
-    |> where(
-      [d],
-      d.id in subquery(metrics_query(key, value, operator))
-    )
-    |> preload_metric(key)
+    |> join(:inner, [d], m in DeviceMetric, on: d.id == m.device_id)
+    |> where([_, m], m.inserted_at == subquery(latest_metric_for_key(key)))
+    |> where([d, m], m.key == ^key)
+    |> gt_or_lt(value, operator)
   end
 
-  def metrics_query(key, value, operator) do
+  defp latest_metric_for_key(key) do
     DeviceMetric
-    |> from
-    |> select([d], d.device_id)
+    |> select([dm], max(dm.inserted_at))
     |> where([dm], dm.key == ^key)
-    |> gt_or_lt(value, operator)
-    |> order_by(desc: :inserted_at)
-    |> distinct(:device_id)
   end
 
-  defp gt_or_lt(query, value, :gt), do: where(query, [dm], dm.value > ^value)
-  defp gt_or_lt(query, value, :lt), do: where(query, [dm], dm.value < ^value)
-
-  defp preload_metric(query, key) do
-    preload_query =
-      DeviceMetric
-      |> distinct(:device_id)
-      |> where(key: ^key)
-      |> order_by([:device_id, desc: :inserted_at])
-
-    query
-    |> preload(device_metrics: ^preload_query)
-  end
+  defp gt_or_lt(query, value, :gt), do: where(query, [_, dm], dm.value > ^value)
+  defp gt_or_lt(query, value, :lt), do: where(query, [_, dm], dm.value < ^value)
 
   defp filtering(query, filters) do
     Enum.reduce(filters, query, fn {key, value}, query ->

lib/nerves_hub_web/live/devices/index.ex

@@ -92,8 +92,10 @@ defmodule NervesHubWeb.Live.Devices.Index do
   def handle_params(unsigned_params, _uri, socket) do
     filters =
       Map.merge(@default_filters, filter_changes(unsigned_params))
+      # Until UI selectors for metrics are implemented,
+      # params can be added to the URL and be merged with filters.
+      #  Example: ?metric=cpu_temp&metric_operator=gt&metric_value=37.0
       |> filter_on_metrics_if_provided(unsigned_params)
-      |> dbg()
 
     pagination_opts = Map.merge(socket.assigns.paginate_opts, pagination_changes(unsigned_params))
 
@@ -108,21 +110,19 @@ defmodule NervesHubWeb.Live.Devices.Index do
     |> noreply()
   end
 
-  def filter_on_metrics_if_provided(filters, %{
-        "metric" => metric_type,
-        "metric_value" => metric_value,
-        "metric_operator" => operator
-      }) do
+  defp filter_on_metrics_if_provided(filters, %{
+         "metric" => metric_type,
+         "metric_value" => metric_value,
+         "metric_operator" => operator
+       }) do
     Map.put(filters, :metrics, %{
       key: metric_type,
       value: String.to_float(metric_value),
       operator: String.to_existing_atom(operator)
     })
   end
 
-  def filter_on_metrics_if_provided(filters, _) do
-    filters
-  end
+  defp filter_on_metrics_if_provided(filters, _), do: filters
 
   defp self_path(socket, extra) do
     params = Enum.into(stringify_keys(extra), socket.assigns.params)

test/nerves_hub_web/live/devices/index_test.exs

@@ -1,6 +1,7 @@
 defmodule NervesHubWeb.Live.Devices.IndexTest do
   use NervesHubWeb.ConnCase.Browser, async: false
 
+  alias NervesHub.Devices
   alias NervesHub.Fixtures
   alias NervesHubWeb.Endpoint
 
@@ -85,6 +86,40 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do
                device2.identifier
     end
 
+    test "filters devices by metrics", %{conn: conn, fixture: fixture} do
+      %{device: device, firmware: firmware, org: org, product: product} = fixture
+
+      device2 = Fixtures.device_fixture(org, product, firmware, %{})
+      {:ok, _view, html} = live(conn, device_index_path(fixture))
+      assert html =~ device.identifier
+      assert html =~ device2.identifier
+
+      # Add metrics for device2, sleep between to secure order.
+      Devices.Metrics.save_metric(%{device_id: device2.id, key: "cpu_temp", value: 36})
+      :timer.sleep(100)
+      Devices.Metrics.save_metric(%{device_id: device2.id, key: "cpu_temp", value: 42})
+      :timer.sleep(100)
+      Devices.Metrics.save_metric(%{device_id: device2.id, key: "load_1min", value: 3})
+
+      greater_than_filter =
+        device_index_path(fixture) <> "?metric=cpu_temp&metric_operator=gt&metric_value=37.0"
+
+      {:ok, _view, html} = live(conn, greater_than_filter)
+
+      # Show only show device2, which has a value greater than 37 on most recent cpu_temp metric.
+      assert html =~ device2.identifier
+      refute html =~ device.identifier
+
+      less_than_filter =
+        device_index_path(fixture) <> "?metric=cpu_temp&metric_operator=lt&metric_value=37.0"
+
+      {:ok, _view, html} = live(conn, less_than_filter)
+
+      # Should not show any device since the query is for values less than 37
+      refute html =~ device2.identifier
+      refute html =~ device.identifier
+    end
+
     test "filters devices by several tags", %{conn: conn, fixture: fixture} do
       %{device: _device, firmware: firmware, org: org, product: product} = fixture