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

Fix count method signature for active_record5 and active_record60 #700

Conversation

kaishuu0123
Copy link
Contributor

@kaishuu0123 kaishuu0123 commented Jan 17, 2024

Description

  • This issue occurs when using (rails 6.0 or 5.0 or 5.1) and passing arguments to the count method
    • ArgumentError: wrong number of arguments (given 1, expected 0)
  • 95ffb25 did not fix active_record5.rb and active_record60.rb
  • I have modified active_record5.rb and active_record60.rb in the same way as 95ffb25

Reproduction code (when use rails 6.0)

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", "~> 6.0.0"

  gem "sqlite3"
  gem "timeout"

  gem "bullet", github: "flyerhzm/bullet", require: false

  # fixed version
  # gem "bullet", github: "kaishuu0123/bullet", branch: "fix/count-method-signature-for-active_record60", require: false
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveSupport::LogSubscriber.colorize_logging = false

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
  end

  create_table :comments, force: true do |t|
    t.integer :post_id
  end
end

class Post < ActiveRecord::Base
  has_many :comments
end

class Comment < ActiveRecord::Base
  belongs_to :post
end

class BugTest < Minitest::Test
  def self.test_order
    :sorted
  end

  def test_001_without_bullet
    post = Post.create!
    post.comments << Comment.create!

    assert_equal 1, post.comments.count(:all)
  end

  # ISSUE
  def test_002_with_bullet
    require "bullet"
    require 'bullet/active_record60'
    require 'bullet/version'

    STDOUT.puts "\n\nBULLET VERSION #{Bullet::VERSION}"

    Bullet.enable = true
    Bullet.rails_logger = true
    Bullet.raise = true
    Bullet.n_plus_one_query_enable = true

    Bullet.profile do
      post = Post.create!
      post.comments << Comment.create!

      assert_equal 1, post.comments.count(:all)
    end
  end
end

Backtrace output by Reproduction code

(snip)
BULLET VERSION 7.1.6
E

Error:
BugTest#test_002_with_bullet:
ArgumentError: wrong number of arguments (given 1, expected 0)
    /usr/local/rvm/gems/default/bundler/gems/bullet-f15e7a130585/lib/bullet/active_record60.rb:285:in `count'
    report.rb:75:in `block in test_002_with_bullet'
    /usr/local/rvm/gems/default/bundler/gems/bullet-f15e7a130585/lib/bullet.rb:239:in `profile'
    report.rb:71:in `test_002_with_bullet'
(snip)

@flyerhzm
Copy link
Owner

@kaishuu0123 thanks for fixing it, would you mind checking if it needs to be fixed in other rails versions?

- fix also count method signature for active_record5
@kaishuu0123
Copy link
Contributor Author

kaishuu0123 commented Jan 18, 2024

@flyerhzm

I checked other rails versions and found that active_record5.rb and active_record60.rb are affected by this issue.
Also fixed active_record5.rb

Thanks for throwing the question out there so i could fix other rails versions.

@kaishuu0123 kaishuu0123 changed the title fix count method signature for active_record60 fix count method signature for active_record5 and active_record60 Jan 18, 2024
@kaishuu0123 kaishuu0123 changed the title fix count method signature for active_record5 and active_record60 Fix count method signature for active_record5 and active_record60 Jan 18, 2024
@flyerhzm flyerhzm merged commit 6abe9a5 into flyerhzm:main Jan 18, 2024
10 checks passed
@kaishuu0123 kaishuu0123 deleted the fix/count-method-signature-for-active_record60 branch January 18, 2024 06:42
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

Successfully merging this pull request may close these issues.

2 participants