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

TypeError (no implicit conversion of nil into String) #961

Closed
dev054 opened this issue May 11, 2017 · 8 comments
Closed

TypeError (no implicit conversion of nil into String) #961

dev054 opened this issue May 11, 2017 · 8 comments

Comments

@dev054
Copy link

dev054 commented May 11, 2017

I'm trying to use the experimental feature (track associations) but I'm facing a issue:

I have the following models:

UserVersion:


class UserVersion < PaperTrail::Version
  self.table_name = :user_versions

  belongs_to :user, foreign_key: :whodunnit
end

User:

class User < ApplicationRecord
  has_paper_trail class_name: 'UserVersion'

  has_many :addresses, inverse_of: :user
  accepts_nested_attributes_for :addresses, allow_destroy: true
end

Address:

class Address < ApplicationRecord
  has_paper_trail # <- The error is here

  belongs_to :user, inverse_of: :addresses
end

So, the issue is: associations aren't being tracked when I have a "parent" custom version class...

Always when I try to create/update "addresses", it show the following error:

TypeError (no implicit conversion of nil into String):

If I remove the line has_paper_trail from Address model it works 100% without errors (but, of course, without versioning).

It seems like it searches for the versions (default) table instead of go to user_versions...

I opened a question in SO, however it seems like it's a bug (it's normal in a experimental feature)..

I'm just trying to find a way to do this... even with a workaround.

Thanks in advance.

@jaredbeck
Copy link
Member

jaredbeck commented May 11, 2017

Hi, thanks for testing the experimental associations tracking feature, and thanks for the bug report. Per our contributing guide please use our bug report template.

@dev054
Copy link
Author

dev054 commented May 11, 2017

Hey @jaredbeck, my script:


# Use this template to report PaperTrail bugs.
# Please include only the minimum code necessary to reproduce your issue.
require "bundler/inline"

# STEP ONE: What versions are you using?
gemfile(true) do
  ruby "2.3.3"
  source "https://rubygems.org"
  gem "activerecord", "5.0.0"
  gem "minitest", "5.9.0"
  gem "paper_trail", require: false
  gem 'pg', '~> 0.18'
end


require "active_record"
require "minitest/autorun"
require "logger"
ActiveRecord::Base.establish_connection(adapter: "postgresql",
                                        database: "api_test",
                                        host: '127.0.0.1',
                                        username: 'postgres',
                                        password: 'postgres')
ActiveRecord::Base.logger = nil
ActiveRecord::Schema.define do
  # STEP TWO: Define your tables here.
  create_table :users, force: true do |t|
    t.text :first_name, null: false
    t.timestamps
  end

  create_table :addresses, force: true do |t|
    t.text :zip_code, null: false
    t.references :user, foreign_key: true, null: false
    t.timestamps
  end

  create_table :user_versions do |t|
    t.string :item_type, null: false
    t.integer :item_id, null: false
    t.string :event, null: false
    t.string :whodunnit
    t.json :object
    t.json :object_changes, null: false
    t.integer :transaction_id
    t.index %i(item_type item_id transaction_id)

    t.timestamps
  end

  create_table :version_associations do |t|
    t.integer  :version_id
    t.string   :foreign_key_name, null: false
    t.integer  :foreign_key_id
  end
  add_index :version_associations, [:version_id]
  add_index :version_associations, %i(foreign_key_name foreign_key_id),
    name: "index_version_associations_on_foreign_key"
end
ActiveRecord::Base.logger = Logger.new(STDOUT)
require "paper_trail/config"

# STEP THREE: Configure PaperTrail as you would in your initializer
PaperTrail::Config.instance.track_associations = true

require "paper_trail"

# STEP FOUR: Define your AR models here.
module PaperTrail
  class Version < ActiveRecord::Base
    include PaperTrail::VersionConcern
    self.abstract_class = true
  end
end

class UserVersion < PaperTrail::Version
  self.table_name = :user_versions

  belongs_to :user, foreign_key: :whodunnit
end

class User < ActiveRecord::Base
  has_paper_trail class_name: 'UserVersion'

  has_many :addresses, inverse_of: :user
  accepts_nested_attributes_for :addresses, allow_destroy: true
end

class Address < ActiveRecord::Base
  has_paper_trail # <- The error is here

  belongs_to :user, inverse_of: :addresses
end

# STEP FIVE: Please write a test that demonstrates your issue.
class BugTest < ActiveSupport::TestCase
  def test_1
    user = User.create(first_name: "Jane")
    user.addresses.create(zip_code: '91239182398')
  end
end

# STEP SIX: Run this script using `ruby my_bug_report.rb`

@jaredbeck
Copy link
Member

Thanks! Using your bug report script, I was able to reproduce the TypeError, so I will mark this as a confirmed bug.

Because the associations-tracking feature is experimental, not recommended for production, and still under development, it is not a personal priority for me. (I also do not use it on any of my own projects) However, I'll do what I can to help you develop a fix.

The first step will be to pare down your bug report into a minimal failing test. Questions:

  1. Could the json column be a text?
  2. Could the database be sqlite?
  3. Is the UserVersion class necessary?

@dev054
Copy link
Author

dev054 commented May 11, 2017

  1. Yes, it can be, however does it has any effect on this bug? (I changed the columns to :text and it doesn't seem to have a different behavior).
  2. Unfortunately, not. I have to use Postgres (it works on sqlite?).
  3. Yes, because I don't want to mix all my versioned models in a giant table. I prefer to split (also it has the advantages described in docs.

@jaredbeck
Copy link
Member

Thanks. For those questions, I think we should focus on what has an effect on this bug. We want to identify the minimum requirements for reproducing the bug.

@cpfarher
Copy link

cpfarher commented May 15, 2017

@jaredbeck same error without association tracking over STI

@jaredbeck jaredbeck changed the title Track versions with parent custom version class TypeError (no implicit conversion of nil into String) Aug 18, 2017
@jaredbeck
Copy link
Member

I'm looking at this more closely and it seems like a misconfiguration.

Every model that has_paper_trail must have a concrete version model, ie. that version model must have a table.

In the provided bug report, UserVersion has a table, but PaperTrail::Version does not. It's fine for PaperTrail::Version to be an abstract_class, but has_paper_trail needs a concrete subclass, eg. AddressVersion.

I think we can easily help people avoid this misconfiguration by raising an error if has_paper_trail is told to use an abstract_class.

What do you think?

jaredbeck added a commit that referenced this issue Jan 22, 2018
Instead of crashing when misconfigured Custom Version Classes are
used, an error will be raised earlier, with a much more helpful message.

[Fixes #961]
jaredbeck added a commit that referenced this issue Jan 22, 2018
Instead of crashing when misconfigured Custom Version Classes are
used, an error will be raised earlier, with a much more helpful message.

[Fixes #961]
jaredbeck added a commit that referenced this issue Jan 22, 2018
Instead of crashing when misconfigured Custom Version Classes are
used, an error will be raised earlier, with a much more helpful message.

[Fixes #961]
jaredbeck added a commit that referenced this issue Jan 22, 2018
Instead of crashing when misconfigured Custom Version Classes are
used, an error will be raised earlier, with a much more helpful message.

[Fixes #961]
@jaredbeck
Copy link
Member

Closed by #1040 to be released in 9.0.0

aried3r pushed a commit to aried3r/paper_trail that referenced this issue Dec 14, 2020
Instead of crashing when misconfigured Custom Version Classes are
used, an error will be raised earlier, with a much more helpful message.

[Fixes paper-trail-gem#961]
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