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

Improve UX for attaching Openstack cloud volumes to instances #110

Conversation

mansam
Copy link
Contributor

@mansam mansam commented Jan 9, 2017

This PR causes the Attach Volume to Instance form to only list volumes that are in an available state, and disables the Attach Volume to Instance button entirely when there are no volumes available to an instance. It also cleans up some tooltip wording for Detach to match other tooltips.

Ported over from ManageIQ/manageiq#13126

Links

@miq-bot
Copy link
Member

miq-bot commented Jan 9, 2017

Checked commits mansam/manageiq-ui-classic@321c64f~...04d224b with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
6 files checked, 4 offenses detected

spec/helpers/application_helper/buttons/instance_attach_spec.rb

@tzumainn
Copy link
Contributor

@miq-bot add_label euwe/yes
@miq-bot add_label blocker
@miq-bot add_label bug

@mansam mansam closed this Jan 10, 2017
@mansam mansam reopened this Jan 10, 2017
@mansam mansam closed this Jan 10, 2017
@mansam mansam reopened this Jan 10, 2017
@mzazrivec mzazrivec added this to the Sprint 52 Ending Jan 16, 2017 milestone Jan 16, 2017
@mzazrivec mzazrivec merged commit ac675e7 into ManageIQ:master Jan 16, 2017
@simaishi
Copy link
Contributor

Euwe backkport conflict:

$ git cherry-pick -x -e -m 1 -Xsubtree=/ ac675e7
error: could not apply ac675e7... Merge pull request #110 from mansam/only-list-cloud-volumes-that-are-available-to-attach
$ git diff
diff --cc app/helpers/application_helper/button/instance_detach.rb
index cb83ac0,cd7fce1..0000000
--- a/app/helpers/application_helper/button/instance_detach.rb
+++ b/app/helpers/application_helper/button/instance_detach.rb
@@@ -1,16 -1,8 +1,22 @@@
  class ApplicationHelper::Button::InstanceDetach < ApplicationHelper::Button::Basic
++<<<<<<< HEAD
 +  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')
 +      }
++=======
+   def disabled?
+     if @record.number_of(:cloud_volumes).zero?
+       @error_message = _("This Instance has no attached Cloud Volumes.")
++>>>>>>> ac675e7... Merge pull request #110 from mansam/only-list-cloud-volumes-that-are-available-to-attach
      end
 -    @error_message.present?
 +  end
 +
 +  def disabled?
 +    @record.number_of(:cloud_volumes) == 0
    end
  end

@simaishi
Copy link
Contributor

@mansam Please resolve conflict and make Euwe-specific PR (referencing this one) or suggest other PRs to backport.

@mansam
Copy link
Contributor Author

mansam commented Jan 16, 2017

@simaishi ManageIQ/manageiq#13437 should do the job.

@simaishi
Copy link
Contributor

Backported to Euwe via ManageIQ/manageiq#13437

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.

5 participants