Small SQL improvement to overview generation.

app/controllers/application_controller/renders_models.rb

@@ -12,12 +12,12 @@ module ApplicationController::RendersModels
     # create object
     generic_object = object.new(clean_params)
 
-    # save object
-    generic_object.save!
-
     # set relations
     generic_object.associations_from_param(params)
 
+    # save object
+    generic_object.save!
+
     if response_expand?
       render json: generic_object.attributes_with_association_names, status: :created
       return

app/models/overview.rb

@@ -10,11 +10,12 @@ class Overview < ApplicationModel
   include Overview::Assets
 
   has_and_belongs_to_many :roles, after_add: :cache_update, after_remove: :cache_update, class_name: 'Role'
-  has_and_belongs_to_many :users, after_add: :cache_update, after_remove: :cache_update
+  has_and_belongs_to_many :users, after_add: :cache_update, after_remove: :cache_update, class_name: 'User'
   store     :condition
   store     :order
   store     :view
   validates :name, presence: true
+  validates :roles, presence: true
 
   before_create :fill_link_on_create, :fill_prio
   before_update :fill_link_on_update, :rearrangement

app/models/ticket/overviews.rb

@@ -5,7 +5,7 @@ module Ticket::Overviews
 
 all overviews by user
 
-  result = Ticket::Overviews.all(current_user: User.find(123))
+  result = Ticket::Overviews.all(current_user: User.find(3))
 
 returns
 
@@ -23,14 +23,8 @@ returns
       if current_user.organization_id && current_user.organization.shared
         overview_filter.delete(:organization_shared)
       end
-      overviews = Overview.joins(:roles).where(overviews_roles: { role_id: role_ids }, overviews: overview_filter).distinct('overview.id').order(:prio)
+      overviews = Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: nil }, overviews: overview_filter).or(Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: current_user.id }, overviews: overview_filter)).distinct('overview.id').order(:prio)
-      overviews_list = []
+      return overviews
-      overviews.each do |overview|
-        user_ids = overview.user_ids
-        next if user_ids.present? && !user_ids.include?(current_user.id)
-        overviews_list.push overview
-      end
-      return overviews_list
     end
 
     # get agent overviews
@@ -40,14 +34,7 @@ returns
     if User.where('out_of_office = ? AND out_of_office_start_at <= ? AND out_of_office_end_at >= ? AND out_of_office_replacement_id = ? AND active = ?', true, Time.zone.today, Time.zone.today, current_user.id, true).count.positive?
       overview_filter_not = {}
     end
-    overviews = Overview.joins(:roles).where(overviews_roles: { role_id: role_ids }, overviews: overview_filter).where.not(overview_filter_not).distinct('overview.id').order(:prio)
+    Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: nil }, overviews: overview_filter).or(Overview.joins(:roles).left_joins(:users).where(overviews_roles: { role_id: role_ids }, overviews_users: { user_id: current_user.id }, overviews: overview_filter)).where.not(overview_filter_not).distinct('overview.id').order(:prio)
-    overviews_list = []
-    overviews.each do |overview|
-      user_ids = overview.user_ids
-      next if user_ids.present? && !user_ids.include?(current_user.id)
-      overviews_list.push overview
-    end
-    overviews_list
   end
 
 =begin

app/models/transaction/trigger.rb

@@ -67,14 +67,14 @@ class Transaction::Trigger
         # check ticket "has changed" options
         has_changed_done = true
         condition.each do |key, value|
-          next if !value
+          next if value.blank?
-          next if !value['operator']
+          next if value['operator'].blank?
           next if !value['operator']['has changed']
 
           # remove condition item, because it has changed
           (object_name, attribute) = key.split('.', 2)
           next if object_name != 'ticket'
-          next if !@item[:changes]
+          next if @item[:changes].blank?
           next if !@item[:changes].key?(attribute)
           condition.delete(key)
           one_has_changed_done = true
@@ -82,8 +82,8 @@ class Transaction::Trigger
 
         # check if we have not matching "has changed" attributes
         condition.each_value do |value|
-          next if !value
+          next if value.blank?
-          next if !value['operator']
+          next if value['operator'].blank?
           next if !value['operator']['has changed']
           has_changed_done = false
           break
@@ -106,7 +106,8 @@ class Transaction::Trigger
             (object_name, attribute) = key.split('.', 2)
             next if object_name != 'ticket'
             one_has_changed_condition = true
-            next if !@item[:changes] || !@item[:changes].key?(attribute)
+            next if @item[:changes].blank?
+            next if !@item[:changes].key?(attribute)
             one_has_changed_done = true
             break
           end

test/controllers/overviews_controller_test.rb

@@ -44,6 +44,7 @@ class OverviewsControllerTest < ActionDispatch::IntegrationTest
     params = {
       name: 'Overview2',
       link: 'my_overview',
+      roles: Role.where(name: 'Agent').pluck(:name),
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -75,6 +76,7 @@ class OverviewsControllerTest < ActionDispatch::IntegrationTest
     params = {
       name: 'Overview2',
       link: 'my_overview',
+      roles: Role.where(name: 'Agent').pluck(:name),
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -109,9 +111,11 @@ class OverviewsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test 'set mass prio' do
+    roles = Role.where(name: 'Agent')
     overview1 = Overview.create!(
       name: 'Overview1',
       link: 'my_overview',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -135,6 +139,7 @@ class OverviewsControllerTest < ActionDispatch::IntegrationTest
     overview2 = Overview.create!(
       name: 'Overview2',
       link: 'my_overview',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',

test/unit/overview_test.rb

@@ -5,8 +5,11 @@ class OverviewTest < ActiveSupport::TestCase
 
   test 'overview link' do
     UserInfo.current_user_id = 1
+    roles = Role.where(name: 'Agent')
+
     overview = Overview.create!(
       name: 'Not Shown Admin 2',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -29,6 +32,7 @@ class OverviewTest < ActiveSupport::TestCase
 
     overview = Overview.create!(
       name: 'My assigned Tickets 2',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -51,6 +55,7 @@ class OverviewTest < ActiveSupport::TestCase
 
     overview = Overview.create!(
       name: 'Übersicht',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -73,6 +78,7 @@ class OverviewTest < ActiveSupport::TestCase
 
     overview = Overview.create!(
       name: "   Übersicht   \n",
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -95,6 +101,7 @@ class OverviewTest < ActiveSupport::TestCase
 
     overview1 = Overview.create!(
       name: 'Meine Übersicht',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -115,6 +122,7 @@ class OverviewTest < ActiveSupport::TestCase
     assert_equal(overview1.link, 'meine_ubersicht')
     overview2 = Overview.create!(
       name: 'Meine Übersicht',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -139,6 +147,7 @@ class OverviewTest < ActiveSupport::TestCase
 
     overview = Overview.create!(
       name: 'Д дФ ф',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -161,6 +170,7 @@ class OverviewTest < ActiveSupport::TestCase
 
     overview = Overview.create!(
       name: ' Д дФ ф abc ',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -184,6 +194,7 @@ class OverviewTest < ActiveSupport::TestCase
     overview = Overview.create!(
       name: 'Übersicht',
       link: 'my_overview',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -215,8 +226,11 @@ class OverviewTest < ActiveSupport::TestCase
   test 'same url' do
     UserInfo.current_user_id = 1
 
+    roles = Role.where(name: 'Agent')
+
     overview1 = Overview.create!(
       name: 'My own assigned Tickets',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -238,6 +252,7 @@ class OverviewTest < ActiveSupport::TestCase
 
     overview2 = Overview.create!(
       name: 'My own assigned Tickets',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -259,6 +274,7 @@ class OverviewTest < ActiveSupport::TestCase
 
     overview3 = Overview.create!(
       name: 'My own assigned Tickets',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -286,9 +302,12 @@ class OverviewTest < ActiveSupport::TestCase
   test 'priority rearrangement' do
     UserInfo.current_user_id = 1
 
+    roles = Role.where(name: 'Agent')
+
     overview1 = Overview.create!(
       name: 'Overview1',
       link: 'my_overview',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -311,6 +330,7 @@ class OverviewTest < ActiveSupport::TestCase
     overview2 = Overview.create!(
       name: 'Overview2',
       link: 'my_overview',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',
@@ -333,6 +353,7 @@ class OverviewTest < ActiveSupport::TestCase
     overview3 = Overview.create!(
       name: 'Overview3',
       link: 'my_overview',
+      roles: roles,
       condition: {
         'ticket.state_id' => {
           operator: 'is',

test/unit/ticket_overview_test.rb

@@ -302,11 +302,12 @@ class TicketOverviewTest < ActiveSupport::TestCase
     )
   end
 
-  test 'bbb overview index' do
+  test 'overview index' do
 
     result = Ticket::Overviews.all(
       current_user: @agent1,
     )
+
     assert_equal(3, result.count)
     assert_equal('My assigned Tickets', result[0].name)
     assert_equal('Unassigned & Open', result[1].name)
@@ -343,7 +344,42 @@ class TicketOverviewTest < ActiveSupport::TestCase
 
   end
 
-  test 'ccc overview content' do
+  test 'missing role' do
+    Ticket.destroy_all
+
+    assert_raises(Exception) do
+      Overview.create!(
+        name: 'new overview',
+        link: 'new_overview',
+        prio: 1200,
+        user_ids: [@customer2.id],
+        organization_shared: true,
+        condition: {
+          'ticket.state_id' => {
+            operator: 'is',
+            value: [1, 2, 3],
+          },
+          'ticket.organization_id' => {
+            operator: 'is',
+            pre_condition: 'current_user.organization_id',
+          },
+        },
+        order: {
+          by: 'created_at',
+          direction: 'DESC',
+        },
+        view: {
+          d: %w[title customer state created_at],
+          s: %w[number title customer state created_at],
+          m: %w[number title customer state created_at],
+          view_mode_default: 's',
+        },
+      )
+    end
+
+  end
+
+  test 'overview content' do
 
     Ticket.destroy_all
 

test/unit/ticket_trigger_test.rb

@@ -3480,4 +3480,201 @@ class TicketTriggerTest < ActiveSupport::TestCase
 
   end
 
+  test '4 tag based auto response' do
+    trigger1 = Trigger.create_or_update(
+      name: '100 add tag if sender 1',
+      condition: {
+        'ticket.action' => {
+          'operator' => 'is',
+          'value' => 'create',
+        },
+        'article.from' => {
+          'operator' => 'contains',
+          'value' => 'sender1',
+        },
+      },
+      perform: {
+        'ticket.tags' => {
+          'operator' => 'add',
+          'value' => 'sender1',
+        },
+      },
+      disable_notification: true,
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    trigger2 = Trigger.create_or_update(
+      name: '200 add tag if sender 2',
+      condition: {
+        'ticket.action' => {
+          'operator' => 'is',
+          'value' => 'create',
+        },
+        'article.from' => {
+          'operator' => 'contains',
+          'value' => 'sender2',
+        },
+      },
+      perform: {
+        'ticket.tags' => {
+          'operator' => 'add',
+          'value' => 'sender2',
+        },
+      },
+      disable_notification: true,
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    trigger3 = Trigger.create_or_update(
+      name: '300 auto reply',
+      condition: {
+        'ticket.action' => {
+          'operator' => 'is',
+          'value' => 'create',
+        },
+        'ticket.state_id' => {
+          'operator' => 'is',
+          'value' => Ticket::State.lookup(name: 'new').id.to_s,
+        },
+        'ticket.tags' => {
+          #'operator' => 'contains one not',
+          'operator' => 'contains all not',
+          'value' => 'sender1, sender2',
+        },
+      },
+      perform: {
+        'notification.email' => {
+          'body' => 'some text<br>#{ticket.customer.lastname}<br>#{ticket.title}<br>#{article.body}',
+          'recipient' => 'ticket_customer',
+          'subject' => 'Thanks for your inquiry (#{ticket.title})!',
+        },
+      },
+      disable_notification: true,
+      active: true,
+      created_by_id: 1,
+      updated_by_id: 1,
+    )
+
+    ticket1 = Ticket.create(
+      title: 'test 1',
+      group: Group.lookup(name: 'Users'),
+      customer: User.lookup(email: 'nicole.braun@zammad.org'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert(ticket1, 'ticket1 created')
+    Ticket::Article.create(
+      ticket_id: ticket1.id,
+      from: 'sender1@example.com',
+      to: 'some_recipient@example.com',
+      subject: 'some subject',
+      message_id: 'some@id',
+      body: "some message <b>note</b>\nnew line",
+      internal: false,
+      sender: Ticket::Article::Sender.find_by(name: 'Customer'),
+      type: Ticket::Article::Type.find_by(name: 'email'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    ticket1.reload
+    assert_equal('test 1', ticket1.title, 'ticket1.title verify')
+    assert_equal('Users', ticket1.group.name, 'ticket1.group verify')
+    assert_equal('new', ticket1.state.name, 'ticket1.state verify')
+    assert_equal('2 normal', ticket1.priority.name, 'ticket1.priority verify')
+    assert_equal(1, ticket1.articles.count, 'ticket1.articles verify')
+    assert_equal([], ticket1.tag_list)
+    Observer::Transaction.commit
+    ticket1.reload
+    assert_equal('test 1', ticket1.title, 'ticket1.title verify')
+    assert_equal('Users', ticket1.group.name, 'ticket1.group verify')
+    assert_equal('new', ticket1.state.name, 'ticket1.state verify')
+    assert_equal('2 normal', ticket1.priority.name, 'ticket1.priority verify')
+    assert_equal(1, ticket1.articles.count, 'ticket1.articles verify')
+    assert_equal(%w[sender1], ticket1.tag_list)
+
+    ticket2 = Ticket.create(
+      title: 'test 2',
+      group: Group.lookup(name: 'Users'),
+      customer: User.lookup(email: 'nicole.braun@zammad.org'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert(ticket2, 'ticket2 created')
+    Ticket::Article.create(
+      ticket_id: ticket2.id,
+      from: 'sender2@example.com',
+      to: 'some_recipient@example.com',
+      subject: 'some subject',
+      message_id: 'some@id',
+      body: "some message <b>note</b>\nnew line",
+      internal: false,
+      sender: Ticket::Article::Sender.find_by(name: 'Customer'),
+      type: Ticket::Article::Type.find_by(name: 'email'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    ticket2.reload
+    assert_equal('test 2', ticket2.title, 'ticket2.title verify')
+    assert_equal('Users', ticket2.group.name, 'ticket2.group verify')
+    assert_equal('new', ticket2.state.name, 'ticket2.state verify')
+    assert_equal('2 normal', ticket2.priority.name, 'ticket2.priority verify')
+    assert_equal(1, ticket2.articles.count, 'ticket2.articles verify')
+    assert_equal([], ticket2.tag_list)
+
+    Observer::Transaction.commit
+
+    ticket2.reload
+    assert_equal('test 2', ticket2.title, 'ticket2.title verify')
+    assert_equal('Users', ticket2.group.name, 'ticket2.group verify')
+    assert_equal('new', ticket2.state.name, 'ticket2.state verify')
+    assert_equal('2 normal', ticket2.priority.name, 'ticket2.priority verify')
+    assert_equal(1, ticket2.articles.count, 'ticket2.articles verify')
+    assert_equal(%w[sender2], ticket2.tag_list)
+
+    ticket3 = Ticket.create(
+      title: 'test 3',
+      group: Group.lookup(name: 'Users'),
+      customer: User.lookup(email: 'nicole.braun@zammad.org'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert(ticket3, 'ticket3 created')
+    Ticket::Article.create(
+      ticket_id: ticket3.id,
+      from: 'sender0@example.com',
+      to: 'some_recipient@example.com',
+      subject: 'some subject',
+      message_id: 'some@id',
+      body: "some message <b>note</b>\nnew line",
+      internal: false,
+      sender: Ticket::Article::Sender.find_by(name: 'Customer'),
+      type: Ticket::Article::Type.find_by(name: 'email'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    ticket3.reload
+    assert_equal('test 3', ticket3.title, 'ticket3.title verify')
+    assert_equal('Users', ticket3.group.name, 'ticket3.group verify')
+    assert_equal('new', ticket3.state.name, 'ticket3.state verify')
+    assert_equal('2 normal', ticket3.priority.name, 'ticket3.priority verify')
+    assert_equal(1, ticket3.articles.count, 'ticket3.articles verify')
+    assert_equal([], ticket3.tag_list)
+    Observer::Transaction.commit
+    ticket3.reload
+    assert_equal('test 3', ticket3.title, 'ticket3.title verify')
+    assert_equal('Users', ticket3.group.name, 'ticket3.group verify')
+    assert_equal('new', ticket3.state.name, 'ticket3.state verify')
+    assert_equal('2 normal', ticket3.priority.name, 'ticket3.priority verify')
+    assert_equal(2, ticket3.articles.count, 'ticket3.articles verify')
+    assert_equal([], ticket3.tag_list)
+    article1 = ticket3.articles.last
+
+  end
 end