Browse Source

Improved usage (autoset) of origin_by of article model.

Martin Edenhofer 7 years ago
parent
commit
a987c10051

+ 21 - 4
app/models/observer/ticket/article/fillup_from_general.rb

@@ -13,7 +13,7 @@ class Observer::Ticket::Article::FillupFromGeneral < ActiveRecord::Observer
     return if ApplicationHandleInfo.postmaster?
 
     # set from on all article types excluding email|twitter status|twitter direct-message|facebook feed post|facebook feed comment
-    return true if !record.type_id
+    return true if record.type_id.blank?
     type = Ticket::Article::Type.lookup(id: record.type_id)
     return true if type['name'] == 'email'
 
@@ -23,9 +23,26 @@ class Observer::Ticket::Article::FillupFromGeneral < ActiveRecord::Observer
     return true if type['name'] == 'facebook feed post'
     return true if type['name'] == 'facebook feed comment'
 
-    return true if !record.created_by_id
-    user = User.find(record.created_by_id)
-    if type.name == 'web'
+    user_id = record.created_by_id
+
+    if record.origin_by_id.present?
+
+      # in case the customer is using origin_by_id, force it to current session user
+      # and set sender to Customer
+      if !record.created_by.permissions?('ticket.agent')
+        record.origin_by_id = record.created_by_id
+        record.sender_id = Ticket::Article::Sender.lookup(name: 'Customer').id
+      end
+
+      # in case origin_by_id is customer, force it to set sender to Customer
+      if record.origin_by != record.created_by_id && !record.origin_by.permissions?('ticket.agent')
+        record.sender_id = Ticket::Article::Sender.lookup(name: 'Customer').id
+        user_id = record.origin_by_id
+      end
+    end
+    return true if user_id.blank?
+    user = User.find(user_id)
+    if type.name == 'web' || type.name == 'phone'
       record.from = "#{user.firstname} #{user.lastname} <#{user.email}>"
       return
     end

+ 4 - 5
app/models/observer/ticket/article/fillup_from_origin_by_id.rb

@@ -8,18 +8,17 @@ class Observer::Ticket::Article::FillupFromOriginById < ActiveRecord::Observer
     # return if we run import mode
     return if Setting.get('import_mode')
 
-    # only do fill of from if article got created via application_server (e. g. not
+    # only do fill origin_by_id if article got created via application_server (e. g. not
     # if article and sender type is set via *.postmaster)
     return if ApplicationHandleInfo.postmaster?
 
     # check if origin_by_id exists
     return if record.origin_by_id.present?
-    return if !record.ticket.customer_id
+    return if record.ticket.customer_id.blank?
     return if record.sender.name != 'Customer'
-    return if record.type.name != 'phone'
+    type_name = record.type.name
+    return if type_name != 'phone' && type_name != 'note' && type_name != 'web'
 
     record.origin_by_id = record.ticket.customer_id
-    user                = User.find(record.origin_by_id)
-    record.from         = "#{user.firstname} #{user.lastname} <#{user.email}>"
   end
 end

+ 1 - 0
app/models/ticket/article.rb

@@ -15,6 +15,7 @@ class Ticket::Article < ApplicationModel
   belongs_to    :sender,      class_name: 'Ticket::Article::Sender'
   belongs_to    :created_by,  class_name: 'User'
   belongs_to    :updated_by,  class_name: 'User'
+  belongs_to    :origin_by,   class_name: 'User'
   store         :preferences
   before_create :check_subject, :check_body, :check_message_id_md5
   before_update :check_subject, :check_body, :check_message_id_md5

+ 1 - 1
config/application.rb

@@ -24,9 +24,9 @@ module Zammad
       'observer::_ticket::_last_owner_update',
       'observer::_ticket::_user_ticket_counter',
       'observer::_ticket::_article_changes',
+      'observer::_ticket::_article::_fillup_from_origin_by_id',
       'observer::_ticket::_article::_fillup_from_general',
       'observer::_ticket::_article::_fillup_from_email',
-      'observer::_ticket::_article::_fillup_from_origin_by_id',
       'observer::_ticket::_article::_communicate_email',
       'observer::_ticket::_article::_communicate_facebook',
       'observer::_ticket::_article::_communicate_twitter',

+ 3 - 3
test/controllers/ticket_articles_controller_test.rb

@@ -298,7 +298,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
 
-    article = Ticket::Article.find_by( ticket_id: result['id'] )
+    article = Ticket::Article.find_by(ticket_id: result['id'])
     assert_equal(@customer_without_org.id, article.origin_by_id)
     assert_equal('Tickets Customer1 <tickets-customer1@example.com>', article.from)
   end
@@ -322,8 +322,8 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
 
-    article = Ticket::Article.find_by( ticket_id: result['id'] )
-    assert_nil(article.origin_by_id)
+    article = Ticket::Article.find_by(ticket_id: result['id'])
+    assert_equal(@customer_without_org.id, article.origin_by_id)
   end
 
   test '04.01 ticket split with html - check attachments' do

+ 231 - 17
test/controllers/tickets_controller_test.rb

@@ -246,7 +246,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
   test '01.09 ticket create with agent - minimal article with guess customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #8',
+      title: 'a new ticket #9',
       group: 'Users',
       priority: '2 normal',
       state: 'new',
@@ -260,7 +260,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
     assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
-    assert_equal('a new ticket #8', result['title'])
+    assert_equal('a new ticket #9', result['title'])
     assert_equal(User.lookup(email: 'some_new_customer@example.com').id, result['customer_id'])
     assert_equal(@agent.id, result['updated_by_id'])
     assert_equal(@agent.id, result['created_by_id'])
@@ -269,7 +269,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
   test '01.10 ticket create with agent - minimal article with guess customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #9',
+      title: 'a new ticket #10',
       group: 'Users',
       customer_id: 'guess:some_new_customer@example.com',
       article: {
@@ -281,7 +281,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
     assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
-    assert_equal('a new ticket #9', result['title'])
+    assert_equal('a new ticket #10', result['title'])
     assert_equal(User.lookup(email: 'some_new_customer@example.com').id, result['customer_id'])
     assert_equal(@agent.id, result['updated_by_id'])
     assert_equal(@agent.id, result['created_by_id'])
@@ -290,7 +290,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
   test '01.11 ticket create with agent - minimal article with customer hash' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #9',
+      title: 'a new ticket #11',
       group: 'Users',
       customer: {
         firstname: 'some firstname',
@@ -306,16 +306,159 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
     assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
-    assert_equal('a new ticket #9', result['title'])
+    assert_equal('a new ticket #11', result['title'])
+    assert_equal(User.lookup(email: 'some_new_customer@example.com').id, result['customer_id'])
+    assert_equal(@agent.id, result['updated_by_id'])
+    assert_equal(@agent.id, result['created_by_id'])
+  end
+
+  test '01.11.1 ticket create with agent - minimal article with customer hash with article.origin_by' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    params = {
+      title: 'a new ticket #11.1',
+      group: 'Users',
+      customer: {
+        firstname: 'some firstname',
+        lastname: 'some lastname',
+        email: 'some_new_customer@example.com',
+      },
+      article: {
+        body: 'some test 123',
+        origin_by: 'some_new_customer@example.com',
+      },
+    }
+    post '/api/v1/tickets', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
+    assert_equal('a new ticket #11.1', result['title'])
+    assert_equal(User.lookup(email: 'some_new_customer@example.com').id, result['customer_id'])
+    assert_equal(@agent.id, result['updated_by_id'])
+    assert_equal(@agent.id, result['created_by_id'])
+    ticket = Ticket.find(result['id'])
+    article = ticket.articles.first
+    assert_equal(@agent.id, article.updated_by_id)
+    assert_equal(@agent.id, article.created_by_id)
+    assert_equal(User.lookup(email: 'some_new_customer@example.com').id, article.origin_by_id)
+    assert_equal('Customer', article.sender.name)
+    assert_equal('note', article.type.name)
+    assert_equal('some firstname some lastname', article.from)
+  end
+
+  test '01.11.2 ticket create with agent - minimal article with customer hash with article.origin_by' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    params = {
+      title: 'a new ticket #11.2',
+      group: 'Users',
+      customer: {
+        firstname: 'some firstname',
+        lastname: 'some lastname',
+        email: 'some_new_customer@example.com',
+      },
+      article: {
+        sender: 'Customer',
+        body: 'some test 123',
+        origin_by: 'some_new_customer@example.com',
+      },
+    }
+
+    post '/api/v1/tickets', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
+    assert_equal('a new ticket #11.2', result['title'])
+    assert_equal(User.lookup(email: 'some_new_customer@example.com').id, result['customer_id'])
+    assert_equal(@agent.id, result['updated_by_id'])
+    assert_equal(@agent.id, result['created_by_id'])
+    ticket = Ticket.find(result['id'])
+    article = ticket.articles.first
+    assert_equal(@agent.id, article.updated_by_id)
+    assert_equal(@agent.id, article.created_by_id)
+    assert_equal(User.lookup(email: 'some_new_customer@example.com').id, article.origin_by_id)
+    assert_equal('Customer', article.sender.name)
+    assert_equal('note', article.type.name)
+    assert_equal('some firstname some lastname', article.from)
+  end
+
+  test '01.11.3 ticket create with agent - minimal article with customer hash with article.origin_by' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    params = {
+      title: 'a new ticket #11.3',
+      group: 'Users',
+      customer: {
+        firstname: 'some firstname',
+        lastname: 'some lastname',
+        email: 'some_new_customer@example.com',
+      },
+      article: {
+        sender: 'Agent',
+        from: 'somebody',
+        body: 'some test 123',
+        origin_by: 'some_new_customer@example.com',
+      },
+    }
+
+    post '/api/v1/tickets', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
+    assert_equal('a new ticket #11.3', result['title'])
     assert_equal(User.lookup(email: 'some_new_customer@example.com').id, result['customer_id'])
     assert_equal(@agent.id, result['updated_by_id'])
     assert_equal(@agent.id, result['created_by_id'])
+    ticket = Ticket.find(result['id'])
+    article = ticket.articles.first
+    assert_equal(@agent.id, article.updated_by_id)
+    assert_equal(@agent.id, article.created_by_id)
+    assert_equal(User.lookup(email: 'some_new_customer@example.com').id, article.origin_by_id)
+    assert_equal('Customer', article.sender.name)
+    assert_equal('note', article.type.name)
+    assert_equal('some firstname some lastname', article.from)
+  end
+
+  test '01.11.4 ticket create with agent - minimal article with customer hash with article.origin_by' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
+    params = {
+      title: 'a new ticket #11.4',
+      group: 'Users',
+      customer: {
+        firstname: 'some firstname',
+        lastname: 'some lastname',
+        email: 'some_new_customer@example.com',
+      },
+      article: {
+        sender: 'Customer',
+        body: 'some test 123',
+        origin_by: @customer_without_org.login,
+      },
+    }
+
+    post '/api/v1/tickets', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
+    assert_equal('a new ticket #11.4', result['title'])
+    assert_equal(User.lookup(email: 'some_new_customer@example.com').id, result['customer_id'])
+    assert_equal(@agent.id, result['updated_by_id'])
+    assert_equal(@agent.id, result['created_by_id'])
+    ticket = Ticket.find(result['id'])
+    article = ticket.articles.first
+    assert_equal(@agent.id, article.updated_by_id)
+    assert_equal(@agent.id, article.created_by_id)
+    assert_equal(@customer_without_org.id, article.origin_by_id)
+    assert_equal('Customer', article.sender.name)
+    assert_equal('note', article.type.name)
+    assert_equal('Tickets Customer1', article.from)
   end
 
   test '01.12 ticket create with agent - minimal article with missing body - with customer.id' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #10',
+      title: 'a new ticket #12',
       group: 'Users',
       customer_id: @customer_without_org.id,
       article: {
@@ -332,7 +475,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
   test '01.13 ticket create with agent - minimal article and attachment with customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #11',
+      title: 'a new ticket #13',
       group: 'Users',
       customer_id: @customer_without_org.id,
       article: {
@@ -350,7 +493,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
     assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
-    assert_equal('a new ticket #11', result['title'])
+    assert_equal('a new ticket #13', result['title'])
     assert_equal(@customer_without_org.id, result['customer_id'])
     assert_equal(@agent.id, result['updated_by_id'])
     assert_equal(@agent.id, result['created_by_id'])
@@ -368,7 +511,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
   test '01.14 ticket create with agent - minimal article and attachment with customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #12',
+      title: 'a new ticket #14',
       group: 'Users',
       customer_id: @customer_without_org.id,
       article: {
@@ -393,7 +536,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
     assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
-    assert_equal('a new ticket #12', result['title'])
+    assert_equal('a new ticket #14', result['title'])
     assert_equal(@customer_without_org.id, result['customer_id'])
     assert_equal(@agent.id, result['updated_by_id'])
     assert_equal(@agent.id, result['created_by_id'])
@@ -411,7 +554,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
   test '01.15 ticket create with agent - minimal article and attachment missing mine-type with customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #13',
+      title: 'a new ticket #15',
       group: 'Users',
       customer_id: @customer_without_org.id,
       article: {
@@ -434,7 +577,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
   test '01.16 ticket create with agent - minimal article and attachment invalid base64 with customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #14',
+      title: 'a new ticket #16',
       group: 'Users',
       customer_id: @customer_without_org.id,
       article: {
@@ -456,7 +599,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest
   test '01.17 ticket create with agent - minimal article and inline attachments with customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #15',
+      title: 'a new ticket #17',
       group: 'Users',
       customer_id: @customer_without_org.id,
       article: {
@@ -473,7 +616,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
     assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
-    assert_equal('a new ticket #15', result['title'])
+    assert_equal('a new ticket #17', result['title'])
     assert_equal(@customer_without_org.id, result['customer_id'])
     assert_equal(@agent.id, result['updated_by_id'])
     assert_equal(@agent.id, result['created_by_id'])
@@ -496,7 +639,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
   test '01.18 ticket create with agent - minimal article and inline attachments with customer' do
     credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-agent@example.com', 'agentpw')
     params = {
-      title: 'a new ticket #16',
+      title: 'a new ticket #18',
       group: 'Users',
       customer_id: @customer_without_org.id,
       article: {
@@ -517,7 +660,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
     assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
-    assert_equal('a new ticket #16', result['title'])
+    assert_equal('a new ticket #18', result['title'])
     assert_equal(@customer_without_org.id, result['customer_id'])
     assert_equal(@agent.id, result['updated_by_id'])
     assert_equal(@agent.id, result['created_by_id'])
@@ -1185,6 +1328,77 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
     assert_equal('Not authorized (admin permission required)!', result['error'])
   end
 
+  test '03.6 ticket create with agent - minimal article with customer hash with article.origin_by' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-customer1@example.com', 'customer1pw')
+    params = {
+      title: 'a new ticket #3.6',
+      group: 'Users',
+      customer: {
+        firstname: 'some firstname',
+        lastname: 'some lastname',
+        email: 'some_new_customer@example.com',
+      },
+      article: {
+        body: 'some test 123',
+        origin_by: @agent.login,
+      },
+    }
+
+    post '/api/v1/tickets', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
+    assert_equal('a new ticket #3.6', result['title'])
+    assert_equal(@customer_without_org.id, result['customer_id'])
+    assert_equal(@customer_without_org.id, result['updated_by_id'])
+    assert_equal(@customer_without_org.id, result['created_by_id'])
+    ticket = Ticket.find(result['id'])
+    article = ticket.articles.first
+    assert_equal(@customer_without_org.id, article.updated_by_id)
+    assert_equal(@customer_without_org.id, article.created_by_id)
+    assert_equal(@customer_without_org.id, article.origin_by_id)
+    assert_equal('Customer', article.sender.name)
+    assert_equal('note', article.type.name)
+    assert_equal('Tickets Customer1', article.from)
+  end
+
+  test '03.6.1 ticket create with agent - minimal article with customer hash with article.origin_by' do
+    credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-customer1@example.com', 'customer1pw')
+    params = {
+      title: 'a new ticket #3.6.1',
+      group: 'Users',
+      customer: {
+        firstname: 'some firstname',
+        lastname: 'some lastname',
+        email: 'some_new_customer@example.com',
+      },
+      article: {
+        sender: 'Agent',
+        body: 'some test 123',
+        origin_by_id: @agent.id,
+      },
+    }
+
+    post '/api/v1/tickets', params: params.to_json, headers: @headers.merge('Authorization' => credentials)
+    assert_response(201)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_equal(Ticket::State.lookup(name: 'new').id, result['state_id'])
+    assert_equal('a new ticket #3.6.1', result['title'])
+    assert_equal(@customer_without_org.id, result['customer_id'])
+    assert_equal(@customer_without_org.id, result['updated_by_id'])
+    assert_equal(@customer_without_org.id, result['created_by_id'])
+    ticket = Ticket.find(result['id'])
+    article = ticket.articles.first
+    assert_equal(@customer_without_org.id, article.updated_by_id)
+    assert_equal(@customer_without_org.id, article.created_by_id)
+    assert_equal(@customer_without_org.id, article.origin_by_id)
+    assert_equal('Customer', article.sender.name)
+    assert_equal('note', article.type.name)
+    assert_equal('Tickets Customer1', article.from)
+  end
+
   test '04.01 ticket show and response format' do
     title = "ticket testagent#{rand(999_999_999)}"
     ticket = Ticket.create!(