Bug 1647482

Summary: Vertical navigation missing IDs
Product: Red Hat Satellite Reporter: Lukáš Hellebrandt <lhellebr>
Component: WebUIAssignee: Ohad Levy <ohadlevy>
WebUI sub component: Foreman QA Contact: Lukáš Hellebrandt <lhellebr>
Status: CLOSED NOTABUG Docs Contact:
Severity: medium    
Priority: unspecified CC: afeferku, bkearney, ehelms, mhulan, mmccune, ohadlevy, okhatavk, sgraessl, tbrisker, zhunting
Version: 6.5.0Keywords: PrioBumpQA
Target Milestone: 6.8.0   
Target Release: Unused   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: foreman-1.22.0.27-1 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-08-05 14:31:14 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Lukáš Hellebrandt 2018-11-07 15:13:44 UTC
Description of problem:
Missing HTML IDs (at least) in Vertical Navigation. Regression against 6.4.

Version-Release number of selected component (if applicable):
Sat 6.5 snap 1

How reproducible:
Deterministic

Steps to Reproduce:
1. Login the Satellite
2. On the homepage, inspect Vertical Navigation's HTML

Actual results:
The tags don't have ID specified

Expected results:
The tags should have ID specified (preferrably the same as in Sat 6.4)

Comment 1 Marek Hulan 2018-11-09 13:42:29 UTC
It would be good to list some ids that are missing explicitly. I suppose it will be related to https://github.com/theforeman/foreman/pull/5898

Looking at diff between 6.4 and 6.5, I see
6.4: <a id="menu_item_fact_values" data-id="aid_fact_values" href="/fact_values"><span class="list-group-item-value">Facts</span></a>
6.5: <a href="/fact_values"><span class="list-group-item-value">Facts</span></a>

I think this should be restored in 6.5 timeframe and given how many tests this break for QE, it should be addressed ASAP.

I'd be more careful with word Regression here, it's not something user would notice, so rather removing. Please add back if you disagree.

Comment 5 Walden Raines 2019-02-14 14:04:22 UTC
Assigning to Ohad as this BZ is related to Gilad's work on the vertical navigation.

Comment 6 Walden Raines 2019-02-14 14:04:48 UTC
Created redmine issue https://projects.theforeman.org/issues/26052 from this bug

Comment 15 Bryan Kearney 2019-05-16 12:03:54 UTC
Moving this bug to POST for triage into Satellite 6 since the upstream issue https://projects.theforeman.org/issues/26052 has been resolved.

Comment 20 Lukáš Hellebrandt 2019-09-16 12:21:50 UTC
FailedQA with Sat 6.6 snap 20.

Top level menu items don't have IDs. For example Infrastructure:

<li class="secondary-nav-item-pf is-hover list-group-item">
 <a href="#">
  <span class="pficon pficon-network" title="Infrastructure"></span>
  <span class="list-group-item-value">Infrastructure</span>
 </a>
 <div class="nav-pf-secondary-nav">
  <div class="nav-item-pf-header">
   <span>Infrastructure</span>
  </div>
  [...]
 </div>
</li>

I don't think this BZ should be considered blocker by now.

Comment 24 Lukáš Hellebrandt 2020-01-29 14:17:04 UTC
I don't think moving this to ON_QA makes sense. Setting back to ASSIGNED because my concerns haven't been addressed.

To answer your question: While this is not an automation blocker anymore, I think this should be implemented for multiple reasons:
1) The IDs were there before and there is simply no reason to remove them - we should avoid changes that don't have any reason
2) It will make our automation cleaner and our jobs easier
3) I think major UI items should all have their IDs
4) For the sake of consistency

Comment 25 Marek Hulan 2020-01-29 19:59:59 UTC
Ad 1) imho you can't call change of internal html ids a regression, that's not a feature or even an API. Not sure what you base your assumption on, but the reason was the reimplementation in different stack, which simply couldn't use helpers from the old one. That is a good technical reason to me.

Ad 2) that is a good justification, could we change id values? Can you list what elements you need to have ids? We'd like to keep the freedom of changing internal html structures, but we'll try to make it as little intrusive to your tests as possible. Ideally you'd let us know right after some change like that is merged.

Ad 3) we need to define some guidelines, I have no idea what "major UI items" means. Is that every div? Everything that's clickable? Everything thay contains text value?

Ad 4) could.you please better describe this? Consistenxy with what?

Comment 26 Lukáš Hellebrandt 2020-01-30 10:19:42 UTC
I'd say that anything that is clickable should have its ID. In some cases, other elements having ID would be useful, too, but it's hard to describe generally. We can talk about it sometime. Ad 4), menu items on non-top level have IDs so it doesn't make sense that top level items don't.