westonganger / paper_trail-association_tracking

Plugin for the PaperTrail gem to track and reify associations
MIT License
128 stars 38 forks source link

fix: foreign_type empty #42

Closed llpereiras closed 3 months ago

llpereiras commented 1 year ago

Hi guys, on this week, we upgrade version of rails to 7.0 in big legacy project on our company. Another upgrade was paper_trail to version 14. After upgrade this problem start happen.

The variable foreign_type started to resolve to string empty ""

    171:   if assoc.options[:polymorphic]
    172:     foreign_type = @record.send(assoc.foreign_type)

    [1] pry(#<PaperTrail::RecordTrail>)> foreign_type
=> ""

In the next line if foreign_type && ::PaperTrail.request.enabled_for_model?(foreign_type.constantize) We get the this error: wrong constant name \n\n Object.const_get(camel_cased_word)

The backtrace of paper_trail:

"backtrace":[
      "/var/app/vendor/bundle/ruby/3.1.0/gems/activesupport-7.0.4/lib/active_support/inflector/methods.rb:280:in `const_get'",
      "/var/app/vendor/bundle/ruby/3.1.0/gems/activesupport-7.0.4/lib/active_support/inflector/methods.rb:280:in `constantize'",
      "/var/app/vendor/bundle/ruby/3.1.0/gems/activesupport-7.0.4/lib/active_support/core_ext/string/inflections.rb:74:in `constantize'",
      "/var/app/vendor/bundle/ruby/3.1.0/gems/paper_trail-association_tracking-2.2.1/lib/paper_trail_association_tracking/record_trail.rb:173:in `save_bt_association'",
      "/var/app/vendor/bundle/ruby/3.1.0/gems/paper_trail-association_tracking-2.2.1/lib/paper_trail_association_tracking/record_trail.rb:123:in `block in save_bt_associations'",
      "/var/app/vendor/bundle/ruby/3.1.0/gems/paper_trail-association_tracking-2.2.1/lib/paper_trail_association_tracking/record_trail.rb:122:in `each'",
      "/var/app/vendor/bundle/ruby/3.1.0/gems/paper_trail-association_tracking-2.2.1/lib/paper_trail_association_tracking/record_trail.rb:122:in `save_bt_associations'",
      "/var/app/vendor/bundle/ruby/3.1.0/gems/paper_trail-association_tracking-2.2.1/lib/paper_trail_association_tracking/record_trail.rb:115:in `save_associations'",
      "/var/app/vendor/bundle/ruby/3.1.0/gems/paper_trail-association_tracking-2.2.1/lib/paper_trail_association_tracking/record_trail.rb:79:in `record_update'",
      "/var/app/vendor/bundle/ruby/3.1.0/gems/paper_trail-14.0.0/lib/paper_trail/model_config.rb:95:in `block in on_touch'"
   ]

We add .presence method to fix this problem.

It's make senses?

westonganger commented 1 year ago

This fix isnt correct. But this does look like it would be an issue for polymorphic relationships.

Seems like it should be something like the following:

if assoc.options[:polymorphic]
  assoc_record = @record.send(assoc.name)
  if assoc_record && ::PaperTrail.request.enabled_for_model?(assoc.class)
    assoc_version_args[:foreign_key_id] = @record.send(assoc.foreign_key)
    assoc_version_args[:foreign_type] = assoc_record.class.name
  end

It seems like we are missing specs for this in https://github.com/westonganger/paper_trail-association_tracking/blob/master/spec/paper_trail/associations/belongs_to_spec.rb

llpereiras commented 1 year ago

@westonganger Thanks for your reply, I will check your suggestion on the next week and try add specs too.

westonganger commented 3 months ago

Duplicate of https://github.com/westonganger/paper_trail-association_tracking/pull/48 which has already been merged