From b58a00b3212d841fa5edf207026922d99bc11591 Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 4 Mar 2016 11:09:14 -0300 Subject: [PATCH 1/4] Extract user creation method if not signed in related to openSUSE/osem#597 Signed-off-by: Carlos Coelho --- app/controllers/proposal_controller.rb | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/app/controllers/proposal_controller.rb b/app/controllers/proposal_controller.rb index 95042af5..64e34196 100644 --- a/app/controllers/proposal_controller.rb +++ b/app/controllers/proposal_controller.rb @@ -26,16 +26,7 @@ class ProposalController < ApplicationController def create @url = conference_program_proposal_index_path(@conference.short_title) - unless current_user - @user = User.new(user_params) - if @user.save - sign_in(@user) - else - flash[:error] = "Could not save user: #{@user.errors.full_messages.join(', ')}" - render action: 'new' - return - end - end + create_user_if_not_signed_in params[:event].delete :user @@ -146,6 +137,19 @@ class ProposalController < ApplicationController def user_params params.require(:user).permit(:email, :password, :password_confirmation, :username) end + + def create_user_if_not_signed_in + unless current_user + @user = User.new(user_params) + if @user.save + sign_in(@user) + else + flash[:error] = "Could not save user: #{@user.errors.full_messages.join(', ')}" + render action: 'new' + return + end + end + end end # FIXME: Introduce strong_parameters pronto! From 2302821c48ef690e5a8133baa3a5aab8d3953f92 Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 4 Mar 2016 11:09:50 -0300 Subject: [PATCH 2/4] Extract method of proposal submission related to openSUSE/osem#597 Signed-off-by: Carlos Coelho --- app/controllers/proposal_controller.rb | 30 +++++++++++++++----------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/app/controllers/proposal_controller.rb b/app/controllers/proposal_controller.rb index 64e34196..c3e16e36 100644 --- a/app/controllers/proposal_controller.rb +++ b/app/controllers/proposal_controller.rb @@ -30,19 +30,7 @@ class ProposalController < ApplicationController params[:event].delete :user - @event = Event.new(event_params) - @event.program = @program - - @event.event_users.new(user: current_user, - event_role: 'submitter') - @event.event_users.new(user: current_user, - event_role: 'speaker') - - unless @event.save - flash[:error] = "Could not submit proposal: #{@event.errors.full_messages.join(', ')}" - render action: 'new' - return - end + proposal_submission ahoy.track 'Event submission', title: 'New submission' @@ -150,6 +138,22 @@ class ProposalController < ApplicationController end end end + + def proposal_submission + @event = Event.new(event_params) + @event.program = @program + + @event.event_users.new(user: current_user, + event_role: 'submitter') + @event.event_users.new(user: current_user, + event_role: 'speaker') + + unless @event.save + flash[:error] = "Could not submit proposal: #{@event.errors.full_messages.join(', ')}" + render action: 'new' + return + end + end end # FIXME: Introduce strong_parameters pronto! From aa912357d635a67321388952cba1c3cbc3f121b5 Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 4 Mar 2016 11:34:16 -0300 Subject: [PATCH 3/4] Using guard clause instead of conditional wrapping Signed-off-by: Carlos Coelho --- app/controllers/proposal_controller.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/proposal_controller.rb b/app/controllers/proposal_controller.rb index c3e16e36..18e62098 100644 --- a/app/controllers/proposal_controller.rb +++ b/app/controllers/proposal_controller.rb @@ -148,11 +148,9 @@ class ProposalController < ApplicationController @event.event_users.new(user: current_user, event_role: 'speaker') - unless @event.save - flash[:error] = "Could not submit proposal: #{@event.errors.full_messages.join(', ')}" - render action: 'new' - return - end + return unless !@event.save + flash[:error] = "Could not submit proposal: #{@event.errors.full_messages.join(', ')}" + render action: 'new' end end From 06c60ef095f10384211083c4c60a896ab03492c1 Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 4 Mar 2016 14:07:07 -0300 Subject: [PATCH 4/4] User creation if not signed in moved to helper It makes more sense to use it as a filter, and also makes it somewhat reusable Signed-off-by: Carlos Coelho --- app/controllers/proposal_controller.rb | 16 +--------------- app/helpers/application_helper.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/app/controllers/proposal_controller.rb b/app/controllers/proposal_controller.rb index 18e62098..bc969e2a 100644 --- a/app/controllers/proposal_controller.rb +++ b/app/controllers/proposal_controller.rb @@ -1,5 +1,6 @@ class ProposalController < ApplicationController before_filter :authenticate_user!, except: [:show, :new, :create] + before_action :create_user_if_not_signed_in, only: [:create] load_resource :conference, find_by: :short_title load_resource :program, through: :conference, singleton: true load_and_authorize_resource :event, parent: false, through: :program @@ -26,8 +27,6 @@ class ProposalController < ApplicationController def create @url = conference_program_proposal_index_path(@conference.short_title) - create_user_if_not_signed_in - params[:event].delete :user proposal_submission @@ -126,19 +125,6 @@ class ProposalController < ApplicationController params.require(:user).permit(:email, :password, :password_confirmation, :username) end - def create_user_if_not_signed_in - unless current_user - @user = User.new(user_params) - if @user.save - sign_in(@user) - else - flash[:error] = "Could not save user: #{@user.errors.full_messages.join(', ')}" - render action: 'new' - return - end - end - end - def proposal_submission @event = Event.new(event_params) @event.program = @program diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 35f127b9..c1f7b671 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -296,4 +296,17 @@ module ApplicationHelper def unread_notifications(user) Comment.accessible_by(current_ability).find_since_last_login(user) end + + def create_user_if_not_signed_in + unless current_user + @user = User.new(user_params) + if @user.save + sign_in(@user) + else + flash[:error] = "Could not save user: #{@user.errors.full_messages.join(', ')}" + render action: 'new' + return + end + end + end end