Bug 1647482 - Vertical navigation missing IDs
Summary: Vertical navigation missing IDs
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: WebUI
Version: 6.5.0
Hardware: Unspecified
OS: Unspecified
Target Milestone: 6.8.0
Assignee: Ohad Levy
QA Contact: Lukáš Hellebrandt
Depends On:
TreeView+ depends on / blocked
Reported: 2018-11-07 15:13 UTC by Lukáš Hellebrandt
Modified: 2020-08-05 14:31 UTC (History)
10 users (show)

Fixed In Version: foreman-
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Last Closed: 2020-08-05 14:31:14 UTC
Target Upstream Version:

Attachments (Terms of Use)

System ID Private Priority Status Summary Last Updated
Foreman Issue Tracker 26052 0 Normal Closed Vertical navigation missing IDs 2020-08-04 07:09:41 UTC

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:

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>
 <div class="nav-pf-secondary-nav">
  <div class="nav-item-pf-header">

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.

Note You need to log in before you can comment on or make changes to this bug.