From e7d6bcce2a04a049926e75074605a2e7f91ede1a Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 20 May 2014 09:08:42 +0200 Subject: move comment related tests out of TicketControllerTest This controller does too much - so the tests are also getting large and hard to keep track of --- .../test/functional/ticket_comments_test.rb | 89 ++++++++++++++++++++++ .../test/functional/tickets_controller_test.rb | 78 ------------------- 2 files changed, 89 insertions(+), 78 deletions(-) create mode 100644 engines/support/test/functional/ticket_comments_test.rb (limited to 'engines/support') diff --git a/engines/support/test/functional/ticket_comments_test.rb b/engines/support/test/functional/ticket_comments_test.rb new file mode 100644 index 0000000..e30a018 --- /dev/null +++ b/engines/support/test/functional/ticket_comments_test.rb @@ -0,0 +1,89 @@ +require 'test_helper' + +class TicketsCommentsTest < ActionController::TestCase + tests TicketsController + + teardown do + # destroy all tickets that were created during the test + Ticket.all.each{|t| t.destroy} + end + + test "add comment to unauthenticated ticket" do + ticket = FactoryGirl.create :ticket, :created_by => nil + + assert_difference('Ticket.find(ticket.id).comments.count') do + put :update, :id => ticket.id, + :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } + end + + assert_equal ticket, assigns(:ticket) # still same ticket, with different comments + assert_not_equal ticket.comments, assigns(:ticket).comments # ticket == assigns(:ticket), but they have different comments (which we want) + + end + + + test "add comment to own authenticated ticket" do + + login + ticket = FactoryGirl.create :ticket, :created_by => @current_user.id + + #they should be able to comment if it is their ticket: + assert_difference('Ticket.find(ticket.id).comments.count') do + put :update, :id => ticket.id, + :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } + end + assert_not_equal ticket.comments, assigns(:ticket).comments + assert_not_nil assigns(:ticket).comments.last.posted_by + assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id + + end + + + test "cannot comment if it is not your ticket" do + + other_user = find_record :user + login :is_admin? => false, :email => nil + ticket = FactoryGirl.create :ticket, :created_by => other_user.id + # they should *not* be able to comment if it is not their ticket + put :update, :id => ticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"not allowed comment"}} } + assert_response :redirect + assert_access_denied + + assert_equal ticket.comments.map(&:body), assigns(:ticket).comments.map(&:body) + + end + + + test "admin add comment to authenticated ticket" do + + other_user = find_record :user + login :is_admin? => true + + ticket = FactoryGirl.create :ticket, :created_by => other_user.id + + #admin should be able to comment: + assert_difference('Ticket.find(ticket.id).comments.count') do + put :update, :id => ticket.id, + :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } + end + assert_not_equal ticket.comments, assigns(:ticket).comments + assert_not_nil assigns(:ticket).comments.last.posted_by + assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id + end + + test "commenting on a ticket adds to tickets that are mine" do + testticket = FactoryGirl.create :ticket + user = find_record :admin_user + login user + get :index, {:user_id => user.id, :open_status => "open"} + assert_difference('assigns[:all_tickets].count') do + put :update, :id => testticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}} + get :index, {:user_id => user.id, :open_status => "open"} + end + + assert assigns(:all_tickets).include?(assigns(:ticket)) + assert_not_nil assigns(:ticket).comments.last.posted_by + assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id + end + +end diff --git a/engines/support/test/functional/tickets_controller_test.rb b/engines/support/test/functional/tickets_controller_test.rb index fc4a6f8..bb86dad 100644 --- a/engines/support/test/functional/tickets_controller_test.rb +++ b/engines/support/test/functional/tickets_controller_test.rb @@ -104,69 +104,6 @@ class TicketsControllerTest < ActionController::TestCase assert_equal assigns(:ticket).comments.first.posted_by, @current_user.id end - test "add comment to unauthenticated ticket" do - ticket = FactoryGirl.create :ticket, :created_by => nil - - assert_difference('Ticket.find(ticket.id).comments.count') do - put :update, :id => ticket.id, - :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } - end - - assert_equal ticket, assigns(:ticket) # still same ticket, with different comments - assert_not_equal ticket.comments, assigns(:ticket).comments # ticket == assigns(:ticket), but they have different comments (which we want) - - end - - - test "add comment to own authenticated ticket" do - - login - ticket = FactoryGirl.create :ticket, :created_by => @current_user.id - - #they should be able to comment if it is their ticket: - assert_difference('Ticket.find(ticket.id).comments.count') do - put :update, :id => ticket.id, - :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } - end - assert_not_equal ticket.comments, assigns(:ticket).comments - assert_not_nil assigns(:ticket).comments.last.posted_by - assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id - - end - - - test "cannot comment if it is not your ticket" do - - other_user = find_record :user - login :is_admin? => false, :email => nil - ticket = FactoryGirl.create :ticket, :created_by => other_user.id - # they should *not* be able to comment if it is not their ticket - put :update, :id => ticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"not allowed comment"}} } - assert_response :redirect - assert_access_denied - - assert_equal ticket.comments.map(&:body), assigns(:ticket).comments.map(&:body) - - end - - - test "admin add comment to authenticated ticket" do - - other_user = find_record :user - login :is_admin? => true - - ticket = FactoryGirl.create :ticket, :created_by => other_user.id - - #admin should be able to comment: - assert_difference('Ticket.find(ticket.id).comments.count') do - put :update, :id => ticket.id, - :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } - end - assert_not_equal ticket.comments, assigns(:ticket).comments - assert_not_nil assigns(:ticket).comments.last.posted_by - assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id - end - test "tickets by admin" do other_user = find_record :user ticket = FactoryGirl.create :ticket, :created_by => other_user.id @@ -196,21 +133,6 @@ class TicketsControllerTest < ActionController::TestCase assert !assigns(:all_tickets).include?(testticket) end - test "commenting on a ticket adds to tickets that are mine" do - testticket = FactoryGirl.create :ticket - user = find_record :admin_user - login user - get :index, {:user_id => user.id, :open_status => "open"} - assert_difference('assigns[:all_tickets].count') do - put :update, :id => testticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}} - get :index, {:user_id => user.id, :open_status => "open"} - end - - assert assigns(:all_tickets).include?(assigns(:ticket)) - assert_not_nil assigns(:ticket).comments.last.posted_by - assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id - end - test "admin ticket ordering" do tickets = FactoryGirl.create_list :ticket, 2 -- cgit v1.2.3 From 730e31017109994c24db431fde12f575ed5c1467 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 20 May 2014 09:13:25 +0200 Subject: FlashResponder will automagically add flash messages --- engines/support/app/controllers/tickets_controller.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'engines/support') diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index 99357ab..19663c3 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -23,11 +23,8 @@ class TicketsController < ApplicationController @ticket.comments.last.posted_by = current_user.id @ticket.comments.last.private = false unless admin? @ticket.created_by = current_user.id - if @ticket.save - flash[:notice] = t(:thing_was_successfully_created, :thing => t(:ticket)) - if !logged_in? - flash[:notice] += " " + t(:access_ticket_text, :full_url => ticket_url(@ticket.id)) - end + if @ticket.save && !logged_in? + flash[:success] = t(:access_ticket_text, :full_url => ticket_url(@ticket.id)) end respond_with(@ticket, :location => auto_ticket_path(@ticket)) end @@ -62,10 +59,8 @@ class TicketsController < ApplicationController end if @ticket.changed? and @ticket.save - flash[:notice] = t(:changes_saved) redirect_to_tickets else - flash[:error] = @ticket.errors.full_messages.join(". ") if @ticket.changed? redirect_to auto_ticket_path(@ticket) end end @@ -88,6 +83,10 @@ class TicketsController < ApplicationController @title = t(:tickets) end + def self.responder + Responders::FlashResponder + end + private # -- cgit v1.2.3 From 29fd85f605ee41275e383681a6b3e8ea0615d525 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 20 May 2014 11:18:28 +0200 Subject: splitting up long functional test case --- engines/support/test/factories.rb | 12 +++- .../test/functional/ticket_comments_test.rb | 16 ++++- .../test/functional/tickets_controller_test.rb | 74 +++++++++++++--------- 3 files changed, 68 insertions(+), 34 deletions(-) (limited to 'engines/support') diff --git a/engines/support/test/factories.rb b/engines/support/test/factories.rb index be04f15..bcf41e8 100644 --- a/engines/support/test/factories.rb +++ b/engines/support/test/factories.rb @@ -6,13 +6,23 @@ FactoryGirl.define do factory :ticket_with_comment do comments_attributes do - { "0" => { "body" => Faker::Lorem.sentences.join(" ") } } + { "0" => { + "body" => Faker::Lorem.sentences.join(" "), + "posted_by" => created_by + } } end end factory :ticket_with_creator do created_by { FactoryGirl.create(:user).id } end + + end + + # TicketComments can not be saved. so only use this with build + # and add to a ticket afterwards + factory :ticket_comment do + body { Faker::Lorem.sentences.join(" ") } end end diff --git a/engines/support/test/functional/ticket_comments_test.rb b/engines/support/test/functional/ticket_comments_test.rb index e30a018..5cbe233 100644 --- a/engines/support/test/functional/ticket_comments_test.rb +++ b/engines/support/test/functional/ticket_comments_test.rb @@ -39,8 +39,7 @@ class TicketsCommentsTest < ActionController::TestCase end - test "cannot comment if it is not your ticket" do - + test "cannot comment if it is another users ticket" do other_user = find_record :user login :is_admin? => false, :email => nil ticket = FactoryGirl.create :ticket, :created_by => other_user.id @@ -50,10 +49,23 @@ class TicketsCommentsTest < ActionController::TestCase assert_access_denied assert_equal ticket.comments.map(&:body), assigns(:ticket).comments.map(&:body) + end + test "authenticated comment on an anonymous ticket adds to my tickets" do + login + ticket = FactoryGirl.create :ticket + other_ticket = FactoryGirl.create :ticket + put :update, :id => ticket.id, + :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } + assert_not_nil assigns(:ticket).comments.last.posted_by + assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id + visible_tickets = Ticket.search admin_status: 'mine', + user_id: @current_user.id, is_admin: false + assert_equal [ticket], visible_tickets.all end + test "admin add comment to authenticated ticket" do other_user = find_record :user diff --git a/engines/support/test/functional/tickets_controller_test.rb b/engines/support/test/functional/tickets_controller_test.rb index bb86dad..acc088f 100644 --- a/engines/support/test/functional/tickets_controller_test.rb +++ b/engines/support/test/functional/tickets_controller_test.rb @@ -155,51 +155,63 @@ class TicketsControllerTest < ActionController::TestCase end - test "tickets for regular user" do + test "own tickets include tickets commented upon" do login ticket = FactoryGirl.create :ticket other_ticket = FactoryGirl.create :ticket - - put :update, :id => ticket.id, - :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } - assert_not_nil assigns(:ticket).comments.last.posted_by - assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id + comment = FactoryGirl.build(:ticket_comment, posted_by: @current_user.id) + ticket.comments << comment + ticket.save get :index, {:open_status => "open"} assert assigns(:all_tickets).count > 0 assert assigns(:all_tickets).include?(ticket) assert !assigns(:all_tickets).include?(other_ticket) + end - # user should have one more ticket if a new tick gets a comment by this user - assert_difference('assigns[:all_tickets].count') do - put :update, :id => other_ticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}} - get :index, {:open_status => "open"} - end - assert assigns(:all_tickets).include?(other_ticket) - - # if we close one ticket, the user should have 1 less open ticket - assert_difference('assigns[:all_tickets].count', -1) do - other_ticket.reload - other_ticket.close - other_ticket.save - get :index, {:open_status => "open"} - end + test "list all tickets created by user" do + login + ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + other_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + get :index, {:open_status => "open"} + assert_equal 2, assigns[:all_tickets].count + end - number_open_tickets = assigns(:all_tickets).count + test "closing ticket removes from open tickets list" do + login + ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + other_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + other_ticket.reload + other_ticket.close + other_ticket.save + get :index, {:open_status => "open"} + assert_equal 1, assigns[:all_tickets].count + end - # look at closed tickets: + test "list closed tickets only" do + login + open_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + closed_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id, is_open: false get :index, {:open_status => "closed"} - assert !assigns(:all_tickets).include?(ticket) - assert assigns(:all_tickets).include?(other_ticket) - number_closed_tickets = assigns(:all_tickets).count + assert_equal [closed_ticket], assigns(:all_tickets).all + end - # all tickets should equal closed + open + test "list all tickets inludes closed + open" do + login + open_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + closed_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id, is_open: false get :index, {:open_status => "all"} - assert assigns(:all_tickets).include?(ticket) - assert assigns(:all_tickets).include?(other_ticket) - assert_equal assigns(:all_tickets).count, number_closed_tickets + number_open_tickets - - + assert_equal 2, assigns(:all_tickets).count + assert assigns(:all_tickets).include?(open_ticket) + assert assigns(:all_tickets).include?(closed_ticket) end end -- cgit v1.2.3 From 7638970e233eaebc48abd499c37c274b48c97a96 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 20 May 2014 11:41:26 +0200 Subject: separate tests for the ticket list from main controller test --- .../test/functional/tickets_controller_test.rb | 120 ++------------------ .../support/test/functional/tickets_list_test.rb | 122 +++++++++++++++++++++ 2 files changed, 131 insertions(+), 111 deletions(-) create mode 100644 engines/support/test/functional/tickets_list_test.rb (limited to 'engines/support') diff --git a/engines/support/test/functional/tickets_controller_test.rb b/engines/support/test/functional/tickets_controller_test.rb index acc088f..e103d01 100644 --- a/engines/support/test/functional/tickets_controller_test.rb +++ b/engines/support/test/functional/tickets_controller_test.rb @@ -1,5 +1,14 @@ require 'test_helper' +# +# Tests for the basic actions in the TicketsController +# +# Also see +# TicketCommentsTest +# TicketsListTest +# +# for detailed functional tests for comments and index action. +# class TicketsControllerTest < ActionController::TestCase teardown do @@ -103,116 +112,5 @@ class TicketsControllerTest < ActionController::TestCase assert_not_nil assigns(:ticket).comments.first.posted_by assert_equal assigns(:ticket).comments.first.posted_by, @current_user.id end - - test "tickets by admin" do - other_user = find_record :user - ticket = FactoryGirl.create :ticket, :created_by => other_user.id - - login :is_admin? => true - - get :index, {:admin_status => "all", :open_status => "open"} - assert assigns(:all_tickets).count > 0 - - # if we close one ticket, the admin should have 1 less open ticket - assert_difference('assigns[:all_tickets].count', -1) do - assigns(:tickets).first.close - assigns(:tickets).first.save - get :index, {:admin_status => "all", :open_status => "open"} - end - end - - - test "admin_status mine vs all" do - testticket = FactoryGirl.create :ticket - user = find_record :user - login :is_admin? => true, :email => nil - - get :index, {:open_status => "open"} - assert assigns(:all_tickets).include?(testticket) - get :index, {:user_id => user.id, :open_status => "open"} - assert !assigns(:all_tickets).include?(testticket) - end - - test "admin ticket ordering" do - tickets = FactoryGirl.create_list :ticket, 2 - - login :is_admin? => true, :email => nil - get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_desc'} - - # this will consider all tickets, not just those on first page - first_tick = assigns(:all_tickets).all.first - last_tick = assigns(:all_tickets).all.last - assert first_tick.created_at > last_tick.created_at - - # and now reverse order: - get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_asc'} - - assert_equal first_tick, assigns(:all_tickets).last - assert_equal last_tick, assigns(:all_tickets).first - - assert_not_equal first_tick, assigns(:all_tickets).first - assert_not_equal last_tick, assigns(:all_tickets).last - - end - - test "own tickets include tickets commented upon" do - login - ticket = FactoryGirl.create :ticket - other_ticket = FactoryGirl.create :ticket - comment = FactoryGirl.build(:ticket_comment, posted_by: @current_user.id) - ticket.comments << comment - ticket.save - - get :index, {:open_status => "open"} - assert assigns(:all_tickets).count > 0 - assert assigns(:all_tickets).include?(ticket) - assert !assigns(:all_tickets).include?(other_ticket) - end - - test "list all tickets created by user" do - login - ticket = FactoryGirl.create :ticket_with_comment, - created_by: @current_user.id - other_ticket = FactoryGirl.create :ticket_with_comment, - created_by: @current_user.id - get :index, {:open_status => "open"} - assert_equal 2, assigns[:all_tickets].count - end - - test "closing ticket removes from open tickets list" do - login - ticket = FactoryGirl.create :ticket_with_comment, - created_by: @current_user.id - other_ticket = FactoryGirl.create :ticket_with_comment, - created_by: @current_user.id - other_ticket.reload - other_ticket.close - other_ticket.save - get :index, {:open_status => "open"} - assert_equal 1, assigns[:all_tickets].count - end - - test "list closed tickets only" do - login - open_ticket = FactoryGirl.create :ticket_with_comment, - created_by: @current_user.id - closed_ticket = FactoryGirl.create :ticket_with_comment, - created_by: @current_user.id, is_open: false - get :index, {:open_status => "closed"} - assert_equal [closed_ticket], assigns(:all_tickets).all - end - - test "list all tickets inludes closed + open" do - login - open_ticket = FactoryGirl.create :ticket_with_comment, - created_by: @current_user.id - closed_ticket = FactoryGirl.create :ticket_with_comment, - created_by: @current_user.id, is_open: false - get :index, {:open_status => "all"} - assert_equal 2, assigns(:all_tickets).count - assert assigns(:all_tickets).include?(open_ticket) - assert assigns(:all_tickets).include?(closed_ticket) - end - end diff --git a/engines/support/test/functional/tickets_list_test.rb b/engines/support/test/functional/tickets_list_test.rb new file mode 100644 index 0000000..4c4cdef --- /dev/null +++ b/engines/support/test/functional/tickets_list_test.rb @@ -0,0 +1,122 @@ +require 'test_helper' + +class TicketsListTest < ActionController::TestCase + tests TicketsController + + teardown do + # destroy all records that were created during the test + Ticket.all.each{|t| t.destroy} + User.all.each{|u| u.account.destroy} + end + + + test "tickets by admin" do + other_user = find_record :user + ticket = FactoryGirl.create :ticket, :created_by => other_user.id + + login :is_admin? => true + + get :index, {:admin_status => "all", :open_status => "open"} + assert assigns(:all_tickets).count > 0 + + # if we close one ticket, the admin should have 1 less open ticket + assert_difference('assigns[:all_tickets].count', -1) do + assigns(:tickets).first.close + assigns(:tickets).first.save + get :index, {:admin_status => "all", :open_status => "open"} + end + end + + + test "admin_status mine vs all" do + testticket = FactoryGirl.create :ticket + user = find_record :user + login :is_admin? => true, :email => nil + + get :index, {:open_status => "open"} + assert assigns(:all_tickets).include?(testticket) + get :index, {:user_id => user.id, :open_status => "open"} + assert !assigns(:all_tickets).include?(testticket) + end + + test "admin ticket ordering" do + tickets = FactoryGirl.create_list :ticket, 2 + + login :is_admin? => true, :email => nil + get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_desc'} + + # this will consider all tickets, not just those on first page + first_tick = assigns(:all_tickets).all.first + last_tick = assigns(:all_tickets).all.last + assert first_tick.created_at > last_tick.created_at + + # and now reverse order: + get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_asc'} + + assert_equal first_tick, assigns(:all_tickets).last + assert_equal last_tick, assigns(:all_tickets).first + + assert_not_equal first_tick, assigns(:all_tickets).first + assert_not_equal last_tick, assigns(:all_tickets).last + + end + + test "own tickets include tickets commented upon" do + login + ticket = FactoryGirl.create :ticket + other_ticket = FactoryGirl.create :ticket + comment = FactoryGirl.build(:ticket_comment, posted_by: @current_user.id) + ticket.comments << comment + ticket.save + + get :index, {:open_status => "open"} + assert assigns(:all_tickets).count > 0 + assert assigns(:all_tickets).include?(ticket) + assert !assigns(:all_tickets).include?(other_ticket) + end + + test "list all tickets created by user" do + login + ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + other_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + get :index, {:open_status => "open"} + assert_equal 2, assigns[:all_tickets].count + end + + test "closing ticket removes from open tickets list" do + login + ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + other_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + other_ticket.reload + other_ticket.close + other_ticket.save + get :index, {:open_status => "open"} + assert_equal 1, assigns[:all_tickets].count + end + + test "list closed tickets only" do + login + open_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + closed_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id, is_open: false + get :index, {:open_status => "closed"} + assert_equal [closed_ticket], assigns(:all_tickets).all + end + + test "list all tickets inludes closed + open" do + login + open_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + closed_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id, is_open: false + get :index, {:open_status => "all"} + assert_equal 2, assigns(:all_tickets).count + assert assigns(:all_tickets).include?(open_ticket) + assert assigns(:all_tickets).include?(closed_ticket) + end +end -- cgit v1.2.3 From 467dd712a19d48fc653cfc0e58201e6657d2c1f9 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 20 May 2014 12:30:55 +0200 Subject: split up and refactor TicketController#update close and open actions for plain opening and closing the tickets respond_with so fields are not cleared on invalid update the custom actions are not strictly restful. But adding a subresource felt like too much overhead and is conceptually hard to grasp (so we destroy the openess of the ticket to close it?). --- .../support/app/controllers/tickets_controller.rb | 66 ++++++++++------------ engines/support/app/models/ticket.rb | 2 + .../support/app/views/tickets/_comments.html.haml | 8 +++ .../support/app/views/tickets/_edit_form.html.haml | 7 ++- engines/support/app/views/tickets/edit.html.haml | 5 ++ engines/support/app/views/tickets/show.html.haml | 9 +-- engines/support/config/routes.rb | 14 ++++- .../test/functional/tickets_controller_test.rb | 17 ++++++ 8 files changed, 78 insertions(+), 50 deletions(-) create mode 100644 engines/support/app/views/tickets/_comments.html.haml create mode 100644 engines/support/app/views/tickets/edit.html.haml (limited to 'engines/support') diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index 19663c3..bb98277 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -5,8 +5,8 @@ class TicketsController < ApplicationController #has_scope :open, :type => boolean before_filter :require_login, :only => [:index] - before_filter :fetch_ticket, :only => [:show, :update, :destroy] - before_filter :require_ticket_access, :only => [:show, :update, :destroy] + before_filter :fetch_ticket, except: [:new, :create, :index] + before_filter :require_ticket_access, except: [:new, :create, :index] before_filter :fetch_user before_filter :set_title @@ -37,33 +37,32 @@ class TicketsController < ApplicationController end end - def update - if params[:button] == 'close' - @ticket.is_open = false - @ticket.save - redirect_to_tickets - elsif params[:button] == 'open' - @ticket.is_open = true - @ticket.save - redirect_to auto_ticket_path(@ticket) - else - @ticket.attributes = cleanup_ticket_params(params[:ticket]) + def close + @ticket.close + @ticket.save + redirect_to redirection_path + end - if params[:button] == 'reply_and_close' - @ticket.close - end + def open + @ticket.reopen + @ticket.save + redirect_to redirection_path + end - if @ticket.comments_changed? - @ticket.comments.last.posted_by = current_user.id - @ticket.comments.last.private = false unless admin? - end + def update + @ticket.attributes = cleanup_ticket_params(params[:ticket]) - if @ticket.changed? and @ticket.save - redirect_to_tickets - else - redirect_to auto_ticket_path(@ticket) - end + if params[:button] == 'reply_and_close' + @ticket.close + end + + if @ticket.comments_changed? + @ticket.comments.last.posted_by = current_user.id + @ticket.comments.last.private = false unless admin? end + + @ticket.save + respond_with @ticket, location: redirection_path end def index @@ -90,19 +89,14 @@ class TicketsController < ApplicationController private # - # redirects to ticket index, if appropriate. - # otherwise, just redirects to @ticket + # ticket index, if appropriate. + # otherwise, just @ticket # - def redirect_to_tickets - if logged_in? - if params[:button] == t(:reply_and_close) - redirect_to auto_tickets_path - else - redirect_to auto_ticket_path(@ticket) - end + def redirection_path + if logged_in? && params[:button] == t(:reply_and_close) + auto_tickets_path else - # if we are not logged in, there is no index to view - redirect_to auto_ticket_path(@ticket) + auto_ticket_path(@ticket) end end diff --git a/engines/support/app/models/ticket.rb b/engines/support/app/models/ticket.rb index bf5df53..161507c 100644 --- a/engines/support/app/models/ticket.rb +++ b/engines/support/app/models/ticket.rb @@ -39,6 +39,8 @@ class Ticket < CouchRest::Model::Base # * valid email address validates :email, :allow_blank => true, :format => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/ + # validates :comments, presence: true + def self.search(options = {}) @selection = TicketSelection.new(options) @selection.tickets diff --git a/engines/support/app/views/tickets/_comments.html.haml b/engines/support/app/views/tickets/_comments.html.haml new file mode 100644 index 0000000..0a3b345 --- /dev/null +++ b/engines/support/app/views/tickets/_comments.html.haml @@ -0,0 +1,8 @@ +%table.table.table-striped.table-bordered + %tbody + = render :partial => 'tickets/comment', :collection => @ticket.comments + %tr + %td.user + = current_user.login || t(:anonymous) + %td.comment + = render 'tickets/new_comment_form' diff --git a/engines/support/app/views/tickets/_edit_form.html.haml b/engines/support/app/views/tickets/_edit_form.html.haml index b8da779..22815f2 100644 --- a/engines/support/app/views/tickets/_edit_form.html.haml +++ b/engines/support/app/views/tickets/_edit_form.html.haml @@ -17,17 +17,18 @@ regarding_user_link = '' end -= simple_form_for @ticket do |f| +- url = url_for([@ticket.is_open? ? :close : :open, @ticket]) += simple_form_for @ticket, url: url do |f| = hidden_ticket_fields %p.first - if @ticket.is_open? %span.label.label-info %b{style: 'padding:10px'}= t(:open) - = f.button :loading, t(:close), value: 'close', class: 'btn-mini' + = f.button :loading, t(:close), class: 'btn-mini' - else %span.label.label-success %b{style: 'padding:10px'}= t(:closed) - = f.button :loading, t(:open), value: 'open', class: 'btn-mini' + = f.button :loading, t(:open), class: 'btn-mini' %span.label.label-clear= t(:created_by_on, :user => created_by, :time => @ticket.created_at.to_s(:short)).html_safe = simple_form_for @ticket do |f| = hidden_ticket_fields diff --git a/engines/support/app/views/tickets/edit.html.haml b/engines/support/app/views/tickets/edit.html.haml new file mode 100644 index 0000000..99afa2a --- /dev/null +++ b/engines/support/app/views/tickets/edit.html.haml @@ -0,0 +1,5 @@ +- @show_navigation = params[:user_id].present? + +.ticket + = render 'tickets/edit_form' + = render 'tickets/comments' diff --git a/engines/support/app/views/tickets/show.html.haml b/engines/support/app/views/tickets/show.html.haml index 4f3c127..99afa2a 100644 --- a/engines/support/app/views/tickets/show.html.haml +++ b/engines/support/app/views/tickets/show.html.haml @@ -2,11 +2,4 @@ .ticket = render 'tickets/edit_form' - %table.table.table-striped.table-bordered - %tbody - = render :partial => 'tickets/comment', :collection => @ticket.comments - %tr - %td.user - = current_user.login || t(:anonymous) - %td.comment - = render 'tickets/new_comment_form' + = render 'tickets/comments' diff --git a/engines/support/config/routes.rb b/engines/support/config/routes.rb index 23e0c11..ca471b3 100644 --- a/engines/support/config/routes.rb +++ b/engines/support/config/routes.rb @@ -1,8 +1,16 @@ Rails.application.routes.draw do - scope "(:locale)", :locale => MATCH_LOCALE do - resources :tickets, :except => :edit + scope "(:locale)", locale: MATCH_LOCALE do + + resources :tickets, except: :edit do + member do + put 'open' + put 'close' + end + end + resources :users do - resources :tickets, :except => :edit + resources :tickets, except: :edit end + end end diff --git a/engines/support/test/functional/tickets_controller_test.rb b/engines/support/test/functional/tickets_controller_test.rb index e103d01..1d074cc 100644 --- a/engines/support/test/functional/tickets_controller_test.rb +++ b/engines/support/test/functional/tickets_controller_test.rb @@ -112,5 +112,22 @@ class TicketsControllerTest < ActionController::TestCase assert_not_nil assigns(:ticket).comments.first.posted_by assert_equal assigns(:ticket).comments.first.posted_by, @current_user.id end + + test "close ticket" do + login + open_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id + post :close, id: open_ticket.id + assert !open_ticket.reload.is_open + end + + test "reopen ticket" do + login + open_ticket = FactoryGirl.create :ticket_with_comment, + created_by: @current_user.id, is_open: false + post :open, id: open_ticket.id + assert open_ticket.reload.is_open + end + end -- cgit v1.2.3 From c10f9311678ff2183443bc03e153b30d3b68ff74 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 20 May 2014 13:09:59 +0200 Subject: Controller#flash_for instead of FlashResponder FlashResponder added a flash before responding. However at the point of responding objects have already been saved. So there is no way to test if they were changed. Now instead we can call flash_for resource before resource.save and it will add the flash messages only if the resource was actually changed. --- engines/support/app/controllers/tickets_controller.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'engines/support') diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index bb98277..7b6a7a0 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -23,10 +23,11 @@ class TicketsController < ApplicationController @ticket.comments.last.posted_by = current_user.id @ticket.comments.last.private = false unless admin? @ticket.created_by = current_user.id + flash_for @ticket if @ticket.save && !logged_in? flash[:success] = t(:access_ticket_text, :full_url => ticket_url(@ticket.id)) end - respond_with(@ticket, :location => auto_ticket_path(@ticket)) + respond_with @ticket, :location => auto_ticket_path(@ticket) end def show @@ -61,6 +62,7 @@ class TicketsController < ApplicationController @ticket.comments.last.private = false unless admin? end + flash_for @ticket @ticket.save respond_with @ticket, location: redirection_path end @@ -82,10 +84,6 @@ class TicketsController < ApplicationController @title = t(:tickets) end - def self.responder - Responders::FlashResponder - end - private # -- cgit v1.2.3 From 560eb039f4778257559395583e1233d052d44127 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 20 May 2014 13:50:32 +0200 Subject: flash_for with_errors option displays error messages --- engines/support/app/controllers/tickets_controller.rb | 2 +- engines/support/app/views/tickets/edit.html.haml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'engines/support') diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index 7b6a7a0..9c1a741 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -62,7 +62,7 @@ class TicketsController < ApplicationController @ticket.comments.last.private = false unless admin? end - flash_for @ticket + flash_for @ticket, with_errors: true @ticket.save respond_with @ticket, location: redirection_path end diff --git a/engines/support/app/views/tickets/edit.html.haml b/engines/support/app/views/tickets/edit.html.haml index 99afa2a..03bda7d 100644 --- a/engines/support/app/views/tickets/edit.html.haml +++ b/engines/support/app/views/tickets/edit.html.haml @@ -1,4 +1,5 @@ - @show_navigation = params[:user_id].present? +- @comment = TicketComment.new .ticket = render 'tickets/edit_form' -- cgit v1.2.3 From 19bce0f114180f355f0df367cf6d21bd957734a6 Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 22 May 2014 14:57:29 +0200 Subject: tickets: structure i18n --- .../support/app/controllers/tickets_controller.rb | 2 +- engines/support/app/helpers/tickets_helper.rb | 14 +-- engines/support/app/models/ticket_comment.rb | 5 + .../support/app/views/tickets/_comment.html.haml | 5 +- .../support/app/views/tickets/_edit_form.html.haml | 29 ++--- .../app/views/tickets/_new_comment_form.html.haml | 6 +- engines/support/app/views/tickets/_tabs.html.haml | 18 ++- engines/support/app/views/tickets/index.html.haml | 10 +- engines/support/app/views/tickets/new.html.haml | 2 +- engines/support/config/locales/en.yml | 124 +++++++++++++++++---- 10 files changed, 142 insertions(+), 73 deletions(-) (limited to 'engines/support') diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index 9c1a741..857d071 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -81,7 +81,7 @@ class TicketsController < ApplicationController protected def set_title - @title = t(:tickets) + @title = t("layouts.title.tickets") end private diff --git a/engines/support/app/helpers/tickets_helper.rb b/engines/support/app/helpers/tickets_helper.rb index 7af50d6..11b02e4 100644 --- a/engines/support/app/helpers/tickets_helper.rb +++ b/engines/support/app/helpers/tickets_helper.rb @@ -35,13 +35,7 @@ module TicketsHelper # def link_to_status(new_status) - if new_status == "open" - label = t(:open_tickets) - elsif new_status == "closed" - label = t(:closed_tickets) - elsif new_status == "all" - label = t(:all_tickets) - end + label = t(:".#{new_status}", cascade: true) link_to label, auto_tickets_path(:open_status => new_status, :sort_order => search_order) end @@ -62,11 +56,7 @@ module TicketsHelper direction = 'desc' end - if order_field == 'updated' - label = t(:updated) - elsif order_field == 'created' - label = t(:created) - end + label = t(:".#{order_field}", cascade: true) link_to auto_tickets_path(:sort_order => order_field + '_at_' + direction, :open_status => search_status) do arrow + label diff --git a/engines/support/app/models/ticket_comment.rb b/engines/support/app/models/ticket_comment.rb index bed5237..2c5df41 100644 --- a/engines/support/app/models/ticket_comment.rb +++ b/engines/support/app/models/ticket_comment.rb @@ -18,6 +18,11 @@ class TicketComment # view :by_body #end + # translations are in the same scope as those of a "proper" couchrest model + def self.i18n_scope + "couchrest" + end + def is_comment_validated? !!posted_by end diff --git a/engines/support/app/views/tickets/_comment.html.haml b/engines/support/app/views/tickets/_comment.html.haml index 778ca13..65ec394 100644 --- a/engines/support/app/views/tickets/_comment.html.haml +++ b/engines/support/app/views/tickets/_comment.html.haml @@ -1,4 +1,5 @@ -- if admin? or !comment.private # only show comment if user is admin or comment is not private +- # only show comment if user is admin or comment is not private +- if admin? or !comment.private %tr %td.user %div @@ -17,4 +18,4 @@ %span.label.label-important = t(:private) %td.comment - = simple_format(comment.body) \ No newline at end of file + = simple_format(comment.body) diff --git a/engines/support/app/views/tickets/_edit_form.html.haml b/engines/support/app/views/tickets/_edit_form.html.haml index 22815f2..522489e 100644 --- a/engines/support/app/views/tickets/_edit_form.html.haml +++ b/engines/support/app/views/tickets/_edit_form.html.haml @@ -23,29 +23,24 @@ %p.first - if @ticket.is_open? %span.label.label-info - %b{style: 'padding:10px'}= t(:open) - = f.button :loading, t(:close), class: 'btn-mini' + %b{style: 'padding:10px'}= t("tickets.status.open") + = f.button :loading, t("tickets.action.close"), class: 'btn-mini' - else %span.label.label-success - %b{style: 'padding:10px'}= t(:closed) - = f.button :loading, t(:open), class: 'btn-mini' - %span.label.label-clear= t(:created_by_on, :user => created_by, :time => @ticket.created_at.to_s(:short)).html_safe + %b{style: 'padding:10px'}= t("tickets.status.closed") + = f.button :loading, t("tickets.action.open"), class: 'btn-mini' + %span.label.label-clear + = t("tickets.created_by_on", user: created_by, time: @ticket.created_at.to_s(:short), cascade: true).html_safe = simple_form_for @ticket do |f| = hidden_ticket_fields - %div= t(:subject) - = f.text_field :subject, :class => 'large full-width' + = f.input :subject, input_html: {:class => 'large full-width'} .row-fluid .span4 - %div= t(:status) - = f.select :is_open, [[t(:open), "true"], [t(:closed), "false"]] + = f.input :is_open, as: :select, collection: [:true, :false], include_blank: false .span4 - %div= t(:email) - = f.text_field :email + = f.input :email .span4 - %div - = t(:regarding_account) - = regarding_user_link - = f.text_field :regarding_user - = f.button :loading, t(:save), :value => 'save' + = f.input :regarding_user, label: Ticket.human_attribute_name(:regarding_user) + regarding_user_link + = f.button :loading - if admin? - = link_to t(:destroy), auto_ticket_path(@ticket), :confirm => t(:are_you_sure), :method => :delete, :class => 'btn' + = link_to t(".destroy", cascade: true), auto_ticket_path(@ticket), :confirm => t("tickets.confirm.destroy.are_you_sure", cascade: true), :method => :delete, :class => 'btn' diff --git a/engines/support/app/views/tickets/_new_comment_form.html.haml b/engines/support/app/views/tickets/_new_comment_form.html.haml index 40c737f..b829b6b 100644 --- a/engines/support/app/views/tickets/_new_comment_form.html.haml +++ b/engines/support/app/views/tickets/_new_comment_form.html.haml @@ -7,7 +7,7 @@ = c.input :body, :label => false, :as => :text, :input_html => {:class => "full-width", :rows=> 5} - if admin? = c.input :private, :as => :boolean, :label => false, :inline_label => true - = f.button :loading, t(:post_reply), class: 'btn-primary', value: 'post_reply' + = f.button :loading, t(".post_reply"), class: 'btn-primary', value: 'post_reply' - if logged_in? && @ticket.is_open - = f.button :loading, t(:reply_and_close), value: 'reply_and_close' - = link_to t(:cancel), auto_tickets_path, :class => :btn + = f.button :loading, t(".reply_and_close"), value: 'reply_and_close' + = link_to t(".cancel"), auto_tickets_path, :class => :btn diff --git a/engines/support/app/views/tickets/_tabs.html.haml b/engines/support/app/views/tickets/_tabs.html.haml index 445a909..7872bb5 100644 --- a/engines/support/app/views/tickets/_tabs.html.haml +++ b/engines/support/app/views/tickets/_tabs.html.haml @@ -3,21 +3,17 @@ -# - unless action?(:new) or action?(:create) %ul.nav.nav-pills.pull-right.slim - %li{:class=> ("active" if search_order.start_with? 'created_at')} - = link_to_order('created') - %li{:class=> ("active" if search_order.start_with? 'updated_at')} - = link_to_order('updated') + - %w(created updated).each do |order| + %li{:class=> ("active" if search_order.start_with? order)} + = link_to_order(order) -# -# STATUS FILTER TABS -# %ul.nav.nav-tabs - if logged_in? - %li{:class => ("active" if search_status == 'open')} - = link_to_status 'open' - %li{:class => ("active" if search_status == 'closed')} - = link_to_status 'closed' - %li{:class => ("active" if search_status == 'all')} - = link_to_status 'all' + - %w(open closed all).each do |status| + %li{:class => ("active" if search_status == status)} + = link_to_status status %li{:class => ("active" if action?(:new) || action?(:create))} - = link_to icon(:plus, :black) + t(:new_ticket), auto_new_ticket_path + = link_to icon(:plus, :black) + t(".new", cascade: true), auto_new_ticket_path diff --git a/engines/support/app/views/tickets/index.html.haml b/engines/support/app/views/tickets/index.html.haml index a4df6e3..526cd6d 100644 --- a/engines/support/app/views/tickets/index.html.haml +++ b/engines/support/app/views/tickets/index.html.haml @@ -5,15 +5,15 @@ %table.table.table-striped.table-bordered %thead %tr - %th= t(:subject) - %th= t(:created) - %th= t(:updated) - %th= t(:voices) + %th= t(".subject") + %th= t(".created") + %th= t(".updated") + %th= t(".voices") %tbody - if @tickets.any? = render @tickets.all - else %tr - %td{:colspan=>4}= t(:none) + %td{:colspan=>4}= t(".none") = paginate @tickets diff --git a/engines/support/app/views/tickets/new.html.haml b/engines/support/app/views/tickets/new.html.haml index 3de5fe9..d3580f9 100644 --- a/engines/support/app/views/tickets/new.html.haml +++ b/engines/support/app/views/tickets/new.html.haml @@ -11,7 +11,7 @@ = f.input :email = f.input :regarding_user = f.simple_fields_for :comments, @comment do |c| - = c.input :body, :label => t(:description), :as => :text, :input_html => {:class => "full-width", :rows=> 5} + = c.input :body, :as => :text, :input_html => {:class => "full-width", :rows=> 5} - if admin? = c.input :private, :as => :boolean, :label => false, :inline_label => true = f.button :wrapped, cancel: (logged_in? ? auto_tickets_path : home_path) diff --git a/engines/support/config/locales/en.yml b/engines/support/config/locales/en.yml index 342adea..f2caecc 100644 --- a/engines/support/config/locales/en.yml +++ b/engines/support/config/locales/en.yml @@ -1,22 +1,104 @@ en: - access_ticket_text: > - You can later access this ticket at the URL %{full_url}. You might want to bookmark this page to find it again. - Anybody with this URL will be able to access this ticket, so if you are on a shared computer you might want to - remove it from the browser history. - support_tickets: "Support Tickets" - all_tickets: "All Tickets" - my_tickets: "My Tickets" - open_tickets: "Open Tickets" - closed_tickets: "Closed Tickets" - new_ticket: "New Ticket" - tickets: "Tickets" - subject: "Subject" - destroy: "Destroy" - open: "Open" - closed: "Closed" - close: "Close" - post_reply: "Post Reply" - reply_and_close: "Reply and Close" - description: "Description" - ticket: "Ticket" - regarding_account: "Regarding Account" \ No newline at end of file + # translations used in the layout views or @title + layouts: + # fallback for all translations of "tickets" nested below: + tickets: "Tickets" + title: + tickets: "Tickets" + header: + tickets: "Tickets" + navigation: + tickets: "Support Tickets" + # Translations used in the views inside the tickets directory + tickets: + # If these do not exist they will be looked up in the global scope. + all: "All Tickets" + open: "Open Tickets" + closed: "Closed Tickets" + new: "New Ticket" + created: "Created at" + updated: "Updated at" + subject: "couchrest.models.tickets.attributes.subject" + status: + open: "Open" + closed: "Closed" + action: + open: "Open" + close: "Close" + confirm: + destroy: + are_you_sure: "Are you sure you want to destroy this ticket?" + # If you want to be more specific you can use the partial as a scope: + tabs: + all: "All Tickets" + open: "Open Tickets" + closed: "Closed Tickets" + index: + none: "No tickets have been found." + voices: "Voices" + destroy: "Destroy" + post_reply: "Post Reply" + reply_and_close: "Reply and Close" + access_ticket_text: > + You can later access this ticket at the URL %{full_url}. You might want to bookmark this page to find it again. + Anybody with this URL will be able to access this ticket, so if you are on a shared computer you might want to + remove it from the browser history. + # rails i18n + helpers: + # translations used for submit buttons. simple form picks these up + submit: + ticket: + create: "Submit Ticket" + update: "Update Ticket" + # translations for the model and its attributes + # serve as fallback for simpleform labels + couchrest: + models: + ticket: "Ticket" + ticket_comment: "Comment" + attributes: + ticket: + # these will fallback to translations in the "attributes" scope + subject: "Subject" + email: "Email" + regarding_user: "Regarding User" + regarding_account: "Regarding Account" + is_open: "Status" + ticket_comment: + body: "Description" + private: "private" + simple_form: + # labels next to the field + labels: + # these will fallback to the human_name translations of the model + ticket: + # these will fallback to translations in "simple_form.labels.defaults" + regarding_: + # you can be specific about translations for a given action: + #edit: + # regaring_user: + email: "Email" + comments: + # these will fall back to "simple_form.labels.comments" + body: "Description" + # comments: ~ + options: + ticket: + is_open: + "true": "Open" + "false": "Closed" + # mouse over hints for the given fields + hints: + ticket: + email: "Please provide an email address so we can get back to you." + # these will fallback to translations in "simple_form.hints.defaults" + # placeholders inside the fields before anything was typed + #placeholders: + # ticket: ~ + # these will fallback to translations in "simple_form.placeholders.defaults" + + # these are generic defaults. They should be moved into the toplevel + # attributes: + #simple_form: + #labels: + # defaults: -- cgit v1.2.3 From 4085e3fabef6427cd3f8be9b61c209bd82eaa595 Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 24 May 2014 10:33:31 +0200 Subject: navigation works with empty locale selected Just in case some translation keys are not present things should still work and make sense. So translation keys should be picked in a meaningful way and scoped rather than prefixed. For example overview.account will turn into "Account" if no translation is present while "overview_account" will turn into "Overview Account". We usually want the former. --- engines/support/config/locales/en.yml | 1 + 1 file changed, 1 insertion(+) (limited to 'engines/support') diff --git a/engines/support/config/locales/en.yml b/engines/support/config/locales/en.yml index f2caecc..8d2af67 100644 --- a/engines/support/config/locales/en.yml +++ b/engines/support/config/locales/en.yml @@ -1,4 +1,5 @@ en: + support_tickets: "Support" # translations used in the layout views or @title layouts: # fallback for all translations of "tickets" nested below: -- cgit v1.2.3 From cc59ce53e52bf48d97de16d66012e8309bf98fe8 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 27 May 2014 16:32:50 +0200 Subject: add btn helper for link_to with .btn Also translates the first arg if it's a symbol and adds more btn- classes if given as html_options[:type] --- engines/support/app/views/tickets/_edit_form.html.haml | 2 +- engines/support/app/views/tickets/_new_comment_form.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'engines/support') diff --git a/engines/support/app/views/tickets/_edit_form.html.haml b/engines/support/app/views/tickets/_edit_form.html.haml index 522489e..9adc2cc 100644 --- a/engines/support/app/views/tickets/_edit_form.html.haml +++ b/engines/support/app/views/tickets/_edit_form.html.haml @@ -43,4 +43,4 @@ = f.input :regarding_user, label: Ticket.human_attribute_name(:regarding_user) + regarding_user_link = f.button :loading - if admin? - = link_to t(".destroy", cascade: true), auto_ticket_path(@ticket), :confirm => t("tickets.confirm.destroy.are_you_sure", cascade: true), :method => :delete, :class => 'btn' + = btn t(".destroy", cascade: true), auto_ticket_path(@ticket), confirm: t("tickets.confirm.destroy.are_you_sure", cascade: true), method: :delete diff --git a/engines/support/app/views/tickets/_new_comment_form.html.haml b/engines/support/app/views/tickets/_new_comment_form.html.haml index b829b6b..711421d 100644 --- a/engines/support/app/views/tickets/_new_comment_form.html.haml +++ b/engines/support/app/views/tickets/_new_comment_form.html.haml @@ -10,4 +10,4 @@ = f.button :loading, t(".post_reply"), class: 'btn-primary', value: 'post_reply' - if logged_in? && @ticket.is_open = f.button :loading, t(".reply_and_close"), value: 'reply_and_close' - = link_to t(".cancel"), auto_tickets_path, :class => :btn + = btn t(".cancel"), auto_tickets_path -- cgit v1.2.3 From df1c2438fcfe39edfb46546be8fcee5021f95fc3 Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 28 May 2014 09:26:17 +0200 Subject: destroy_btn helper method --- engines/support/app/views/tickets/_edit_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'engines/support') diff --git a/engines/support/app/views/tickets/_edit_form.html.haml b/engines/support/app/views/tickets/_edit_form.html.haml index 9adc2cc..889dac2 100644 --- a/engines/support/app/views/tickets/_edit_form.html.haml +++ b/engines/support/app/views/tickets/_edit_form.html.haml @@ -43,4 +43,4 @@ = f.input :regarding_user, label: Ticket.human_attribute_name(:regarding_user) + regarding_user_link = f.button :loading - if admin? - = btn t(".destroy", cascade: true), auto_ticket_path(@ticket), confirm: t("tickets.confirm.destroy.are_you_sure", cascade: true), method: :delete + = destroy_btn t(".destroy", cascade: true), auto_ticket_path(@ticket) -- cgit v1.2.3 From 6a5e5edf54c76bea3de93475d949c3372d376a81 Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 29 May 2014 20:10:11 +0200 Subject: adopt tests to new translations --- engines/support/test/integration/create_ticket_test.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'engines/support') diff --git a/engines/support/test/integration/create_ticket_test.rb b/engines/support/test/integration/create_ticket_test.rb index 0f8453c..90e9a8a 100644 --- a/engines/support/test/integration/create_ticket_test.rb +++ b/engines/support/test/integration/create_ticket_test.rb @@ -7,7 +7,7 @@ class CreateTicketTest < BrowserIntegrationTest click_on 'Get Help' fill_in 'Subject', with: 'test ticket' fill_in 'Description', with: 'description of the problem goes here' - click_on 'Create Ticket' + click_on 'Submit 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) @@ -20,12 +20,12 @@ class CreateTicketTest < BrowserIntegrationTest click_on 'Get Help' fill_in 'Subject', with: 'test ticket' fill_in 'Email', with: 'invalid data' - fill_in 'Regarding user', with: 'some user' + fill_in 'Regarding User', with: 'some user' fill_in 'Description', with: 'description of the problem goes here' - click_on 'Create Ticket' + click_on 'Submit Ticket' assert page.has_content?("is invalid") assert_equal 'invalid data', find_field('Email').value - assert_equal 'some user', find_field('Regarding user').value + assert_equal 'some user', find_field('Regarding User').value end test "prefills fields" do @@ -35,7 +35,7 @@ class CreateTicketTest < BrowserIntegrationTest click_on "New Ticket" email = "#{@user.login}@#{APP_CONFIG[:domain]}" assert_equal email, find_field('Email').value - assert_equal @user.login, find_field('Regarding user').value + assert_equal @user.login, find_field('Regarding User').value end test "no prefill of email without email service" do @@ -44,7 +44,7 @@ class CreateTicketTest < BrowserIntegrationTest click_on "Support Tickets" click_on "New Ticket" assert_equal "", find_field('Email').value - assert_equal @user.login, find_field('Regarding user').value + assert_equal @user.login, find_field('Regarding User').value end test "cleared email field should remain clear" do @@ -55,7 +55,7 @@ class CreateTicketTest < BrowserIntegrationTest fill_in 'Subject', with: 'test ticket' fill_in 'Email', with: '' fill_in 'Description', with: 'description of the problem goes here' - click_on 'Create Ticket' + click_on 'Submit Ticket' ticket = Ticket.last assert_equal "", ticket.email ticket.destroy -- cgit v1.2.3 From 9e3be686ff2751707369894382293924420830d0 Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 29 May 2014 20:11:29 +0200 Subject: fix flash for creating anonymous tickets --- engines/support/app/controllers/tickets_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'engines/support') diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index 857d071..fab26f3 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -25,7 +25,9 @@ class TicketsController < ApplicationController @ticket.created_by = current_user.id flash_for @ticket if @ticket.save && !logged_in? - flash[:success] = t(:access_ticket_text, :full_url => ticket_url(@ticket.id)) + flash[:success] += t 'tickets.access_ticket_text', + full_url: ticket_url(@ticket.id), + default: "" end respond_with @ticket, :location => auto_ticket_path(@ticket) end -- cgit v1.2.3 From 366ff2e7f5ecd44aab1cddfd0a7b73ab7b213e85 Mon Sep 17 00:00:00 2001 From: elijah Date: Tue, 3 Jun 2014 01:12:17 -0700 Subject: tickets: fix bug that allow index of other users --- .../support/app/controllers/tickets_controller.rb | 22 ++++++++++++++++------ .../test/functional/tickets_controller_test.rb | 15 ++++++++++----- 2 files changed, 26 insertions(+), 11 deletions(-) (limited to 'engines/support') diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index fab26f3..1ccbd16 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -4,10 +4,10 @@ class TicketsController < ApplicationController respond_to :html, :json #has_scope :open, :type => boolean + before_filter :fetch_user before_filter :require_login, :only => [:index] before_filter :fetch_ticket, except: [:new, :create, :index] - before_filter :require_ticket_access, except: [:new, :create, :index] - before_filter :fetch_user + before_filter :require_ticket_access, except: [:new, :create] before_filter :set_title def new @@ -129,14 +129,24 @@ class TicketsController < ApplicationController end def ticket_access? - admin? or - @ticket.created_by.blank? or - current_user.id == @ticket.created_by + admin? or ( + @ticket && + @ticket.created_by.blank? + ) or ( + @ticket && + @ticket.created_by == current_user.id + ) or ( + @ticket.nil? && + @user && + @user.id == current_user.id + ) end def fetch_user if params[:user_id] @user = User.find(params[:user_id]) + else + @user = current_user end end @@ -146,7 +156,7 @@ class TicketsController < ApplicationController def search_options(params) params.merge( :admin_status => params[:user_id] ? 'mine' : 'all', - :user_id => @user ? @user.id : current_user.id, + :user_id => @user.id, :is_admin => admin? ) end diff --git a/engines/support/test/functional/tickets_controller_test.rb b/engines/support/test/functional/tickets_controller_test.rb index 1d074cc..ebaa3a4 100644 --- a/engines/support/test/functional/tickets_controller_test.rb +++ b/engines/support/test/functional/tickets_controller_test.rb @@ -45,8 +45,7 @@ class TicketsControllerTest < ActionController::TestCase user = find_record :user ticket = find_record :ticket, :created_by => user.id get :show, :id => ticket.id - assert_response :redirect - assert_redirected_to login_url + assert_login_required end test "user tickets are visible to creator" do @@ -57,13 +56,19 @@ class TicketsControllerTest < ActionController::TestCase assert_response :success end - test "other users tickets are not visible" do + test "ticket of other user is not visible" do other_user = find_record :user ticket = find_record :ticket, :created_by => other_user.id login get :show, :id => ticket.id - assert_response :redirect - assert_redirected_to home_url + assert_access_denied + end + + test "ticket list of other user is not visible" do + other_user = find_record :user + login + get :index, :user_id => other_user.id + assert_access_denied end test "should create unauthenticated ticket" do -- cgit v1.2.3