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

Allow to specify an (arel) attribute as locale for queries #314

Closed
doits opened this issue Jan 25, 2019 · 6 comments
Closed

Allow to specify an (arel) attribute as locale for queries #314

doits opened this issue Jan 25, 2019 · 6 comments

Comments

@doits
Copy link
Contributor

doits commented Jan 25, 2019

I have a model Post with a title. The title can be correctly translated to other locales by mobility, for example with title_de, title_fr, ... etc.

For each Post there is a main_locale attribute. In this attribute, each post has a locale which is the main locale for this Post. This locale for example is used as a fallback, so the fallback is different per instance and not per model.

I can get the per instance fallback with the default option:

class Post
  extend Mobility
 
  translates :title, default: ->(attr, _locale) { public_send("#{attr}_main") }

  def title_main
    title(locale: main_locale, default: nil)
  end

  def title_main=(value)
    public_send(:title=, value, locale: main_locale)
  end
end

But I am stuck with queries. For example I'd like to sort or find them by main locale, but I don't know how. I could imagine an interface like this:

# `locale_column` as a special key
Post.all.i18n.order(title: :desc, locale_column: :main_locale)

Same goes for finding records:

Post.all.i18n.where(title: 'abc', locale_column: :main_locale)

Post.all.i18n do
  title(locale: arel_table[:main_locale]).matches('abc')
end

... and pluck...

Post.all.i18n.pluck(:title, locale_column: :main_locale)

# I think actually `locale` is not even implemented yet?
# Post.all.i18n.pluck(:title, locale: :fr) # does not work?

... and probably some more query methods.

Or is there a way to archive this with the current version I don't know of? Mostly I'm interested in sorting and filtering, but other query methods would be nice, too, of course :-)

@doits doits changed the title Allow to specify an attribute or arel table as locale for queries Allow to specify an (arel) attribute as locale for queries Jan 25, 2019
@shioyama
Copy link
Owner

Interesting question! You're right that locale is not a keyword argument on pluck, so your last code will not work.

I think this will be challenging to implement in a backend-independent way... may hit on limits of Arel. I'll have to think about it.

This is related to #51 btw, which is an issue that has been around for a while... although Mobility has gotten much closer in the last year 😄

@doits
Copy link
Contributor Author

doits commented Jan 28, 2019

For the record, this is what I came up so far with arel and the key value store. I didn't test it much for correctness yet, but based on some manual quick tests it seems to work.

  1. Ordering
scope :order_by_title_main, lambda { |dir|
  dir = dir.to_s.casecmp('asc').zero? ? 'asc' : 'desc'

  mobility_table =
    Mobility::ActiveRecord::StringTranslation.arel_table.alias('MobilityOrderByTitleMain')

  joins(
    arel_table.join(mobility_table, Arel::Nodes::OuterJoin).on(
      mobility_table[:translatable_type].eq(klass.name)
      .and(mobility_table[:translatable_id].eq(arel_table[:id]))
      .and(mobility_table[:key].eq(:title))
      .and(mobility_table[:locale].eq(arel_table[:main_locale]))
    ).join_sources
  ).order(mobility_table[:value].public_send(dir))
}
  1. Searching
# search over main title
scope :title_main_search, lambda { |search|
  next none if search.blank?

  search_like = "%#{sanitize_sql_like(search)}%"

  mobility_table =
    Mobility::ActiveRecord::StringTranslation.arel_table.alias('MobilityTitleMainSearch')

  joins(
    arel_table.join(mobility_table).on(
      mobility_table[:translatable_type].eq(klass.name)
      .and(mobility_table[:translatable_id].eq(arel_table[:id]))
      .and(mobility_table[:key].eq(:title))
      .and(mobility_table[:locale].eq(arel_table[:main_locale]))
    ).join_sources
  ).where(mobility_table[:value].matches(search_like))
}

# search over title in all languages
scope :title_search, lambda { |search|
  next none if search.blank?

  search_like = "%#{sanitize_sql_like(search)}%"

  mobility_table =
    Mobility::ActiveRecord::StringTranslation.arel_table.alias('MobilityTitleSearch')

  joins(
    arel_table.join(mobility_table).on(
      mobility_table[:translatable_type].eq(klass.name)
      .and(mobility_table[:translatable_id].eq(arel_table[:id]))
      .and(mobility_table[:key].eq(:title))
    ).join_sources
  ).where(mobility_table[:value].matches(search_like))
}

@doits
Copy link
Contributor Author

doits commented Jan 28, 2019

Oh one gotcha though: You cannot use the search scopes with .or. For example Post.title_search('abc').or(Post.where(something: 'other')) does not work (because of the join).

The search scopes can be reimplemented as a subquery though, then it would work. But it might be slower.

@shioyama
Copy link
Owner

shioyama commented Jan 29, 2019

Wow, really nice work!

You're code made me think, would Mobility maybe just handle an arel attribute as a locale if you passed it in? i.e. if instead of passing a symbol or string, you pass an Arel attribute to the keyword argument on a query method, would it just work as you have done above?

And it seems that, in some cases, it does.

Post.i18n.where(title: 'foo', locale: Post.arel_table[:main_locale])

This will not work like this for the KeyValue backend (which you are using), but it actually does work out of the box for jsonb backends. The reason is that nowhere in that backend code does the code actually explicitly assume that locale is a String or Symbol.

The KeyValue backend almost works with this out of the box. Here is the actual join:

relation.joins(m.join(t, join_type).
on(t[:key].eq(key).
and(t[:locale].eq(locale).
and(t[:translatable_type].eq(model_class.base_class.name).
and(t[:translatable_id].eq(m[:id]))))).join_sources)

And you see here, locale can be anything! This is the beauty of polymorphism, isn't it?

The problem (and the reason this won't work if you try it with KeyValue) is actually minor, but Mobility aliases the table and in that case it does assume that locale is a string:

def table_alias(locale)
"#{table_name}_#{Mobility.normalize_locale(locale)}"
end

But actually, you can get around this without even changing Mobility. Mobility wants the arel node to return something reasonably string-like from to_s. So we can make it do that:

main_locale = Post.arel_table[:main_locale]
main_locale.define_singleton_method :to_s do
  "main_locale"
end

And now, you should find that this:

posts = Post.i18n.where(title: 'foo', locale: main_locale)

returns you the expected results:

posts.map { |p| p.title(locale: p.main_locale) }
#=> ["foo", "foo", ...]

and gives you this SQL:

SELECT "posts".* FROM "posts"
  INNER JOIN "mobility_string_translations" "Post_title_main_locale_string_translations"
    ON "Post_title_main_locale_string_translations"."key" = 'content'
    AND "Post_title_main_locale_string_translations"."locale" = "posts"."main_locale"
    AND "Post_title_main_locale_string_translations"."translatable_type" = 'Post'
    AND "Post_title_main_locale_string_translations"."translatable_id" = "posts"."id"
  WHERE "Post_title_main_locale_string_translations"."value" = 'foo'

Which is exactly what you want, right?

@doits
Copy link
Contributor Author

doits commented Jan 29, 2019

Thanks, this looks good! I tried passing an arel attribute to locale before but didn't think of overwriting #to_s to actually make it work, it's a good trick/workaround. I will see how far I come with this and report back.

@shioyama
Copy link
Owner

One thing is that order doesn't take a locale, so it won't work like that. But I think having locale be an optional keyword argument to order (and pluck, group, select, etc.) probably makes sense, and shouldn't be very hard to add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants