Merge branch 'last-deploy-key' into 'master'

Delete deploy key when last connection to a project is destroyed.

Addresses #1959.

See merge request !1710

4 files changed, 44 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 1ee3b1a7d1b..fd80ad6a680 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -75,6 +75,7 @@ v 7.9.0 (unreleased)
   - Use same layout and interactivity for project members as group members.
   - Prevent gitlab-shell character encoding issues by receiving its changes as raw data.
   - Ability to unsubscribe/subscribe to issue or merge request
+  - Delete deploy key when last connection to a project is destroyed.
 
 v 7.8.4
   - Fix issue_tracker_id substitution in custom issue trackers
diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb
index b7cc305899c..679a5d76ec0 100644
--- a/app/controllers/projects/deploy_keys_controller.rb
+++ b/app/controllers/projects/deploy_keys_controller.rb
@@ -37,7 +37,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
     @key.destroy
 
     respond_to do |format|
-      format.html { redirect_to project_deploy_keys_url }
+      format.html { redirect_to namespace_project_deploy_keys_path(@project.namespace, @project) }
       format.js { render nothing: true }
     end
   end
@@ -50,7 +50,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
   end
 
   def disable
-    @project.deploy_keys_projects.where(deploy_key_id: params[:id]).last.destroy
+    @project.deploy_keys_projects.find_by(deploy_key_id: params[:id]).destroy
 
     redirect_to namespace_project_deploy_keys_path(@project.namespace,
                                                    @project)
diff --git a/app/models/deploy_keys_project.rb b/app/models/deploy_keys_project.rb
index f23d8205ddc..7e88903b9af 100644
--- a/app/models/deploy_keys_project.rb
+++ b/app/models/deploy_keys_project.rb
@@ -16,4 +16,12 @@ class DeployKeysProject < ActiveRecord::Base
   validates :deploy_key_id, presence: true
   validates :deploy_key_id, uniqueness: { scope: [:project_id], message: "already exists in project" }
   validates :project_id, presence: true
+
+  after_destroy :destroy_orphaned_deploy_key
+
+  private
+
+  def destroy_orphaned_deploy_key
+    self.deploy_key.destroy if self.deploy_key.deploy_keys_projects.length == 0
+  end
 end
diff --git a/spec/models/deploy_keys_project_spec.rb b/spec/models/deploy_keys_project_spec.rb
index aacd9bf38bf..f351aab9238 100644
--- a/spec/models/deploy_keys_project_spec.rb
+++ b/spec/models/deploy_keys_project_spec.rb
@@ -21,4 +21,37 @@ describe DeployKeysProject do
     it { is_expected.to validate_presence_of(:project_id) }
     it { is_expected.to validate_presence_of(:deploy_key_id) }
   end
+
+  describe "Destroying" do
+    let(:project)     { create(:project) }
+    subject           { create(:deploy_keys_project, project: project) }
+    let(:deploy_key)  { subject.deploy_key }
+
+    context "when the deploy key is only used by this project" do
+      it "destroys the deploy key" do
+        subject.destroy
+
+        expect {
+          deploy_key.reload
+        }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+
+    context "when the deploy key is used by more than one project" do
+
+      let!(:other_project) { create(:project) }
+
+      before do
+        other_project.deploy_keys << deploy_key
+      end
+
+      it "doesn't destroy the deploy key" do
+        subject.destroy
+
+        expect {
+          deploy_key.reload
+        }.not_to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
+  end
 end