Sort by default_sort on association class. #750
Conversation
lunks
changed the title
lunks
force-pushed
the
sort-by-association-attribute
branch
2 times, most recently
from
2625f24 to
72ae815
Compare
|
hi @Graysonwright and @nickcharlton could you pls take a look on this PR? thanks |
lunks
force-pushed
the
sort-by-association-attribute
branch
from
72ae815 to
e91f43c
Compare
|
|
||
| ordered = order.apply(relation) | ||
|
|
||
| expect(relation).to have_received(:order).with('customers.updated_at ASC') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [82/80]
| end | ||
|
|
||
| context 'when `order` uses an association with a default_sort method' do | ||
| it 'orders by the attribute given in the default_sort method' do |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| end | ||
| end | ||
|
|
||
| context 'when `order` uses an association with a default_sort method' do |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
|
|
||
| ordered = order.apply(relation) | ||
|
|
||
| expect(relation).to have_received(:order).with('customers.name DESC') |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| @@ -51,6 +52,33 @@ | |||
| expect(ordered).to eq(relation) | |||
| end | |||
| end | |||
|
|
|||
| context 'when `order` uses an association with no default_sort method' do | |||
| it 'orders by `name` on the associated record by default' do | |||
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| @@ -51,6 +52,33 @@ | |||
| expect(ordered).to eq(relation) | |||
| end | |||
| end | |||
|
|
|||
| context 'when `order` uses an association with no default_sort method' do | |||
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
|
|
||
| def order_name | ||
| if association.klass.respond_to? :default_sort | ||
| association.klass.default_sort |
There was a problem hiding this comment.
Use 2 (not 0) spaces for indentation.
| end | ||
|
|
||
| def relation_with_order | ||
| association ? relation.includes(association.name).order("#{association.plural_name}.#{order_name} #{direction_for_sql}") : relation |
This allows us to sort by these associations using a given attribute that uses name by default but can be overwritten by the static method default_sort on the associated class.
lunks
force-pushed
the
sort-by-association-attribute
branch
from
e91f43c to
5b57358
Compare
|
Thanks for your PR! I'm going to close this, as I've merged #810 instead. |
|
Great to have the feature in, thanks! |
This allows us to sort by these associations using a given attribute that uses name by default but can be overwritten by the static method default_sort on the associated class.
This is an initial implementation, feedback is more than welcome. I'd like to know if this is something that is desirable and/or if this approach is ok for the given problem.
Relates to #278.