The first is what I'll term the "Classic School" or "DHH School" characterized by full-blown utilization of all Rails magic such as AR/AC callbacks, pretty skinny but not obsessively skinny controllers, an eschewance of service objects and more.
The "Emerging School" relies less on some aspects of Rails magic, and introduces patterns like services/interactors/DCI and seems to be more focused on building a domain models which attempt to be less coupled to the framework.
I think the Grouper folks are in the second camp given their previous blog post about interactors which showed up here on HN. Also, I think the desire to encapsulate authorization into a reusable object seems slightly more in the spirit of the emerging school.
What I find odd is that they'd offer a solution for authorization which is so bound up in Rails magic. It seems to me authorization is really part of the domain model and as such you would expect to find that code in whatever gets called by the controller (which I would expect to be something like an interactor, but in this blog post is just plain old Rails code.)
In a nutshell, almost all DCI/service object blog posts I have read seem to have almost all of their issues addressed by a liberal use of `concerns`.
In the end, I think we're all talking about how to separate "concerns/responsibilities/dependencies" into atomic units so our code is easier to reason about; it turns out my user object does a Ton Of Things and it becomes more maintainable once we split it up.
The schism in my mind comes more from random blog post declaring "We have seen the truth! and it is $new_jargon_here" and then DHH pops out of the wilderness and crankily declares "yes, yes, this is why we introduced xyz, jeez."
What strikes me most in the end is that most of the random blog posts seem to lack a coherent theory for why their code has improved, rendering most of the conversation around it moot. DHH on the other hand has strong, and well formed opinion on the subject which is why he can occasionally come through and refactor it along his mental model.
This is all to say: I have rather strong, if currently unarticulated, opinions on how to think about code organization, and I tend to be more on DHH's side when it comes to these debates. Furthermore, I'm increasingly convinced we're all talking about the same things using different words.
DHH / standard-Rails seems to encourage you to fit things into Models, Views or Controllers. Models got too fat, and so Concerns were introduced. This is problematic, because you still have massive god-models, but now their code is split between a dozen different files, called "Concerns". The main problem is that by making ActiveRecord models the core of your application, you generally give each model too much responsibility & too much knowledge of its associated objects. This means it's very hard to break down & recompose the different functionality into new objects. A model only functions correctly if all of its associated objects are present. This is tight coupling.
The alternative view is to introduce several new single-responsibility objects like Interactors/Service-Objects (and Policies, as here) which should contain the core of your application's business logic. These Service Objects are the opposite of concerns - they do not extend the API of models. They exist to control the interactions between the models. The models become dumb interfaces that deal with data validation and persistence. Because they know hardly anything about the other objects in the system, and have very limited APIs, they can be passed around easily, and re-composed into more complex interactions. They are also incredibly simple to test. The same applies to Interactors - they should be broken down into small units of behaviour with very limited APIs, which can then be passed around easily and composed into more complex interactions. This is loose coupling.
Your architecture choices change the way you write your application code and how you utilize the Rails framework. On the other hand HAML is not going to change the way you write markup, it just provides a different syntax that some people prefer. Postgres has features people like that mySQL lacks. MiniTest & Rspec are different syntaxes for testing. These three choices have very little to do with how you use Rails & write your app code, they just happen to be made by the same people who advocate big architecture changes.
I fall in a camp where I use HAML, postgres & Rspec but I also use fat models and skinny controllers. I have a few really small services in my apps but I try to reserve them for cases when the standard Rails pattern is extremely ugly rather than actively seeking out replacements for the Rails way.
But I don't think that moving towards more single-responsibility objects means you have to give up all that Rails offers - particularly the preference for concise, human-readable APIs and convention-over-configuration. Ie, we don't need tonnes of boiler-plate just because we want to use more objects.
We put this permission check in the controller and not the interactor because it seems like the controller should generally be responsible for authentication & permissioning. Once you get to the interactor, it should just be told "Perform this interaction". Not "check if you can perform it, and then perform it".
They are overlapping camps but not exactly the same.
Here's a much simpler approach that keeps the validation logic in the domain model and uses features that Rails has had since the dawn of the framework: https://gist.github.com/dhh/9672827
Reinventing basic features doesn't make your Rails deployment "advanced", it just makes it convoluted. There's no bonus prize for introducing fancy patterns to situations that do not call for them.
Further more, here's the definition of the Active Record pattern, as described by Martin Fowler: "An object that wraps a row in a database table or view, encapsulates the database access, and adds domain logic on that data". The key part is that last sentence.
So a quote like what follows simply misunderstands the purpose the Active Record pattern: "A recurring theme in these posts is that ActiveRecord models should be very simple interfaces to a datastore – the User model should be responsible for validating and persisting attributes of a user, like name and date-of-birth, and not much else."
See the example diagram (which I actually drew for Martin back before even starting Rails!): http://www.martinfowler.com/eaaCatalog/activeRecord.html -- it includes domain logic methods like "getExemption", "isFlaggedForAudit", and so forth. Exactly like the pattern describes.
You're not a beautiful and unique snowflake.
I guess where we diverge is the amount of domain logic which lives on an ActiveRecord model.
My experience is relatively limited, but in 4 or so years of professional Rails development, across many different codebases, the overwhelming majority of problems have been caused by bloated models with too many responsibilities.
You clearly have more experience with Rails, but your solution always seems to be "If you were smarter, you wouldn't have these problems with my framework". I've seen this problem recur again and again across multiple teams & codebases.
We're seeking to address that problem with these concepts that have been successfully used in other frameworks & languages.
Agreed on so many level. I see so many new gems that does the exact same thing as what rails do by default. I've spent some times browsing Rails source. It took me a while and as a side effect, the number of gems I am using now is a fraction of what I used when I started.
You can't build libraries and abstraction on top of Rails if you don't understand rails to begin with.
Edit: I should probably note that I _don't_ think you should be "required" to help out like this at all. Simply developing/sharing Rails is certainly _way_ more than I'm doing for other people. I'm just saying if you _are_ going to comment on this stuff, it would be nice if you transferred some of your greater understanding with your comments.
I respect your opinion but it seems that you aren't really addressing the advantages the article is claiming their design has.
The likelihood of change is much different for the policies than for the basic domain model attributes of a ticket, and this to me is the key reason to prefer the policy design that I'd like to hear you address objectively.
Based on your comment about swappable policies, I'd guess that your position is to wait until you need to pull it out before you do. But if you can see a likely change coming and there is no cost to designing for it, why not do it? Is your bone of contention with the idea that we can assign reasonable probabilities to what is likely to change; with the idea that there is no cost to pulling the policy object out now; with both of those things; or with something else entirely?
The more you know...
As an intermediate Rails developer...I don't understand why this if/else forest:
if user.blacklisted? # They've been banned for bad-behaviour
fail! “You can't book a Grouper at this time”
elsif grouper.full?
fail! “This grouper is full, please pick another date”
elsif grouper.past?
fail! “This grouper has already occured!”
elsif user.has_existing_grouper?(grouper)
fail! “You’re already going on a Grouper that day”
elsif ticket.confirmed?
fail! “You’ve already confirmed this grouper”
end
edit: OK, not ask for `ticket.confirmable?`, but rather, try to `save` the ticket and the Ticket constructor/validators can pass on the errors to the controller.
This tends to cause a lot of problems - it's hard to exercise a single model in tests without having a very large number of associated objects present. This also tends to require them to be in the database. This makes your tests very slow, and your application very tightly coupled and hard to refactor.
If you rethink your Rails application and make models responsible for only data validation & persistence, you tend to avoid these kinds of problems. You then need somewhere else for the "core" of your application logic to reside. We suggest these new concepts are Interactors, Policies (and Decorators - more to come in a new blog post).
Case in point, I'm really uncomfortable with class names with verbs. Typing CanConfirmTicketPolicy.new(ticket: ticket).allowed? smells bad to my fingers.
It strikes me that you haven't eliminated the coupling? because the CanConfirmTicketPolicy still depends on different domain objects. Kind of by definition, you can't remove it because it's explicitly operating on User, Group and Ticket; the main difference to me seems that they're easier to mock?
I would argue that this ought to be either an explicit inter domain class that models the interaction between Users, Group and Tickets - some appropriate noun like Purchase or GroupActivity or whatever - that can then be solely responsible and reused as appropriate.
You have a bit of text that explains your decisions,
>We could move the logic to a controller mixin, or define the method on the ApplicationController, but it would still not be available in our view
Could you not just define a helper method, then? Helpers are available in both views and controllers if memory serves, are mixed in by default, are equally easy to test as your verbed policy object and have the (minor) added benefit of not polluting your namespace while being equally easy to reason about - it's unclear to me how can_confirm_ticket?(ticket) would necessarily be inferior.
If you're really interested in putting these in models (which is also acceptable) then a regular concern namespaced to your preferred model would work just as well.
Am I missing something? I would like to understand your use case but I don't seem to get it.
It seems that the logical ontology is based on ruby's objects. Unfortunately, this makes the solution much more convoluted than a simple function exported in a module.
But both leave a lot to be desired. I personally think in both cases they are a code smell, and we ought to either introduce an object whose explicit job it is to worry about these inter domain interactions or pile it up as a mixin into the appropriate domain models.
case
when user.blacklisted?
# ... code ...
when grouper.full?
# ... code ...
# ... and so on ...
endhttp://api.rubyonrails.org/classes/ActiveModel/Validator.htm...
Policies, on the other hand, are far more flexible (assuming your application interface supports them, or they're used to wrap a block, or some other gatekeeping mechanism) because they can be swapped out. Imagine a circumstance in which you want one set of validations for creation by a normal user, and another set of validations when the creation of an instance occurs through a privileged API; yet another set of rules for creation by an admin, and so on...
You could implement those as a series of validators with an :if option on the validates call, and if you want to stick with Rails' canon, go right ahead. But in my opinion (and having made this mistake before) you're simply staving off an inevitable point at which your dependence on Rails has hamstrung your ability to iterate code.
We have abilities, which is just a table full of strings like "modify users". The policy (from Pundit) looks these up to find whether an actor (user, typically) can do something to a model.
We have roles, which is just a user-definable collection of abilities, so you might define an 'admin' role that has every permission, and a 'reporter' role that has the ability to run reports and not much else.
We have permissions, which includes a role, a manager, and a manageable. The manager is the actor, or the thing asking to do something to another object; the managable is the object the actor is trying to act upon.
There is a "Managable" concern that gets included in any object that may be guarded by policies, and a "Manager" concern that gets added to any object that might be an actor. The Manager concern sets up the associations gives us methods like "#has_ability_on(ability, managable)", and "#has_ability)", which is useful for deciding whether to show gui widgets. These methods are how Pundit looks up the abilities one (manager) object has on another (managable) object.
This simple setup has allowed us to greatly simplify our application.
I'm just using Pundit in a "real" app for the first time where strict user types won't be enough and I need to have a role based approach. I'm doing something similar to what you have now, but a couple steps back in my design process, and the managable concern sounds promising.
If you only intend to redirect to one url in case of fail, then using normal validations (possibly on a service object implementing ActiveModel::Validations will suffice and produce as simple code as their solution.
Which isn't saying that I dislike it, only that it is quite equivalent with a separate service model without controller integration.
However, the OP apparently realized in the refactoring that it made more sense to simply have two paths...or setup a convention so that `redirect` can infer the correct error page from the error itself. If the OP had insisted that the Policy keep the path logic, I'm sure DHH would jump all over that as being bad-design-looking-for-a-solution.
We didn't want the Policy object to know about the redirect paths, so we opted to cut down the number of responses available - we simply redirect_to :back by default, but this can be configured in the controller.
It's already a file where you just throw in all of your authorization logic anyways so it always feels a bit unruly once you get beyond basics.
I love the idea of Pundit because it decouples all that as much as seems practical. I'm about to find out if theory informs practice or not...