Merge pull request #771 from lusergit/chore/errors-update-channel

chore(update_channel): better errors
edgehog-device-manager · Jan 13, 2025 · 44777a1 · 44777a1
2 parents 3821af7 + 5374612
commit 44777a1
Show file tree

Hide file tree

Showing 8 changed files with 134 additions and 47 deletions.
diff --git a/backend/lib/edgehog/groups/device_group/device_group.ex b/backend/lib/edgehog/groups/device_group/device_group.ex
@@ -1,7 +1,7 @@
 #
 # This file is part of Edgehog.
 #
-# Copyright 2022-2024 SECO Mind Srl
+# Copyright 2022-2025 SECO Mind Srl
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -60,6 +60,14 @@ defmodule Edgehog.Groups.DeviceGroup do
       accept [:update_channel_id]
     end
 
+    update :assign_update_channel do
+      accept [:update_channel_id]
+
+      require_atomic? false
+
+      validate Validations.UpdateChannelAbsent
+    end
+
     destroy :destroy do
       description "Deletes a device group."
       primary? true

diff --git a/backend/lib/edgehog/groups/device_group/validations/update_channel_absent.ex b/backend/lib/edgehog/groups/device_group/validations/update_channel_absent.ex
@@ -0,0 +1,38 @@
+#
+# This file is part of Edgehog.
+#
+# Copyright 2025 SECO Mind Srl
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# SPDX-License-Identifier: Apache-2.0
+#
+
+defmodule Edgehog.Groups.DeviceGroup.Validations.UpdateChannelAbsent do
+  @moduledoc false
+  use Ash.Resource.Validation
+
+  @impl Ash.Resource.Validation
+  def validate(changeset, _opts, _context) do
+    device_group = changeset.data
+
+    if device_group.update_channel_id do
+      {:error,
+       field: :update_channel_id,
+       message: "The update channel is already set for the device group \"#{device_group.name}\"",
+       short_message: "Update channel already set"}
+    else
+      :ok
+    end
+  end
+end
diff --git a/backend/lib/edgehog/update_campaigns/update_channel/changes/relate_target_groups.ex b/backend/lib/edgehog/update_campaigns/update_channel/changes/relate_target_groups.ex
@@ -1,7 +1,7 @@
 #
 # This file is part of Edgehog.
 #
-# Copyright 2024 SECO Mind Srl
+# Copyright 2024-2025 SECO Mind Srl
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -22,36 +22,13 @@ defmodule Edgehog.UpdateCampaigns.UpdateChannel.Changes.RelateTargetGroups do
   @moduledoc false
   use Ash.Resource.Change
 
-  alias Ash.Error.Changes.InvalidArgument
-  alias Edgehog.Groups.DeviceGroup
-
   @impl Ash.Resource.Change
-  def change(changeset, _opts, context) do
-    %{tenant: tenant} = context
-
+  def change(changeset, _opts, _context) do
     {:ok, target_group_ids} = Ash.Changeset.fetch_argument(changeset, :target_group_ids)
 
-    Ash.Changeset.after_action(changeset, fn _changeset, update_channel ->
-      relate_target_groups(tenant, update_channel, target_group_ids)
-    end)
-  end
-
-  defp relate_target_groups(tenant, update_channel, target_group_ids) do
-    %Ash.BulkResult{status: status, records: records} =
-      DeviceGroup
-      |> Ash.Query.filter(id in ^target_group_ids)
-      |> Ash.Query.filter(is_nil(update_channel_id))
-      |> Ash.Query.set_tenant(tenant)
-      |> Ash.bulk_update(:update_update_channel, %{update_channel_id: update_channel.id}, return_records?: true)
-
-    if status == :success and length(records) == length(target_group_ids) do
-      {:ok, update_channel}
-    else
-      {:error,
-       InvalidArgument.exception(
-         field: :target_group_ids,
-         message: "some target groups were not found or are already associated with an update channel"
-       )}
-    end
+    Ash.Changeset.manage_relationship(changeset, :target_groups, target_group_ids,
+      on_lookup: {:relate, :assign_update_channel},
+      on_no_match: :error
+    )
   end
 end
diff --git a/backend/lib/edgehog/update_campaigns/update_channel/error_handler.ex b/backend/lib/edgehog/update_campaigns/update_channel/error_handler.ex
@@ -0,0 +1,49 @@
+#
+# This file is part of Edgehog.
+#
+# Copyright 2025 SECO Mind Srl
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# SPDX-License-Identifier: Apache-2.0
+#
+
+defmodule Edgehog.UpdateCampaigns.UpdateChannel.ErrorHandler do
+  @moduledoc false
+
+  def handle_error(error, context) do
+    %{action: action} = context
+
+    case action do
+      :create -> target_ids_translation(error)
+      :update -> target_ids_translation(error)
+      _ -> error
+    end
+  end
+
+  defp target_ids_translation(error) do
+    if missing_target_ids?(error) do
+      %{
+        error
+        | fields: [:target_group_ids],
+          message: "One or more target groups could not be found"
+      }
+    else
+      error
+    end
+  end
+
+  defp missing_target_ids?(error) do
+    error[:code] == "not_found"
+  end
+end
diff --git a/backend/lib/edgehog/update_campaigns/update_channel/update_channel.ex b/backend/lib/edgehog/update_campaigns/update_channel/update_channel.ex
@@ -1,7 +1,7 @@
 #
 # This file is part of Edgehog.
 #
-# Copyright 2023-2024 SECO Mind Srl
+# Copyright 2023-2025 SECO Mind Srl
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -28,6 +28,7 @@ defmodule Edgehog.UpdateCampaigns.UpdateChannel do
 
   alias Edgehog.UpdateCampaigns.UpdateChannel.Calculations
   alias Edgehog.UpdateCampaigns.UpdateChannel.Changes
+  alias Edgehog.UpdateCampaigns.UpdateChannel.ErrorHandler
 
   resource do
     description """
@@ -40,6 +41,8 @@ defmodule Edgehog.UpdateCampaigns.UpdateChannel do
 
   graphql do
     type :update_channel
+
+    error_handler {ErrorHandler, :handle_error, []}
   end
 
   actions do

diff --git a/backend/mix.lock b/backend/mix.lock
@@ -2,8 +2,8 @@
   "absinthe": {:hex, :absinthe, "1.7.8", "43443d12ad2b4fcce60e257ac71caf3081f3d5c4ddd5eac63a02628bcaf5b556", [:mix], [{:dataloader, "~> 1.0.0 or ~> 2.0", [hex: :dataloader, repo: "hexpm", optional: true]}, {:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}, {:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}, {:opentelemetry_process_propagator, "~> 0.2.1 or ~> 0.3", [hex: :opentelemetry_process_propagator, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "c4085df201892a498384f997649aedb37a4ce8a726c170d5b5617ed3bf45d40b"},
   "absinthe_plug": {:hex, :absinthe_plug, "1.5.8", "38d230641ba9dca8f72f1fed2dfc8abd53b3907d1996363da32434ab6ee5d6ab", [:mix], [{:absinthe, "~> 1.5", [hex: :absinthe, repo: "hexpm", optional: false]}, {:plug, "~> 1.4", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "bbb04176647b735828861e7b2705465e53e2cf54ccf5a73ddd1ebd855f996e5a"},
   "absinthe_relay": {:hex, :absinthe_relay, "1.5.2", "cfb8aed70f4e4c7718d3f1c212332d2ea728f17c7fc0f68f1e461f0f5f0c4b9a", [:mix], [{:absinthe, "~> 1.5.0 or ~> 1.6.0 or ~> 1.7.0", [hex: :absinthe, repo: "hexpm", optional: false]}, {:ecto, "~> 2.0 or ~> 3.0", [hex: :ecto, repo: "hexpm", optional: true]}], "hexpm", "0587ee913afa31512e1457a5064ee88427f8fe7bcfbeeecd41c71d9cff0b62b6"},
-  "ash": {:hex, :ash, "3.4.52", "24b8410869eab84fc36a9152b5c9835e7f244eb3697e4332d84ab860171b6538", [:mix], [{:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:igniter, ">= 0.4.8 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:reactor, "~> 0.9", [hex: :reactor, repo: "hexpm", optional: false]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:spark, ">= 2.2.29 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, ">= 0.2.6 and < 1.0.0-0", [hex: :splode, repo: "hexpm", optional: false]}, {:stream_data, "~> 1.0", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "2636e940859c21394437192abb3f31543523045a47beeb3cf1c9db244fafd08c"},
-  "ash_graphql": {:hex, :ash_graphql, "1.4.7", "60fc9e756e3b8ef6b377a32d81deb53c9bbb67f3732f0b8683c9fb2f0d0827cb", [:mix], [{:absinthe, "~> 1.7", [hex: :absinthe, repo: "hexpm", optional: false]}, {:absinthe_phoenix, "~> 2.0.0", [hex: :absinthe_phoenix, repo: "hexpm", optional: true]}, {:absinthe_plug, "~> 1.4", [hex: :absinthe_plug, repo: "hexpm", optional: false]}, {:ash, ">= 3.2.3 and < 4.0.0-0", [hex: :ash, repo: "hexpm", optional: false]}, {:igniter, ">= 0.3.34 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.2", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:spark, ">= 2.2.10 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}], "hexpm", "4b9f9eaab909eabf826217f72feaf392ce955a5d8215259ebe744f3c5d0b986e"},
+  "ash": {:hex, :ash, "3.4.53", "85a4740636199a8aedf835bc69c400ca5bdc587a263a3439e97d9a82ac099467", [:mix], [{:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:igniter, ">= 0.4.8 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:reactor, "~> 0.9", [hex: :reactor, repo: "hexpm", optional: false]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:spark, ">= 2.2.29 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, ">= 0.2.6 and < 1.0.0-0", [hex: :splode, repo: "hexpm", optional: false]}, {:stream_data, "~> 1.0", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "57de328520a9af92364a4dd2b4c877182d90953ec145e9e67ff76be377961699"},
+  "ash_graphql": {:hex, :ash_graphql, "1.5.0", "29dce267c7bd86fa7c94feb5ce861674d46046e528299f095db659411f32cd24", [:mix], [{:absinthe, "~> 1.7", [hex: :absinthe, repo: "hexpm", optional: false]}, {:absinthe_phoenix, "~> 2.0.0", [hex: :absinthe_phoenix, repo: "hexpm", optional: true]}, {:absinthe_plug, "~> 1.4", [hex: :absinthe_plug, repo: "hexpm", optional: false]}, {:ash, ">= 3.2.3 and < 4.0.0-0", [hex: :ash, repo: "hexpm", optional: false]}, {:igniter, ">= 0.3.34 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.2", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:spark, ">= 2.2.10 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}], "hexpm", "7657ad63d2fba2feeac3ce9b158077e7b8cdcd823573c8888619de8ba929c188"},
   "ash_json_api": {:hex, :ash_json_api, "1.4.7", "ced9c146e6e7a4ab2e9891efb133bcaaf031ac7b05541a41d549262af2988ee7", [:mix], [{:ash, "~> 3.3", [hex: :ash, repo: "hexpm", optional: false]}, {:igniter, ">= 0.3.14 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: false]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:json_xema, "~> 0.4", [hex: :json_xema, repo: "hexpm", optional: false]}, {:open_api_spex, "~> 3.16", [hex: :open_api_spex, repo: "hexpm", optional: true]}, {:plug, "~> 1.11", [hex: :plug, repo: "hexpm", optional: false]}, {:spark, ">= 2.2.10 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}], "hexpm", "a00f8657e2492d87d7e7e851302ee320af21f12fc104a49cace972c804df8f4a"},
   "ash_postgres": {:hex, :ash_postgres, "2.4.21", "0bbe2f8603fc6709742fd666d347c9236bd1a5794d52692964d9170794a2b7d6", [:mix], [{:ash, ">= 3.4.48 and < 4.0.0-0", [hex: :ash, repo: "hexpm", optional: false]}, {:ash_sql, ">= 0.2.43 and < 1.0.0-0", [hex: :ash_sql, repo: "hexpm", optional: false]}, {:ecto, ">= 3.12.1 and < 4.0.0-0", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.12", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:igniter, ">= 0.4.4 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:inflex, "~> 2.1", [hex: :inflex, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:postgrex, ">= 0.0.0", [hex: :postgrex, repo: "hexpm", optional: false]}], "hexpm", "dac98a6bc8f6836a942257674a038c682b017bafd3480d2f73c9889c808e86a9"},
   "ash_sql": {:hex, :ash_sql, "0.2.44", "d9ab30acb8bedfbaf73f1a737ab319cdb04bcff40e4f4997b95d4e0a8a0e3536", [:mix], [{:ash, ">= 3.1.7 and < 4.0.0-0", [hex: :ash, repo: "hexpm", optional: false]}, {:ecto, "~> 3.9", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.9", [hex: :ecto_sql, repo: "hexpm", optional: false]}], "hexpm", "a4a178da467c57b443d7525632a686712c0f561b518ad870d14779b550d9e855"},

diff --git a/backend/test/edgehog_web/schema/mutation/create_update_channel_test.exs b/backend/test/edgehog_web/schema/mutation/create_update_channel_test.exs
@@ -1,7 +1,7 @@
 #
 # This file is part of Edgehog.
 #
-# Copyright 2023-2024 SECO Mind Srl
+# Copyright 2023-2025 SECO Mind Srl
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -27,6 +27,7 @@ defmodule EdgehogWeb.Schema.Mutation.CreateUpdateChannelTest do
   describe "createUpdateChannel mutation" do
     test "creates update_channel with valid data", %{tenant: tenant} do
       target_group = device_group_fixture(tenant: tenant)
+      assert target_group.update_channel_id == nil
 
       target_group_id = AshGraphql.Resource.encode_relay_id(target_group)
 
@@ -177,14 +178,19 @@ defmodule EdgehogWeb.Schema.Mutation.CreateUpdateChannelTest do
       assert %{
                path: ["createUpdateChannel"],
                fields: [:target_group_ids],
-               message: "some target groups were not found or are already associated with an update channel",
-               code: "invalid_argument"
+               message: "One or more target groups could not be found",
+               code: "not_found"
              } = error
     end
 
     test "fails when trying to use already assigned target groups", %{tenant: tenant} do
       target_group = device_group_fixture(tenant: tenant)
-      _ = update_channel_fixture(tenant: tenant, target_group_ids: [target_group.id])
+
+      _ =
+        update_channel_fixture(
+          tenant: tenant,
+          target_group_ids: [target_group.id]
+        )
 
       target_group_id = AshGraphql.Resource.encode_relay_id(target_group)
 
@@ -195,10 +201,12 @@ defmodule EdgehogWeb.Schema.Mutation.CreateUpdateChannelTest do
 
       assert %{
                path: ["createUpdateChannel"],
-               fields: [:target_group_ids],
-               message: "some target groups were not found or are already associated with an update channel",
-               code: "invalid_argument"
+               fields: [:update_channel_id],
+               message: "The update channel is already set for the device group " <> name,
+               code: "invalid_attribute"
              } = error
+
+      assert name == ~s["#{target_group.name}"]
     end
   end
 

diff --git a/backend/test/edgehog_web/schema/mutation/update_update_channel_test.exs b/backend/test/edgehog_web/schema/mutation/update_update_channel_test.exs
@@ -1,7 +1,7 @@
 #
 # This file is part of Edgehog.
 #
-# Copyright 2023-2024 SECO Mind Srl
+# Copyright 2023-2025 SECO Mind Srl
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -155,14 +155,16 @@ defmodule EdgehogWeb.Schema.Mutation.UpdateUpdateChannelTest do
       assert %{
                path: ["updateUpdateChannel"],
                fields: [:target_group_ids],
-               code: "invalid_argument",
-               message: "some target groups were not found or are already associated with an update channel"
+               code: "not_found",
+               message: "One or more target groups could not be found"
              } = error
     end
 
     test "fails when trying to use already assigned target groups", %{tenant: tenant, id: id} do
       target_group = device_group_fixture(tenant: tenant)
-      _ = update_channel_fixture(tenant: tenant, target_group_ids: [target_group.id])
+
+      _ =
+        update_channel_fixture(tenant: tenant, target_group_ids: [target_group.id])
 
       target_group_id = AshGraphql.Resource.encode_relay_id(target_group)
 
@@ -173,10 +175,12 @@ defmodule EdgehogWeb.Schema.Mutation.UpdateUpdateChannelTest do
 
       assert %{
                path: ["updateUpdateChannel"],
-               fields: [:target_group_ids],
-               code: "invalid_argument",
-               message: "some target groups were not found or are already associated with an update channel"
+               fields: [:update_channel_id],
+               code: "invalid_attribute",
+               message: "The update channel is already set for the device group " <> name
              } = error
+
+      assert name == ~s["#{target_group.name}"]
     end
   end