From d93d4049025be2f5a544c3894d257f12a14285f3 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sat, 17 Jun 2017 00:53:20 +0200 Subject: [PATCH] Improved tests (do not break on before/after callbacks). --- .../application_model/can_latest_change.rb | 4 +- .../checks_attribute_length.rb | 1 + app/models/application_model/checks_import.rb | 1 + .../checks_user_columns_fillup.rb | 10 ++- .../application_model/has_recent_views.rb | 1 + app/models/calendar.rb | 7 +- .../concerns/checks_condition_validation.rb | 3 +- app/models/concerns/checks_html_sanitized.rb | 3 +- .../concerns/checks_latest_change_observed.rb | 2 + .../concerns/has_activity_stream_log.rb | 9 +- app/models/concerns/has_karma_activity_log.rb | 1 + app/models/concerns/has_links.rb | 1 + .../concerns/has_online_notifications.rb | 1 + .../concerns/has_search_index_backend.rb | 3 + app/models/concerns/has_tags.rb | 1 + app/models/job.rb | 2 + .../ticket/article/fillup_from_email.rb | 15 ++-- .../ticket/article/fillup_from_general.rb | 18 ++-- app/models/observer/ticket/close_time.rb | 4 +- app/models/observer/transaction.rb | 8 +- app/models/observer/user/geo.rb | 2 + app/models/organization.rb | 3 +- app/models/overview.rb | 3 + app/models/role.rb | 8 +- app/models/setting.rb | 10 ++- app/models/tag.rb | 1 + app/models/ticket.rb | 36 ++++---- app/models/ticket/article.rb | 12 ++- app/models/token.rb | 2 +- app/models/translation.rb | 6 +- app/models/user.rb | 65 ++++++++------ lib/sessions/backend/collections/base.rb | 6 +- test/controllers/search_controller_test.rb | 8 +- test/controllers/tickets_controller_test.rb | 2 +- test/unit/session_basic_test.rb | 84 +++++++++++-------- test/unit/session_basic_ticket_test.rb | 7 +- test/unit/session_collections_test.rb | 17 ++-- test/unit/session_enhanced_test.rb | 16 ++-- 38 files changed, 218 insertions(+), 165 deletions(-) diff --git a/app/models/application_model/can_latest_change.rb b/app/models/application_model/can_latest_change.rb index ad69dcc48..caaa6456e 100644 --- a/app/models/application_model/can_latest_change.rb +++ b/app/models/application_model/can_latest_change.rb @@ -23,7 +23,7 @@ returns # if we do not have it cached, do lookup if !updated_at - o = select(:updated_at).order(updated_at: :desc).limit(1).first + o = select(:updated_at).order(updated_at: :desc, id: :desc).limit(1).first if o updated_at = o.updated_at latest_change_set(updated_at) @@ -34,7 +34,7 @@ returns def latest_change_set(updated_at) key = "#{new.class.name}_latest_change" - expires_in = 31_536_000 # 1 year + expires_in = 86_400 # 1 day if updated_at.nil? Cache.delete(key) diff --git a/app/models/application_model/checks_attribute_length.rb b/app/models/application_model/checks_attribute_length.rb index 5e49f4fe7..5254b035c 100644 --- a/app/models/application_model/checks_attribute_length.rb +++ b/app/models/application_model/checks_attribute_length.rb @@ -34,5 +34,6 @@ check string/varchar size and cut them if needed self[attribute[0]] = self[ attribute[0] ].utf8_to_3bytesutf8 end } + true end end diff --git a/app/models/application_model/checks_import.rb b/app/models/application_model/checks_import.rb index 82e7e6f9a..feb1def4f 100644 --- a/app/models/application_model/checks_import.rb +++ b/app/models/application_model/checks_import.rb @@ -15,5 +15,6 @@ module ApplicationModel::ChecksImport return if Setting.get('import_mode') && import_class_list.include?(self.class.to_s) return if !has_attribute?(:id) self[:id] = nil + true end end diff --git a/app/models/application_model/checks_user_columns_fillup.rb b/app/models/application_model/checks_user_columns_fillup.rb index b70bf8060..a7b640197 100644 --- a/app/models/application_model/checks_user_columns_fillup.rb +++ b/app/models/application_model/checks_user_columns_fillup.rb @@ -31,14 +31,15 @@ returns end end - return if !self.class.column_names.include? 'created_by_id' + return true if !self.class.column_names.include? 'created_by_id' - return if !UserInfo.current_user_id + return true if !UserInfo.current_user_id if created_by_id && created_by_id != UserInfo.current_user_id logger.info "NOTICE create - self.created_by_id is different: #{created_by_id}/#{UserInfo.current_user_id}" end self.created_by_id = UserInfo.current_user_id + true end =begin @@ -56,9 +57,10 @@ returns =end def fill_up_user_update - return if !self.class.column_names.include? 'updated_by_id' - return if !UserInfo.current_user_id + return true if !self.class.column_names.include? 'updated_by_id' + return true if !UserInfo.current_user_id self.updated_by_id = UserInfo.current_user_id + true end end diff --git a/app/models/application_model/has_recent_views.rb b/app/models/application_model/has_recent_views.rb index b8f493e86..b753a18e8 100644 --- a/app/models/application_model/has_recent_views.rb +++ b/app/models/application_model/has_recent_views.rb @@ -17,5 +17,6 @@ delete object recent viewed list, will be executed automatically def recent_view_destroy RecentView.log_destroy(self.class.to_s, id) + true end end diff --git a/app/models/calendar.rb b/app/models/calendar.rb index 882e19a6b..56fee6a93 100644 --- a/app/models/calendar.rb +++ b/app/models/calendar.rb @@ -240,13 +240,14 @@ returns # if changed calendar is default, set all others default to false def sync_default - return if !default + return true if !default Calendar.find_each { |calendar| next if calendar.id == id next if !calendar.default calendar.default = false calendar.save } + true end # check if min one is set to default true @@ -270,11 +271,13 @@ returns sla.save! end } + true end # fetch ical feed def fetch_ical sync(true) + true end # validate format of public holidays @@ -292,6 +295,6 @@ returns false end } - + true end end diff --git a/app/models/concerns/checks_condition_validation.rb b/app/models/concerns/checks_condition_validation.rb index 945f978c0..7bdee3770 100644 --- a/app/models/concerns/checks_condition_validation.rb +++ b/app/models/concerns/checks_condition_validation.rb @@ -27,8 +27,7 @@ module ChecksConditionValidation } ticket_count, tickets = Ticket.selectors(validate_condition, 1, User.find(1)) - return if ticket_count.present? - + return true if ticket_count.present? raise Exceptions::UnprocessableEntity, 'Invalid ticket selector conditions' end end diff --git a/app/models/concerns/checks_html_sanitized.rb b/app/models/concerns/checks_html_sanitized.rb index f3db21a9a..e89a12fa4 100644 --- a/app/models/concerns/checks_html_sanitized.rb +++ b/app/models/concerns/checks_html_sanitized.rb @@ -9,7 +9,7 @@ module ChecksHtmlSanitized def sanitized_html_attributes html_attributes = self.class.instance_variable_get(:@sanitized_html) || [] - return if html_attributes.empty? + return true if html_attributes.empty? html_attributes.each do |attribute| value = send(attribute) @@ -19,6 +19,7 @@ module ChecksHtmlSanitized send("#{attribute}=".to_sym, HtmlSanitizer.strict(value)) end + true end def sanitizeable?(_attribute, _value) diff --git a/app/models/concerns/checks_latest_change_observed.rb b/app/models/concerns/checks_latest_change_observed.rb index 97b8a37ca..09bad426f 100644 --- a/app/models/concerns/checks_latest_change_observed.rb +++ b/app/models/concerns/checks_latest_change_observed.rb @@ -11,9 +11,11 @@ module ChecksLatestChangeObserved def latest_change_set_from_observer self.class.latest_change_set(updated_at) + true end def latest_change_set_from_observer_destroy self.class.latest_change_set(nil) + true end end diff --git a/app/models/concerns/has_activity_stream_log.rb b/app/models/concerns/has_activity_stream_log.rb index 8af68e63a..d921945ea 100644 --- a/app/models/concerns/has_activity_stream_log.rb +++ b/app/models/concerns/has_activity_stream_log.rb @@ -19,6 +19,7 @@ log object create activity stream, if configured - will be executed automaticall def activity_stream_create activity_stream_log('create', self['created_by_id']) + true end =begin @@ -31,7 +32,7 @@ log object update activity stream, if configured - will be executed automaticall =end def activity_stream_update - return if !changed? + return true if !changed? ignored_attributes = self.class.instance_variable_get(:@activity_stream_attributes_ignored) || [] ignored_attributes += %i(created_at updated_at created_by_id updated_by_id) @@ -42,10 +43,9 @@ log object update activity stream, if configured - will be executed automaticall log = true } - - return if !log - + return true if !log activity_stream_log('update', self['updated_by_id']) + true end =begin @@ -59,6 +59,7 @@ delete object activity stream, will be executed automatically def activity_stream_destroy ActivityStream.remove(self.class.to_s, id) + true end # methods defined here are going to extend the class, not the instance of it diff --git a/app/models/concerns/has_karma_activity_log.rb b/app/models/concerns/has_karma_activity_log.rb index 70768a309..6c7f61ceb 100644 --- a/app/models/concerns/has_karma_activity_log.rb +++ b/app/models/concerns/has_karma_activity_log.rb @@ -17,5 +17,6 @@ delete object online notification list, will be executed automatically def karma_activity_log_destroy Karma::ActivityLog.remove(self.class.to_s, id) + true end end diff --git a/app/models/concerns/has_links.rb b/app/models/concerns/has_links.rb index 4a8e0dc5f..088fa104f 100644 --- a/app/models/concerns/has_links.rb +++ b/app/models/concerns/has_links.rb @@ -20,5 +20,6 @@ delete object link list, will be executed automatically link_object: self.class.to_s, link_object_value: id, ) + true end end diff --git a/app/models/concerns/has_online_notifications.rb b/app/models/concerns/has_online_notifications.rb index cb2ee529f..a6fd9653e 100644 --- a/app/models/concerns/has_online_notifications.rb +++ b/app/models/concerns/has_online_notifications.rb @@ -17,5 +17,6 @@ delete object online notification list, will be executed automatically def online_notification_destroy OnlineNotification.remove(self.class.to_s, id) + true end end diff --git a/app/models/concerns/has_search_index_backend.rb b/app/models/concerns/has_search_index_backend.rb index 42b14a822..8ab494ce2 100644 --- a/app/models/concerns/has_search_index_backend.rb +++ b/app/models/concerns/has_search_index_backend.rb @@ -24,6 +24,7 @@ update search index, if configured - will be executed automatically # start background job to transfer data to search index return if !SearchIndexBackend.enabled? Delayed::Job.enqueue(BackgroundJobSearchIndex.new(self.class.to_s, id)) + true end =begin @@ -38,6 +39,7 @@ delete search index object, will be executed automatically def search_index_destroy return if ignore_search_indexing?(:destroy) SearchIndexBackend.remove(self.class.to_s, id) + true end =begin @@ -60,6 +62,7 @@ returns # update backend SearchIndexBackend.add(self.class.to_s, attributes) + true end =begin diff --git a/app/models/concerns/has_tags.rb b/app/models/concerns/has_tags.rb index 5dbbd5fae..3575d832b 100644 --- a/app/models/concerns/has_tags.rb +++ b/app/models/concerns/has_tags.rb @@ -73,6 +73,7 @@ destroy all tags of an object o_id: id, created_by_id: current_user_id, ) + true end end diff --git a/app/models/job.rb b/app/models/job.rb index 332a58944..03f06eecf 100644 --- a/app/models/job.rb +++ b/app/models/job.rb @@ -203,10 +203,12 @@ class Job < ApplicationModel def updated_matching self.matching = matching_count + true end def update_next_run_at self.next_run_at = next_run_at_calculate + true end def match_minutes(minutes) diff --git a/app/models/observer/ticket/article/fillup_from_email.rb b/app/models/observer/ticket/article/fillup_from_email.rb index abc376395..1fc206ecd 100644 --- a/app/models/observer/ticket/article/fillup_from_email.rb +++ b/app/models/observer/ticket/article/fillup_from_email.rb @@ -6,22 +6,22 @@ class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer def before_create(record) # return if we run import mode - return if Setting.get('import_mode') + return true if Setting.get('import_mode') # only do fill of email from if article got created via application_server (e. g. not # if article and sender type is set via *.postmaster) - return if ApplicationHandleInfo.current.split('.')[1] == 'postmaster' + return true if ApplicationHandleInfo.current.split('.')[1] == 'postmaster' # if sender is customer, do not change anything - return if !record.sender_id + return true if !record.sender_id sender = Ticket::Article::Sender.lookup(id: record.sender_id) - return if sender.nil? - return if sender['name'] == 'Customer' + return true if sender.nil? + return true if sender['name'] == 'Customer' # set email attributes - return if !record.type_id + return true if !record.type_id type = Ticket::Article::Type.lookup(id: record.type_id) - return if type['name'] != 'email' + return true if type['name'] != 'email' # set subject if empty ticket = Ticket.lookup(id: record.ticket_id) @@ -59,5 +59,6 @@ class Observer::Ticket::Article::FillupFromEmail < ActiveRecord::Observer else record.from = Channel::EmailBuild.recipient_line(email_address.realname, email_address.email) end + true end end diff --git a/app/models/observer/ticket/article/fillup_from_general.rb b/app/models/observer/ticket/article/fillup_from_general.rb index 8fbb9504d..7c6ce9166 100644 --- a/app/models/observer/ticket/article/fillup_from_general.rb +++ b/app/models/observer/ticket/article/fillup_from_general.rb @@ -6,24 +6,24 @@ class Observer::Ticket::Article::FillupFromGeneral < ActiveRecord::Observer def before_create(record) # return if we run import mode - return if Setting.get('import_mode') + return true if Setting.get('import_mode') # only do fill of from if article got created via application_server (e. g. not # if article and sender type is set via *.postmaster) - return if ApplicationHandleInfo.current.split('.')[1] == 'postmaster' + return true if ApplicationHandleInfo.current.split('.')[1] == 'postmaster' # set from on all article types excluding email|twitter status|twitter direct-message|facebook feed post|facebook feed comment - return if !record.type_id + return true if !record.type_id type = Ticket::Article::Type.lookup(id: record.type_id) - return if type['name'] == 'email' + return true if type['name'] == 'email' # from will be set by channel backend - return if type['name'] == 'twitter status' - return if type['name'] == 'twitter direct-message' - return if type['name'] == 'facebook feed post' - return if type['name'] == 'facebook feed comment' + return true if type['name'] == 'twitter status' + return true if type['name'] == 'twitter direct-message' + return true if type['name'] == 'facebook feed post' + return true if type['name'] == 'facebook feed comment' - return if !record.created_by_id + return true if !record.created_by_id user = User.find(record.created_by_id) if type.name == 'web' record.from = "#{user.firstname} #{user.lastname} <#{user.email}>" diff --git a/app/models/observer/ticket/close_time.rb b/app/models/observer/ticket/close_time.rb index bcedea1fd..afb1ce845 100644 --- a/app/models/observer/ticket/close_time.rb +++ b/app/models/observer/ticket/close_time.rb @@ -16,13 +16,13 @@ class Observer::Ticket::CloseTime < ActiveRecord::Observer def _check(record) # return if we run import mode - return if Setting.get('import_mode') + return true if Setting.get('import_mode') # check if close_at is already set return true if record.close_at # check if ticket is closed now - return if !record.state_id + return true if !record.state_id state = Ticket::State.lookup(id: record.state_id) state_type = Ticket::StateType.lookup(id: state.state_type_id) return true if state_type.name != 'closed' diff --git a/app/models/observer/transaction.rb b/app/models/observer/transaction.rb index be7092504..e20e5965d 100644 --- a/app/models/observer/transaction.rb +++ b/app/models/observer/transaction.rb @@ -176,7 +176,7 @@ class Observer::Transaction < ActiveRecord::Observer def after_create(record) # return if we run import mode - return if Setting.get('import_mode') + return true if Setting.get('import_mode') e = { object: record.class.name, @@ -187,12 +187,13 @@ class Observer::Transaction < ActiveRecord::Observer created_at: Time.zone.now, } EventBuffer.add('transaction', e) + true end def before_update(record) # return if we run import mode - return if Setting.get('import_mode') + return true if Setting.get('import_mode') # ignore certain attributes real_changes = {} @@ -210,7 +211,7 @@ class Observer::Transaction < ActiveRecord::Observer } # do not send anything if nothing has changed - return if real_changes.empty? + return true if real_changes.empty? changed_by_id = nil changed_by_id = if record.respond_to?('updated_by_id') @@ -229,6 +230,7 @@ class Observer::Transaction < ActiveRecord::Observer created_at: Time.zone.now, } EventBuffer.add('transaction', e) + true end end diff --git a/app/models/observer/user/geo.rb b/app/models/observer/user/geo.rb index 9d55129c2..f78327f10 100644 --- a/app/models/observer/user/geo.rb +++ b/app/models/observer/user/geo.rb @@ -5,10 +5,12 @@ class Observer::User::Geo < ActiveRecord::Observer def before_create(record) check_geo(record) + true end def before_update(record) check_geo(record) + true end # check if geo need to be updated diff --git a/app/models/organization.rb b/app/models/organization.rb index a68b8505e..967e72558 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -25,11 +25,12 @@ class Organization < ApplicationModel private def domain_cleanup - return if domain.blank? + return true if domain.blank? domain.gsub!(/@/, '') domain.gsub!(/\s*/, '') domain.strip! domain.downcase! + true end end diff --git a/app/models/overview.rb b/app/models/overview.rb index aa1213b4b..1abc9f2be 100644 --- a/app/models/overview.rb +++ b/app/models/overview.rb @@ -24,17 +24,20 @@ class Overview < ApplicationModel def fill_prio return true if prio self.prio = 9999 + true end def fill_link_on_create return true if !link.empty? self.link = link_name(name) + true end def fill_link_on_update return true if link.empty? return true if !changes['name'] self.link = link_name(name) + true end def link_name(name) diff --git a/app/models/role.rb b/app/models/role.rb index 174ea005d..2a362a4c0 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -125,7 +125,7 @@ returns private def validate_permissions - return if !self.permission_ids + return true if !self.permission_ids permission_ids.each { |permission_id| permission = Permission.lookup(id: permission_id) raise "Unable to find permission for id #{permission_id}" if !permission @@ -137,17 +137,19 @@ returns raise "Permission #{permission.name} conflicts with #{local_permission.name}" if permission_ids.include?(local_permission.id) } } + true end def validate_agent_limit(permission) - return if !Setting.get('system_agent_limit') - return if permission.name != 'ticket.agent' + return true if !Setting.get('system_agent_limit') + return true if permission.name != 'ticket.agent' ticket_agent_role_ids = Role.joins(:permissions).where(permissions: { name: 'ticket.agent' }).pluck(:id) ticket_agent_role_ids.push(id) count = User.joins(:roles).where(roles: { id: ticket_agent_role_ids }, users: { active: true }).count raise Exceptions::UnprocessableEntity, 'Agent limit exceeded, please check your account settings.' if count > Setting.get('system_agent_limit') + true end end diff --git a/app/models/setting.rb b/app/models/setting.rb index 17ff09e71..3734cb848 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -131,6 +131,7 @@ reload config settings # set initial value in state_initial def set_initial self.state_initial = state_current + true end def reset_change_id @@ -139,6 +140,7 @@ reload config settings logger.debug "Setting.reset_change_id: set new cache, #{change_id}" Cache.write('Setting::ChangeId', change_id, { expires_in: 24.hours }) @@lookup_at = nil # rubocop:disable Style/ClassVars + true end # check if cache is still valid @@ -160,14 +162,15 @@ reload config settings # convert state into hash to be able to store it as store def state_check - return if !state - return if state && state.respond_to?('has_key?') && state.key?(:value) + return true if !state + return true if state && state.respond_to?('has_key?') && state.key?(:value) self.state_current = { value: state } + true end # notify clients about public config changes def check_broadcast - return if frontend != true + return true if frontend != true value = state_current if state_current.key?(:value) value = state_current[:value] @@ -179,5 +182,6 @@ reload config settings }, 'public' ) + true end end diff --git a/app/models/tag.rb b/app/models/tag.rb index 529114905..3c327071e 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -292,6 +292,7 @@ remove tag item (destroy with reverences) def fill_namedowncase self.name_downcase = name.downcase + true end end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index d549e9561..ecb483310 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -1045,42 +1045,42 @@ result private def check_generate - return if number + return true if number self.number = Ticket::Number.generate + true end def check_title - return if !title + return true if !title title.gsub!(/\s|\t|\r/, ' ') + true end def check_defaults if !owner_id self.owner_id = 1 end - - return if !customer_id - + return true if !customer_id customer = User.find_by(id: customer_id) - return if !customer - return if organization_id == customer.organization_id - + return true if !customer + return true if organization_id == customer.organization_id self.organization_id = customer.organization_id + true end def reset_pending_time # ignore if no state has changed - return if !changes['state_id'] + return true if !changes['state_id'] # check if new state isn't pending* current_state = Ticket::State.lookup(id: state_id) current_state_type = Ticket::StateType.lookup(id: current_state.state_type_id) # in case, set pending_time to nil - return if current_state_type.name =~ /^pending/i - + return true if current_state_type.name =~ /^pending/i self.pending_time = nil + true end def check_escalation_update @@ -1089,20 +1089,18 @@ result end def set_default_state - return if state_id - + return true if state_id default_ticket_state = Ticket::State.find_by(default_create: true) - return if !default_ticket_state - + return true if !default_ticket_state self.state_id = default_ticket_state.id + true end def set_default_priority - return if priority_id - + return true if priority_id default_ticket_priority = Ticket::Priority.find_by(default_create: true) - return if !default_ticket_priority - + return true if !default_ticket_priority self.priority_id = default_ticket_priority.id + true end end diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index 546f614fa..8bc557bfe 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -37,8 +37,7 @@ class Ticket::Article < ApplicationModel # fillup md5 of message id to search easier on very long message ids def check_message_id_md5 - return if !message_id - return if message_id_md5 + return true if message_id.blank? self.message_id_md5 = Digest::MD5.hexdigest(message_id.to_s) end @@ -55,7 +54,7 @@ returns =end def self.insert_urls(article) - return article if article['attachments'].empty? + return article if article['attachments'].blank? return article if article['content_type'] !~ %r{text/html}i return article if article['body'] !~ / Setting.get('system_agent_limit') + true end def domain_based_assignment - return if !email - return if organization_id + return true if !email + return true if organization_id begin domain = Mail::Address.new(email).domain - return if !domain + return true if !domain organization = Organization.find_by(domain: domain.downcase, domain_assignment: true) - return if !organization + return true if !organization self.organization_id = organization.id rescue - return + return true end + true end # sets locale of the user def set_locale # set the user's locale to the one of the "executing" user - return if !UserInfo.current_user_id + return true if !UserInfo.current_user_id user = User.find_by(id: UserInfo.current_user_id) - return if !user - return if !user.preferences[:locale] + return true if !user + return true if !user.preferences[:locale] preferences[:locale] = user.preferences[:locale] + true end def avatar_for_email_check - return if email.blank? - return if email !~ /@/ - return if !changes['email'] && updated_at > Time.zone.now - 10.days + return true if email.blank? + return true if email !~ /@/ + return true if !changes['email'] && updated_at > Time.zone.now - 10.days # save/update avatar avatar = Avatar.auto_detection( @@ -974,10 +982,11 @@ raise 'Minimum one user need to have admin permissions' ) # update user link - return if !avatar + return true if !avatar update_column(:image, avatar.store_hash) cache_delete + true end def avatar_destroy @@ -985,9 +994,10 @@ raise 'Minimum one user need to have admin permissions' end def ensure_password - return if password_empty? - return if PasswordHash.crypted?(password) + return true if password_empty? + return true if PasswordHash.crypted?(password) self.password = PasswordHash.crypt(password) + true end def password_empty? @@ -1006,8 +1016,9 @@ raise 'Minimum one user need to have admin permissions' # reset login_failed if password is changed def reset_login_failed - return if !changes - return if !changes['password'] + return true if !changes + return true if !changes['password'] self.login_failed = 0 + true end end diff --git a/lib/sessions/backend/collections/base.rb b/lib/sessions/backend/collections/base.rb index d85c5dd14..32cf96ba3 100644 --- a/lib/sessions/backend/collections/base.rb +++ b/lib/sessions/backend/collections/base.rb @@ -36,13 +36,13 @@ class Sessions::Backend::Collections::Base < Sessions::Backend::Base # check if update has been done last_change = self.class.model.constantize.latest_change - return if last_change == @last_change - @last_change = last_change + return if last_change.to_s == @last_change + @last_change = last_change.to_s # load current data items = load - return if !items || items.empty? + return if items.blank? # get relations of data all = [] diff --git a/test/controllers/search_controller_test.rb b/test/controllers/search_controller_test.rb index dffe4e693..d348240ac 100644 --- a/test/controllers/search_controller_test.rb +++ b/test/controllers/search_controller_test.rb @@ -85,8 +85,6 @@ class SearchControllerTest < ActionDispatch::IntegrationTest organization_id: @organization.id, ) - Ticket.all.destroy_all - @ticket1 = Ticket.create!( title: 'test 1234-1', group: Group.lookup(name: 'Users'), @@ -105,7 +103,7 @@ class SearchControllerTest < ActionDispatch::IntegrationTest sender: Ticket::Article::Sender.where(name: 'Customer').first, type: Ticket::Article::Type.where(name: 'email').first, ) - sleep 1 + travel 1.second @ticket2 = Ticket.create!( title: 'test 1234-2', group: Group.lookup(name: 'Users'), @@ -124,7 +122,7 @@ class SearchControllerTest < ActionDispatch::IntegrationTest sender: Ticket::Article::Sender.where(name: 'Customer').first, type: Ticket::Article::Type.where(name: 'email').first, ) - sleep 1 + travel 1.second @ticket3 = Ticket.create!( title: 'test 1234-2', group: Group.lookup(name: 'Users'), @@ -162,6 +160,8 @@ class SearchControllerTest < ActionDispatch::IntegrationTest Setting.set('es_index', ENV['ES_INDEX']) end + travel 1.minute + # drop/create indexes Rake::Task.clear Zammad::Application.load_tasks diff --git a/test/controllers/tickets_controller_test.rb b/test/controllers/tickets_controller_test.rb index 5fe9c8537..799eb78d4 100644 --- a/test/controllers/tickets_controller_test.rb +++ b/test/controllers/tickets_controller_test.rb @@ -822,7 +822,7 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO created_by_id: 1, ) tickets.push ticket - sleep 1 + travel 2.seconds } credentials = ActionController::HttpAuthentication::Basic.encode_credentials('tickets-admin', 'adminpw') diff --git a/test/unit/session_basic_test.rb b/test/unit/session_basic_test.rb index aa9e7f398..02fd41fc8 100644 --- a/test/unit/session_basic_test.rb +++ b/test/unit/session_basic_test.rb @@ -2,12 +2,6 @@ require 'test_helper' class SessionBasicTest < ActiveSupport::TestCase - setup do - user = User.lookup(id: 1) - roles = Role.where(name: %w(Agent Admin)) - user.roles = roles - user.save! - end test 'b cache' do Sessions::CacheIn.set('last_run_test', true, { expires_in: 1.second }) @@ -59,7 +53,6 @@ class SessionBasicTest < ActiveSupport::TestCase roles = Role.where(name: %w(Agent)) groups = Group.all - UserInfo.current_user_id = 1 agent1 = User.create_or_update( login: 'session-agent-1', firstname: 'Session', @@ -69,9 +62,9 @@ class SessionBasicTest < ActiveSupport::TestCase active: true, roles: roles, groups: groups, + updated_by_id: 1, + created_by_id: 1, ) - agent1.roles = roles - agent1.save # create sessions client_id1 = '123456789' @@ -109,10 +102,25 @@ class SessionBasicTest < ActiveSupport::TestCase test 'c collections group' do require 'sessions/backend/collections/group.rb' - UserInfo.current_user_id = 2 - user = User.lookup(id: 1) - collection_client1 = Sessions::Backend::Collections::Group.new(user, {}, false, '123-1', 3) - collection_client2 = Sessions::Backend::Collections::Group.new(user, {}, false, '234-2', 3) + # create users + roles = Role.where(name: ['Agent']) + groups = Group.all + + agent1 = User.create_or_update( + login: 'session-collection-agent-1', + firstname: 'Session', + lastname: 'Agent 1', + email: 'session-collection-agent1@example.com', + password: 'agentpw', + active: true, + roles: roles, + groups: groups, + updated_by_id: 1, + created_by_id: 1, + ) + + collection_client1 = Sessions::Backend::Collections::Group.new(agent1, {}, false, '123-1', 3) + collection_client2 = Sessions::Backend::Collections::Group.new(agent1, {}, false, '234-2', 3) # get whole collections result1 = collection_client1.push @@ -131,12 +139,14 @@ class SessionBasicTest < ActiveSupport::TestCase # change collection group = Group.first + travel 4.seconds group.touch travel 4.seconds # get whole collections result1 = collection_client1.push assert(!result1.empty?, 'check collections - after touch') + result2 = collection_client2.push assert(!result2.empty?, 'check collections - after touch') assert_equal(result1.to_yaml, result2.to_yaml, 'check collections') @@ -148,9 +158,11 @@ class SessionBasicTest < ActiveSupport::TestCase assert_nil(result2, 'check collections - after touch - recall') # change collection - group = Group.create( - name: "SomeGroup::#{rand(999_999)}", - active: true + group = Group.create!( + name: "SomeGroup::#{rand(999_999)}", + active: true, + created_by_id: 1, + updated_by_id: 1, ) travel 4.seconds @@ -194,7 +206,6 @@ class SessionBasicTest < ActiveSupport::TestCase roles = Role.where(name: %w(Agent Admin)) groups = Group.all - UserInfo.current_user_id = 2 agent1 = User.create_or_update( login: 'activity-stream-agent-1', firstname: 'Session', @@ -204,9 +215,9 @@ class SessionBasicTest < ActiveSupport::TestCase active: true, roles: roles, groups: groups, + updated_by_id: 1, + created_by_id: 1, ) - agent1.roles = roles - assert(agent1.save, 'create/update agent1') # create min. on activity record random_name = "Random:#{rand(9_999_999_999)}" @@ -233,7 +244,15 @@ class SessionBasicTest < ActiveSupport::TestCase assert(!result1, 'check as agent1 - recall 2') agent1.update_attribute(:email, 'activity-stream-agent11@example.com') - ticket = Ticket.create(title: '12323', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) + ticket = Ticket.create( + title: '12323', + group_id: 1, + priority_id: 1, + state_id: 1, + customer_id: 1, + updated_by_id: 1, + created_by_id: 1, + ) travel 4.seconds @@ -246,21 +265,21 @@ class SessionBasicTest < ActiveSupport::TestCase test 'c ticket_create' do # create users - roles = Role.where(name: %w(Agent)) + roles = Role.where(name: %w(Agent Admin)) groups = Group.all - UserInfo.current_user_id = 1 agent1 = User.create_or_update( - login: 'session-agent-1', + login: 'ticket_create-agent-1', firstname: 'Session', - lastname: 'Agent 1', - email: 'session-agent-1@example.com', + lastname: "ticket_create #{rand(99_999)}", + email: 'ticket_create-agent1@example.com', password: 'agentpw', active: true, roles: roles, groups: groups, + updated_by_id: 1, + created_by_id: 1, ) - agent1.save! ticket_create_client1 = Sessions::Backend::TicketCreate.new(agent1, {}, false, '123-1', 3) @@ -278,19 +297,18 @@ class SessionBasicTest < ActiveSupport::TestCase result1 = ticket_create_client1.push assert(!result1, 'check ticket_create - recall 2') - group = Group.create(name: "SomeTicketCreateGroup::#{rand(999_999)}", active: true) - agent1.groups = Group.all - agent1.save! - - # next check should be empty - result1 = ticket_create_client1.push + Group.create( + name: "SomeTicketCreateGroup::#{rand(999_999)}", + active: true, + updated_by_id: 1, + created_by_id: 1, + ) travel 4.seconds # get as stream result1 = ticket_create_client1.push assert(result1, 'check ticket_create - recall 3') - travel_back end diff --git a/test/unit/session_basic_ticket_test.rb b/test/unit/session_basic_ticket_test.rb index 072c9e1a4..7cc0d8ddc 100644 --- a/test/unit/session_basic_ticket_test.rb +++ b/test/unit/session_basic_ticket_test.rb @@ -5,9 +5,6 @@ class SessionBasicTicketTest < ActiveSupport::TestCase test 'b ticket_overview_List' do UserInfo.current_user_id = 1 - Ticket.destroy_all - - # create users roles = Role.where(name: ['Agent']) groups = Group.all @@ -21,9 +18,7 @@ class SessionBasicTicketTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - - agent1.roles = roles - assert(agent1.save, 'create/update agent1') + assert(agent1.save!, 'create/update agent1') Ticket.create(title: 'default overview test', group_id: 1, priority_id: 1, state_id: 1, customer_id: 1) diff --git a/test/unit/session_collections_test.rb b/test/unit/session_collections_test.rb index 08b5b61d5..2b5691909 100644 --- a/test/unit/session_collections_test.rb +++ b/test/unit/session_collections_test.rb @@ -22,8 +22,7 @@ class SessionCollectionsTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - agent1.roles = roles - agent1.save + agent1.save! roles = Role.where(name: ['Agent']) groups = Group.all @@ -39,8 +38,7 @@ class SessionCollectionsTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - agent2.roles = roles - agent2.save + agent2.save! roles = Role.where(name: ['Customer']) customer1 = User.create_or_update( @@ -53,9 +51,7 @@ class SessionCollectionsTest < ActiveSupport::TestCase active: true, roles: roles, ) - customer1.roles = roles - customer1.save - + customer1.save! collection_client1 = Sessions::Backend::Collections.new(agent1, {}, nil, 'aaa-1', 2) collection_client2 = Sessions::Backend::Collections.new(agent2, {}, nil, 'bbb-2', 2) collection_client3 = Sessions::Backend::Collections.new(customer1, {}, nil, 'ccc-2', 2) @@ -107,11 +103,13 @@ class SessionCollectionsTest < ActiveSupport::TestCase # change collection group = Group.first + travel 6.seconds group.touch - travel 4.seconds + travel 6.seconds # get whole collections result1 = collection_client1.push + assert(result1, 'check collections - after touch') assert(check_if_collection_exists(result1, :Group), 'check collections - after touch') travel 0.1.seconds @@ -173,7 +171,6 @@ class SessionCollectionsTest < ActiveSupport::TestCase end test 'b assets' do - # create users roles = Role.where(name: %w(Agent Admin)) groups = Group.all.order(id: :asc) @@ -188,7 +185,7 @@ class SessionCollectionsTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - assert(agent1.save, 'create/update agent1') + assert(agent1.save!, 'create/update agent1') assets = {} client1 = Sessions::Backend::Collections::Group.new(agent1, assets, false, '123-1', 4) diff --git a/test/unit/session_enhanced_test.rb b/test/unit/session_enhanced_test.rb index 9ce8c0ece..0c3303003 100644 --- a/test/unit/session_enhanced_test.rb +++ b/test/unit/session_enhanced_test.rb @@ -19,8 +19,7 @@ class SessionEnhancedTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - agent1.roles = roles - agent1.save + agent1.save! agent2 = User.create_or_update( login: 'session-agent-2', firstname: 'Session', @@ -31,8 +30,7 @@ class SessionEnhancedTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - agent2.roles = roles - agent2.save + agent2.save! agent3 = User.create_or_update( login: 'session-agent-3', firstname: 'Session', @@ -43,8 +41,7 @@ class SessionEnhancedTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - agent3.roles = roles - agent3.save + agent3.save! # create sessions client_id1 = 'a1234' @@ -197,7 +194,7 @@ class SessionEnhancedTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - agent1.save + agent1.save! agent2 = User.create_or_update( login: 'session-agent-2', firstname: 'Session', @@ -209,7 +206,7 @@ class SessionEnhancedTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - agent2.save + agent2.save! agent3 = User.create_or_update( login: 'session-agent-3', firstname: 'Session', @@ -221,7 +218,7 @@ class SessionEnhancedTest < ActiveSupport::TestCase roles: roles, groups: groups, ) - agent3.save + agent3.save! # create sessions client_id1_0 = 'b1234-1' @@ -288,6 +285,7 @@ class SessionEnhancedTest < ActiveSupport::TestCase # change collection group = Group.first + travel 4.seconds group.touch travel 12.seconds