From 286fc9847c973c16b0c5de7cf1f75d148593281b Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Mon, 23 Nov 2015 09:03:10 -0700 Subject: [PATCH 1/6] Move search logic from controller into model --- app/controllers/posts_controller.rb | 46 ++++------------------------- app/models/post.rb | 28 ++++++++++++++++++ spec/controllers/post_spec.rb | 25 +++++++++++++--- spec/features/post_spec.rb | 2 +- 4 files changed, 56 insertions(+), 45 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 9dffb31..c971a23 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -5,11 +5,10 @@ class PostsController < ApplicationController :confirm] def index - if check_for_search_params - @posts = post_search - else - @posts = Post.active - end + @posts = Post.active. + filter_near(search_params[:location]). + filter_query(search_params[:query]). + filter_type(search_params[:type]) end def new @@ -89,40 +88,7 @@ def find_post_params params.permit(:id, :validation) end - def check_for_search_params - [:type, :location, :query].any? { |k| params.key?(k) } - end - - def query_location(posts) - if params[:location].blank? - posts - else - posts.near(params[:location]) - end - end - - def query_description(posts) - if params[:query].blank? - posts - else - posts.search(params[:query]) - end - end - - def filter_type(posts) - if params[:type] == "available" - posts.available - elsif params[:type] == "needed" - posts.needed - else - posts - end - end - - def post_search - posts = Post.active - posts = query_location(posts) - posts = query_description(posts) - filter_type(posts) + def search_params + params.permit(:location, :query, :type) end end diff --git a/app/models/post.rb b/app/models/post.rb index aebc2d9..8224406 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -57,6 +57,34 @@ def address [street, city, state, postal_code, country].compact.join(", ") end + def self.filter_type(type) + if type == "needed" || type == "available" + send(type.to_sym) + else + all + end + end + + def self.filter_near(location) + results = near(location) + + if results.blank? + all + else + results + end + end + + def self.filter_query(query) + results = search(query) + + if results.blank? + all + else + results + end + end + def self.generate_validation require "securerandom" # a collision here has low probability, but might as well check diff --git a/spec/controllers/post_spec.rb b/spec/controllers/post_spec.rb index 332e050..4f35b4f 100644 --- a/spec/controllers/post_spec.rb +++ b/spec/controllers/post_spec.rb @@ -4,8 +4,8 @@ describe "GET #index" do it "responds successfully with an HTTP 200 status code" do get :index + expect(response).to be_success - expect(response).to have_http_status(200) end it "renders the index template" do @@ -21,7 +21,7 @@ expect(assigns(:posts)).to match_array([post1, post2]) end - it "searches for location" do + it "searches by location" do post1 = create( :boston_post, show: true @@ -37,7 +37,24 @@ expect(assigns(:posts)).to match_array([post1]) end - it "searches for description" do + it "searches by title" do + post1 = create( + :post, + title: "Xyz", + show: true + ) + + create( + :post, + show: true + ) + + get :index, query: "xyz" + + expect(assigns(:posts)).to match_array([post1]) + end + + it "searches by description" do post1 = create( :post, description: "Need housing in Boston", @@ -54,7 +71,7 @@ expect(assigns(:posts)).to match_array([post1]) end - it "searches for description and location" do + it "searches by description and location" do post1 = create( :boston_post, description: "Need housing in Boston", diff --git a/spec/features/post_spec.rb b/spec/features/post_spec.rb index beaec9e..e4914b3 100644 --- a/spec/features/post_spec.rb +++ b/spec/features/post_spec.rb @@ -28,7 +28,7 @@ expect(page).to have_content post.title end - it "shows a post with a query in it's description" do + it "shows a post with a query in its description" do post1 = create( :available_post, description: "I want some tofu pups", From 481a2edd270520d07c7f365405fa096f78091eba Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Mon, 23 Nov 2015 09:16:40 -0700 Subject: [PATCH 2/6] Fix indentation to please Rubocop --- app/controllers/posts_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index c971a23..49a0a59 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -5,10 +5,10 @@ class PostsController < ApplicationController :confirm] def index - @posts = Post.active. - filter_near(search_params[:location]). - filter_query(search_params[:query]). - filter_type(search_params[:type]) + @posts = Post.active + .filter_near(search_params[:location]) + .filter_query(search_params[:query]) + .filter_type(search_params[:type]) end def new From 7e21126c3421e245f3e684a1f286883e89f44602 Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 4 Dec 2015 16:36:46 -0700 Subject: [PATCH 3/6] Remove 'send' call to enum scope, in favor of where --- app/models/post.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 8224406..ae017b1 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -58,8 +58,10 @@ def address end def self.filter_type(type) - if type == "needed" || type == "available" - send(type.to_sym) + if post_types.keys.include?(type) + # TODO: For rails 5 we can just do post_type: type + # (see: rails/rails#18387) + where(post_type: post_types[type]) else all end From b3300d6027ca5844f59e1761ed18a41f6f8eb471 Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 4 Dec 2015 16:43:59 -0700 Subject: [PATCH 4/6] Restrict expected response to just 200, not 2xx --- spec/controllers/post_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/post_spec.rb b/spec/controllers/post_spec.rb index 4f35b4f..0dbbd5a 100644 --- a/spec/controllers/post_spec.rb +++ b/spec/controllers/post_spec.rb @@ -2,10 +2,10 @@ describe PostsController, type: :controller do describe "GET #index" do - it "responds successfully with an HTTP 200 status code" do + it "responds successfully with an 200 OK HTTP status code" do get :index - expect(response).to be_success + expect(response).to have_http_status(:ok) end it "renders the index template" do From f5cf25a777a53841d2a546c7ea5af4fbfba09e0c Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 4 Dec 2015 16:54:58 -0700 Subject: [PATCH 5/6] Fix filtering based on query, not results --- app/models/post.rb | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index ae017b1..96eab06 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -58,7 +58,7 @@ def address end def self.filter_type(type) - if post_types.keys.include?(type) + if type.present? # TODO: For rails 5 we can just do post_type: type # (see: rails/rails#18387) where(post_type: post_types[type]) @@ -68,22 +68,18 @@ def self.filter_type(type) end def self.filter_near(location) - results = near(location) - - if results.blank? - all + if location.present? + near(location) else - results + all end end def self.filter_query(query) - results = search(query) - - if results.blank? - all + if query.present? + search(query) else - results + all end end From dca5f66630cc2a478760dd7f016eea90224eea17 Mon Sep 17 00:00:00 2001 From: Sean Collins Date: Fri, 4 Dec 2015 16:56:01 -0700 Subject: [PATCH 6/6] =?UTF-8?q?Make=20spec=20more=20=C2=ABrobust=C2=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/controllers/post_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/controllers/post_spec.rb b/spec/controllers/post_spec.rb index 0dbbd5a..30c458d 100644 --- a/spec/controllers/post_spec.rb +++ b/spec/controllers/post_spec.rb @@ -38,9 +38,11 @@ end it "searches by title" do + title = "Xyz" + post1 = create( :post, - title: "Xyz", + title: title, show: true ) @@ -49,7 +51,7 @@ show: true ) - get :index, query: "xyz" + get :index, query: title expect(assigns(:posts)).to match_array([post1]) end