cst1/openstreetmap-website
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Prefer find_by() instead of where().first

Browse source 
These are very similar, differing only if we would expect multiple
results and the sorting is important. However, in all our cases
we're only expecting one result to be returned, and so find_by is
easier to read.
This commit is contained in:
Andy Allan 2023-10-04 17:53:58 +01:00
parent c8fc2218e5
commit 1700c23dd1
11 changed files with 44 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
.rubocop.yml
View file

@ -48,6 +48,9 @@ Naming/MethodParameterName:
Rails/CreateTableWithTimestamps:
Enabled: false
Rails/FindBy:
IgnoreWhereFirst: false
Rails/FindEach:
Enabled: false

2
app/controllers/api/traces_controller.rb
View file

@ -115,7 +115,7 @@ module Api
trace.save!
# Finally save the user's preferred privacy level
if pref = current_user.preferences.where(:k => "gps.trace.visibility").first
if pref = current_user.preferences.find_by(:k => "gps.trace.visibility")
pref.v = visibility
pref.save
else

2
app/controllers/application_controller.rb
View file

@ -22,7 +22,7 @@ class ApplicationController < ActionController::Base
def authorize_web
if session[:user]
self.current_user = User.where(:id => session[:user], :status => %w[active confirmed suspended]).first
self.current_user = User.find_by(:id => session[:user], :status => %w[active confirmed suspended])
if session[:fingerprint] &&
session[:fingerprint] != current_user.fingerprint

6
app/controllers/diary_entries_controller.rb
View file

@ -61,7 +61,7 @@ class DiaryEntriesController < ApplicationController
def show
entries = @user.diary_entries
entries = entries.visible unless can? :unhide, DiaryEntry
@entry = entries.where(:id => params[:id]).first
@entry = entries.find_by(:id => params[:id])
if @entry
@title = t ".title", :user => params[:display_name], :title => @entry.title
@comments = can?(:unhidecomment, DiaryEntry) ? @entry.comments : @entry.visible_comments
@ -74,7 +74,7 @@ class DiaryEntriesController < ApplicationController
def new
@title = t ".title"
default_lang = current_user.preferences.where(:k => "diary.default_language").first
default_lang = current_user.preferences.find_by(:k => "diary.default_language")
lang_code = default_lang ? default_lang.v : current_user.preferred_language
@diary_entry = DiaryEntry.new(entry_params.merge(:language_code => lang_code))
set_map_location
@ -99,7 +99,7 @@ class DiaryEntriesController < ApplicationController
@diary_entry.user = current_user
if @diary_entry.save
default_lang = current_user.preferences.where(:k => "diary.default_language").first
default_lang = current_user.preferences.find_by(:k => "diary.default_language")
if default_lang
default_lang.v = @diary_entry.language_code
default_lang.save!

8
app/controllers/traces_controller.rb
View file

@ -19,7 +19,7 @@ class TracesController < ApplicationController
# from display name, pick up user id if one user's traces only
display_name = params[:display_name]
if display_name.present?
target_user = User.active.where(:display_name => display_name).first
target_user = User.active.find_by(:display_name => display_name)
if target_user.nil?
render_unknown_user display_name
return
@ -283,7 +283,7 @@ class TracesController < ApplicationController
# Save the trace object
if trace.save
# Finally save the user's preferred privacy level
if pref = current_user.preferences.where(:k => "gps.trace.visibility").first
if pref = current_user.preferences.find_by(:k => "gps.trace.visibility")
pref.v = visibility
pref.save
else
@ -303,11 +303,11 @@ class TracesController < ApplicationController
end
def default_visibility
visibility = current_user.preferences.where(:k => "gps.trace.visibility").first
visibility = current_user.preferences.find_by(:k => "gps.trace.visibility")
if visibility
visibility.v
elsif current_user.preferences.where(:k => "gps.trace.public", :v => "default").first.nil?
elsif current_user.preferences.find_by(:k => "gps.trace.public", :v => "default").nil?
"private"
else
"public"

12
test/controllers/api/traces_controller_test.rb
View file

@ -194,7 +194,7 @@ module Api
# Now authenticated
create(:user_preference, :user => user, :k => "gps.trace.visibility", :v => "identifiable")
assert_not_equal "trackable", user.preferences.where(:k => "gps.trace.visibility").first.v
assert_not_equal "trackable", user.preferences.find_by(:k => "gps.trace.visibility").v
auth_header = basic_authorization_header user.display_name, "test"
post gpx_create_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable" }, :headers => auth_header
assert_response :success
@ -206,13 +206,13 @@ module Api
assert_not trace.inserted
assert_equal File.new(fixture).read, trace.file.blob.download
trace.destroy
assert_equal "trackable", user.preferences.where(:k => "gps.trace.visibility").first.v
assert_equal "trackable", user.preferences.find_by(:k => "gps.trace.visibility").v
# Rewind the file
file.rewind
# Now authenticated, with the legacy public flag
assert_not_equal "public", user.preferences.where(:k => "gps.trace.visibility").first.v
assert_not_equal "public", user.preferences.find_by(:k => "gps.trace.visibility").v
auth_header = basic_authorization_header user.display_name, "test"
post gpx_create_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 1 }, :headers => auth_header
assert_response :success
@ -224,14 +224,14 @@ module Api
assert_not trace.inserted
assert_equal File.new(fixture).read, trace.file.blob.download
trace.destroy
assert_equal "public", user.preferences.where(:k => "gps.trace.visibility").first.v
assert_equal "public", user.preferences.find_by(:k => "gps.trace.visibility").v
# Rewind the file
file.rewind
# Now authenticated, with the legacy private flag
second_user = create(:user)
assert_nil second_user.preferences.where(:k => "gps.trace.visibility").first
assert_nil second_user.preferences.find_by(:k => "gps.trace.visibility")
auth_header = basic_authorization_header second_user.display_name, "test"
post gpx_create_path, :params => { :file => file, :description => "New Trace", :tags => "new,trace", :public => 0 }, :headers => auth_header
assert_response :success
@ -243,7 +243,7 @@ module Api
assert_not trace.inserted
assert_equal File.new(fixture).read, trace.file.blob.download
trace.destroy
assert_equal "private", second_user.preferences.where(:k => "gps.trace.visibility").first.v
assert_equal "private", second_user.preferences.find_by(:k => "gps.trace.visibility").v
end
# Check updating a trace through the api

6
test/controllers/diary_entries_controller_test.rb
View file

@ -164,7 +164,7 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
assert_response :success
assert_template :new
assert_nil UserPreference.where(:user => user, :k => "diary.default_language").first
assert_nil UserPreference.find_by(:user => user, :k => "diary.default_language")
end
def test_create
@ -189,7 +189,7 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
# checks if user was subscribed
assert_equal 1, entry.subscribers.length
assert_equal "en", UserPreference.where(:user => user, :k => "diary.default_language").first.v
assert_equal "en", UserPreference.find_by(:user => user, :k => "diary.default_language").v
end
def test_create_german
@ -216,7 +216,7 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
# checks if user was subscribed
assert_equal 1, entry.subscribers.length
assert_equal "de", UserPreference.where(:user => user, :k => "diary.default_language").first.v
assert_equal "de", UserPreference.find_by(:user => user, :k => "diary.default_language").v
end
def test_new_spammy

32
test/controllers/friendships_controller_test.rb
View file

@ -28,7 +28,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
friend = create(:user)
# Check that the users aren't already friends
assert_nil Friendship.where(:befriender => user, :befriendee => friend).first
assert_nil Friendship.find_by(:befriender => user, :befriendee => friend)
# When not logged in a GET should ask us to login
get make_friend_path(friend)
@ -37,7 +37,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
# When not logged in a POST should error
post make_friend_path(friend)
assert_response :forbidden
assert_nil Friendship.where(:befriender => user, :befriendee => friend).first
assert_nil Friendship.find_by(:befriender => user, :befriendee => friend)
session_for(user)
@ -49,7 +49,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
assert_select "input[type='hidden'][name='referer']", 0
assert_select "input[type='submit']", 1
end
assert_nil Friendship.where(:befriender => user, :befriendee => friend).first
assert_nil Friendship.find_by(:befriender => user, :befriendee => friend)
# When logged in a POST should add the friendship
assert_difference "ActionMailer::Base.deliveries.size", 1 do
@ -59,7 +59,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
end
assert_redirected_to user_path(friend)
assert_match(/is now your friend/, flash[:notice])
assert Friendship.where(:befriender => user, :befriendee => friend).first
assert Friendship.find_by(:befriender => user, :befriendee => friend)
email = ActionMailer::Base.deliveries.first
assert_equal 1, email.to.count
assert_equal friend.email, email.to.first
@ -73,7 +73,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
end
assert_redirected_to user_path(friend)
assert_match(/You are already friends with/, flash[:warning])
assert Friendship.where(:befriender => user, :befriendee => friend).first
assert Friendship.find_by(:befriender => user, :befriendee => friend)
end
def test_make_friend_with_referer
@ -83,7 +83,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
session_for(user)
# Check that the users aren't already friends
assert_nil Friendship.where(:befriender => user, :befriendee => friend).first
assert_nil Friendship.find_by(:befriender => user, :befriendee => friend)
# The GET should preserve any referer
get make_friend_path(friend), :params => { :referer => "/test" }
@ -93,7 +93,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
assert_select "input[type='hidden'][name='referer'][value='/test']", 1
assert_select "input[type='submit']", 1
end
assert_nil Friendship.where(:befriender => user, :befriendee => friend).first
assert_nil Friendship.find_by(:befriender => user, :befriendee => friend)
# When logged in a POST should add the friendship and refer us
assert_difference "ActionMailer::Base.deliveries.size", 1 do
@ -103,7 +103,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
end
assert_redirected_to "/test"
assert_match(/is now your friend/, flash[:notice])
assert Friendship.where(:befriender => user, :befriendee => friend).first
assert Friendship.find_by(:befriender => user, :befriendee => friend)
email = ActionMailer::Base.deliveries.first
assert_equal 1, email.to.count
assert_equal friend.email, email.to.first
@ -125,7 +125,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
create(:friendship, :befriender => user, :befriendee => friend)
# Check that the users are friends
assert Friendship.where(:befriender => user, :befriendee => friend).first
assert Friendship.find_by(:befriender => user, :befriendee => friend)
# When not logged in a GET should ask us to login
get remove_friend_path(friend)
@ -134,7 +134,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
# When not logged in a POST should error
post remove_friend_path, :params => { :display_name => friend.display_name }
assert_response :forbidden
assert Friendship.where(:befriender => user, :befriendee => friend).first
assert Friendship.find_by(:befriender => user, :befriendee => friend)
session_for(user)
@ -146,19 +146,19 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
assert_select "input[type='hidden'][name='referer']", 0
assert_select "input[type='submit']", 1
end
assert Friendship.where(:befriender => user, :befriendee => friend).first
assert Friendship.find_by(:befriender => user, :befriendee => friend)
# When logged in a POST should remove the friendship
post remove_friend_path(friend)
assert_redirected_to user_path(friend)
assert_match(/was removed from your friends/, flash[:notice])
assert_nil Friendship.where(:befriender => user, :befriendee => friend).first
assert_nil Friendship.find_by(:befriender => user, :befriendee => friend)
# A second POST should report that the friendship does not exist
post remove_friend_path(friend)
assert_redirected_to user_path(friend)
assert_match(/is not one of your friends/, flash[:error])
assert_nil Friendship.where(:befriender => user, :befriendee => friend).first
assert_nil Friendship.find_by(:befriender => user, :befriendee => friend)
end
def test_remove_friend_with_referer
@ -169,7 +169,7 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
session_for(user)
# Check that the users are friends
assert Friendship.where(:befriender => user, :befriendee => friend).first
assert Friendship.find_by(:befriender => user, :befriendee => friend)
# The GET should preserve any referer
get remove_friend_path(friend), :params => { :referer => "/test" }
@ -179,13 +179,13 @@ class FriendshipsControllerTest < ActionDispatch::IntegrationTest
assert_select "input[type='hidden'][name='referer'][value='/test']", 1
assert_select "input[type='submit']", 1
end
assert Friendship.where(:befriender => user, :befriendee => friend).first
assert Friendship.find_by(:befriender => user, :befriendee => friend)
# When logged in a POST should remove the friendship and refer
post remove_friend_path(friend), :params => { :referer => "/test" }
assert_redirected_to "/test"
assert_match(/was removed from your friends/, flash[:notice])
assert_nil Friendship.where(:befriender => user, :befriendee => friend).first
assert_nil Friendship.find_by(:befriender => user, :befriendee => friend)
end
def test_remove_friend_unknown_user

6
test/controllers/traces_controller_test.rb
View file

@ -658,7 +658,7 @@ class TracesControllerTest < ActionDispatch::IntegrationTest
# Now authenticated
create(:user_preference, :user => user, :k => "gps.trace.visibility", :v => "identifiable")
assert_not_equal "trackable", user.preferences.where(:k => "gps.trace.visibility").first.v
assert_not_equal "trackable", user.preferences.find_by(:k => "gps.trace.visibility").v
session_for(user)
post traces_path, :params => { :trace => { :gpx_file => file, :description => "New Trace", :tagstring => "new,trace", :visibility => "trackable" } }
assert_response :redirect
@ -672,7 +672,7 @@ class TracesControllerTest < ActionDispatch::IntegrationTest
assert_not trace.inserted
assert_equal File.new(fixture).read, trace.file.blob.download
trace.destroy
assert_equal "trackable", user.preferences.where(:k => "gps.trace.visibility").first.v
assert_equal "trackable", user.preferences.find_by(:k => "gps.trace.visibility").v
end
# Test creating a trace with validation errors
@ -684,7 +684,7 @@ class TracesControllerTest < ActionDispatch::IntegrationTest
# Now authenticated
create(:user_preference, :user => user, :k => "gps.trace.visibility", :v => "identifiable")
assert_not_equal "trackable", user.preferences.where(:k => "gps.trace.visibility").first.v
assert_not_equal "trackable", user.preferences.find_by(:k => "gps.trace.visibility").v
session_for(user)
post traces_path, :params => { :trace => { :gpx_file => file, :description => "", :tagstring => "new,trace", :visibility => "trackable" } }
assert_template :new

6
test/models/node_test.rb
View file

@ -87,7 +87,7 @@ class NodeTest < ActiveSupport::TestCase
assert_equal node_template.timestamp.to_i, node.timestamp.to_i
assert_equal(1, OldNode.where(:node_id => node_template.id).count)
old_node = OldNode.where(:node_id => node_template.id).first
old_node = OldNode.find_by(:node_id => node_template.id, :version => 1)
assert_not_nil old_node
assert_equal node_template.latitude, old_node.latitude
assert_equal node_template.longitude, old_node.longitude
@ -120,7 +120,7 @@ class NodeTest < ActiveSupport::TestCase
# assert_equal node_template.tags, node.tags
assert_equal(2, OldNode.where(:node_id => node_template.id).count)
old_node = OldNode.where(:node_id => node_template.id, :version => 2).first
old_node = OldNode.find_by(:node_id => node_template.id, :version => 2)
assert_not_nil old_node
assert_equal node_template.latitude, old_node.latitude
assert_equal node_template.longitude, old_node.longitude
@ -149,7 +149,7 @@ class NodeTest < ActiveSupport::TestCase
# assert_equal node_template.tags, node.tags
assert_equal(2, OldNode.where(:node_id => node_template.id).count)
old_node = OldNode.where(:node_id => node_template.id, :version => 2).first
old_node = OldNode.find_by(:node_id => node_template.id, :version => 2)
assert_not_nil old_node
assert_equal node_template.latitude, old_node.latitude
assert_equal node_template.longitude, old_node.longitude

2
test/models/trace_test.rb
View file

@ -202,7 +202,7 @@ class TraceTest < ActiveSupport::TestCase
# Check that the tile has been set prior to the bulk import
# i.e. that the callbacks have been run correctly
assert_equal 3221331576, Tracepoint.where(:trace => trace).first.tile
assert_equal 3221331576, Tracepoint.find_by(:trace => trace).tile
end
def test_import_creates_icon