globalize/globalize

Should #saved_changes contain the changed values?

ppostma opened this issue · 4 comments

In activerecord 5.0, #changes can be used to get updated attributes with the old/new values. But in 5.1 this is deprecated when used in callbacks and #saved_changes needs to be used instead. However, this does not return the old/new values correctly. See attached code snippet, which fails on line 48 (Expected: ["title", "changed title"], Actual: [nil, nil]).

I'm wondering whether this is a bug or do I need to retrieve the changed values in a different way (i.e. from the translations association?)

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'activerecord', '5.2.5'
  gem 'globalize', '6.0.0'
  gem 'sqlite3', '1.4.2'
end

require 'active_record'
require 'globalize'
require 'minitest/autorun'
require 'logger'

# Ensure backward compatibility with Minitest 4
Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

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

  create_table :post_translations, force: true do |t|
    t.references :post
    t.string     :title
    t.text       :content
    t.string     :locale
  end
end

class Post < ActiveRecord::Base
  translates :content, :title
end

class BugTest < Minitest::Test
  def test_association_stuff
    post = Post.create!(title: 'title')

    post.title = 'changed title'
    assert_equal(['title', 'changed title'], post.changes['title'])
    assert_equal([nil, nil], post.saved_changes['title'])

    post.save
    assert_equal(nil, post.changes['title'])
    assert_equal(['title', 'changed title'], post.saved_changes['title']) # fails
  end
end

@ppostma we have this custom method for changes - does this help you for saved_changes?

def changes
super.merge(globalize.changes(::Globalize.locale))
end

Maybe we just need something similar?

@parndt Thanks, that helps.

I've experimented a bit by implementing custom tracking of saved attributes, but it might be just easier to get the saved_changes from the translation record itself. This is implemented here and seems to work fine:
KentaaNL@5aaa9d8

Not sure if there are drawbacks to this method so any comments are welcome.
Also, I'm happy to submit a PR for these changes.

A PR would be great, thanks!

Done: #780