-
Notifications
You must be signed in to change notification settings - Fork 0
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
Move search logic from controller into model #25
base: dry-up-posts-controller
Are you sure you want to change the base?
Changes from 2 commits
286fc98
481a2ed
7e21126
b3300d6
f5cf25a
dca5f66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's the reason for 'send' in this case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just making our conditional shorter, and DRYing it up. I should make this less clever though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was leveraging the enum helpers but that's not needed, especially when I make it all complicated by using |
||
else | ||
all | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for some reason I'm not able to find docs for an 'all' method anywhere. I can infer what it's doing, but is this a builtin for rails? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's just like calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know why I didn't realize this haha. Probably would have answered that immediately by opening a terminal :P. |
||
end | ||
end | ||
|
||
def self.filter_near(location) | ||
results = near(location) | ||
|
||
if results.blank? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this going to return everything if there is nothing within the range? Wouldn't we prefer a 'there were no results' in this case and return something else here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, here I was filtering based on the results were blank, I should've been checking if the query was 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably should have a different description, since it's not checking for a '200' anymore, but 2xx. I wouldn't be opposed to just the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I always thought :success was just for 200, but that's not the case! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I was thinking of :ok as equal to 200. https://github.com/rack/rack/blob/2f782ce099989e214466872c797f530b5349357d/lib/rack/utils.rb#L509 |
||
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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. def better! |
||
|
||
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", | ||
|
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.
I assume these all just handle 'nil' gracefully in the queries?
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.
yea! I did it wrong (see next comment) but I fixed it!