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

Fixes #5707 - disassociate does not appear for physical host #3286

Closed
wants to merge 1 commit into from

Conversation

orrabin
Copy link
Member

@orrabin orrabin commented Mar 7, 2016

No description provided.

@@ -648,6 +650,10 @@ def associate!(cr, vm)
self.save!(:validate => false) # don't want to trigger callbacks
end

def can_disassociate?
Copy link
Member

Choose a reason for hiding this comment

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

I think this method is equivalent to compute?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is, I was confused with the compute? in managed.rb which is actually only for unattended.

@dLobatog
Copy link
Member

dLobatog commented Mar 7, 2016

@orrabin 👍 , works well. Can you add some tests?

@orrabin
Copy link
Member Author

orrabin commented Mar 7, 2016

@dLobatog thanks, I'll add tests after I extract to a helper.


<%= form_tag update_multiple_disassociate_hosts_path({:host_ids => params[:host_ids]}) do %>
<small><%= _('Disassociate the selected hosts from their VMs') %></small>
<br>
<% search = '' %>
<% physical_hosts.each { |host| search += "name = #{host.name} OR " } %>
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: there is an operator just for this you can use:

"name ^ (#{physical_hosts.join(',')})"

@dLobatog
Copy link
Member

dLobatog commented Mar 8, 2016

Merged as 5a9ad12, thanks @orrabin!

@dLobatog dLobatog closed this Mar 8, 2016
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.

4 participants