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.

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

Rename tabs in the haml to match current tab in the controller scope #4836

Merged

Conversation

AparnaKarve
Copy link
Contributor

Adjusted Container Provider tab names so that they are consistent with what's specified in the haml and the value passed to the angular controller when Validate is clicked.

This is also a continuation of work for this PR - #4768

https://bugzilla.redhat.com/show_bug.cgi?id=1609735
https://bugzilla.redhat.com/show_bug.cgi?id=1640304

$scope.currentTab needs to always match the tab name passed in from the haml
in order to make sure that the validation is done correctly when the "Enter" key is pressed
'container_metrics' is actually a proper name for a tab to differentiate it from the RHV metrics tab.
So ideally  'container_metrics' tab name should be retained, however this would mean a whole bunch of other changes
from the backend side, since the backend expects cred_type to be 'metrics' (as opposed to 'container_metrics')

Therefore, unfortunately we have to rename the tab to 'metrics' . This is probably an easier change than changing the backend rails controller code.
@AparnaKarve
Copy link
Contributor Author

@miq-bot add_label bug,hammer/yes

@AparnaKarve
Copy link
Contributor Author

@masayag Thank you for testing this.

@miq-bot assign @h-kataria

@miq-bot
Copy link
Member

miq-bot commented Oct 25, 2018

Checked commits AparnaKarve/manageiq-ui-classic@583d776~...bc783ed with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Haml - Linter::Haml STDERR:
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

@miq-bot miq-bot assigned masayag and unassigned h-kataria Oct 25, 2018
@AparnaKarve
Copy link
Contributor Author

@miq-bot add_reviewer @masayag

@miq-bot miq-bot requested a review from masayag October 25, 2018 17:27
@h-kataria h-kataria assigned h-kataria and unassigned masayag Oct 25, 2018
Copy link
Contributor

@masayag masayag left a comment

Choose a reason for hiding this comment

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

Verified on my env.

@h-kataria h-kataria added this to the Sprint 98 Ending Nov 5, 2018 milestone Oct 26, 2018
@h-kataria h-kataria merged commit 020f33c into ManageIQ:master Oct 26, 2018
@AparnaKarve AparnaKarve deleted the rename_tab_to_match_current_tab branch October 26, 2018 15:06
simaishi pushed a commit that referenced this pull request Oct 26, 2018
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit e931e5c6d1926ccdc2330383121fd173b7fdbaa6
Author: Harpreet Kataria <[email protected]>
Date:   Fri Oct 26 10:04:28 2018 -0400

    Merge pull request #4836 from AparnaKarve/rename_tab_to_match_current_tab
    
    Rename tabs in the haml to match current tab in the controller scope
    
    (cherry picked from commit 020f33c9fc202507c7f66d20bff80714f40e913a)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1609735
    https://bugzilla.redhat.com/show_bug.cgi?id=1640304

@himdel
Copy link
Contributor

himdel commented Nov 27, 2018

@AparnaKarve
Copy link
Contributor Author

My first guess is something was probably missed in the first commit 583d776 while renaming virtualization tab to kubevirt.

@AparnaKarve
Copy link
Contributor Author

@masayag Would you be able to review #5001?

/cc @himdel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants