diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2c329b60a19..fb74919ea23 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -25,7 +25,7 @@ class ApplicationController < ActionController::Base helper_method :abilities, :can?, :current_application_settings helper_method :import_sources_enabled?, :github_import_enabled?, :github_import_configured?, :gitlab_import_enabled?, :gitlab_import_configured?, :bitbucket_import_enabled?, :bitbucket_import_configured?, :gitorious_import_enabled?, :google_code_import_enabled?, :fogbugz_import_enabled?, :git_import_enabled? - helper_method :repository + helper_method :repository, :can_collaborate_with_project? rescue_from Encoding::CompatibilityError do |exception| log_exception(exception) @@ -410,6 +410,13 @@ class ApplicationController < ActionController::Base current_user.nil? && root_path == request.path end + def can_collaborate_with_project?(project = nil) + project ||= @project + + can?(current_user, :push_code, project) || + (current_user && current_user.already_forked?(project)) + end + private def set_default_sort diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 4d7b1b80ded..4410e4d6505 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -82,7 +82,9 @@ module CreatesCommit return @merge_request if defined?(@merge_request) @merge_request = @mr_target_project.merge_requests.opened.find_by( - source_branch: @mr_source_branch, target_branch: @mr_target_branch) + source_branch: @mr_source_branch, + target_branch: @mr_target_branch + ) end def different_project? diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index e436aed1240..a326bc58215 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -3,7 +3,6 @@ class Projects::ApplicationController < ApplicationController before_action :repository layout 'project' - helper_method :can_collaborate_with_project? def authenticate_user! # Restrict access to Projects area only # for non-signed users @@ -37,11 +36,4 @@ class Projects::ApplicationController < ApplicationController def builds_enabled return render_404 unless @project.builds_enabled? 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 diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 78e99202a27..89c36edbe14 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -128,8 +128,7 @@ module CommitsHelper if show_modal_condition content_tag :span, 'data-toggle' => 'modal', 'data-target' => '#modal-revert-commit' do - link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'tooltip', - 'data-original-title' => 'Create merge request to revert commit', class: "btn btn-close btn-#{btn_class}" + link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'tooltip', 'data-original-title' => 'Create merge request to revert commit', class: "btn btn-close btn-#{btn_class}" end else continue_params = { @@ -138,12 +137,10 @@ module CommitsHelper 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 - ) + namespace_key: current_user.namespace.id, + continue: continue_params) - link_to 'Revert', fork_path, class: 'btn btn-grouped btn-close', method: :post, - 'data-toggle' => 'tooltip', 'data-original-title' => 'Create merge request to revert commit' + link_to 'Revert', fork_path, class: 'btn btn-grouped btn-close', method: :post, 'data-toggle' => 'tooltip', 'data-original-title' => 'Create merge request to revert commit' end end diff --git a/app/models/repository.rb b/app/models/repository.rb index e53daff2c8f..d98277fc257 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -642,8 +642,7 @@ class Repository committer: committer, tree: revert_index.write_tree(rugged), parents: [rugged.lookup(source_sha)], - update_ref: ref - ) + update_ref: ref) end end diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb index 7793bf1e421..bbe400dad88 100644 --- a/spec/controllers/commit_controller_spec.rb +++ b/spec/controllers/commit_controller_spec.rb @@ -143,4 +143,53 @@ describe Projects::CommitController do expect(assigns(:tags)).to include("v1.1.0") end end + + describe '#revert' do + context 'when target branch is not provided' do + it 'should render the 404 page' do + post(:revert, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: commit.id) + + expect(response).not_to be_success + expect(response.status).to eq(404) + end + end + + context 'when the revert was successful' do + it 'should redirect to the commits page' do + post(:revert, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: commit.id) + + expect(response).to redirect_to namespace_project_commits_path(project.namespace, project, 'master') + expect(flash[:notice]).to eq('The commit has been successfully reverted.') + end + end + + context 'when the revert failed' do + before do + post(:revert, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: commit.id) + end + + it 'should redirect to the commit page' do + # Reverting a commit that has been already reverted. + post(:revert, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: commit.id) + + expect(response).to redirect_to namespace_project_commit_path(project.namespace, project, commit.id) + expect(flash[:alert]).to match('Sorry, we cannot revert this commit automatically.') + end + end + end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 9fc9a873575..9242f755449 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -9,9 +9,10 @@ describe Repository, models: true do author = repository.user_to_committer(user) { message: 'Test message', committer: author, author: author } end - let(:merge_commit_id) do + let(:merge_commit) do source_sha = repository.find_branch('feature').target - repository.merge(user, source_sha, 'master', commit_options) + merge_commit_id = repository.merge(user, source_sha, 'master', commit_options) + repository.commit(merge_commit_id) end describe :branch_names_contains do @@ -437,16 +438,14 @@ describe Repository, models: true do describe '#merge' do it 'should merge the code and return the commit id' do - expect(merge_commit_id).to be_present - expect(repository.blob_at(merge_commit_id, 'files/ruby/feature.rb')).to be_present + expect(merge_commit).to be_present + expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present end end describe '#revert_merge' do it 'should revert the changes' do - repository.revert_merge(user, merge_commit_id, 'revert-changes', 'Revert changes') - source_sha = repository.find_branch('revert-changes').target - repository.merge(user, source_sha, 'master', commit_options) + repository.revert(user, merge_commit, 'master') expect(repository.blob_at_branch('master', 'files/ruby/feature.rb')).not_to be_present end