Skip to content

Commit

Permalink
Merge branch 'Juleffel-develop' into develop
Browse files Browse the repository at this point in the history
  • Loading branch information
coorasse committed Mar 22, 2022
2 parents 7629f56 + d0ea89a commit 373532d
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 3 deletions.
4 changes: 4 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Metrics/BlockLength:
- 'lib/cancan/matchers.rb'
- '**/*_spec.rb'

Metrics/ClassLength:
Exclude:
- 'lib/cancan/model_adapters/active_record_adapter.rb'

# TODO
# Offense count: 2
# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist.
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ When first developing, you need to run `bundle install` and then `bundle exec ap

You can then run all appraisal files (like CI does), with `appraisal rake` or just run a specific set `DB='sqlite' bundle exec appraisal activerecord_5.2.2 rake`.

If you use RubyMine, you can run RSpec tests by configuring the RSpec configuration template like this:
![rubymine_rspec.png](rubymine_rspec.png)

See the [CONTRIBUTING](./CONTRIBUTING.md) for more information.

## Special Thanks
Expand Down
12 changes: 10 additions & 2 deletions lib/cancan/conditions_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,16 @@ def matches_non_block_conditions(subject)
end

def nested_subject_matches_conditions?(subject_hash)
parent, _child = subject_hash.first
matches_conditions_hash?(parent, @conditions[parent.class.name.downcase.to_sym] || {})
parent, child = subject_hash.first

matches_base_parent_conditions = matches_conditions_hash?(parent,
@conditions[parent.class.name.downcase.to_sym] || {})

adapter = model_adapter(parent)

matches_base_parent_conditions &&
(!adapter.override_nested_subject_conditions_matching?(parent, child, @conditions) ||
adapter.nested_subject_matches_conditions?(parent, child, @conditions))
end

# Checks if the given subject matches the given conditions hash.
Expand Down
12 changes: 12 additions & 0 deletions lib/cancan/model_adapters/abstract_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ def self.matches_conditions_hash?(_subject, _conditions)
raise NotImplemented, 'This model adapter does not support matching on a conditions hash.'
end

# Used above override_conditions_hash_matching to determine if this model adapter will override the
# matching behavior for nested subject.
# If this returns true then nested_subject_matches_conditions? will be called.
def self.override_nested_subject_conditions_matching?(_parent, _child, _all_conditions)
false
end

# Override if override_nested_subject_conditions_matching? returns true
def self.nested_subject_matches_conditions?(_parent, _child, _all_conditions)
raise NotImplemented, 'This model adapter does not support matching on a nested subject.'
end

# Used to determine if this model adapter will override the matching behavior for a specific condition.
# If this returns true then matches_condition? will be called. See Rule#matches_conditions_hash
def self.override_condition_matching?(_subject, _name, _value)
Expand Down
25 changes: 25 additions & 0 deletions lib/cancan/model_adapters/active_record_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,31 @@ def initialize(model_class, rules)
ConditionsNormalizer.normalize(model_class, @compressed_rules)
end

class << self
# When belongs_to parent_id is a condition for a model,
# we want to check the parent when testing ability for a hash {parent => model}
def override_nested_subject_conditions_matching?(parent, child, all_conditions)
parent_child_conditions(parent, child, all_conditions).present?
end

# parent_id condition can be an array of integer or one integer, we check the parent against this
def nested_subject_matches_conditions?(parent, child, all_conditions)
id_condition = parent_child_conditions(parent, child, all_conditions)
return id_condition.include?(parent.id) if id_condition.is_a? Array
return id_condition == parent.id if id_condition.is_a? Integer

false
end

def parent_child_conditions(parent, child, all_conditions)
child_class = child.is_a?(Class) ? child : child.class
foreign_key = child_class.reflect_on_all_associations(:belongs_to).find do |association|
association.klass == parent.class
end&.foreign_key&.to_sym
foreign_key.nil? ? nil : all_conditions[foreign_key]
end
end

# Returns conditions intended to be used inside a database query. Normally you will not call this
# method directly, but instead go through ModelAdditions#accessible_by.
#
Expand Down
Binary file added rubymine_rspec.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
73 changes: 72 additions & 1 deletion spec/cancan/model_adapters/active_record_adapter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

require 'spec_helper'

describe CanCan::ModelAdapters::ActiveRecordAdapter do
RSpec.describe CanCan::ModelAdapters::ActiveRecordAdapter do
let(:true_v) do
ActiveRecord::Base.connection.quoted_true
end
Expand Down Expand Up @@ -49,6 +49,11 @@
t.timestamps null: false
end

create_table(:legacy_comments) do |t|
t.integer :post_id
t.timestamps null: false
end

create_table(:legacy_mentions) do |t|
t.integer :user_id
t.integer :article_id
Expand All @@ -62,6 +67,7 @@
end

class Project < ActiveRecord::Base
has_many :articles
has_many :comments
end

Expand Down Expand Up @@ -96,6 +102,12 @@ class Mention < ActiveRecord::Base

class Comment < ActiveRecord::Base
belongs_to :article
belongs_to :project
end

class LegacyComment < ActiveRecord::Base
belongs_to :article, foreign_key: 'post_id'
belongs_to :project
end

class User < ActiveRecord::Base
Expand Down Expand Up @@ -473,6 +485,65 @@ class User < ActiveRecord::Base
end
end

describe 'when can? is used with a Hash (nested resources)' do
let(:user1) { User.create!(name: 'user1') }
let(:user2) { User.create!(name: 'user2') }

before do
category = Category.create!(name: 'category')
@article1 = Article.create!(name: 'article1', category: category, user: user1)
@article2 = Article.create!(name: 'article2', category: category, user: user2)
@comment1 = Comment.create!(article: @article1)
@comment2 = Comment.create!(article: @article2)
@legacy_comment1 = LegacyComment.create!(article: @article1)
@legacy_comment2 = LegacyComment.create!(article: @article2)
end

context 'when conditions are defined using the parent model' do
let(:ability) do
Ability.new(user1).tap do |ability|
ability.can :read, Article
ability.can :manage, Article, user: user1
ability.can :manage, Comment, article: user1.articles
ability.can :manage, LegacyComment, article: user1.articles
end
end

it 'verifies parent equality correctly' do
expect(ability.can?(:manage, { @article1 => Comment })).to eq(true)
expect(ability.can?(:manage, { @article1 => LegacyComment })).to eq(true)
expect(ability.can?(:manage, { @article1 => @comment1 })).to eq(true)
expect(ability.can?(:manage, { @article1 => @legacy_comment1 })).to eq(true)

expect(ability.can?(:manage, { @article2 => Comment })).to eq(false)
expect(ability.can?(:manage, { @article2 => LegacyComment })).to eq(false)
expect(ability.can?(:manage, { @article2 => @legacy_comment2 })).to eq(false)
end
end

context 'when conditions are defined using the parent id' do
let(:ability) do
Ability.new(user1).tap do |ability|
ability.can :read, Article
ability.can :manage, Article, user_id: user1.id
ability.can :manage, Comment, article_id: user1.article_ids
ability.can :manage, LegacyComment, post_id: user1.article_ids
end
end

it 'verifies parent equality correctly' do
expect(ability.can?(:manage, { @article1 => Comment })).to eq(true)
expect(ability.can?(:manage, { @article1 => LegacyComment })).to eq(true)
expect(ability.can?(:manage, { @article1 => @comment1 })).to eq(true)
expect(ability.can?(:manage, { @article1 => @legacy_comment1 })).to eq(true)

expect(ability.can?(:manage, { @article2 => Comment })).to eq(false)
expect(ability.can?(:manage, { @article2 => LegacyComment })).to eq(false)
expect(ability.can?(:manage, { @article2 => @legacy_comment2 })).to eq(false)
end
end
end

unless CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0')
context 'base behaviour subquery specific' do
before :each do
Expand Down

0 comments on commit 373532d

Please sign in to comment.