administrate icon indicating copy to clipboard operation
administrate copied to clipboard

Searching translated attributes yields "column does not exist" error

Open sedubois opened this issue 4 years ago • 10 comments

  • What were you trying to do?

Our app's content is stored in DB and translated using Mobility. For instance a Thought model has translates :quote and ThoughtDashboard has quote: Field::Text. Listing, showing and editing the thoughts' quotes in the different languages from the admin dashboard works as expected (*). Behind the scenes Mobility automatically generates the required getter and setter methods which it maps to ActiveRecord associations.

When using the administrate search box, records whose searchable attributes contain the provided text should be returned, whether these attributes are translated or not.

  • What did you end up with (logs, or, even better, example apps are great!)?

This error is raised when performing a search:

ActionView::Template::Error: PG::UndefinedColumn: ERROR:  column thoughts.quote does not exist
LINE 1: ...me" AS CHAR(256))) LIKE '%thought%' OR LOWER(CAST("thoughts"...
                                                             ^

It appears that because the attribute is Field::Text, Administrate assumes that the data is stored in a text column in the same table. However the data may well be stored in the same table but using JSON, or in a key-value polymorphic table, or (as in our case) in a belongs-to table, or why not even in other ways.

Although Mobility has various backend strategies, it provides a unified querying API, e.g. thought.quote "just works" and shields us from worrying about "implementation details" regarding how the translation is actually stored. This is why I can simply rely on Field::Text in the dashboard to display and edit the attribute. Unfortunately this assumption no longer holds when performing a search.

Exact-match searches can be done with Thought.i18n.where(quote: "blah") and partial case-insensitive matches with Arel predicates: Thought.i18n { quote.matches("%thought%") }. Mobility also has an integration with Ransack allowing for queries like Thought.ransack(quote_cont: 'foo').result. An important improvement would be having real full-text search (e.g. using pg_search or searchkick) where fuzzy matches are returned by order or decreasing relevance. Maybe it would be too much for Administrate to do all of this out of the box, but making it possible by plugging in an API would be great. Or can this somehow already be done by overriding some methods?

  • What versions are you running?
    • Rails 6.1.0.rc1
    • administrate master 1a35ab210435995a5e3afcd1efd4163c9aff9f83
    • mobility 1.0.0.rc1

(*) As in the main app, the admin routes are scoped to set the proper I18n.locale, and there is a language switcher.

sedubois avatar Dec 02 '20 11:12 sedubois

This is going to be a tricky one. Administrate's search currently makes some assumptions that don't hold well here. Ideally we would want to create a custom field type (eg: Field::MobilityI18nText) that described how to search it. However this is not currently supported, and it's not trivial to add.

An alternative would be creating your own search, instead of using Administrate's default one. Remove Administrate's search field, use your own one, send queries to a custom action, handle it, and render the result with an Administrate index view. I think that should work, but I haven't tried.

pablobm avatar Dec 03 '20 16:12 pablobm

@pablobm OK below is a custom solution to get ILIKE search working for translated models.

# app/controllers/admin/application_controller.rb
module Admin
  class ApplicationController < Administrate::ApplicationController
    #...
    
    def index
      # copied from superclass, with following changes:
+     return super unless resource_class < Translatable
      search_term = params[:search].to_s.strip
-     resources = Administrate::Search.new(scoped_resource,
-                                          dashboard_class,
-                                          search_term).run
+     resources = search_term.blank? ? scoped_resource.all : i18n_search(search_term)
      resources = apply_collection_includes(resources)
      resources = order.apply(resources)
      resources = resources.page(params[:page]).per(records_per_page)
      page = Administrate::Page::Collection.new(dashboard, order: order)

      render locals: {
        resources: resources,
        search_term: search_term,
        page: page,
        show_search_bar: show_search_bar?,
      }
    end

    #...
    def i18n_search(search_term)
      search_term = sanitize_sql_like(search_term)
      attributes = dashboard_class::ATTRIBUTE_TYPES.select { |_, field| field.searchable? }

      resource_class.i18n do
        def match((attribute, field), search_term)
          # TODO add API to fields to allow matchers for other column types, e.g. `.eq(search_term)`
          instance_eval(attribute.to_s).matches("%#{search_term}%")
        end

        attributes.reduce(nil) do |memo, nxt|
          memo.nil? ? match(nxt, search_term) : memo.or(match(nxt, search_term))
        end
      end
    end
  end
end

My translatable models include a Translatable module:

module Translatable
  extend ActiveSupport::Concern

  included do
    extend Mobility

    default_scope { i18n }

    # various other useful scopes defined as well
    # ...
  end
end

Actually the method is called i18n_search(), but except for the scope .i18n coming from Mobility it is quite generic (and I already apply it using a default_scope). I just don't know how to replace that scope to perform the Arel query on the class' default scope, so that it would also work in non-translated models. Any idea how to make that generic?

The great thing with the solution above is that thanks to Mobility's API it should work no matter how the translation data is stored (in a key-value table, in a JSON column, etc).

Anyway at least now I have something working for my app. Next steps could be to retrofit something like this in Administrate if you deem it worthwhile, and also play with having more proper full-text search with pg_search or searchkick.

sedubois avatar Dec 09 '20 18:12 sedubois

@shioyama any idea about this question above?

Actually the method is called i18n_search(), but except for the scope .i18n coming from Mobility it is quite generic (and I already apply it using a default_scope). I just don't know how to replace that scope to perform the Arel query on the class' default scope, so that it would also work in non-translated models. Any idea how to make that generic?

sedubois avatar Dec 11 '20 11:12 sedubois

Hmm I don't know administrate at all, so it's going to be hard to say very much. Under the hood the i18n scope in Mobility is doing two things, creating an Arel node for each translated attribute:

backend_class = Post.mobility_backend_class(:title)
node = backend_class.build_node(:title, :en)

You can do things like node.eq('foo') etc like any other Arel node.

Once you have the node, you the build a relation by applying the backend scope to a relation (like Post.all):

relation = backend_class.apply_scope(Post.all, node, :en)

You can then call relation.all, relation.first, relation.to_sql etc. (This second step is necessary for some backends like Table and KeyValue to join translations, which can't be done at the arel node level.)

That's pretty complicated which is why Mobility gives you the ability to just pass a block to the query scope like this:

Post.i18n do
  title.eq('foo')
end

The title in the block resolves to the node above.

If you need to do something tricky, you might need to actually build up the relation using the steps above.

shioyama avatar Dec 11 '20 13:12 shioyama

Thanks @shioyama for taking the time to reply. My question wasn't clear. In a plain Rails project (where Mobility or Administrate for that matter are not involved at all), how to make the below query work? I just don't know how to navigate the ActiveRecord/Mobility codebase to find the answer myself.

Post.??? { title.matches("%foo%") }

Supposing that such an API is available in plain ActiveRecord, using it in Administrate would allow to make queries that would "just work", regardless of whether the model is plain vanilla Rails or is translated with Mobility (or supposedly, something else).

sedubois avatar Dec 11 '20 15:12 sedubois

Hmm, so actually you made me realize that some things in the query plugin that are currently private should actually be exposed.

But to answer your question, there is no ??? which will work AFAIK in Rails, since Rails does not support block-format query. Mobility custom implements it using a class called Mobility::Plugins::ActiveRecord::Query::VirtualRow, which is actually private but I will make it public in the next release.

You can do this currently but it's going to be quite ugly:

Mobility::Plugins::ActiveRecord::Query.const_get(:VirtualRow).build_query(resource_class, Mobility.locale) do
  title.matches("%foo%")
end

I think that might do what you want. I will make the VirtualRow class public in the next release so you don't need that const_get (which gets around private_constant).

shioyama avatar Dec 12 '20 02:12 shioyama

Ok I created https://github.com/shioyama/mobility/pull/471 which will make it slightly easier to do this:

Mobility::Plugins::ActiveRecord::Query.build_query(resource_class) do
  title.matches("%foo%")
end

shioyama avatar Dec 12 '20 02:12 shioyama

there is no ??? which will work AFAIK in Rails, since Rails does not support block-format query

@shioyama Do you think a case could be made to request adding this in Rails? If there is a reason to refuse it, then the flip-side of that question would be, how to make queries in Mobility which match "the Rails way"? Or would you recommend I just give up on this idea of having the same API in both?

sedubois avatar Dec 12 '20 07:12 sedubois

Do you think a case could be made to request adding this in Rails?

This is basically what Wharel does, and I believe there is a PR somewhere to make Rails do it as well (might have been merged actually).

shioyama avatar Dec 19 '20 06:12 shioyama

You are possibly referring to this PR, but it seems far from ready for merge, and would take a long time before being available in all supported Rails versions unfortunately.

sedubois avatar Dec 19 '20 08:12 sedubois