Add ability to specify default_sort_direction on a resource

[stevegeek]

Description

The ability to specify a default_sort_column was added recently, this PR adds the ability to also specify the direction of the sort.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Manual review steps

Simply add a .default_sort_direction = :asc to a Resource

[stevegeek]

I will need to update the docs when I get a chance

[codeclimate]

Code Climate has analyzed commit 484bfc3 and detected 0 issues on this pull request.

View more on Code Climate.

[adrianthedev]

FYI, these are the docs for sort column
https://github.com/avo-hq/docs.avohq.io/pull/269/files

[stevegeek]

Docs added avo-hq/docs.avohq.io#274

[Paul-Bob]

Feature tests are breaking

[stevegeek]

@Paul-Bob ok seems to be related to the change to remove the || :desc , will look at it now

[stevegeek]

@Paul-Bob so activesupport Class.class_attribute sets the default only on definition of the attribute, ie it does not act as a default when the attribute is specifically set to nil. Im not sure what the desired behaviour here is. My initial implementation meant that if default_sort_direction was specifically set to nil then the sort direction would default to :desc , now in the current implementation, setting default_sort_direction to nil explicitly removes a sort order parameter.

[Paul-Bob]

Why would someone set self.default_sort_direction = nil?

[stevegeek]

I imagine one would only do so to explicitly force to use the default, or in the case of it being set by some other configuration value say which may end up being nil. Hence I would suggest we revert back to the original implementation with || :desc which is more defensive, setting the sort direction to :desc in the event self.default_sort_direction is set to nil (on purpose or inadvertently)

[Paul-Bob]

In my opinion, setting self.default_sort_direction = nil to enforce an explicit default is not the right approach and we should not maintain it. The intention behind this line is to explicitly set default_sort_direction to nil. In some specific edge cases, this behavior might be necessary, and we should preserve the ability to set it to nil when required.

I made this change, which resolved the test issues.

Thank you for your contribution, @stevegeek! We appreciate it.

[stevegeek]

@Paul-Bob ok! You are welcome