Skip to content

Ensure subclassed IR controllers can correctly resolve their resource_class #245

Closed
wants to merge 2 commits into from

3 participants

@tardate
tardate commented Oct 22, 2012

Until recently, it has been possible to inherit from an IR-controller. Although this wasn't exactly documented or tested behaviour - it worked, and I've certainly used it quite often. But it broke in recent history.

These couple of commits restore the behaviour, and make sure there are tests to match.

NB: this reverts some changes in commit b21aa27 which allow for resource_class to be overwritten by a method in the controller class (which breaks the class inheritance behaviour and is also not asserted by any tests).

It seems b21aa27 is possibly redundant(?), since resource_class may be over-ridden in defaults anyway. And it looks a little tricky to be able to allow overriding resource_class with a method and allowing controller inheritance. I've tried to find out more about b21aa27 but haven't seen much discussion on the point yet - see b21aa27#app/controllers/inherited_resources/base.rb

Hopefully this pull request will put the matter up for consideration one way or the other. It kind of boils down to choosing if IR should:
(a) allow overriding resource_class with a method OR
(b) continuing to support the ability to subclass IR controllers OR
(c) dig deeper to see if we can do both (a) and (b)

Since both (a) and (b) are actually undocumented behaviours at this point, I'm not actually sure what the "right" answer is. Anyone?

tardate added some commits Oct 17, 2012
@tardate tardate ensure that subclassed controllers resolve correct resource_class
NB: this reverts b21aa27 and restores the previous ability to subclass IR controllers effectively. b21aa27 is likely redundant, since resource_class may be over-ridden in defaults. Trying to find out more before making a formal pull request - see josevalim@b21aa27#app/controllers/inherited_resources/base.rb
799747f
@tardate tardate ensure subclassed controllers can override default resource_class 2408f3e
@bashmish

+1

I have STI with three levels of inheritance and controllers for the last two of them.
resource_class is broken in the second case as it has been already set on the previous level.

Simply change

self.resource_class ||= begin

in first priority into

self.resource_class = begin

and everything works as expected.

Is this pull request be merged sometime? Or some exploration needed?

@rafaelfranca
Collaborator

This need a rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.