Merge branch 'release/3.1.0'

.github/FUNDING.yml

@@ -0,0 +1,12 @@
+# These are supported funding model platforms
+
+github: [coorasse]
+patreon: # Replace with a single Patreon username
+open_collective: # Replace with a single Open Collective username
+ko_fi: # Replace with a single Ko-fi username
+tidelift: # Replace with a single Tidelift platform-name/package-name e.g., npm/babel
+community_bridge: # Replace with a single Community Bridge project-name e.g., cloud-foundry
+liberapay: # Replace with a single Liberapay username
+issuehunt: # Replace with a single IssueHunt username
+otechie: # Replace with a single Otechie username
+custom: # Replace with up to 4 custom sponsorship URLs e.g., ['link1', 'link2']

.travis.yml

@@ -8,6 +8,7 @@ rvm:
   - 2.4.2
   - 2.5.1
   - 2.6.3
+  - 2.7.0
   - ruby-head
   - jruby-9.1.17.0
   - jruby-9.2.7.0
@@ -32,6 +33,8 @@ matrix:
     gemfile: gemfiles/activerecord_6.0.0.gemfile
   - rvm: 2.4.2
     gemfile: gemfiles/activerecord_6.0.0.gemfile
+  - rvm: 2.7.0
+    gemfile: gemfiles/activerecord_4.2.0.gemfile
   - rvm: jruby-9.1.17.0
     gemfile: gemfiles/activerecord_5.0.2.gemfile
   - rvm: jruby-9.1.17.0

CHANGELOG.md

@@ -1,3 +1,12 @@
+## Unreleased
+
+* [#605](https://github.com/CanCanCommunity/cancancan/pull/605): Generate inner queries instead of join+distinct. ([@fsateler][])
+* [#608](https://github.com/CanCanCommunity/cancancan/pull/608): Spec for json column regression. ([@aleksejleonov][])
+* [#571](https://github.com/CanCanCommunity/cancancan/pull/571): Allows to check ability even the object implements `#to_a`. ([@mtsmfm][])
+* [#612](https://github.com/CanCanCommunity/cancancan/pull/612): Suppress keyword arguments warning for Ruby 2.7.0. ([@koic][])
+* [#569](https://github.com/CanCanCommunity/cancancan/pull/569): Fix accessible_by fires query for rules using association as condition. ([@albb0920][])
+* [#594](https://github.com/CanCanCommunity/cancancan/pull/594): Support translation of action name. ([@ayumu838][])
+
 ## 3.0.2
 
 * [#590](https://github.com/CanCanCommunity/cancancan/pull/590): Fix Rule#inspect when rule is created through a SQL array. ([@frostblooded][])
@@ -650,3 +659,9 @@ Please read the [guide on migrating from CanCanCan 2.x to 3.0](https://github.co
 [@kaspernj]: https://github.com/kaspernj
 [@frostblooded]: https://github.com/frostblooded
 [@eloyesp]: https://github.com/eloyesp
+[@mtsmfm]: https://github.com/mtsmfm
+[@koic]: https://github.com/koic
+[@fsateler]: https://github.com/fsateler
+[@aleksejleonov]: https://github.com/aleksejleonov
+[@albb0920]: https://github.com/albb0920
+[@ayumu838]: https://github.com/ayumu838

README.md

@@ -264,8 +264,7 @@ When first developing, you need to run `bundle install` and then `appraisal inst
 
 You can then run all appraisal files (like CI does), with `appraisal rake` or just run a specific set `appraisal activerecord_5.0 rake`.
 
-See the [CONTRIBUTING](https://github.com/CanCanCommunity/cancancan/blob/develop/CONTRIBUTING.md) and
-[spec/README](https://github.com/CanCanCommunity/cancancan/blob/master/spec/README.rdoc) for more information.
+See the [CONTRIBUTING](https://github.com/CanCanCommunity/cancancan/blob/develop/CONTRIBUTING.md) for more information.
 
 
 ## Special Thanks

lib/cancan/conditions_matcher.rb

@@ -97,7 +97,10 @@ def model_adapter(subject)
     end
 
     def conditions_empty?
-      @conditions == {} || @conditions.nil?
+      # @conditions might be an ActiveRecord::Associations::CollectionProxy
+      # which it's `==` implementation will fetch all records for comparison
+
+      (@conditions.is_a?(Hash) && @conditions == {}) || @conditions.nil?
     end
   end
 end

lib/cancan/model_adapters/active_record_5_adapter.rb

@@ -22,9 +22,14 @@ def self.matches_condition?(subject, name, value)
       private
 
       def build_relation(*where_conditions)
-        relation = @model_class.where(*where_conditions)
-        relation = relation.left_joins(joins).distinct if joins.present?
-        relation
+        if joins.present?
+          inner = @model_class.unscoped do
+            @model_class.left_joins(joins).where(*where_conditions)
+          end
+          @model_class.where(@model_class.primary_key => inner)
+        else
+          @model_class.where(*where_conditions)
+        end
       end
 
       # Rails 4.2 deprecates `sanitize_sql_hash_for_conditions`

lib/cancan/relevant.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module CanCan
+  module Relevant
+    # Matches both the action, subject, and attribute, not necessarily the conditions
+    def relevant?(action, subject)
+      subject = subject.values.first if subject.class == Hash
+      @match_all || (matches_action?(action) && matches_subject?(subject))
+    end
+
+    private
+
+    def matches_action?(action)
+      @expanded_actions.include?(:manage) || @expanded_actions.include?(action)
+    end
+
+    def matches_subject?(subject)
+      @subjects.include?(:all) || @subjects.include?(subject) || matches_subject_class?(subject)
+    end
+
+    def matches_subject_class?(subject)
+      @subjects.any? do |sub|
+        sub.is_a?(Module) && (subject.is_a?(sub) ||
+            subject.class.to_s == sub.to_s ||
+            (subject.is_a?(Module) && subject.ancestors.include?(sub)))
+      end
+    end
+  end
+end

lib/cancan/rule.rb

@@ -1,12 +1,15 @@
 # frozen_string_literal: true
 
 require_relative 'conditions_matcher.rb'
+require_relative 'relevant.rb'
+
 module CanCan
   # This class is used internally and should only be called through Ability.
   # it holds the information about a "can" call made on Ability and provides
   # helpful methods to determine permission checking and conditions hash generation.
   class Rule # :nodoc:
     include ConditionsMatcher
+    include Relevant
     include ParameterValidators
     attr_reader :base_behavior, :subjects, :actions, :conditions, :attributes
     attr_writer :expanded_actions, :conditions
@@ -24,9 +27,9 @@ def initialize(base_behavior, action, subject, *extra_args, &block)
       raise Error, "Subject is required for #{action}" if action && subject.nil?
 
       @base_behavior = base_behavior
-      @actions = Array(action)
-      @subjects = Array(subject)
-      @attributes = Array(attributes)
+      @actions = wrap(action)
+      @subjects = wrap(subject)
+      @attributes = wrap(attributes)
       @conditions = extra_args || {}
       @block = block
     end
@@ -57,12 +60,6 @@ def catch_all?
         (!with_scope? && [nil, false, [], {}, '', ' '].include?(@conditions))
     end
 
-    # Matches both the action, subject, and attribute, not necessarily the conditions
-    def relevant?(action, subject)
-      subject = subject.values.first if subject.class == Hash
-      @match_all || (matches_action?(action) && matches_subject?(subject))
-    end
-
     def only_block?
       conditions_empty? && @block
     end
@@ -104,22 +101,6 @@ def matches_attributes?(attribute)
 
     private
 
-    def matches_action?(action)
-      @expanded_actions.include?(:manage) || @expanded_actions.include?(action)
-    end
-
-    def matches_subject?(subject)
-      @subjects.include?(:all) || @subjects.include?(subject) || matches_subject_class?(subject)
-    end
-
-    def matches_subject_class?(subject)
-      @subjects.any? do |sub|
-        sub.is_a?(Module) && (subject.is_a?(sub) ||
-          subject.class.to_s == sub.to_s ||
-          (subject.is_a?(Module) && subject.ancestors.include?(sub)))
-      end
-    end
-
     def parse_attributes_from_extra_args(args)
       attributes = args.shift if valid_attribute_param?(args.first)
       extra_args = args.shift
@@ -132,5 +113,15 @@ def condition_and_block_check(conditions, block, action, subject)
       raise BlockAndConditionsError, 'A hash of conditions is mutually exclusive with a block. '\
         "Check \":#{action} #{subject}\" ability."
     end
+
+    def wrap(object)
+      if object.nil?
+        []
+      elsif object.respond_to?(:to_ary)
+        object.to_ary || [object]
+      else
+        [object]
+      end
+    end
   end
 end

lib/cancan/unauthorized_message_resolver.rb

@@ -3,10 +3,12 @@
 module CanCan
   module UnauthorizedMessageResolver
     def unauthorized_message(action, subject)
+      subject = subject.values.last if subject.is_a?(Hash)
       keys = unauthorized_message_keys(action, subject)
-      variables = { action: action.to_s }
+      variables = {}
+      variables[:action] = I18n.translate("actions.#{action}", default: action.to_s)
       variables[:subject] = translate_subject(subject)
-      message = I18n.translate(keys.shift, variables.merge(scope: :unauthorized, default: keys + ['']))
+      message = I18n.translate(keys.shift, **variables.merge(scope: :unauthorized, default: keys + ['']))
       message.blank? ? nil : message
     end
 

lib/cancan/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module CanCan
-  VERSION = '3.0.2'.freeze
+  VERSION = '3.1.0'.freeze
 end

spec/cancan/ability_spec.rb

@@ -441,6 +441,31 @@ class Container < Hash
     expect(@ability.attributes_for(:new, Range)).to eq(foo: 'foo', bar: 123, baz: 'baz')
   end
 
+  it 'allows to check ability even the object implements `#to_a`' do
+    stub_const('X', Class.new do
+      def self.to_a
+        []
+      end
+    end)
+
+    @ability.can :read, X
+    expect(@ability.can?(:read, X.new)).to be(true)
+  end
+
+  it 'respects `#to_ary`' do
+    stub_const('X', Class.new do
+      def self.to_ary
+        [Y]
+      end
+    end)
+
+    stub_const('Y', Class.new)
+
+    @ability.can :read, X
+    expect(@ability.can?(:read, X.new)).to be(false)
+    expect(@ability.can?(:read, Y.new)).to be(true)
+  end
+
   # rubocop:disable Style/SymbolProc
   describe 'different usages of blocks and procs' do
     class A
@@ -682,11 +707,46 @@ class Account
       end
     end
 
+    it "uses action's name in i18n" do
+      class Account
+        include ActiveModel::Model
+      end
+
+      I18n.backend.store_translations :en,
+                                      actions: { update: 'english name' },
+                                      unauthorized: { update: { all: '%{action}' } }
+      I18n.backend.store_translations :ja,
+                                      actions: { update: 'japanese name' },
+                                      unauthorized: { update: { all: '%{action}' } }
+
+      I18n.with_locale(:en) do
+        expect(@ability.unauthorized_message(:update, Account)).to eq('english name')
+      end
+
+      I18n.with_locale(:ja) do
+        expect(@ability.unauthorized_message(:update, Account)).to eq('japanese name')
+      end
+    end
+
     it 'uses symbol as subject directly' do
       I18n.backend.store_translations :en, unauthorized: { has: { cheezburger: 'Nom nom nom. I eated it.' } }
       expect(@ability.unauthorized_message(:has, :cheezburger)).to eq('Nom nom nom. I eated it.')
     end
 
+    it 'uses correct i18n keys when hashes are used' do
+      # Hashes are sent as subject when using:
+      # load_and_authorize_resource :blog
+      # load_and_authorize_resource through: :blog
+      # And subject for collection actions (ie: index) returns: { <Blog id:1> => Post(id:integer) }
+      I18n.backend.store_translations :en, unauthorized: { update: { all: 'all', array: 'foo' } }
+      expect(@ability.unauthorized_message(:update, Hash => Array)).to eq('foo')
+    end
+
+    it 'uses correct subject when hashes are used' do
+      I18n.backend.store_translations :en, unauthorized: { manage: { all: '%<action>s %<subject>s' } }
+      expect(@ability.unauthorized_message(:update, Hash => Array)).to eq('update array')
+    end
+
     it "falls back to 'manage' and 'all'" do
       I18n.backend.store_translations :en, unauthorized: {
         manage: { all: 'manage all', array: 'manage array' },

spec/cancan/model_adapters/accessible_by_has_many_through_spec.rb

@@ -74,7 +74,7 @@ class Editor < ActiveRecord::Base
 
   describe 'preloading of associations' do
     it 'preloads associations correctly' do
-      posts = Post.accessible_by(ability).includes(likes: :user)
+      posts = Post.accessible_by(ability).where(published: true).includes(likes: :user)
       expect(posts[0].association(:likes)).to be_loaded
       expect(posts[0].likes[0].association(:user)).to be_loaded
     end
@@ -85,12 +85,16 @@ class Editor < ActiveRecord::Base
       posts = Post.accessible_by(ability).where(published: true)
       expect(posts.length).to eq 1
     end
+    it 'adds the where clause correctly with joins' do
+      posts = Post.joins(:editors).where('editors.user_id': @user1.id).accessible_by(ability)
+      expect(posts.length).to eq 1
+    end
   end
 
   if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
     describe 'selecting custom columns' do
       it 'extracts custom columns correctly' do
-        posts = Post.accessible_by(ability).select('title as mytitle')
+        posts = Post.accessible_by(ability).where(published: true).select('title as mytitle')
         expect(posts[0].mytitle).to eq 'post1'
       end
     end

spec/cancan/model_adapters/accessible_by_integration_spec.rb

@@ -76,7 +76,7 @@ class Editor < ActiveRecord::Base
 
   describe 'preloading of associatons' do
     it 'preloads associations correctly' do
-      posts = Post.accessible_by(ability).includes(likes: :user)
+      posts = Post.accessible_by(ability).where(published: true).includes(likes: :user)
       expect(posts[0].association(:likes)).to be_loaded
       expect(posts[0].likes[0].association(:user)).to be_loaded
     end
@@ -92,7 +92,7 @@ class Editor < ActiveRecord::Base
   if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0')
     describe 'selecting custom columns' do
       it 'extracts custom columns correctly' do
-        posts = Post.accessible_by(ability).select('title as mytitle')
+        posts = Post.accessible_by(ability).where(published: true).select('title as mytitle')
         expect(posts[0].mytitle).to eq 'post1'
       end
     end