gravitystorm / openstreetmap-website

The Rails application powering OpenStreetMap
http://www.openstreetmap.org/
GNU General Public License v2.0
2 stars 1 forks source link

Standardise on find_by() vs where().first #272

Closed gravitystorm closed 1 year ago

gravitystorm commented 1 year ago

There's a lot of code where we want to try loading one record and get nil if that's not working (e.g. record is not visible).

We currently use a mixture of find_by(:id => 123) and where(:id => 123).first. These are slightly different, but only when you expect the where() to return more than one record, which afaict doens't apply to us.

I think the find_by is cleaner and easier to read.

gravitystorm commented 1 year ago

https://github.com/rubocop/rubocop-rails/issues/1058

gravitystorm commented 1 year ago

Given there's already a PR for the issue, I think the best approach is to wait for it to be released, then fix the issues while turning on the option.

gravitystorm commented 1 year ago

https://github.com/openstreetmap/openstreetmap-website/pull/4274