Merge branch 'master' into members-ui

9 files changed, 92 insertions, 51 deletions

diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index d5a8a962662..4c497711fc0 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -23,15 +23,24 @@ module AuthenticatesWithTwoFactor
   #
   # Returns nil
   def prompt_for_two_factor(user)
+    return locked_user_redirect(user) if user.access_locked?
+
     session[:otp_user_id] = user.id
     setup_u2f_authentication(user)
     render 'devise/sessions/two_factor'
   end
 
+  def locked_user_redirect(user)
+    flash.now[:alert] = 'Invalid Login or password'
+    render 'devise/sessions/new'
+  end
+
   def authenticate_with_two_factor
     user = self.resource = find_user
 
-    if user_params[:otp_attempt].present? && session[:otp_user_id]
+    if user.access_locked?
+      locked_user_redirect(user)
+    elsif user_params[:otp_attempt].present? && session[:otp_user_id]
       authenticate_with_two_factor_via_otp(user)
     elsif user_params[:device_response].present? && session[:otp_user_id]
       authenticate_with_two_factor_via_u2f(user)
@@ -50,8 +59,9 @@ module AuthenticatesWithTwoFactor
       remember_me(user) if user_params[:remember_me] == '1'
       sign_in(user)
     else
+      user.increment_failed_attempts!
       flash.now[:alert] = 'Invalid two-factor code.'
-      render :two_factor
+      prompt_for_two_factor(user)
     end
   end
 
@@ -65,6 +75,7 @@ module AuthenticatesWithTwoFactor
       remember_me(user) if user_params[:remember_me] == '1'
       sign_in(user)
     else
+      user.increment_failed_attempts!
       flash.now[:alert] = 'Authentication via U2F device failed.'
       prompt_for_two_factor(user)
     end
diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb
index b8ed2c159a7..c13333641d3 100644
--- a/app/controllers/concerns/membership_actions.rb
+++ b/app/controllers/concerns/membership_actions.rb
@@ -15,18 +15,17 @@ module MembershipActions
   end
 
   def leave
-    @member = membershipable.members.find_by(user_id: current_user) ||
-      membershipable.requesters.find_by(user_id: current_user)
-    Members::DestroyService.new(@member, current_user).execute
+    member = Members::DestroyService.new(membershipable, current_user, user_id: current_user.id).
+      execute(:all)
 
-    source_type = @member.real_source_type.humanize(capitalize: false)
+    source_type = membershipable.class.to_s.humanize(capitalize: false)
     notice =
-      if @member.request?
+      if member.request?
         "Your access request to the #{source_type} has been withdrawn."
       else
-        "You left the \"#{@member.source.human_name}\" #{source_type}."
+        "You left the \"#{membershipable.human_name}\" #{source_type}."
       end
-    redirect_path = @member.request? ? @member.source : [:dashboard, @member.real_source_type.tableize]
+    redirect_path = member.request? ? member.source : [:dashboard, membershipable.class.to_s.tableize]
 
     redirect_to redirect_path, notice: notice
   end
diff --git a/app/controllers/explore/projects_controller.rb b/app/controllers/explore/projects_controller.rb
index 88a0c18180b..38e5943eb76 100644
--- a/app/controllers/explore/projects_controller.rb
+++ b/app/controllers/explore/projects_controller.rb
@@ -21,7 +21,7 @@ class Explore::ProjectsController < Explore::ApplicationController
   end
 
   def trending
-    @projects = TrendingProjectsFinder.new.execute(current_user)
+    @projects = TrendingProjectsFinder.new.execute
     @projects = filter_projects(@projects)
     @projects = @projects.page(params[:page])
 
diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index 9c323d7705a..18cd800c619 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -40,10 +40,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
   end
 
   def destroy
-    @group_member = @group.members.find_by(id: params[:id]) ||
-      @group.requesters.find_by(id: params[:id])
-
-    Members::DestroyService.new(@group_member, current_user).execute
+    Members::DestroyService.new(@group, current_user, id: params[:id]).execute(:all)
 
     respond_to do |format|
       format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' }
diff --git a/app/controllers/projects/boards_controller.rb b/app/controllers/projects/boards_controller.rb
index 33206717089..0035633b774 100644
--- a/app/controllers/projects/boards_controller.rb
+++ b/app/controllers/projects/boards_controller.rb
@@ -1,4 +1,6 @@
 class Projects::BoardsController < Projects::ApplicationController
+  include IssuableCollections
+  
   respond_to :html
 
   before_action :authorize_read_board!, only: [:show]
diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb
index 2994d8c9666..ae060abee5c 100644
--- a/app/controllers/projects/group_links_controller.rb
+++ b/app/controllers/projects/group_links_controller.rb
@@ -5,17 +5,25 @@ class Projects::GroupLinksController < Projects::ApplicationController
 
   def index
     @group_links = project.project_group_links.all
+
+    @skip_groups = @group_links.pluck(:group_id)
+    @skip_groups << project.group.try(:id)
   end
 
   def create
-    group = Group.find(params[:link_group_id])
-    return render_404 unless can?(current_user, :read_group, group)
-
-    project.project_group_links.create(
-      group: group,
-      group_access: params[:link_group_access],
-      expires_at: params[:expires_at]
-    )
+    group = Group.find(params[:link_group_id]) if params[:link_group_id].present?
+
+    if group
+      return render_404 unless can?(current_user, :read_group, group)
+
+      project.project_group_links.create(
+        group: group,
+        group_access: params[:link_group_access],
+        expires_at: params[:expires_at]
+      )
+    else
+      flash[:alert] = 'Please select a group.'
+    end
 
     redirect_to namespace_project_group_links_path(project.namespace, project)
   end
diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb
index 28fa4a5b141..a6626df4826 100644
--- a/app/controllers/projects/labels_controller.rb
+++ b/app/controllers/projects/labels_controller.rb
@@ -30,9 +30,15 @@ class Projects::LabelsController < Projects::ApplicationController
     @label = @project.labels.create(label_params)
 
     if @label.valid?
-      redirect_to namespace_project_labels_path(@project.namespace, @project)
+      respond_to do |format|
+        format.html { redirect_to namespace_project_labels_path(@project.namespace, @project) }
+        format.json { render json: @label }
+      end
     else
-      render 'new'
+      respond_to do |format|
+        format.html { render 'new' }
+        format.json { render json: { message: @label.errors.messages }, status: 400 }
+      end
     end
   end
 
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 8c8c56228ad..ffd9833e3b1 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -19,6 +19,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   before_action :define_diff_comment_vars, only: [:diffs]
   before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :pipelines]
   before_action :close_merge_request_without_source_project, only: [:show, :diffs, :commits, :builds, :pipelines]
+  before_action :apply_diff_view_cookie!, only: [:new_diffs]
+  before_action :build_merge_request, only: [:new, :new_diffs]
 
   # Allow read any merge_request
   before_action :authorize_read_merge_request!
@@ -210,29 +212,26 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def new
-    apply_diff_view_cookie!
-
-    build_merge_request
-    @noteable = @merge_request
-
-    @target_branches = if @merge_request.target_project
-                         @merge_request.target_project.repository.branch_names
-                       else
-                         []
-                       end
-
-    @target_project = merge_request.target_project
-    @source_project = merge_request.source_project
-    @commits = @merge_request.compare_commits.reverse
-    @commit = @merge_request.diff_head_commit
-    @base_commit = @merge_request.diff_base_commit
-    @diffs = @merge_request.diffs(diff_options) if @merge_request.compare
-    @diff_notes_disabled = true
-    @pipeline = @merge_request.pipeline
-    @statuses = @pipeline.statuses.relevant if @pipeline
+    define_new_vars
+  end
 
-    @note_counts = Note.where(commit_id: @commits.map(&:id)).
-      group(:commit_id).count
+  def new_diffs
+    respond_to do |format|
+      format.html do
+        define_new_vars
+        render "new"
+      end
+      format.json do
+        @diffs = if @merge_request.can_be_created
+                   @merge_request.diffs(diff_options)
+                 else
+                   []
+                 end
+        @diff_notes_disabled = true
+
+        render json: { html: view_to_html_string('projects/merge_requests/_new_diffs', diffs: @diffs) }
+      end
+    end
   end
 
   def create
@@ -490,6 +489,27 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     )
   end
 
+  def define_new_vars
+    @noteable = @merge_request
+
+    @target_branches = if @merge_request.target_project
+                         @merge_request.target_project.repository.branch_names
+                       else
+                         []
+                       end
+
+    @target_project = merge_request.target_project
+    @source_project = merge_request.source_project
+    @commits = @merge_request.compare_commits.reverse
+    @commit = @merge_request.diff_head_commit
+    @base_commit = @merge_request.diff_base_commit
+
+    @pipeline = @merge_request.pipeline
+    @statuses = @pipeline.statuses.relevant if @pipeline
+    @note_counts = Note.where(commit_id: @commits.map(&:id)).
+      group(:commit_id).count
+  end
+
   def invalid_mr
     # Render special view for MR with removed target branch
     render 'invalid'
@@ -521,7 +541,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   def build_merge_request
     params[:merge_request] ||= ActionController::Parameters.new(source_project: @project)
-    @merge_request = MergeRequests::BuildService.new(project, current_user, merge_request_params).execute
+    @merge_request = MergeRequests::BuildService.new(project, current_user, merge_request_params.merge(diff_options: diff_options)).execute
   end
 
   def compared_diff_version
diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index b2c8656d124..67b41c4573a 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -59,10 +59,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   end
 
   def destroy
-    @project_member = @project.members.find_by(id: params[:id]) ||
-      @project.requesters.find_by(id: params[:id])
-
-    Members::DestroyService.new(@project_member, current_user).execute
+    Members::DestroyService.new(@project, current_user, params).
+      execute(:all)
 
     respond_to do |format|
       format.html do