-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Filter tomatoes by creation date #255
Conversation
@@ -39,6 +42,14 @@ def destroy | |||
|
|||
private | |||
|
|||
def from | |||
@from ||= Time.zone.parse(params[:from].to_s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does it handle nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work! :)
2.3.3 :001 > nil.to_s
=> ""
2.3.3 :002 > Time.zone.parse ''
=> nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's how. #parse
should work on any string, it returns nil
in case of parsing failure. #to_s
will convert any params[:from]
to a string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me is ok!
The only one thing I'd like to change is creating a common scope for sorting tomatoes.
The common use case is sorting tomatoes by creation:
So we can create something like this:
scope :sorted_desc, -> { order_by([[:created_at, :desc], [:_id, :desc]]) }
@@ -39,6 +42,14 @@ def destroy | |||
|
|||
private | |||
|
|||
def from | |||
@from ||= Time.zone.parse(params[:from].to_s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work! :)
2.3.3 :001 > nil.to_s
=> ""
2.3.3 :002 > Time.zone.parse ''
=> nil
@tomatoes = current_user.tomatoes | ||
@tomatoes = @tomatoes.after(from) if from | ||
@tomatoes = @tomatoes.before(to) if to | ||
@tomatoes = @tomatoes.order_by([[:created_at, :desc], [:_id, :desc]]).page params[:page] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you create a scope for sorting tomatoes?
@@ -75,7 +75,7 @@ def create | |||
if @tomato.save | |||
format.js do | |||
@highlight = @tomato | |||
@tomatoes = current_user.tomatoes.after(Time.zone.now.beginning_of_day) | |||
@tomatoes = current_user.tomatoes.after(Time.zone.now.beginning_of_day).order_by([[:created_at, :desc]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you create a scope for sorting, you can reuse it here too :)
Yes @dalpo, I wanted to create such a scope, but I didn't find a good name for it. |
I like sorted by Maybe simply: |
Closes #241.