diff --git a/.travis.yml b/.travis.yml index aef817216..473a994f6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,7 @@ --- before_install: gem update bundler before_script: bundle exec rake db:create db:schema:load +after_script: bundle exec rubocop -R bundler_args: --without assets:development:production language: ruby rvm: diff --git a/app/controllers/addresses_controller.rb b/app/controllers/addresses_controller.rb index 022bc2fc7..4800a8a1d 100644 --- a/app/controllers/addresses_controller.rb +++ b/app/controllers/addresses_controller.rb @@ -3,10 +3,10 @@ class AddressesController < ApplicationController def show @address = Address.geocode("#{params[:address]}, #{params[:city_state]}") - unless @address.blank? - respond_with @address + if @address.blank? + render(json: {errors: {address: [t('errors.not_found', thing: t('defaults.address'))]}}, status: 404) else - render(json: {errors: {address: [t("errors.not_found", thing: t("defaults.address"))]}}, status: 404) + respond_with @address end end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 27d3b221d..f616b4829 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,8 +2,8 @@ class ApplicationController < ActionController::Base # Prevent CSRF attacks by raising an exception. # For APIs, you may want to use :null_session instead. protect_from_forgery with: :exception - before_filter :set_flash_from_params - before_filter :set_locale + before_action :set_flash_from_params + before_action :set_locale protected @@ -16,8 +16,8 @@ def set_flash_from_params end def set_locale - available_languages = Dir.glob(Rails.root + "config/locales/??.yml").map do |file| - File.basename(file, ".yml") + available_languages = Dir.glob(Rails.root + 'config/locales/??.yml').collect do |file| + File.basename(file, '.yml') end I18n.locale = http_accept_language.compatible_language_from(available_languages) || I18n.default_locale end diff --git a/app/controllers/info_window_controller.rb b/app/controllers/info_window_controller.rb index 293d34a5e..d6b6bf62e 100644 --- a/app/controllers/info_window_controller.rb +++ b/app/controllers/info_window_controller.rb @@ -3,15 +3,15 @@ def index @thing = Thing.find_by_id(params[:thing_id]) if @thing.adopted? if user_signed_in? && current_user == @thing.user - render("users/thank_you") + render('users/thank_you') else - render("users/profile") + render('users/profile') end else if user_signed_in? - render("things/adopt") + render('things/adopt') else - render("users/sign_in") + render('users/sign_in') end end end diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index d7d87cd7a..68b694a60 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -12,7 +12,7 @@ def create def edit self.resource = resource_class.new resource.reset_password_token = params[:reset_password_token] - render("edit", layout: "info_window") + render('edit', layout: 'info_window') end def update @@ -22,7 +22,7 @@ def update resource.unlock_access! if unlockable?(resource) sign_in(resource_name, resource) end - redirect_to(controller: "main", action: "index") + redirect_to(controller: 'main', action: 'index') end private diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 18e463e9e..b759846f9 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,5 +1,5 @@ class SessionsController < Devise::SessionsController - skip_before_filter :verify_authenticity_token, only: [:destroy] + skip_before_action :verify_authenticity_token, only: [:destroy] def new redirect_to(root_path) @@ -12,7 +12,7 @@ def create yield resource if block_given? render(json: resource) else - render(json: {errors: {password: [t("errors.password")]}}, status: 401) + render(json: {errors: {password: [t('errors.password')]}}, status: 401) end end diff --git a/app/controllers/things_controller.rb b/app/controllers/things_controller.rb index f6680ae48..6074e89ea 100644 --- a/app/controllers/things_controller.rb +++ b/app/controllers/things_controller.rb @@ -3,10 +3,10 @@ class ThingsController < ApplicationController def show @things = Thing.find_closest(params[:lat], params[:lng], params[:limit] || 10) - unless @things.blank? - respond_with @things + if @things.blank? + render(json: {errors: {address: [t('errors.not_found', thing: t('defaults.thing'))]}}, status: 404) else - render(json: {errors: {address: [t("errors.not_found", thing: t("defaults.thing"))]}}, status: 404) + respond_with @things end end @@ -19,7 +19,7 @@ def update end end - private +private def thing_params params.require(:thing).permit(:name, :user_id) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 23738ffc4..9463aff76 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,16 +1,15 @@ class UsersController < Devise::RegistrationsController def edit - render("sidebar/edit_profile", layout: "sidebar") + render('sidebar/edit_profile', layout: 'sidebar') end def update self.resource = resource_class.to_adapter.get!(send(:"current_#{resource_name}").to_key) - prev_unconfirmed_email = resource.unconfirmed_email if resource.respond_to?(:unconfirmed_email) if update_resource(resource, account_update_params) yield resource if block_given? sign_in(resource_name, resource, bypass: true) - flash[:notice] = "Profile updated!" - redirect_to(controller: "sidebar", action: "search") + flash[:notice] = 'Profile updated!' + redirect_to(controller: 'sidebar', action: 'search') else clean_up_passwords(resource) render(json: {errors: resource.errors}, status: 500) @@ -38,5 +37,4 @@ def sign_up_params def account_update_params params.require(:user).permit(:address_1, :address_2, :city, :current_password, :email, :name, :organization, :password, :password_confirmation, :remember_me, :sms_number, :state, :voice_number, :zip) end - end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 64405029d..8410d8495 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,5 +1,5 @@ module ApplicationHelper - def us_states + def us_states # rubocop:disable MethodLength [ ['Massachusetts', 'MA'], ['Alabama', 'AL'], diff --git a/app/mailers/thing_mailer.rb b/app/mailers/thing_mailer.rb index 3483172c2..abb6b9293 100644 --- a/app/mailers/thing_mailer.rb +++ b/app/mailers/thing_mailer.rb @@ -1,14 +1,9 @@ class ThingMailer < ActionMailer::Base - default from: "adoptahydrant@cityofboston.gov" + default from: 'adoptahydrant@cityofboston.gov' def reminder(thing) @thing = thing @user = thing.user - mail( - { - to: thing.user.email, - subject: ["Remember to shovel", thing.name].compact.join(' '), - } - ) + mail(to: thing.user.email, subject: ['Remember to shovel', thing.name].compact.join(' ')) end end diff --git a/app/models/address.rb b/app/models/address.rb index 582d364e5..087cb7e28 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -2,6 +2,6 @@ class Address include Geokit::Geocoders def self.geocode(address) - MultiGeocoder.geocode(address).ll.split(',').map{|s| s.to_f} + MultiGeocoder.geocode(address).ll.split(',').collect { |s| s.to_f } end end diff --git a/app/models/reminder.rb b/app/models/reminder.rb index 543ffa2b7..734c1ad27 100644 --- a/app/models/reminder.rb +++ b/app/models/reminder.rb @@ -1,7 +1,9 @@ class Reminder < ActiveRecord::Base include ActiveModel::ForbiddenAttributesProtection - validates_presence_of :from_user, :to_user, :thing - belongs_to :from_user, class_name: "User" - belongs_to :to_user, class_name: "User" + belongs_to :from_user, class_name: 'User' belongs_to :thing + belongs_to :to_user, class_name: 'User' + validates :from_user, presence: true + validates :thing, presence: true + validates :to_user, presence: true end diff --git a/app/models/thing.rb b/app/models/thing.rb index a24b8c9a0..62a10ecb9 100644 --- a/app/models/thing.rb +++ b/app/models/thing.rb @@ -2,12 +2,13 @@ class Thing < ActiveRecord::Base include ActiveModel::ForbiddenAttributesProtection - validates_uniqueness_of :city_id, allow_nil: true - validates_presence_of :lat, :lng belongs_to :user has_many :reminders + validates :city_id, uniqueness: true, allow_nil: true + validates :lat, presence: true + validates :lng, presence: true - def self.find_closest(lat, lng, limit=10) + def self.find_closest(lat, lng, limit = 10) query = <<-SQL SELECT *, (3959 * ACOS(COS(RADIANS(?)) * COS(RADIANS(lat)) * COS(RADIANS(lng) - RADIANS(?)) + SIN(RADIANS(?)) * SIN(RADIANS(lat)))) AS distance FROM things diff --git a/app/models/user.rb b/app/models/user.rb index aa217686e..dfee94abb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,21 +1,19 @@ class User < ActiveRecord::Base include ActiveModel::ForbiddenAttributesProtection - # Include default devise modules. Others available are: - # :token_authenticatable, :confirmable, - # :lockable, :timeoutable and :omniauthable devise :database_authenticatable, :registerable, :recoverable, :rememberable, - :trackable, :validatable + :trackable, :validatable + before_validation :remove_non_digits_from_phone_numbers + has_many :reminders_from, class_name: 'Reminder', foreign_key: 'from_user_id' + has_many :reminders_to, class_name: 'Reminder', foreign_key: 'to_user_id' + has_many :things + validates :name, presence: true validates_formatting_of :email, using: :email validates_formatting_of :sms_number, using: :us_phone, allow_blank: true validates_formatting_of :voice_number, using: :us_phone, allow_blank: true validates_formatting_of :zip, using: :us_zip, allow_blank: true - validates_presence_of :name - has_many :reminders_to, class_name: "Reminder", foreign_key: "to_user_id" - has_many :reminders_from, class_name: "Reminder", foreign_key: "from_user_id" - has_many :things - before_validation :remove_non_digits_from_phone_numbers + def remove_non_digits_from_phone_numbers - self.sms_number = self.sms_number.to_s.gsub(/\D/, '').to_i if self.sms_number.present? - self.voice_number = self.voice_number.to_s.gsub(/\D/, '').to_i if self.voice_number.present? + self.sms_number = sms_number.to_s.gsub(/\D/, '').to_i if sms_number.present? + self.voice_number = voice_number.to_s.gsub(/\D/, '').to_i if voice_number.present? end end diff --git a/config/boot.rb b/config/boot.rb index 359673666..5e5f0c1fa 100644 --- a/config/boot.rb +++ b/config/boot.rb @@ -1,4 +1,4 @@ # Set up gems listed in the Gemfile. ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__) -require 'bundler/setup' if File.exists?(ENV['BUNDLE_GEMFILE']) +require 'bundler/setup' if File.exist?(ENV['BUNDLE_GEMFILE']) diff --git a/config/environments/production.rb b/config/environments/production.rb index 4aba39c57..83caf1fe4 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -82,8 +82,8 @@ end ActionMailer::Base.smtp_settings = { - address: "smtp.sendgrid.net", - port: "25", + address: 'smtp.sendgrid.net', + port: '25', authentication: :plain, user_name: ENV['SENDGRID_USERNAME'], password: ENV['SENDGRID_PASSWORD'], diff --git a/config/environments/test.rb b/config/environments/test.rb index 3e1c938a2..f0bf79840 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -14,7 +14,7 @@ # Configure static asset server for tests with Cache-Control for performance. config.serve_static_assets = true - config.static_cache_control = "public, max-age=3600" + config.static_cache_control = 'public, max-age=3600' # Show full error reports and disable caching. config.consider_all_requests_local = true diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 303359c04..5275d449a 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -41,12 +41,12 @@ # Configure which authentication keys should be case-insensitive. # These keys will be downcased upon creating or modifying a user and when used # to authenticate or find a user. Default is :email. - config.case_insensitive_keys = [ :email ] + config.case_insensitive_keys = [:email] # Configure which authentication keys should have whitespace stripped. # These keys will have whitespace before and after removed upon creating or # modifying a user and when used to authenticate or find a user. Default is :email. - config.strip_whitespace_keys = [ :email ] + config.strip_whitespace_keys = [:email] # Tell if authentication through request.params is enabled. True by default. # It can be set to an array that will enable params authentication only for the @@ -95,7 +95,7 @@ config.stretches = Rails.env.test? ? 1 : 10 # Setup a pepper to generate the encrypted password. - config.pepper = "d0ce05a602094357144e8d2ce90258904f8cb26fb943cefd6fe0b824752616a9254fadabed3a47ba5c0de66a359513768ab1ab233d9cfef893f376a9b5ebcf68" + config.pepper = 'd0ce05a602094357144e8d2ce90258904f8cb26fb943cefd6fe0b824752616a9254fadabed3a47ba5c0de66a359513768ab1ab233d9cfef893f376a9b5ebcf68' # ==> Configuration for :confirmable # A period that the user is allowed to access the website even without diff --git a/config/initializers/rails_admin.rb b/config/initializers/rails_admin.rb index 5506dfb80..28e479316 100644 --- a/config/initializers/rails_admin.rb +++ b/config/initializers/rails_admin.rb @@ -1,5 +1,5 @@ RailsAdmin.config do |config| config.authenticate_with do - redirect_to(main_app.root_path, flash: {warning: "You must be signed-in as an administrator to access that page"}) unless signed_in? && current_user.admin? + redirect_to(main_app.root_path, flash: {warning: 'You must be signed-in as an administrator to access that page'}) unless signed_in? && current_user.admin? end end diff --git a/config/initializers/secret_token.rb b/config/initializers/secret_token.rb index 087e3967d..5964f4d01 100644 --- a/config/initializers/secret_token.rb +++ b/config/initializers/secret_token.rb @@ -8,7 +8,7 @@ # You can use `rake secret` to generate a secure secret key. if Rails.env.production? && ENV['SECRET_TOKEN'].blank? - raise 'The SECRET_TOKEN environment variable is not set.\n + fail 'The SECRET_TOKEN environment variable is not set.\n To generate it, run "rake secret", then set it with "heroku config:set SECRET_TOKEN=the_token_you_generated"' end diff --git a/db/migrate/00000000000002_add_devise_to_users.rb b/db/migrate/00000000000002_add_devise_to_users.rb index 079866b1b..aa2a83ddb 100644 --- a/db/migrate/00000000000002_add_devise_to_users.rb +++ b/db/migrate/00000000000002_add_devise_to_users.rb @@ -3,7 +3,7 @@ def up change_table(:users) do |t| ## Database authenticatable # t.string :email, null: false, default: "" - t.string :encrypted_password, null: false, default: "" + t.string :encrypted_password, null: false, default: '' ## Recoverable t.string :reset_password_token @@ -30,7 +30,6 @@ def up # t.string :unlock_token # Only if unlock strategy is :email or :both # t.datetime :locked_at - # Uncomment below if timestamps were not included in your original model. # t.timestamps end @@ -44,6 +43,6 @@ def up def down # By default, we don't want to make any assumption about how to roll back a migration when your # model already existed. Please edit below which fields you would like to remove in this migration. - raise ActiveRecord::IrreversibleMigration + fail ActiveRecord::IrreversibleMigration end end diff --git a/db/migrate/00000000000005_create_rails_admin_histories_table.rb b/db/migrate/00000000000005_create_rails_admin_histories_table.rb index 7c5412aa8..97ddaeccc 100644 --- a/db/migrate/00000000000005_create_rails_admin_histories_table.rb +++ b/db/migrate/00000000000005_create_rails_admin_histories_table.rb @@ -10,6 +10,6 @@ def change t.timestamps end - add_index(:rails_admin_histories, [:item, :table, :month, :year], name: 'index_rails_admin_histories' ) + add_index(:rails_admin_histories, [:item, :table, :month, :year], name: 'index_rails_admin_histories') end end diff --git a/test/controllers/addresses_controller_test.rb b/test/controllers/addresses_controller_test.rb index c1478e9fe..f98bb3693 100644 --- a/test/controllers/addresses_controller_test.rb +++ b/test/controllers/addresses_controller_test.rb @@ -2,19 +2,19 @@ class AddressesControllerTest < ActionController::TestCase test 'should return latitude and longitude for a valid address' do - stub_request(:get, "http://maps.google.com/maps/api/geocode/json"). - with(query: {address: "City Hall, Boston, MA", sensor: "false"}). + stub_request(:get, 'http://maps.google.com/maps/api/geocode/json'). + with(query: {address: 'City Hall, Boston, MA', sensor: 'false'}). to_return(body: File.read(File.expand_path('../../fixtures/city_hall.json', __FILE__))) - get :show, address: 'City Hall', city_state: "Boston, MA", format: 'json' + get :show, address: 'City Hall', city_state: 'Boston, MA', format: 'json' assert_not_nil assigns :address end test 'should return an error for an invalid address' do - stub_request(:get, "http://maps.google.com/maps/api/geocode/json"). - with(query: {address: ", ", sensor: "false"}). + stub_request(:get, 'http://maps.google.com/maps/api/geocode/json'). + with(query: {address: ', ', sensor: 'false'}). to_return(body: File.read(File.expand_path('../../fixtures/unknown_address.json', __FILE__))) - stub_request(:get, "http://geocoder.us/service/csv/geocode"). - with(query: {address: ", "}). + stub_request(:get, 'http://geocoder.us/service/csv/geocode'). + with(query: {address: ', '}). to_return(body: File.read(File.expand_path('../../fixtures/unknown_address.json', __FILE__))) get :show, address: '', city_state: '', format: 'json' assert_response :missing diff --git a/test/controllers/info_window_controller_test.rb b/test/controllers/info_window_controller_test.rb index 3b1f0fae5..c1deb8341 100644 --- a/test/controllers/info_window_controller_test.rb +++ b/test/controllers/info_window_controller_test.rb @@ -17,7 +17,7 @@ class InfoWindowControllerTest < ActionController::TestCase assert_template 'users/thank_you' assert_select 'h2', 'Thank you for adopting this hydrant!' assert_select 'form#abandon_form' do - assert_select '[action=?]', "/things" + assert_select '[action=?]', '/things' assert_select '[method=?]', 'post' end assert_select 'input[name="_method"]' do @@ -48,7 +48,7 @@ class InfoWindowControllerTest < ActionController::TestCase assert_template :adopt assert_select 'h2', 'Adopt this Hydrant' assert_select 'form#adoption_form' do - assert_select '[action=?]', "/things" + assert_select '[action=?]', '/things' assert_select '[method=?]', 'post' end assert_select 'input[name="_method"]' do diff --git a/test/controllers/main_controller_test.rb b/test/controllers/main_controller_test.rb index e80b292ab..0143cf42b 100644 --- a/test/controllers/main_controller_test.rb +++ b/test/controllers/main_controller_test.rb @@ -4,7 +4,7 @@ class MainControllerTest < ActionController::TestCase include Devise::TestHelpers setup do - request.env["devise.mapping"] = Devise.mappings[:user] + request.env['devise.mapping'] = Devise.mappings[:user] @user = users(:erik) end diff --git a/test/controllers/passwords_controller_test.rb b/test/controllers/passwords_controller_test.rb index 96ad9738f..76f2de988 100644 --- a/test/controllers/passwords_controller_test.rb +++ b/test/controllers/passwords_controller_test.rb @@ -3,7 +3,7 @@ class PasswordsControllerTest < ActionController::TestCase include Devise::TestHelpers setup do - request.env["devise.mapping"] = Devise.mappings[:user] + request.env['devise.mapping'] = Devise.mappings[:user] @user = users(:erik) end diff --git a/test/controllers/reminders_controller_test.rb b/test/controllers/reminders_controller_test.rb index 2b2a8de9e..2303e9c84 100644 --- a/test/controllers/reminders_controller_test.rb +++ b/test/controllers/reminders_controller_test.rb @@ -3,14 +3,14 @@ class RemindersControllerTest < ActionController::TestCase include Devise::TestHelpers setup do - request.env["devise.mapping"] = Devise.mappings[:user] + request.env['devise.mapping'] = Devise.mappings[:user] @thing = things(:thing_1) @dan = users(:dan) @user = users(:erik) @thing.user = @dan @thing.save! - stub_request(:get, "http://maps.google.com/maps/api/geocode/json"). - with(query: {latlng: "42.383339,-71.049226", sensor: "false"}). + stub_request(:get, 'http://maps.google.com/maps/api/geocode/json'). + with(query: {latlng: '42.383339,-71.049226', sensor: 'false'}). to_return(body: File.read(File.expand_path('../../fixtures/city_hall.json', __FILE__))) end diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 8da6a2876..f1458beea 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -3,7 +3,7 @@ class SessionsControllerTest < ActionController::TestCase include Devise::TestHelpers setup do - request.env["devise.mapping"] = Devise.mappings[:user] + request.env['devise.mapping'] = Devise.mappings[:user] @user = users(:erik) end @@ -31,7 +31,7 @@ class SessionsControllerTest < ActionController::TestCase test 'should empty session on sign out' do sign_in @user get :destroy, format: :json - assert_equal {}, session + assert session.empty? assert_response :success end end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index ad39b97a5..4ba256e75 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -3,7 +3,7 @@ class UsersControllerTest < ActionController::TestCase include Devise::TestHelpers setup do - request.env["devise.mapping"] = Devise.mappings[:user] + request.env['devise.mapping'] = Devise.mappings[:user] @user = users(:erik) end diff --git a/test/test_helper.rb b/test/test_helper.rb index 7922c92c6..3e7e38ef2 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,11 +1,11 @@ -ENV["RAILS_ENV"] = "test" +ENV['RAILS_ENV'] = 'test' require 'simplecov' require 'coveralls' SimpleCov.formatter = SimpleCov::Formatter::MultiFormatter[ - SimpleCov::Formatter::HTMLFormatter, - Coveralls::SimpleCov::Formatter + SimpleCov::Formatter::HTMLFormatter, + Coveralls::SimpleCov::Formatter, ] SimpleCov.start('rails') @@ -13,14 +13,16 @@ require 'rails/test_help' require 'webmock/minitest' -class ActiveSupport::TestCase - ActiveRecord::Migration.check_pending! +module ActiveSupport + class TestCase + ActiveRecord::Migration.check_pending! - # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. - # - # Note: You'll currently still have to declare fixtures explicitly in integration tests - # -- they do not yet inherit this setting - fixtures :all + # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. + # + # Note: You'll currently still have to declare fixtures explicitly in integration tests + # -- they do not yet inherit this setting + fixtures :all - # Add more helper methods to be used by all tests here... + # Add more helper methods to be used by all tests here... + end end