Skip to content

[6.18.z] hostgroup nonadmin viewer read - #20887

Merged
Satellite-QE merged 1 commit into
6.18.zfrom
cherry-pick-6.18.z-0da20aba3a316b090dff2355ba0b152b2a213a94
Feb 26, 2026
Merged

[6.18.z] hostgroup nonadmin viewer read#20887
Satellite-QE merged 1 commit into
6.18.zfrom
cherry-pick-6.18.z-0da20aba3a316b090dff2355ba0b152b2a213a94

Conversation

@Satellite-QE

@Satellite-QE Satellite-QE commented Feb 26, 2026

Copy link
Copy Markdown
Collaborator

Cherrypick of PR: #20875

Problem Statement

New test coverage for bug SAT-38451: Non-admin users on Satellite with viewer role, unable to see the hostgroup.

Solution

Ensure that non-admin user with viewer role can see hostgroup created by admin user.

Also added new UserFactory class to help with more reusable user fixtures.

PRT

trigger: test-robottelo
pytest: tests/foreman/ui/test_hostgroup.py -k test_positive_non_admin_viewer_role_read

Summary by Sourcery

Add test coverage and shared fixtures to validate that non-admin users with the Viewer role can read host groups created by admin users.

New Features:

  • Introduce a reusable UserFactory helper to create users with defaulted passwords in fixtures.
  • Add a module-scoped fixture providing a Viewer-role non-admin user bound to a specific organization and location.
  • Add a module-scoped fixture creating a host group associated with a given organization and location.

Tests:

  • Add a UI test ensuring a non-admin user with the Viewer role can see a host group created by an admin user, verifying SAT-38451 behavior.

hostgroups: new test for non-admin viewer role

Verifies: SAT-38451

New test for bug "Non-admin users on Satellite with viewer role, unable to see the hostgroup."
Ensure that non-admin user with viewer role can see hostgroup created by admin user.

Also added new `UserFactory` class to help with more reusable user fixtures.

(cherry picked from commit 0da20ab)
@Satellite-QE Satellite-QE added 6.18.z Introduced in or relating directly to Satellite 6.18 Auto_Cherry_Picked Automatically cherrypicked PR using GHA No-CherryPick PR doesnt need CherryPick to previous branches labels Feb 26, 2026
@Satellite-QE

Copy link
Copy Markdown
Collaborator Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_hostgroup.py -k test_positive_non_admin_viewer_role_read

@Satellite-QE Satellite-QE added the AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing label Feb 26, 2026
@Satellite-QE

Copy link
Copy Markdown
Collaborator Author

PRT Result

Build Number: 14491
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_hostgroup.py -k test_positive_non_admin_viewer_role_read --external-logging
Test Result : =========== 1 passed, 7 deselected, 8 warnings in 1176.12s (0:19:36) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Feb 26, 2026
@sourcery-ai

sourcery-ai Bot commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds a UI test to verify a non-admin user with the Viewer role can see host groups created by an admin and introduces reusable fixtures, including a UserFactory helper, a shared Viewer role fixture, and a hostgroup fixture bound to organization and location.

Class diagram for new UserFactory helper

classDiagram
    class UserFactory {
        <<static>> create_user(target_sat, params) User
    }

    class Satellite {
    }

    class APIUser {
        password
        create()
    }

    UserFactory ..> Satellite : uses target_sat
    Satellite ..> APIUser : api.User(**params)
    UserFactory ..> APIUser : returns
Loading

Flow diagram for new Viewer user and hostgroup fixtures

flowchart TD
    subgraph User_related_fixtures
        A[viewer_role fixture
        searches Role name=Viewer
        on session_target_sat] --> B[default_viewer_role fixture
        creates non_admin user
        via UserFactory]
        A --> C[module_user_viewer fixture
        creates non_admin Viewer user
        via UserFactory]
        D[UserFactory.create_user
        sets default password
        creates API User
        returns user with password]
        B --> D
        C --> D
    end

    subgraph Hostgroup_related_fixtures
        E[module_hostgroup_with_org_loc fixture
        creates HostGroup
        bound to module_org and module_location]
    end
Loading

File-Level Changes

Change Details Files
Introduce a reusable UserFactory helper and shared Viewer role fixture for creating users with roles in tests.
  • Add UserFactory class with a static create_user method that ensures a password is set and returns a created User object with password populated.
  • Add a session-scoped viewer_role fixture that searches for and returns the existing Viewer role from Satellite.
  • Refactor default_viewer_role fixture to depend on viewer_role and use UserFactory.create_user instead of manually constructing and creating the user, removing the gen_string dependency.
pytest_fixtures/component/user_role.py
Add a dedicated fixture for a non-admin Viewer-role user for module-scoped tests.
  • Introduce module_user_viewer fixture that creates a non-admin user scoped to module_org and module_location with the Viewer role using UserFactory.create_user.
pytest_fixtures/component/user_role.py
Add test to verify Viewer-role non-admin users can view host groups created by admin users.
  • Introduce test_positive_non_admin_viewer_role_read UI test that logs in as module_user_viewer, selects the appropriate organization and location, and asserts the admin-created host group appears in the hostgroup search results.
tests/foreman/ui/test_hostgroup.py
Add a hostgroup fixture that binds host groups to specific organization and location for module-scoped tests.
  • Create module_hostgroup_with_org_loc fixture that creates a HostGroup associated with module_org and module_location via the API for reuse in tests like the new Viewer-role visibility test.
pytest_fixtures/component/hostgroup.py

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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The new viewer_role fixture is bound to session_target_sat but is later passed into module_target_sat.api.User(...) calls; if those API clients are not interchangeable this could cause subtle issues, so consider resolving the role via the same module_target_sat used to create the user.
  • For consistency and easier future maintenance, you might want to route other user-creation fixtures (e.g., module_user) through UserFactory.create_user as well so user initialization logic is centralized in one place.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new `viewer_role` fixture is bound to `session_target_sat` but is later passed into `module_target_sat.api.User(...)` calls; if those API clients are not interchangeable this could cause subtle issues, so consider resolving the role via the same `module_target_sat` used to create the user.
- For consistency and easier future maintenance, you might want to route other user-creation fixtures (e.g., `module_user`) through `UserFactory.create_user` as well so user initialization logic is centralized in one place.

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.

@Satellite-QE
Satellite-QE merged commit c808b83 into 6.18.z Feb 26, 2026
18 of 20 checks passed
@Satellite-QE
Satellite-QE deleted the cherry-pick-6.18.z-0da20aba3a316b090dff2355ba0b152b2a213a94 branch February 26, 2026 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.18.z Introduced in or relating directly to Satellite 6.18 Auto_Cherry_Picked Automatically cherrypicked PR using GHA AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing No-CherryPick PR doesnt need CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants