Setup CI to fail builds on lint failures (#34)

* Run phare as part of CI * Update rubocop styles to match Venuu style * Autofix rubocop autofixable lint warnings * Ignore vendor/bundle and tmp dirs from rubocop * Ignore Style/FileName lint warning for main import file * Fix lint warning for semicolon usage * Don't warn about line length in .gemspec file * Split long lines in AuthorizingProcessor * Split long lines in pundit stubs spec helpers * Exclude request specs for line length lint for now The authorization stubs and checks are quite long and we might want to think about creating a better way of writing them instead of trying to add unnecessary line length by splitting them on multiple lines. * Move LineLength disable from .gemspec file to rubocop conf * Move Style/FileName disable for main import to rubocop conf
venuu · Sep 19, 2016 · ae8ddad · ae8ddad
1 parent 51a65b7
commit ae8ddad
Show file tree

Hide file tree

Showing 12 changed files with 100 additions and 23 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -1,10 +1,27 @@
+AllCops:
+  Exclude:
+    - 'bin/*'
+    - 'spec/dummy/db/schema.rb'
+    - 'vendor/bundle/**/*'
+    - 'tmp/**/*'
+
+Style/FileName:
+  Exclude:
+    - lib/jsonapi-authorization.rb
+
 Metrics/LineLength:
   Enabled: true
   Max: 100
+  Exclude:
+    - spec/requests/**/*.rb
+    - jsonapi-authorization.gemspec
 
 Style/MultilineOperationIndentation:
   EnforcedStyle: indented
 
+Style/MultilineMethodCallIndentation:
+  EnforcedStyle: indented
+
 Metrics/ClassLength:
   Enabled: false
 
@@ -37,7 +54,10 @@ Style/SpaceInsideHashLiteralBraces:
   EnforcedStyle: space
 
 Style/IndentHash:
-  Enabled: false
+  EnforcedStyle: consistent
+
+Style/IndentArray:
+  EnforcedStyle: consistent
 
 Style/ClassAndModuleChildren:
   Enabled: false
@@ -86,3 +106,6 @@ Style/SingleLineBlockParams:
     - inject:
         - acc
         - obj
+
+Style/Alias:
+  EnforcedStyle: prefer_alias_method
diff --git a/.travis.yml b/.travis.yml
@@ -15,3 +15,5 @@ matrix:
   allow_failures:
     - env: JSONAPI_RESOURCES_VERSION=master RAILS_VERSION=4.2.0
     - env: JSONAPI_RESOURCES_VERSION=master RAILS_VERSION=4.1.0
+script:
+  - ./bin/phare
diff --git a/bin/phare b/bin/phare
@@ -0,0 +1,17 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+#
+# This file was generated by Bundler.
+#
+# The application 'phare' is installed as part of a gem, and
+# this file is here to facilitate running it.
+#
+
+require "pathname"
+ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile",
+  Pathname.new(__FILE__).realpath)
+
+require "rubygems"
+require "bundler/setup"
+
+load Gem.bin_path("phare", "phare")
diff --git a/bin/rubocop b/bin/rubocop
@@ -0,0 +1,17 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+#
+# This file was generated by Bundler.
+#
+# The application 'rubocop' is installed as part of a gem, and
+# this file is here to facilitate running it.
+#
+
+require "pathname"
+ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile",
+  Pathname.new(__FILE__).realpath)
+
+require "rubygems"
+require "bundler/setup"
+
+load Gem.bin_path("rubocop", "rubocop")
diff --git a/jsonapi-authorization.gemspec b/jsonapi-authorization.gemspec
@@ -28,4 +28,6 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency "pry-byebug", "~> 1.3"
   spec.add_development_dependency "pry-doc", "~> 0.6"
   spec.add_development_dependency "pry-rails", "~> 0.3.4"
+  spec.add_development_dependency "rubocop", "~> 0.36.0"
+  spec.add_development_dependency "phare", "~> 0.7.1"
 end
diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb
@@ -135,10 +135,13 @@ def authorize_replace_to_one_relationship
 
         old_related_record = source_resource.records_for(params[:relationship_type].to_sym)
         unless params[:key_value].nil?
-          new_related_resource = @resource_klass._relationship(params[:relationship_type].to_sym).resource_klass.find_by_key(
-            params[:key_value],
-            context: context
-          )
+          new_related_resource = @resource_klass
+            ._relationship(params[:relationship_type].to_sym)
+            .resource_klass
+            .find_by_key(
+              params[:key_value],
+              context: context
+            )
           new_related_record = new_related_resource._model unless new_related_resource.nil?
         end
 
@@ -183,10 +186,13 @@ def authorize_remove_to_many_relationship
         )
         source_record = source_resource._model
 
-        related_resource = @resource_klass._relationship(params[:relationship_type].to_sym).resource_klass.find_by_key(
-          params[:associated_key],
-          context: context
-        )
+        related_resource = @resource_klass
+          ._relationship(params[:relationship_type].to_sym)
+          .resource_klass
+          .find_by_key(
+            params[:associated_key],
+            context: context
+          )
         related_record = related_resource._model unless related_resource.nil?
 
         authorizer.remove_to_many_relationship(

diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb
@@ -134,7 +134,7 @@ def remove_resource(source_record)
       # * +old_related_record+ - The current associated record
       # * +new_related_record+ - The new record replacing the +old_record+
       #   association, or +nil+ if the association is to be cleared
-      def replace_to_one_relationship(source_record, old_related_record, new_related_record)
+      def replace_to_one_relationship(_source_record, _old_related_record, _new_related_record)
         raise NotImplementedError
       end
 
@@ -146,7 +146,7 @@ def replace_to_one_relationship(source_record, old_related_record, new_related_r
       #
       # * +source_record+ - The record whose relationship is modified
       # * +new_related_records+ - The new records to be added to the association
-      def create_to_many_relationship(source_record, new_related_records)
+      def create_to_many_relationship(_source_record, _new_related_records)
         raise NotImplementedError
       end
 
@@ -161,7 +161,7 @@ def create_to_many_relationship(source_record, new_related_records)
       #   association
       #--
       # TODO: Should probably take old records as well
-      def replace_to_many_relationship(source_record, new_related_records)
+      def replace_to_many_relationship(_source_record, _new_related_records)
         raise NotImplementedError
       end
 
@@ -175,7 +175,7 @@ def replace_to_many_relationship(source_record, new_related_records)
       #
       # * +source_record+ - The record whose relationship is modified
       # * +related_record+ - The record which will be deassociatied from +source_record+
-      def remove_to_many_relationship(source_record, related_record)
+      def remove_to_many_relationship(_source_record, _related_record)
         raise NotImplementedError
       end
 
@@ -187,7 +187,7 @@ def remove_to_many_relationship(source_record, related_record)
       #
       # * +source_record+ - The record whose relationship is modified
       # * +related_record+ - The record which will be deassociatied from +source_record+
-      def remove_to_one_relationship(source_record, related_record)
+      def remove_to_one_relationship(_source_record, _related_record)
         raise NotImplementedError
       end
 
@@ -206,7 +206,7 @@ def remove_to_one_relationship(source_record, related_record)
       #                     article.comments check
       # * +record_class+ - The underlying record class for the relationships
       #                    resource.
-      def include_has_many_resource(source_record, record_class)
+      def include_has_many_resource(_source_record, record_class)
         ::Pundit.authorize(user, record_class, 'index?')
       end
 
@@ -221,7 +221,7 @@ def include_has_many_resource(source_record, record_class)
       # * +source_record+ — The source relationship record, e.g. an Article in
       #                     article.author check
       # * +related_record+ - The associated record to return
-      def include_has_one_resource(source_record, related_record)
+      def include_has_one_resource(_source_record, related_record)
         ::Pundit.authorize(user, related_record, 'show?')
       end
     end

diff --git a/spec/dummy/config/application.rb b/spec/dummy/config/application.rb
@@ -22,7 +22,7 @@ class Application < Rails::Application
 
   config.middleware.delete "Rack::Lock"
   config.middleware.delete "ActionDispatch::Flash"
-  #config.middleware.delete "ActionDispatch::BestStandardsSupport"
+  # config.middleware.delete "ActionDispatch::BestStandardsSupport"
 
   config.secret_key_base = "correct-horse-battery-staple"
 end

diff --git a/spec/jsonapi/authorization/configuration_spec.rb b/spec/jsonapi/authorization/configuration_spec.rb
@@ -20,13 +20,13 @@
 
     context "given a proc" do
       it "returns the 'user'" do
-        JSONAPI::Authorization.configuration.pundit_user = ->(context){ context[:current_user] }
+        JSONAPI::Authorization.configuration.pundit_user = ->(context) { context[:current_user] }
 
         user = User.new
         jsonapi_context = { current_user: user }
         user_context = JSONAPI::Authorization.configuration.user_context(jsonapi_context)
 
-        expect(user_context).to be user        
+        expect(user_context).to be user
       end
     end
   end

diff --git a/spec/requests/relationship_operations_spec.rb b/spec/requests/relationship_operations_spec.rb
@@ -164,7 +164,10 @@
           allow_operation('replace_to_many_relationship', article, [])
         end
 
-        it { pending 'TODO: Maybe this actually should be succesful?'; is_expected.to be_not_found }
+        it do
+          pending 'TODO: Maybe this actually should be succesful?'
+          is_expected.to be_not_found
+        end
       end
 
       # If this happens in real life, it's mostly a bug. We want to document the

diff --git a/spec/requests/tricky_operations_spec.rb b/spec/requests/tricky_operations_spec.rb
@@ -128,7 +128,10 @@
         let(:comments_policy_scope) { Comment.where("id NOT IN (?)", new_comments.map(&:id)) }
         before { allow_operation('replace_fields', article, new_comments) }
 
-        it { pending 'DISCUSS: Should this error out somehow?'; is_expected.to be_not_found }
+        it do
+          pending 'DISCUSS: Should this error out somehow?'
+          is_expected.to be_not_found
+        end
       end
     end
 

diff --git a/spec/support/pundit_stubs.rb b/spec/support/pundit_stubs.rb
@@ -1,11 +1,15 @@
 module PunditStubs
   def allow_action(action, record)
     policy = ::Pundit::PolicyFinder.new(record).policy
-    allow(policy).to receive(:new).with(any_args, record) { instance_double(policy, action => true) }
+    allow(policy).to(
+      receive(:new).with(any_args, record) { instance_double(policy, action => true) }
+    )
   end
 
   def disallow_action(action, record)
     policy = ::Pundit::PolicyFinder.new(record).policy
-    allow(policy).to receive(:new).with(any_args, record) { instance_double(policy, action => false) }
+    allow(policy).to(
+      receive(:new).with(any_args, record) { instance_double(policy, action => false) }
+    )
   end
 end