From c275f43147e7a8ab54623bdc5b9a8124b8592330 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 22 Apr 2014 14:39:43 +0200 Subject: return nil as auto_ticket_path for invalid tickets The auto_ticket_path is referenced from the tickets_controller like this: respond_with(@ticket, :location => auto_ticket_path(@ticket)) While this only uses the location parameter when the ticket is valid it will still attampt to calculate it if not. During the create action this will lead to crashes because the ticket_path can not be calculated for a ticket without an id. This led to https://leap.se/code/issues/5552 and probably https://leap.se/code/issues/5545. --- engines/support/app/helpers/auto_tickets_path_helper.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'engines') diff --git a/engines/support/app/helpers/auto_tickets_path_helper.rb b/engines/support/app/helpers/auto_tickets_path_helper.rb index 93f3cb9..5638222 100644 --- a/engines/support/app/helpers/auto_tickets_path_helper.rb +++ b/engines/support/app/helpers/auto_tickets_path_helper.rb @@ -23,6 +23,7 @@ module AutoTicketsPathHelper end def auto_ticket_path(ticket, options={}) + return unless ticket.persisted? options = ticket_view_options.merge options if @user user_ticket_path(@user, ticket, options) @@ -50,4 +51,4 @@ module AutoTicketsPathHelper hsh end -end \ No newline at end of file +end -- cgit v1.2.3 From c5c54ec2035813949a81e8b5977a8f2538897260 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 22 Apr 2014 14:40:27 +0200 Subject: let's only add the flash notice if the ticket has been created --- engines/support/app/controllers/tickets_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'engines') diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index d65ee43..4be3493 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -24,11 +24,11 @@ class TicketsController < ApplicationController if @ticket.save flash[:notice] = t(:thing_was_successfully_created, :thing => t(:ticket)) - end - # cannot set this until ticket has been saved, as @ticket.id will not be set - if !logged_in? and flash[:notice] - flash[:notice] += " " + t(:access_ticket_text, :full_url => ticket_url(@ticket.id)) + # cannot set this until ticket has been saved, as @ticket.id will not be set + if !logged_in? and flash[:notice] + flash[:notice] += " " + t(:access_ticket_text, :full_url => ticket_url(@ticket.id)) + end end respond_with(@ticket, :location => auto_ticket_path(@ticket)) end -- cgit v1.2.3 From 7df11eed6aa48d1f61ee7f3700295c78b62358f0 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 22 Apr 2014 14:43:19 +0200 Subject: check user_id param for present? instead of !nil? when rendered from teh create action due to an error user_id param will sometimes be an empty string. We should still avoid rendering the navigation because the path's can not be resolved without a user_id. --- engines/support/app/views/tickets/index.html.haml | 2 +- engines/support/app/views/tickets/new.html.haml | 4 ++-- engines/support/app/views/tickets/show.html.haml | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'engines') diff --git a/engines/support/app/views/tickets/index.html.haml b/engines/support/app/views/tickets/index.html.haml index c02a326..a4df6e3 100644 --- a/engines/support/app/views/tickets/index.html.haml +++ b/engines/support/app/views/tickets/index.html.haml @@ -1,4 +1,4 @@ -- @show_navigation = !params[:user_id].nil? +- @show_navigation = params[:user_id].present? = render 'tickets/tabs' diff --git a/engines/support/app/views/tickets/new.html.haml b/engines/support/app/views/tickets/new.html.haml index 8f217a5..ba86ac3 100644 --- a/engines/support/app/views/tickets/new.html.haml +++ b/engines/support/app/views/tickets/new.html.haml @@ -1,4 +1,4 @@ -- @show_navigation = !params[:user_id].nil? +- @show_navigation = params[:user_id].present? = render 'tickets/tabs' @@ -27,4 +27,4 @@ - if logged_in? = link_to t(:cancel), auto_tickets_path, :class => :btn - else - = link_to t(:cancel), home_path, :class => 'btn' \ No newline at end of file + = link_to t(:cancel), home_path, :class => 'btn' diff --git a/engines/support/app/views/tickets/show.html.haml b/engines/support/app/views/tickets/show.html.haml index bfdb773..bbca4bf 100644 --- a/engines/support/app/views/tickets/show.html.haml +++ b/engines/support/app/views/tickets/show.html.haml @@ -1,4 +1,4 @@ -- @show_navigation = !params[:user_id].nil? +- @show_navigation = params[:user_id].present? .ticket = render 'tickets/edit_form' @@ -9,4 +9,4 @@ %td.user = logged_in? ? current_user.login : t(:anonymous) %td.comment - = render 'tickets/new_comment_form' \ No newline at end of file + = render 'tickets/new_comment_form' -- cgit v1.2.3 From 5d6a82a67bffc0930ea480164be2aca60ea0d6a1 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 22 Apr 2014 14:44:11 +0200 Subject: add tests for invalid ticket creation --- .../test/functional/tickets_controller_test.rb | 11 +++++++++ .../support/test/integration/create_ticket_test.rb | 28 ++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 engines/support/test/integration/create_ticket_test.rb (limited to 'engines') diff --git a/engines/support/test/functional/tickets_controller_test.rb b/engines/support/test/functional/tickets_controller_test.rb index 416fb73..d746b59 100644 --- a/engines/support/test/functional/tickets_controller_test.rb +++ b/engines/support/test/functional/tickets_controller_test.rb @@ -72,6 +72,17 @@ class TicketsControllerTest < ActionController::TestCase end + test "handle invalid ticket" do + params = {:subject => "unauth ticket test subject", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}, :email => 'a'} + + assert_no_difference('Ticket.count') do + post :create, :ticket => params + end + + assert_template :new + assert_equal params[:subject], assigns(:ticket).subject + end + test "should create authenticated ticket" do params = {:subject => "auth ticket test subject", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}} diff --git a/engines/support/test/integration/create_ticket_test.rb b/engines/support/test/integration/create_ticket_test.rb new file mode 100644 index 0000000..2583fc7 --- /dev/null +++ b/engines/support/test/integration/create_ticket_test.rb @@ -0,0 +1,28 @@ +require 'test_helper' + +class CreateTicketTest < BrowserIntegrationTest + + test "can submit ticket anonymously" do + visit '/' + click_on 'Get Help' + fill_in 'Subject', with: 'test ticket' + fill_in 'Description', with: 'description of the problem goes here' + click_on 'Create Ticket' + assert page.has_content?("Ticket was successfully created.") + assert page.has_content?("You can later access this ticket at the URL") + assert page.has_content?(current_url) + assert ticket = Ticket.last + ticket.destroy + end + + test "get help when creating ticket with invalid email" do + visit '/' + click_on 'Get Help' + fill_in 'Subject', with: 'test ticket' + fill_in 'Email', with: 'invalid data' + fill_in 'Description', with: 'description of the problem goes here' + click_on 'Create Ticket' + assert page.has_content?("is invalid") + end + +end -- cgit v1.2.3