From 5dc8fe31ba19f321041d2ad5c73765062d41a312 Mon Sep 17 00:00:00 2001 From: Martin Cabrera Date: Tue, 10 Jan 2017 23:11:36 +0100 Subject: Fix Compare page throws 500 error when any branch/reference is not selected --- app/controllers/projects/compare_controller.rb | 7 ++++++- changelogs/unreleased/i--25814-500-error.yml | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/i--25814-500-error.yml diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index ec02fc15d35..5f14581ac69 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -25,8 +25,13 @@ class Projects::CompareController < Projects::ApplicationController end def create - redirect_to namespace_project_compare_path(@project.namespace, @project, + if params[:from].blank? || params[:to].blank? + flash[:alert] = "You must select from & to branches" + redirect_to namespace_project_compare_index_path + else + redirect_to namespace_project_compare_path(@project.namespace, @project, params[:from], params[:to]) + end end private diff --git a/changelogs/unreleased/i--25814-500-error.yml b/changelogs/unreleased/i--25814-500-error.yml new file mode 100644 index 00000000000..cd55ede84c8 --- /dev/null +++ b/changelogs/unreleased/i--25814-500-error.yml @@ -0,0 +1,4 @@ +--- +title: Fix Compare page throws 500 error when any branch/reference is not selected +merge_request: 8492 +author: Martin Cabrera -- cgit v1.2.1 From 6972a35fec05d08f2d243622d68d064105026398 Mon Sep 17 00:00:00 2001 From: Martin Cabrera Date: Tue, 10 Jan 2017 23:46:32 +0100 Subject: Added specs for branches compare when from & to are empty --- spec/controllers/projects/compare_controller_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 7a57801c437..c4905937878 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -64,6 +64,16 @@ describe Projects::CompareController do expect(assigns(:diffs)).to eq(nil) expect(assigns(:commits)).to eq(nil) end + + it 'redirects back to index when params[:from] & params[:to] are empty' do + post(:create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + from: '', + to: '') + + expect(response).to redirect_to(namespace_project_compare_index_path) + end end describe 'GET diff_for_path' do -- cgit v1.2.1 From c2283a2db716cf52818a99ac8db1ddb7839818d7 Mon Sep 17 00:00:00 2001 From: Martin Cabrera Date: Sun, 15 Jan 2017 01:12:19 +0100 Subject: Changed alert message character: from & to and --- app/controllers/projects/compare_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 5f14581ac69..746ca6568f8 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -26,7 +26,8 @@ class Projects::CompareController < Projects::ApplicationController def create if params[:from].blank? || params[:to].blank? - flash[:alert] = "You must select from & to branches" + flash[:alert] = "You must select from and to branches" + byebug redirect_to namespace_project_compare_index_path else redirect_to namespace_project_compare_path(@project.namespace, @project, -- cgit v1.2.1 From 0413a7ad03b94171038336ca7e13c7911e3ea3d3 Mon Sep 17 00:00:00 2001 From: Martin Cabrera Date: Sun, 15 Jan 2017 01:53:56 +0100 Subject: from or to get variables gets preserved if the other one is missing --- app/controllers/projects/compare_controller.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 746ca6568f8..91793cc3650 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -27,8 +27,8 @@ class Projects::CompareController < Projects::ApplicationController def create if params[:from].blank? || params[:to].blank? flash[:alert] = "You must select from and to branches" - byebug - redirect_to namespace_project_compare_index_path + from_to_preservation = from_to_hash(params) + redirect_to namespace_project_compare_index_path(@project.namespace, @project, from_to_preservation) else redirect_to namespace_project_compare_path(@project.namespace, @project, params[:from], params[:to]) @@ -62,4 +62,11 @@ class Projects::CompareController < Projects::ApplicationController @merge_request ||= MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened. find_by(source_project: @project, source_branch: @head_ref, target_branch: @start_ref) end + + def from_to_hash(params) + return_hash = {} + return_hash[:from] = params[:from].presence + return_hash[:to] = params[:to].presence + return_hash + end end -- cgit v1.2.1 From 87c39b6d393d211c038bff084a3f8d21f0391d71 Mon Sep 17 00:00:00 2001 From: Martin Cabrera Date: Sun, 15 Jan 2017 02:28:01 +0100 Subject: Added specs for the preservation of the from/to parameter --- .../projects/compare_controller_spec.rb | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index c4905937878..b03c4b52de6 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -65,7 +65,27 @@ describe Projects::CompareController do expect(assigns(:commits)).to eq(nil) end - it 'redirects back to index when params[:from] & params[:to] are empty' do + it 'redirects back to index when params[:from] is empty and preserves params[:to]' do + post(:create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + from: '', + to: 'master') + + expect(response).to redirect_to(namespace_project_compare_index_path(project.namespace, project, to: 'master')) + end + + it 'redirects back to index when params[:to] is empty and preserves params[:from]' do + post(:create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + from: 'master', + to: '') + + expect(response).to redirect_to(namespace_project_compare_index_path(project.namespace, project, from: 'master')) + end + + it 'redirects back to index when params[:from] and params[:to] are empty' do post(:create, namespace_id: project.namespace.to_param, project_id: project.to_param, -- cgit v1.2.1 From 9844c1f222fa910fc5fe23de0821c83a095d84f9 Mon Sep 17 00:00:00 2001 From: Martin Cabrera Date: Mon, 16 Jan 2017 20:23:30 +0100 Subject: =?UTF-8?q?Refactored=20=E2=80=98from=E2=80=99=20and=20=E2=80=98to?= =?UTF-8?q?=E2=80=99=20variable=20preservation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/controllers/projects/compare_controller.rb | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 91793cc3650..2ab381f6850 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -27,8 +27,11 @@ class Projects::CompareController < Projects::ApplicationController def create if params[:from].blank? || params[:to].blank? flash[:alert] = "You must select from and to branches" - from_to_preservation = from_to_hash(params) - redirect_to namespace_project_compare_index_path(@project.namespace, @project, from_to_preservation) + from_to_vars = { + from: params[:from].presence, + to: params[:to].presence + } + redirect_to namespace_project_compare_index_path(@project.namespace, @project, from_to_vars) else redirect_to namespace_project_compare_path(@project.namespace, @project, params[:from], params[:to]) @@ -62,11 +65,4 @@ class Projects::CompareController < Projects::ApplicationController @merge_request ||= MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened. find_by(source_project: @project, source_branch: @head_ref, target_branch: @start_ref) end - - def from_to_hash(params) - return_hash = {} - return_hash[:from] = params[:from].presence - return_hash[:to] = params[:to].presence - return_hash - end -end +end \ No newline at end of file -- cgit v1.2.1 From 856003cfea894b56784f0fea4ee64655e6c16f06 Mon Sep 17 00:00:00 2001 From: Martin Cabrera Date: Mon, 16 Jan 2017 23:17:24 +0100 Subject: refactor rubocop suggestion --- app/controllers/projects/compare_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 2ab381f6850..d32966645c8 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -65,4 +65,4 @@ class Projects::CompareController < Projects::ApplicationController @merge_request ||= MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened. find_by(source_project: @project, source_branch: @head_ref, target_branch: @start_ref) end -end \ No newline at end of file +end -- cgit v1.2.1