diff options
author | Jan Provaznik <jprovaznik@gitlab.com> | 2018-01-09 16:49:39 +0100 |
---|---|---|
committer | Jan Provaznik <jprovaznik@gitlab.com> | 2018-01-09 17:04:28 +0100 |
commit | a560f785f7f34b932c285365790a27d15bd100ec (patch) | |
tree | 62a54db8a361c25b4afdae140c3895f48a1d8a0e | |
parent | 678a00d60a21fcd39fa5c8043fadc4a94e618f4d (diff) | |
download | gitlab-ce-a560f785f7f34b932c285365790a27d15bd100ec.tar.gz |
Store only generic message if rebase fails4020-rebase-message
Instead of storing detailed rebase error, only a generic message is
stored with MR. The reason is that this message is exposed and displayed
to end user and there is no reason to expose detailed backend
information. Error message is still logged so detailed information
can be found in logfile by admin if needed.
Related #41820
-rw-r--r-- | app/services/merge_requests/rebase_service.rb | 8 | ||||
-rw-r--r-- | changelogs/unreleased/4020-rebase-message.yml | 5 | ||||
-rw-r--r-- | spec/services/merge_requests/rebase_service_spec.rb | 14 |
3 files changed, 17 insertions, 10 deletions
diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb index 0d5a25fa28e..c0083cd6afd 100644 --- a/app/services/merge_requests/rebase_service.rb +++ b/app/services/merge_requests/rebase_service.rb @@ -1,12 +1,14 @@ module MergeRequests class RebaseService < MergeRequests::WorkingCopyBaseService + REBASE_ERROR = 'Rebase failed. Please rebase locally'.freeze + def execute(merge_request) @merge_request = merge_request if rebase success else - error('Failed to rebase. Should be done manually') + error(REBASE_ERROR) end end @@ -22,8 +24,8 @@ module MergeRequests true rescue => e - log_error('Failed to rebase branch:') - log_error(e.message, save_message_on_model: true) + log_error(REBASE_ERROR, save_message_on_model: true) + log_error(e.message) false end end diff --git a/changelogs/unreleased/4020-rebase-message.yml b/changelogs/unreleased/4020-rebase-message.yml new file mode 100644 index 00000000000..4793f3d9cb9 --- /dev/null +++ b/changelogs/unreleased/4020-rebase-message.yml @@ -0,0 +1,5 @@ +--- +title: Display user friendly error message if rebase fails. +merge_request: +author: +type: fixed diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb index d1b37cdd073..5f047e61c31 100644 --- a/spec/services/merge_requests/rebase_service_spec.rb +++ b/spec/services/merge_requests/rebase_service_spec.rb @@ -32,7 +32,7 @@ describe MergeRequests::RebaseService do it 'returns an error' do expect(service.execute(merge_request)).to match(status: :error, - message: 'Failed to rebase. Should be done manually') + message: described_class::REBASE_ERROR) end end @@ -41,15 +41,15 @@ describe MergeRequests::RebaseService do allow(repository).to receive(:run_git!).and_raise('Something went wrong') end - it 'saves the error message' do + it 'saves a generic error message' do subject.execute(merge_request) - expect(merge_request.reload.merge_error).to eq 'Something went wrong' + expect(merge_request.reload.merge_error).to eq described_class::REBASE_ERROR end it 'returns an error' do expect(service.execute(merge_request)).to match(status: :error, - message: 'Failed to rebase. Should be done manually') + message: described_class::REBASE_ERROR) end end @@ -58,15 +58,15 @@ describe MergeRequests::RebaseService do allow(repository).to receive(:run_git!).and_raise(Gitlab::Git::Repository::GitError, 'Something went wrong') end - it 'saves the error message' do + it 'saves a generic error message' do subject.execute(merge_request) - expect(merge_request.reload.merge_error).to eq 'Something went wrong' + expect(merge_request.reload.merge_error).to eq described_class::REBASE_ERROR end it 'returns an error' do expect(service.execute(merge_request)).to match(status: :error, - message: 'Failed to rebase. Should be done manually') + message: described_class::REBASE_ERROR) end end |