Fix deleting and destroying devices in new UI (#2150)

This PR:

- Allows devices to be deleted and destroyed in the new UI
- Update device settings footer when device is soft-deleted
- Rejects update attempts through `Device.changeset/2` when soft-deleted
- New UI tests
- Tests for device API:
  - Soft-deleted devices don't show up in `index`
  - Soft-deleted devices can be queried for by `identifier`

Solves #2214.

---------

Co-authored-by: Josh Kalderimis <[email protected]>

Author: nshoes

lib/nerves_hub/devices.ex

@@ -130,7 +130,6 @@ defmodule NervesHub.Devices do
   def filter(product, user, opts) do
     base_query =
       Device
-      |> Repo.exclude_deleted()
       |> join(:left, [d], dc in assoc(d, :latest_connection), as: :latest_connection)
       |> join(:left, [d, dc], dh in assoc(d, :latest_health), as: :latest_health)
       |> join(:left, [d, dc, dh], pd in PinnedDevice,
@@ -233,7 +232,6 @@ defmodule NervesHub.Devices do
   def get_device_by_identifier!(org, identifier, preload_assoc \\ nil)
       when is_binary(identifier) do
     get_device_by_identifier_query(org, identifier, preload_assoc)
-    |> Repo.exclude_deleted()
     |> Repo.one!()
   end
 
@@ -1809,6 +1807,15 @@ defmodule NervesHub.Devices do
     Firmwares.get_firmware_url(target)
   end
 
+  @spec soft_deleted_devices_exist_for_product?(non_neg_integer()) :: boolean()
+  def soft_deleted_devices_exist_for_product?(product_id) do
+    from(d in Device,
+      where: d.product_id == ^product_id,
+      where: not is_nil(d.deleted_at)
+    )
+    |> Repo.exists?()
+  end
+
   defp update_tool() do
     Application.get_env(
       :nerves_hub,

lib/nerves_hub/devices/device.ex

@@ -97,5 +97,12 @@ defmodule NervesHub.Devices.Device do
     |> validate_required(@required_params)
     |> validate_length(:tags, min: 1)
     |> unique_constraint(:identifier)
+    |> then(fn changeset ->
+      if device.deleted_at && !Map.has_key?(changeset.changes, :deleted_at) do
+        add_error(changeset, :deleted_at, "cannot update while marked as deleted")
+      else
+        changeset
+      end
+    end)
   end
 end

lib/nerves_hub/devices/device_filtering.ex

@@ -115,13 +115,14 @@ defmodule NervesHub.Devices.DeviceFiltering do
     filter_on_metric(query, filters)
   end
 
-  def filter(query, _filters, :is_pinned, value) do
-    if value do
-      where(query, [pinned: pd], not is_nil(pd))
-    else
-      query
-    end
-  end
+  def filter(query, _filters, :display_deleted, "include"),
+    do: order_by(query, [d], asc_nulls_last: d.deleted_at)
+
+  def filter(query, _filters, :display_deleted, "exclude"),
+    do: where(query, [d], is_nil(d.deleted_at))
+
+  def filter(query, _filters, :display_deleted, "only"),
+    do: where(query, [d], not is_nil(d.deleted_at))
 
   def filter(query, _filters, :search, value) when is_binary(value) and value != "" do
     search_term = "%#{value}%"

lib/nerves_hub_web/components/device_page/details_tab.ex

@@ -749,8 +749,15 @@ defmodule NervesHubWeb.Components.DevicePage.DetailsTab do
       {:error, changeset} ->
         Logger.info("Couldn't update device.priority_updates: #{inspect(changeset)}")
 
+        error =
+          if Keyword.has_key?(changeset.errors, :deleted_at) do
+            "Device cannot be updated because it has been deleted. Please restore the device to make changes."
+          else
+            "There was an issue updating the device. Please contact support if this happens again."
+          end
+
         socket
-        |> put_flash(:error, "There was an issue updating the device, please check the logs.")
+        |> put_flash(:error, error)
         |> halt()
     end
   end

lib/nerves_hub_web/components/device_page/settings_tab.ex

@@ -210,30 +210,10 @@ defmodule NervesHubWeb.Components.DevicePage.SettingsTab do
       </div>
 
       <div :if={@device.deleted_at && authorized?(:"device:update", @org_user)} class="flex flex-col w-full bg-zinc-900 border border-zinc-700 rounded">
-        <div class="flex items-center p-6 gap-6 border-t border-zinc-700">
-          <div>
-            <button class="flex px-3 py-1.5 gap-2 rounded bg-zinc-800 border border-zinc-600" type="button" phx-click="restore-device" data-confirm="Are you sure you want to restore this device?">
-              <svg class="size-5" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
-                <path
-                  d="M8.5 18.9999H5.39903C3.87406 18.9999 2.91012 17.3617 3.65071 16.0287L7 9.99994M7 9.99994L3 11.9999M7 9.99994L8 13.9999M18.9999 13.9999L20.1987 16.0122C20.9929 17.3454 20.0323 19.0358 18.4805 19.0358L12.768 19.0358M12.768 19.0358L16 21.9999M12.768 19.0358L16 15.9999M8.5 6.99994L10.5883 3.86749C11.4401 2.58975 13.3545 2.70894 14.0413 4.08246L17 9.99994M17 9.99994L18 5.99994M17 9.99994L13 8.99994"
-                  stroke="#A1A1AA"
-                  stroke-width="1.2"
-                  stroke-linecap="round"
-                  stroke-linejoin="round"
-                />
-              </svg>
-
-              <span class="text-sm font-medium text-zinc-300">Restore device</span>
-            </button>
-          </div>
-          <div class="text-zinc-300">
-            The device has been disabled. Attempts to connect to NervesHub will be blocked.
-          </div>
+        <div class="text-zinc-300 p-6 pb-0">
+          The device has been disabled. Attempts to connect to NervesHub will be blocked.
         </div>
-      </div>
-
-      <div :if={@device.deleted_at && authorized?(:"device:update", @org_user)} class="flex flex-col w-full bg-zinc-900 border border-zinc-700 rounded">
-        <div class="flex items-center p-6 gap-6 border-t border-zinc-700">
+        <div class="flex items-center p-6 gap-6 border-zinc-700">
           <div>
             <button
               class="flex px-3 py-1.5 gap-2 rounded bg-zinc-800 border border-red-500"
@@ -253,6 +233,22 @@ defmodule NervesHubWeb.Components.DevicePage.SettingsTab do
               <span class="text-sm font-medium text-red-500">Permanently delete device</span>
             </button>
           </div>
+          <div class="text-zinc-300">or</div>
+          <div>
+            <button class="flex px-3 py-1.5 gap-2 rounded bg-zinc-800 border border-zinc-600" type="button" phx-click="restore-device" data-confirm="Are you sure you want to restore this device?">
+              <svg class="size-5" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
+                <path
+                  d="M8.5 18.9999H5.39903C3.87406 18.9999 2.91012 17.3617 3.65071 16.0287L7 9.99994M7 9.99994L3 11.9999M7 9.99994L8 13.9999M18.9999 13.9999L20.1987 16.0122C20.9929 17.3454 20.0323 19.0358 18.4805 19.0358L12.768 19.0358M12.768 19.0358L16 21.9999M12.768 19.0358L16 15.9999M8.5 6.99994L10.5883 3.86749C11.4401 2.58975 13.3545 2.70894 14.0413 4.08246L17 9.99994M17 9.99994L18 5.99994M17 9.99994L13 8.99994"
+                  stroke="#A1A1AA"
+                  stroke-width="1.2"
+                  stroke-linecap="round"
+                  stroke-linejoin="round"
+                />
+              </svg>
+
+              <span class="text-sm font-medium text-zinc-300">Restore device</span>
+            </button>
+          </div>
         </div>
       </div>
 
@@ -300,8 +296,15 @@ defmodule NervesHubWeb.Components.DevicePage.SettingsTab do
         |> halt()
 
       {:error, :update_with_audit, changeset, _} ->
+        error =
+          if Keyword.has_key?(changeset.errors, :deleted_at) do
+            "Device cannot be updated because it has been deleted. Please restore the device to make changes."
+          else
+            "We couldn't save your changes. Please contact support if this happens again."
+          end
+
         socket
-        |> put_flash(:error, "We couldn't save your changes.")
+        |> put_flash(:error, error)
         |> assign(:settings_form, to_form(changeset))
         |> halt()
 

lib/nerves_hub_web/components/filter_sidebar.ex

@@ -56,7 +56,7 @@ defmodule NervesHubWeb.Components.FilterSidebar do
                     <input class="sidebar-text-input" type="text" name={filter.attr} id={"input_#{filter.attr}"} value={@current_filters[filter.attr]} phx-debounce="500" />
                   <% :select -> %>
                     <select class="sidebar-select" name={filter.attr} id={"input_#{filter.attr}"}>
-                      <option :for={{label, value} <- filter.values} value={value} selected={@current_filters[filter.attr] == value}>
+                      <option :for={{label, value} <- filter.values} value={value} selected={to_string(@current_filters[filter.attr]) == value}>
                         {label}
                       </option>
                     </select>

lib/nerves_hub_web/controllers/api/device_json.ex

@@ -37,7 +37,8 @@ defmodule NervesHubWeb.API.DeviceJSON do
       product_name: device.product.name,
       # deprecated
       last_communication: connection_last_seen_at(device),
-      priority_updates: device.priority_updates
+      priority_updates: device.priority_updates,
+      deleted: deleted(device)
     }
   end
 
@@ -62,4 +63,7 @@ defmodule NervesHubWeb.API.DeviceJSON do
 
   defp connection_status(%{latest_connection: %{status: status}}), do: status
   defp connection_status(_), do: :not_seen
+
+  defp deleted(%{deleted_at: nil}), do: false
+  defp deleted(%{deleted_at: _}), do: true
 end

lib/nerves_hub_web/live/devices/index-new.html.heex

@@ -54,7 +54,7 @@
     <.button type="link" navigate={~p"/org/#{@org}/#{@product}/devices/new"} aria-label="Add new device">
       <.icon name="add" /> Add Device
     </.button>
-    <.button :if={has_results?(@devices, @currently_filtering)} type="button" phx-click="toggle-filters" phx-value-toggle={to_string(@show_filters)}>
+    <.button :if={has_results?(@devices, @currently_filtering) || @soft_deleted_devices_exist} type="button" phx-click="toggle-filters" phx-value-toggle={to_string(@show_filters)}>
       <.icon name="filter" /> Filters
     </.button>
   </div>
@@ -224,6 +224,13 @@
                   </span>
                   <.link navigate={~p"/org/#{@org}/#{@product}/devices/#{device}"} class="ff-m font-mono">
                     {device.identifier}
+                    <div :if={device.deleted_at} class="relative inline-block align-middle z-50" id={"health-tooltip-#{device.identifier}"} phx-hook="ToolTip" data-placement="right">
+                      <.icon name="info" class="stroke-zinc-400" />
+                      <div class="tooltip-content hidden w-max absolute top-0 left-0 z-20 text-xs px-2 py-1.5 rounded border border-[#3F3F46] bg-base-900 flex">
+                        This device has been deleted. <br /> You can destroy or restore it in the device's settings.
+                        <div class="tooltip-arrow absolute w-2 h-2 border-[#3F3F46] bg-base-900 origin-center rotate-45"></div>
+                      </div>
+                    </div>
                     <span class="clickable-table-row-mask" />
                   </.link>
                   <span class={["flex items-center gap-1 ml-2 pl-2.5 pr-2.5 py-0.5 border border-zinc-700 rounded-full bg-zinc-800", !@progress[device.id] && "invisible"]}>
@@ -391,6 +398,16 @@
   <:filter attr="metrics_operator" label="Metrics Operator" type={:select} values={[{"Greater Than", "gt"}, {"Less Than", "lt"}]} />
   <:filter attr="metrics_value" label="Metrics Value" type={:number} />
   <:filter attr="is_pinned" label="Pinned" type={:select} values={[{"All", "false"}, {"Only pinned devices", "true"}]} />
+  <:filter
+    attr="display_deleted"
+    label="Include Deleted Devices?"
+    type={:select}
+    values={[
+      {"Yes", "include"},
+      {"No", "exclude"},
+      {"Only deleted devices", "only"}
+    ]}
+  />
 </FilterSidebar.render>
 
 <div class="pointer-events-none fixed inset-y-0 right-0 flex max-w-full pl-10 mb-[119px] sm:pl-16 z-40">

lib/nerves_hub_web/live/devices/index.ex

@@ -46,7 +46,8 @@ defmodule NervesHubWeb.Live.Devices.Index do
     metrics_value: "",
     deployment_id: "",
     is_pinned: false,
-    search: ""
+    search: "",
+    display_deleted: "exclude"
   }
 
   @filter_types %{
@@ -67,7 +68,8 @@ defmodule NervesHubWeb.Live.Devices.Index do
     metrics_value: :string,
     deployment_id: :string,
     is_pinned: :boolean,
-    search: :string
+    search: :string,
+    display_deleted: :string
   }
 
   @default_page 1
@@ -110,6 +112,10 @@ defmodule NervesHubWeb.Live.Devices.Index do
     |> assign(:deployment_groups, ManagedDeployments.get_deployment_groups_by_product(product))
     |> assign(:available_deployment_groups_for_filtered_platform, [])
     |> assign(:target_deployment_group, nil)
+    |> assign(
+      :soft_deleted_devices_exist,
+      Devices.soft_deleted_devices_exist_for_product?(product.id)
+    )
     |> subscribe_and_refresh_device_list_timer()
     |> ok()
   end

test/nerves_hub_web/controllers/api/device_controller_test.exs

@@ -51,6 +51,20 @@ defmodule NervesHubWeb.API.DeviceControllerTest do
                device.identifier == identifier
              end)
     end
+
+    test "does not return soft-deleted devices", %{conn: conn, user: user, org: org} do
+      product = Fixtures.product_fixture(user, org)
+      org_key = Fixtures.org_key_fixture(org, user)
+      firmware = Fixtures.firmware_fixture(org_key, product)
+
+      {:ok, _device} =
+        Fixtures.device_fixture(org, product, firmware)
+        |> Devices.update_device(%{deleted_at: DateTime.utc_now()})
+
+      conn = get(conn, Routes.api_device_path(conn, :index, org.name, product.name))
+
+      assert json_response(conn, 200)["data"] == []
+    end
   end
 
   describe "show" do
@@ -111,6 +125,55 @@ defmodule NervesHubWeb.API.DeviceControllerTest do
       end)
       |> assert_authorization_error(404)
     end
+
+    test "deleted device can be queried", %{
+      conn: conn,
+      user: user,
+      org: org
+    } do
+      product = Fixtures.product_fixture(user, org)
+      org_key = Fixtures.org_key_fixture(org, user)
+      firmware = Fixtures.firmware_fixture(org_key, product)
+
+      {:ok, device} =
+        Fixtures.device_fixture(org, product, firmware)
+        |> Devices.update_device(%{deleted_at: DateTime.utc_now()})
+
+      conn =
+        get(conn, Routes.api_device_path(conn, :show, device.identifier))
+
+      assert json_response(conn, 200)["data"]
+
+      assert json_response(conn, 200)["data"]["identifier"] == device.identifier
+    end
+
+    test "device indicates if it has been deleted", %{
+      conn: conn,
+      user: user,
+      org: org
+    } do
+      product = Fixtures.product_fixture(user, org)
+      org_key = Fixtures.org_key_fixture(org, user)
+      firmware = Fixtures.firmware_fixture(org_key, product)
+
+      {:ok, device} =
+        Fixtures.device_fixture(org, product, firmware)
+        |> Devices.update_device(%{deleted_at: DateTime.utc_now()})
+
+      conn =
+        get(conn, Routes.api_device_path(conn, :show, device.identifier))
+
+      assert json_response(conn, 200)["data"]
+
+      assert json_response(conn, 200)["data"]["deleted"] == true
+
+      {:ok, device} = Devices.update_device(device, %{deleted_at: nil})
+
+      conn =
+        get(conn, Routes.api_device_path(conn, :show, device.identifier))
+
+      assert json_response(conn, 200)["data"]["deleted"] == false
+    end
   end
 
   describe "delete devices" do
@@ -131,14 +194,6 @@ defmodule NervesHubWeb.API.DeviceControllerTest do
         )
 
       assert response(conn, 204)
-
-      assert_error_sent(404, fn ->
-        get(
-          conn,
-          Routes.api_device_path(conn, :show, org.name, product.name, to_delete.identifier)
-        )
-      end)
-      |> assert_authorization_error(404)
     end
 
     test "does not have required role to delete chosen device", %{