Vijay: Incorporating and running rubocop on the whole codebase.

nikhilsu · Jun 30, 2016 · df66fa5 · df66fa5
1 parent bad363e
commit df66fa5
Show file tree

Hide file tree

Showing 132 changed files with 927 additions and 885 deletions.
diff --git a/.ruby-version b/.ruby-version
@@ -1 +1 @@
-ruby-2.3.0
+2.3
diff --git a/Gemfile b/Gemfile
@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 source 'https://rubygems.org'
 
 ruby '2.3.0'
@@ -6,7 +7,7 @@ ruby '2.3.0'
 # TODO: Explicitly require some gems as false so that startup time is optimized in dev mode.
 gem 'rails',                   '4.2.5.2'
 gem 'bcrypt',                  '3.1.11'
-gem 'faker',                   '1.4.2'    # TODO: Shouldn't this only be required for non-prod (or more specifically only for test) envs?
+gem 'faker',                   '1.4.2' # TODO: Shouldn't this only be required for non-prod (or more specifically only for test) envs?
 gem 'carrierwave',             '0.10.0'
 gem 'mini_magick',             '3.8.0'
 gem 'fog',                     '1.36.0'
@@ -20,11 +21,11 @@ gem 'turbolinks',              '2.3.0'
 gem 'jbuilder',                '2.2.3'
 gem 'jc-validates_timeliness', '3.1.1'
 gem 'pg',                      '0.18.4'
-gem 'sdoc',                    '0.4.0', group: :doc   # TODO: Are we generating some kind of docs? If not, this is not required
+gem 'sdoc',                    '0.4.0', group: :doc # TODO: Are we generating some kind of docs? If not, this is not required
 gem 'net-ssh',                 '3.1.1'
 gem 'feature_toggle',          '0.0.3'
 gem 'ruby-saml',               '1.1.1'
-gem 'webmock',                 '2.1.0'    # TODO: Shouldn't this only be required for non-prod (or more specifically only for test) envs?
+gem 'webmock',                 '2.1.0' # TODO: Shouldn't this only be required for non-prod (or more specifically only for test) envs?
 gem 'bootstrap_form',          '2.3.0'
 gem 'json',                    '~> 1.7', '>= 1.7.7'
 gem 'selectize-rails',         '0.12.1'
@@ -60,8 +61,8 @@ group :development do
   gem 'rack-mini-profiler', '~> 0.9.8'
   gem 'binding_of_caller', '~> 0.7.2'
 
-  gem 'rubocop', '~> 0.37.2', require: false
-  gem 'rubocop-rspec', '~> 1.4', require: false
+  gem 'rubocop', '~> 0.41.1', require: false
+  gem 'rubocop-rspec', '~> 1.5', require: false
   # gem 'pronto', '~> 0.3.3'
 
   gem 'guard-bundler', '~> 2.1.0', require: false
@@ -89,7 +90,7 @@ group :development do
 end
 
 group :test do
-  gem 'simplecov',    '0.11.2'
+  gem 'simplecov', '0.11.2'
 end
 
 group :production, :staging do

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -472,13 +472,14 @@ GEM
       rspec-mocks (~> 3.4.0)
       rspec-support (~> 3.4.0)
     rspec-support (3.4.1)
-    rubocop (0.37.2)
-      parser (>= 2.3.0.4, < 3.0)
+    rubocop (0.41.1)
+      parser (>= 2.3.1.1, < 3.0)
       powerpack (~> 0.1)
       rainbow (>= 1.99.1, < 3.0)
       ruby-progressbar (~> 1.7)
-      unicode-display_width (~> 0.3)
-    rubocop-rspec (1.4.0)
+      unicode-display_width (~> 1.0, >= 1.0.1)
+    rubocop-rspec (1.5.0)
+      rubocop (>= 0.40.0)
     ruby-progressbar (1.8.1)
     ruby-saml (1.1.1)
       nokogiri (>= 1.5.10)
@@ -540,7 +541,7 @@ GEM
     uglifier (2.7.2)
       execjs (>= 0.3.0)
       json (>= 1.8.0)
-    unicode-display_width (0.3.1)
+    unicode-display_width (1.1.0)
     uniform_notifier (1.9.0)
     unparser (0.2.5)
       abstract_type (~> 0.0.7)
@@ -616,8 +617,8 @@ DEPENDENCIES
   rails_12factor (= 0.0.2)
   rspec-activemodel-mocks (= 1.0.3)
   rspec-rails (= 3.4.2)
-  rubocop (~> 0.37.2)
-  rubocop-rspec (~> 1.4)
+  rubocop (~> 0.41.1)
+  rubocop-rspec (~> 1.5)
   ruby-saml (= 1.1.1)
   sass-rails (= 5.0.2)
   sdoc (= 0.4.0)

diff --git a/Rakefile b/Rakefile
@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 # Add your own tasks in files placed in lib/tasks ending in .rake,
 # for example lib/tasks/capistrano.rake, and they will automatically be available to Rake.
 

diff --git a/app/controllers/admin/admin_controller.rb b/app/controllers/admin/admin_controller.rb
@@ -1,10 +1,10 @@
+# frozen_string_literal: true
 class Admin::AdminController < ApplicationController
-
   before_action :authorized_admin?
 
   def authorized_admin?
-    if current_user.nil? or current_user.role.name != 'admin'
+    if current_user.nil? || current_user.role.name != 'admin'
       render 'custom_errors/not_found_error'
     end
   end
-end
+end
diff --git a/app/controllers/admin/cabpools_controller.rb b/app/controllers/admin/cabpools_controller.rb
@@ -1,9 +1,10 @@
+# frozen_string_literal: true
 class Admin::CabpoolsController < Admin::AdminController
   before_action :company_provided_cabpool?, only: [:edit, :update]
 
   def show
     company_provided_cabpools = Cabpool.company_provided_cab
-    @cabpools = company_provided_cabpools.paginate(page: params[:page], :per_page => 10)
+    @cabpools = company_provided_cabpools.paginate(page: params[:page], per_page: 10)
   end
 
   def new
@@ -14,12 +15,12 @@ def create
     @cabpool = Cabpool.new(cabpool_params)
     user_ids = params[:passengers].nil? ? [] : params[:passengers].values
     locality_ids = params[:localities].nil? ? [] : params[:localities].values
-    association_of_cabpool = {localities: LocalityService.fetch_all_localities(locality_ids), users: UserService.fetch_all_users(user_ids)}
+    association_of_cabpool = { localities: LocalityService.fetch_all_localities(locality_ids), users: UserService.fetch_all_users(user_ids) }
     response = CabpoolService.persist(@cabpool, association_of_cabpool)
     if response.success?
       flash[:success] = 'Cabpool creation successful'
       MailService.send_emails_to_cabpool_members_when_admin_creates_a_pool @cabpool
-      redirect_to '/admin' and return
+      redirect_to('/admin') && return
     else
       flash[:danger] = response.message
       render 'admin/cabpools/new'
@@ -34,19 +35,18 @@ def update
     members_before_cabpool_update = get_members_before_cabpool_update
     user_ids = params[:passengers].nil? ? [] : params[:passengers].values
     locality_ids = params[:localities].nil? ? [] : params[:localities].values
-    associations_of_cabpool = {users: UserService.fetch_all_users(user_ids), localities: LocalityService.fetch_all_localities(locality_ids)}
+    associations_of_cabpool = { users: UserService.fetch_all_users(user_ids), localities: LocalityService.fetch_all_localities(locality_ids) }
     response = CabpoolService.persist(@cabpool, associations_of_cabpool)
     if response.success?
       MailService.send_email_to_cabpool_users_about_cabpool_update_by_admin(@cabpool, members_before_cabpool_update)
       flash[:success] = 'Cabpool has been Updated'
-      redirect_to '/admin' and return
+      redirect_to('/admin') && return
     else
       flash[:danger] = response.message
       render 'edit'
     end
   end
 
-
   def delete
     cabpool = Cabpool.find(params[:id])
     destroy cabpool
@@ -56,7 +56,7 @@ def delete
 
   private
 
-  def destroy cabpool
+  def destroy(cabpool)
     cabpool.users.clear
     cabpool.requests.clear
     cabpool.destroy!
@@ -73,14 +73,14 @@ def get_members_before_cabpool_update
     @cabpool.users.each do |user|
       members << user
     end
-    return members
+    members
   end
 
   def company_provided_cabpool?
     @cabpool = Cabpool.find_by_id(params[:id])
-    if !@cabpool.company_provided_cab?
+    unless @cabpool.company_provided_cab?
       flash[:danger] = 'Cannot Edit a Non-Company Provided Cabpool'
       redirect_to '/admin'
     end
   end
-end
+end
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
@@ -1,5 +1,5 @@
+# frozen_string_literal: true
 class Admin::UsersController < Admin::AdminController
-
   def new
     @user = User.new
     @localities = Locality.all.order(:name) << Locality.new(id: -1, name: 'Other')
@@ -8,7 +8,7 @@ def new
   def create
     @user = User.new(user_params)
     if @user.save
-      flash[:success] = "The Profile has been updated"
+      flash[:success] = 'The Profile has been updated'
       redirect_to admin_path
     else
       @localities = Locality.all.order(:name) << Locality.new(id: -1, name: 'Other')
@@ -17,11 +17,11 @@ def create
   end
 
   private
+
   def user_params
     allowed_params = params.require(:user).permit(:emp_id, :address, :phone_no, :name, :email)
     locality_id = params[:user][:locality]
     locality = Locality.find_by_id(locality_id)
     allowed_params.merge(locality: locality)
   end
-
-end
+end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
@@ -1,11 +1,12 @@
+# frozen_string_literal: true
 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
   include SessionsHelper
 
   if FEATURES.active?('okta_feature')
-    before_action :authorized? , except: [:init, :consume, :approve_reject_handler]
+    before_action :authorized?, except: [:init, :consume, :approve_reject_handler]
   end
   before_action :set_user_name, except: [:init, :consume]
   before_filter :check_feature_activated?

diff --git a/app/controllers/cabpools_controller.rb b/app/controllers/cabpools_controller.rb
@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 class CabpoolsController < ApplicationController
   include CabpoolsHelper
   before_action :registered?, except: [:show, :approve_reject_handler]
@@ -8,7 +9,7 @@ class CabpoolsController < ApplicationController
 
   def user_should_not_have_cabpool
     user = current_user
-    if !user.nil?
+    unless user.nil?
       if user.cabpool
         flash[:danger] = 'You are already part of a Cab pool. Please leave the cabpool to create a new cab pool.'
         redirect_to your_cabpools_path
@@ -28,7 +29,7 @@ def create
       redirect_to root_path
     else
       locality_ids = params[:localities].nil? ? [] : params[:localities].values
-      associations_of_the_cabpool = {localities: LocalityService.fetch_all_localities(locality_ids), users: [current_user]}
+      associations_of_the_cabpool = { localities: LocalityService.fetch_all_localities(locality_ids), users: [current_user] }
       response = CabpoolService.persist(@cabpool, associations_of_the_cabpool)
       if response.success?
         flash[:success] = 'You have successfully created your cab pool.'
@@ -46,12 +47,12 @@ def edit
   def update
     @cabpool.attributes = params.require(:cabpool).permit(:number_of_people, :remarks, :timein, :timeout, :route)
     locality_ids = params[:localities].nil? ? [] : params[:localities].values
-    associations_of_the_cabpool = {localities: LocalityService.fetch_all_localities(locality_ids)}
+    associations_of_the_cabpool = { localities: LocalityService.fetch_all_localities(locality_ids) }
     response = CabpoolService.persist(@cabpool, associations_of_the_cabpool)
     if response.success?
       MailService.send_email_to_cabpool_members_about_cabpool_update(@cabpool, current_user)
       flash[:success] = 'Your Cabpool has been Updated'
-      redirect_to your_cabpools_path and return
+      redirect_to(your_cabpools_path) && return
     else
       flash[:danger] = 'Cannot update because of the following errors'
       render 'edit'
@@ -60,10 +61,10 @@ def update
 
   def show
     response = CabpoolService.fetch_all_cabpools_of_a_particular_locality(params[:localities])
-    flash[:danger] = response.message if !response.success?
+    flash[:danger] = response.message unless response.success?
     cabpools_to_render = remove_current_users_cabpool(response.data)
     sorted_cabpools = sort_by_available_seats_in_cabpool cabpools_to_render
-    @cabpools = sorted_cabpools.paginate(page: params[:page], :per_page => 10)
+    @cabpools = sorted_cabpools.paginate(page: params[:page], per_page: 10)
   end
 
   def join
@@ -80,13 +81,12 @@ def join
     redirect_to root_path
   end
 
-
   def leave
     current_user_cabpool = current_user.cabpool
     current_user.cabpool = nil
     current_user.save
     users = current_user_cabpool.users
-    if current_user_cabpool.users.size == 0
+    if current_user_cabpool.users.empty?
       current_user_cabpool.destroy
     elsif current_user_cabpool.users.size == 1
       MailService.send_email_to_admin_about_invalid_cabpool(current_user_cabpool)
@@ -99,7 +99,6 @@ def leave
   end
 
   def your_cabpools
-
   end
 
   def approve_reject_handler
@@ -131,7 +130,7 @@ def approve_via_notification
     current_user = User.find_by_email(session[:Email])
     requested_user = User.find(requested_users_id)
     request = Request.find_by(user_id: requested_users_id, cabpool_id: current_user.cabpool.id)
-    if (current_user.cabpool.requested_users.exists?(:id => requested_users_id))
+    if current_user.cabpool.requested_users.exists?(id: requested_users_id)
       approve_user requested_user, request, current_user
     else
       render 'request_invalid'
@@ -143,7 +142,7 @@ def reject_via_notification
     current_user = User.find_by_email(session[:Email])
     requested_user = User.find(requested_users_id)
     request = Request.find_by(user_id: requested_users_id, cabpool_id: current_user.cabpool.id)
-    if (current_user.cabpool.requested_users.exists?(:id => requested_users_id))
+    if current_user.cabpool.requested_users.exists?(id: requested_users_id)
       reject_user requested_user, request
     else
       render 'request_invalid'
@@ -166,8 +165,9 @@ def view_notification
   end
 
   private
+
   def approve_user(user, request, approver)
-    if !user.cabpool.nil?
+    unless user.cabpool.nil?
       if user.cabpool.users.length > 1
         MailService.send_email_to_cabpool_users_on_member_leaving(user.cabpool.users.reject { |u| u.id == user.id }, user)
       else
@@ -177,15 +177,13 @@ def approve_user(user, request, approver)
     user.cabpool = request.cabpool
     user.status = 'approved'
     user.save
-    request.user.requests.each do |req|
-      req.destroy!
-    end
+    request.user.requests.each(&:destroy!)
     MailService.send_member_addition_email_to_cabpool_members(approver, user)
     MailService.send_email_to_approved_user user
     render 'request_accept'
   end
 
-  def destroy cabpool
+  def destroy(cabpool)
     cabpool.users.clear
     cabpool.requests.clear
     cabpool.destroy!
@@ -201,15 +199,15 @@ def reject_user(user, request)
 
   def registered?
     store_location
-    if !is_registered?
+    unless is_registered?
       flash[:danger] = 'Please update your profile to create a new cab pool.'
       redirect_to new_user_path
     end
   end
 
   def cabpool_params
     allowed_params = params.require(:cabpool).permit(:number_of_people, :timein, :timeout, :route, :remarks)
-    _, cabpool_type_id = params[:cabpool_type].first if !params[:cabpool_type].nil?
+    _, cabpool_type_id = params[:cabpool_type].first unless params[:cabpool_type].nil?
     cabpool_type = get_cabpool_type_by_id(cabpool_type_id)
     allowed_params.merge!(cabpool_type: cabpool_type)
   end
@@ -230,9 +228,9 @@ def not_company_provided_cabpool?
 
   def cabpool_exists_and_user_part_of_it?
     @cabpool = Cabpool.find_by_id(params[:id])
-    if @cabpool.nil? or !@cabpool.user_is_part_of_cabpool? current_user
+    if @cabpool.nil? || !@cabpool.user_is_part_of_cabpool?(current_user)
       flash[:danger] = 'Cannot Edit a cabpool that you are not part of'
       redirect_to root_url
     end
   end
-end
+end
diff --git a/app/controllers/custom_errors_controller.rb b/app/controllers/custom_errors_controller.rb
@@ -1,3 +1,4 @@
+# frozen_string_literal: true
 class CustomErrorsController < ApplicationController
   def page_not_found
     render 'not_found_error', status: 404