Fixes #5017 - No "From"-Field in Articles created on behalf of a cust…

…omer with auto-generated login

app/models/ticket/article/adds_metadata_general.rb

@@ -63,11 +63,12 @@ def metadata_general_process_origin_by
   def metadata_general_process_from
     type        = Ticket::Article::Type.lookup(id: type_id)
     is_customer = !author.permissions?('ticket.agent')
+    fullname    = author.fullname(email_fallback: false).presence
 
-    self.from = if %w[web phone].include?(type.name) && is_customer
-                  Channel::EmailBuild.recipient_line "#{author.firstname} #{author.lastname}", author.email
+    self.from = if %w[web phone].include?(type.name) && is_customer && author.email.present?
+                  Channel::EmailBuild.recipient_line fullname, author.email
                 else
-                  "#{author.firstname} #{author.lastname}"
+                  fullname
                 end
   end
 end

app/models/user.rb

@@ -137,20 +137,13 @@ def ignore_search_indexing?(_action)
 
 =end
 
-  def fullname
-    name = ''
-    if firstname.present?
-      name = firstname
-    end
-    if lastname.present?
-      if name != ''
-        name += ' '
-      end
-      name += lastname
-    end
-    if name.blank? && email.present?
-      name = email
+  def fullname(email_fallback: true)
+    name = "#{firstname} #{lastname}".strip
+
+    if name.blank? && email.present? && email_fallback
+      return email
     end
+
     name
   end
 

spec/models/ticket/article/adds_metadata_general_spec.rb

@@ -3,8 +3,6 @@
 require 'rails_helper'
 
 RSpec.describe Ticket::Article::AddsMetadataGeneral do
-  let(:agent) { create(:agent) }
-
   context 'when Agent creates Article' do
     shared_examples 'not including email in from' do |factory|
       subject(:article) { create(:ticket_article, factory, ticket: ticket, created_by_id: agent.id, updated_by_id: agent.id) }
@@ -23,8 +21,8 @@
     context 'when as Customer' do
       subject(:article) { create(:ticket_article, :inbound_phone, ticket: ticket) }
 
-      let(:customer) { agent }
-      let(:ticket) { create(:ticket, customer_id: customer.id) }
+      let(:agent)  { create(:agent) }
+      let(:ticket) { create(:ticket, customer_id: agent.id) }
 
       it 'includes email in from' do
         expect(article.from).not_to include agent.email
@@ -51,4 +49,25 @@
       expect(article.origin_by).to be_nil
     end
   end
+
+  context 'when Customer creates Article', current_user_id: -> { customer.id } do
+    let(:ticket)  { create(:ticket, customer:) }
+    let(:article) { create(:ticket_article, :inbound_web, ticket:) }
+
+    context 'when customer has email address' do
+      let(:customer) { create(:customer) }
+
+      it '#from is set correctly to customer full name and email' do
+        expect(article.from).to eq("#{customer.fullname} <#{customer.email}>")
+      end
+    end
+
+    context 'when customer has no email address' do
+      let(:customer) { create(:customer, email: nil) }
+
+      it '#from is set correctly to customer full name' do
+        expect(article.from).to eq(customer.fullname)
+      end
+    end
+  end
 end

spec/requests/ticket_spec.rb

@@ -2578,7 +2578,7 @@ def delete_article_body
 
       expect(Ticket.last.articles.first).to have_attributes(
         origin_by_id: User.find_by(email: 'dummy@example.com').id,
-        from:         '  <dummy@example.com>',
+        from:         'dummy@example.com',
       )
     end
   end

test/unit/ticket_trigger_test.rb

@@ -4443,7 +4443,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
     assert_equal(2, ticket1.articles.count, 'ticket1.articles verify')
     assert_equal(%w[tag1 tag2], ticket1.tag_list)
 
-    assert_match('- ', article1.from)
+    assert_match('-', article1.from)
     assert_match('some_recipient@example.com', article1.to)
     assert_match('some subject', article1.subject)
     assert_match("some message <b>note</b>\nnew line", article1.body)
@@ -4514,14 +4514,14 @@ class TicketTriggerTest < ActiveSupport::TestCase
     assert_equal(3, ticket1.articles.count, 'ticket1.articles verify')
     assert_equal([], ticket1.tag_list)
 
-    assert_match('- ', article1.from)
+    assert_match('-', article1.from)
     assert_match('some_recipient@example.com', article1.to)
     assert_match('some subject', article1.subject)
     assert_match("some message <b>note</b>\nnew line", article1.body)
     assert_equal('text/plain', article1.content_type)
 
     article_note1 = ticket1.articles[1]
-    assert_match('- ', article_note1.from)
+    assert_match('-', article_note1.from)
     assert_nil(article_note1.to)
     assert_match("some subject! #{ticket1.id}", article_note1.subject)
     assert_match("I can integrate with 3rd party services at <a href=\"https://my.saas/foo/#{ticket1.id}\" rel=\"nofollow noreferrer noopener\" target=\"_blank\">https://my.saas/foo/#{ticket1.id}</a>", article_note1.body)