Skip to content

feat: base images subscriptions#1197

Merged
lusergit merged 2 commits intoedgehog-device-manager:mainfrom
lusergit:push-wksltpvruorn
Jan 29, 2026
Merged

feat: base images subscriptions#1197
lusergit merged 2 commits intoedgehog-device-manager:mainfrom
lusergit:push-wksltpvruorn

Conversation

@lusergit
Copy link
Copy Markdown
Collaborator

Base images subscriptions

Exposes subscriptions for the base images domain, allowing users to subscribe to CUD operations on

  • Base Images
  • Base Images Collections

Checklist

  • I have read the CONTRIBUTING.md
  • I have added tests that prove my fix is effective or that my feature works
  • I have added or updated documentation (if appropriate)

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 27, 2026

Pull Request Test Coverage Report for Build bf28b85f909e6cfa1a2d8faf0979a4904073df67-PR-1197

Details

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

Totals Coverage Status
Change from base Build 21471501704: 77.8%
Covered Lines: 2545
Relevant Lines: 3273

💛 - Coveralls

@ArnelaL
Copy link
Copy Markdown
Collaborator

ArnelaL commented Jan 27, 2026

Since we’re also adding subscriptions for the :destroy action, it would be beneficial to add tests covering those cases as well.

Additionally, I suggest adding an option to subscribe only to a specific base_image or base_image_collection. This can be achieved by defining a read action by ID on the resource:

    read :get_by_id do
      argument :id, :uuid, allow_nil?: false
      get? true

      filter(expr(id == ^arg(:id)))
    end

and using that action in subscription:

subscribe :base_image_updated do
        action_types [:update]
        read_action :get_by_id
        relay_id_translations [id: :base_image]
      end

Comment on lines +56 to +65
base_image_collection_updated_query = """
subscription {
baseImageCollection {
updated {
id
name
}
}
}
"""
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd prefer if we stay consistent in all tests, and decide on a single format for defining queries for subscriptions. Here one test uses globally defined query and other local one, while in device_subscriptions_test the query is defined inside subscribe function.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that we can let the tests decide, we don't need a fully standardized way of testing this.
I updated however the queries to provide a default one and let the tester then set the actual query to run, so that it's more similar to what happens with the devices subscriptions 👍

@lusergit
Copy link
Copy Markdown
Collaborator Author

Additionally, I suggest adding an option to subscribe only to a specific base_image or base_image_collection

I agree with you that it should be available and I think this is something the filters would be great for! Unfortunately ash does not allow to use relay IDs in sorts and filters yet, which is not ideal :(

By using a custom by_id action the schema changes, reporting an id field

type RootSubscriptionType {
  baseImage(
    "A filter to limit the results"
    filter: BaseImageFilterInput # <- this is where I would expect filters to happen
    id: ID! # <- not really here
  ): base_image_result
}

My idea hence is to wait for this kind of support from the ash library itself and not allowing this specific behavior for now (without using raw IDs),

what do you think @ArnelaL ?

@lusergit lusergit force-pushed the push-wksltpvruorn branch 3 times, most recently from 141de7e to 77a00f7 Compare January 28, 2026 14:47
@ArnelaL
Copy link
Copy Markdown
Collaborator

ArnelaL commented Jan 28, 2026

Additionally, I suggest adding an option to subscribe only to a specific base_image or base_image_collection

I agree with you that it should be available and I think this is something the filters would be great for! Unfortunately ash does not allow to use relay IDs in sorts and filters yet, which is not ideal :(

By using a custom by_id action the schema changes, reporting an id field

type RootSubscriptionType {
  baseImage(
    "A filter to limit the results"
    filter: BaseImageFilterInput # <- this is where I would expect filters to happen
    id: ID! # <- not really here
  ): base_image_result
}

My idea hence is to wait for this kind of support from the ash library itself and not allowing this specific behavior for now (without using raw IDs),

what do you think @ArnelaL ?

In my opinion, since we’ll need this behavior for certain resources when adding subscriptions on the frontend, the best approach is to implement it selectively for those resources. If and when Ash adds support for filtering by Relay ID, we can then extend this to the remaining resources.

For this PR specifically, I don’t think this type of subscription is needed on the frontend, so there’s no need to add it here.

@lusergit
Copy link
Copy Markdown
Collaborator Author

Since we’re also adding subscriptions for the :destroy action, it would be beneficial to add tests covering those cases as well.

On this, I'd make a new PR based on #1198, as in one of those commit osman introduced a macro to check the deletion, I didn't forget 👍

Adds a required field on `assert_created` macro to account for the actual query
being retrieved.

Signed-off-by: Luca Zaninotto <luca.zaninotto@secomind.com>
Exposes subscriptions for the base images domain, allowing users to subscribe to
CUD operations on
- Base Images
- Base Images Collections

Signed-off-by: Luca Zaninotto <luca.zaninotto@secomind.com>
@lusergit
Copy link
Copy Markdown
Collaborator Author

/fast-forward

@lusergit lusergit merged commit bf28b85 into edgehog-device-manager:main Jan 29, 2026
19 checks passed
@lusergit lusergit deleted the push-wksltpvruorn branch January 29, 2026 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants