patricklindsay / wice_grid

A Rails grid plugin to create grids with sorting, pagination, and (automatically generated) filters
http://wicegrid.herokuapp.com/
MIT License
33 stars 29 forks source link

Wrong Element count when using with Association #60

Open CR4567 opened 5 years ago

CR4567 commented 5 years ago

Hi, when I use the associations I get a wrong count of items in my table. I attached a Screenshot which shows an example of my list. There you have multiple households with associated users. In total there are 16 households and pagination should show 20 per page. So you should see all of the items, but it shows only 7 items. The reason must be that it doesn't shows distinct households and instead just filters them.

I'm not sure how to explain it in simple words. But I think you will see it on the screenshots. Not sure if this is a problem of wice_grid or kaminari. In older versions this was working correctly. Any help appreciated.

bildschirmfoto 2019-01-11 um 14 51 10

bildschirmfoto 2019-01-11 um 14 53 45
afdev82 commented 5 years ago

Any news on that? Could you identify the latest version without the problem?

CR4567 commented 5 years ago

hi, I just debugged some versions and finally got it... or at least hope so... it seems that the problem is not in one of the wice_grid versions, instead happens when updating from rails 5.2.0 to newer. Will try to look deeper into this issue.

afdev82 commented 5 years ago

Hi, thank you.
I had also the idea that it should be related to something more on the "core" (ActiveRecord).

afdev82 commented 5 years ago

Using git bisect, I found the bad commit:

f5d5a02318501c4af49cbf1fa0e5fd2a905113cc is the first bad commit
commit f5d5a02318501c4af49cbf1fa0e5fd2a905113cc
Author: Ryuta Kamizono <kamipo@gmail.com>
Date:   Sun May 28 09:29:49 2017 +0900

    Fix GROUP BY queries to apply LIMIT/OFFSET after aggregations

    If `eager_loading` is true, `apply_join_dependency` force applies
    LIMIT/OFFSET before JOINs by `limited_ids_for` to keep parent records
    count. But for aggregation queries, LIMIT/OFFSET should be applied after
    aggregations the same as SQL semantics.

    And also, we could not replace SELECT list by `limited_ids_for` when a
    query has a GROUP BY clause. It had never been worked since it will
    causes generating invalid SQL for MySQL, PostgreSQL, and probably most
    backends.
% ARCONN=postgresql be ruby -w -Itest test/cases/calculations_test.rb -n test_group_by_with_limit
Using postgresql
Run options: -n test_group_by_with_limit --seed 20925

# Running:

E

Error:
CalculationsTest#test_group_by_with_limit:
ActiveRecord::StatementInvalid: PG::GroupingError: ERROR:  column "posts.id" must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: SELECT  DISTINCT "posts"."id", "posts"."type" AS alias_0 FRO...                         ^
: SELECT  DISTINCT "posts"."id", "posts"."type" AS alias_0 FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" GROUP BY "posts"."type" ORDER BY "posts"."type" ASC LIMIT $1
```

Fixes #8103.
Closes #27249.

:040000 040000 f75c29b60b0b04c320a8271782f74841745f5ff8 f6218fe29e135d0175cf696659de682d1be3d50a M activerecord



I don't know how to fix this, I hope someone with more experience will have a look.
Anyway, reverting the commit seems to fix the issue.
JasonBarnabe commented 5 years ago

I believe the test suite covers this issue, but it does not currently test anything after Rails 5.2.0.

~/wice_grid (master) $ appraisal rails-5.2 rspec ./spec/features/action_column_request_spec.rb:19
Error loading RubyGems plugin "/Users/jasonbarnabe/.rvm/gems/ruby-2.6.1/gems/yard-0.8.7.6/lib/rubygems_plugin.rb": can't modify frozen Hash (FrozenError)
>> BUNDLE_GEMFILE=/Users/jasonbarnabe/wice_grid/gemfiles/rails_5.2.gemfile bundle exec rspec ./spec/features/action_column_request_spec.rb:19
-- create_table("dummies", {:force=>:cascade})
   -> 0.0063s
Run options: include {:locations=>{"./spec/features/action_column_request_spec.rb"=>[19]}}

action_column WiceGrid
  should select rows with the select all button and deselect them with the deselect button

Finished in 4.7 seconds (files took 3.69 seconds to load)
1 example, 0 failures

Coverage report generated for RSpec to /Users/jasonbarnabe/wice_grid/coverage. 1224 / 1911 LOC (64.05%) covered.
Coverage (64.05%) is below the expected minimum coverage (87.53%).
~/wice_grid (master) $ appraisal rails-5.2 bundle update rails > /dev/null
Error loading RubyGems plugin "/Users/jasonbarnabe/.rvm/gems/ruby-2.6.1/gems/yard-0.8.7.6/lib/rubygems_plugin.rb": can't modify frozen Hash (FrozenError)
Error loading RubyGems plugin "/Users/jasonbarnabe/.rvm/gems/ruby-2.6.1/gems/yard-0.8.7.6/lib/rubygems_plugin.rb": can't modify frozen Hash (FrozenError)
~/wice_grid (master) $ appraisal rails-5.2 rspec ./spec/features/action_column_request_spec.rb:19
Error loading RubyGems plugin "/Users/jasonbarnabe/.rvm/gems/ruby-2.6.1/gems/yard-0.8.7.6/lib/rubygems_plugin.rb": can't modify frozen Hash (FrozenError)
>> BUNDLE_GEMFILE=/Users/jasonbarnabe/wice_grid/gemfiles/rails_5.2.gemfile bundle exec rspec ./spec/features/action_column_request_spec.rb:19
-- create_table("dummies", {:force=>:cascade})
   -> 0.0074s
Run options: include {:locations=>{"./spec/features/action_column_request_spec.rb"=>[19]}}

action_column WiceGrid
  should select rows with the select all button and deselect them with the deselect button (FAILED - 1)

Failures:

  1) action_column WiceGrid should select rows with the select all button and deselect them with the deselect button
     Failure/Error: expect(page).to have_content('Selected tasks: 507, 508, 509, 510, 511, 512, 513, 514, 515, 516, 517, 518, 519, 520, 521, 522, 523, 524, 525, and 526')
       expected #has_content?("Selected tasks: 507, 508, 509, 510, 511, 512, 513, 514, 515, 516, 517, 518, 519, 520, 521, 522, 523, 524, 525, and 526") to return true, got false
     # ./spec/features/action_column_request_spec.rb:24:in `block (2 levels) in <top (required)>'

Finished in 6.39 seconds (files took 3.6 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/features/action_column_request_spec.rb:19 # action_column WiceGrid should select rows with the select all button and deselect them with the deselect button

Coverage report generated for RSpec to /Users/jasonbarnabe/wice_grid/coverage. 1224 / 1911 LOC (64.05%) covered.
CR4567 commented 5 years ago

for a quick fix (maybe more a hack ;-) ), I'm using a grouping on 'id'.

initialize_grid(...,
   :order => 'title',
   :group => 'id',
   :order_direction => 'asc',
   :include => [...])

but in my case I have to remove strict sql_mode, since I don't want a group for all joins. You can do this by setting it in the database.yml

development:
...
  variables:
    sql_mode: TRADITIONAL

Maybe this helps someone... still looking for a better solution.

afdev82 commented 4 years ago

Maybe this commit could fix this problem: https://github.com/kreintjes/wice_grid/commit/e3006b3c65a482543bf2d8401025fe28662dc312