From 53746129a49e9e1b5f54a5df6bc0c7c6477ab943 Mon Sep 17 00:00:00 2001 From: Luca Zaninotto Date: Tue, 10 Dec 2024 12:46:03 +0100 Subject: [PATCH] chore(update_channel): better errors When creating and updating an update channel, target group ids are validated to be available, meaning: - they are present in the DB - they are not related to any other update channel closes #529 Signed-off-by: Luca Zaninotto --- .../groups/device_group/device_group.ex | 10 +++- .../validations/update_channel_absent.ex | 38 ++++++++++++++ .../changes/relate_target_groups.ex | 35 +++---------- .../update_channel/error_handler.ex | 49 +++++++++++++++++++ .../update_channel/update_channel.ex | 5 +- backend/mix.lock | 4 +- .../mutation/create_update_channel_test.exs | 22 ++++++--- .../mutation/update_update_channel_test.exs | 18 ++++--- 8 files changed, 134 insertions(+), 47 deletions(-) create mode 100644 backend/lib/edgehog/groups/device_group/validations/update_channel_absent.ex create mode 100644 backend/lib/edgehog/update_campaigns/update_channel/error_handler.ex diff --git a/backend/lib/edgehog/groups/device_group/device_group.ex b/backend/lib/edgehog/groups/device_group/device_group.ex index 26d84996b..22118be42 100644 --- 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 new file mode 100644 index 000000000..f6b346998 --- /dev/null +++ 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 index bb3f11129..dff3c1a03 100644 --- 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 new file mode 100644 index 000000000..975c0b316 --- /dev/null +++ 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 index f6e765eff..c098fe8aa 100644 --- 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 index 31e25906f..20ba9c1fc 100644 --- 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 index 154d9e34c..9a2471e2b 100644 --- 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 index b4b1e3a47..1646254d4 100644 --- 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