From b6219a60ada3f518d73e2389dd5ae28983edaaa3 Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Tue, 23 Aug 2016 10:19:50 +0300 Subject: [PATCH 01/12] Run phare as part of CI --- .travis.yml | 2 ++ bin/phare | 17 +++++++++++++++++ bin/rubocop | 17 +++++++++++++++++ jsonapi-authorization.gemspec | 2 ++ 4 files changed, 38 insertions(+) create mode 100755 bin/phare create mode 100755 bin/rubocop diff --git a/.travis.yml b/.travis.yml index 742483fa..72a34b0a 100644 --- 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 new file mode 100755 index 00000000..3edef8d9 --- /dev/null +++ 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 new file mode 100755 index 00000000..ccb4d563 --- /dev/null +++ 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 index be2fb589..c82a9653 100644 --- 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 From b1b4251c42a9fc936b71d75aef6096f2b5463567 Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:34:00 +0300 Subject: [PATCH 02/12] Update rubocop styles to match Venuu style --- .rubocop.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index ce7ac30d..41c5b6a5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,3 +1,8 @@ +AllCops: + Exclude: + - 'bin/*' + - 'spec/dummy/db/schema.rb' + Metrics/LineLength: Enabled: true Max: 100 @@ -5,6 +10,9 @@ Metrics/LineLength: Style/MultilineOperationIndentation: EnforcedStyle: indented +Style/MultilineMethodCallIndentation: + EnforcedStyle: indented + Metrics/ClassLength: Enabled: false @@ -37,7 +45,10 @@ Style/SpaceInsideHashLiteralBraces: EnforcedStyle: space Style/IndentHash: - Enabled: false + EnforcedStyle: consistent + +Style/IndentArray: + EnforcedStyle: consistent Style/ClassAndModuleChildren: Enabled: false @@ -86,3 +97,6 @@ Style/SingleLineBlockParams: - inject: - acc - obj + +Style/Alias: + EnforcedStyle: prefer_alias_method From 07fc7b224d454325fde7afe8bf8cd931682e23d7 Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:34:25 +0300 Subject: [PATCH 03/12] Autofix rubocop autofixable lint warnings --- .../authorization/default_pundit_authorizer.rb | 14 +++++++------- spec/dummy/config/application.rb | 2 +- spec/jsonapi/authorization/configuration_spec.rb | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 19fa7911..899b0bda 100644 --- 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 index ef8def28..1fca7381 100644 --- 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 index a5f2c785..35b16bc4 100644 --- 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 From 91fe60256a0fef3a322e2aac9c3469db98987202 Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:39:51 +0300 Subject: [PATCH 04/12] Ignore vendor/bundle and tmp dirs from rubocop --- .rubocop.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 41c5b6a5..199088da 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,6 +2,8 @@ AllCops: Exclude: - 'bin/*' - 'spec/dummy/db/schema.rb' + - 'vendor/bundle/**/*' + - 'tmp/**/*' Metrics/LineLength: Enabled: true From 2a103097268086e4c9f72d9ebcc3199b9287219e Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:42:58 +0300 Subject: [PATCH 05/12] Ignore Style/FileName lint warning for main import file --- lib/jsonapi-authorization.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/jsonapi-authorization.rb b/lib/jsonapi-authorization.rb index 76f149b8..4957205f 100644 --- a/lib/jsonapi-authorization.rb +++ b/lib/jsonapi-authorization.rb @@ -1 +1,2 @@ +# rubocop:disable Style/FileName require 'jsonapi/authorization' From 8a4663bc87f6fc295a7635ec9fa27c6ad0c0a6c8 Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:43:15 +0300 Subject: [PATCH 06/12] Fix lint warning for semicolon usage --- spec/requests/relationship_operations_spec.rb | 5 ++++- spec/requests/tricky_operations_spec.rb | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/spec/requests/relationship_operations_spec.rb b/spec/requests/relationship_operations_spec.rb index 1315a7bf..c63f597d 100644 --- 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 index e06eeea0..19bc353f 100644 --- 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 From 35bb62a1a51e30fa2c9e87da9aa405add9260c39 Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:44:34 +0300 Subject: [PATCH 07/12] Don't warn about line length in .gemspec file --- jsonapi-authorization.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/jsonapi-authorization.gemspec b/jsonapi-authorization.gemspec index c82a9653..416b27ba 100644 --- a/jsonapi-authorization.gemspec +++ b/jsonapi-authorization.gemspec @@ -1,4 +1,5 @@ # coding: utf-8 +# rubocop:disable Metrics/LineLength lib = File.expand_path('../lib', __FILE__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'jsonapi/authorization/version' From faad28ab1416e898cb28a6c53dce8c9a1705dfeb Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:47:49 +0300 Subject: [PATCH 08/12] Split long lines in AuthorizingProcessor --- .../authorization/authorizing_processor.rb | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index 616e540a..911d39f8 100644 --- 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( From 35573a15acddefa144655525b203104d4bd8d354 Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:48:04 +0300 Subject: [PATCH 09/12] Split long lines in pundit stubs spec helpers --- spec/support/pundit_stubs.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/support/pundit_stubs.rb b/spec/support/pundit_stubs.rb index 90608ce6..d3c8b31f 100644 --- 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 From 76c6965895e6be18f77260a96fe139964fcfd69d Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:48:16 +0300 Subject: [PATCH 10/12] 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. --- .rubocop.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 199088da..57242f77 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -8,6 +8,8 @@ AllCops: Metrics/LineLength: Enabled: true Max: 100 + Exclude: + - spec/requests/**/*.rb Style/MultilineOperationIndentation: EnforcedStyle: indented From 95857f0b5b22050383a4ef42172e067efe8a255c Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:50:18 +0300 Subject: [PATCH 11/12] Move LineLength disable from .gemspec file to rubocop conf --- .rubocop.yml | 1 + jsonapi-authorization.gemspec | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 57242f77..ff6c8e71 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -10,6 +10,7 @@ Metrics/LineLength: Max: 100 Exclude: - spec/requests/**/*.rb + - jsonapi-authorization.gemspec Style/MultilineOperationIndentation: EnforcedStyle: indented diff --git a/jsonapi-authorization.gemspec b/jsonapi-authorization.gemspec index 416b27ba..c82a9653 100644 --- a/jsonapi-authorization.gemspec +++ b/jsonapi-authorization.gemspec @@ -1,5 +1,4 @@ # coding: utf-8 -# rubocop:disable Metrics/LineLength lib = File.expand_path('../lib', __FILE__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'jsonapi/authorization/version' From 95328fe9e9f39953579af099b0860c2aeef3b103 Mon Sep 17 00:00:00 2001 From: Vesa Laakso Date: Sun, 18 Sep 2016 17:51:05 +0300 Subject: [PATCH 12/12] Move Style/FileName disable for main import to rubocop conf --- .rubocop.yml | 4 ++++ lib/jsonapi-authorization.rb | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index ff6c8e71..10c150f6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,6 +5,10 @@ AllCops: - 'vendor/bundle/**/*' - 'tmp/**/*' +Style/FileName: + Exclude: + - lib/jsonapi-authorization.rb + Metrics/LineLength: Enabled: true Max: 100 diff --git a/lib/jsonapi-authorization.rb b/lib/jsonapi-authorization.rb index 4957205f..76f149b8 100644 --- a/lib/jsonapi-authorization.rb +++ b/lib/jsonapi-authorization.rb @@ -1,2 +1 @@ -# rubocop:disable Style/FileName require 'jsonapi/authorization'