Skip to content
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

Merged
merged 4 commits into from
Feb 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Lint/UnderscorePrefixedVariableName:

# Offense count: 6
Metrics/AbcSize:
Max: 33
Max: 33.56

# Offense count: 1
# Configuration parameters: CountComments.
Expand Down
13 changes: 12 additions & 1 deletion app/controllers/api/tomatoes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ class TomatoesController < BaseController
before_action :find_tomato, only: [:show, :update, :destroy]

def index
@tomatoes = current_user.tomatoes.order_by([[:created_at, :desc], [:_id, :desc]]).page params[:page]
@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]
Copy link
Member

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?


render json: Presenter::Tomatoes.new(@tomatoes)
end
Expand Down Expand Up @@ -39,6 +42,14 @@ def destroy

private

def from
@from ||= Time.zone.parse(params[:from].to_s)
Copy link
Member

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?

Copy link
Member

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 

Copy link
Member Author

@potomak potomak Feb 5, 2017

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.

end

def to
@to ||= Time.zone.parse(params[:to].to_s)
end

def find_tomato
@tomato = current_user.tomatoes.find(params[:id])
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/tomatoes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]])
Copy link
Member

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 :)

@tomatoes_count = current_user.tomatoes_counters
@projects = @tomatoes.collect(&:projects).flatten.uniq

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/welcome_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def new_tomato
end

def daily_tomatoes
@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]])
end

def tomatoes_counters
Expand Down
9 changes: 4 additions & 5 deletions app/models/tomato.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,10 @@ class Tomato
include ActionView::Helpers::TextHelper
include ApplicationHelper

class << self
def after(time)
where(created_at: { '$gte': time }).order_by([[:created_at, :desc]])
end
scope :before, -> (time) { where(:created_at.lt => time) }
scope :after, -> (time) { where(:created_at.gte => time) }

class << self
def by_day(tomatoes)
to_lines(tomatoes) do |tomatoes_by_day|
yield(tomatoes_by_day)
Expand Down Expand Up @@ -65,7 +64,7 @@ def projects
private

def must_not_overlap
last_tomato = user.tomatoes.after(Time.zone.now - DURATION.seconds).first
last_tomato = user.tomatoes.after(Time.zone.now - DURATION.seconds).order_by([[:created_at, :desc]]).first
return unless last_tomato
limit = (DURATION.seconds - (Time.zone.now - last_tomato.created_at)).seconds
errors.add(:base, "Must not overlap saved tomaotes, please wait #{humanize(limit)}")
Expand Down
4 changes: 4 additions & 0 deletions app/views/pages/api_reference.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ Authorization: d994a295cf68342b99e3036827d3ef8a

* `page` a positive integer value to select a page in the range
[1, `total_pages`]
* `from` a ISO 8601 date time, selects tomatoes where `created_at` is greater
than or equals to the parameter value
* `to` a ISO 8601 date time, selects tomatoes where `created_at` is less than
the parameter value

#### Response

Expand Down
46 changes: 41 additions & 5 deletions test/functional/api/tomatoes_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@ class TomatoesControllerTest < ActionController::TestCase
@user = User.create!(name: 'name', email: '[email protected]')
@user.authorizations.create!(provider: 'tomatoes', token: '123')
@tomato1 = @user.tomatoes.build
@tomato1.created_at = 2.hours.ago
@tomato1.created_at = 3.hours.ago
@tomato1.save!
@tomato2 = @user.tomatoes.build(tag_list: 'one, two')
@tomato2.created_at = 1.hour.ago
@tomato2.created_at = 2.hours.ago
@tomato2.save!
@tomato3 = @user.tomatoes.build(tag_list: 'three, four')
@tomato3.created_at = 1.hour.ago
@tomato3.save!

@other_user = User.create!
@tomato3 = @other_user.tomatoes.create!
@other_tomato = @other_user.tomatoes.create!
end

teardown do
Expand All @@ -36,7 +39,40 @@ class TomatoesControllerTest < ActionController::TestCase
tomatoes_ids = parsed_response['tomatoes'].map { |t| t['id'] }
assert_includes tomatoes_ids, @tomato1.id.to_s
assert_includes tomatoes_ids, @tomato2.id.to_s
assert_not_includes tomatoes_ids, @tomato3.id.to_s
assert_includes tomatoes_ids, @tomato3.id.to_s
assert_not_includes tomatoes_ids, @other_tomato.id.to_s
end

test 'GET /index, it should return current user\'s list of tomatoes filtered by date (from)' do
get :index, token: '123', from: 150.minutes.ago.iso8601
assert_response :success
assert_equal 'application/json', @response.content_type
parsed_response = JSON.parse(@response.body)
tomatoes_ids = parsed_response['tomatoes'].map { |t| t['id'] }
assert_equal tomatoes_ids.size, 2
assert_includes tomatoes_ids, @tomato2.id.to_s
assert_includes tomatoes_ids, @tomato3.id.to_s
end

test 'GET /index, it should return current user\'s list of tomatoes filtered by date (from, to)' do
get :index, token: '123', from: 150.minutes.ago.iso8601, to: 90.minutes.ago.iso8601
assert_response :success
assert_equal 'application/json', @response.content_type
parsed_response = JSON.parse(@response.body)
tomatoes_ids = parsed_response['tomatoes'].map { |t| t['id'] }
assert_equal tomatoes_ids.size, 1
assert_includes tomatoes_ids, @tomato2.id.to_s
end

test 'GET /index, it should return current user\'s list of tomatoes filtered by date (to)' do
get :index, token: '123', to: 90.minutes.ago.iso8601
assert_response :success
assert_equal 'application/json', @response.content_type
parsed_response = JSON.parse(@response.body)
tomatoes_ids = parsed_response['tomatoes'].map { |t| t['id'] }
assert_equal tomatoes_ids.size, 2
assert_includes tomatoes_ids, @tomato1.id.to_s
assert_includes tomatoes_ids, @tomato2.id.to_s
end

test 'GET /show, given an invalid token, it should return an error' do
Expand All @@ -55,7 +91,7 @@ class TomatoesControllerTest < ActionController::TestCase

test 'GET /show, it should not return other users\' tomatoes' do
assert_raises(Mongoid::Errors::DocumentNotFound) do
get :show, token: '123', id: @tomato3.id.to_s
get :show, token: '123', id: @other_tomato.id.to_s
end
end

Expand Down