metaminded / tabulatr2

A very JS enhanced Tabulatr edition. Rails4, Ruby 2 required
Other
21 stars 10 forks source link

How to deal with chained associations? #66

Open mlt opened 6 years ago

mlt commented 6 years ago

I want to (left) join like 5 tables into a flattened representation with tabulatr2. Defining a block for each column would be inefficient. I have been getting by with views on DB side, but I wonder if there is a better approach. If I somewhat manually form a query and select extra columns I want, those are gone after eager_load call from join_required_tables. What would be the canonical approach? Allow arrays of associations?

mlt commented 6 years ago

rails_select_on_includes gem solves the issue of eager_load wiping virtual attributes. Nevertheless, we do need to replace #count calls with #count(:all) otherwise it produced malformed SQL. https://github.com/mlt/tabulatr2/commit/0023d1a68714bf7b6daf46c6b02c848465ba0b45. P.S. One has to provide correct fully qualified name with sort_sql for a "virtual" column if that comes from some join-chained table. P.P.S. I still feel like join_required_tables shall be rewritten to handle join1:join2:join3:join4:column stuff, and eager loading a hash instead of plain array.

mlt commented 6 years ago

This is not perfect, but allows stuff like

  association %i[vendor parent], :name, filter: :exact, sortable: true
hanspolo commented 6 years ago

Can you give me an example on which count won't work. If I can reproduce the behaviour I would happily replace it with your code.

mlt commented 6 years ago

I'm about to clear up the patch before PR. I think it is better to use (more) dashes instead of colons for nested associations. Meanwhile here is what causes issues with at least Rails 4.2.10 with Ruby 2.4.4. This is a far fetched example as there is only 1 level of associations that is handled well as is with tabulatr2, but if I were to use some fancy Arel expression and a bunch of tables, the result would be the same. We ought to use hash with eager loading and, perhaps, there will be no need in fancy queries in advance and we could leave count alone as is.

diff --git a/spec/dummy/app/controllers/products_controller.rb b/spec/dummy/app/controllers/products_controller.rb
index aff292d..e783c51 100644
--- a/spec/dummy/app/controllers/products_controller.rb
+++ b/spec/dummy/app/controllers/products_controller.rb
@@ -1,7 +1,8 @@
 class ProductsController < ApplicationController

   def simple_index
-    tabulatr_for Product
+    p = Product.joins(:vendor).select('*')
+    tabulatr_for p
   end

   def one_item_per_page_with_pagination
   (1.0ms)  SELECT COUNT(*) FROM "products" INNER JOIN "vendors" ON "vendors"."id" = "products"."vendor_id"
   (1.0ms)  SELECT COUNT(DISTINCT *) FROM "products" INNER JOIN "vendors" ON "vendors"."id" = "products"."vendor_id" LEFT OUTER JOIN "products_tags" ON "products_tags"."product_id" = "products"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "products_tags"."tag_id"
Completed 500 Internal Server Error in 10ms (ActiveRecord: 5.0ms)

SQLite3::SQLException - near "*": syntax error:
  app/controllers/products_controller.rb:5:in `simple_index'

P.S. Just to clarify the need. One may use regular column definition with aforementioned approach to get attributes coming from deeply joined tables instead of using blocks and re-querying the chain.