Merge pull request #34 from fusion94/rubocop

Set up Rubocop, apply autofixes, fix a few warnings manually

.rubocop.yml

@@ -0,0 +1,33 @@
+require:
+  - rubocop-performance
+  - rubocop-rails
+  - rubocop-rspec
+
+AllCops:
+  TargetRubyVersion: 2.5
+  Exclude:
+    - 'spec/dummy/**/*'
+
+Rails:
+  Enabled: true
+
+Style/HashEachMethods:
+  Enabled: true
+
+Style/HashTransformKeys:
+  Enabled: true
+
+Style/HashTransformValues:
+  Enabled: true
+
+Layout/LineLength:
+  Max: 120
+
+RSpec/NestedGroups:
+  Enabled: false
+
+RSpec/MultipleExpectations:
+  Enabled: false
+
+Metrics/BlockLength:
+  Enabled: false

Appraisals

@@ -1,29 +1,31 @@
+# frozen_string_literal: true
+
 {
-  "4.2.2" => %w[3.0 4.0 5.0 6.0 7.0 8.0],
-  "5.0.0" => %w[5.0 6.0 7.0 8.0],
-  "5.1.0" => %w[7.0 8.0 9.0 10.0],
-  "5.2.0" => %w[9.0 10.0],
-  "6.0.0" => %w[10.0],
+  '4.2.2' => %w[3.0 4.0 5.0 6.0 7.0 8.0],
+  '5.0.0' => %w[5.0 6.0 7.0 8.0],
+  '5.1.0' => %w[7.0 8.0 9.0 10.0],
+  '5.2.0' => %w[9.0 10.0],
+  '6.0.0' => %w[10.0]
 }.each do |rails_version, paper_trail_versions|
   paper_trail_versions.each do |paper_trail_version|
     appraise "rails-#{rails_version}-paper_trail-#{paper_trail_version}-will-paginate" do
-      gem "rails", "~> #{rails_version}"
-      gem "jquery-rails"
-      gem "sqlite3", rails_version == "6.0.0" ? "~> 1.4" : "~> 1.3.6"
-      gem "paper_trail", "~> #{paper_trail_version}"
-      gem "will_paginate", "~> 3.0"
+      gem 'rails', "~> #{rails_version}"
+      gem 'jquery-rails'
+      gem 'sqlite3', rails_version == '6.0.0' ? '~> 1.4' : '~> 1.3.6'
+      gem 'paper_trail', "~> #{paper_trail_version}"
+      gem 'will_paginate', '~> 3.0'
 
-      gem "webpacker" if rails_version == "6.0.0"
+      gem 'webpacker' if rails_version == '6.0.0'
     end
 
     appraise "rails-#{rails_version}-paper_trail-#{paper_trail_version}-kaminari" do
-      gem "rails", "~> #{rails_version}"
-      gem "jquery-rails"
-      gem "sqlite3", rails_version == "6.0.0" ? "~> 1.4" : "~> 1.3.6"
-      gem "paper_trail", "~> #{paper_trail_version}"
-      gem "kaminari", ">= 0.16"
+      gem 'rails', "~> #{rails_version}"
+      gem 'jquery-rails'
+      gem 'sqlite3', rails_version == '6.0.0' ? '~> 1.4' : '~> 1.3.6'
+      gem 'paper_trail', "~> #{paper_trail_version}"
+      gem 'kaminari', '>= 0.16'
 
-      gem "webpacker" if rails_version == "6.0.0"
+      gem 'webpacker' if rails_version == '6.0.0'
     end
   end
 end

Gemfile

@@ -1,5 +1,6 @@
+# frozen_string_literal: true
+
 source 'https://rubygems.org'
 
 # Specify your gem's dependencies in paper_trail_manager.gemspec
 gemspec
-

Rakefile

@@ -1,11 +1,11 @@
-# encoding: UTF-8
+# frozen_string_literal: true
 
 require 'rubygems'
 
-require "bundler/gem_tasks"
+require 'bundler/gem_tasks'
 Bundler::GemHelper.install_tasks
 
-app_rakefile_path = File.expand_path("../spec/dummy/Rakefile", __FILE__)
+app_rakefile_path = File.expand_path('spec/dummy/Rakefile', __dir__)
 
 if File.exist?(app_rakefile_path)
   APP_RAKEFILE = app_rakefile_path
@@ -17,7 +17,7 @@ require 'rdoc/task'
 
 require 'rspec/core/rake_task'
 RSpec::Core::RakeTask.new
-task :default => ["db:create", "db:migrate", "spec"]
+task default: ['db:create', 'db:migrate', 'spec']
 
 Rake::RDocTask.new(:rdoc) do |rdoc|
   rdoc.rdoc_dir = 'rdoc'

app/controllers/paper_trail_manager/changes_controller.rb

@@ -1,128 +1,131 @@
+# frozen_string_literal: true
+
 # Allow the parent class of ChangesController to be configured in the host app
 PaperTrailManager::ChangesController = Class.new(PaperTrailManager.base_controller.constantize)
 
-class PaperTrailManager::ChangesController
-  # Default number of changes to list on a pagenated index page.
-  PER_PAGE = 50
+class PaperTrailManager
+  class ChangesController
+    # Default number of changes to list on a pagenated index page.
+    PER_PAGE = 50
 
-  helper PaperTrailManager.route_helpers if PaperTrailManager.route_helpers
-  helper PaperTrailManager::ChangesHelper
-  layout PaperTrailManager.layout if PaperTrailManager.layout
+    helper PaperTrailManager.route_helpers if PaperTrailManager.route_helpers
+    helper PaperTrailManager::ChangesHelper
+    layout PaperTrailManager.layout if PaperTrailManager.layout
 
-  # List changes
-  def index
-    unless change_index_allowed?
-      flash[:error] = "You do not have permission to list changes."
-      return(redirect_to root_url)
-    end
+    # List changes
+    def index
+      unless change_index_allowed?
+        flash[:error] = 'You do not have permission to list changes.'
+        return(redirect_to root_url)
+      end
 
-    @versions = PaperTrail::Version.order('created_at DESC, id DESC')
-    if params[:type]
-      @versions = @versions.where(:item_type => params[:type])
-    end
-    if params[:id]
-      @versions = @versions.where(:item_id => params[:id])
-    end
+      @versions = PaperTrail::Version.order('created_at DESC, id DESC')
+      @versions = @versions.where(item_type: params[:type]) if params[:type]
+      @versions = @versions.where(item_id: params[:id]) if params[:id]
 
-    # Ensure pagination parameters have sensible values
-    @page = (v = params[:page].to_i; v == 0 ? nil : v)
-    @per_page = (v = params[:per_page].to_i; v == 0 ? PER_PAGE : v)
+      # Ensure pagination parameters have sensible values
+      @page = params[:page].to_i
+      @page = nil if @page.zero?
 
-    if defined?(WillPaginate)
-      @versions = @versions.paginate(:page => @page, :per_page => @per_page)
-    else
-      @versions = @versions.page(@page).per(@per_page)
-    end
+      @per_page = params[:per_page].to_i
+      @per_page = nil if @per_page.zero?
 
-    respond_to do |format|
-      format.html # index.html.erb
-      format.atom # index.atom.builder
-      format.json { render :json => @versions }
-    end
-  end
+      @versions = if defined?(WillPaginate)
+                    @versions.paginate(page: @page, per_page: @per_page)
+                  else
+                    @versions.page(@page).per(@per_page)
+                  end
 
-  # Show a change
-  def show
-    begin
-      @version = PaperTrail::Version.find(params[:id])
-    rescue ActiveRecord::RecordNotFound
-      flash[:error] = "No such version."
-      return(redirect_to :action => :index)
+      respond_to do |format|
+        format.html # index.html.erb
+        format.atom # index.atom.builder
+        format.json { render json: @versions }
+      end
     end
 
-    unless change_show_allowed?(@version)
-      flash[:error] = "You do not have permission to show that change."
-      return(redirect_to :action => :index)
-    end
+    # Show a change
+    def show
+      begin
+        @version = PaperTrail::Version.find(params[:id])
+      rescue ActiveRecord::RecordNotFound
+        flash[:error] = 'No such version.'
+        return(redirect_to action: :index)
+      end
 
-    respond_to do |format|
-      format.html # show.html.erb
-      format.json { render :json => @version }
-    end
-  end
+      unless change_show_allowed?(@version)
+        flash[:error] = 'You do not have permission to show that change.'
+        return(redirect_to action: :index)
+      end
 
-  # Rollback a change
-  def update
-    begin
-      @version = PaperTrail::Version.find(params[:id])
-    rescue ActiveRecord::RecordNotFound
-      flash[:error] = "No such version."
-      return(redirect_to(changes_path))
+      respond_to do |format|
+        format.html # show.html.erb
+        format.json { render json: @version }
+      end
     end
 
-    unless change_revert_allowed?(@version)
-      flash[:error] = "You do not have permission to revert this change."
-      return(redirect_to changes_path)
-    end
+    # Rollback a change
+    def update
+      begin
+        @version = PaperTrail::Version.find(params[:id])
+      rescue ActiveRecord::RecordNotFound
+        flash[:error] = 'No such version.'
+        return(redirect_to(changes_path))
+      end
 
-    if @version.event == "create"
-      @record = @version.item_type.constantize.find(@version.item_id)
-      @result = @record.destroy
-    else
-      @record = @version.reify
-      @result = @record.save
-    end
+      unless change_revert_allowed?(@version)
+        flash[:error] = 'You do not have permission to revert this change.'
+        return(redirect_to changes_path)
+      end
 
-    if @result
-      if @version.event == "create"
-        flash[:notice] = "Rolled back newly-created record by destroying it."
-        redirect_to changes_path
+      if @version.event == 'create'
+        @record = @version.item_type.constantize.find(@version.item_id)
+        @result = @record.destroy
       else
-        flash[:notice] = "Rolled back changes to this record."
-        redirect_to change_item_url(@version)
+        @record = @version.reify
+        @result = @record.save
+      end
+
+      if @result
+        if @version.event == 'create'
+          flash[:notice] = 'Rolled back newly-created record by destroying it.'
+          redirect_to changes_path
+        else
+          flash[:notice] = 'Rolled back changes to this record.'
+          redirect_to change_item_url(@version)
+        end
+      else
+        flash[:error] = "Couldn't rollback. Sorry."
+        redirect_to changes_path
       end
-    else
-      flash[:error] = "Couldn't rollback. Sorry."
-      redirect_to changes_path
     end
-  end
 
-protected
+    protected
 
-  # Return the URL for the item represented by the +version+, e.g. a Company record instance referenced by a version.
-  def change_item_url(version)
-    version_type = version.item_type.underscore.split('/').last
-    return send("#{version_type}_url", version.item_id)
-  rescue NoMethodError
-    return nil
-  end
-  helper_method :change_item_url
+    # Return the URL for the item represented by the +version+, e.g. a Company record instance referenced by a version.
+    def change_item_url(version)
+      version_type = version.item_type.underscore.split('/').last
+      send("#{version_type}_url", version.item_id)
+    rescue NoMethodError
+      nil
+    end
+    helper_method :change_item_url
 
-  # Allow index?
-  def change_index_allowed?
-    return PaperTrailManager.allow_index?(self)
-  end
-  helper_method :change_index_allowed?
+    # Allow index?
+    def change_index_allowed?
+      PaperTrailManager.allow_index?(self)
+    end
+    helper_method :change_index_allowed?
 
-  # Allow show?
-  def change_show_allowed?(version)
-    return PaperTrailManager.allow_show?(self, version)
-  end
-  helper_method :change_show_allowed?
+    # Allow show?
+    def change_show_allowed?(version)
+      PaperTrailManager.allow_show?(self, version)
+    end
+    helper_method :change_show_allowed?
 
-  # Allow revert?
-  def change_revert_allowed?(version)
-    return PaperTrailManager.allow_revert?(self, version)
+    # Allow revert?
+    def change_revert_allowed?(version)
+      PaperTrailManager.allow_revert?(self, version)
+    end
+    helper_method :change_revert_allowed?
   end
-  helper_method :change_revert_allowed?
 end