Skip to content

Commit

Permalink
Merge pull request #13437 from mansam/backport-openstack-volume-attac…
Browse files Browse the repository at this point in the history
…h-detachment-ux-improvements

[EUWE] Backport UX improvements for attaching Openstack cloud volumes to instances
  • Loading branch information
simaishi authored Jan 17, 2017
2 parents d7c7dd1 + 014a286 commit 201c88f
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 14 deletions.
2 changes: 1 addition & 1 deletion app/controllers/vm_cloud_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def attach
assert_privileges("instance_attach")
@volume_choices = {}
@record = @vm = find_by_id_filtered(VmCloud, params[:id])
@vm.cloud_tenant.cloud_volumes.each { |volume| @volume_choices[volume.name] = volume.id }
@vm.cloud_tenant.cloud_volumes.where(:status => 'available').each { |v| @volume_choices[v.name] = v.id }

@in_a_form = true
drop_breadcrumb(
Expand Down
13 changes: 13 additions & 0 deletions app/helpers/application_helper/button/instance_attach.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class ApplicationHelper::Button::InstanceAttach < ApplicationHelper::Button::Basic
def calculate_properties
super
self[:title] = @error_message if disabled?
end

def disabled?
if @record.cloud_tenant.cloud_volumes.where(:status => 'available').count.zero?
@error_message = _("There are no Cloud Volumes available to attach to this Instance.")
end
@error_message.present?
end
end
13 changes: 5 additions & 8 deletions app/helpers/application_helper/button/instance_detach.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
class ApplicationHelper::Button::InstanceDetach < ApplicationHelper::Button::Basic
def calculate_properties
super
if @record.number_of(:cloud_volumes) == 0
self[:title] = _("%{model} \"%{name}\" has no attached %{volumes}") % {
:model => ui_lookup(:table => 'vm_cloud'),
:name => @record.name,
:volumes => ui_lookup(:tables => 'cloud_volumes')
}
end
self[:title] = @error_message if disabled?
end

def disabled?
@record.number_of(:cloud_volumes) == 0
if @record.number_of(:cloud_volumes).zero?
@error_message = _("This Instance has no attached Cloud Volumes.")
end
@error_message.present?
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ class ApplicationHelper::Toolbar::OpenstackVmCloudCenter < ApplicationHelper::To
:instance_attach,
'pficon pficon-volume fa-lg',
t = N_('Attach a Cloud Volume to this Instance'),
t),
t,
:klass => ApplicationHelper::Button::InstanceAttach),
button(
:instance_detach,
'pficon pficon-volume fa-lg',
Expand Down
44 changes: 44 additions & 0 deletions spec/helpers/application_helper/buttons/instance_attach_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
describe ApplicationHelper::Button::InstanceAttach do
describe '#disabled?' do
it "when there are available volumes, then the button is enabled" do
view_context = setup_view_context_with_sandbox({})

tenant = FactoryGirl.create(:cloud_tenant_openstack)
volume = FactoryGirl.create(:cloud_volume_openstack, :cloud_tenant => tenant, :status => 'available')
record = FactoryGirl.create(:vm_openstack, :cloud_tenant => tenant)
button = described_class.new(view_context, {}, {"record" => record}, {})
expect(button.disabled?).to be false
end

it "when there are no available volumes then the button is disabled" do
view_context = setup_view_context_with_sandbox({})
tenant = FactoryGirl.create(:cloud_tenant_openstack)
volume = FactoryGirl.create(:cloud_volume_openstack, :cloud_tenant => tenant, :status => 'in-use')
record = FactoryGirl.create(:vm_openstack, :cloud_tenant => tenant)
button = described_class.new(view_context, {}, {"record" => record}, {})
expect(button.disabled?).to be true
end
end

describe '#calculate_properties' do
it "when there are no available volumes then the button has the error in the title" do
view_context = setup_view_context_with_sandbox({})
tenant = FactoryGirl.create(:cloud_tenant_openstack)
volume = FactoryGirl.create(:cloud_volume_openstack, :cloud_tenant => tenant, :status => 'in-use')
record = FactoryGirl.create(:vm_openstack, :cloud_tenant => tenant)
button = described_class.new(view_context, {}, {"record" => record}, {})
button.calculate_properties
expect(button[:title]).to eq(_("There are no Cloud Volumes available to attach to this Instance."))
end

it "when there are available volumes, the button has no error in the title" do
view_context = setup_view_context_with_sandbox({})
tenant = FactoryGirl.create(:cloud_tenant_openstack)
volume = FactoryGirl.create(:cloud_volume_openstack, :cloud_tenant => tenant, :status => 'available')
record = FactoryGirl.create(:vm_openstack, :cloud_tenant => tenant)
button = described_class.new(view_context, {}, {"record" => record}, {})
button.calculate_properties
expect(button[:title]).to be nil
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@
)}, {}
)
button.calculate_properties
expect(button[:title]).to eq(_("%{model} \"TestVM\" has no attached %{volumes}") % {
:model => ui_lookup(:table => 'vm_cloud'),
:volumes => ui_lookup(:tables => 'cloud_volumes')
})
expect(button[:title]).to eq(_("This Instance has no attached Cloud Volumes."))
end

it "when there are volumes to detach, the button has no error in the title" do
Expand Down

0 comments on commit 201c88f

Please sign in to comment.