From dcabd65e548c302b124d6db749919d2995b59b58 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sun, 29 Sep 2013 23:37:49 +0200 Subject: [PATCH] Improved query of activity stream. --- app/models/activity_stream.rb | 37 +++++++++++++------ app/models/application_model.rb | 9 ++--- .../application_model/activity_stream_base.rb | 4 +- app/models/ticket.rb | 22 ++++++----- app/models/ticket/activity_stream_log.rb | 32 ++++++++++++++++ app/models/ticket/article.rb | 7 +++- .../ticket/article/activity_stream_log.rb | 33 +++++++++++++++++ test/unit/activity_stream_test.rb | 24 ++++++++++-- 8 files changed, 138 insertions(+), 30 deletions(-) create mode 100644 app/models/ticket/activity_stream_log.rb create mode 100644 app/models/ticket/article/activity_stream_log.rb diff --git a/app/models/activity_stream.rb b/app/models/activity_stream.rb index 6ff5682fe..13f8d67ef 100644 --- a/app/models/activity_stream.rb +++ b/app/models/activity_stream.rb @@ -35,10 +35,11 @@ add a new activity entry for an object role_id = nil if data[:role] - role_id = Role.lookup( :name => data[:role] ) - if !role_id + role = Role.lookup( :name => data[:role] ) + if !role raise "No such Role #{data[:role]}" end + role_id = role.id end # check if entry is needed @@ -52,16 +53,18 @@ add a new activity entry for an object # resturn if old entry is really freash return result if result && result.created_at >= (data[:created_at] - 10.seconds) - puts "AS: #{data[:type]} #{data[:object]} #{data[:o_id]}" # create history record = { :o_id => data[:o_id], :activity_stream_type_id => type.id, :activity_stream_object_id => object.id, + :role_id => role_id, + :group_id => data[:group_id], :created_at => data[:created_at], :created_by_id => data[:created_by_id] } + ActivityStream.create(record) end @@ -90,10 +93,22 @@ return all activity entries of an user =end def self.list(user,limit) -# stream = ActivityStream.where( :role_id => user.roles, :group_id => user.groups ) - stream = ActivityStream.where('1=1'). - order( 'created_at DESC, id DESC' ). - limit( limit ) + role_ids = user.role_ids + group_ids = user.group_ids + + # do not return an activity stream for custoers + customer_role = Role.lookup( :name => 'Customer' ) + + return [] if role_ids.include?(customer_role.id) + if group_ids.empty? + stream = ActivityStream.where('(role_id IN (?) AND group_id is NULL)', role_ids ). + order( 'created_at DESC, id DESC' ). + limit( limit ) + else + stream = ActivityStream.where('(role_id IN (?) AND group_id is NULL) OR ( role_id IN (?) AND group_id IN (?) ) OR ( role_id is NULL AND group_id IN (?) )', role_ids, role_ids, group_ids, group_ids ). + order( 'created_at DESC, id DESC' ). + limit( limit ) + end list = [] stream.each do |item| data = item.attributes @@ -114,7 +129,7 @@ return all activity entries of an user return @@cache_type[ id ] if @@cache_type[ id ] # lookup - type = ActivityStream::Type.find(id) + type = ActivityStream::Type.lookup( :id => id ) @@cache_type[ id ] = type type end @@ -125,7 +140,7 @@ return all activity entries of an user return @@cache_type[ name ] if @@cache_type[ name ] # lookup - type = ActivityStream::Type.where( :name => name ).first + type = ActivityStream::Type.lookup( :name => name ) if type @@cache_type[ name ] = type return type @@ -145,7 +160,7 @@ return all activity entries of an user return @@cache_object[ id ] if @@cache_object[ id ] # lookup - object = ActivityStream::Object.find(id) + object = ActivityStream::Object.lookup( :id => id ) @@cache_object[ id ] = object object end @@ -156,7 +171,7 @@ return all activity entries of an user return @@cache_object[ name ] if @@cache_object[ name ] # lookup - object = ActivityStream::Object.where( :name => name ).first + object = ActivityStream::Object.lookup( :name => name ) if object @@cache_object[ name ] = object return object diff --git a/app/models/application_model.rb b/app/models/application_model.rb index eeba7143d..95950ef72 100644 --- a/app/models/application_model.rb +++ b/app/models/application_model.rb @@ -533,15 +533,12 @@ log object update history with all updated attributes, if configured - will be e #puts 'updated ' + self.changes.inspect return if changes['id'] && !changes['id'][0] - # TODO: Swop it to config file later + # default ignored attributes ignore_attributes = { :created_at => true, :updated_at => true, :created_by_id => true, :updated_by_id => true, - :article_count => true, - :create_article_type_id => true, - :create_article_sender_id => true, } changes.each {|key, value| @@ -562,7 +559,7 @@ log object update history with all updated attributes, if configured - will be e if self.respond_to?( attribute_name ) relation_class = self.send(attribute_name).class - if relation_class + if relation_class && value_id[0] relation_model = relation_class.lookup( :id => value_id[0] ) if relation_model if relation_model['name'] @@ -571,6 +568,8 @@ log object update history with all updated attributes, if configured - will be e value[0] = relation_model.send('fullname') end end + end + if relation_class && value_id[1] relation_model = relation_class.lookup( :id => value_id[1] ) if relation_model if relation_model['name'] diff --git a/app/models/application_model/activity_stream_base.rb b/app/models/application_model/activity_stream_base.rb index 91aaaeaaa..da6a415cf 100644 --- a/app/models/application_model/activity_stream_base.rb +++ b/app/models/application_model/activity_stream_base.rb @@ -17,11 +17,13 @@ returns def activity_stream_log (type, user_id) return if !self.class.activity_stream_support_config + role = self.class.activity_stream_support_config[:role] ActivityStream.add( :o_id => self['id'], :type => type, :object => self.class.name, -# :role => self.activity_stream_role, + :group_id => self['group_id'], + :role => role, :created_at => self.updated_at, :created_by_id => user_id, ) diff --git a/app/models/ticket.rb b/app/models/ticket.rb index ebed9d467..fd92504f3 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -1,6 +1,14 @@ # Copyright (C) 2012-2013 Zammad Foundation, http://zammad-foundation.org/ class Ticket < ApplicationModel + include Ticket::Escalation + include Ticket::Subject + include Ticket::Permission + include Ticket::Assets + include Ticket::HistoryLog + include Ticket::ActivityStreamLog + extend Ticket::Search + before_create :check_generate, :check_defaults before_update :check_defaults before_destroy :destroy_dependencies @@ -8,8 +16,11 @@ class Ticket < ApplicationModel after_update :notify_clients_after_update after_destroy :notify_clients_after_destroy - activity_stream_support :role => 'User' - history_support + activity_stream_support + + history_support :ignore_attributes => { + :article_count => true, + } belongs_to :group has_many :articles, :class_name => 'Ticket::Article', :after_add => :cache_update, :after_remove => :cache_update @@ -22,13 +33,6 @@ class Ticket < ApplicationModel belongs_to :create_article_type, :class_name => 'Ticket::Article::Type' belongs_to :create_article_sender, :class_name => 'Ticket::Article::Sender' - include Ticket::Escalation - include Ticket::Subject - include Ticket::Permission - include Ticket::Assets - include Ticket::HistoryLog - extend Ticket::Search - attr_accessor :callback_loop =begin diff --git a/app/models/ticket/activity_stream_log.rb b/app/models/ticket/activity_stream_log.rb new file mode 100644 index 000000000..b47483a7d --- /dev/null +++ b/app/models/ticket/activity_stream_log.rb @@ -0,0 +1,32 @@ +# Copyright (C) 2012-2013 Zammad Foundation, http://zammad-foundation.org/ + +module Ticket::ActivityStreamLog + +=begin + +log activity for this object + + ticket = Ticket.find(123) + result = ticket.activity_stream_log( 'created', user_id ) + +returns + + result = true # false + +=end + + def activity_stream_log (type, user_id) + return if !self.class.activity_stream_support_config + role = self.class.activity_stream_support_config[:role] + ActivityStream.add( + :o_id => self['id'], + :type => type, + :object => self.class.name, + :group_id => self['group_id'], + :role => role, + :created_at => self.updated_at, + :created_by_id => user_id, + ) + end + +end \ No newline at end of file diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index 143044cb1..c292e7111 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -3,6 +3,7 @@ class Ticket::Article < ApplicationModel include Ticket::Article::Assets include Ticket::Article::HistoryLog + include Ticket::Article::ActivityStreamLog belongs_to :ticket belongs_to :ticket_article_type, :class_name => 'Ticket::Article::Type' @@ -14,7 +15,11 @@ class Ticket::Article < ApplicationModel after_destroy :notify_clients_after_destroy activity_stream_support - history_support + + history_support :ignore_attributes => { + :create_article_type_id => true, + :create_article_sender_id => true, + } attr_accessor :attachments diff --git a/app/models/ticket/article/activity_stream_log.rb b/app/models/ticket/article/activity_stream_log.rb new file mode 100644 index 000000000..c9b10da9d --- /dev/null +++ b/app/models/ticket/article/activity_stream_log.rb @@ -0,0 +1,33 @@ +# Copyright (C) 2012-2013 Zammad Foundation, http://zammad-foundation.org/ + +module Ticket::Article::ActivityStreamLog + +=begin + +log activity for this object + + article = Ticket::Article.find(123) + result = article.activity_stream_log( 'created', user_id ) + +returns + + result = true # false + +=end + + def activity_stream_log (type, user_id) + return if !self.class.activity_stream_support_config + role = self.class.activity_stream_support_config[:role] + ticket = Ticket.lookup( :id => self.ticket_id ) + ActivityStream.add( + :o_id => self['id'], + :type => type, + :object => self.class.name, + :group_id => ticket.group_id, + :role => role, + :created_at => self.updated_at, + :created_by_id => user_id, + ) + end + +end \ No newline at end of file diff --git a/test/unit/activity_stream_test.rb b/test/unit/activity_stream_test.rb index 32db60f8b..62940df4e 100644 --- a/test/unit/activity_stream_test.rb +++ b/test/unit/activity_stream_test.rb @@ -2,6 +2,21 @@ require 'test_helper' class ActivityStreamTest < ActiveSupport::TestCase + role = Role.lookup( :name => 'Admin' ) + group = Group.lookup( :name => 'Users' ) + user = User.create_or_update( + :login => 'admin', + :firstname => 'Bob', + :lastname => 'Smith', + :email => 'bob@example.com', + :password => 'some_pass', + :active => true, + :role_ids => [role.id], + :group_ids => [group.id], + :updated_by_id => 1, + :created_by_id => 1 + ) + test 'ticket+user' do tests = [ @@ -78,7 +93,9 @@ class ActivityStreamTest < ActiveSupport::TestCase # update ticket if test[:update][:ticket] ticket.update_attributes( test[:update][:ticket] ) - test[:check][2][:o_id] = ticket.id + + # check updated user + test[:check][2][:o_id] = User.lookup( :login => 'nicole.braun@zammad.org' ).id test[:check][2][:created_at] = ticket.created_at test[:check][2][:created_by_id] = User.lookup( :login => 'nicole.braun@zammad.org' ).id end @@ -91,7 +108,7 @@ class ActivityStreamTest < ActiveSupport::TestCase tickets.push ticket # check activity_stream - activity_stream_check( User.find(1).activity_stream(3), test[:check] ) + activity_stream_check( user.activity_stream(3), test[:check] ) } # delete tickets @@ -164,7 +181,7 @@ class ActivityStreamTest < ActiveSupport::TestCase organizations.push organization # check activity_stream - activity_stream_check( User.find(1).activity_stream(2), test[:check] ) + activity_stream_check( user.activity_stream(2), test[:check] ) } # delete tickets @@ -178,6 +195,7 @@ class ActivityStreamTest < ActiveSupport::TestCase def activity_stream_check( activity_stream_list, checks ) + puts 'AS ' + activity_stream_list.inspect checks.each { |check_item| # puts '+++++++++++' # puts check_item.inspect