From 60121a25fb186a0a3f35e078625cca4ef6902d8b Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Mon, 27 Apr 2015 22:49:17 +0200 Subject: [PATCH] Some new rubocop checks. --- app/controllers/application_controller.rb | 2 +- app/controllers/getting_started_controller.rb | 51 ++++++++------- app/controllers/long_polling_controller.rb | 12 ++-- app/controllers/tags_controller.rb | 4 +- app/controllers/taskbar_controller.rb | 3 +- app/controllers/tickets_controller.rb | 10 +-- app/controllers/users_controller.rb | 6 +- app/models/user/permission.rb | 29 ++++----- app/models/user/search.rb | 63 ++++++++++--------- app/models/user/search_index.rb | 22 ++++--- test/browser_test_helper.rb | 52 ++++++--------- test/integration_test_helper.rb | 34 +++++----- test/test_helper.rb | 54 ++++++++-------- 13 files changed, 167 insertions(+), 175 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9022ec280..45597b390 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -171,7 +171,7 @@ class ApplicationController < ActionController::Base } end - return { + { auth: true } end diff --git a/app/controllers/getting_started_controller.rb b/app/controllers/getting_started_controller.rb index e1b840b97..f19d7ccb4 100644 --- a/app/controllers/getting_started_controller.rb +++ b/app/controllers/getting_started_controller.rb @@ -173,7 +173,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} end # check domain based attributes - providerMap = { + provider_map = { google: { domain: 'gmail.com|googlemail.com|gmail.de', inbound: { @@ -231,7 +231,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} if mail_exchangers && mail_exchangers[0] && mail_exchangers[0][0] domains.push mail_exchangers[0][0] end - providerMap.each {|provider, settings| + provider_map.each {|provider, settings| domains.each {|domain_to_check| if domain_to_check =~ /#{settings[:domain]}/i @@ -259,9 +259,9 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} } # probe inbound - inboundMap = [] + inbound_map = [] if mail_exchangers && mail_exchangers[0] && mail_exchangers[0][0] - inboundMx = [ + inbound_mx = [ { adapter: 'imap', options: { @@ -283,9 +283,9 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} }, }, ] - inboundMap = inboundMap + inboundMx + inbound_map = inbound_map + inbound_mx end - inboundAuto = [ + inbound_auto = [ { adapter: 'imap', options: { @@ -387,10 +387,10 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} }, }, ] - inboundMap = inboundMap + inboundAuto + inbound_map = inbound_map + inbound_auto settings = {} success = false - inboundMap.each {|config| + inbound_map.each {|config| logger.info "INBOUND PROBE: #{config.inspect}" result = email_probe_inbound( config ) logger.info "INBOUND RESULT: #{result.inspect}" @@ -409,9 +409,9 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} end # probe outbound - outboundMap = [] + outbound_map = [] if mail_exchangers && mail_exchangers[0] && mail_exchangers[0][0] - outboundMx = [ + outbound_mx = [ { adapter: 'smtp', options: { @@ -453,9 +453,9 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} }, }, ] - outboundMap = outboundMap + outboundMx + outbound_map = outbound_map + outbound_mx end - outboundAuto = [ + outbound_auto = [ { adapter: 'smtp', options: { @@ -539,7 +539,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} ] success = false - outboundMap.each {|config| + outbound_map.each {|config| logger.info "OUTBOUND PROBE: #{config.inspect}" result = email_probe_outbound( config, params[:email] ) logger.info "OUTBOUND RESULT: #{result.inspect}" @@ -599,7 +599,6 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} result = email_probe_inbound( params ) render json: result - return end def email_verify @@ -737,7 +736,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} end # test connection - translationMap = { + translation_map = { 'authentication failed' => 'Authentication failed!', 'Incorrect username' => 'Authentication failed!', 'getaddrinfo: nodename nor servname provided, or not known' => 'Hostname not found!', @@ -747,10 +746,10 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} if params[:adapter] =~ /^smtp$/i # in case, fill missing params - if !params[:options].has_key?(:port) + if !params[:options].key?(:port) params[:options][:port] = 25 end - if !params[:options].has_key?(:ssl) + if !params[:options].key?(:ssl) params[:options][:ssl] = true end @@ -765,10 +764,10 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} # check if sending email was ok, but mailserver rejected if !subject - whiteMap = { + white_map = { 'Recipient address rejected' => true, } - whiteMap.each {|key, message| + white_map.each {|key, message| if e.message =~ /#{Regexp.escape(key)}/i result = { result: 'ok', @@ -780,7 +779,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} } end message_human = '' - translationMap.each {|key, message| + translation_map.each {|key, message| if e.message =~ /#{Regexp.escape(key)}/i message_human = message end @@ -806,7 +805,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} ) rescue Exception => e message_human = '' - translationMap.each {|key, message| + translation_map.each {|key, message| if e.message =~ /#{Regexp.escape(key)}/i message_human = message end @@ -822,7 +821,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} result = { result: 'ok', } - return result + result end def email_probe_inbound(params) @@ -833,7 +832,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} end # connection test - translationMap = { + translation_map = { 'authentication failed' => 'Authentication failed!', 'Incorrect username' => 'Authentication failed!', 'getaddrinfo: nodename nor servname provided, or not known' => 'Hostname not found!', @@ -846,7 +845,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} Channel::IMAP.new.fetch( { options: params[:options] }, 'check' ) rescue Exception => e message_human = '' - translationMap.each {|key, message| + translation_map.each {|key, message| if e.message =~ /#{Regexp.escape(key)}/i message_human = message end @@ -869,7 +868,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} Channel::POP3.new.fetch( { options: params[:options] }, 'check' ) rescue Exception => e message_human = '' - translationMap.each {|key, message| + translation_map.each {|key, message| if e.message =~ /#{Regexp.escape(key)}/i message_human = message end @@ -885,7 +884,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} result = { result: 'ok', } - return result + result end def mxers(domain) diff --git a/app/controllers/long_polling_controller.rb b/app/controllers/long_polling_controller.rb index e6ab7838b..3ac933f38 100644 --- a/app/controllers/long_polling_controller.rb +++ b/app/controllers/long_polling_controller.rb @@ -29,7 +29,7 @@ class LongPollingController < ApplicationController # error handling if params['data']['timestamp'] - log 'notice', "request spool data > '#{Time.at( params['data']['timestamp'] ).to_s}'", client_id + log 'notice', "request spool data > '#{Time.at( params['data']['timestamp'] )}'", client_id else log 'notice', 'request spool init data', client_id end @@ -75,13 +75,13 @@ class LongPollingController < ApplicationController if params['data']['recipient'] && params['data']['recipient']['user_id'] params['data']['recipient']['user_id'].each { |user_id| if local_client[:user]['id'].to_s == user_id.to_s - log 'notice', "send broadcast from (#{client_id.to_s}) to (user_id #{user_id})", local_client_id + log 'notice', "send broadcast from (#{client_id}) to (user_id #{user_id})", local_client_id Sessions.send( local_client_id, params['data'] ) end } # broadcast every client else - log 'notice', "send broadcast from (#{client_id.to_s})", local_client_id + log 'notice', "send broadcast from (#{client_id})", local_client_id Sessions.send( local_client_id, params['data'] ) end else @@ -124,7 +124,7 @@ class LongPollingController < ApplicationController count = count - 1 queue = Sessions.queue( client_id ) if queue && queue[0] - # puts "send " + queue.inspect + client_id.to_s + # puts "send " + queue.inspect + client_id.to_s render json: queue return end @@ -155,14 +155,14 @@ class LongPollingController < ApplicationController return if !params[:client_id] sessions = Sessions.sessions return if !sessions.include?( params[:client_id].to_s ) - return params[:client_id].to_s + params[:client_id].to_s end def log( level, data, client_id = '-' ) if false return if level == 'debug' end - puts "#{Time.now}:client(#{ client_id }) #{ data }" + puts "#{Time.zone.now}:client(#{ client_id }) #{ data }" # puts "#{Time.now}:#{ level }:client(#{ client_id }) #{ data }" end end diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 16ba7dd72..da7015341 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -32,7 +32,7 @@ class TagsController < ApplicationController object: params[:object], o_id: params[:o_id], item: params[:item], - ); + ) if success render json: success, status: :created else @@ -46,7 +46,7 @@ class TagsController < ApplicationController object: params[:object], o_id: params[:o_id], item: params[:item], - ); + ) if success render json: success, status: :created else diff --git a/app/controllers/taskbar_controller.rb b/app/controllers/taskbar_controller.rb index 49da083c7..db5b8b9ff 100644 --- a/app/controllers/taskbar_controller.rb +++ b/app/controllers/taskbar_controller.rb @@ -38,11 +38,12 @@ class TaskbarController < ApplicationController end private + def access(taskbar) if taskbar.user_id != current_user.id render json: { error: 'Not allowed to access this task.' }, status: :unprocessable_entity return false end - return true + true end end diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index f92ce43cd..509e177c5 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -360,7 +360,7 @@ class TicketsController < ApplicationController limit = 100 assets = {} access_condition = Ticket.access_condition( current_user ) - now = DateTime.now + now = Time.zone.now user_tickets_open_ids = [] user_tickets_closed_ids = [] user_ticket_volume_by_year = [] @@ -395,8 +395,8 @@ class TicketsController < ApplicationController # generate stats by user (0..11).each {|month_back| - date_to_check = DateTime.now - month_back.month - date_start = "#{date_to_check.year}-#{date_to_check.month}-#{01} 00:00:00" + date_to_check = Time.zone.now - month_back.month + date_start = "#{date_to_check.year}-#{date_to_check.month}-01 00:00:00" date_end = "#{date_to_check.year}-#{date_to_check.month}-#{date_to_check.end_of_month.day} 00:00:00" condition = { @@ -463,7 +463,7 @@ class TicketsController < ApplicationController # generate stats by org (0..11).each {|month_back| date_to_check = DateTime.now - month_back.month - date_start = "#{date_to_check.year}-#{date_to_check.month}-#{01} 00:00:00" + date_start = "#{date_to_check.year}-#{date_to_check.month}-01 00:00:00" date_end = "#{date_to_check.year}-#{date_to_check.month}-#{date_to_check.end_of_month.day} 00:00:00" condition = { @@ -507,7 +507,7 @@ class TicketsController < ApplicationController ticket_ids.push ticket.id assets = ticket.assets(assets) end - return ticket_ids + ticket_ids end def article_create(ticket, params) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 463786880..23756e83e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -644,7 +644,7 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content content: file_resize[:content], mime_type: file_resize[:mime_type], }, - source: 'upload ' + Time.now.to_s, + source: 'upload ' + Time.zone.now.to_s, deletable: true, ) @@ -719,7 +719,7 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content return true if is_role('Agent') response_access_deny - return false + false end def permission_check @@ -730,7 +730,7 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content return true if is_role(Z_ROLENAME_CUSTOMER) && params[:id].to_i == current_user.id response_access_deny - return false + false end end diff --git a/app/models/user/permission.rb b/app/models/user/permission.rb index afac37a24..6b94eee2b 100644 --- a/app/models/user/permission.rb +++ b/app/models/user/permission.rb @@ -1,6 +1,7 @@ # Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ -module User::Permission +module User + module Permission =begin @@ -15,22 +16,22 @@ returns =end - def permission (data) + def permission (data) - # check customer - if data[:current_user].is_role(Z_ROLENAME_CUSTOMER) + # check customer + if data[:current_user].is_role(Z_ROLENAME_CUSTOMER) - # access ok if its own user - return true if self.id == data[:current_user].id + # access ok if its own user + return true if self.id == data[:current_user].id - # no access - return false + # no access + return false + end + + # check agent + return true if data[:current_user].is_role(Z_ROLENAME_ADMIN) + return true if data[:current_user].is_role('Agent') + false end - - # check agent - return true if data[:current_user].is_role(Z_ROLENAME_ADMIN) - return true if data[:current_user].is_role('Agent') - false end - end diff --git a/app/models/user/search.rb b/app/models/user/search.rb index 3f1fb4144..8e0eead5f 100644 --- a/app/models/user/search.rb +++ b/app/models/user/search.rb @@ -1,6 +1,7 @@ # Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ -module User::Search +module User + module Search =begin @@ -18,39 +19,39 @@ returns =end - def search(params) + def search(params) - # get params - query = params[:query] - limit = params[:limit] || 10 - current_user = params[:current_user] + # get params + query = params[:query] + limit = params[:limit] || 10 + current_user = params[:current_user] - # enable search only for agents and admins - return [] if !current_user.is_role('Agent') && !current_user.is_role(Z_ROLENAME_ADMIN) + # enable search only for agents and admins + return [] if !current_user.is_role('Agent') && !current_user.is_role(Z_ROLENAME_ADMIN) - # try search index backend - if SearchIndexBackend.enabled? - items = SearchIndexBackend.search( query, limit, 'User' ) - users = [] - items.each { |item| - users.push User.lookup( id: item[:id] ) - } - return users + # try search index backend + if SearchIndexBackend.enabled? + items = SearchIndexBackend.search( query, limit, 'User' ) + users = [] + items.each { |item| + users.push User.lookup( id: item[:id] ) + } + return users + end + + # fallback do sql query + # - stip out * we already search for *query* - + query.gsub! '*', '' + if params[:role_ids] + users = User.joins(:roles).where( 'roles.id' => params[:role_ids] ).where( + '(users.firstname LIKE ? or users.lastname LIKE ? or users.email LIKE ?) AND users.id != 1', "%#{query}%", "%#{query}%", "%#{query}%", + ).order('firstname').limit(limit) + else + users = User.where( + '(firstname LIKE ? or lastname LIKE ? or email LIKE ?) AND id != 1', "%#{query}%", "%#{query}%", "%#{query}%", + ).order('firstname').limit(limit) + end + users end - - # fallback do sql query - # - stip out * we already search for *query* - - query.gsub! '*', '' - if params[:role_ids] - users = User.joins(:roles).where( 'roles.id' => params[:role_ids] ).where( - '(users.firstname LIKE ? or users.lastname LIKE ? or users.email LIKE ?) AND users.id != 1', "%#{query}%", "%#{query}%", "%#{query}%", - ).order('firstname').limit(limit) - else - users = User.where( - '(firstname LIKE ? or lastname LIKE ? or email LIKE ?) AND id != 1', "%#{query}%", "%#{query}%", "%#{query}%", - ).order('firstname').limit(limit) - end - users end - end diff --git a/app/models/user/search_index.rb b/app/models/user/search_index.rb index 0d6b4df04..f4748a70e 100644 --- a/app/models/user/search_index.rb +++ b/app/models/user/search_index.rb @@ -1,6 +1,7 @@ # Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ -module User::SearchIndex +module User + module SearchIndex =begin @@ -15,14 +16,15 @@ returns =end - def search_index_data - attributes = { 'fullname' => "#{ self['firstname'] } #{ self['lastname'] }" } - ['login', 'firstname', 'lastname', 'phone', 'email', 'city', 'country', 'note', 'created_at'].each { |key| - if self[key] && (!self.respond_to?('empty?') || !self[key].empty?) - attributes[key] = self[key] - end - } - return if attributes.empty? - attributes + def search_index_data + attributes = { 'fullname' => "#{ self['firstname'] } #{ self['lastname'] }" } + ['login', 'firstname', 'lastname', 'phone', 'email', 'city', 'country', 'note', 'created_at'].each { |key| + if self[key] && (!self.respond_to?('empty?') || !self[key].empty?) + attributes[key] = self[key] + end + } + return if attributes.empty? + attributes + end end end diff --git a/test/browser_test_helper.rb b/test/browser_test_helper.rb index 0890a8f34..e0c707458 100644 --- a/test/browser_test_helper.rb +++ b/test/browser_test_helper.rb @@ -220,9 +220,7 @@ class TestCase < Test::Unit::TestCase else instance.find_elements( { partial_link_text: params[:text] } )[0].click end - if !params[:fast] - sleep 0.4 - end + sleep 0.4 if !params[:fast] end =begin @@ -357,9 +355,7 @@ class TestCase < Test::Unit::TestCase element = instance.find_elements( { css: params[:css] } )[0] checked = element.attribute('checked') - if !checked - element.click - end + element.click if !checked end =begin @@ -378,9 +374,7 @@ class TestCase < Test::Unit::TestCase element = instance.find_elements( { css: params[:css] } )[0] checked = element.attribute('checked') - if checked - element.click - end + element.click if checked end =begin @@ -760,8 +754,6 @@ class TestCase < Test::Unit::TestCase sleep 0.5 return true end - rescue - # just try again end end sleep 0.5 @@ -816,8 +808,6 @@ wait untill text in selector disabppears sleep 1 return true end - rescue - # just try again end end sleep 1 @@ -839,7 +829,7 @@ wait untill text in selector disabppears instance = params[:browser] || @browser - for i in 1..100 + (1..100).each do sleep 1 begin if instance.find_elements( { css: '.navigation .tasks .task:first-child' } )[0] @@ -860,8 +850,6 @@ wait untill text in selector disabppears else break end - rescue - # just try again end end sleep 1 @@ -1047,7 +1035,7 @@ wait untill text in selector disabppears sleep 2.5 id = instance.current_url id.gsub!(//, ) - id.gsub!(/^.+?\/(\d+)$/, '\\1') + id.gsub!(%r{^.+?/(\d+)$}, '\\1') element = instance.find_elements( { css: '.active .page-header .ticket-number' } )[0] if element @@ -1100,18 +1088,18 @@ wait untill text in selector disabppears instance.execute_script( '$(".content.active .page-header .ticket-title-update").text("' + data[:title] + '")' ) instance.execute_script( '$(".content.active .page-header .ticket-title-update").blur()' ) instance.execute_script( '$(".content.active .page-header .ticket-title-update").trigger("blur")' ) -# { -# :where => :instance2, -# :execute => 'sendkey', -# :css => '.content.active .page-header .ticket-title-update', -# :value => 'TTT', -# }, -# { -# :where => :instance2, -# :execute => 'sendkey', -# :css => '.content.active .page-header .ticket-title-update', -# :value => :tab, -# }, + # { + # :where => :instance2, + # :execute => 'sendkey', + # :css => '.content.active .page-header .ticket-title-update', + # :value => 'TTT', + # }, + # { + # :where => :instance2, + # :execute => 'sendkey', + # :css => '.content.active .page-header .ticket-title-update', + # :value => :tab, + # }, end if data[:customer] @@ -1203,8 +1191,6 @@ wait untill text in selector disabppears if text =~ /(Discard your unsaved changes.|Verwerfen der)/ found = true end - rescue - # just try again end sleep 1 end @@ -1227,8 +1213,6 @@ wait untill text in selector disabppears if !text || text.empty? return true end - rescue - # just try again end sleep 1 } @@ -1374,7 +1358,7 @@ wait untill text in selector disabppears overviews = {} instance.find_elements( { css: '.content.active .sidebar a[href]' } ).each {|element| url = element.attribute('href') - url.gsub!(/(http|https):\/\/.+?\/(.+?)$/, '\\2') + url.gsub!(%r{(http|https)://.+?/(.+?)$}, '\\2') overviews[url] = 0 #puts url.inspect #puts element.inspect diff --git a/test/integration_test_helper.rb b/test/integration_test_helper.rb index 0475463fe..a19e0d349 100644 --- a/test/integration_test_helper.rb +++ b/test/integration_test_helper.rb @@ -3,25 +3,27 @@ require File.expand_path('../../config/environment', __FILE__) require 'rails/test_help' require 'cache' -class ActiveSupport::TestCase - # disable transactions - #self.use_transactional_fixtures = false - - # clear cache - Cache.clear - - # load seeds - load "#{Rails.root}/db/seeds.rb" - - setup do +module ActiveSupport + class TestCase + # disable transactions + #self.use_transactional_fixtures = false # clear cache Cache.clear - # set current user - puts 'reset UserInfo.current_user_id' - UserInfo.current_user_id = nil - end + # load seeds + load "#{Rails.root}/db/seeds.rb" - # Add more helper methods to be used by all tests here... + setup do + + # clear cache + Cache.clear + + # set current user + puts 'reset UserInfo.current_user_id' + UserInfo.current_user_id = nil + end + + # Add more helper methods to be used by all tests here... + end end diff --git a/test/test_helper.rb b/test/test_helper.rb index b674375e8..66a79882a 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -5,36 +5,38 @@ require 'cache' require 'simplecov' require 'simplecov-rcov' -class ActiveSupport::TestCase - # Setup all fixtures in test/fixtures/*.(yml|csv) for all tests in alphabetical order. - # - # Note: You'll currently still have to declare fixtures explicitly in integration tests - # -- they do not yet inherit this setting - SimpleCov.formatter = SimpleCov::Formatter::RcovFormatter - SimpleCov.start - fixtures :all +module ActiveSupport + class TestCase + # Setup all fixtures in test/fixtures/*.(yml|csv) for all tests in alphabetical order. + # + # Note: You'll currently still have to declare fixtures explicitly in integration tests + # -- they do not yet inherit this setting + SimpleCov.formatter = SimpleCov::Formatter::RcovFormatter + SimpleCov.start + fixtures :all - # disable transactions - self.use_transactional_fixtures = false - - # clear cache - Cache.clear - - # load seeds - load "#{Rails.root}/db/seeds.rb" - - # set system mode to done / to activate - Setting.set('system_init_done', true) - - setup do + # disable transactions + self.use_transactional_fixtures = false # clear cache Cache.clear - # set current user - puts 'reset UserInfo.current_user_id' - UserInfo.current_user_id = nil - end + # load seeds + load "#{Rails.root}/db/seeds.rb" - # Add more helper methods to be used by all tests here... + # set system mode to done / to activate + Setting.set('system_init_done', true) + + setup do + + # clear cache + Cache.clear + + # set current user + puts 'reset UserInfo.current_user_id' + UserInfo.current_user_id = nil + end + + # Add more helper methods to be used by all tests here... + end end