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

Disable editing of NetworkProviders except Nuage #3394

Merged
merged 1 commit into from Feb 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/helpers/application_helper/button/ems_network.rb
@@ -1,5 +1,9 @@
class ApplicationHelper::Button::EmsNetwork < ApplicationHelper::Button::Basic
def visible?
::Settings.product.nuage
# Nuage is only provider that supports adding NetworkProvider manually
# therefore we hide drop-down option completely unless Nuage is enabled.
return ::Settings.product.nuage unless @record # add new

@record.supports?(:ems_network_new) # edit
Copy link
Contributor

Choose a reason for hiding this comment

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

@miha-plesko Quick question.
Does this work from the list view?
I think, it won't - because @record does not exist in the list view.

I need to do something similar in one other PR and was looking into options of disabling edit in the list view for a particular type of record.
(sadly, there is no easy way to do this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AparnaKarve This doesn't work indeed on the list view. However, I remember we're using something similar on other location like this:

https://github.com/manageiq/manageiq-ui-classic/blob/master/app/controllers/application_controller/ci_processing.rb#L322-L335

What we do there is we loop all selected items and check for each if it supports specific feature. If there is at least one item found that doesn't support the feature, we refuse performing action and fire up popup saying "Operation does not apply to at least one of the selected items" instead.

Something similar should probably also be implemented here. However, I must apologize, but I'm fully loaded and can't work on this ATM - can it wait a week or two?

Copy link
Contributor

Choose a reason for hiding this comment

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

@miha-plesko That may work - although there would be a slight compromise in the UX, I think.
Meaning, you would have to first go through the Edit button click, and then it would be determined if the Edit operation is supported or not -- as opposed to the UI intelligently determining beforehand if the Edit button needs to be visible in the first place.

I was thinking an other easier option would be to simply disable/hide Edit in the list view, because all of the Network Providers, (with the exception of Nuage) do not support Edit
...and, display the Edit button for a Nuage Network Provider only in the summary screen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, Edit button would best not appear on list view at all.

end
end
@@ -0,0 +1,26 @@
describe ApplicationHelper::Toolbar::EmsNetworkCenter do
let(:ems_network_vmdb) { described_class.definition['ems_network_vmdb'] }
let(:ems_network_vmdb_choice) { ems_network_vmdb.buttons.detect { |b| b[:id] == 'ems_network_vmdb_choice' } }

describe 'Edit this Network Provider' do
let(:button_hash) { ems_network_vmdb_choice[:items].detect { |b| b[:id] == 'ems_network_edit' } }
let(:button_klass) { button_hash[:klass] }
let(:button) { button_klass.new(nil, nil, {}, {}) }
let(:ems_nuage) { FactoryGirl.create(:ems_nuage_network) }
let(:ems_openstack) { FactoryGirl.create(:ems_openstack) }

it 'appropriate button class' do
expect(button_klass).to eq(ApplicationHelper::Button::EmsNetwork)
end

it 'visible for nuage provider' do
button.instance_variable_set(:@record, ems_nuage)
expect(button.visible?).to eq(true)
end

it 'not visible for openstack provider' do
button.instance_variable_set(:@record, ems_openstack)
expect(button.visible?).to eq(false)
end
end
end
@@ -0,0 +1,24 @@
describe ApplicationHelper::Toolbar::EmsNetworksCenter do
let(:ems_network_vmdb) { described_class.definition['ems_network_vmdb'] }
let(:ems_network_vmdb_choice) { ems_network_vmdb.buttons.detect { |b| b[:id] == 'ems_network_vmdb_choice' } }

describe 'Add a New Network Provider' do
let(:button_hash) { ems_network_vmdb_choice[:items].detect { |b| b[:id] == 'ems_network_new' } }
let(:button_klass) { button_hash[:klass] }
let(:button) { button_klass.new(nil, nil, {}, {}) }

it 'appropriate button class' do
expect(button_klass).to eq(ApplicationHelper::Button::EmsNetworkNew)
end

it 'visible when nuage provider enabled' do
allow(::Settings.product).to receive(:nuage).and_return(true)
expect(button.visible?).to eq(true)
end

it 'hidden when nuage provider disabled' do
allow(::Settings.product).to receive(:nuage).and_return(false)
expect(button.visible?).to eq(false)
end
end
end