Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[6.15.z] ignore all hidden columns in SatTable widget #1468

Merged
merged 1 commit into from
Jul 24, 2024
Merged

[6.15.z] ignore all hidden columns in SatTable widget #1468

merged 1 commit into from
Jul 24, 2024

Conversation

Satellite-QE
Copy link
Contributor

Cherrypick of PR: #1465

Problem Statement

The Hosts table HTML contains multiple duplicate columns that are hidden, like Name, Model, Operating system, etc.
On the eye, the rendered result looks and behaves correctly,
however these hidden columns are causing the widget airgun.widgets.SatTable to behave not fully correctly.

This flaw started to fully manifest by merging PR #1453
where the SatTable widget interacted with the hidden columns instead of the visible ones (and sometimes vice versa).

Solution

The SatTable widget is modified to ignore all hidden headers and columns in the table.

@pnovotny @web-flow
hosts: new widget for Hosts table (#1465)
0d8d759 
The `SatTable` widget is modified to ignore hidden headers and columns in the table.
Hidden duplicate columns cause the widget to work incorrectly internally,
while on the outside everything seems in order.

This behavior is especially present in the Hosts table,
where instead of seemingly interacting with the visible columns,
the widget interacts with the hidden ones.

The table widget for Hosts somehow worked properly(-ish),
until commit 886a1a8 exposed this flaw.

Excluding the hidden columns during the table cells collection
by the widget solves the problem.

(cherry picked from commit 71a745e)
@Satellite-QE Satellite-QE added 6.15.z Auto_Cherry_Picked GHA has automatically cherrypicked this PR No-CherryPick PR doesnt need CherryPick to previous branches labels Jul 17, 2024
@Satellite-QE
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_host.py::test_positive_read_from_details_page tests/foreman/ui/test_errata.py::test_positive_apply_for_all_hosts tests/foreman/ui/test_rhcloud_insights.py::test_insights_registration_with_capsule

@Satellite-QE Satellite-QE added the AutoMerge_Cherry_Picked Automatically merge the PR is PRT and all checks are passing label Jul 17, 2024
@Satellite-QE
Copy link
Contributor Author

PRT Result

Build Number: 321
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_host.py::test_positive_read_from_details_page tests/foreman/ui/test_errata.py::test_positive_apply_for_all_hosts tests/foreman/ui/test_rhcloud_insights.py::test_insights_registration_with_capsule --external-logging
Test Result : ================== 10 warnings, 5 errors in 268.86s (0:04:28) ==================

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Jul 17, 2024
@Satellite-QE
Copy link
Contributor Author

PRT Result

Build Number: 322
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_host.py::test_positive_read_from_details_page tests/foreman/ui/test_errata.py::test_positive_apply_for_all_hosts tests/foreman/ui/test_rhcloud_insights.py::test_insights_registration_with_capsule --external-logging
Test Result : =========== 1 failed, 4 passed, 1058 warnings in 4496.78s (1:14:56) ============

@Satellite-QE
Copy link
Contributor Author

PRT Result

Build Number: 323
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_host.py::test_positive_read_from_details_page tests/foreman/ui/test_errata.py::test_positive_apply_for_all_hosts tests/foreman/ui/test_rhcloud_insights.py::test_insights_registration_with_capsule --external-logging
Test Result : ============ 1 failed, 4 passed, 835 warnings in 3620.80s (1:00:20) ============

@pnovotny
Copy link
Contributor

Marking as PRT-Passed.
After investigating the failure in the PRT run, test test_positive_apply_for_all_hosts does not fail because of navigation error to the host details page.
It goes to the details page, but fails to find the Content tab there. The fix for this issue is outside of this PR scope.

@pnovotny pnovotny added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Jul 22, 2024
LadislavVasina1
LadislavVasina1 approved these changes Jul 23, 2024
View reviewed changes
Copy link
Contributor

@LadislavVasina1 LadislavVasina1 left a comment

Choose a reason for hiding this comment

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

ACK

@Gauravtalreja1 Gauravtalreja1 merged commit d27049d into 6.15.z Jul 24, 2024
16 of 21 checks passed
@Gauravtalreja1 Gauravtalreja1 deleted the cherry-pick-6.15.z-71a745e3771fc5ea836cd0e3a43d75acf8d91951 branch July 24, 2024 07:11
@Gauravtalreja1 Gauravtalreja1 mentioned this pull request Jul 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LadislavVasina1 LadislavVasina1 LadislavVasina1 approved these changes

@Gauravtalreja1 Gauravtalreja1 Gauravtalreja1 approved these changes

@lhellebr lhellebr Awaiting requested review from lhellebr

@pondrejk pondrejk Awaiting requested review from pondrejk

Assignees

@pnovotny pnovotny

Labels
6.15.z Auto_Cherry_Picked GHA has automatically cherrypicked this PR AutoMerge_Cherry_Picked Automatically merge the PR is PRT and all checks are 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Satellite-QE @pnovotny @Gauravtalreja1 @LadislavVasina1