Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure available deployments match selected devices architecture and platform when batch-updating #1912

Merged
merged 2 commits into from
Feb 26, 2025
Merged

Ensure available deployments match selected devices architecture and platform when batch-updating #1912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75ab470
Ensure available deployments match selected devices architecture and …
nshoes Feb 20, 2025
47279db
Filter by platform first then show available deployments to bulk move…
nshoes Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/nerves_hub/deployments.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,16 @@
|> Repo.one!()
end

@spec get_by_product_and_platform(Product.t(), binary()) :: [Deployment.t()]
def get_by_product_and_platform(product, platform) do
Deployment
|> where(product_id: ^product.id)
|> join(:left, [d], f in assoc(d, :firmware))
|> where([_d, f], f.platform == ^platform)
|> preload([_d, f], firmware: f)
|> Repo.all()
end

@spec get_deployment_by_name(Product.t(), String.t()) ::
{:ok, Deployment.t()} | {:error, :not_found}
def get_deployment_by_name(product, name) do
Expand Down Expand Up @@ -389,7 +399,7 @@
end

# Check that a device version matches for a deployment's conditions
# A deployment not having a version condition returns true

Check warning on line 402 in lib/nerves_hub/deployments.ex

View workflow job for this annotation

GitHub Actions / compile-and-test 

Nested modules could be aliased at the top of the invoking module.
defp version_match?(_device, %{conditions: %{"version" => ""}}), do: true

defp version_match?(device, %{conditions: %{"version" => version}}) when not is_nil(version) do
Expand All @@ -398,7 +408,7 @@

defp version_match?(_device, _deployment), do: true

@spec verify_deployment_membership(Device.t()) :: Device.t()

Check warning on line 411 in lib/nerves_hub/deployments.ex

View workflow job for this annotation

GitHub Actions / compile-and-test 

Nested modules could be aliased at the top of the invoking module.
def verify_deployment_membership(%Device{deployment_id: deployment_id} = device)
when not is_nil(deployment_id) do
{:ok, deployment} = get_deployment_for_device(device)
Expand All @@ -407,7 +417,7 @@
bad_platform = device.firmware_metadata.platform != deployment.firmware.platform

reason =
cond do

Check warning on line 420 in lib/nerves_hub/deployments.ex

View workflow job for this annotation

GitHub Actions / compile-and-test 

Nested modules could be aliased at the top of the invoking module.
bad_architecture and bad_platform ->
"mismatched architecture and platform"

Expand Down
6 changes: 3 additions & 3 deletions lib/nerves_hub_web/live/devices/index-new.html.heex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,13 +442,13 @@
</div>
</form>

<form id="deployment-move" class="flex flex-col gap-2" phx-change="target-deployment" phx-submit="move-devices-deployment">
<label for="move_to" class="sidebar-label">Move device(s) to deployment:</label>
<form :if={Enum.any?(@available_deployments_for_filtered_platform)} id="deployment-move" class="flex flex-col gap-2" phx-change="target-deployment" phx-submit="move-devices-deployment">
<label for="move_to" class="sidebar-label">Move device(s) to deployment filtered by platform:</label>

<div class="flex gap-2">
<select name="deployment" id="move_to_deployment" class="sidebar-select">
<option value="">Select deployment</option>
<%= for deployment <- @deployments do %>
<%= for deployment <- @available_deployments_for_filtered_platform do %>
<option value={deployment.id} {if @target_deployment && @target_deployment.id == deployment.id, do: [selected: true], else: []}>
{deployment.name} - {deployment.firmware.architecture} - {deployment.firmware.platform}
</option>
Expand Down
38 changes: 32 additions & 6 deletions lib/nerves_hub_web/live/devices/index.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ defmodule NervesHubWeb.Live.Devices.Index do
use NervesHubWeb, :updated_live_view

require Logger

require OpenTelemetry.Tracer, as: Tracer

alias NervesHub.AuditLogs.DeviceTemplates
Expand Down Expand Up @@ -98,7 +97,7 @@ defmodule NervesHubWeb.Live.Devices.Index do
|> assign(:total_entries, 0)
|> assign(:current_alarms, Alarms.get_current_alarm_types(product.id))
|> assign(:metrics_keys, Metrics.default_metrics())
|> assign(:deployments, Deployments.get_deployments_by_product(product))
|> assign(:available_deployments_for_filtered_platform, [])
|> assign(:target_deployment, nil)
|> subscribe_and_refresh_device_list_timer()
|> ok()
Expand All @@ -118,6 +117,7 @@ defmodule NervesHubWeb.Live.Devices.Index do
|> assign(:currently_filtering, filters != @default_filters)
|> assign(:params, unsigned_params)
|> assign_display_devices()
|> maybe_assign_available_deployments_for_filtered_platform()
|> noreply()
end

Expand Down Expand Up @@ -222,7 +222,11 @@ defmodule NervesHubWeb.Live.Devices.Index do
[id | selected_devices]
end

{:noreply, assign(socket, :selected_devices, selected_devices)}
socket =
socket
|> assign(:selected_devices, selected_devices)

{:noreply, socket}
end

def handle_event("select-all", _, socket) do
Expand All @@ -235,11 +239,16 @@ defmodule NervesHubWeb.Live.Devices.Index do
Enum.map(socket.assigns.devices, & &1.id)
end

{:noreply, assign(socket, :selected_devices, selected_devices)}
socket =
socket
|> assign(:selected_devices, selected_devices)

{:noreply, socket}
end

def handle_event("deselect-all", _, socket) do
{:noreply, assign(socket, selected_devices: [])}
{:noreply,
assign(socket, %{selected_devices: [], available_deployments_for_filtered_platform: []})}
end

def handle_event("validate-tags", %{"tags" => tags}, socket) do
Expand Down Expand Up @@ -289,7 +298,10 @@ defmodule NervesHubWeb.Live.Devices.Index do

def handle_event("target-deployment", %{"deployment" => deployment_id}, socket) do
deployment =
Enum.find(socket.assigns.deployments, &(&1.id == String.to_integer(deployment_id)))
Enum.find(
socket.assigns.available_deployments_for_filtered_platform,
&(&1.id == String.to_integer(deployment_id))
)

{:noreply, assign(socket, target_deployment: deployment)}
end
Expand Down Expand Up @@ -702,4 +714,18 @@ defmodule NervesHubWeb.Live.Devices.Index do
background-size: #{progress}% 1px, #{progress * 1.1}% 100%;
"""
end

defp maybe_assign_available_deployments_for_filtered_platform(
%{assigns: %{product: product, current_filters: %{platform: platform}}} = socket
)
when platform != "" do
assign(
socket,
:available_deployments_for_filtered_platform,
Deployments.get_by_product_and_platform(product, platform)
)
end

defp maybe_assign_available_deployments_for_filtered_platform(socket),
do: assign(socket, :available_deployments_for_filtered_platform, [])
end
4 changes: 2 additions & 2 deletions lib/nerves_hub_web/live/devices/index.html.heex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,13 +234,13 @@
<div class={if @valid_tags, do: "hidden"}><span class="has-error"> Tags Cannot Contain Spaces </span></div>
</form>

<form id="move-deployment" class="col-lg-4" phx-change="target-deployment" phx-submit="move-devices-deployment">
<form :if={Enum.any?(@available_deployments_for_filtered_platform)} id="move-deployment" class="col-lg-4" phx-change="target-deployment" phx-submit="move-devices-deployment">
<label for="move_to_deployment">Move device(s) to deployment:</label>
<div class="flex-row align-items-center">
<div class="flex-grow pos-rel">
<select name="deployment" id="move_to_deployment" class="form-control">
<option value=""></option>
<%= for deployment <- @deployments do %>
<%= for deployment <- @available_deployments_for_filtered_platform do %>
<option value={deployment.id} {if @target_deployment && @target_deployment.id == deployment.id, do: [selected: true], else: []}>{deployment.name}</option>
<% end %>
</select>
Expand Down
79 changes: 3 additions & 76 deletions test/nerves_hub_web/live/devices/index_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do
use NervesHubWeb.ConnCase.Browser, async: false

alias NervesHub.Devices
alias NervesHub.Firmwares.FirmwareMetadata
alias NervesHub.Fixtures

alias NervesHub.Repo
Expand Down Expand Up @@ -300,7 +299,9 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do
refute device2.deployment_id

conn
|> visit("/org/#{org.name}/#{product.name}/devices")
|> visit(
"/org/#{org.name}/#{product.name}/devices?platform=#{deployment.firmware.platform}"
)
|> unwrap(fn view ->
render_change(view, "select-all", %{"id" => device.id})
end)
Expand All @@ -317,80 +318,6 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do
assert Repo.reload(device) |> Map.get(:deployment_id)
assert Repo.reload(device2) |> Map.get(:deployment_id)
end

test "selecting multiple devices to add to deployment but some don't match firmware requirements",
%{conn: conn, fixture: fixture} do
%{
device: device,
org: org,
product: product,
firmware: firmware,
deployment: deployment
} = fixture

device2 = Fixtures.device_fixture(org, product, firmware)

different_firmware_params =
%FirmwareMetadata{device2.firmware_metadata | platform: "foo"} |> Map.from_struct()

{:ok, device2} = Devices.update_firmware_metadata(device2, different_firmware_params)

refute device.deployment_id
refute device2.deployment_id

conn
|> visit("/org/#{org.name}/#{product.name}/devices")
|> unwrap(fn view ->
render_change(view, "select-all", %{"id" => device.id})
end)
|> assert_has("span", text: "2 selected")
|> unwrap(fn view ->
render_change(view, "target-deployment", %{"deployment" => to_string(deployment.id)})
end)
|> click_button("#move-deployment-submit", "Move")
|> assert_has("div", text: "1 device added to deployment")
|> assert_has("div", text: "1 device could not be added")

assert Repo.reload(device) |> Map.get(:deployment_id)
refute Repo.reload(device2) |> Map.get(:deployment_id)
end

test "selecting multiple devices to add to deployment but none match firmware requirements",
%{conn: conn, fixture: fixture} do
%{
device: device,
org: org,
product: product,
firmware: firmware,
deployment: deployment
} = fixture

device2 = Fixtures.device_fixture(org, product, firmware)

different_firmware_params =
%FirmwareMetadata{device2.firmware_metadata | platform: "foo"} |> Map.from_struct()

{:ok, device} = Devices.update_firmware_metadata(device, different_firmware_params)
{:ok, device2} = Devices.update_firmware_metadata(device2, different_firmware_params)

refute device.deployment_id
refute device2.deployment_id

conn
|> visit("/org/#{org.name}/#{product.name}/devices")
|> unwrap(fn view ->
render_change(view, "select-all", %{"id" => device.id})
end)
|> assert_has("span", text: "2 selected")
|> unwrap(fn view ->
render_change(view, "target-deployment", %{"deployment" => to_string(deployment.id)})
end)
|> click_button("#move-deployment-submit", "Move")
|> assert_has("div", text: "No devices selected could be added to deployment")

refute Repo.reload(device) |> Map.get(:deployment_id)
refute Repo.reload(device2) |> Map.get(:deployment_id)
end
end

def device_index_path(%{org: org, product: product}) do
Expand Down
Loading