Clean up code.

7 files changed, 48 insertions, 27 deletions

diff --git a/app/controllers/admin/deploy_keys_controller.rb b/app/controllers/admin/deploy_keys_controller.rb
index 0664b26ddda..e93603bef36 100644
--- a/app/controllers/admin/deploy_keys_controller.rb
+++ b/app/controllers/admin/deploy_keys_controller.rb
@@ -1,8 +1,9 @@
 class Admin::DeployKeysController < Admin::ApplicationController
+  before_filter :deploy_keys, only: [:index]
   before_filter :deploy_key, only: [:show, :destroy]
 
   def index
-    @deploy_keys = DeployKey.are_public
+
   end
 
   def show
@@ -10,12 +11,11 @@ class Admin::DeployKeysController < Admin::ApplicationController
   end
 
   def new
-    @deploy_key = DeployKey.new(public: true)
+    @deploy_key = deploy_keys.new
   end
 
   def create
-    @deploy_key = DeployKey.new(deploy_key_params)
-    @deploy_key.public = true
+    @deploy_key = deploy_keys.new(deploy_key_params)
 
     if @deploy_key.save
       redirect_to admin_deploy_keys_path
@@ -36,7 +36,11 @@ class Admin::DeployKeysController < Admin::ApplicationController
   protected
 
   def deploy_key
-    @deploy_key ||= DeployKey.find(params[:id])
+    @deploy_key ||= deploy_keys.find(params[:id])
+  end
+
+  def deploy_keys
+    @deploy_keys ||= DeployKey.are_public
   end
 
   def deploy_key_params
diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb
index 176d112a4d7..6fba3ce299b 100644
--- a/app/controllers/projects/deploy_keys_controller.rb
+++ b/app/controllers/projects/deploy_keys_controller.rb
@@ -8,9 +8,14 @@ class Projects::DeployKeysController < Projects::ApplicationController
 
   def index
     @enabled_keys = @project.deploy_keys
-    @available_keys = available_keys - @enabled_keys
+
+    @available_keys         = accessible_keys - @enabled_keys
     @available_project_keys = current_user.project_deploy_keys - @enabled_keys
-    @available_public_keys = DeployKey.are_public - @available_project_keys - @enabled_keys
+    @available_public_keys  = DeployKey.are_public - @enabled_keys
+
+    # Public keys that are already used by another accessible project are already
+    # in @available_project_keys.
+    @available_public_keys -= @available_project_keys
   end
 
   def show
@@ -35,7 +40,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
   end
 
   def enable
-    @key = current_user.accessible_deploy_keys.find(params[:id])
+    @key = accessible_keys.find(params[:id])
     @project.deploy_keys << @key
 
     redirect_to namespace_project_deploy_keys_path(@project.namespace,
@@ -50,8 +55,8 @@ class Projects::DeployKeysController < Projects::ApplicationController
 
   protected
 
-  def available_keys
-    @available_keys ||= current_user.accessible_deploy_keys
+  def accessible_keys
+    @accessible_keys ||= current_user.accessible_deploy_keys
   end
 
   def deploy_key_params
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index e3734023be3..ebbd2bfd77d 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -127,6 +127,14 @@ module ProjectsHelper
     html + count_html
   end
 
+  def project_for_deploy_key(deploy_key)
+    if deploy_key.projects.include?(@project)
+      @project
+    else
+      deploy_key.projects.find { |project| can?(current_user, :read_project, project) }
+    end
+  end
+
   private
 
   def get_project_nav_tabs(project, current_user)
diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb
index 1770dde320f..85d52d558cd 100644
--- a/app/models/deploy_key.rb
+++ b/app/models/deploy_key.rb
@@ -19,9 +19,20 @@ class DeployKey < Key
 
   scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
   scope :are_public,  -> { where(public: true) }
-  scope :are_private, -> { where(public: false) }
 
   def private?
     !public?
   end
+
+  def orphaned?
+    self.deploy_keys_projects.length == 0
+  end
+
+  def almost_orphaned?
+    self.deploy_keys_projects.length == 1
+  end
+
+  def destroyed_when_orphaned?
+    self.private?
+  end
 end
diff --git a/app/models/deploy_keys_project.rb b/app/models/deploy_keys_project.rb
index 5f679f4b3ef..18db521741f 100644
--- a/app/models/deploy_keys_project.rb
+++ b/app/models/deploy_keys_project.rb
@@ -22,10 +22,8 @@ class DeployKeysProject < ActiveRecord::Base
   private
 
   def destroy_orphaned_deploy_key
-    # Public deploy keys are never automatically deleted
-    return if self.deploy_key.public?
-    return if self.deploy_key.deploy_keys_projects.length > 0
-
+    return unless self.deploy_key.destroyed_when_orphaned? && self.deploy_key.orphaned?
+    
     self.deploy_key.destroy
   end
 end
diff --git a/app/views/projects/deploy_keys/_deploy_key.html.haml b/app/views/projects/deploy_keys/_deploy_key.html.haml
index 8837a65918c..c577dfa8d55 100644
--- a/app/views/projects/deploy_keys/_deploy_key.html.haml
+++ b/app/views/projects/deploy_keys/_deploy_key.html.haml
@@ -5,21 +5,15 @@
         %i.fa.fa-plus
         Enable
     - else
-      - if deploy_key.projects.count > 1 || deploy_key.public?
+      - if deploy_key.destroyed_when_orphaned? && deploy_key.almost_orphaned?
+        = link_to 'Remove', disable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), data: { confirm: 'You are going to remove deploy key. Are you sure?'}, method: :put, class: "btn btn-remove delete-key btn-sm pull-right"
+      - else
         = link_to disable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), class: 'btn btn-sm', method: :put do
           %i.fa.fa-power-off
           Disable
-      - else
-        = link_to 'Remove', disable_namespace_project_deploy_key_path(@project.namespace, @project, deploy_key), data: { confirm: 'You are going to remove deploy key. Are you sure?'}, method: :put, class: "btn btn-remove delete-key btn-sm pull-right"
-
 
-  - if deploy_key.projects.include?(@project)
-    - key_project = @project
-  - else
-    - key_project = deploy_key.projects.find { |project| can?(current_user, :read_project, project) }
-    
-  - if key_project
-    = link_to namespace_project_deploy_key_path(key_project.namespace, key_project, deploy_key) do
+  - if project = project_for_deploy_key(deploy_key)
+    = link_to namespace_project_deploy_key_path(project.namespace, project, deploy_key) do
       %i.fa.fa-key
       %strong= deploy_key.title
   - else
diff --git a/app/views/projects/deploy_keys/index.html.haml b/app/views/projects/deploy_keys/index.html.haml
index 186cdfeea33..472a13a8524 100644
--- a/app/views/projects/deploy_keys/index.html.haml
+++ b/app/views/projects/deploy_keys/index.html.haml
@@ -22,7 +22,8 @@
         .light-well
           .nothing-here-block Create a #{link_to 'new deploy key', new_namespace_project_deploy_key_path(@project.namespace, @project)} or add an existing one
   .col-md-6.available-keys
-    - unless @available_project_keys.blank? && @available_public_keys.any?
+    - # If there are available public deploy keys but no available project deploy keys, only public deploy keys are shown.
+    - if @available_project_keys.any? || @available_public_keys.blank?
       %h5
         %strong Deploy keys
         from projects you have access to