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

Fix Topology graph for Physical Infra Providers #656

Merged
merged 1 commit into from Apr 5, 2017

Conversation

AparnaKarve
Copy link
Contributor

screen shot 2017-03-10 at 10 03 32 am

@himdel This is a Topology follow-up PR for #196

%circle{:r => "17"}
-# pficon-cloud-tenant
%text{:y => "9"} 
-# pficon-physical_servers
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Aaah.. pficon-server-group :)

Copy link
Contributor

Choose a reason for hiding this comment

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

=> #922

@himdel
Copy link
Contributor

himdel commented Mar 13, 2017

Double-clicking the provider gives me a 404 on /physical_infra_manager/show/10000000000038 (guessing it should be /ems_physical_infra/10r38, which works).

@himdel
Copy link
Contributor

himdel commented Mar 13, 2017

Maybe add Topology to the menu as well?

@AparnaKarve
Copy link
Contributor Author

Maybe add Topology to the menu as well?

This change would be in core MIQ.

Can we get this classic UI specific fix merged today?

@himdel
Copy link
Contributor

himdel commented Mar 27, 2017

This change would be in core MIQ.

app/presenters/menu/default_menu.rb lives in manageiq-ui-classic ;)

@himdel
Copy link
Contributor

himdel commented Mar 27, 2017

..but feel free to add it to the menu in a separate PR :)

But, there's still the double click issue..

@miq-bot
Copy link
Member

miq-bot commented Mar 30, 2017

Checked commit AparnaKarve@465ae8c with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks good. 🏆

@AparnaKarve
Copy link
Contributor Author

app/presenters/menu/default_menu.rb lives in manageiq-ui-classic ;)

True, but not the product feature id required for it ;)
Created #862 and ManageIQ/manageiq#14589

@AparnaKarve
Copy link
Contributor Author

Also fixed the double-click issue on the Physical Infra Provider.

Note that for the Physical Server double-click to work, we would need #755 merged.

IMO, this PR can now be merged after you have verified that Physical Infra Provider double-click is fixed.

@AparnaKarve AparnaKarve reopened this Mar 31, 2017
@himdel
Copy link
Contributor

himdel commented Apr 5, 2017

Thanks @AparnaKarve fixes topology nicely, no sense in waiting for that remaining icon name issue, merging.

@himdel himdel merged commit ecc60b7 into ManageIQ:master Apr 5, 2017
@himdel himdel added this to the Sprint 58 Ending Apr 10, 2017 milestone Apr 5, 2017
simaishi pushed a commit that referenced this pull request Apr 5, 2017
Fix Topology graph for Physical Infra Providers
(cherry picked from commit ecc60b7)
@simaishi
Copy link
Contributor

simaishi commented Apr 5, 2017

Fine backport details:

$ git log -1
commit 3c36649c8b24737b8ec6a28da476fcb3f50d62d3
Author: Martin Hradil <himdel@seznam.cz>
Date:   Wed Apr 5 09:43:14 2017 +0000

    Merge pull request #656 from AparnaKarve/fix_physical_infra_topology
    
    Fix Topology graph for Physical Infra Providers
    (cherry picked from commit ecc60b7f609d95d7989354af9b1b5507ef99c10a)

@AparnaKarve AparnaKarve deleted the fix_physical_infra_topology branch July 26, 2017 18:03
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

5 participants