From 11a349d15cfcd642481f19347ccc56f524ae08d8 Mon Sep 17 00:00:00 2001 From: Giovanni Cappellotto Date: Sun, 5 Feb 2017 00:31:09 -0500 Subject: [PATCH 1/4] Filter tomatoes by creation date Closes https://github.com/tomatoes-app/tomatoes/issues/241. --- app/controllers/api/tomatoes_controller.rb | 9 +++- app/models/tomato.rb | 2 + .../api/tomatoes_controller_test.rb | 46 +++++++++++++++++-- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/app/controllers/api/tomatoes_controller.rb b/app/controllers/api/tomatoes_controller.rb index 62295612..0a8da78c 100644 --- a/app/controllers/api/tomatoes_controller.rb +++ b/app/controllers/api/tomatoes_controller.rb @@ -4,7 +4,14 @@ 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 + if from = Time.zone.parse(params[:from].to_s) + @tomatoes = @tomatoes.after(from) + end + if to = Time.zone.parse(params[:to].to_s) + @tomatoes = @tomatoes.before(to) + end + @tomatoes = @tomatoes.order_by([[:created_at, :desc], [:_id, :desc]]).page params[:page] render json: Presenter::Tomatoes.new(@tomatoes) end diff --git a/app/models/tomato.rb b/app/models/tomato.rb index 47501e56..d35b1f95 100644 --- a/app/models/tomato.rb +++ b/app/models/tomato.rb @@ -22,6 +22,8 @@ class Tomato include ActionView::Helpers::TextHelper include ApplicationHelper + scope :before, -> (time) { where(:created_at.lt => time) } + class << self def after(time) where(created_at: { '$gte': time }).order_by([[:created_at, :desc]]) diff --git a/test/functional/api/tomatoes_controller_test.rb b/test/functional/api/tomatoes_controller_test.rb index 61efa2af..3d64ed82 100644 --- a/test/functional/api/tomatoes_controller_test.rb +++ b/test/functional/api/tomatoes_controller_test.rb @@ -6,14 +6,17 @@ class TomatoesControllerTest < ActionController::TestCase @user = User.create!(name: 'name', email: 'email@example.com') @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.hour.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 @@ -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 @@ -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 From 5858d1db73c8e495bb299278a1c43306bb6120c0 Mon Sep 17 00:00:00 2001 From: Giovanni Cappellotto Date: Sun, 5 Feb 2017 00:36:53 -0500 Subject: [PATCH 2/4] Redefine after scope --- app/controllers/tomatoes_controller.rb | 2 +- app/controllers/welcome_controller.rb | 2 +- app/models/tomato.rb | 7 ++----- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/app/controllers/tomatoes_controller.rb b/app/controllers/tomatoes_controller.rb index 02fa2319..2fd62c18 100644 --- a/app/controllers/tomatoes_controller.rb +++ b/app/controllers/tomatoes_controller.rb @@ -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]]) @tomatoes_count = current_user.tomatoes_counters @projects = @tomatoes.collect(&:projects).flatten.uniq diff --git a/app/controllers/welcome_controller.rb b/app/controllers/welcome_controller.rb index bcb59a78..5d9c735c 100644 --- a/app/controllers/welcome_controller.rb +++ b/app/controllers/welcome_controller.rb @@ -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 diff --git a/app/models/tomato.rb b/app/models/tomato.rb index d35b1f95..752ccf27 100644 --- a/app/models/tomato.rb +++ b/app/models/tomato.rb @@ -23,12 +23,9 @@ class Tomato include ApplicationHelper scope :before, -> (time) { where(:created_at.lt => time) } + scope :after, -> (time) { where(:created_at.gte => time) } class << self - def after(time) - where(created_at: { '$gte': time }).order_by([[:created_at, :desc]]) - end - def by_day(tomatoes) to_lines(tomatoes) do |tomatoes_by_day| yield(tomatoes_by_day) @@ -67,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)}") From 36a38c79bae76a51314df5f9f03f02f93d45ffbd Mon Sep 17 00:00:00 2001 From: Giovanni Cappellotto Date: Sun, 5 Feb 2017 00:50:25 -0500 Subject: [PATCH 3/4] Fix rubocop offenses --- .rubocop.yml | 2 +- app/controllers/api/tomatoes_controller.rb | 16 ++++++++++------ test/functional/api/tomatoes_controller_test.rb | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 8dfcc0a2..d7ad8780 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -23,7 +23,7 @@ Lint/UnderscorePrefixedVariableName: # Offense count: 6 Metrics/AbcSize: - Max: 33 + Max: 33.56 # Offense count: 1 # Configuration parameters: CountComments. diff --git a/app/controllers/api/tomatoes_controller.rb b/app/controllers/api/tomatoes_controller.rb index 0a8da78c..44187271 100644 --- a/app/controllers/api/tomatoes_controller.rb +++ b/app/controllers/api/tomatoes_controller.rb @@ -5,12 +5,8 @@ class TomatoesController < BaseController def index @tomatoes = current_user.tomatoes - if from = Time.zone.parse(params[:from].to_s) - @tomatoes = @tomatoes.after(from) - end - if to = Time.zone.parse(params[:to].to_s) - @tomatoes = @tomatoes.before(to) - end + @tomatoes = @tomatoes.after(from) if from + @tomatoes = @tomatoes.before(to) if to @tomatoes = @tomatoes.order_by([[:created_at, :desc], [:_id, :desc]]).page params[:page] render json: Presenter::Tomatoes.new(@tomatoes) @@ -46,6 +42,14 @@ def destroy private + def from + @from ||= Time.zone.parse(params[:from].to_s) + end + + def to + @to ||= Time.zone.parse(params[:to].to_s) + end + def find_tomato @tomato = current_user.tomatoes.find(params[:id]) end diff --git a/test/functional/api/tomatoes_controller_test.rb b/test/functional/api/tomatoes_controller_test.rb index 3d64ed82..3d851f6c 100644 --- a/test/functional/api/tomatoes_controller_test.rb +++ b/test/functional/api/tomatoes_controller_test.rb @@ -9,7 +9,7 @@ class TomatoesControllerTest < ActionController::TestCase @tomato1.created_at = 3.hours.ago @tomato1.save! @tomato2 = @user.tomatoes.build(tag_list: 'one, two') - @tomato2.created_at = 2.hour.ago + @tomato2.created_at = 2.hours.ago @tomato2.save! @tomato3 = @user.tomatoes.build(tag_list: 'three, four') @tomato3.created_at = 1.hour.ago From e20a4ee47dd22074b3a9ced790958f5d8d7155f8 Mon Sep 17 00:00:00 2001 From: Giovanni Cappellotto Date: Sun, 5 Feb 2017 00:58:11 -0500 Subject: [PATCH 4/4] Update API doc --- app/views/pages/api_reference.html.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/views/pages/api_reference.html.md b/app/views/pages/api_reference.html.md index 92dba76b..7ec7b6e4 100644 --- a/app/views/pages/api_reference.html.md +++ b/app/views/pages/api_reference.html.md @@ -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