From a9359cd6ff10866acc943889ee77ab3f971dfa4e Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curoverse.com>
Date: Tue, 25 Aug 2015 15:10:57 -0400
Subject: [PATCH] 6781: Clarify "logins" vs. "get_all_logins" APIs by moving
 common code into render_logins_for.

---
 .../arvados/v1/virtual_machines_controller.rb  | 18 +++++++++++-------
 .../v1/virtual_machines_controller_test.rb     |  7 +++++++
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/services/api/app/controllers/arvados/v1/virtual_machines_controller.rb b/services/api/app/controllers/arvados/v1/virtual_machines_controller.rb
index 84251db470..e6474aa4e0 100644
--- a/services/api/app/controllers/arvados/v1/virtual_machines_controller.rb
+++ b/services/api/app/controllers/arvados/v1/virtual_machines_controller.rb
@@ -4,18 +4,22 @@ class Arvados::V1::VirtualMachinesController < ApplicationController
   before_filter(:admin_required,
                 :only => [:logins, :get_all_logins])
 
+  # Get all login permissons (user uuid, login account, SSH key) for a
+  # single VM
   def logins
-    get_all_logins
+    render_logins_for VirtualMachine.where(uuid: @object.uuid)
   end
 
+  # Get all login permissons for all VMs
   def get_all_logins
+    render_logins_for VirtualMachine
+  end
+
+  protected
+
+  def render_logins_for vm_query
     @response = []
-    @vms = VirtualMachine.eager_load :login_permissions
-    if @object
-      @vms = @vms.where uuid: @object.uuid
-    else
-      @vms = @vms.all
-    end
+    @vms = vm_query.eager_load :login_permissions
     @users = {}
     User.eager_load(:authorized_keys).
       where('users.uuid in (?)',
diff --git a/services/api/test/functional/arvados/v1/virtual_machines_controller_test.rb b/services/api/test/functional/arvados/v1/virtual_machines_controller_test.rb
index 7c3270c5c5..329bc1589a 100644
--- a/services/api/test/functional/arvados/v1/virtual_machines_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/virtual_machines_controller_test.rb
@@ -65,4 +65,11 @@ class Arvados::V1::VirtualMachinesControllerTest < ActionController::TestCase
     assert_equal u.uuid, json_response['items'][0]['user_uuid']
     assert_equal 'bobblogin', json_response['items'][0]['username']
   end
+
+  test 'get all logins' do
+    authorize_with :admin
+    get :get_all_logins
+    find_login :admin
+    find_login :active
+  end
 end
-- 
2.30.2