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

History only for current locale #15

Open
0urobor0s opened this issue Mar 19, 2020 · 3 comments
Open

History only for current locale #15

0urobor0s opened this issue Mar 19, 2020 · 3 comments

Comments

@0urobor0s
Copy link

When using this nice gem with the history option I found that the translated slugs where not being saved.
I checked the issues opened and found #10 is my problem, but it is closed. However in that issue the default_backend was table and not, for example, key_value.
Was the fix for all mobility backends?

@shioyama
Copy link
Owner

Hello! Thanks for the issue. There is a test that was added to cover this case, which is now passing with this fix. The fix should work for all backends.

Can you write a failing test for your situation? Then I can maybe try to debug it.

@0urobor0s
Copy link
Author

After seeing the spec file, maybe the behaviour I was expecting was not the intended.
Nonetheless the failing test is:

begin
  require 'bundler/inline'
rescue LoadError => e
  $stderr.puts 'Bundler 1.10 or later is required. Please update Bundler'
  raise e
end

gemfile(true) do
  source 'https://rubygems.org'
  gem 'rails', '6.0.2.2'
  gem 'sqlite3'
  gem 'pry'
  gem 'friendly_id', '~> 5.2.4'
  gem 'mobility', '~> 0.8.9'
  gem 'friendly_id-mobility', '~> 0.5.4'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'
require 'mobility'
require 'friendly_id'
require 'friendly_id/mobility'

Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)

ActiveRecord::Base.establish_connection(
  adapter: 'sqlite3', database: ':memory:'
)
ActiveRecord::Base.logger = Logger.new(STDOUT)

I18n.enforce_available_locales = false

ActiveRecord::Schema.define do
  create_table :articles do |t|
    t.timestamps
  end

  create_table :mobility_string_translations do |t|
    t.string  :locale
    t.string  :key
    t.string  :value
    t.integer :translatable_id
    t.string  :translatable_type
    t.timestamps
  end

  create_table :friendly_id_slugs do |t|
    t.string   :slug,                      null: false
    t.integer  :sluggable_id,              null: false
    t.string   :sluggable_type, limit: 50
    t.string   :locale,                    null: false
    t.string   :scope
    t.datetime :created_at
  end
end

class Article < ActiveRecord::Base
  extend FriendlyId
  extend Mobility

  translates :slug, :title, type: :string, dirty: true, backend: :key_value
  friendly_id :title, use: [:history, :mobility]
end

class SlugWithAllLocalesTest < Minitest::Test
  def test_creation_of_foo_with_relations
    I18n.locale = :en
    article = Article.create!(title_en: "Title", title_pt: "Título")

    assert article.persisted?
    assert FriendlyId::Slug.count == 2
  end
end

@thedarkside
Copy link

thedarkside commented Jul 22, 2022

Ive stumbled upon this too! It is definitely not a desired behavior imho. If one changes a slug and has configured the :history extension it should record any changes regardless of the currently active locale.

Ive debugged this a little and to me it looks like the :history extension does not iterate over all possible localized changes but only considers the one of the currently active locale (being I18n.locale). This is due to the implementation runs in the after_save hook.

So while the change is correctly being persisted for all locales in the entity's translated slug column (in my case post_translations.slug because of the table backend iam using), the change is not being pushed into the friendly_id_slugs table which is the expected behavior.

I suppose that the FriendlyId::History#create_slug needs to be overridden somehow to fix it.

Here is the test from my app showing the issue:

    I18n.with_locale :en do
      assert_difference -> { post.slugs.unscoped.count } do
        assert_changes -> { post.reload.slug_en }, from: 'title-one', to: 'new-slug' do
          post.slug_en = "new-slug"
          post.save!
        end
      end

      assert_difference -> { post.slugs.unscoped.count } do # <-- it breaks here
        assert_changes -> { post.reload.slug_de }, from: 'titel-eins', to: 'neuer-slug' do
          post.slug_de = "neuer-slug"
          post.save!
        end
      end
    end

While running this with I18n.with_locale :de it succeeds but swapping to I18n.with_locale :en it fails with the following error:

Minitest::Assertion: #<Proc:0x000055892d1577f8 ... (lambda)> didn't change by 1.
Expected: 7
  Actual: 6

Ive marked the line in the test code with a comment.

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