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

Add a button for Re-checking Authentication the Middleware providers GTL view #10643

Merged

Conversation

AparnaKarve
Copy link
Contributor

@AparnaKarve AparnaKarve commented Aug 19, 2016

This PR is identical to #10609, except specifically meant for Middleware GTL view and hence purely for upstream (or non-darga)

@abonas
Copy link
Member

abonas commented Aug 21, 2016

functionality related to #10153

@abonas
Copy link
Member

abonas commented Aug 21, 2016

there are currently 2 issues with this PR:

  1. the tooltip doesn't auto fill correctly the string and keeps the placeholder
  2. the reference is to ems_cloud instead of ems_middleware

screenshot at 2016-08-21 10-25-36

@AparnaKarve
Copy link
Contributor Author

AparnaKarve commented Aug 22, 2016

@abonas Good find! I would be fixing that shortly.
#{ui_lookup(:table=>"ems_middleware")} would not work here since we do not have a record available to us at this point.
I'm looking at other examples in the GTL toolbar, and it looks like for cases like these, we should just say --
"Re-check Authentication Status for the selected middleware providers"

@abonas
Copy link
Member

abonas commented Aug 22, 2016

@abonas Good find! I would be fixing that shortly.
#{ui_lookup(:table=>"ems_middleware")} would not work here since we do not have a record available to us at this point.
I'm looking at other examples in the GTL toolbar, and it looks like for cases like these, we should just say --
"Re-check Authentication Status for the selected middleware providers"

not sure what you mean that there aren't records available - isn't this option enabled when at least one record is selected? and the translation comes not from the record but from the table name/en.yml? also, isn't this similar to the case in #10153?

@abonas
Copy link
Member

abonas commented Aug 22, 2016

@mtho11 fyi

@AparnaKarve
Copy link
Contributor Author

AparnaKarve commented Aug 22, 2016

isn't this option enabled when at least one record is selected?

Yes, it is. But when you click on the toolbar button after a selection is made, what's available to you at that point is merely a record id of the record ...not the actual record.

EDIT:
And for multiple selections, it could be just record ids (plural) not the actual records

@AparnaKarve AparnaKarve force-pushed the recheck_auth_in_gtl_middleware branch from 32aed12 to c789b83 Compare August 22, 2016 15:16
:onwhen => "1+",
:items => [
button(
:ems_cloud_recheck_auth_status,
Copy link
Member

Choose a reason for hiding this comment

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

ems_cloud -> ems_middleware

@AparnaKarve AparnaKarve force-pushed the recheck_auth_in_gtl_middleware branch from c789b83 to 06a06dd Compare August 22, 2016 16:35
@miq-bot
Copy link
Member

miq-bot commented Aug 22, 2016

Checked commit AparnaKarve@06a06dd with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
1 file checked, 2 offenses detected

app/helpers/application_helper/toolbar/ems_middlewares_center.rb

  • ❗ - Line 56, Col 5 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.
  • ❗ - Line 74, Col 3 - Style/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.

@abonas
Copy link
Member

abonas commented Aug 22, 2016

LGTM. thanks @AparnaKarve 🌷
@miq-bot assign @chessbyte

@miq-bot miq-bot assigned chessbyte and unassigned abonas Aug 22, 2016
@chessbyte chessbyte merged commit 45a2e01 into ManageIQ:master Aug 22, 2016
@chessbyte chessbyte added this to the Sprint 45 Ending Aug 22, 2016 milestone Aug 22, 2016
@AparnaKarve AparnaKarve deleted the recheck_auth_in_gtl_middleware branch August 22, 2016 19:19
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.

None yet

4 participants