Add Update Channels #243

rbino · 2023-02-01T17:36:04Z

Add a GraphQL CRUD for UpdateChannels, allowing to manage them an associate them with Target Groups

coveralls · 2023-02-01T17:39:30Z

Pull Request Test Coverage Report for Build 9047d5d5078d8ba8db64f16f4f67cb6498347d84-PR-243

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+81.4%) to 81.434%

Totals
Change from base Build 4053144426:	81.4%
Covered Lines:	943
Relevant Lines:	1158

💛 - Coveralls

backend/lib/edgehog/update_campaigns.ex

backend/test/edgehog/update_campaigns_test.exs

backend/test/support/fixtures/groups_fixtures.ex

davidebriani · 2023-02-02T17:21:12Z

backend/priv/repo/migrations/20230130175529_add_update_channel_id_to_device_groups.exs

+          references(:update_channels,
+            with: [tenant_id: :tenant_id],
+            on_delete: :nilify_all
+          )


You might want to add a match: full on the foreign key here.

match: full doesn't allow the id to be NULL (see here), and since a device_group is allowed to have no connected update_channel, this should be supported

backend/test/support/fixtures/update_campaigns_fixtures.ex

backend/lib/edgehog/update_campaigns.ex

davidebriani · 2023-02-02T17:41:38Z

backend/lib/edgehog/update_campaigns/update_channel.ex

+  @doc false
+  def create_changeset(update_channel, attrs) do
+    update_channel
+    |> changeset(attrs)
+    |> validate_required([:target_group_ids])
+  end


I am missing the broader context, why a different create_changeset/2 that does not cast or perform all the validations included in changeset/2?

Actually it does, we usually do separate create/update changesets by duplicating the whole function, but I realized that if you need a changeset that is strictly a superset of another one, you can just create the smaller changeset with its function and then add additional validation/casting, which eliminates duplicate code and the need to keep the two changesets in sync for the common parts.
In this case, target_group_ids is already cast in the changeset function, but when you update an UpdateChannel you don't have to necessarily pass target_group_ids if you don't need to update them, while you do have to pass them when you create it, so I just add an additional validate_required on top of the other changeset

backend/test/edgehog/update_campaigns_test.exs

backend/lib/edgehog/update_campaigns.ex

szakhlypa · 2023-02-28T16:35:25Z

backend/lib/edgehog/update_campaigns/update_channel.ex

+    |> changeset(attrs)
+    |> validate_required([:target_group_ids])
+  end
+
  @doc false
  def changeset(update_channel, attrs) do


When we have few changesets (see changesets for base_image, ota_operation) usually we prefix their names with verb (like create_changeset, update_changeset). Do we want to rename changeset -> update_changeset?

In this case I left this name like this because it's actually reused in create_changeset (which is something I suspect we can do more when we split the whole update/create stuff). It doesn't make much sense for create_changeset to call update_changeset, hence the naming.

backend/test/edgehog_web/schema/query/update_channels_test.exs

backend/test/support/fixtures/update_campaigns_fixtures.ex

Generated with: mix phx.gen.context UpdateCampaigns UpdateChannel update_channels \ tenant_id:references:tenants name:string:unique handle:string:unique Copyright headers manually added after generation Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

- Change get_update_channel!/1 to fetch_update_channel/1 - Scope unique indexes to tenant - Validate handle format - Adapt tests and add some more Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

Prerequisite for using this fixtures in the upcoming UpdateChannel fixtures Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

Identify which update channel (if any) a device group is associated with Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

The association is explicit only on UpdateChannel since we want the dependency between context to only go in that direction (i.e. UpdateCampaigns knows about Groups, not the other way around). Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

Allow assigning and unassigning Device Groups when creating, updating or deleting an UpdateChannel. Add relevant tests and fixture changes. Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

Also add relevant tests Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

Also add relevant tests. Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

Allow retrieving a device group id -> update channel map for a list of device group ids using a single database query. Also add relevant tests. Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

Allow retrieving the UpdateChannel associated with a specific DeviceGroup. Do this using batching so we minimize N+1 query problems. Also add a relevant test and update CHANGELOG since the UpdateChannels feature is now complete. Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

rbino force-pushed the update-channels branch from 61454c3 to 9047d5d Compare February 1, 2023 17:48

rbino requested review from davidebriani and szakhlypa February 1, 2023 17:49

rbino assigned szakhlypa Feb 1, 2023

davidebriani requested changes Feb 2, 2023

View reviewed changes

rbino force-pushed the update-channels branch 4 times, most recently from 0b079ee to 4107493 Compare February 8, 2023 16:30

rbino marked this pull request as ready for review February 8, 2023 16:37

rbino requested review from davidebriani, bettio and Pavinati February 8, 2023 16:39

rbino force-pushed the update-channels branch from 4107493 to 0b5bbff Compare February 24, 2023 10:38

szakhlypa mentioned this pull request Feb 27, 2023

[frontend] Add Update Channels #245

Merged

szakhlypa reviewed Feb 28, 2023

View reviewed changes

rbino marked this pull request as draft March 7, 2023 11:24

rbino force-pushed the update-channels branch 2 times, most recently from 4181537 to 84c7dbd Compare March 7, 2023 17:14

rbino added 5 commits March 7, 2023 18:15

Adapt the generated UpdateChannel code to the actual requirements

d581280

- Change get_update_channel!/1 to fetch_update_channel/1 - Scope unique indexes to tenant - Validate handle format - Adapt tests and add some more Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

groups_fixture: add uniqueness to fixtures

fadc621

Prerequisite for using this fixtures in the upcoming UpdateChannel fixtures Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

Add update_channel_id column to device_groups table

9bc9cbc

Identify which update channel (if any) a device group is associated with Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

rbino force-pushed the update-channels branch from 84c7dbd to 78a359e Compare March 7, 2023 17:15

rbino marked this pull request as ready for review March 7, 2023 17:15

rbino added 3 commits March 9, 2023 17:54

update_campaigns: handle DeviceGroups in UpdateChannel functions

b34dd2f

Allow assigning and unassigning Device Groups when creating, updating or deleting an UpdateChannel. Add relevant tests and fixture changes. Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

GraphQL: add API to fetch and list UpdateChannel objects

88de5b9

Also add relevant tests Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

GraphQL: add createUpdateChannel mutation

1c455aa

Also add relevant tests Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

rbino added 4 commits March 9, 2023 17:55

GraphQL: add updateUpdateChannel mutation

333485f

Also add relevant tests Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

GraphQL: add deleteUpdateChannel mutation

0aab6fc

Also add relevant tests. Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

update_campaigns: add get_update_channels_for_device_group_ids/1

877d398

Allow retrieving a device group id -> update channel map for a list of device group ids using a single database query. Also add relevant tests. Signed-off-by: Riccardo Binetti <riccardo.binetti@secomind.com>

rbino force-pushed the update-channels branch from 78a359e to 8840b2b Compare March 9, 2023 16:55

szakhlypa approved these changes Mar 9, 2023

View reviewed changes

davidebriani approved these changes Mar 10, 2023

View reviewed changes

szakhlypa mentioned this pull request Mar 14, 2023

Update docs for Update Channels #260

Closed

szakhlypa merged commit e2245a7 into edgehog-device-manager:main Mar 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Update Channels #243

Add Update Channels #243

rbino commented Feb 1, 2023

coveralls commented Feb 1, 2023 •

edited

Loading

davidebriani Feb 2, 2023

rbino Feb 6, 2023 •

edited

Loading

davidebriani Feb 2, 2023

rbino Feb 6, 2023

szakhlypa Feb 28, 2023

rbino Mar 7, 2023

Add Update Channels #243

Add Update Channels #243

Conversation

rbino commented Feb 1, 2023

coveralls commented Feb 1, 2023 • edited Loading

Pull Request Test Coverage Report for Build 9047d5d5078d8ba8db64f16f4f67cb6498347d84-PR-243

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

davidebriani Feb 2, 2023

Choose a reason for hiding this comment

rbino Feb 6, 2023 • edited Loading

Choose a reason for hiding this comment

davidebriani Feb 2, 2023

Choose a reason for hiding this comment

rbino Feb 6, 2023

Choose a reason for hiding this comment

szakhlypa Feb 28, 2023

Choose a reason for hiding this comment

rbino Mar 7, 2023

Choose a reason for hiding this comment

coveralls commented Feb 1, 2023 •

edited

Loading

rbino Feb 6, 2023 •

edited

Loading