Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Monkeypatching "left" in String conflicts with Active Record #47

Closed
norman opened this issue Jan 26, 2015 · 3 comments
Closed

Monkeypatching "left" in String conflicts with Active Record #47

norman opened this issue Jan 26, 2015 · 3 comments

Comments

@norman
Copy link

norman commented Jan 26, 2015

I recently had to track down a problem with a Rails application where a query was failing and isolated it to this code in Active Record:

counts = table_joins.map do |join|
  if join.is_a?(Arel::Nodes::StringJoin)
    # Table names + table aliases
    join.left.downcase.scan(
      /join(?:\s+\w+)?\s+(\S+\s+)?#{quoted_name}\son/
    ).size
  elsif join.respond_to? :left
    join.left.table_name == name ? 1 : 0
  else
    0
  end
end

Since terminal_table aliases String#ljust, to left, any instance of String responds to left when using this gem. In the example above this leads to an ArgumentError. This happens to me on Rails 4.0.12, but I've verified that similar code exists in Rails 4.2.0 as well. To reproduce, create a scope with a manual join, and then invoke the exists? method on the scope. For example:

class Foo < ActiveRecord::Base
  belongs_to :bar
  scope :baz, -> {joins('INNER JOIN bars on bars.id = foos.bar_id')}
end

Foo.baz.exists?(1)

I would suggest removing the aliases in String completely and using the ljust and rjust methods internally instead; left is a very generic method name to add to a core class like String and is likely to conflict with other libraries as well.

@desheikh
Copy link

+1 Just traced down the same issue when installing a gem in our app. terminal-table also breaks group_by queries.

@nateberkopec
Copy link
Collaborator

Fixed by #45.

@norman
Copy link
Author

norman commented Apr 14, 2015

Excellent, thanks @nateberkopec!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants