Merge commit '8af23def1d6' into object-storage-ee-to-ce-backport

29 files changed, 157 insertions, 82 deletions

diff --git a/app/controllers/admin/deploy_keys_controller.rb b/app/controllers/admin/deploy_keys_controller.rb
index a7ab481519d..b0c4c31cffc 100644
--- a/app/controllers/admin/deploy_keys_controller.rb
+++ b/app/controllers/admin/deploy_keys_controller.rb
@@ -50,10 +50,10 @@ class Admin::DeployKeysController < Admin::ApplicationController
   end
 
   def create_params
-    params.require(:deploy_key).permit(:key, :title, :can_push)
+    params.require(:deploy_key).permit(:key, :title)
   end
 
   def update_params
-    params.require(:deploy_key).permit(:title, :can_push)
+    params.require(:deploy_key).permit(:title)
   end
 end
diff --git a/app/controllers/admin/gitaly_servers_controller.rb b/app/controllers/admin/gitaly_servers_controller.rb
new file mode 100644
index 00000000000..11c4dfe3d8d
--- /dev/null
+++ b/app/controllers/admin/gitaly_servers_controller.rb
@@ -0,0 +1,5 @@
+class Admin::GitalyServersController < Admin::ApplicationController
+  def index
+    @gitaly_servers = Gitaly::Server.all
+  end
+end
diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb
index 77e3c95d197..2b47819303e 100644
--- a/app/controllers/admin/hooks_controller.rb
+++ b/app/controllers/admin/hooks_controller.rb
@@ -59,11 +59,9 @@ class Admin::HooksController < Admin::ApplicationController
   def hook_params
     params.require(:hook).permit(
       :enable_ssl_verification,
-      :push_events,
-      :tag_push_events,
-      :repository_update_events,
       :token,
-      :url
+      :url,
+      *SystemHook.triggers.values
     )
   end
 end
diff --git a/app/controllers/admin/jobs_controller.rb b/app/controllers/admin/jobs_controller.rb
index 5162273ef8a..ae7a7f6279c 100644
--- a/app/controllers/admin/jobs_controller.rb
+++ b/app/controllers/admin/jobs_controller.rb
@@ -20,6 +20,6 @@ class Admin::JobsController < Admin::ApplicationController
   def cancel_all
     Ci::Build.running_or_pending.each(&:cancel)
 
-    redirect_to admin_jobs_path
+    redirect_to admin_jobs_path, status: 303
   end
 end
diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb
index 38b808cdc31..4b01904f2a1 100644
--- a/app/controllers/admin/runners_controller.rb
+++ b/app/controllers/admin/runners_controller.rb
@@ -65,6 +65,7 @@ class Admin::RunnersController < Admin::ApplicationController
       else
         Project.all
       end
+
     @projects = @projects.where.not(id: runner.projects.select(:id)) if runner.projects.any?
     @projects = @projects.page(params[:page]).per(30)
   end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index ee21d81f23e..95ad38d9230 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -147,6 +147,8 @@ class ApplicationController < ActionController::Base
       format.html do
         render file: Rails.root.join("public", "404"), layout: false, status: "404"
       end
+      # Prevent the Rails CSRF protector from thinking a missing .js file is a JavaScript file
+      format.js { render json: '', status: :not_found, content_type: 'application/json' }
       format.any { head :not_found }
     end
   end
diff --git a/app/controllers/concerns/group_tree.rb b/app/controllers/concerns/group_tree.rb
index b10147835f3..fafb10090ca 100644
--- a/app/controllers/concerns/group_tree.rb
+++ b/app/controllers/concerns/group_tree.rb
@@ -2,12 +2,17 @@ module GroupTree
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def render_group_tree(groups)
     @groups = if params[:filter].present?
-                Gitlab::GroupHierarchy.new(groups.search(params[:filter]))
+                # We find the ancestors by ID of the search results here.
+                # Otherwise the ancestors would also have filters applied,
+                # which would cause them not to be preloaded.
+                group_ids = groups.search(params[:filter]).select(:id)
+                Gitlab::GroupHierarchy.new(Group.where(id: group_ids))
                   .base_and_ancestors
               else
                 # Only show root groups if no parent-id is given
                 groups.where(parent_id: params[:parent_id])
               end
+
     @groups = @groups.with_selects_for_list(archived: params[:archived])
                 .sort(@sort = params[:sort])
                 .page(params[:page])
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index 74a4f437dc8..337957c366d 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -45,7 +45,7 @@ module IssuableActions
     }
 
     if issuable.edited?
-      response[:updated_at] = issuable.updated_at
+      response[:updated_at] = issuable.last_edited_at.to_time.iso8601
       response[:updated_by_name] = issuable.last_edited_by.name
       response[:updated_by_path] = user_path(issuable.last_edited_by)
     end
diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb
index b25e753a5ad..0d7ee06deb6 100644
--- a/app/controllers/concerns/issuable_collections.rb
+++ b/app/controllers/concerns/issuable_collections.rb
@@ -12,11 +12,9 @@ module IssuableCollections
 
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def set_issuables_index
-    @issuables          = issuables_collection
-    @issuables          = @issuables.page(params[:page])
-    @issuable_meta_data = issuable_meta_data(@issuables, collection_type)
-    @total_pages        = issuable_page_count
+    @issuables = issuables_collection
 
+    set_pagination
     return if redirect_out_of_range(@total_pages)
 
     if params[:label_name].present?
@@ -35,14 +33,26 @@ module IssuableCollections
       @users.push(author) if author
     end
   end
+
+  def set_pagination
+    return if pagination_disabled?
+
+    @issuables          = @issuables.page(params[:page])
+    @issuable_meta_data = issuable_meta_data(@issuables, collection_type)
+    @total_pages        = issuable_page_count
+  end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
+  def pagination_disabled?
+    false
+  end
+
   def issuables_collection
     finder.execute.preload(preload_for_collection)
   end
 
   def redirect_out_of_range(total_pages)
-    return false if total_pages.zero?
+    return false if total_pages.nil? || total_pages.zero?
 
     out_of_range = @issuables.current_page > total_pages # rubocop:disable Gitlab/ModuleWithInstanceVariables
 
@@ -84,6 +94,7 @@ module IssuableCollections
       @filter_params[:project_id] = @project.id
     elsif @group
       @filter_params[:group_id] = @group.id
+      @filter_params[:include_subgroups] = true
     else
       # TODO: this filter ignore issues/mr created in public or
       # internal repos where you are not a member. Enable this filter
diff --git a/app/controllers/concerns/routable_actions.rb b/app/controllers/concerns/routable_actions.rb
index 4199da9cdf5..f745deb083c 100644
--- a/app/controllers/concerns/routable_actions.rb
+++ b/app/controllers/concerns/routable_actions.rb
@@ -32,6 +32,7 @@ module RoutableActions
       if canonical_path.casecmp(requested_full_path) != 0
         flash[:notice] = "#{routable.class.to_s.titleize} '#{requested_full_path}' was moved to '#{canonical_path}'. Please update any links and bookmarks that may still have the old path."
       end
+
       redirect_to build_canonical_path(routable)
     end
   end
diff --git a/app/controllers/concerns/with_performance_bar.rb b/app/controllers/concerns/with_performance_bar.rb
index 230bbe4b1aa..6a8b1a4de7b 100644
--- a/app/controllers/concerns/with_performance_bar.rb
+++ b/app/controllers/concerns/with_performance_bar.rb
@@ -6,13 +6,22 @@ module WithPerformanceBar
   end
 
   def peek_enabled?
-    return true if Rails.env.development?
     return false unless Gitlab::PerformanceBar.enabled?(current_user)
 
     if RequestStore.active?
-      RequestStore.fetch(:peek_enabled) { cookies[:perf_bar_enabled].present? }
+      RequestStore.fetch(:peek_enabled) { cookie_or_default_value }
     else
-      cookies[:perf_bar_enabled].present?
+      cookie_or_default_value
+    end
+  end
+
+  private
+
+  def cookie_or_default_value
+    if cookies[:perf_bar_enabled].present?
+      cookies[:perf_bar_enabled] == 'true'
+    else
+      cookies[:perf_bar_enabled] = 'true' if Rails.env.development?
     end
   end
 end
diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb
index bc0948cd3fb..6d9c38d9581 100644
--- a/app/controllers/confirmations_controller.rb
+++ b/app/controllers/confirmations_controller.rb
@@ -17,7 +17,7 @@ class ConfirmationsController < Devise::ConfirmationsController
     else
       Gitlab::AppLogger.info("Email Confirmed: username=#{resource.username} email=#{resource.email} ip=#{request.remote_ip}")
       flash[:notice] += " Please sign in."
-      new_session_path(:user)
+      new_session_path(:user, anchor: 'login-pane')
     end
   end
 
diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index f013d21275e..acf6aaf57f4 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -75,8 +75,6 @@ class Groups::MilestonesController < Groups::ApplicationController
   end
 
   def milestones
-    search_params = params.merge(group_ids: group.id)
-
     milestones = MilestonesFinder.new(search_params).execute
     legacy_milestones = GroupMilestone.build_collection(group, group_projects, params)
 
@@ -94,4 +92,8 @@ class Groups::MilestonesController < Groups::ApplicationController
 
     render_404 unless @milestone
   end
+
+  def search_params
+    params.permit(:state).merge(group_ids: group.id)
+  end
 end
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index eb53a522f90..bb652832cb1 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -118,10 +118,10 @@ class GroupsController < Groups::ApplicationController
   end
 
   def group_params
-    params.require(:group).permit(group_params_ce)
+    params.require(:group).permit(group_params_attributes)
   end
 
-  def group_params_ce
+  def group_params_attributes
     [
       :avatar,
       :description,
@@ -150,7 +150,6 @@ class GroupsController < Groups::ApplicationController
     @projects = GroupProjectsFinder.new(params: params, group: group, options: options, current_user: current_user)
                   .execute
                   .includes(:namespace)
-                  .page(params[:page])
 
     @events = EventCollection
       .new(@projects, offset: params[:offset].to_i, filter: event_filter)
diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb
index a931b456a93..16abf7bab7e 100644
--- a/app/controllers/health_controller.rb
+++ b/app/controllers/health_controller.rb
@@ -8,7 +8,8 @@ class HealthController < ActionController::Base
     Gitlab::HealthChecks::Redis::CacheCheck,
     Gitlab::HealthChecks::Redis::QueuesCheck,
     Gitlab::HealthChecks::Redis::SharedStateCheck,
-    Gitlab::HealthChecks::FsShardsCheck
+    Gitlab::HealthChecks::FsShardsCheck,
+    Gitlab::HealthChecks::GitalyCheck
   ].freeze
 
   def readiness
diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb
index 38f379dbf4f..a394521698c 100644
--- a/app/controllers/help_controller.rb
+++ b/app/controllers/help_controller.rb
@@ -5,7 +5,7 @@ class HelpController < ApplicationController
 
   # Taken from Jekyll
   # https://github.com/jekyll/jekyll/blob/3.5-stable/lib/jekyll/document.rb#L13
-  YAML_FRONT_MATTER_REGEXP = %r!\A(---\s*\n.*?\n?)^((---|\.\.\.)\s*$\n?)!m
+  YAML_FRONT_MATTER_REGEXP = /\A(---\s*\n.*?\n?)^((---|\.\.\.)\s*$\n?)/m
 
   def index
     # Remove YAML frontmatter so that it doesn't look weird
diff --git a/app/controllers/metrics_controller.rb b/app/controllers/metrics_controller.rb
index d81ad135198..33b682d2859 100644
--- a/app/controllers/metrics_controller.rb
+++ b/app/controllers/metrics_controller.rb
@@ -12,6 +12,7 @@ class MetricsController < ActionController::Base
                                           )
                  "# Metrics are disabled, see: #{help_page}\n"
                end
+
     render text: response, content_type: 'text/plain; version=0.0.4'
   end
 
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index e3c18cba1dd..d631d09f1b8 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -83,6 +83,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     if ticket
       handle_service_ticket oauth['provider'], ticket
     end
+
     handle_omniauth
   end
 
@@ -90,6 +91,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     if params['sid']
       handle_service_ticket oauth['provider'], params['sid']
     end
+
     handle_omniauth
   end
 
@@ -110,6 +112,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
 
       continue_login_process
     end
+  rescue Gitlab::OAuth::SigninDisabledForProviderError
+    handle_disabled_provider
   rescue Gitlab::OAuth::SignupDisabledError
     handle_signup_error
   end
@@ -124,6 +128,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     # Only allow properly saved users to login.
     if @user.persisted? && @user.valid?
       log_audit_event(@user, with: oauth['provider'])
+
       if @user.two_factor_enabled?
         params[:remember_me] = '1' if remember_me?
         prompt_for_two_factor(@user)
@@ -165,6 +170,13 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     redirect_to new_user_session_path
   end
 
+  def handle_disabled_provider
+    label = Gitlab::OAuth::Provider.label_for(oauth['provider'])
+    flash[:alert] = "Signing in using #{label} has been disabled"
+
+    redirect_to new_user_session_path
+  end
+
   def log_audit_event(user, options = {})
     AuditEventService.new(user, user, options)
       .for_authentication.security_event
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index d838b8dc29e..35e67730a27 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -150,6 +150,7 @@ class Projects::BlobController < Projects::ApplicationController
         if params[:file].present?
           params[:file_name] = params[:file].original_filename
         end
+
         File.join(@path, params[:file_name])
       elsif params[:file_path].present?
         params[:file_path]
diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb
index 25608df0b9c..4fc515bd03e 100644
--- a/app/controllers/projects/clusters/gcp_controller.rb
+++ b/app/controllers/projects/clusters/gcp_controller.rb
@@ -1,8 +1,9 @@
 class Projects::Clusters::GcpController < Projects::ApplicationController
   before_action :authorize_read_cluster!
   before_action :authorize_google_api, except: [:login]
-  before_action :authorize_google_project_billing, only: [:new]
+  before_action :authorize_google_project_billing, only: [:new, :create]
   before_action :authorize_create_cluster!, only: [:new, :create]
+  before_action :verify_billing, only: [:create]
 
   def login
     begin
@@ -23,24 +24,34 @@ class Projects::Clusters::GcpController < Projects::ApplicationController
   end
 
   def create
+    @cluster = ::Clusters::CreateService
+      .new(project, current_user, create_params)
+      .execute(token_in_session)
+
+    if @cluster.persisted?
+      redirect_to project_cluster_path(project, @cluster)
+    else
+      render :new
+    end
+  end
+
+  private
+
+  def verify_billing
     case google_project_billing_status
     when 'true'
-      @cluster = ::Clusters::CreateService
-        .new(project, current_user, create_params)
-        .execute(token_in_session)
-
-      return redirect_to project_cluster_path(project, @cluster) if @cluster.persisted?
+      return
     when 'false'
-      flash[:error] = _('Please enable billing for one of your projects to be able to create a cluster.')
+      flash[:alert] = _('Please <a href=%{link_to_billing} target="_blank" rel="noopener noreferrer">enable billing for one of your projects to be able to create a cluster</a>, then try again.').html_safe % { link_to_billing: "https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral" }
     else
-      flash[:error] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.')
+      flash[:alert] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.')
     end
 
+    @cluster = ::Clusters::Cluster.new(create_params)
+
     render :new
   end
 
-  private
-
   def create_params
     params.require(:cluster).permit(
       :enabled,
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index 2e7344b1cad..effb484ef0f 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -12,7 +12,7 @@ class Projects::CommitController < Projects::ApplicationController
   before_action :authorize_download_code!
   before_action :authorize_read_pipeline!, only: [:pipelines]
   before_action :commit
-  before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines]
+  before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines, :merge_requests]
   before_action :define_note_vars, only: [:show, :diff_for_path]
   before_action :authorize_edit_tree!, only: [:revert, :cherry_pick]
 
@@ -52,6 +52,18 @@ class Projects::CommitController < Projects::ApplicationController
     end
   end
 
+  def merge_requests
+    @merge_requests = @commit.merge_requests.map do |mr|
+      { iid: mr.iid, path: merge_request_path(mr), title: mr.title }
+    end
+
+    respond_to do |format|
+      format.json do
+        render json: @merge_requests.to_json
+      end
+    end
+  end
+
   def branches
     # branch_names_contains/tag_names_contains can take a long time when there are thousands of
     # branches/tags - each `git branch --contains xxx` request can consume a cpu core.
diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb
index 026708169f4..0a40c67368f 100644
--- a/app/controllers/projects/commits_controller.rb
+++ b/app/controllers/projects/commits_controller.rb
@@ -13,31 +13,37 @@ class Projects::CommitsController < Projects::ApplicationController
     @merge_request = MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened
       .find_by(source_project: @project, source_branch: @ref, target_branch: @repository.root_ref)
 
-    respond_to do |format|
-      format.html
-      format.atom { render layout: 'xml.atom' }
-
-      format.json do
-        pager_json(
-          'projects/commits/_commits',
-          @commits.size,
-          project: @project,
-          ref: @ref)
+    # https://gitlab.com/gitlab-org/gitaly/issues/931
+    Gitlab::GitalyClient.allow_n_plus_1_calls do
+      respond_to do |format|
+        format.html
+        format.atom { render layout: 'xml.atom' }
+
+        format.json do
+          pager_json(
+            'projects/commits/_commits',
+            @commits.size,
+            project: @project,
+            ref: @ref)
+        end
       end
     end
   end
 
   def signatures
-    respond_to do |format|
-      format.json do
-        render json: {
-          signatures: @commits.select(&:has_signature?).map do |commit|
-            {
-              commit_sha: commit.sha,
-              html: view_to_html_string('projects/commit/_signature', signature: commit.signature)
-            }
-          end
-        }
+    # https://gitlab.com/gitlab-org/gitaly/issues/931
+    Gitlab::GitalyClient.allow_n_plus_1_calls do
+      respond_to do |format|
+        format.json do
+          render json: {
+            signatures: @commits.select(&:has_signature?).map do |commit|
+              {
+                commit_sha: commit.sha,
+                html: view_to_html_string('projects/commit/_signature', signature: commit.signature)
+              }
+            end
+          }
+        end
       end
     end
   end
diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb
index cf8829ba95b..f43ef2e5f2f 100644
--- a/app/controllers/projects/deploy_keys_controller.rb
+++ b/app/controllers/projects/deploy_keys_controller.rb
@@ -24,9 +24,10 @@ class Projects::DeployKeysController < Projects::ApplicationController
   def create
     @key = DeployKeys::CreateService.new(current_user, create_params).execute
 
-    unless @key.valid? && @project.deploy_keys << @key
+    unless @key.valid?
       flash[:alert] = @key.errors.full_messages.join(', ').html_safe
     end
+
     redirect_to_repository_settings(@project)
   end
 
@@ -70,11 +71,14 @@ class Projects::DeployKeysController < Projects::ApplicationController
   end
 
   def create_params
-    params.require(:deploy_key).permit(:key, :title, :can_push)
+    create_params = params.require(:deploy_key)
+                          .permit(:key, :title, deploy_keys_projects_attributes: [:can_push])
+    create_params.dig(:deploy_keys_projects_attributes, '0')&.merge!(project_id: @project.id)
+    create_params
   end
 
   def update_params
-    params.require(:deploy_key).permit(:title, :can_push)
+    params.require(:deploy_key).permit(:title, deploy_keys_projects_attributes: [:id, :can_push])
   end
 
   def authorize_update_deploy_key!
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 85d35900c71..dd7aa1a67b9 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -21,6 +21,7 @@ class Projects::HooksController < Projects::ApplicationController
       @hooks = @project.hooks.select(&:persisted?)
       flash[:alert] = @hook.errors.full_messages.join.html_safe
     end
+
     redirect_to project_settings_integrations_path(@project)
   end
 
@@ -63,18 +64,10 @@ class Projects::HooksController < Projects::ApplicationController
 
   def hook_params
     params.require(:hook).permit(
-      :job_events,
-      :pipeline_events,
       :enable_ssl_verification,
-      :issues_events,
-      :confidential_issues_events,
-      :merge_requests_events,
-      :note_events,
-      :push_events,
-      :tag_push_events,
       :token,
       :url,
-      :wiki_page_events
+      *ProjectHook.triggers.values
     )
   end
 end
diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb
index 221b31430d8..8041bdbb0bd 100644
--- a/app/controllers/projects/jobs_controller.rb
+++ b/app/controllers/projects/jobs_controller.rb
@@ -31,7 +31,7 @@ class Projects::JobsController < Projects::ApplicationController
       :project,
       :tags
     ])
-    @builds = @builds.page(params[:page]).per(30)
+    @builds = @builds.page(params[:page]).per(30).without_count
   end
 
   def cancel_all
diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb
index dc524b790a0..0df80fa700f 100644
--- a/app/controllers/projects/merge_requests/creations_controller.rb
+++ b/app/controllers/projects/merge_requests/creations_controller.rb
@@ -43,11 +43,8 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
   end
 
   def diffs
-    @diffs = if @merge_request.can_be_created
-               @merge_request.diffs(diff_options)
-             else
-               []
-             end
+    @diffs = @merge_request.diffs(diff_options) if @merge_request.can_be_created
+
     @diff_notes_disabled = true
 
     @environment = @merge_request.environments_for(current_user).last
diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb
index 980bbf699b6..75b17d05e22 100644
--- a/app/controllers/projects/milestones_controller.rb
+++ b/app/controllers/projects/milestones_controller.rb
@@ -83,7 +83,7 @@ class Projects::MilestonesController < Projects::ApplicationController
     Milestones::DestroyService.new(project, current_user).execute(milestone)
 
     respond_to do |format|
-      format.html { redirect_to namespace_project_milestones_path, status: 302 }
+      format.html { redirect_to namespace_project_milestones_path, status: 303 }
       format.js { head :ok }
     end
   end
@@ -92,12 +92,6 @@ class Projects::MilestonesController < Projects::ApplicationController
 
   def milestones
     @milestones ||= begin
-      if @project.group && can?(current_user, :read_group, @project.group)
-        group = @project.group
-      end
-
-      search_params = params.merge(project_ids: @project.id, group_ids: group&.id)
-
       MilestonesFinder.new(search_params).execute
     end
   end
@@ -113,4 +107,12 @@ class Projects::MilestonesController < Projects::ApplicationController
   def milestone_params
     params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event)
   end
+
+  def search_params
+    if @project.group && can?(current_user, :read_group, @project.group)
+      group = @project.group
+    end
+
+    params.permit(:state).merge(project_ids: @project.id, group_ids: group&.id)
+  end
 end
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 6f229b08c0c..8158934322d 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -203,6 +203,7 @@ class ProjectsController < Projects::ApplicationController
     else
       flash[:alert] = _("Project export could not be deleted.")
     end
+
     redirect_to(edit_project_path(@project))
   end
 
@@ -402,6 +403,6 @@ class ProjectsController < Projects::ApplicationController
     # to
     #   localhost/group/project
     #
-    redirect_to request.original_url.sub(/\.git\/?\Z/, '') if params[:format] == 'git'
+    redirect_to request.original_url.sub(%r{\.git/?\Z}, '') if params[:format] == 'git'
   end
 end
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index d79108c88fb..c73306a6b66 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -28,6 +28,7 @@ class SessionsController < Devise::SessionsController
         resource.update_attributes(reset_password_token: nil,
                                    reset_password_sent_at: nil)
       end
+
       # hide the signed-in notification
       flash[:notice] = nil
       log_audit_event(current_user, resource, with: authentication_method)