From dcb98c49f9d423215bfda6443caf77f2f2b604f9 Mon Sep 17 00:00:00 2001 From: Aditya Prakash Date: Thu, 24 Mar 2016 00:24:52 +0530 Subject: [PATCH 1/4] Improvements in proposal controller `if..else` is better than unless with return. On failed update render `edit` template and not render `new` template. On failed save redirect to index path and not render `new` template. Change `destory` to `withdraw`, cause `destroy` makes DELETE request when we are updating the event record. Check for success for save in withdraw (save can fail for reasons other than validation). --- app/controllers/proposal_controller.rb | 65 +++++++++++++------------- app/views/proposal/index.html.haml | 2 +- config/routes.rb | 3 +- 3 files changed, 36 insertions(+), 34 deletions(-) diff --git a/app/controllers/proposal_controller.rb b/app/controllers/proposal_controller.rb index cac68d4a..9bfca93c 100644 --- a/app/controllers/proposal_controller.rb +++ b/app/controllers/proposal_controller.rb @@ -24,13 +24,14 @@ class ProposalController < ApplicationController end def edit - authorize! :edit, @event @url = conference_program_proposal_path(@conference.short_title, params[:id]) end def create @url = conference_program_proposal_index_path(@conference.short_title) + # We allow proposal submission and sign up on same page. + # If user is not signed in then first create new user and then sign them in unless current_user @user = User.new(user_params) if @user.save @@ -47,39 +48,38 @@ class ProposalController < ApplicationController @event = Event.new(event_params) @event.program = @program + # User which creates the proposal is both `submitter` and `speaker` of proposal + # by default. + # TODO: Allow submitter to add speakers to proposals @event.event_users.new(user: current_user, event_role: 'submitter') @event.event_users.new(user: current_user, event_role: 'speaker') authorize! :new, @event - unless @event.save + if @event.save + ahoy.track 'Event submission', title: 'New submission' + redirect_to conference_program_proposal_index_path(@conference.short_title), notice: 'Proposal was successfully submitted.' + else flash[:error] = "Could not submit proposal: #{@event.errors.full_messages.join(', ')}" render action: 'new' - return end - - ahoy.track 'Event submission', title: 'New submission' - - redirect_to conference_program_proposal_index_path(@conference.short_title), notice: 'Proposal was successfully submitted.' end def update - authorize! :update, @event @url = conference_program_proposal_path(@conference.short_title, params[:id]) - if !@event.update(event_params) + if @event.update(event_params) + redirect_to(conference_program_proposal_index_path(conference_id: @conference.short_title), + notice: 'Proposal was successfully updated.') + else flash[:error] = "Could not update proposal: #{@event.errors.full_messages.join(', ')}" - render action: 'new' - return + render action: 'edit' end - - redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), - notice: 'Proposal was successfully updated.' end - def destroy - authorize! :destroy, @event + def withdraw + authorize! :update, @event @url = conference_program_proposal_path(@conference.short_title, params[:id]) begin @@ -89,9 +89,13 @@ class ProposalController < ApplicationController return end - @event.save(validate: false) - redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), - notice: 'Proposal was successfully withdrawn.' + if @event.save + redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), + notice: 'Proposal was successfully withdrawn.' + else + redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), + error: "Could not withdraw proposal: #{@event.errors.full_messages.join(', ')}" + end end def confirm @@ -99,15 +103,15 @@ class ProposalController < ApplicationController @url = conference_program_proposal_path(@conference.short_title, params[:id]) begin - @event.confirm! + @event.confirm rescue Transitions::InvalidTransition redirect_to :back, error: "Event can't be confirmed" return end - if !@event.save - flash[:error] = "Could not confirm proposal: #{@event.errors.full_messages.join(', ')}" - render action: 'new' + unless @event.save + redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), + error: "Could not confirm proposal: #{@event.errors.full_messages.join(', ')}" return end @@ -132,14 +136,13 @@ class ProposalController < ApplicationController return end - if !@event.save - flash[:error] = "Could not re-submit proposal: #{@event.errors.full_messages.join(', ')}" - render action: 'new' - return + if @event.save + redirect_to(conference_program_proposal_index_path(conference_id: @conference.short_title), + notice: "The proposal was re-submitted. The #{@conference.short_title} organizers will review it again.") + else + redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), + error: "Could not re-submit proposal: #{@event.errors.full_messages.join(', ')}" end - - redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), - notice: "The proposal was re-submitted. The #{@conference.short_title} organizers will review it again." end private @@ -152,5 +155,3 @@ class ProposalController < ApplicationController params.require(:user).permit(:email, :password, :password_confirmation, :username) end end - -# FIXME: Introduce strong_parameters pronto! diff --git a/app/views/proposal/index.html.haml b/app/views/proposal/index.html.haml index f9afd688..007299c2 100644 --- a/app/views/proposal/index.html.haml +++ b/app/views/proposal/index.html.haml @@ -87,7 +87,7 @@ method: :patch, class: 'btn btn-mini btn-success', id: "confirm_proposal_#{event.id}" - if event.transition_possible? :withdraw - = link_to 'Withdraw', conference_program_proposal_path(@conference.short_title, event.id), method: :delete, + = link_to 'Withdraw', withdraw_conference_program_proposal_path(@conference.short_title, event.id), method: :patch, data: { confirm: 'Are you sure you want to withdraw this proposal?' }, class: 'btn btn-mini btn-warning', id: "delete_proposal_#{event.id}" - if event.state == 'withdrawn' || event.state == 'rejected' diff --git a/config/routes.rb b/config/routes.rb index b22775e5..1c4eb7c9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -95,11 +95,12 @@ Osem::Application.routes.draw do resources :conference, only: [:index, :show] do resource :program, except: :destroy do - resources :proposal do + resources :proposal, except: :destroy do get 'commercials/render_commercial' => 'commercials#render_commercial' resources :commercials, only: [:create, :update, :destroy] resources :event_attachment, controller: 'event_attachments' member do + patch '/withdraw' => 'proposal#withdraw' patch '/confirm' => 'proposal#confirm' patch '/restart' => 'proposal#restart' end From e851c0b4ff7db6be992727e63bd281075521b158 Mon Sep 17 00:00:00 2001 From: Aditya Prakash Date: Thu, 24 Mar 2016 17:42:00 +0530 Subject: [PATCH 2/4] Add authorization rule for create user by unsigned user Add authorization to failed user save path cause it was complaining that no authorization was performed in that path. --- app/controllers/proposal_controller.rb | 1 + app/models/ability.rb | 2 +- spec/models/ability_spec.rb | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/proposal_controller.rb b/app/controllers/proposal_controller.rb index 9bfca93c..b0678150 100644 --- a/app/controllers/proposal_controller.rb +++ b/app/controllers/proposal_controller.rb @@ -34,6 +34,7 @@ class ProposalController < ApplicationController # If user is not signed in then first create new user and then sign them in unless current_user @user = User.new(user_params) + authorize! :create, @user if @user.save sign_in(@user) else diff --git a/app/models/ability.rb b/app/models/ability.rb index 68f4ad8a..5a520afa 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -44,7 +44,7 @@ class Ability # can view Commercials of confirmed Events can :show, Commercial, commercialable_type: 'Event', commercialable_id: Event.where(state: 'confirmed').pluck(:id) - can :show, User + can [:show, :create], User unless CONFIG['authentication']['ichain']['enabled'] can [:show, :create], Registration do |registration| registration.new_record? diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index a4a8ace6..fedff97d 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -56,6 +56,7 @@ describe 'User' do it{ should_not be_able_to(:show, commercial_event_unconfirmed)} it{ should be_able_to(:show, User)} + it{ should be_able_to(:create, User)} it{ should be_able_to(:create, Registration)} it{ should be_able_to(:show, Registration.new)} From f4678e54155ae9d547f4e43083dca295998364a4 Mon Sep 17 00:00:00 2001 From: Aditya Prakash Date: Thu, 24 Mar 2016 17:49:34 +0530 Subject: [PATCH 3/4] Add proposal controller tests --- app/controllers/proposal_controller.rb | 8 +- spec/controllers/proposal_controller_spec.rb | 571 +++++++++++++++++++ 2 files changed, 575 insertions(+), 4 deletions(-) create mode 100644 spec/controllers/proposal_controller_spec.rb diff --git a/app/controllers/proposal_controller.rb b/app/controllers/proposal_controller.rb index b0678150..c66c0d8c 100644 --- a/app/controllers/proposal_controller.rb +++ b/app/controllers/proposal_controller.rb @@ -71,8 +71,8 @@ class ProposalController < ApplicationController @url = conference_program_proposal_path(@conference.short_title, params[:id]) if @event.update(event_params) - redirect_to(conference_program_proposal_index_path(conference_id: @conference.short_title), - notice: 'Proposal was successfully updated.') + redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), + notice: 'Proposal was successfully updated.' else flash[:error] = "Could not update proposal: #{@event.errors.full_messages.join(', ')}" render action: 'edit' @@ -138,8 +138,8 @@ class ProposalController < ApplicationController end if @event.save - redirect_to(conference_program_proposal_index_path(conference_id: @conference.short_title), - notice: "The proposal was re-submitted. The #{@conference.short_title} organizers will review it again.") + redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), + notice: "The proposal was re-submitted. The #{@conference.short_title} organizers will review it again." else redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), error: "Could not re-submit proposal: #{@event.errors.full_messages.join(', ')}" diff --git a/spec/controllers/proposal_controller_spec.rb b/spec/controllers/proposal_controller_spec.rb new file mode 100644 index 00000000..f9b350ae --- /dev/null +++ b/spec/controllers/proposal_controller_spec.rb @@ -0,0 +1,571 @@ +require 'spec_helper' + +describe ProposalController do + let(:user) { create(:user) } + let(:conference) { create(:conference, short_title: 'lama101') } + let(:event) { create(:event, program: conference.program) } + + context 'user is not signed in' do + describe 'GET #new' do + before do + # We allow new proposal only if program has open cfp + conference.program.update_attributes(cfp: create(:cfp)) + get :new, conference_id: conference.short_title + end + + it 'assigns user and url variables' do + expect(assigns(:user)).to be_instance_of(User) + expect(assigns(:url)).to eq '/conference/lama101/program/proposal' + end + + it 'renders new template' do + expect(response).to render_template('new') + end + end + + describe 'POST #create' do + # We allow proposal create only if program has open cfp + before { conference.program.update_attributes(cfp: create(:cfp)) } + + it 'assigns url variables' do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title, + user: attributes_for(:user) + expect(assigns(:url)).to eq '/conference/lama101/program/proposal' + end + + context 'user is saved successfully' do + describe 'user related actions' do + before do + @new_user = attributes_for(:user) + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title, + user: @new_user + end + + it 'creates new user' do + expect(User.last.username).to eq @new_user[:username] + end + + it 'signs in new user' do + expect(controller.current_user.username).to eq @new_user[:username] + end + end + + context 'creates proposal successfully' do + before(:each, run: true) do + @new_user = attributes_for(:user) + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title, + user: @new_user + end + + it 'assigns event variable', run: true do + expect(assigns(:event)).not_to be_nil + end + + it 'assigns program to event', run: true do + expect(assigns(:event).program).to eq conference.program + end + + it 'assigns submitter and speaker to event', run: true do + expect(assigns(:event).submitter.username).to eq @new_user[:username] + expect(assigns(:event).speakers.first.username).to eq @new_user[:username] + end + + it 'redirects to proposal index path', run: true do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows success message in flash notice', run: true do + expect(flash[:notice]).to match('Proposal was successfully submitted.') + end + + it 'creates new event' do + expect do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title, + user: attributes_for(:user) + end.to change{ Event.count }.by 1 + end + end + + context 'proposal save fails' do + before(:each, run: true) do + allow_any_instance_of(Event).to receive(:save).and_return(false) + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title, + user: attributes_for(:user) + end + + it 'renders new template', run: true do + expect(response).to render_template('new') + end + + it 'shows error in flash message', run: true do + expect(flash[:error]).to match("Could not submit proposal: #{event.errors.full_messages.join(', ')}") + end + + it 'does not create new proposal' do + allow_any_instance_of(Event).to receive(:save).and_return(false) + expect do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title, + user: attributes_for(:user) + end.not_to change{ Event.count } + end + end + end + + context 'user save fails' do + before { allow_any_instance_of(User).to receive(:save).and_return(false) } + + it 'does not create new user' do + expect do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title, + user: attributes_for(:user) + end.not_to change { User.count } + end + + it 'does not create new event' do + expect do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title, + user: attributes_for(:user) + end.not_to change { Event.count } + end + + describe 'response' do + before do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title, + user: attributes_for(:user) + end + + it 'renders new template' do + expect(response).to render_template('new') + end + + it 'shows error in flash message' do + expect(flash[:error]).to match "Could not save user: #{user.errors.full_messages.join(', ')}" + end + end + end + end + end + + context 'event submitter is signed in' do + before do + sign_in event.submitter + end + + describe 'GET #index' do + before { get :index, conference_id: conference.short_title } + + it 'assigns conference, program and events variables' do + expect(assigns(:conference)).to eq conference + expect(assigns(:program)).to eq conference.program + expect(assigns(:events)).to eq [event] + end + + it 'renders index template' do + expect(response).to render_template('index') + end + end + + describe 'GET #show' do + before do + get :show, conference_id: conference.short_title, id: event.id + end + + it 'assigns event and speaker variables' do + expect(assigns(:event)).to eq event + expect(assigns(:speaker)).to eq event.submitter + end + + it 'renders show template' do + expect(response).to render_template('show') + end + end + + describe 'GET #new' do + before do + # We allow new proposal only if program has open cfp + conference.program.update_attributes(cfp: create(:cfp)) + get :new, conference_id: conference.short_title + end + + it 'assigns user and url variables' do + expect(assigns(:user)).to be_instance_of(User) + expect(assigns(:url)).to eq '/conference/lama101/program/proposal' + end + + it 'renders new template' do + expect(response).to render_template('new') + end + end + + describe 'GET #edit' do + before do + get :edit, conference_id: conference.short_title, id: event.id + end + + it 'assigns event and url variables' do + expect(assigns(:event)).to eq event + expect(assigns(:url)).to eq '/conference/lama101/program/proposal/1' + end + + it 'renders edit template' do + expect(response).to render_template('edit') + end + end + + describe 'POST #create' do + # We allow proposal create only if program has open cfp + before { conference.program.update_attributes(cfp: create(:cfp)) } + + it 'assigns url variables' do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title + expect(assigns(:url)).to eq '/conference/lama101/program/proposal' + end + + context 'creates proposal successfully' do + before(:each, run: true) do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title + end + + it 'assigns event variable', run: true do + expect(assigns(:event)).not_to be_nil + end + + it 'assigns program to event', run: true do + expect(assigns(:event).program).to eq conference.program + end + + it 'assigns submitter and speaker to event', run: true do + expect(assigns(:event).submitter).to eq event.submitter + expect(assigns(:event).speakers.first).to eq event.submitter + end + + it 'redirects to proposal index path', run: true do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows success message in flash notice', run: true do + expect(flash[:notice]).to match('Proposal was successfully submitted.') + end + + it 'creates new event' do + expect do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title + end.to change{ Event.count }.by 1 + end + end + + context 'proposal save fails' do + before(:each, run: true) do + allow_any_instance_of(Event).to receive(:save).and_return(false) + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title + end + + it 'renders new template', run: true do + expect(response).to render_template('new') + end + + it 'shows error in flash message', run: true do + expect(flash[:error]).to match("Could not submit proposal: #{event.errors.full_messages.join(', ')}") + end + + it 'does not create new proposal' do + allow_any_instance_of(Event).to receive(:save).and_return(false) + expect do + post :create, event: attributes_for(:event, event_type_id: 1), + conference_id: conference.short_title + end.not_to change{ Event.count } + end + end + end + + describe 'PATCH #update' do + + it 'assigns url variable' do + patch :update, event: attributes_for(:event, title: 'some title', event_type_id: 1), + conference_id: conference.short_title, + id: event.id + expect(assigns(:url)).to eq '/conference/lama101/program/proposal/1' + end + + context 'updates successfully' do + before do + patch :update, event: attributes_for(:event, title: 'some title', event_type_id: 1), + conference_id: conference.short_title, + id: event.id + end + + it 'updates the proposal' do + event.reload + expect(event.title).to eq 'some title' + end + + it 'redirects to proposal index path' do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows success message in flash notice' do + expect(flash[:notice]).to match('Proposal was successfully updated.') + end + end + + context 'update fails' do + before do + allow_any_instance_of(Event).to receive(:save).and_return(false) + patch :update, event: attributes_for(:event, title: 'some title', event_type_id: 1), + conference_id: conference.short_title, + id: event.id + end + + it 'does not update the proposal' do + event.reload + expect(event.title).not_to eq 'some title' + end + + it 'renders edit template' do + expect(response).to render_template('edit') + end + + it 'shows error in flash message', run: true do + expect(flash[:error]).to match("Could not update proposal: #{event.errors.full_messages.join(', ')}") + end + end + end + + describe 'PATCH #withdraw' do + + it 'assigns url variable' do + patch :withdraw, conference_id: conference.short_title, id: event.id + expect(assigns(:url)).to eq '/conference/lama101/program/proposal/1' + end + + context 'withdraws successfully' do + before do + patch :withdraw, conference_id: conference.short_title, id: event.id + end + + it 'changes state of event to withdrawn' do + event.reload + expect(event.withdrawn?).to be true + end + + it 'redirects to proposal index path' do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows success message in flash notice' do + expect(flash[:notice]).to match('Proposal was successfully withdrawn.') + end + end + + context 'event withdraw fails' do + before do + request.env['HTTP_REFERER'] = '/' + allow_any_instance_of(Event).to receive(:withdraw).and_raise(Transitions::InvalidTransition) + patch :withdraw, conference_id: conference.short_title, id: event.id + end + + it 'does not withdraw event' do + event.reload + expect(event.withdrawn?).to be false + end + + it 'redirects to previous path' do + expect(response).to redirect_to '/' + end + + it 'shows error in flash message' do + expect(flash[:error]).to match("Event can't be withdrawn") + end + end + + context 'event save fails' do + before do + allow_any_instance_of(Event).to receive(:save).and_return(false) + patch :withdraw, conference_id: conference.short_title, id: event.id + end + + it 'does not withdraw event' do + event.reload + expect(event.withdrawn?).to be false + end + + it 'redirects to proposal index path' do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows error in flash message' do + expect(flash[:error]).to match("Could not withdraw proposal: #{event.errors.full_messages.join(', ')}") + end + end + end + + describe 'PATCH #confirm' do + before { event.update_attributes(state: 'unconfirmed') } + + context 'confirmed successfully' do + describe 'general actions' do + before { patch :confirm, conference_id: conference.short_title, id: event.id } + + it 'assigns url variable' do + expect(assigns(:url)).to eq '/conference/lama101/program/proposal/1' + end + + it 'change state of event to confirmed' do + event.reload + expect(event.confirmed?).to be true + end + end + + context 'user has registered for the conference' do + before do + create(:registration, conference: conference, user: event.submitter) + patch :confirm, conference_id: conference.short_title, id: event.id + end + + it 'redirects to proposal index path' do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows success message in flash notice' do + expect(flash[:notice]).to match('The proposal was confirmed.') + end + end + + context 'user has not registered for the conference' do + before do + patch :confirm, conference_id: conference.short_title, id: event.id + end + + it 'redirects to new registration path' do + expect(response).to redirect_to new_conference_conference_registrations_path conference.short_title + end + + it 'shows flash alert asking user to register' do + expect(flash[:alert]).to match('The proposal was confirmed. Please register to attend the conference.') + end + end + end + + context 'event confirm fails' do + before do + request.env['HTTP_REFERER'] = '/' + allow_any_instance_of(Event).to receive(:confirm).and_raise(Transitions::InvalidTransition) + patch :confirm, conference_id: conference.short_title, id: event.id + end + + it 'does not confirm event' do + expect(event.confirmed?).to be false + end + + it 'redirects to previous path' do + expect(response).to redirect_to '/' + end + + it 'shows error in flash message' do + expect(flash[:error]).to match("Event can't be confirmed") + end + end + + context 'event save fails' do + before do + event.update_attributes(state: 'unconfirmed') + allow_any_instance_of(Event).to receive(:save).and_return(false) + patch :confirm, conference_id: conference.short_title, id: event.id + end + + it 'does not confirm event' do + expect(event.confirmed?).to be false + end + + it 'redirects to proposal index path' do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows error in flash message' do + expect(flash[:error]).to match("Could not confirm proposal: #{event.errors.full_messages.join(', ')}") + end + end + end + + describe 'PATCH #restart' do + before { event.update_attributes(state: 'withdrawn') } + + it 'assigns url variable' do + patch :restart, conference_id: conference.short_title, id: event.id + expect(assigns(:url)).to eq '/conference/lama101/program/proposal/1' + end + + context 'resubmits successfully' do + before do + patch :restart, conference_id: conference.short_title, id: event.id + end + + it 'changes state of event to new' do + event.reload + expect(event.new?).to be true + end + + it 'redirects to proposal index path' do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows success message in flash notice' do + expect(flash[:notice]).to match("The proposal was re-submitted. The #{conference.short_title} organizers will review it again.") + end + end + + context 'event resubmission fails' do + before do + allow_any_instance_of(Event).to receive(:restart).and_raise(Transitions::InvalidTransition) + patch :restart, conference_id: conference.short_title, id: event.id + end + + it 'does not change state of event to new' do + event.reload + expect(event.new?).to be false + end + + it 'redirects to proposal index path' do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows error in flash message' do + expect(flash[:error]).to match("The proposal can't be re-submitted.") + end + end + + context 'event save fails' do + before do + allow_any_instance_of(Event).to receive(:save).and_return(false) + patch :restart, conference_id: conference.short_title, id: event.id + end + + it 'does not change state of event to new' do + event.reload + expect(event.new?).to be false + end + + it 'redirects to proposal index path' do + expect(response).to redirect_to conference_program_proposal_index_path conference.short_title + end + + it 'shows error in flash message' do + expect(flash[:error]).to match("Could not re-submit proposal: #{event.errors.full_messages.join(', ')}") + end + end + end + end +end From c35270ad315de5455dc4446144694906a6e70e5b Mon Sep 17 00:00:00 2001 From: Aditya Prakash Date: Thu, 31 Mar 2016 15:31:54 +0530 Subject: [PATCH 4/4] More sane branching in confirm action of proposal controller --- app/controllers/proposal_controller.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/app/controllers/proposal_controller.rb b/app/controllers/proposal_controller.rb index c66c0d8c..ae8d82aa 100644 --- a/app/controllers/proposal_controller.rb +++ b/app/controllers/proposal_controller.rb @@ -110,18 +110,17 @@ class ProposalController < ApplicationController return end - unless @event.save + if @event.save + if @conference.user_registered?(current_user) + redirect_to conference_program_proposal_index_path(@conference.short_title), + notice: 'The proposal was confirmed.' + else + redirect_to new_conference_conference_registrations_path(conference_id: @conference.short_title), + alert: 'The proposal was confirmed. Please register to attend the conference.' + end + else redirect_to conference_program_proposal_index_path(conference_id: @conference.short_title), error: "Could not confirm proposal: #{@event.errors.full_messages.join(', ')}" - return - end - - if @conference.user_registered?(current_user) - redirect_to conference_program_proposal_index_path(@conference.short_title), - notice: 'The proposal was confirmed.' - else - redirect_to new_conference_conference_registrations_path(conference_id: @conference.short_title), - alert: 'The proposal was confirmed. Please register to attend the conference.' end end