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

Fixed quadicon link for automation providers #840

Conversation

lgalis
Copy link
Contributor

@lgalis lgalis commented Mar 29, 2017

Fixes the switch to grid view for Ansible Tower Provider.
Also Fixes the link to the Inventory group list from the provider quadicon.

Depends on ManageIQ/manageiq#14691

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

screenshot from 2017-04-10 14-45-39
screenshot from 2017-04-10 14-45-43

@lgalis
Copy link
Contributor Author

lgalis commented Mar 29, 2017

@miq-bot add_label bug, fine/yes

@lgalis lgalis closed this Mar 31, 2017
@lgalis lgalis reopened this Mar 31, 2017
@lgalis lgalis force-pushed the automation_providers_error_when_switching_view_type branch from 1d25a0c to aa480b8 Compare April 4, 2017 22:20
@lgalis lgalis changed the title [WIP] Fixed quadicon link for automation providers Fixed quadicon link for automation providers Apr 5, 2017
@miq-bot miq-bot removed the wip label Apr 5, 2017
@lgalis lgalis changed the title Fixed quadicon link for automation providers [WIP] Fixed quadicon link for automation providers Apr 5, 2017
@miq-bot miq-bot added the wip label Apr 5, 2017
@h-kataria h-kataria self-assigned this Apr 6, 2017
@lgalis lgalis force-pushed the automation_providers_error_when_switching_view_type branch from c9bdea8 to ea99233 Compare April 6, 2017 21:49
@miq-bot
Copy link
Member

miq-bot commented Apr 10, 2017

This pull request is not mergeable. Please rebase and repush.

@lgalis lgalis force-pushed the automation_providers_error_when_switching_view_type branch 2 times, most recently from d8f0b87 to f581f91 Compare April 10, 2017 13:09
@lgalis lgalis force-pushed the automation_providers_error_when_switching_view_type branch 2 times, most recently from 878dcd9 to bcde536 Compare April 10, 2017 14:10
@lgalis lgalis closed this Apr 10, 2017
@lgalis lgalis reopened this Apr 10, 2017
@lgalis lgalis closed this Apr 10, 2017
@lgalis lgalis reopened this Apr 10, 2017
@lgalis lgalis closed this Apr 10, 2017
@lgalis lgalis reopened this Apr 10, 2017
@lgalis lgalis changed the title [WIP] Fixed quadicon link for automation providers Fixed quadicon link for automation providers Apr 10, 2017
@miq-bot miq-bot removed the wip label Apr 10, 2017
@lgalis
Copy link
Contributor Author

lgalis commented Apr 10, 2017

@dclarizio - please review

@lgalis
Copy link
Contributor Author

lgalis commented Apr 10, 2017

@miq-bot add_label euwe/no

@lgalis lgalis force-pushed the automation_providers_error_when_switching_view_type branch from bcde536 to 6db1040 Compare April 10, 2017 17:21
@@ -4,7 +4,7 @@
= render_quadicon(@record, :mode => :icon, :size => 72)
- if @record.kind_of?(ConfigurationProfile)
= render :partial => "#{controller}/main_configuration_profile"
- elsif @record.kind_of?(ManageIQ::Providers::AutomationManager::InventoryGroup)
- elsif @record.kind_of?(ManageIQ::Providers::ExternalAutomationManager::InventoryGroup)
Copy link
Member

Choose a reason for hiding this comment

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

Is this also supposed to be InventoryRootGroup? I don't really know what this file does, but we only refresh InventoryRootGroups right now.

@@ -225,6 +225,11 @@
expect(subject).to eq(helper.url_for_db(helper.controller_for_vm(helper.model_for_vm(@record)), @action))
end

it "when record is ManageIQ::Providers::AutomationManager" do
@record = ManageIQ::Providers::AutomationManager.new
Copy link
Member

Choose a reason for hiding this comment

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

Should this be testing against base classes or the leaf classes that will actually be created?

Copy link
Member

@bdunne bdunne left a comment

Choose a reason for hiding this comment

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

Hi @lgalis you have two merge commits in this branch. It looks like you don't have pull.rebase = true in your .gitconfig

@lgalis lgalis force-pushed the automation_providers_error_when_switching_view_type branch 2 times, most recently from cdc670a to 9bdf0a8 Compare April 11, 2017 04:39
@lgalis
Copy link
Contributor Author

lgalis commented Apr 11, 2017

@mzazrivec - would you be able to review this?

@lgalis
Copy link
Contributor Author

lgalis commented Apr 12, 2017

@miq-bot assign @dclarizio

@miq-bot miq-bot assigned dclarizio and unassigned h-kataria Apr 12, 2017
@lgalis lgalis force-pushed the automation_providers_error_when_switching_view_type branch from 9bdf0a8 to e594ecc Compare April 12, 2017 13:47
@miq-bot
Copy link
Member

miq-bot commented Apr 12, 2017

Checked commits lgalis/manageiq-ui-classic@cdc757c~...e594ecc with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
6 files checked, 0 offenses detected
Everything looks good. 🏆

@dclarizio dclarizio merged commit 6f5c40f into ManageIQ:master Apr 12, 2017
@dclarizio dclarizio added this to the Sprint 59 Ending Apr 24, 2017 milestone Apr 12, 2017
@lgalis lgalis deleted the automation_providers_error_when_switching_view_type branch April 12, 2017 16:01
simaishi pushed a commit that referenced this pull request Apr 13, 2017
…witching_view_type

Fixed quadicon link for automation providers
(cherry picked from commit 6f5c40f)

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

Fine backport details:

$ git log -1
commit e81ac92d3067bda4978d50bd24c438935621df30
Author: Dan Clarizio <dclarizi@redhat.com>
Date:   Wed Apr 12 08:56:05 2017 -0700

    Merge pull request #840 from lgalis/automation_providers_error_when_switching_view_type
    
    Fixed quadicon link for automation providers
    (cherry picked from commit 6f5c40fd9fff48052ac40f722821db55533f9afd)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1442174

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