Skip to content

Commit

Permalink
Enable Lint/UselessAssignment cop.
Browse files Browse the repository at this point in the history
  • Loading branch information
Ryan Lue authored and thorsteneckel committed Jun 28, 2019
1 parent 7930ffd commit a1da3a2
Show file tree
Hide file tree
Showing 142 changed files with 547 additions and 663 deletions.
4 changes: 0 additions & 4 deletions .rubocop_todo.rspec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Offense count: 43
Lint/UselessAssignment:
Enabled: false

# # Offense count: 17 (Competes with same override in .rubocop_todo.yml)
# Metrics/AbcSize:
# Max: 24
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/application_channel_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def channel
end

def channel_params
params = params.permit!.to_s
params.permit!.to_s
end

end
19 changes: 4 additions & 15 deletions app/controllers/application_controller/renders_models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,22 +101,11 @@ def model_show_render_item(generic_object)
end

def model_index_render(object, params)
offset = 0
per_page = 500
if params[:page] && params[:per_page]
offset = (params[:page].to_i - 1) * params[:per_page].to_i
limit = params[:per_page].to_i
end

if per_page > 500
per_page = 500
end
page = (params[:page] || 1).to_i
per_page = (params[:per_page] || 500).to_i
offset = (page - 1) * per_page

generic_objects = if offset.positive?
object.limit(params[:per_page]).order(id: :asc).offset(offset).limit(limit)
else
object.all.order(id: :asc).offset(offset).limit(limit)
end
generic_objects = object.order(id: :asc).offset(offset).limit(per_page)

if response_expand?
list = []
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/channels_email_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def verify
channel_id: channel.id,
)
else
address = EmailAddress.create(
EmailAddress.create(
realname: params[:meta][:realname],
email: email,
active: true,
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/creates_ticket_articles.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def article_create(ticket, params)
begin
base64_data = attachment[:data].gsub(/[\r\n]/, '')
attachment_data = Base64.strict_decode64(base64_data)
rescue ArgumentError => e
rescue ArgumentError
raise Exceptions::UnprocessableEntity, "Invalid base64 for attachment with index '#{index}'"
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/import_otrs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def import_start
end

def import_check
statistic = Import::OTRS::Requester.list
Import::OTRS::Requester.list
issues = []

# check count of dynamic fields
Expand Down
5 changes: 2 additions & 3 deletions app/controllers/integration/check_mk_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def update
ticket = Ticket.find_by(id: ticket_id)
next if !ticket

article = Ticket::Article.create!(
Ticket::Article.create!(
ticket_id: ticket_id,
type_id: Ticket::Article::Type.find_by(name: 'web').id,
sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id,
Expand All @@ -81,7 +81,6 @@ def update
}
return
end
state = Ticket::State.lookup(id: auto_close_state_id)
ticket_ids_found.each do |ticket_id|
ticket = Ticket.find_by(id: ticket_id)
next if !ticket
Expand All @@ -107,7 +106,7 @@ def update
},
}
)
article = Ticket::Article.create!(
Ticket::Article.create!(
ticket_id: ticket.id,
type_id: Ticket::Article::Type.find_by(name: 'web').id,
sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class KnowledgeBase::Answer::AttachmentsController < ApplicationController
def create
file = params[:file]

store = Store.add(
Store.add(
object: @answer.class.name,
o_id: @answer.id,
data: file.read,
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/long_polling_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def message_receive
return
end
end
rescue => e
rescue
raise Exceptions::UnprocessableEntity, 'Invalid client_id in receive loop!'
end
end
Expand Down
1 change: 0 additions & 1 deletion app/controllers/slas_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ def index

# calendars
assets = {}
calendar_ids = []
Calendar.all.each do |calendar|
assets = calendar.assets(assets)
end
Expand Down
1 change: 0 additions & 1 deletion app/controllers/ticket_overviews_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ def show
index_and_lists = Ticket::Overviews.index(current_user)
indexes = []
index_and_lists.each do |index|
assets = {}
overview = Overview.lookup(id: index[:overview][:id])
meta = {
name: overview.name,
Expand Down
1 change: 0 additions & 1 deletion app/controllers/time_accountings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ def by_ticket
end
if !agents[local_time_unit[:agent_id]]
agent_user = User.lookup(id: local_time_unit[:agent_id])
agent = '-'
if agent_user
agents[local_time_unit[:agent_id]] = agent_user.fullname
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ def search

query = params[:query]
if query.respond_to?(:permit!)
query = query.permit!.to_h
query.permit!.to_h
end

query = params[:query] || params[:term]
Expand Down Expand Up @@ -543,7 +543,7 @@ def email_verify_send
# return
#end

token = Token.create(action: 'Signup', user_id: user.id)
Token.create(action: 'Signup', user_id: user.id)

result = User.signup_new_token(user)
if result && result[:token]
Expand Down
1 change: 0 additions & 1 deletion app/jobs/ticket_article_communicate_email_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ def perform(article_id)

# build subject
ticket = Ticket.lookup(id: record.ticket_id)
article_count = Ticket::Article.where(ticket_id: ticket.id).count

subject_prefix_mode = record.preferences[:subtype]

Expand Down
3 changes: 1 addition & 2 deletions app/models/application_model/can_lookup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ def lookup(**attr)
attr.transform_keys!(&:to_sym).slice!(*lookup_keys)
raise ArgumentError, "Valid lookup attribute required (#{lookup_keys.join(', ')})" if attr.empty?

record = cache_get(attr.values.first)
record ||= find_and_save_to_cache_by(attr)
cache_get(attr.values.first) || find_and_save_to_cache_by(attr)
end

=begin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ def search_index_attribute_lookup
next if !value

# get attribute name
attribute_name_with_id = key.to_s
attribute_name = key.to_s
attribute_name = key.to_s
next if attribute_name[-3, 3] != '_id'

attribute_name = attribute_name[ 0, attribute_name.length - 3 ]
Expand Down
1 change: 0 additions & 1 deletion app/models/application_model/has_attachments.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,5 @@ def attachments_buffer_check
created_by_id: created_by_id,
)
end
attachments_buffer = nil

This comment has been minimized.

Copy link
@dominikklein

dominikklein Sep 21, 2023

Collaborator

@rolfschmidt @mantas @mgruner I'm not sure if this was good. I have the situation during an import when an article is updated the attachments_buffer_check runs twice.

First time for this situation:

instance = Ticket::Article.find(12)

instance.with_lock do
  instance.assign_attributes({
     "attachments"=>[]
  })
end

This is because of the code inside the def attachments=(attachments) method.

And then after the real save:

instance.save!

This comment has been minimized.

Copy link
@mantas

mantas Sep 21, 2023

Collaborator

Yep. Looks like a bug.

This comment has been minimized.

Copy link
@dominikklein

dominikklein Sep 22, 2023

Collaborator

And in general is it good that an instance.assign_attributes already updates the attachments?

This comment has been minimized.

Copy link
@mantas

mantas Sep 22, 2023

Collaborator

I think the whole def attachments is weird when it's saving to database before #save is called. If it's used somewhere to attach attachments without saving parent objects, maybe that functionality should become a separate #append_attachments method. Now it feels like a bunch of black magic :D

This comment has been minimized.

Copy link
@dominikklein

dominikklein Sep 25, 2023

Collaborator

Yes, I agree, I was really confused!
Clearing the buffer will help. But I would add a technical debt topic about the other situation.

end
end
2 changes: 1 addition & 1 deletion app/models/channel/driver/sms/twilio.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def process(_options, attr, channel)
# find sender
user = User.where(mobile: attr[:From]).order(:updated_at).first
if !user
from_comment, preferences = Cti::CallerId.get_comment_preferences(attr[:From], 'from')
_from_comment, preferences = Cti::CallerId.get_comment_preferences(attr[:From], 'from')
if preferences && preferences['from'] && preferences['from'][0]
if preferences['from'][0]['level'] == 'known' && preferences['from'][0]['object'] == 'User'
user = User.find_by(id: preferences['from'][0]['o_id'])
Expand Down
9 changes: 2 additions & 7 deletions app/models/channel/email_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,6 @@ def _process(channel, msg)
# save changes set by x-zammad-ticket-followup-* headers
ticket.save! if ticket.has_changes_to_save?

state = Ticket::State.find(ticket.state_id)
state_type = Ticket::StateType.find(state.state_type_id)

# set ticket to open again or keep create state
if !mail['x-zammad-ticket-followup-state'.to_sym] && !mail['x-zammad-ticket-followup-state_id'.to_sym]
new_state = Ticket::State.find_by(default_create: true)
Expand Down Expand Up @@ -480,7 +477,7 @@ def self.process_unprocessable_mails(params = {})
path = Rails.root.join('tmp', 'unprocessable_mail')
files = []
Dir.glob("#{path}/*.eml") do |entry|
ticket, article, user, mail = Channel::EmailParser.new.process(params, IO.binread(entry))
ticket, _article, _user, _mail = Channel::EmailParser.new.process(params, IO.binread(entry))
next if ticket.blank?

files.push entry
Expand Down Expand Up @@ -608,15 +605,13 @@ def get_attachments(file, attachments, mail)
file.header.fields.each do |field|

# full line, encode, ready for storage

value = field.to_utf8
if value.blank?
value = field.raw_value
end
headers_store[field.name.to_s] = value
rescue => e
rescue
headers_store[field.name.to_s] = field.raw_value

end

# cleanup content id, <> will be added automatically later
Expand Down
3 changes: 0 additions & 3 deletions app/models/channel/filter/monitoring_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,6 @@ def self.run(_channel, mail)
end
end

# check if ticket with host is open
customer = User.lookup(id: session_user_id)

# follow up detection by meta data
open_states = Ticket::State.by_category(:open)
ticket_ids = Ticket.where(state: open_states).order(created_at: :desc).limit(5000).pluck(:id)
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/checks_condition_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def validate_condition
value: 1,
}

ticket_count, tickets = Ticket.selectors(validate_condition, limit: 1, current_user: User.find(1))
ticket_count, _tickets = Ticket.selectors(validate_condition, limit: 1, current_user: User.find(1))
return true if ticket_count.present?

raise Exceptions::UnprocessableEntity, 'Invalid ticket selector conditions'
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/has_groups.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def flush_group_access_buffer
return if group_access_buffer.nil?

yield
group_access_buffer = nil
self.group_access_buffer = nil
cache_delete
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def in_timeplan?(time = Time.zone.now)
end

def matching_count
ticket_count, tickets = Ticket.selectors(condition, limit: 1)
ticket_count, _tickets = Ticket.selectors(condition, limit: 1)
ticket_count || 0
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/knowledge_base/answer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def assets(data = {})
end

data = ApplicationModel::CanAssets.reduce(siblings, data)
data = ApplicationModel::CanAssets.reduce(translations, data)
ApplicationModel::CanAssets.reduce(translations, data)
end

attachments_cleanup!
Expand Down
2 changes: 1 addition & 1 deletion app/models/knowledge_base/category.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def assets(data = {})
data = ApplicationModel::CanAssets.reduce(translations, data)

# include parent category or KB for root to have full path
data = (parent || knowledge_base).assets(data)
(parent || knowledge_base).assets(data)
end

def self_parent?(candidate)
Expand Down
3 changes: 1 addition & 2 deletions app/models/object_manager/attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ def self.list_full
result = ObjectManager::Attribute.all.order('position ASC, name ASC')
references = ObjectManager::Attribute.attribute_to_references_hash
attributes = []
assets = {}
result.each do |item|
attribute = item.attributes
attribute[:object] = ObjectLookup.by_id(item.object_lookup_id)
Expand Down Expand Up @@ -908,7 +907,7 @@ def check_name
# fixes issue #2236 - Naming an attribute "attribute" causes ActiveRecord failure
begin
ObjectLookup.by_id(object_lookup_id).constantize.instance_method_already_implemented? name
rescue ActiveRecord::DangerousAttributeError => e
rescue ActiveRecord::DangerousAttributeError
raise "#{name} is a reserved word, please choose a different one"
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/object_manager/attribute/validation/required.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def system_user?
end

def user_id
user_id ||= UserInfo.current_user_id
@user_id ||= UserInfo.current_user_id
end

def user
Expand Down
1 change: 0 additions & 1 deletion app/models/observer/sla/ticket_rebuild_escalation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ def _check(record)

# check if condition has changed
changed = false
fields_to_check = nil
fields_to_check = if record.class == Sla
%w[condition calendar_id first_response_time update_time solution_time]
else
Expand Down
1 change: 0 additions & 1 deletion app/models/observer/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ def before_update(record)
# do not send anything if nothing has changed
return true if real_changes.blank?

changed_by_id = nil
changed_by_id = if record.respond_to?('updated_by_id')
record.updated_by_id
else
Expand Down
2 changes: 0 additions & 2 deletions app/models/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,6 @@ def self.migrate(package, direction = 'normal')

return true if !File.exist?(location)

migrations_done = Package::Migration.where(name: package.underscore)

# get existing migrations
migrations_existing = []
Dir.foreach(location) do |entry|
Expand Down
1 change: 0 additions & 1 deletion app/models/store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,6 @@ def image_resize(content, width)
local_sha = Digest::SHA256.hexdigest(content)

cache_key = "image-resize-#{local_sha}_#{width}"
all = nil
image = Cache.get(cache_key)
return image if image

Expand Down
2 changes: 1 addition & 1 deletion app/models/store/provider/file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def self.get_location(sha)
if !File.exist?(location)
FileUtils.mkdir_p(location)
end
full_path = location += file
full_path = location + file
full_path.gsub('//', '/')
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/text_module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def self.push(locale)

# set new translator_key if given
if result.data['translator_key']
translator_key = Setting.set('translator_key', result.data['translator_key'])
Setting.set('translator_key', result.data['translator_key'])
end

true
Expand Down
Loading

0 comments on commit a1da3a2

Please sign in to comment.