From ed097df62758880eb6412770f661266d4a6e9299 Mon Sep 17 00:00:00 2001 From: Douwe Maan <douwe@gitlab.com> Date: Fri, 3 Apr 2015 12:22:44 +0200 Subject: [PATCH] Clean up code. --- app/controllers/admin/deploy_keys_controller.rb | 14 +++++++++----- .../projects/deploy_keys_controller.rb | 15 ++++++++++----- app/helpers/projects_helper.rb | 8 ++++++++ app/models/deploy_key.rb | 13 ++++++++++++- app/models/deploy_keys_project.rb | 6 ++---- .../projects/deploy_keys/_deploy_key.html.haml | 16 +++++----------- app/views/projects/deploy_keys/index.html.haml | 3 ++- 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 -- 2.30.9