From 6393e398c14d440c1eb658eab4f1bdaca7ec403c Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Thu, 4 Apr 2019 11:01:20 +0800 Subject: [PATCH] Refactoring: Migrate email_process_customer_selection_based_on_sender_recipient_test to RSpec --- spec/models/channel/email_parser_spec.rb | 95 ++++++++++++++++++- spec/models/ticket/article_spec.rb | 40 +++++++- ...election_based_on_sender_recipient_test.rb | 78 --------------- 3 files changed, 131 insertions(+), 82 deletions(-) delete mode 100644 test/unit/email_process_customer_selection_based_on_sender_recipient_test.rb diff --git a/spec/models/channel/email_parser_spec.rb b/spec/models/channel/email_parser_spec.rb index 4b8963f8f..42629063d 100644 --- a/spec/models/channel/email_parser_spec.rb +++ b/spec/models/channel/email_parser_spec.rb @@ -85,7 +85,65 @@ RSpec.describe Channel::EmailParser, type: :model do end end - describe 'associating emails to tickets' do + describe 'creating new tickets' do + context 'when subject contains no ticket reference' do + let(:raw_mail) { <<~RAW.chomp } + From: foo@bar.com + To: baz@qux.net + Subject: Foo + + Lorem ipsum dolor + RAW + + it 'creates a ticket and article' do + expect { Channel::EmailParser.new.process({}, raw_mail) } + .to change { Ticket.count }.by(1) + .and change { Ticket::Article.count }.by_at_least(1) # triggers may cause additional articles to be created + end + + it 'sets #title to email subject' do + Channel::EmailParser.new.process({}, raw_mail) + + expect(Ticket.last.title).to eq('Foo') + end + + it 'sets #state to "new"' do + Channel::EmailParser.new.process({}, raw_mail) + + expect(Ticket.last.state.name).to eq('new') + end + + context 'when from address matches an existing agent' do + let!(:agent) { create(:agent_user, email: 'foo@bar.com') } + + it 'sets article.sender to "Agent"' do + Channel::EmailParser.new.process({}, raw_mail) + + expect(Ticket::Article.last.sender.name).to eq('Agent') + end + end + + context 'when from address matches an existing customer' do + let!(:customer) { create(:customer_user, email: 'foo@bar.com') } + + it 'sets article.sender to "Customer"' do + Channel::EmailParser.new.process({}, raw_mail) + + expect(Ticket.last.articles.first.sender.name).to eq('Customer') + end + end + + context 'when from address is unrecognized' do + it 'sets article.sender to "Customer"' do + Channel::EmailParser.new.process({}, raw_mail) + + expect(Ticket.last.articles.first.sender.name).to eq('Customer') + end + end + end + end + + describe 'associating emails to existing tickets' do let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail001.box') } let(:ticket_ref) { Setting.get('ticket_hook') + Setting.get('ticket_hook_divider') + ticket.number } let(:ticket) { create(:ticket) } @@ -183,7 +241,40 @@ RSpec.describe Channel::EmailParser, type: :model do end end - describe 'sender/recipient address formatting' do + describe 'assigning ticket.customer' do + let(:agent) { create(:agent_user) } + let(:customer) { create(:customer_user) } + + let(:raw_mail) { <<~RAW.chomp } + From: #{agent.email} + To: #{customer.email} + Subject: Foo + + Lorem ipsum dolor + RAW + + context 'when "postmaster_sender_is_agent_search_for_customer" setting is true (default)' do + it 'sets ticket.customer to user with To: email' do + expect { Channel::EmailParser.new.process({}, raw_mail) } + .to change { Ticket.count }.by(1) + + expect(Ticket.last.customer).to eq(customer) + end + end + + context 'when "postmaster_sender_is_agent_search_for_customer" setting is false' do + before { Setting.set('postmaster_sender_is_agent_search_for_customer', false) } + + it 'sets ticket.customer to user with To: email' do + expect { Channel::EmailParser.new.process({}, raw_mail) } + .to change { Ticket.count }.by(1) + + expect(Ticket.last.customer).to eq(agent) + end + end + end + + describe 'formatting to/from addresses' do # see https://github.com/zammad/zammad/issues/2198 context 'when sender address contains spaces (#2198)' do let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail071.box') } diff --git a/spec/models/ticket/article_spec.rb b/spec/models/ticket/article_spec.rb index c60f71afd..9dce6b42c 100644 --- a/spec/models/ticket/article_spec.rb +++ b/spec/models/ticket/article_spec.rb @@ -8,7 +8,9 @@ RSpec.describe Ticket::Article, type: :model do it_behaves_like 'CanBeImported' it_behaves_like 'HasObjectManagerAttributesValidation' - describe 'Callbacks, Observers, & Async Transactions' do + subject(:article) { create(:ticket_article) } + + describe 'Callbacks, Observers, & Async Transactions -' do describe 'NULL byte handling (via ChecksAttributeValuesAndLength concern):' do it 'removes them from #subject on creation, if necessary (postgres doesn’t like them)' do expect(create(:ticket_article, subject: "com test 1\u0000")) @@ -21,6 +23,40 @@ RSpec.describe Ticket::Article, type: :model do end end + describe 'Setting of ticket.create_article_{sender,type}' do + let!(:ticket) { create(:ticket) } + + context 'on creation' do + context 'of first article on a ticket' do + subject(:article) do + create(:ticket_article, ticket: ticket, sender_name: 'Agent', type_name: 'email') + end + + it 'sets ticket sender/type attributes based on article sender/type' do + expect { article } + .to change { ticket.reload.create_article_sender&.name }.to('Agent') + .and change { ticket.reload.create_article_type&.name }.to('email') + end + end + + context 'of subsequent articles on a ticket' do + let!(:first_article) do + create(:ticket_article, ticket: ticket, sender_name: 'Agent', type_name: 'email') + end + + subject(:article) do + create(:ticket_article, ticket: ticket, sender_name: 'Customer', type_name: 'twitter status') + end + + it 'does not modify ticket’s sender/type attributes' do + expect { article } + .to not_change { ticket.reload.create_article_sender.name } + .and not_change { ticket.reload.create_article_type.name } + end + end + end + end + describe 'Cti::Log syncing:' do context 'with existing Log records' do context 'for an incoming call from an unknown number' do @@ -44,7 +80,7 @@ RSpec.describe Ticket::Article, type: :model do end end - describe 'Auto-setting of outgoing Twitter article attributes (via bj jobs):' do + describe 'Auto-setting of outgoing Twitter article attributes (via bg jobs):' do subject!(:twitter_article) { create(:twitter_article, sender_name: 'Agent') } let(:channel) { Channel.find(twitter_article.ticket.preferences[:channel_id]) } diff --git a/test/unit/email_process_customer_selection_based_on_sender_recipient_test.rb b/test/unit/email_process_customer_selection_based_on_sender_recipient_test.rb deleted file mode 100644 index 68193fabf..000000000 --- a/test/unit/email_process_customer_selection_based_on_sender_recipient_test.rb +++ /dev/null @@ -1,78 +0,0 @@ -require 'test_helper' - -class EmailProcessCustomerSelectionBasedOnSenderRecipient < ActiveSupport::TestCase - - setup do - groups = Group.all - roles = Role.where(name: 'Agent') - @agent1 = User.create_or_update( - login: 'user-customer-selection-agent1@example.com', - firstname: 'UserOutOfOffice', - lastname: 'Agent1', - email: 'user-customer-selection-agent1@example.com', - password: 'agentpw', - active: true, - roles: roles, - groups: groups, - updated_by_id: 1, - created_by_id: 1, - ) - roles = Role.where(name: 'Customer') - @customer1 = User.create_or_update( - login: 'user-customer-selection-customer1@example.com', - firstname: 'UserOutOfOffice', - lastname: 'customer1', - email: 'user-customer-selection-customer1@example.com', - password: 'customerpw', - active: true, - roles: roles, - updated_by_id: 1, - created_by_id: 1, - ) - end - - test 'customer need to be customer' do - - email_raw_string = "From: #{@agent1.email} -To: #{@customer1.email} -Subject: test - -Some Text" - - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) - ticket = Ticket.find(ticket_p.id) - article = Ticket::Article.find(article_p.id) - assert_equal('test', ticket.title) - assert_equal('new', ticket.state.name) - assert_equal('Agent', ticket.create_article_sender.name) - assert_equal('Agent', article.sender.name) - assert_equal(@customer1.email, ticket.customer.email) - assert_equal(@customer1.firstname, ticket.customer.firstname) - assert_equal(@customer1.lastname, ticket.customer.lastname) - - end - - test 'agent need to be customer' do - - Setting.set('postmaster_sender_is_agent_search_for_customer', false) - - email_raw_string = "From: #{@agent1.email} -To: #{@customer1.email} -Subject: test - -Some Text" - - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) - ticket = Ticket.find(ticket_p.id) - article = Ticket::Article.find(article_p.id) - assert_equal('test', ticket.title) - assert_equal('new', ticket.state.name) - assert_equal('Agent', ticket.create_article_sender.name) - assert_equal('Agent', article.sender.name) - assert_equal(@agent1.email, ticket.customer.email) - assert_equal(@agent1.firstname, ticket.customer.firstname) - assert_equal(@agent1.lastname, ticket.customer.lastname) - - end - -end