rails_best_practices

rails_best_practices copied to clipboard

We override verified_request? to work around some kind of issue where JSON requests were getting rejected because they didn't have a CSRF token:

class ApplicationController
  # ... omitting other stuff ...
  def verified_request?
    if request.content_type == 'application/json'
      true
    else
      super
    end
  end
end

This method triggers the "Remove unused method" warning even though it's fairly obvious that it is overriding a method in the superclass, which would normally be assumed to be called by the framework.

There are half a dozen other warnings following the same pattern in our codebase.

I am having the same issue. When you create custom classes for models (such as for internal elements of arrays), to run .uniq on them, .eql? must be called. If you do not override .eql? and hash, .uniq will never be false(as it will compare not the elements of the class but the class itself) and therefore never remove duplicates.

I.E. Users model

# == Schema Information
# 
# Table name: users
# 
# id                 :bigint(8)         not null, primary key
# emails          :string             default([]), not null, is an Array
# created_at   :datetime         not null
# updated_at  :datetime         not null
# 

class User < ApplicationRecord
  def emails
    super.map! { |email| email.is_a?(Email) ? email: Email.new(email) }
  end
end

and Email model

# Users email wrapper
class Email
  attr_reader :attributes
  attr_reader :email

  def initialize(email = "")
    @email = email
  end

  def email
    @email
  end

  def email=(email)
    @email = email
  end

  def blank?
    @email.blank?
  end

  def eql?(other)
    @email.to_s == other.email.to_s
  end

  def hash
    @email.to_s.hash
  end

  def to_s
    @email.to_s
  end
end

And in this scenario, best practices catches the eql? (which is called by .uniq) but not hash (which is also called by .uniq). I think disabling it for the entire email.rb file is not a good solution. Inline comments to disable certain checks could handle special cases like this similar to rubocop/reek/...

Inline Disables: https://github.com/flyerhzm/rails_best_practices/issues/271