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
mzazrivec
merged 1 commit into
ManageIQ:master
from
miha-plesko:disable-edit-network-manager
Feb 15, 2018
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
end | ||
end |
26 changes: 26 additions & 0 deletions
26
spec/helpers/application_helper/toolbar/ems_network_center_spec.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
24 changes: 24 additions & 0 deletions
24
spec/helpers/application_helper/toolbar/ems_networks_center_spec.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.