Translated slugs not generated

[n-rodriguez]

Hi there!

I'm migrating from Globalize to Mobility. I'm also using FriendlyID on some models so I'm migrating from friendly_id-globalize to friendly_id-mobility too.

I've followed the migration tutorial and almost everything works so far, except one thing : translated slugs are not generated.

The slug for the current locale (fr) is well generated and saved, but the slugs for other locales are not generated.

Here my setup taken from the README :

* friendly_id (5.2.4)
* friendly_id-mobility (0.5.3)
* mobility (0.7.6)
* rails (5.1.6)

# Rails config (config/application.rb)
config.i18n.default_locale    = :fr
config.i18n.available_locales = [:fr, :en, :es, :pt]
config.i18n.fallbacks         = { en: [:en, :fr], es: [:es, :fr], pt: [:pt, :es, :fr] }

# Mobility config (config/initializers/mobility.rb)
Mobility.configure do |config|
  config.default_backend                    = :table
  config.accessor_method                    = :translates
  config.query_method                       = :i18n
  config.default_options[:dirty]            = true
  config.default_options[:locale_accessors] = true
  config.default_options[:fallbacks]        = { en: [:en, :fr], es: [:es, :fr], pt: [:pt, :es, :fr] }
end

# the model
class Post < ApplicationRecord
  # Translations
  extend Mobility
  translates :slug, :title
  # translates :slug, :title, dirty: true # also tried this one

  # FriendlyId
  extend FriendlyId
  friendly_id :title, use: :mobility, routes: :rails
  # friendly_id :title, use: [:slugged, :mobility], routes: :rails # also tried this one
end

# the form
= form_for @model do |f|

  - [:fr, :es, :en, :pt].each do |locale|
    = f.text_field "title_#{locale}".to_sym, required: french_locale?(locale)

  = f.form_group do
    = f.submit

When I set all title_* fields, it only generates the fr slug.

I also tried to force slug regeneration as in the FriendlyID doc but got some strange results :

# this one works
Mobility.with_locale(:fr) do
  model.slug_fr = nil
  model.save && model.save
end

# this one doesn't
Mobility.with_locale(:es) do
  model.slug_es = nil
  model.save && model.save
end

the SQL output :

FR update
   (0.2ms)  BEGIN
  Post Exists (0.5ms)  SELECT  1 AS one FROM "posts" INNER JOIN "post_translations" "post_translations_fr" ON "post_translations_fr"."post_id" = "posts"."id" AND "post_translations_fr"."locale" = 'fr' WHERE ("posts"."id" != 9) AND "post_translations_fr"."slug" = 'test-test' LIMIT $1  [["LIMIT", 1]]
  SQL (0.4ms)  DELETE FROM "post_translations" WHERE "post_translations"."id" = $1  [["id", 65]]
  SQL (0.3ms)  UPDATE "post_translations" SET "slug" = $1, "updated_at" = $2 WHERE "post_translations"."id" = $3  [["slug", "test-test"], ["updated_at", "2018-07-10 13:02:38.064513"], ["id", 61]]
  SQL (0.3ms)  UPDATE "posts" SET "updated_at" = $1 WHERE "posts"."id" = $2  [["updated_at", "2018-07-10 13:02:38.065731"], ["id", 9]]
   (5.2ms)  COMMIT
   (0.2ms)  BEGIN
   (0.2ms)  COMMIT
ES update
   (0.2ms)  BEGIN
   (0.2ms)  COMMIT
   (0.2ms)  BEGIN
   (0.1ms)  COMMIT

It's not an issue with strong params since all title_* are persisted.

As I said, the only issue is that slug_es(and friends : en , pt) are never generated, only the current locale.

I don't know if it's a bug or if I missed something in the configuration but I can't finalize the migration without this feature.

Thanks for your help!

[n-rodriguez]

After some digging, it seems that it's not yet a feature. The implementation is not exactly the same as friendly_id-globalize :

current : https://github.com/shioyama/friendly_id-mobility/blob/master/lib/friendly_id/mobility.rb#L44
friendly_id-globalize : https://github.com/norman/friendly_id-globalize/blob/master/lib/friendly_id/globalize.rb#L88

[shioyama]

Hmm I don't see anything obvoiusly wrong with your setup. The last thing you noticed when trying to force slug regeneration is triggering Mobility to destroy the blank translations (that's why you see the DELETE in the SQL).

It would help if you could figure out if this is an issue specific to slugs or whether you can reproduce it without friendly_id-mobility. I think this should work in general:

class Post < ApplicationRecord
  extend Mobility
  translates :title, locale_accessors: true
end

Mobility.locale = :en
post = Post.create(title: "foo")
post.title_en
#=> "foo"
post.title_ja = "foo ja"
post.title_de = "foo de"
post.save
post = Post.first
post.title_en
#=> "foo"
post.title_ja
#=> "foo ja"
post.title_de
#=> "foo de"

Just tried this on 0.7.5 and it works fine AFAICT (and I'm pretty sure tests would fail if it did not).

[n-rodriguez]

I'm writing a standalone test suite

[shioyama]

Oh sorry missed your other comment, reading up now.

[n-rodriguez]

#!/usr/bin/env ruby

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

gemfile(true) do
  gem 'rails', '5.1.6'
  gem 'sqlite3'
  gem 'mobility'
  gem 'friendly_id'
  gem 'friendly_id-mobility'
  gem 'database_cleaner'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'
require 'database_cleaner'

# 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)

I18n.config.available_locales = [:fr, :en, :es, :pt]
I18n.default_locale           = :fr

Mobility.configure do |config|
  config.default_backend                    = :table
  config.accessor_method                    = :translates
  config.query_method                       = :i18n
  config.default_options[:dirty]            = true
  config.default_options[:locale_accessors] = true
end

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

  create_table :post_translations, force: :cascade do |t|
    t.string   :slug
    t.string   :title
    t.text     :content
    t.string   :locale,     null: false
    t.integer  :post_id,    null: false
    t.datetime :created_at, null: false
    t.datetime :updated_at, null: false

    t.index ["locale"], name: "index_post_translations_on_locale"
    t.index ["post_id", "locale"], name: "index_post_translations_on_post_id_and_locale", unique: true
    t.index ["post_id"], name: "index_post_translations_on_post_id"
  end
end

class Post < ActiveRecord::Base
  # Translations
  extend Mobility
  translates :slug, :title, :content, fallbacks: { en: [:fr], pt: [:es, :fr] }

  # FriendlyId
  extend FriendlyId
  friendly_id :title, use: :mobility

  # Uncomment to make tests pass

  # def should_generate_new_friendly_id?
  #   translation_for(::Mobility.locale).send(friendly_id_config.slug_column).nil?
  # end

  # def translation_for(locale)
  #   translation = translations.in_locale(locale)
  #   translation ||= translations.build(locale: locale)
  #   translation
  # end

  # def set_slug(normalized_slug = nil)
  #   super
  #   changed.each do |change|
  #     if change =~ /\A(?:#{friendly_id_config.base}|#{friendly_id_config.slug_column})_([a-z]{2}(_[a-z]{2})?)/
  #       locale, suffix = $1.split('_'.freeze)
  #       locale = "#{locale}-#{suffix.upcase}".freeze if suffix
  #       ::Mobility.with_locale(locale) { super }
  #     end
  #   end
  # end
end

class BugTest < Minitest::Test
  extend Minitest::Spec::DSL

  before :each do
    DatabaseCleaner.start
  end

  after :each do
    DatabaseCleaner.clean
  end

  def test_slug_generation
    post = Post.new(title_fr: 'Titre français', title_es: 'Título español', title_en: 'English title')
    post.save

    # Here what I expect :
    assert_equal('titre-francais', post.slug_fr) # this one pass
    assert_equal('titulo-espanol', post.slug_es) # this one doesn't
    assert_equal('english-title', post.slug_en) # neither this one
    assert_nil post.slug_pt # normal, we didn't provide any value

    post
  end

  def test_slug_regeneration
    # Create post
    post = test_slug_generation

    # Reset slug post
    post.slug_fr = nil
    post.slug_es = nil
    post.save

    # Here what I expect :
    assert_equal('titre-francais', post.slug_fr) # this one pass
    assert_equal('titulo-espanol', post.slug_es) # this one doesn't
  end
end

[n-rodriguez]

I think I found the culprit :)

[n-rodriguez]

should_generate_new_friendly_id? don't care of locale : https://github.com/shioyama/friendly_id-mobility/blob/master/lib/friendly_id/mobility.rb#L49

[shioyama]

Nice catch! Would you like to make a PR to fix it? 😉 Or even just a failing spec would be great.

[n-rodriguez]

Sure :) Do you have an equivalent of translation_for(::Globalize.locale).send(friendly_id_config.slug_column).nil?
Mobility: https://github.com/shioyama/friendly_id-mobility/blob/master/lib/friendly_id/mobility.rb#L49
Globalize : https://github.com/norman/friendly_id-globalize/blob/master/lib/friendly_id/globalize.rb#L92

[n-rodriguez]

On Globalize side : https://github.com/globalize/globalize/blob/e37c471775d196cd4318e61954572c300c015467/lib/globalize/active_record/instance_methods.rb#L125

[n-rodriguez]

I found https://github.com/shioyama/mobility/blob/master/lib/mobility/backends/active_record/table.rb#L287 but got an error : NoMethodError: undefined method "translation_for" for #<Post id: nil>

[shioyama]

Yeah, there is an equivalent but you can't use it since it's specific to the table backend. I'll have a look later but you'll need to use the generic interface, otherwise the change will break any other backend.

[n-rodriguez]

I'll have a look later but you'll need to use the generic interface, otherwise the change will break any other backend.

Ok. For now it works as expected if I add this to the model :

class Post < ActiveRecord::Base
  ....
  def should_generate_new_friendly_id?
    translation_for(::Mobility.locale).send(friendly_id_config.slug_column).nil?
  end

  def translation_for(locale)
    translation = translations.in_locale(locale)
    translation ||= translations.build(locale: locale)
    translation
  end
end

[shioyama]

Yes, that works but it won't work a general solution unfortunately. This is the challenge of Mobility 😉 I'll have a look later.

[n-rodriguez]

Yes, that works but it won't work a general solution unfortunately.

Yes I understand. For now I can live with this small patch and go ahead to finalize the migration to this awesome gem :)
Keep up the good work 👍 and let me know when you'll know the right direction to take.

[shioyama]

Do you think you could create a failing spec? Then I think I can fix this.

[n-rodriguez]

Hi there! I also found a bug in set_slug method :
Currently it doesn't cover the case where you reset a translated slug :

def test_slug_regeneration
  post = Post.new(title_fr: 'Titre français', title_es: 'Título español', title_en: 'English title')
  post.save

  post.slug_fr = nil
  post.slug_es = nil
  post.save

  # Here what I expect :
  assert_equal('titre-francais', post.slug_fr) # this one pass
  assert_equal('titulo-espanol', post.slug_es) # this one doesn't
end

I had to override set_slug method to take of friendly_id_config.slug_column :

def set_slug(normalized_slug = nil)
  super
  changed.each do |change|
    if change =~ /\A#{friendly_id_config.base}_([a-z]{2}(_[a-z]{2})?)/ || change =~ /\A#{friendly_id_config.slug_column}_([a-z]{2}(_[a-z]{2})?)/
      locale, suffix = $1.split('_'.freeze)
      locale = "#{locale}-#{suffix.upcase}".freeze if suffix
      ::Mobility.with_locale(locale) { super }
    end
  end
end

[shioyama]

I added a test in #12 but it's passing for me locally, can you have a look?

[shioyama]

Oh never mind, seems to be failing on Travis.

[shioyama]

Ah ok, I wasn't running the spec correctly locally. So the spec identifies the issue. I'll try adding your fix 😄