diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index 885220aa1..930a97b8e 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -314,10 +314,7 @@ curl http://localhost/api/v1/organization/{id} -v -u #{login}:#{password} -H "Co organization = Organization.find(params[:id]) # get history of organization - history = organization.history_get(true) - - # return result - render json: history + render json: organization.history_get(true) end # @path [GET] /organizations/import_example diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index e5972d077..bcbc7869e 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -294,10 +294,7 @@ class TicketsController < ApplicationController access!(ticket, 'read') # get history of ticket - history = ticket.history_get(true) - - # return result - render json: history + render json: ticket.history_get(true) end # GET /api/v1/ticket_related/1 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 1e6b9a86c..fbb0cded5 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -502,10 +502,7 @@ class UsersController < ApplicationController user = User.find(params[:id]) # get history of user - history = user.history_get(true) - - # return result - render json: history + render json: user.history_get(true) end =begin diff --git a/app/models/concerns/has_history.rb b/app/models/concerns/has_history.rb index d96e80740..46bcbc528 100644 --- a/app/models/concerns/has_history.rb +++ b/app/models/concerns/has_history.rb @@ -204,12 +204,14 @@ returns =end def history_get(fulldata = false) + relation_object = self.class.instance_variable_get(:@history_relation_object) || nil + if !fulldata - return History.list(self.class.name, self['id']) + return History.list(self.class.name, self['id'], relation_object) end # get related objects - history = History.list(self.class.name, self['id'], nil, true) + history = History.list(self.class.name, self['id'], relation_object, true) history[:list].each do |item| record = Kernel.const_get(item['object']).find(item['o_id']) @@ -242,5 +244,21 @@ end def history_attributes_ignored(*attributes) @history_attributes_ignored = attributes end + +=begin + +serve methode to ignore model attributes in historization + +class Model < ApplicationModel + include HasHistory + history_relation_object 'Some::Relation::Object' +end + +=end + + def history_relation_object(attribute) + @history_relation_object = attribute + end + end end diff --git a/app/models/history.rb b/app/models/history.rb index 5232aa683..29012769a 100644 --- a/app/models/history.rb +++ b/app/models/history.rb @@ -40,19 +40,19 @@ add a new history entry for an object return if Setting.get('import_mode') && !data[:id] # lookups - if data[:history_type] + if data[:history_type].present? history_type = type_lookup(data[:history_type]) end - if data[:history_object] + if data[:history_object].present? history_object = object_lookup(data[:history_object]) end related_history_object_id = nil - if data[:related_history_object] + if data[:related_history_object].present? related_history_object = object_lookup(data[:related_history_object]) related_history_object_id = related_history_object.id end history_attribute_id = nil - if data[:history_attribute] + if data[:history_attribute].present? history_attribute = attribute_lookup(data[:history_attribute]) history_attribute_id = history_attribute.id end @@ -80,11 +80,11 @@ add a new history entry for an object if history_record history_record.update!(record) else - record_new = History.create(record) + record_new = History.create!(record) if record[:id] record_new.id = record[:id] end - record_new.save + record_new.save! end end @@ -148,7 +148,7 @@ returns =end def self.list(requested_object, requested_object_id, related_history_object = nil, assets = nil) - if !related_history_object + if related_history_object.blank? history_object = object_lookup(requested_object) history = History.where(history_object_id: history_object.id) .where(o_id: requested_object_id) @@ -220,14 +220,10 @@ returns def self.type_lookup(name) # lookup history_type = History::Type.lookup(name: name) - if history_type - return history_type - end + return history_type if history_type # create - History::Type.create( - name: name - ) + History::Type.create!(name: name) end def self.object_lookup_id(id) @@ -237,14 +233,10 @@ returns def self.object_lookup(name) # lookup history_object = History::Object.lookup(name: name) - if history_object - return history_object - end + return history_object if history_object # create - History::Object.create( - name: name - ) + History::Object.create!(name: name) end def self.attribute_lookup_id(id) @@ -254,14 +246,10 @@ returns def self.attribute_lookup(name) # lookup history_attribute = History::Attribute.lookup(name: name) - if history_attribute - return history_attribute - end + return history_attribute if history_attribute # create - History::Attribute.create( - name: name - ) + History::Attribute.create!(name: name) end class Object < ApplicationModel diff --git a/app/models/ticket.rb b/app/models/ticket.rb index d9febb4d3..c5eef5a22 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -53,6 +53,8 @@ class Ticket < ApplicationModel :article_count, :preferences + history_relation_object 'Ticket::Article' + belongs_to :group belongs_to :organization has_many :articles, class_name: 'Ticket::Article', after_add: :cache_update, after_remove: :cache_update, dependent: :destroy, inverse_of: :ticket @@ -878,7 +880,7 @@ perform changes on ticket next if value['value'].blank? next if value['value'] != 'delete' - logger.debug { "Deleted ticket from #{perform_origin} #{perform.inspect} Ticket.find(#{id})" } + logger.info { "Deleted ticket from #{perform_origin} #{perform.inspect} Ticket.find(#{id})" } destroy! next end @@ -1158,27 +1160,6 @@ result Ticket::Article.where(ticket_id: id).order(:created_at, :id) end - def history_get(fulldata = false) - list = History.list(self.class.name, self['id'], 'Ticket::Article') - return list if !fulldata - - # get related objects - assets = {} - list.each do |item| - record = Kernel.const_get(item['object']).find(item['o_id']) - assets = record.assets(assets) - - if item['related_object'] - record = Kernel.const_get(item['related_object']).find( item['related_o_id']) - assets = record.assets(assets) - end - end - { - history: list, - assets: assets, - } - end - private def check_generate diff --git a/lib/notification_factory/mailer.rb b/lib/notification_factory/mailer.rb index 2a5cf6d2b..ee90b0bed 100644 --- a/lib/notification_factory/mailer.rb +++ b/lib/notification_factory/mailer.rb @@ -210,7 +210,7 @@ retunes =end def self.already_sent?(ticket, recipient, type) - result = ticket.history_get() + result = ticket.history_get count = 0 result.each do |item| next if item['type'] != 'notification' diff --git a/spec/requests/organization_spec.rb b/spec/requests/organization_spec.rb index 1809c3a8f..973d4675d 100644 --- a/spec/requests/organization_spec.rb +++ b/spec/requests/organization_spec.rb @@ -452,6 +452,22 @@ RSpec.describe 'Organization', type: :request, searchindex: true do end + it 'does organization history' do + organization1 = create( + :organization, + name: 'some org', + ) + + authenticated_as(agent_user) + get "/api/v1/organizations/history/#{organization1.id}", params: {}, as: :json + expect(response).to have_http_status(200) + expect(json_response).to be_a_kind_of(Hash) + expect(json_response['history'].class).to eq(Array) + expect(json_response['assets'].class).to eq(Hash) + expect(json_response['assets']['Ticket']).to be_nil + expect(json_response['assets']['Organization'][organization1.id.to_s]).not_to be_nil + end + it 'does csv example - customer no access' do authenticated_as(customer_user) get '/api/v1/organizations/import_example', params: {}, as: :json diff --git a/spec/requests/ticket_spec.rb b/spec/requests/ticket_spec.rb index aba6c348a..79eebf696 100644 --- a/spec/requests/ticket_spec.rb +++ b/spec/requests/ticket_spec.rb @@ -2078,6 +2078,30 @@ RSpec.describe 'Ticket', type: :request do expect(json_response).to be_a_kind_of(Hash) expect(json_response['tickets']).to eq([ticket2.id, ticket1.id]) end + + it 'does ticket history ' do + ticket1 = create( + :ticket, + title: 'some title', + group: ticket_group, + customer_id: customer_user.id, + ) + create( + :ticket_article, + type: Ticket::Article::Type.lookup(name: 'note'), + sender: Ticket::Article::Sender.lookup(name: 'Customer'), + ticket_id: ticket1.id, + ) + + authenticated_as(agent_user) + get "/api/v1/ticket_history/#{ticket1.id}", params: {}, as: :json + expect(response).to have_http_status(200) + expect(json_response).to be_a_kind_of(Hash) + expect(json_response['history'].class).to eq(Array) + expect(json_response['assets'].class).to eq(Hash) + expect(json_response['assets']['User'][customer_user.id.to_s]).not_to be_nil + expect(json_response['assets']['Ticket'][ticket1.id.to_s]).not_to be_nil + end end describe 'stats' do diff --git a/spec/requests/user_spec.rb b/spec/requests/user_spec.rb index 8137afeb6..fdf77611a 100644 --- a/spec/requests/user_spec.rb +++ b/spec/requests/user_spec.rb @@ -919,6 +919,25 @@ RSpec.describe 'User', type: :request, searchindex: true do user2.destroy! end + it 'does user history' do + user1 = create( + :customer_user, + login: 'history@example.com', + firstname: 'History', + lastname: 'Customer1', + email: 'history@example.com', + ) + + authenticated_as(agent_user) + get "/api/v1/users/history/#{user1.id}", params: {}, as: :json + expect(response).to have_http_status(200) + expect(json_response).to be_a_kind_of(Hash) + expect(json_response['history'].class).to eq(Array) + expect(json_response['assets'].class).to eq(Hash) + expect(json_response['assets']['Ticket']).to be_nil + expect(json_response['assets']['User'][user1.id.to_s]).not_to be_nil + end + it 'does user search sortable' do firstname = "user_search_sortable #{rand(999_999_999)}" diff --git a/test/unit/history_test.rb b/test/unit/history_test.rb index 36c540b20..3563633db 100644 --- a/test/unit/history_test.rb +++ b/test/unit/history_test.rb @@ -1,9 +1,9 @@ require 'test_helper' class HistoryTest < ActiveSupport::TestCase - current_user = User.lookup(email: 'nicole.braun@zammad.org') test 'ticket' do + current_user = User.lookup(email: 'nicole.braun@zammad.org') tests = [ # test 1 @@ -188,6 +188,7 @@ class HistoryTest < ActiveSupport::TestCase end test 'user' do + current_user = User.lookup(email: 'nicole.braun@zammad.org') name = rand(999_999) tests = [ @@ -275,6 +276,7 @@ class HistoryTest < ActiveSupport::TestCase end test 'organization' do + current_user = User.lookup(email: 'nicole.braun@zammad.org') tests = [ # test 1 @@ -337,6 +339,57 @@ class HistoryTest < ActiveSupport::TestCase organizations.each(&:destroy!) end + test 'ticket assets' do + UserInfo.current_user_id = 1 + agent1 = User.create!( + login: 'agent1@example.com', + firstname: 'agent', + lastname: '1', + email: 'agent1@example.com', + password: 'agentpw', + active: true, + roles: Role.where(name: %w[Agent Admin]), + groups: Group.all, + ) + current_user = User.lookup(email: 'nicole.braun@zammad.org') + UserInfo.current_user_id = current_user.id + + ticket = Ticket.create!( + title: 'test 1', + group: Group.first, + customer_id: current_user.id, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + ) + article = Ticket::Article.create!( + ticket_id: ticket.id, + from: 'some_customer_com-1@example.com', + to: 'some_zammad_com-1@example.com', + subject: 'com test 1', + message_id: 'some@id_com_1', + body: 'some message 123', + internal: false, + sender: Ticket::Article::Sender.find_by(name: 'Customer'), + type: Ticket::Article::Type.find_by(name: 'email'), + ) + + # verify if user of history record is in assets + UserInfo.current_user_id = agent1.id + ticket.state = Ticket::State.find_by(name: 'closed') + ticket.save! + + # update updated_by (to not include agent1 in assets by ticket) + UserInfo.current_user_id = current_user.id + ticket.priority = Ticket::Priority.find_by(name: '3 high') + ticket.save! + + history = ticket.history_get(true) + assert(history[:assets][:User][current_user.id]) + assert(history[:assets][:User][agent1.id]) + assert(history[:assets][:Ticket][ticket.id]) + assert(history[:assets][:TicketArticle][article.id]) + end + def history_check(history_list, history_check) history_check.each do |check_item| match = false diff --git a/test/unit/notification_factory_mailer_template_test.rb b/test/unit/notification_factory_mailer_template_test.rb index 29183d008..79839f6ff 100644 --- a/test/unit/notification_factory_mailer_template_test.rb +++ b/test/unit/notification_factory_mailer_template_test.rb @@ -8,7 +8,7 @@ class NotificationFactoryMailerTemplateTest < ActiveSupport::TestCase groups = Group.where(name: 'Users') roles = Role.where(name: 'Agent') - agent1 = User.create_or_update( + agent1 = User.create!( login: 'notification-template-agent1@example.com', firstname: 'Notificationxxx', lastname: 'Agent1yyy', @@ -24,7 +24,7 @@ class NotificationFactoryMailerTemplateTest < ActiveSupport::TestCase created_by_id: 1, ) - agent_current_user = User.create_or_update( + agent_current_user = User.create!( login: 'notification-template-current_user@example.com', firstname: 'Notification Current', lastname: 'User', diff --git a/test/unit/notification_factory_mailer_test.rb b/test/unit/notification_factory_mailer_test.rb index 57b4d0dd4..12ef3d318 100644 --- a/test/unit/notification_factory_mailer_test.rb +++ b/test/unit/notification_factory_mailer_test.rb @@ -83,7 +83,7 @@ class NotificationFactoryMailerTest < ActiveSupport::TestCase groups = Group.all roles = Role.where(name: 'Agent') - agent1 = User.create_or_update( + agent1 = User.create!( login: 'notification-settings-agent1@example.com', firstname: 'Notificationxxx', lastname: 'Agent1', @@ -96,7 +96,7 @@ class NotificationFactoryMailerTest < ActiveSupport::TestCase created_by_id: 1, ) - agent2 = User.create_or_update( + agent2 = User.create!( login: 'notification-settings-agent2@example.com', firstname: 'Notificationxxx', lastname: 'Agent2', @@ -109,7 +109,7 @@ class NotificationFactoryMailerTest < ActiveSupport::TestCase created_by_id: 1, ) - group_notification_setting = Group.create_or_update( + group_notification_setting = Group.create!( name: 'NotificationSetting', updated_by_id: 1, created_by_id: 1,