search

ari / jobsworth

Project Management, Collaboration and Time Tracking.
GNU Affero General Public License v3.0
639 stars 198 forks source link

includes() behavior in Rails 4 #548

Closed zaidakram closed 8 years ago

zaidakram commented 8 years ago

TaskFilter model is currently suffering from an issue brought up by a change in behavior of includes() in Rails 4. Long story short, Rails 3 knew (by checking the where clause) whether to eager load or join the associative table when used inlucdes(). Rails 4 asks you to check this manually... Now a combination of references() and includes() is to be used to get a JOIN where an associative table's column is accessed in a where clause.

count, display_count and tasks methods in TaskFilter model rely on other methods to format a query based on different conditions. This change means all those methods are to be refactored.

References:

1 - 3 ways to do eager loading (preloading) in Rails 3 & 4 2 - Preload, Eagerload, Includes and Joins

ari commented 8 years ago

The code removed from Rails is this: https://github.com/senny/rails/commit/22b3481ba2aa55fad1f9a5db94072312b345fb55

ari commented 8 years ago

Also worth commenting here that when this is fixed we need to revert https://github.com/ari/jobsworth/commit/80143ad8749e5cfc8f405ae2a74e1228a436d871

ari commented 8 years ago

@nasa42 Vikrant, if you have any ideas here they would be welcome.

nasa42 commented 8 years ago

Seems like @zaidakram has fixed this issue. @zaidakram can this be closed?

zaidakram commented 8 years ago

display and display_count methods are working fine. I'm not sure if the fix is working for tasks method. We can leave this open until we are sure.

ari commented 8 years ago

If the tasks method shows counts for the filters, then it is working since I saw it in action.