Fixes #38443 - Improve rabl and scoped search on content view environments #11405

jeremylenz · 2025-06-03T17:04:42Z

What are the changes introduced in this pull request?

Add 'id' to the rabl for the content_view_environments endpoint
Add fields to scoped search for content view environments:

label
content_view
lifecycle_environment

Address a bug where hammer host create with --content-view-environments option creates the host with no errors, but content view environments are not assigned.

Considerations taken when implementing this change?

This should unblock full Ansible support for assigning content view environments. I added more than just label since I thought the other two would be useful as well.

What are the testing steps for this pull request?

I used hammer -d, but you can use anything that allows you to inspect API responses.

make sure API response includes content view environment ID
Try searches: 'content_view ~ xxx' (even with Default_Organization_View); 'lifecycle_environment = xxx', 'label ~ xxxx'

Summary by Sourcery

Improve API discoverability and searchability for content view environments by exposing IDs, adding scoped_search fields, and ensuring host content facets are created and assigned correctly

Enhancements:

Expose id in RABL templates to include content view environment IDs in API responses
Add scoped_search on id, label, content_view, and lifecycle_environment for ContentViewEnvironment
Adjust host controller to use after_action for setting content view environments and auto-create missing content facets

sourcery-ai · 2025-06-03T17:04:46Z

Reviewer's Guide

Refactors host endpoint callback and content facet creation, exposes id in API views for content_view_environments, and adds scoped_search filters for label, content_view, and lifecycle_environment in the ContentViewEnvironment model.

Sequence Diagram: Host Entity Update/Create with ContentFacet Initialization

sequenceDiagram
    participant Client as API Client
    participant HC as HostsController
    participant SCVE as set_content_view_environments
    participant CF as Katello::Host::ContentFacet

    Client->>HC: POST/PUT /api/v2/hosts/... (create/update host)
    activate HC
    HC-->>HC: Core create/update action logic
    HC->>SCVE: after_action: set_content_view_environments()
    activate SCVE
    SCVE-->>SCVE: Read params (content_facet_attributes, cve_params)
    alt @host is present AND @host.content_facet is blank
        SCVE->>CF: Katello::Host::ContentFacet.create!(host: @host)
        activate CF
        CF-->>SCVE: New ContentFacet instance
        deactivate CF
    end
    SCVE-->>SCVE: Potentially call Katello::ContentViewEnvironment.fetch_content_view_environments()
    SCVE-->>HC: Return
    deactivate SCVE
    HC-->>Client: HTTP Response
    deactivate HC

ER Diagram: ContentViewEnvironment Scoped Search Enhancements

erDiagram
    ContentViewEnvironment {
        integer id PK "Newly searchable, Exposed in API"
        string label "Newly searchable"
        integer content_view_id FK
        integer lifecycle_environment_id FK
        string name
        boolean default_environment
    }
    ContentView {
        integer id PK
        string label "Search target for CVE via relation"
        string name
    }
    LifecycleEnvironment {
        integer id PK
        string label "Search target for CVE via relation"
        string name
    }

    ContentViewEnvironment }o--|| ContentView : "belongs_to"
    ContentViewEnvironment }o--|| LifecycleEnvironment : "belongs_to"

Class Diagram: ContentViewEnvironment Model and HostsControllerExtensions Changes

classDiagram
    class Katello.ContentViewEnvironment {
        <<Model>>
        +id: integer
        +label: string
        +content_view_id: integer
        +lifecycle_environment_id: integer
        # Other attributes...
    }
    note for Katello.ContentViewEnvironment "Changes:\n- 'id' attribute is now exposed in API responses (base.json.rabl, show.json.rabl).
- Added scoped_search capabilities for:
  - id (direct on ContentViewEnvironment)
  - label (direct on ContentViewEnvironment)
  - content_view.label (via relationship to ContentView)
  - lifecycle_environment.label (via relationship to LifecycleEnvironment)"

    class Katello.Concerns.Api.V2.HostsControllerExtensions {
        <<Ruby Concern>>
        +set_content_view_environments(): void
    }
    note for Katello.Concerns.Api.V2.HostsControllerExtensions "Changes to set_content_view_environments method:
- Callback execution changed from 'before_action' to 'after_action' for :create and :update host operations.
- Method logic updated: now creates a Katello::Host::ContentFacet record if the host is present and its content_facet is blank before proceeding."

    Katello.ContentViewEnvironment "many" -- "1" Katello.ContentView : content_view
    Katello.ContentViewEnvironment "many" -- "1" Katello.LifecycleEnvironment : lifecycle_environment

    class Katello.ContentView {
        +label: string
        # Other attributes...
    }
    class Katello.LifecycleEnvironment {
        +label: string
        # Other attributes...
    }
    class Katello.Host.ContentFacet {
        # Attributes...
    }

File-Level Changes

Change	Details	Files
Refactor content_view_environments assignment in hosts controller	Changed `before_action` to `after_action` for setting content view environments Simplified guard clause to skip when required parameters are present Automatically create a ContentFacet for hosts missing one before assignment	`app/controllers/katello/concerns/api/v2/hosts_controller_extensions.rb`
Enhance scoped_search in ContentViewEnvironment model	Added `scoped_search` on `label` Added `scoped_search` on related `content_view.label` renamed to `content_view` Added `scoped_search` on related `lifecycle_environment.label` renamed to `lifecycle_environment`	`app/models/katello/content_view_environment.rb`
Expose `id` attribute in API JSON templates	Inserted `id` node in content_facet base JSON view Added `id` attribute in content_view_environments show JSON view	`app/views/katello/api/v2/content_facet/base.json.rabl` `app/views/katello/api/v2/content_view_environments/show.json.rabl`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @jeremylenz - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

jeremylenz · 2025-06-03T17:15:31Z

Rubocop made me do something and I am not sure why. @MariaAga @evgeni does this look ok?

engines/bastion/bastion.gemspec

evgeni · 2025-06-04T07:26:26Z

/packit build

evgeni · 2025-06-04T07:33:58Z

What rubocop version were you using? The only offense I see (both in the GHA logs and locally) with your initial commit is:

app/models/katello/content_view_environment.rb:39:1: C: [Correctable] Layout/EmptyLines: Extra blank line detected.

The gemspec ones I can't reproduce with

% bundle list |grep rubocop
  * rubocop (1.23.0)
  * rubocop-ast (1.45.0)
  * rubocop-checkstyle_formatter (0.4.0)
  * rubocop-graphql (0.11.3)
  * rubocop-minitest (0.17.2)
  * rubocop-performance (1.8.1)
  * rubocop-rails (2.12.4)
  * rubocop-rspec (2.11.1)
  * theforeman-rubocop (0.1.2)

app/views/katello/api/v2/content_view_environments/show.json.rabl

jeremylenz · 2025-06-04T16:34:12Z

ok, let's try Rubocop again from scratch

app/models/katello/content_view_environment.rb

app/controllers/katello/concerns/api/v2/hosts_controller_extensions.rb

jeremylenz · 2025-06-04T18:35:00Z

Seems like my after_action strategy won't work because it causes multiple calls to render..

Error: Failure: test_set_content_view_environments_with_invalid_ids_param

AbstractController::DoubleRenderError: Render and/or redirect were called multiple times in this action. Please note that you may only call render OR redirect, and at most once per action. Also note that neither redirect nor render terminate execution of the action, so if you want to exit an action after redirecting, you need to do something like "redirect_to(...) and return".
    katello/app/lib/katello/api/v2/error_handling.rb:132:in `block (2 levels) in respond_for_exception'
    katello/app/lib/katello/api/v2/error_handling.rb:130:in `respond_for_exception'
    katello/app/lib/katello/api/v2/error_handling.rb:55:in `rescue_from_wrapped_error'
    katello/test/controllers/api/v2/hosts_controller_test.rb:181:in `test_set_content_view_environments_with_invalid_ids_param'

will have to keep thinking about it

jeremylenz · 2025-06-04T19:07:07Z

@sourcery-ai review

sourcery-ai

Hey @jeremylenz - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 1 issue found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

app/controllers/katello/concerns/api/v2/hosts_controller_extensions.rb

app/models/katello/content_view_environment.rb

app/views/katello/api/v2/content_facet/base.json.rabl

lfu · 2025-06-10T19:49:43Z

app/views/katello/api/v2/content_view_environments/show.json.rabl

@@ -3,6 +3,7 @@ extends 'katello/api/v2/common/identifier'

 extends 'katello/api/v2/common/timestamps'
 attributes :default_environment? => :default
+attributes :id


How do I test this one?

hammer content-view-environment doesn't have a show subcommand, so testing with hammer may not be possible there? but could see if hammer -d content-view-environment list shows it in the JSON.

I see the id in the response:

[2] { "default" => false, "id" => 4, <------------------------ content view environment id "name" => "stage/cv1", "label" => "stage/cv1", "created_at" => "2025-03-26 17:26:32 UTC", "updated_at" => "2025-03-26 17:26:32 UTC", "organization" => { "name" => "Default Organization", "label" => "Default_Organization", "id" => 1 <---------------------- organization id },

Can you try this without your patch?
I got same result from master branch.

Result from master branch:

"results" => [ [ 0] { "default" => true, "id" => 1, "name" => "Library", "label" => "Library", "created_at" => "2025-04-01 19:57:52 UTC", "updated_at" => "2025-04-01 19:57:52 UTC", "organization" => { "name" => "Default Organization", "label" => "Default_Organization", "id" => 1 },

Turns out :id was already included because of

extends 'katello/api/v2/common/identifier'

So I removed this line. 👍

jeremylenz · 2025-06-12T12:42:06Z

@lfu @sourcery-ai Came up with a solution for the host create issue. I split the validation and the actual saving into two. I am able to get both hammer host create and hammer host update to work this way, and content view environments are assigned correctly. let me know what you think

jeremylenz · 2025-06-12T16:35:44Z

Tests will need theforeman/actions#66

app/controllers/katello/concerns/api/v2/hosts_controller_extensions.rb

lfu · 2025-06-12T17:19:32Z

Could we add some test cases for the new methods?

jeremylenz · 2025-06-13T19:12:31Z

Added tests for the new methods. 👍

…ments - add id field to rabl in a few places - split validation and saving of content view environments params

lfu

LGTM 👍 Thanks @jeremylenz

pr-processor bot added the Not yet reviewed label Jun 3, 2025

sourcery-ai bot reviewed Jun 3, 2025

View reviewed changes

pr-processor bot removed the Not yet reviewed label Jun 3, 2025

github-actions bot added the Packaging Change label Jun 3, 2025

evgeni reviewed Jun 4, 2025

View reviewed changes

engines/bastion/bastion.gemspec Outdated Show resolved Hide resolved

evgeni reviewed Jun 4, 2025

View reviewed changes

app/views/katello/api/v2/content_view_environments/show.json.rabl Outdated Show resolved Hide resolved

jeremylenz force-pushed the 38443-cves-endpoint branch from 325ea97 to e409798 Compare June 4, 2025 16:33

github-actions bot removed the Packaging Change label Jun 4, 2025

evgeni reviewed Jun 4, 2025

View reviewed changes

app/models/katello/content_view_environment.rb Show resolved Hide resolved

evgeni reviewed Jun 4, 2025

View reviewed changes

app/controllers/katello/concerns/api/v2/hosts_controller_extensions.rb Outdated Show resolved Hide resolved

sourcery-ai bot reviewed Jun 4, 2025

View reviewed changes

app/controllers/katello/concerns/api/v2/hosts_controller_extensions.rb Outdated Show resolved Hide resolved

lfu reviewed Jun 10, 2025

View reviewed changes

app/models/katello/content_view_environment.rb Show resolved Hide resolved

lfu reviewed Jun 10, 2025

View reviewed changes

app/models/katello/content_view_environment.rb Show resolved Hide resolved

lfu reviewed Jun 10, 2025

View reviewed changes

app/views/katello/api/v2/content_facet/base.json.rabl Show resolved Hide resolved

lfu reviewed Jun 10, 2025

View reviewed changes

jeremylenz force-pushed the 38443-cves-endpoint branch 4 times, most recently from 5e5deb2 to c8d16cb Compare June 12, 2025 12:40

jeremylenz force-pushed the 38443-cves-endpoint branch from c8d16cb to ceefeb0 Compare June 12, 2025 12:43

lfu reviewed Jun 12, 2025

View reviewed changes

app/controllers/katello/concerns/api/v2/hosts_controller_extensions.rb Show resolved Hide resolved

jeremylenz force-pushed the 38443-cves-endpoint branch 2 times, most recently from d9c4d5e to 16b4347 Compare June 16, 2025 14:05

Fixes #38443 - Improve rabl and scoped search on content view environ…

9581fb8

…ments - add id field to rabl in a few places - split validation and saving of content view environments params

jeremylenz force-pushed the 38443-cves-endpoint branch from 16b4347 to 9581fb8 Compare June 16, 2025 14:11

lfu approved these changes Jun 16, 2025

View reviewed changes

Fixes #38443 - Improve rabl and scoped search on content view environments #11405

Are you sure you want to change the base?

Fixes #38443 - Improve rabl and scoped search on content view environments #11405

Uh oh!

Conversation

jeremylenz commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What are the changes introduced in this pull request?

Considerations taken when implementing this change?

What are the testing steps for this pull request?

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

Sequence Diagram: Host Entity Update/Create with ContentFacet Initialization

ER Diagram: ContentViewEnvironment Scoped Search Enhancements

Class Diagram: ContentViewEnvironment Model and HostsControllerExtensions Changes

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

jeremylenz commented Jun 3, 2025

Uh oh!

Uh oh!

evgeni commented Jun 4, 2025

Uh oh!

evgeni commented Jun 4, 2025

Uh oh!

Uh oh!

jeremylenz commented Jun 4, 2025

Uh oh!

Uh oh!

Uh oh!

jeremylenz commented Jun 4, 2025

Uh oh!

jeremylenz commented Jun 4, 2025

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lfu Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

lfu Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

jeremylenz Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

jeremylenz Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

lfu Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

lfu Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

jeremylenz Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

jeremylenz commented Jun 12, 2025

Uh oh!

jeremylenz commented Jun 12, 2025

Uh oh!

Uh oh!

lfu commented Jun 12, 2025

Uh oh!

jeremylenz commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeremylenz commented Jun 3, 2025 •

edited

Loading

sourcery-ai bot commented Jun 3, 2025 •

edited

Loading

jeremylenz commented Jun 13, 2025 •

edited

Loading