Change Owner/Org/Loc entities/views coverage

[LadislavVasina1]

This PR adds coverage for:

• change the owner of multiple hosts (SAT-31026)
• change the location and organization of multiple hosts (SAT-31030)
  (@vijaysawant needed a backup with finishing the airgun coverage for this feature, so the additions regarding org and loc are inspired by all host change org and location airgun support #1927 and refactored)

This PR also introduces new function all_hosts_navigate_and_select_hosts_helper.
This function does repetitive navigation that is present almost everywhere in the entities/all_hosts.py.
In short, the function navigates to All Hosts and selects specified hosts or all hosts and does some assertion on the validity of calling this function.
I have tried to use this function on the places where I thought it might be beneficial.

I would like us to use this function forward when dealing with the entities navigation and host selection.

Needed by: SatelliteQE/robottelo#19030

Summary by Sourcery

Add bulk change owner, organization, and location actions for hosts and refactor host navigation selection logic.

New Features:

• Add bulk change owner action for multiple hosts
• Add bulk change organization action with fix/fail on mismatch options
• Add bulk change location action with fix/fail on mismatch options

Enhancements:

• Introduce all_hosts_navigate_and_select_hosts_helper to centralize navigation and selection logic
• Refactor existing bulk action methods to use the new helper
• Extend AllHostsTableView with change associations menu and add PF5 MenuToggleSelect component
• Add PF5 modal classes for change owner, organization, and location operations

[sourcery-ai]

Reviewer's Guide

This PR refactors multi-host workflows by extracting a shared navigation and selection helper, and adds full coverage for changing owners, organizations, and locations of multiple hosts through new entity methods, UI modals, and view components.

Sequence diagram for changing owner of multiple hosts

sequenceDiagram
    actor User
    participant AllHostsEntity
    participant AllHostsTableView
    participant ChangeHostsOwnerModal
    User->>AllHostsEntity: change_hosts_owner(host_names, new_owner_name, select_all_hosts)
    AllHostsEntity->>AllHostsTableView: all_hosts_navigate_and_select_hosts_helper(...)
    AllHostsEntity->>AllHostsTableView: bulk_actions_kebab.click()
    AllHostsEntity->>AllHostsTableView: bulk_actions_menu.item_select('Change owner')
    AllHostsEntity->>ChangeHostsOwnerModal: instantiate modal
    AllHostsEntity->>ChangeHostsOwnerModal: owner_select.item_select(new_owner_name)
    AllHostsEntity->>ChangeHostsOwnerModal: confirm_btn.click()

Loading

Sequence diagram for changing organization of multiple hosts

sequenceDiagram
    actor User
    participant AllHostsEntity
    participant AllHostsTableView
    participant ChangeOrganizationModal
    User->>AllHostsEntity: change_associations_organization(host_names, new_organization, select_all_hosts, option)
    AllHostsEntity->>AllHostsTableView: all_hosts_navigate_and_select_hosts_helper(...)
    AllHostsEntity->>AllHostsTableView: bulk_actions_kebab.click()
    AllHostsEntity->>AllHostsTableView: bulk_actions_menu.item_element('Change associations')
    AllHostsEntity->>AllHostsTableView: bulk_actions_change_associations_menu.item_select('Organization')
    AllHostsEntity->>ChangeOrganizationModal: instantiate modal
    AllHostsEntity->>ChangeOrganizationModal: organization_menu.item_select(new_organization)
    AllHostsEntity->>ChangeOrganizationModal: organization_fix_on_mismatch.fill(True) or organization_fail_on_mismatch.fill(True)
    AllHostsEntity->>ChangeOrganizationModal: save_button.click()

Loading

Sequence diagram for changing location of multiple hosts

sequenceDiagram
    actor User
    participant AllHostsEntity
    participant AllHostsTableView
    participant ChangeLocationModal
    User->>AllHostsEntity: change_associations_location(host_names, new_location, select_all_hosts, option)
    AllHostsEntity->>AllHostsTableView: all_hosts_navigate_and_select_hosts_helper(...)
    AllHostsEntity->>AllHostsTableView: bulk_actions_kebab.click()
    AllHostsEntity->>AllHostsTableView: bulk_actions_menu.item_element('Change associations')
    AllHostsEntity->>AllHostsTableView: bulk_actions_change_associations_menu.item_select('Location')
    AllHostsEntity->>ChangeLocationModal: instantiate modal
    AllHostsEntity->>ChangeLocationModal: location_menu.item_select(new_location)
    AllHostsEntity->>ChangeLocationModal: location_fix_on_mismatch.fill(True) or location_fail_on_mismatch.fill(True)
    AllHostsEntity->>ChangeLocationModal: save_button.click()

Loading

Class diagram for new and updated AllHosts entity methods and modals

classDiagram
    class AllHostsEntity {
        +delete(host_name)
        +bulk_delete_all()
        +build_management(reboot, rebuild)
        +change_hostgroup(name)
        +manage_packages(...)
        +manage_errata(...)
        +manage_repository_sets(...)
        +disassociate_hosts(host_names, select_all_hosts)
        +all_hosts_navigate_and_select_hosts_helper(host_names, select_all_hosts)
        +change_hosts_owner(host_names, new_owner_name, select_all_hosts)
        +change_associations_organization(host_names, new_organization, select_all_hosts, option)
        +change_associations_location(host_names, new_location, select_all_hosts, option)
    }
    class AllHostsTableView {
        +bulk_actions_kebab
        +bulk_actions_menu
        +bulk_actions_change_associations_menu
    }
    class ChangeHostsOwnerModal {
        +owner_select
        +confirm_btn
        +cancel_btn
        +is_displayed
    }
    class BaseChangeOrgLocModal {
        +title
        +menu_toggle
        +success_alert
        +error_alert
        +is_displayed
    }
    class ChangeOrganizationModal {
        +organization_menu
        +organization_fix_on_mismatch
        +organization_fail_on_mismatch
        +save_button
        +cancel_button
    }
    class ChangeLocationModal {
        +location_menu
        +location_fix_on_mismatch
        +location_fail_on_mismatch
        +save_button
        +cancel_button
    }
    class MenuToggleSelect {
    }
    AllHostsEntity --> AllHostsTableView
    AllHostsEntity --> ChangeHostsOwnerModal
    AllHostsEntity --> ChangeOrganizationModal
    AllHostsEntity --> ChangeLocationModal
    ChangeOrganizationModal --|> BaseChangeOrgLocModal
    ChangeLocationModal --|> BaseChangeOrgLocModal
    ChangeHostsOwnerModal --> MenuToggleSelect
    ChangeOrganizationModal --> MenuToggleSelect
    ChangeLocationModal --> MenuToggleSelect

Loading

File-Level Changes

Change	Details	Files
Extracted a common helper for navigating to All Hosts and selecting hosts	Added all_hosts_navigate_and_select_hosts_helper with parameter validation Replaced duplicate navigation/search/select code in delete, bulk_delete_all, build_management, change_hostgroup, manage_packages, manage_errata, manage_repository_sets, disassociate_hosts	airgun/entities/all_hosts.py
Introduced entity methods for change owner, organization, and location actions	Implemented change_hosts_owner to select hosts and trigger owner modal Added change_associations_organization with option flags Added change_associations_location with option flags	airgun/entities/all_hosts.py
Added UI components and modals to support owner/org/loc changes	Created MenuToggleSelect for PF5 dropdowns Extended AllHostsTableView with bulk_actions_change_associations_menu Defined ChangeHostsOwnerModal, BaseChangeOrgLocModal, ChangeOrganizationModal, ChangeLocationModal classes	airgun/views/all_hosts.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

• 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 @LadislavVasina1 - I've reviewed your changes and they look great!

Prompt for AI Agents

Please address the comments from this code review:
## Individual Comments

### Comment 1
<location> `airgun/entities/all_hosts.py:474` </location>
<code_context>
+        view = DisassociateHostsModal(self.browser)
+        view.confirm_btn.click()
+
+    def all_hosts_navigate_and_select_hosts_helper(self, host_names=None, select_all_hosts=False):
+        """
+        Helper function to navigate to All Hosts and select specified hosts or all hosts.
+        This function is used to avoid code duplication in methods that require host selection.
+
+        :param host_names: str with one host or list of hosts to select
+        :param select_all_hosts: bool, if True, all hosts will be selected
+
+        :raises ValueError: if both select_all_hosts and host_names are specified
+
+        :return: view (AllHostsTableView)
+        """
+
         if select_all_hosts and host_names:
             raise ValueError('Cannot select all and specify host names at the same time!')

</code_context>

<issue_to_address>
The helper does not handle the case where both host_names and select_all_hosts are falsy.

If neither host_names nor select_all_hosts is provided, the function navigates without selecting any hosts. If this is unintended, consider raising an error or documenting the expected behavior.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
        if select_all_hosts and host_names:
            raise ValueError('Cannot select all and specify host names at the same time!')

=======
        if select_all_hosts and host_names:
            raise ValueError('Cannot select all and specify host names at the same time!')

        if not select_all_hosts and not host_names:
            raise ValueError('Must specify either host_names or select_all_hosts.')
>>>>>>> REPLACE

</suggested_fix>

### Comment 2
<location> `airgun/entities/all_hosts.py:560` </location>
<code_context>
+            view.organization_fail_on_mismatch.fill(True)
+            view.save_button.click()
+
+    def change_associations_location(
+        self, host_names=None, new_location=None, select_all_hosts=False, option="Fix on mismatch"
+    ):
+        """
+        Navigate to change location modal after selecting number of hosts,
+        select desired location, select one of the options and apply changes.
+
+        :param host_names: str with one host or list of hosts to select
+        :param new_location: str location name which will be selected
+        :param select_all_hosts: select all hosts flag
+        :param option: str options either 'Fix on mismatch' or 'Fail on mismatch'
+        """
+
+        if new_location is None:
+            raise ValueError('new_location argument is None, it will not allow to Save changes')
+
</code_context>

<issue_to_address>
Redundant check for new_location before selecting location.

Since a ValueError is raised if new_location is None, the subsequent check is redundant and can be removed.

Suggested implementation:

```python
        if new_location is None:
            raise ValueError('new_location argument is None, it will not allow to Save changes')

```

```python
        if new_location is None:
            raise ValueError('new_location argument is None, it will not allow to Save changes')


```
</issue_to_address>

### Comment 3
<location> `airgun/views/all_hosts.py:615` </location>
<code_context>
         return self.browser.wait_for_element(self.title, exception=False) is not None
+
+
+class MenuToggleSelect(PF5Select):
+    """
+    This class is PF5 implementation of the Select component within the new PF5 structure
+    Which is MenuToggle->Select and not just Select as it was in PF4.
+    """
+
+    BUTTON_LOCATOR = './/button[contains(@class, "pf-v5-c-menu-toggle")]'
+    DEFAULT_LOCATOR = (
+        './/div[contains(@class, "pf-v5-c-menu") and @data-ouia-component-type="PF5/Select"]'
+    )
+    ROOT = f"{BUTTON_LOCATOR}/.."
+    ITEMS_LOCATOR = ".//ul[contains(@class, 'pf-v5-c-menu__list')]/li"
+    ITEM_LOCATOR = (
+        "//*[contains(@class, 'pf-v5-c-menu__item') and .//*[contains(normalize-space(.), {})]]"
+    )
</code_context>

<issue_to_address>
ITEM_LOCATOR may not interpolate the item value as intended.

If ITEM_LOCATOR is meant for string formatting, ensure all usages apply .format(). Otherwise, consider refactoring it into a method or property that accepts the item value.
</issue_to_address>

### Comment 4
<location> `airgun/views/all_hosts.py:652` </location>
<code_context>
+        return self.browser.wait_for_element(self.title, exception=False) is not None
+
+
+class BaseChangeOrgLocModal(PF5Modal):
+    """
+    Base class representing the modal for changing organization or location.
+    """
+
+    title = './/h1[@class="pf-v5-c-modal-box__title"]'
+
+    menu_toggle = PF5Menu(locator='.//button[@class="pf-v5-c-menu-toggle"]')
+
+    success_alert = PF5Alert(
</code_context>

<issue_to_address>
menu_toggle locator may be too generic.

Consider refining the locator to target a specific menu toggle to prevent matching multiple elements in the modal.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
    menu_toggle = PF5Menu(locator='.//button[@class="pf-v5-c-menu-toggle"]')
=======
    menu_toggle = PF5Menu(locator='.//div[contains(@class, "pf-v5-c-modal-box")]//button[@class="pf-v5-c-menu-toggle"]')
>>>>>>> REPLACE

</suggested_fix>

---

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

• X
• Mastodon
• LinkedIn
• Facebook

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

[LadislavVasina1]

PRT passed here SatelliteQE/robottelo#19030

[vsedmik]

Looks good to me, left one non-blocking nit bellow.

[vijaysawant]

Please look at comment, rest of stuff is good.