From 0261e82686ec4fcfc8b633664fadb1dd6d9c8070 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 13 May 2014 10:52:55 +0200 Subject: keep empty email field if user removed prefill We should respect the users choice. We can still get their email from the user id if we really need to. --- .../support/app/controllers/tickets_controller.rb | 1 - .../support/test/integration/create_ticket_test.rb | 28 ++++++++++++++++++++++ 2 files changed, 28 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 d552209..8ec8e4d 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -22,7 +22,6 @@ class TicketsController < ApplicationController @ticket.comments.last.posted_by = current_user.id @ticket.comments.last.private = false unless admin? @ticket.created_by = current_user.id - @ticket.email = current_user.email_address if current_user.email_address if @ticket.save flash[:notice] = t(:thing_was_successfully_created, :thing => t(:ticket)) diff --git a/engines/support/test/integration/create_ticket_test.rb b/engines/support/test/integration/create_ticket_test.rb index 2583fc7..59b263e 100644 --- a/engines/support/test/integration/create_ticket_test.rb +++ b/engines/support/test/integration/create_ticket_test.rb @@ -25,4 +25,32 @@ class CreateTicketTest < BrowserIntegrationTest assert page.has_content?("is invalid") end + test "prefills email when user has email service" do + login FactoryGirl.create(:premium_user) + visit '/' + click_on "Support Tickets" + click_on "New Ticket" + email = "#{@user.login}@#{APP_CONFIG[:domain]}" + assert_equal email, find_field('Email').value + end + + test "no prefill of email without email service" do + login + visit '/' + click_on "Support Tickets" + click_on "New Ticket" + assert_equal "", find_field('Email').value + end + + test "cleared email field should remain clear" do + login FactoryGirl.create(:premium_user) + visit '/' + click_on "Support Tickets" + click_on "New Ticket" + fill_in 'Subject', with: 'test ticket' + fill_in 'Email', with: '' + fill_in 'Description', with: 'description of the problem goes here' + click_on 'Create Ticket' + assert_nil Ticket.last.email + end end -- cgit v1.2.3 From 81a4a0527639fe4b560b8d98f977f6dbac67bb41 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 13 May 2014 13:52:16 +0200 Subject: prefill ticket form from the model - fixes #5657 email and regarding user fields can be set to defaults based on created_by user. If these fields are emptied by the submitting user they will be set to whereas they are nil if they have not been initialized. In that case we will use meaningful defaults from the user who created the ticket. --- .../support/app/controllers/tickets_controller.rb | 5 ++--- engines/support/app/models/ticket.rb | 21 ++++++++++++--------- .../support/app/views/tickets/_edit_form.html.haml | 2 +- engines/support/app/views/tickets/new.html.haml | 4 ++-- .../support/test/integration/create_ticket_test.rb | 12 ++++++++++-- 5 files changed, 27 insertions(+), 17 deletions(-) (limited to 'engines/support') diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb index 8ec8e4d..99357ab 100644 --- a/engines/support/app/controllers/tickets_controller.rb +++ b/engines/support/app/controllers/tickets_controller.rb @@ -12,6 +12,7 @@ class TicketsController < ApplicationController def new @ticket = Ticket.new + @ticket.created_by = current_user.id @ticket.comments.build end @@ -24,9 +25,7 @@ class TicketsController < ApplicationController @ticket.created_by = current_user.id if @ticket.save flash[:notice] = t(:thing_was_successfully_created, :thing => t(:ticket)) - - # cannot set this until ticket has been saved, as @ticket.id will not be set - if !logged_in? and flash[:notice] + if !logged_in? flash[:notice] += " " + t(:access_ticket_text, :full_url => ticket_url(@ticket.id)) end end diff --git a/engines/support/app/models/ticket.rb b/engines/support/app/models/ticket.rb index cd22758..d5a0b5d 100644 --- a/engines/support/app/models/ticket.rb +++ b/engines/support/app/models/ticket.rb @@ -19,8 +19,6 @@ class Ticket < CouchRest::Model::Base timestamps! - before_validation :set_email, :set_regarding_user, :on => :create - design do view :by_updated_at view :by_created_at @@ -34,7 +32,12 @@ class Ticket < CouchRest::Model::Base end validates :subject, :presence => true - validates :email, :allow_blank => true, :format => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/ + + # email can have three states: + # * nil - prefilled with created_by's email + # * "" - cleared + # * valid email address + validates :email, :allow_blank => true, :format => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/ def self.search(options = {}) @selection = TicketSelection.new(options) @@ -48,15 +51,15 @@ class Ticket < CouchRest::Model::Base end def is_creator_validated? - !!created_by + created_by_user.is_a? User end - def set_email - self.email = nil if self.email == "" + def email + read_attribute(:email) || created_by_user.email end - def set_regarding_user - self.regarding_user = nil if self.regarding_user == "" + def regarding_user + read_attribute(:regarding_user) || created_by_user.login end def close @@ -95,7 +98,7 @@ class Ticket < CouchRest::Model::Base end def created_by_user - User.find(self.created_by) + User.find(self.created_by) || AnonymousUser.new end def regarding_user_actual_user diff --git a/engines/support/app/views/tickets/_edit_form.html.haml b/engines/support/app/views/tickets/_edit_form.html.haml index bf175fe..fb279fb 100644 --- a/engines/support/app/views/tickets/_edit_form.html.haml +++ b/engines/support/app/views/tickets/_edit_form.html.haml @@ -1,6 +1,6 @@ :ruby # created by user link - if @ticket.created_by_user + if @ticket.is_creator_validated? created_by = link_to @ticket.created_by_user.login, @ticket.created_by_user else created_by = t(:anonymous) diff --git a/engines/support/app/views/tickets/new.html.haml b/engines/support/app/views/tickets/new.html.haml index ab008d2..3de5fe9 100644 --- a/engines/support/app/views/tickets/new.html.haml +++ b/engines/support/app/views/tickets/new.html.haml @@ -8,8 +8,8 @@ = simple_form_for @ticket, :validate => true, :html => {:class => 'form-horizontal'} do |f| = hidden_ticket_fields = f.input :subject - = f.input :email, input_html: {value: user.email_address} - = f.input :regarding_user, input_html: {value: user.login} + = 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} - if admin? diff --git a/engines/support/test/integration/create_ticket_test.rb b/engines/support/test/integration/create_ticket_test.rb index 59b263e..0f8453c 100644 --- a/engines/support/test/integration/create_ticket_test.rb +++ b/engines/support/test/integration/create_ticket_test.rb @@ -20,18 +20,22 @@ 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 'Description', with: 'description of the problem goes here' click_on 'Create Ticket' assert page.has_content?("is invalid") + assert_equal 'invalid data', find_field('Email').value + assert_equal 'some user', find_field('Regarding user').value end - test "prefills email when user has email service" do + test "prefills fields" do login FactoryGirl.create(:premium_user) visit '/' click_on "Support Tickets" 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 end test "no prefill of email without email service" do @@ -40,6 +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 end test "cleared email field should remain clear" do @@ -51,6 +56,9 @@ class CreateTicketTest < BrowserIntegrationTest fill_in 'Email', with: '' fill_in 'Description', with: 'description of the problem goes here' click_on 'Create Ticket' - assert_nil Ticket.last.email + ticket = Ticket.last + assert_equal "", ticket.email + ticket.destroy end + end -- cgit v1.2.3 From 3278e474a32ef4926b1dab0d97ca4df1c59aa2a0 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 13 May 2014 17:21:08 +0200 Subject: adjust tests to new config and method implementation Ticket.is_creator_vlidated? now actually fetches the user from the db and returns false if it does not exist. --- engines/support/app/models/ticket.rb | 6 +++++- engines/support/test/functional/tickets_controller_test.rb | 4 ++-- engines/support/test/unit/ticket_test.rb | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) (limited to 'engines/support') diff --git a/engines/support/app/models/ticket.rb b/engines/support/app/models/ticket.rb index d5a0b5d..bf5df53 100644 --- a/engines/support/app/models/ticket.rb +++ b/engines/support/app/models/ticket.rb @@ -98,7 +98,11 @@ class Ticket < CouchRest::Model::Base end def created_by_user - User.find(self.created_by) || AnonymousUser.new + if self.created_by + User.find(self.created_by) || AnonymousUser.new + else + AnonymousUser.new + end end def regarding_user_actual_user diff --git a/engines/support/test/functional/tickets_controller_test.rb b/engines/support/test/functional/tickets_controller_test.rb index d746b59..fc4a6f8 100644 --- a/engines/support/test/functional/tickets_controller_test.rb +++ b/engines/support/test/functional/tickets_controller_test.rb @@ -85,7 +85,7 @@ class TicketsControllerTest < ActionController::TestCase test "should create authenticated ticket" do - params = {:subject => "auth ticket test subject", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}} + params = {:subject => "auth ticket test subject",:email => "", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}} login @@ -97,7 +97,7 @@ class TicketsControllerTest < ActionController::TestCase assert_not_nil assigns(:ticket).created_by assert_equal assigns(:ticket).created_by, @current_user.id - assert_equal assigns(:ticket).email, @current_user.email_address + assert_equal "", assigns(:ticket).email assert_equal 1, assigns(:ticket).comments.count assert_not_nil assigns(:ticket).comments.first.posted_by diff --git a/engines/support/test/unit/ticket_test.rb b/engines/support/test/unit/ticket_test.rb index f5e6ea7..678d8dc 100644 --- a/engines/support/test/unit/ticket_test.rb +++ b/engines/support/test/unit/ticket_test.rb @@ -27,10 +27,10 @@ class TicketTest < ActiveSupport::TestCase end test "creation validated" do + user = FactoryGirl.create :user @sample = Ticket.new assert !@sample.is_creator_validated? - #p current_user - @sample.created_by = 22 #current_user + @sample.created_by = user.id assert @sample.is_creator_validated? end -- cgit v1.2.3