Implement detection for sti for including subclasses in check #689
Conversation
| @@ -12,6 +14,8 @@ def self.matches_subject_class?(subjects, subject) | |||
| def self.matching_class_check(subject, sub, has_subclasses) | |||
| matches = matches_class_or_is_related(subject, sub) | |||
| if has_subclasses | |||
| return matches unless StiDetector.sti_class?(sub) | |||
There was a problem hiding this comment.
Only include the subclasses in the check if the model is an sti class
| @@ -0,0 +1,10 @@ | |||
| class StiDetector | |||
| def self.sti_class?(subject) | |||
There was a problem hiding this comment.
A general detection for STI classes
| @@ -909,6 +909,37 @@ class JsonTransaction < ActiveRecord::Base | |||
| end | |||
| end | |||
|
|
|||
| context 'with rule application to subclass for non sti class' do | |||
There was a problem hiding this comment.
Check that rules are no longer applied to parents for non sti
ghost
mentioned this pull request
|
Thank you for working on this. We've tried this code and it seems to work—our test cases pass and it seems to block a parent class of resources as we expected. If we see any problems with it, we'll report back. |
|
Any news on this? 🙏 This issue prevents us from updating to cancancan 3.2+ :( |
ghost
mentioned this pull request
There was a problem hiding this comment.
@Liberatys do we need to check further scenarios here, like @phallstrom suggested, or are they covered already somewhere else?
| @@ -0,0 +1,10 @@ | |||
| class StiDetector | |||
| def self.sti_class?(subject) | |||
|
@coorasse I now also added the spec that covers the example from #677(as per @phallstrom). CI is blue/green ^^ |
@coorasse how do you want to handle this ? It seems like STI will have some issues with the current implementation. I'll probably have to add conditional loading at 3 places to fully comply with the expected behaviour.