pf5 on job_invocation wizard

[rmynar]

While attempting to fix minor failures of REX tests i discovered that major updates are needed to support the new PF5 job invocation page.

Added support for PF5 in Job Invocation wizard
Improved search functions (added retries)
Improved error messages for better debugging

co-authored with Claude

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In JobInvocationCreateView, the custom after_fill override is left as pass while the docstring describes non-trivial navigation behavior; either implement the intended logic or remove the override to avoid confusing, dead code and potential divergence from base-class behavior.
• There are several places where hard-coded XPath targeting the hosts PF5 table (e.g. data-ouia-component-id='hosts-index-table') are duplicated across HostsView, NewHostEntity.schedule_remote_job, and navigation steps; consider centralizing these locators (or using existing table widgets consistently) to reduce the chance of divergence when the DOM changes.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `JobInvocationCreateView`, the custom `after_fill` override is left as `pass` while the docstring describes non-trivial navigation behavior; either implement the intended logic or remove the override to avoid confusing, dead code and potential divergence from base-class behavior.
- There are several places where hard-coded XPath targeting the hosts PF5 table (e.g. `data-ouia-component-id='hosts-index-table'`) are duplicated across `HostsView`, `NewHostEntity.schedule_remote_job`, and navigation steps; consider centralizing these locators (or using existing table widgets consistently) to reduce the chance of divergence when the DOM changes.

## Individual Comments

### Comment 1
<location path="airgun/entities/host_new.py" line_range="462-471" />
<code_context>
+            sleep(1)  # Wait for search results to load
+
+            # Select checkbox for each host found in results
+            for entity_name in entities_list:
+                # Find the row for this host and check its checkbox
+                row_checkbox_locator = (
+                    f".//table[@data-ouia-component-id='hosts-index-table']"
+                    f"//tbody/tr[.//a[contains(text(), '{entity_name}')]]//input[@type='checkbox']"
+                )
+                wait_for(
+                    lambda loc=row_checkbox_locator: self.browser.element(
+                        loc, check_visibility=True
+                    ),
+                    timeout=5,
+                    delay=0.5,
+                )
+                checkbox = self.browser.element(row_checkbox_locator)
+                if not self.browser.is_selected(checkbox):
+                    self.browser.click(checkbox)
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard `wait_for` calls that wrap `browser.element(...)` with `handle_exception=True` to avoid propagating `NoSuchElementException`.

Here, `self.browser.element(...)` inside `wait_for` can raise `NoSuchElementException` before the element appears, since `wait_for` defaults `handle_exception=False`. That bypasses the retry logic and can cause flaky tests under slow UI conditions.

Please set `handle_exception=True` on these `wait_for` calls (including the one for the schedule button) so `NoSuchElementException` is swallowed and retried:

```python
wait_for(
    lambda loc=row_checkbox_locator: self.browser.element(loc, check_visibility=True),
    timeout=5,
    delay=0.5,
    handle_exception=True,
)
```
</issue_to_address>

---

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]

trigger: test-robottelo
pytest: tests/foreman/ui/test_host.py

[Copilot]

Pull request overview

This PR updates Airgun’s UI layer to support the new PatternFly 5 (PF5) Job Invocation wizard flow and improves robustness of host search/navigation in the new PF5 Hosts UI, aiming to reduce Remote Execution (REX) test failures and improve debugging signals.

Changes:

• Added PF5 wizard-step handling for Job Invocation creation, including PF5 footer buttons and updated display checks.
• Introduced a PF5 actions dropdown wrapper widget compatible with existing fill() expectations.
• Reworked PF5 Hosts search and host-details navigation to be more resilient (retries / waits), plus new multi-host “Schedule a job” flow.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
airgun/widgets.py	Adds a PF5 actions dropdown wrapper to keep compatibility with fill()-based call sites.
airgun/views/job_invocation.py	Updates Job Invocation views for PF5 wizard steps, footer controls, and scheduled-job status behavior.
airgun/views/host_new.py	Expands the PF5 Hosts view with new controls and a custom PF5 search implementation.
airgun/views/common.py	Adds PF5WizardStepView to avoid PF4 expander-based wizard behavior in PF5.
airgun/entities/job_invocation.py	Switches job submission to rely on PF5-aware submit.click() behavior.
airgun/entities/host_new.py	Adds retrying search for PF5 hosts, multi-host “schedule remote job”, and updates PF5 navigation to host details.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[LadislavVasina1]

Several comments added please look at them @rmynar

[Satellite-QE]

PRT Result

Build Number: 848
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_host.py --external-logging
Test Result : ========== 13 failed, 85 passed, 1404 warnings in 23496.98s (6:31:36) ==========

[LadislavVasina1]

trigger: test-robottelo
pytest: tests/foreman/ui/test_host.py

[LadislavVasina1]

@rmynar Shouldn't this be placed in entities/all_hosts.py as it does action through all hosts page?

[rmynar]

That's a tricky question. Actually this is shared for both - host_new and all_hosts. I would prefer to keep it here.

[LadislavVasina1]

I now see that, the entity has a part where it does action through host details, my bad, but as the header is shared across all hosts page and host details, can't we just use the all host approach, and keep this entity (and moved to all_hosts.py) minimal with just the else branch of this entity?
AllHostsEntity.all_hosts_navigate_and_select_hosts_helper( handles one or more hosts well...

[LadislavVasina1]

@rmynar ^^^ what do you think about this comment?

[rmynar]

Previously this entity was only in host_new. This PR reflects new state when the entity can be used also in all_hosts. Although current design is not perfect I would like to keep it as is in order to increase pass rate. However I agree with need to redesign, which has to be done separately.

[LadislavVasina1]

@rmynar Another batch of comments added, please resolve the comments that you have addressed.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

[Satellite-QE]

PRT Result

Build Number: 860
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_host.py --external-logging
Test Result : === 4 failed, 93 passed, 1 deselected, 1423 warnings in 29899.59s (8:18:19) ====

[LadislavVasina1]

@rmynar Have you checked that there isn't something we could wait for in each wizard step instead of putting hard sleep here?

[rmynar]

I already removed about four hardcoded sleeps since the original state of this PR, just decided to don't spend too much time on these.