Commit 328b52d5 authored by Rubén Dávila's avatar Rubén Dávila Committed by Robert Speicher

Some updates after last code review.

parent 38e708f0
...@@ -13,16 +13,10 @@ module CreatesCommit ...@@ -13,16 +13,10 @@ module CreatesCommit
result = service.new(@tree_edit_project, current_user, commit_params).execute result = service.new(@tree_edit_project, current_user, commit_params).execute
if result[:status] == :success if result[:status] == :success
flash[:notice] = success_notice || "Your changes have been successfully committed." update_flash_notice(success_notice)
if create_merge_request?
success_path = merge_request_exists? ? existent_merge_request_path : new_merge_request_path
target = different_project? ? "project" : "branch"
flash[:notice] << " You can now submit a merge request to get this change into the original #{target}."
end
respond_to do |format| respond_to do |format|
format.html { redirect_to success_path } format.html { redirect_to final_success_path(success_path) }
format.json { render json: { message: "success", filePath: success_path } } format.json { render json: { message: "success", filePath: success_path } }
end end
else else
...@@ -41,14 +35,32 @@ module CreatesCommit ...@@ -41,14 +35,32 @@ module CreatesCommit
end end
def authorize_edit_tree! def authorize_edit_tree!
return if can?(current_user, :push_code, project) return if can_collaborate_with_project?
return if current_user && current_user.already_forked?(project)
access_denied! access_denied!
end end
private private
def update_flash_notice(success_notice)
flash[:notice] = success_notice || "Your changes have been successfully committed."
if create_merge_request?
if merge_request_exists?
flash[:notice] = nil
else
target = different_project? ? "project" : "branch"
flash[:notice] << " You can now submit a merge request to get this change into the original #{target}."
end
end
end
def final_success_path(success_path)
return success_path unless create_merge_request?
merge_request_exists? ? existing_merge_request_path : new_merge_request_path
end
def new_merge_request_path def new_merge_request_path
new_namespace_project_merge_request_path( new_namespace_project_merge_request_path(
@mr_source_project.namespace, @mr_source_project.namespace,
...@@ -62,15 +74,15 @@ module CreatesCommit ...@@ -62,15 +74,15 @@ module CreatesCommit
) )
end end
def existent_merge_request_path def existing_merge_request_path
namespace_project_merge_request_path(@mr_target_project.namespace, @mr_target_project, @merge_request) namespace_project_merge_request_path(@mr_target_project.namespace, @mr_target_project, @merge_request)
end end
def merge_request_exists? def merge_request_exists?
@merge_request = @mr_target_project.merge_requests.opened.where( return @merge_request if defined?(@merge_request)
source_branch: @mr_source_branch,
target_branch: @mr_target_branch @merge_request = @mr_target_project.merge_requests.opened.find_by(
).first source_branch: @mr_source_branch, target_branch: @mr_target_branch)
end end
def different_project? def different_project?
......
...@@ -3,6 +3,7 @@ class Projects::ApplicationController < ApplicationController ...@@ -3,6 +3,7 @@ class Projects::ApplicationController < ApplicationController
before_action :repository before_action :repository
layout 'project' layout 'project'
helper_method :can_collaborate_with_project?
def authenticate_user! def authenticate_user!
# Restrict access to Projects area only # Restrict access to Projects area only
# for non-signed users # for non-signed users
...@@ -36,4 +37,11 @@ class Projects::ApplicationController < ApplicationController ...@@ -36,4 +37,11 @@ class Projects::ApplicationController < ApplicationController
def builds_enabled def builds_enabled
return render_404 unless @project.builds_enabled? return render_404 unless @project.builds_enabled?
end end
def can_collaborate_with_project?(project = nil)
project ||= @project
can?(current_user, :push_code, project) ||
(current_user && current_user.already_forked?(project))
end
end end
...@@ -123,11 +123,28 @@ module CommitsHelper ...@@ -123,11 +123,28 @@ module CommitsHelper
) )
end end
def can_collaborate_with_project?(project = nil) def revert_commit_link(show_modal_condition, continue_to_path)
project ||= @project if show_modal_condition
link_to('Revert', '#modal-revert-commit',
'data-target' => '#modal-revert-commit',
'data-toggle' => 'modal',
class: 'btn btn-grouped btn-close',
title: 'Create merge request to revert commit'
)
else
continue_params = {
to: continue_to_path,
notice: edit_in_new_fork_notice + ' Try to revert this commit again.',
notice_now: edit_in_new_fork_notice_now
}
fork_path = namespace_project_forks_path(@project.namespace, @project,
namespace_key: current_user.namespace.id,
continue: continue_params
)
can?(current_user, :push_code, project) || link_to 'Revert', fork_path, class: 'btn btn-grouped btn-close', method: :post,
(current_user && current_user.already_forked?(project)) title: 'Create merge request to revert commit'
end
end end
protected protected
......
...@@ -96,7 +96,6 @@ module MergeRequestsHelper ...@@ -96,7 +96,6 @@ module MergeRequestsHelper
def can_update_merge_request? def can_update_merge_request?
project ||= @project project ||= @project
can?(current_user, :update_merge_request, project) || can_collaborate_with_project?(project)
(current_user && current_user.already_forked?(project))
end end
end end
...@@ -56,8 +56,7 @@ module TreeHelper ...@@ -56,8 +56,7 @@ module TreeHelper
return false unless on_top_of_branch?(project, ref) return false unless on_top_of_branch?(project, ref)
can?(current_user, :push_code, project) || can_collaborate_with_project?(project)
(current_user && current_user.already_forked?(project))
end end
def tree_edit_branch(project = @project, ref = @ref) def tree_edit_branch(project = @project, ref = @ref)
......
...@@ -220,17 +220,17 @@ class Commit ...@@ -220,17 +220,17 @@ class Commit
end end
def revert_message def revert_message
"Revert \"#{safe_message.lines.first.chomp}\"".truncate(80) + "\n\nReverts #{to_reference}" "Revert \"#{title}\"".truncate(80) + "\n\nReverts #{sha}"
end end
def is_a_merge_commit? def merge_commit?
parents.size > 1 parents.size > 1
end end
def merged_merge_request def merged_merge_request
return @merged_merge_request if defined?(@merged_merge_request) return @merged_merge_request if defined?(@merged_merge_request)
@merged_merge_request = is_a_merge_commit? && MergeRequest.where(merge_commit_sha: id).first @merged_merge_request = merge_commit? && MergeRequest.find_by(merge_commit_sha: id)
end end
private private
......
...@@ -626,7 +626,7 @@ class Repository ...@@ -626,7 +626,7 @@ class Repository
source_sha = find_branch(base_branch).target source_sha = find_branch(base_branch).target
target_branch = create_mr ? commit.revert_branch_name : base_branch target_branch = create_mr ? commit.revert_branch_name : base_branch
args = [commit.id, source_sha] args = [commit.id, source_sha]
args << { mainline: 1 } if commit.is_a_merge_commit? args << { mainline: 1 } if commit.merge_commit?
# Temporary branch exists and contains the revert commit # Temporary branch exists and contains the revert commit
return true if create_mr && find_branch(target_branch) return true if create_mr && find_branch(target_branch)
...@@ -638,14 +638,14 @@ class Repository ...@@ -638,14 +638,14 @@ class Repository
commit_with_hooks(user, target_branch) do |ref| commit_with_hooks(user, target_branch) do |ref|
committer = user_to_committer(user) committer = user_to_committer(user)
source_sha = Rugged::Commit.create(rugged, { source_sha = Rugged::Commit.create(rugged,
message: commit.revert_message, message: commit.revert_message,
author: committer, author: committer,
committer: committer, committer: committer,
tree: revert_index.write_tree(rugged), tree: revert_index.write_tree(rugged),
parents: [rugged.lookup(source_sha)], parents: [rugged.lookup(source_sha)],
update_ref: ref update_ref: ref
}) )
end end
end end
......
...@@ -16,17 +16,7 @@ ...@@ -16,17 +16,7 @@
= link_to namespace_project_tree_path(@project.namespace, @project, @commit), class: "btn btn-grouped" do = link_to namespace_project_tree_path(@project.namespace, @project, @commit), class: "btn btn-grouped" do
= icon('files-o') = icon('files-o')
Browse Files Browse Files
- if can_collaborate_with_project? = revert_commit_link(can_collaborate_with_project?, namespace_project_commit_path(@project.namespace, @project, @commit.id))
= link_to '#modal-revert-commit', { 'data-target' => '#modal-revert-commit', 'data-toggle' => 'modal', class: 'btn btn-grouped btn-close'} do
Revert
- else
- continue_params = { to: namespace_project_commit_path(@project.namespace, @project, @commit.id),
notice: edit_in_new_fork_notice,
notice_now: edit_in_new_fork_notice_now }
- fork_path = namespace_project_forks_path(@project.namespace, @project, namespace_key: current_user.namespace.id,
continue: continue_params)
= link_to fork_path, { class: 'btn btn-grouped', method: :post } do
Revert
%div %div
%p %p
......
...@@ -20,14 +20,4 @@ ...@@ -20,14 +20,4 @@
= link_to 'Reopen', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: 'btn btn-nr btn-grouped btn-reopen reopen-mr-link', title: "Reopen merge request" = link_to 'Reopen', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: 'btn btn-nr btn-grouped btn-reopen reopen-mr-link', title: "Reopen merge request"
- if @merge_request.merged? && @merge_request.merge_commit_sha.present? - if @merge_request.merged? && @merge_request.merge_commit_sha.present?
- if can_update_merge_request? = revert_commit_link(can_update_merge_request?, namespace_project_merge_request_path(@project.namespace, @project, @merge_request))
= link_to '#modal-revert-commit', { 'data-target' => '#modal-revert-commit', 'data-toggle' => 'modal', class: 'btn btn-grouped btn-close'} do
Revert
- else
- continue_params = { to: namespace_project_merge_request_path(@project.namespace, @project, @merge_request),
notice: edit_in_new_fork_notice,
notice_now: edit_in_new_fork_notice_now }
- fork_path = namespace_project_forks_path(@project.namespace, @project, namespace_key: current_user.namespace.id,
continue: continue_params)
= link_to fork_path, { class: 'btn btn-grouped btn-close', method: :post } do
Revert
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment