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 missing condition for retrieving data on the provider topology pages #3214

Merged
merged 1 commit into from Jan 10, 2018

Conversation

skateman
Copy link
Member

@skateman skateman commented Jan 10, 2018

When you go to a provider's (e.g. infra) summary screen and select the topology in the table view, the topology data are not being loaded onto the controller.

screenshot from 2018-01-10 14-17-41

The problem was that the condition for building the URL did not cover this way of displaying a topology screen. I updated the condition to make this work, however, some refactoring should be done in the future. The code is too much spaghetti for adding any tests, but after @Hyperkid123 is finished with the refactoring, it should be easy.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1532404

@miq-bot
Copy link
Member

miq-bot commented Jan 10, 2018

@skateman unrecognized command 'add', ignoring...

Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

@miq-bot
Copy link
Member

miq-bot commented Jan 10, 2018

Checked commit skateman@fe7ae03 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. ⭐

@himdel
Copy link
Contributor

himdel commented Jan 10, 2018

LGTM, matches what used to happen before (https://github.com/ManageIQ/manageiq-ui-classic/pull/3087/files#diff-ae0b2046e6aacd6b9890ae1d5b1648a4L27), merging when green...

@himdel
Copy link
Contributor

himdel commented Jan 10, 2018

Cloud, infra, container project, and physical server seem to work completely now :)
Containers work except from the summary - because it's not restful? and the url is
/ems_container/10000000000035?display=topology

@himdel
Copy link
Contributor

himdel commented Jan 10, 2018

OK, ems_container will be a separate bug, the fix is a bit different there.

Merging when green :)

@himdel himdel merged commit 769e80d into ManageIQ:master Jan 10, 2018
@himdel himdel added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 10, 2018
@skateman skateman deleted the fix-provider-topology branch January 10, 2018 15:03
simaishi pushed a commit that referenced this pull request Jan 10, 2018
Add missing condition for retrieving data on the provider topology pages
(cherry picked from commit 769e80d)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1533237
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit e651e1ca0e3cfdaf78e31c321145618b0c2eefdb
Author: Martin Hradil <himdel@seznam.cz>
Date:   Wed Jan 10 16:01:17 2018 +0100

    Merge pull request #3214 from skateman/fix-provider-topology
    
    Add missing condition for retrieving data on the provider topology pages
    (cherry picked from commit 769e80d09146d468ec7605bb82fd7394424c9c1a)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1533237

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