Fixes #3902 - Ticket updates without articles do provide article variables set

app/assets/javascripts/app/controllers/widget/placeholder.coffee

@@ -69,6 +69,30 @@ class App.WidgetPlaceholder extends App.Controller
               content: content
             }
 
+    # modify article placeholders
+    replaces = [
+      { display: __('Last Article'), name: 'last_article' },
+      { display: __('Last Internal Article'), name: 'last_internal_article' },
+      { display: __('Last External Article'), name: 'last_external_article' },
+      { display: __('Created Article'), name: 'created_article' },
+      { display: __('Created Internal Article'), name: 'created_internal_article' },
+      { display: __('Created External Article'), name: 'created_external_article' },
+    ]
+
+    for item in all
+      if item.name.startsWith('Article')
+        for replace in replaces
+          all.push {
+            name: item.name.replace('Article', App.i18n.translateInline(replace.display))
+            content: item.content.replace('article', replace.name)
+            id: item.id.replace('article', replace.name)
+            keywords: item.keywords.replace('article', replace.name)
+          }
+
+    all = _.filter(all, (item) ->
+      return !item.name.startsWith('Article')
+    )
+
     # add config
     for setting in App.Setting.all()
       if setting.frontend && setting.preferences && setting.preferences.placeholder

app/models/ticket.rb

@@ -1034,9 +1034,30 @@ result
   # if another email notification trigger preceded this one
   # (see https://github.com/zammad/zammad/issues/1543)
   def build_notification_template_objects(article)
+    last_article = nil
+    last_internal_article = nil
+    last_external_article = nil
+    all_articles = articles
+
+    if article.nil?
+      last_article = all_articles.last
+      last_internal_article = all_articles.reverse.find(&:internal?)
+      last_external_article = all_articles.reverse.find { |a| !a.internal? }
+    else
+      last_article = article
+      last_internal_article = article.internal? ? article : all_articles.reverse.find(&:internal?)
+      last_external_article = article.internal? ? all_articles.reverse.find { |a| !a.internal? } : article
+    end
+
     {
-      ticket:  self,
-      article: article || articles.last
+      ticket:                   self,
+      article:                  last_article,
+      last_article:             last_article,
+      last_internal_article:    last_internal_article,
+      last_external_article:    last_external_article,
+      created_article:          article,
+      created_internal_article: article&.internal? ? article : nil,
+      created_external_article: article&.internal? ? nil : article,
     }
   end
 

i18n/zammad.pot

@@ -2610,12 +2610,24 @@ msgstr ""
 msgid "Created"
 msgstr ""
 
+#: app/assets/javascripts/app/controllers/widget/placeholder.coffee
+msgid "Created Article"
+msgstr ""
+
 #: app/assets/javascripts/app/views/time_accounting/by_activity.jst.eco
 #: app/assets/javascripts/app/views/time_accounting/by_ticket.jst.eco
 #: lib/excel_sheet/ticket.rb
 msgid "Created At"
 msgstr ""
 
+#: app/assets/javascripts/app/controllers/widget/placeholder.coffee
+msgid "Created External Article"
+msgstr ""
+
+#: app/assets/javascripts/app/controllers/widget/placeholder.coffee
+msgid "Created Internal Article"
+msgstr ""
+
 #: app/assets/javascripts/app/models/knowledge_base_answer_translation.coffee
 #: app/assets/javascripts/app/models/organization.coffee
 #: app/assets/javascripts/app/models/ticket.coffee
@@ -5964,6 +5976,10 @@ msgstr ""
 msgid "Languages"
 msgstr ""
 
+#: app/assets/javascripts/app/controllers/widget/placeholder.coffee
+msgid "Last Article"
+msgstr ""
+
 #: lib/excel_sheet/ticket.rb
 msgid "Last Contact Agent At"
 msgstr ""
@@ -5976,6 +5992,14 @@ msgstr ""
 msgid "Last Contact Customer At"
 msgstr ""
 
+#: app/assets/javascripts/app/controllers/widget/placeholder.coffee
+msgid "Last External Article"
+msgstr ""
+
+#: app/assets/javascripts/app/controllers/widget/placeholder.coffee
+msgid "Last Internal Article"
+msgstr ""
+
 #: app/assets/javascripts/app/controllers/ticket_zoom/attribute_bar.coffee
 msgid "Last change %s %s by %s"
 msgstr ""

lib/notification_factory/renderer.rb

@@ -51,21 +51,26 @@ examples how to use
     # do validation, ignore some methods
     return "\#{#{key} / not allowed}" if !data_key_valid?(key)
 
+    article_tags = %w[article last_article last_internal_article last_external_article
+                      created_article created_internal_article created_external_article]
+
     # aliases
-    map = {
-      'article.body' => 'article.body_as_text_with_quote.text2html',
-      'ticket.tags'  => 'ticket.tag_list',
-    }
+    map = { 'ticket.tags' => 'ticket.tag_list' }
+    article_tags.each do |tag|
+      map["#{tag}.body"] = "#{tag}.body_as_text_with_quote.text2html"
+    end
+
     if map[key]
       key = map[key]
     end
 
     # escape in html mode
     if escape
-      no_escape = {
-        'article.body_as_html'                      => true,
-        'article.body_as_text_with_quote.text2html' => true,
-      }
+      no_escape = {}
+      article_tags.each do |tag|
+        no_escape["#{tag}.body_as_html"] = true
+        no_escape["#{tag}.body_as_text_with_quote.text2html"] = true
+      end
       if no_escape[key]
         escape = false
       end

spec/lib/notification_factory/renderer/article_tags_spec.rb

@@ -0,0 +1,225 @@
+# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'NotificationFactory::Renderer article tags' do # rubocop:disable RSpec/DescribeClass
+  let(:ticket) do
+    ticket = create(:ticket)
+    create(
+      :ticket_article,
+      type:         Ticket::Article::Type.lookup(name: 'note'),
+      sender:       Ticket::Article::Sender.lookup(name: 'System'),
+      body:         'Welcome to Zammad!',
+      content_type: 'text/plain',
+      ticket_id:    ticket.id,
+    )
+    create(
+      :ticket_article,
+      type:         Ticket::Article::Type.lookup(name: 'note'),
+      sender:       Ticket::Article::Sender.lookup(name: 'Customer'),
+      body:         'Thank you!',
+      content_type: 'text/plain',
+      ticket_id:    ticket.id,
+    )
+    create(
+      :ticket_article,
+      type:         Ticket::Article::Type.lookup(name: 'note'),
+      sender:       Ticket::Article::Sender.lookup(name: 'System'),
+      body:         'Received reply.',
+      content_type: 'text/plain',
+      internal:     true,
+      ticket_id:    ticket.id,
+    )
+
+    ticket
+  end
+
+  let(:article) { nil }
+
+  let(:objects) do
+    last_article = nil
+    last_internal_article = nil
+    last_external_article = nil
+    all_articles = ticket.articles
+
+    if article.nil?
+      last_article = all_articles.last
+      last_internal_article = all_articles.reverse.find(&:internal?)
+      last_external_article = all_articles.reverse.find { |a| !a.internal? }
+    else
+      last_article = article
+      last_internal_article = article.internal? ? article : all_articles.reverse.find(&:internal?)
+      last_external_article = article.internal? ? all_articles.reverse.find { |a| !a.internal? } : article
+    end
+
+    {
+      ticket:                   ticket,
+      article:                  last_article,
+      last_article:             last_article,
+      last_internal_article:    last_internal_article,
+      last_external_article:    last_external_article,
+      created_article:          article,
+      created_internal_article: article&.internal? ? article : nil,
+      created_external_article: article&.internal? ? nil : article,
+    }
+  end
+  let(:renderer) do
+    build(:notification_factory_renderer,
+          objects:  objects,
+          template: template)
+  end
+
+  describe 'last_article' do
+    let(:template) { "\#{last_article.body}" }
+
+    it 'has body content from the last article' do
+      expect(renderer.render).to eq "&gt; #{ticket.articles.last.body}<br>"
+    end
+  end
+
+  describe 'last_internal_article' do
+    let(:template) { "\#{last_internal_article.body}" }
+
+    it 'has body content from the last article' do
+      expect(renderer.render).to eq "&gt; #{ticket.articles.last.body}<br>"
+    end
+  end
+
+  describe 'last_external_article' do
+    let(:template) { "\#{last_external_article.body}" }
+
+    it 'has body content from the third article' do
+      expect(renderer.render).to eq "&gt; #{ticket.articles.second.body}<br>"
+    end
+  end
+
+  describe 'created_article' do
+    let(:template) { "\#{created_article.body}" }
+
+    it 'no such object' do
+      expect(renderer.render).to eq '#{created_article / no such object}' # rubocop:disable Lint/InterpolationCheck
+    end
+  end
+
+  describe 'created_internal_article' do
+    let(:template) { "\#{created_internal_article.body}" }
+
+    it 'no such object' do
+      expect(renderer.render).to eq '#{created_internal_article / no such object}' # rubocop:disable Lint/InterpolationCheck
+    end
+  end
+
+  describe 'created_external_article' do
+    let(:template) { "\#{created_external_article.body}" }
+
+    it 'no such object' do
+      expect(renderer.render).to eq '#{created_external_article / no such object}' # rubocop:disable Lint/InterpolationCheck
+    end
+  end
+
+  context 'when creating a new internal article' do
+    let(:article) do
+      create(
+        :ticket_article,
+        type:         Ticket::Article::Type.lookup(name: 'note'),
+        sender:       Ticket::Article::Sender.lookup(name: 'Agent'),
+        body:         'Nice dude!',
+        content_type: 'text/plain',
+        internal:     true,
+        ticket_id:    ticket.id,
+      )
+    end
+
+    describe 'created_article' do
+      let(:template) { "\#{created_article.body}" }
+
+      it 'has body content from the new article' do
+        expect(renderer.render).to eq "&gt; #{ticket.articles.last.body}<br>"
+      end
+    end
+
+    describe 'created_internal_article' do
+      let(:template) { "\#{created_internal_article.body}" }
+
+      it 'has body content from the new article' do
+        expect(renderer.render).to eq "&gt; #{ticket.articles.last.body}<br>"
+      end
+    end
+
+    describe 'created_external_article' do
+      let(:template) { "\#{created_external_article.body}" }
+
+      it 'no such object' do
+        expect(renderer.render).to eq '#{created_external_article / no such object}' # rubocop:disable Lint/InterpolationCheck
+      end
+    end
+
+    describe 'last_article' do
+      let(:template) { "\#{last_article.body}" }
+
+      it 'has body content from the last article' do
+        expect(renderer.render).to eq "&gt; #{ticket.articles.last.body}<br>"
+      end
+    end
+
+    context 'when setting new article external' do
+      before do
+        article.update!(internal: false)
+      end
+
+      describe 'created_article' do
+        let(:template) { "\#{created_article.body}" }
+
+        it 'has body content from the new article' do
+          expect(renderer.render).to eq "&gt; #{ticket.articles.last.body}<br>"
+        end
+      end
+
+      describe 'created_internal_article' do
+        let(:template) { "\#{created_internal_article.body}" }
+
+        it 'no such object' do
+          expect(renderer.render).to eq '#{created_internal_article / no such object}' # rubocop:disable Lint/InterpolationCheck
+        end
+      end
+
+      describe 'created_external_article' do
+        let(:template) { "\#{created_external_article.body}" }
+
+        it 'has body content from the new article' do
+          expect(renderer.render).to eq "&gt; #{ticket.articles.last.body}<br>"
+        end
+      end
+
+      describe 'last_article' do
+        let(:template) { "\#{last_article.body}" }
+
+        it 'has body content from the last article' do
+          expect(renderer.render).to eq "&gt; #{ticket.articles.last.body}<br>"
+        end
+      end
+    end
+  end
+
+  context 'when updating ticket attribute' do
+    before do
+      ticket.update!(title: 'New title')
+    end
+
+    describe 'last_article' do
+      let(:template) { "\#{last_article.body}" }
+
+      it 'has body content from the last article' do
+        expect(renderer.render).to eq "&gt; #{ticket.articles.last.body}<br>"
+      end
+    end
+
+    describe 'created_article' do
+      let(:template) { "\#{created_article.body}" }
+
+      it 'no such object' do
+        expect(renderer.render).to eq '#{created_article / no such object}' # rubocop:disable Lint/InterpolationCheck
+      end
+    end
+  end
+end

spec/lib/notification_factory/renderer_spec.rb

@@ -59,6 +59,77 @@ RSpec.describe NotificationFactory::Renderer do
       expect { renderer.render }.to raise_error(StandardError)
     end
 
+    context 'with different article variables' do
+
+      let(:customer) { create(:customer, firstname: 'Nicole') }
+      let(:ticket)   { create(:ticket, customer: customer) }
+      let(:objects)  do
+        last_article = nil
+        last_internal_article = nil
+        last_external_article = nil
+        all_articles = ticket.articles
+
+        if article.nil?
+          last_article = all_articles.last
+          last_internal_article = all_articles.reverse.find(&:internal?)
+          last_external_article = all_articles.reverse.find { |a| !a.internal? }
+        else
+          last_article = article
+          last_internal_article = article.internal? ? article : all_articles.reverse.find(&:internal?)
+          last_external_article = article.internal? ? all_articles.reverse.find { |a| !a.internal? } : article
+        end
+
+        {
+          ticket:                   ticket,
+          article:                  last_article,
+          last_article:             last_article,
+          last_internal_article:    last_internal_article,
+          last_external_article:    last_external_article,
+          created_article:          article,
+          created_internal_article: article&.internal? ? article : nil,
+          created_external_article: article&.internal? ? nil : article,
+        }
+      end
+      let(:renderer) do
+        build(:notification_factory_renderer,
+              objects:  objects,
+              template: template)
+      end
+      let(:body)     { 'test' }
+      let(:article)  { create(:ticket_article, ticket: ticket, body: body) }
+
+      context 'with ticket.tags as template' do
+        let(:template) { '#{ticket.tags}' }
+
+        before do
+          ticket.tag_add('Tag1', customer.id)
+        end
+
+        it 'correctly renders ticket tags references' do
+          expect(renderer.render).to eq 'Tag1'
+        end
+      end
+
+      %w[article last_article last_internal_article last_external_article
+         created_article created_internal_article created_external_article].each do |tag|
+        context "with #{tag}.body as template" do
+          let(:template) { "\#{#{tag}.body}" }
+          let(:article) do
+            create(
+              :ticket_article,
+              ticket:   ticket,
+              body:     body,
+              internal: tag.match?('internal')
+            )
+          end
+
+          it "renders an #{tag} body with quote" do
+            expect(renderer.render).to eq "&gt; #{body}<br>"
+          end
+        end
+      end
+    end
+
     context 'when handling ObjectManager::Attribute usage', db_strategy: :reset do
       before do
         create_object_manager_attribute

spec/models/ticket_spec.rb

@@ -617,13 +617,41 @@ RSpec.describe Ticket, type: :model do
                   })
           end
 
+          let(:objects) do
+            last_article = nil
+            last_internal_article = nil
+            last_external_article = nil
+            all_articles = ticket.articles
+
+            if article.nil?
+              last_article = all_articles.last
+              last_internal_article = all_articles.reverse.find(&:internal?)
+              last_external_article = all_articles.reverse.find { |a| !a.internal? }
+            else
+              last_article = article
+              last_internal_article = article.internal? ? article : all_articles.reverse.find(&:internal?)
+              last_external_article = article.internal? ? all_articles.reverse.find { |a| !a.internal? } : article
+            end
+
+            {
+              ticket:                   ticket,
+              article:                  last_article,
+              last_article:             last_article,
+              last_internal_article:    last_internal_article,
+              last_external_article:    last_external_article,
+              created_article:          article,
+              created_internal_article: article&.internal? ? article : nil,
+              created_external_article: article&.internal? ? nil : article,
+            }
+          end
+
           # required by Ticket#perform_changes for email notifications
           before { article.ticket.group.update(email_address: create(:email_address)) }
 
           it 'passes the first article to NotificationFactory::Mailer' do
             expect(NotificationFactory::Mailer)
               .to receive(:template)
-              .with(hash_including(objects: { ticket: ticket, article: article }))
+              .with(hash_including(objects: objects))
               .at_least(:once)
               .and_call_original
 
@@ -642,6 +670,15 @@ RSpec.describe Ticket, type: :model do
         # Notification triggers should log notification as private or public
         # according to given configuration
         let(:user) { create(:admin, mobile: '+37061010000') }
+        let(:item) do
+          {
+            object:     'Ticket',
+            object_id:  ticket.id,
+            user_id:    user.id,
+            type:       'update',
+            article_id: ticket_article.id
+          }
+        end
 
         before { ticket.group.users << user }
 
@@ -655,7 +692,8 @@ RSpec.describe Ticket, type: :model do
           }.deep_merge(additional_options).deep_stringify_keys
         end
 
-        let(:notification_key) { "notification.#{notification_type}" }
+        let(:notification_key)  { "notification.#{notification_type}" }
+        let!(:ticket_article)   { create(:ticket_article, ticket: ticket) }
 
         shared_examples 'verify log visibility status' do
           shared_examples 'notification trigger' do
@@ -718,7 +756,7 @@ RSpec.describe Ticket, type: :model do
 
         shared_examples 'add a new article' do
           it 'adds a new article' do
-            expect { ticket.perform_changes(performable, 'trigger', ticket, user) }
+            expect { ticket.perform_changes(performable, 'trigger', item, user) }
               .to change { ticket.articles.count }.by(1)
           end
         end
@@ -727,7 +765,7 @@ RSpec.describe Ticket, type: :model do
           include_examples 'add a new article'
 
           it 'adds attachment to the new article' do
-            ticket.perform_changes(performable, 'trigger', ticket, user)
+            ticket.perform_changes(performable, 'trigger', item, user)
             article = ticket.articles.last
 
             expect(article.type.name).to eq('email')
@@ -742,7 +780,7 @@ RSpec.describe Ticket, type: :model do
           include_examples 'add a new article'
 
           it 'does not add attachment to the new article' do
-            ticket.perform_changes(performable, 'trigger', ticket, user)
+            ticket.perform_changes(performable, 'trigger', item, user)
             article = ticket.articles.last
 
             expect(article.type.name).to eq('email')
@@ -765,7 +803,6 @@ RSpec.describe Ticket, type: :model do
 
             before do
               UserInfo.current_user_id = 1
-              ticket_article = create(:ticket_article, ticket: ticket)
 
               create(:store,
                      object:      'Ticket::Article',
@@ -801,7 +838,6 @@ RSpec.describe Ticket, type: :model do
 
             before do
               UserInfo.current_user_id = 1
-              ticket_article = create(:ticket_article, ticket: ticket)
 
               create(:store,
                      object:      'Ticket::Article',