Hacker News new | comments | show | ask | jobs | submit login
Rails – The Missing Parts – Policies (joingrouper.com)
63 points by nottombrown 950 days ago | hide | past | web | 63 comments | favorite

If I can overly generalize, and as a person who generally loves Ruby and Rails, I'll suggest that there are two schools of Rails app development emerging.

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.)

I think there's a lot of miscommunication going on - people are doing the same things but using different vocabulary to describe it - and so I'm not sure I agree with your characterization.

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.

I disagree - I think we're talking about two competing philosophies.

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.

I agree. I'd go so far as to call the non-"Classic" school the "OO Experienced" school — people who've had copious experience in SOLID OO development in Ruby and/or other languages, who are generally horrified by many aspects of The Rails Way, but find several of the tools and components useful. The apps developed by this school generally transition fairly quickly from The Rails Way apps, to apps that use Rails, to apps that use several components that are also used by Rails but with increasingly strict separation between the AR persistence/validation/nothing else layer and the domain model. The farther along that arc an app progresses, the more survivable it generally becomes for the team and the more flexible (and profitable) a tool for the organisation.

The Rails Way is fine for a prototype, or for an app that is so well-understood that the team know from the beginning will have sharply limited change in certain very-well-known areas. It's safe to say that the vast majority of the apps I've seen in my career do not fall into that category.

But this isn't a failure of "DHH/Rails" philosphy, it's inherent in the MVC paradigm. If we're going to "blame" something, we should be a bit more careful about where that blame actually lies.

The disparity here is between the MVC paradigm and these "emergent" groups being discussed.

The problem is that they don't have a coherent solution. Rather, there are 100 little "solutions" running around with no central philosophy behind them. That's a Bad Thing.

See, I still genuinely think we're saying the same things! We're just going about it in different ways.

Rails encourages you to use the "primitives" it ships with, because they've all been more or less designed to work together, and I think this is what DHH is getting at - you get a lot of free behaviour, and in his mind you need a rather strong reason to be throwing them away.

>This is problematic, because you still have massive god-models, but now their code is split between a dozen different files, called "Concerns".

When you put it that way, that still sounds identical to your policies and interactors - you've just chosen a different nomenclature.

Both your approach and concerns are reactions to the way we've been shoving "responsibilities" into our domain objects. When we talk about coupling, we're talking about our ability to reason about our business logic in isolation; generally, the fewer atomic items we have to think about, the easier it is to maintain.

Concerns are just a DSL for writing Ruby mixins; I recently upgraded an older app and discovered I could port it all over by renaming my app/modules folder to app/concerns and I was basically done. So, the differentiating aspect rests in your preferred mental model for breaking up the business logic into these "atomic units".

When you talk about disliking AR models, to my ears it sounds like what you're really saying is "I hate the overhead of hitting the database when I'm writing tests", which is a different idea altogether from "these semantic units ought to only be responsible for serialization".

To me, it's not necessarily relevant if this one object can be persisted; what matters is that it's semantically relevant in my model of the domain/business/etc.

So, at the end of the day there is a ton of logic that is more meaningful when associated with my "users"; but it's an enormous pain in the ass to have huge swaths of my User model governing totally different bits of unrelated functionality. In this circumstance, splitting off the different unrelated bits of functionality is a net win, from both testing and maintenance.

Meanwhile, if we find that there is no good place for stuffing this bit of functionality… then we're probably missing a whole new domain model and we didn't even know it, and imho we're better off encapsulating those behaviours into an independent model.

This is to say: policies as presented in your post strike me as a more convoluted way to achieve this separation of concerns already afford to us by regular ruby mixins/classes.

Actually I disagree in a big way. I think it's a mistake to conflate choices in tooling like HAML/HTML & mySQL/pg with architecture choices.

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.

Ha! Yes. Exactly so.

I agree that Rails is polarizing into two schools - Steve Klabnik's blog post details this really well.

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".

As I point out here https://news.ycombinator.com/item?id=7437872 I think Steve Klabnik's argument is actually conflating two phenomenona. One that there are a set of tools/gems that are more conducive to productivity than the Rails defaults (HAML/Postgres/Rspec) and another that some developers find the Rails architecture to be a pain point.

They are overlapping camps but not exactly the same.

Ensuring that the Ticket is valid is obviously a domain model concern. Policy objects can be a fine idea when they're swappable and you need to allow for multiple different policies. This is not one of those cases.

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

On a separate note, I feel like this article series might better be titled "The Missing Parts of Our Knowledge of Basic Rails Features".

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.

Thanks for taking a look - I certainly respect your viewpoint.

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.

Yes, when stumbling across bad code, the first instinct should be: how can I make this simpler. Not how can I wrap this bad code in more convoluted patterns. Don't use a big word when a small one will do.

Additionally, I find that the key problem with bloated models stems from missing domain models (often has-many-through models). Not from moving the logic of the existing models into more noun classes. In your example here, the purpose of the ticket is to tie a user to a grouper: that's exactly the place to put logic that governs that connection!

Finally, what gets my goat is this notion that these patterns are necessitated by "advanced deployments". As it was some law of nature that when your app hits a certain size, you have to introduce this litany of misfit patterns. That's like arguing that if your book is longer than 300 pages, it must also use really big words and complex sentence structures. What?

The solution to large applications is to double down on simplicity, not give up on it. It is even more important to get the basics right. Execute them beautifully before even contemplating to freewheel from there.

> Additionally, I find that the key problem with bloated models stems from missing domain models (often has-many-through models). Not from moving the logic of the existing models into more noun classes.

When I first started learning Rails 4 years ago, I watched a video of a talk you gave where you refactored a kludgy piece of code to use an additional resource/model. The code unraveled before our eyes.

I still remember that as a Neo moment.

Don't suppose you have a link to that video? Or remember any more details so I could dig it up?

I think it was this one, but unfortunately I can't find the whole video.



> That's like arguing that if your book is longer than 300 pages, it must also use really big words and complex sentence structures. What?

I don't think this analogy applies. If my house is full of things, I prefer to put them in small, well recognizable boxes, instead of in one gigantic mess. This is simplicity too.

Just food for thoughts, I forked DHH's implementation and modified it to support a bigger structure (so you can keep stuff in different files/class). I haven't tested the file so it may contains bugs, I apologize if it does.

The file name contains dash(-) replace them with slash(/) as github doesn't like path.


I hope you like it ;)

Edit: It had a shit loads of typos, I believe I have fixed most of them.

Love when people play code ping pong. There's too much talk and not enough play in these threads. But I have to ask you, do you really think that splitting out those two classes improved things for the example? Or was this just future coding for possibly-maybe extension points? Because from where I'm sitting, it made things less clear and harder to follow with no additional upside.

In the context of your example, your implementation is the best, hands down.

My fork had two objectives. The first was to nail the argument of "What do I do if my model has 1.2k LOC". Look at it as an extension of your solution.

The second was to show that before using Policy, there is mechanism in rails that you can use to extract bit of code away from the model (concerns, validators, etc.)

So yeah, you could say this was a future coding for possibly-if moons are aligned- extension points ;)

To reiterate, I start with in-model validation 100% of the time and move from there as needs be.

I don't think your model has to have 1200 lines of code to see the benefits. DHH's ticket class stars with 2 lines of model code, which is a very rare case in the Rails applications I've seen. Counting by hand, I see DHH's example has one, two, three...

Twenty-six (26) lines of model code that are dedicated to validating this specific "confirmation" validation.

Given that validation is just one of the concerns of models built with The Rails Way, imagine sprinkling all of the other code that a model will have in-and-around these 26 lines of code. The central purpose of those 26 lines of code vanish into the fog of the rest of the model, creating the mystical behaviors that are common in some Rails applications.

I totally understand the concern about preemptive architecture, but once the code count for a given behavior creeps from 4, to 8, to 12... it's time to organize it into some sort of central place that's isolated from the rest. I mean, gosh, we know how this works in the real world, right?

Dev A: Here's my 26 lines of validation code to the model, DHH says it's the right thing to do.

Dev B (hours later): I need to add a little bit of behavior here, but I'm only adding a little bit of code. I'm not going to try to deduce what all of that code does, so I'll just tack mine on the bottom.

Dev C (days later): I have to add a bit of code... hmm... this is sort of a mess, I see mostly validation code but I also see some behavior. I'm not touching any of it. I'll just patch my stuff here... and here!

[iterate over and over, you get a mess of a Rails app]

I've seen this happen to multiple Rails applications... Rails applications built by experienced developers who know Rails in-and-out.

Plopping a bunch of code in something as important as a Rails model and leaving the cleanup for the next dev isn't very polite.

The theme I'm seeing in this series is a desire to add a layer between controllers and the domain. This new layer seems closer to the controller than the domain (it always acts upon the domain) so are you not simpler making the controller layer thicker? By removing logic from the domain you're creating a distance between the information and your business policies. For instance you now have to remember when to apply particular policy objects. If the logic lived in the domain then the objects would take care of this automatically.

Fundamentally I have an issue with this whole approach, if you have bloated domain objects then organise them better (as DHH mentions a common problem is not having enough classes in your domain). Also, given the choice between bloated objects and logic separated from it's information I would take bloated objects any day.

But if you have a beef with how much of the domain logic is in the Model, don't blame Rails, blame MVC. I mean, come on.

Either you want to use MVC or you want to go in a different direction. Fine. But don't blame Rails for being a good MVC implementation. Instead, formulate your own, new "philosophy" about how this all should work.

While I don't generally acknowledge the validity of "Before you criticize, think of something better", in this case I think it's appropriate. You appear to be critical of the whole MVC philosophy. And that's FINE. No problem. But before you go criticizing Rails I think you should formulate your own philosophy of how it all should work instead, rather than blaming Rails for doing what it is supposed to do, and doing it well.

>> On a separate note, I feel like this article series might better be titled "The Missing Parts of Our Knowledge of Basic Rails Features".

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.

I (more or less) agree with the sentiment expressed by "The Missing Parts of Our Knowledge of Basic Rails Features". That said, looking at your example, grouper_cant_be_full would not allow me to provide their desired functionality unless I already knew how to do it. Simply passing a symbol rather than a string there, along with a fairly short comment about what to do with it (i.e. in the controller and config/locales) would easily help people expand their knowledge. In short, you might be right in the above sentiment, but posts like these make hard for me to fault people like the author.

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.

Thanks for responding to these threads. While the "beautiful & unique snowflake" stuff is a bit much, I appreciate developers standing up for simplicity in design. The architecture astronauts try to take the intellectual high ground by providing a complex solution and it drives me nuts. We should be working to make our code less complex not more complex.

Non-facetious question: In what sense do you consider your solution simpler than the one suggested by the article? The only additional complexity I see in their solution is creating one more class, but that seems like a surface complexity.

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?

Well I was probably the only one, but I didn't even know you could use valid? with a custom context (which apparently calls the validations on that context).

The more you know...

Oh boy, is DHH going to jump into this too?

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”
-- can't simply be part of the Ticket object? The Ticket clearly has a relation to User and Grouper and talking to those objects (i.e. `if user.not_blacklisted? && user.not_booked(self.date)`) don't violate Demeter...how is making a policy object cleaner than just using a Concern that is included into Ticket? The controller then just needs to ask for `ticket.confirmable?`

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.

I think ultimately it's a philosophical difference; the problem with very many large Rails deployments is that the ActiveRecord models are the core of the application, and DHH's conception of Rails seems to encourage this.

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).

It sounds like you're letting "ease of test maintenance" drive the architecture of the rest of your app; I'm inclined to accept that incentivizing test creation will lead to more confidence in the face of changes but am somewhat unconvinced it's 1:1 with "less coupled, more maintainable" a priori.

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 sounds like you're letting "ease of test maintenance" drive the architecture of the rest of your app; I'm inclined to accept that incentivizing test creation will lead to more confidence in the face of changes but am somewhat unconvinced it's 1:1 with "less coupled, more maintainable" a priori.

I've thought of making a similar comment on _many_ other articles but never have. This is actually the first time I've ever even _seen_ anyone else saying it. I wonder if we're relatively unique or if many other people feel the same way and simply (like myself) don't actually comment on it.

Difficulty of testing is a generally reliable indicator of a design that's in deep pain. [Growing Object-Oriented Software, Guided by Tests](http://www.growing-object-oriented-software.com) is easily one of the two or three most influential technical books I've read in my career — and the book's only five years old.

When I'm spelunking through some dank and dimly-lit Rails-model God Objects, I often fantasise about how things would be different if certain influential developers had read that book or one like it before setting out, and had taken the lessons to heart. Then I swap the batteries in my headlamp, and continue my descent.

Not to dull the shine of my own brilliance but I'm pretty sure I've seen dhh say something along those lines in his previous commentary.

I've spent a lot of time thinking about this subject, and have only recently begun to feel confident enough to speak with some authority on the subject.

I think people aren't thinking critically about their code in quite the right way, despite their blog posts on the topic, but I've hitherto been unable to compose my thoughts outside of comment boxes.

I think you might be missing the point of tests. Its not "ease of test maintenance" that's driving architecture; its testing that elucidates bad design. The fundamental problem in the rails community is that instead of evolving architecture, we've added tooling to decrease the pain (e.g. guard/spork, zeus/spring, et al) rather than fix the way we write code that makes testing easy (easier?) and (more) painless.

I have no problem with verbs in class (or module) names - I'd actually say we should encapsulate more of this "activity" or "verb" behaviour in single-responsibility objects, and not within ActiveRecord models.

Eg, a ConfirmGrouper.perform object (http://eng.joingrouper.com/blog/2014/03/03/rails-the-missing...)

Uncle Bob's talk gives good reasons for this pattern: http://www.confreaks.com/videos/759-rubymidwest2011-keynote-...

Basically, your data-persistence objects (ActiveRecord models) become more composable because they do fewer things and rely less on associated objects being present to work properly.

Your "interactor" objects which perform "verb"-style activities can also be broken down into very small units with single-responsibility and then composed with each other to perform more complex interactions.

Putting all of this logic into models makes it very very hard to break apart units of behaviour and re-compose them.

This is what I mean by "less coupled, more maintainable"

>Basically, your data-persistence objects (ActiveRecord models) become more composable because they do fewer things and rely less on associated objects being present to work properly.

I agree with this entirely, and elsewhere in this thread have said so, and I'm not advocating fat models.

I'm more questioning the precise way in which you've chosen to perform your refactoring. To me, that-method-in-partcular seems like a code smell, and I've not been convinced by your blog post compared to the alternate methods I've suggested in my comment. Does that make sense?

This seems like, in essence, a simple function placed in a domain context. For example, in node.js, I would make this a module (a file that exports a function). The module would be named and nested in a directory structure that matches the domain.

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.

The solution you describe is isomorphic to the one proposed in the blog post, which I think you recognize?

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.

The solution I describe is basically a functional approach. The function is responsible for the policy concerns over all data in the system.

The function itself can be decomposed into smaller functions.

If you need extensibility, you could make the function composable and register additional pieces to the policy. Most codebases don't need this sort of extensibility.

Simple constructs & consistent, accurate, precise naming is preferable to complicated architectures.

:%s/function/module/ and no meaning is lost :).

I've yet to catch up on my lisp and fully grok the "functional paradigm" and, admittedly, passing around consts is less elegant than passing around function pointers, but what you're describing sounds isomorphic to me in terms of code organization. Look at the blog post; he's just passing a one method class, which… is basically a less elegant function.

> :%s/function/module/ and no meaning is lost :).

Javascript has functions, from the function keyword. Node.js uses commonjs. In commonjs, every file is a module. You can use

module.exports = <value>;

You can access the module by using require.

var moduleValue = require("path/to/module");

It's powerful because it doesn't have the namespace collisions that Ruby has. Everything is not global all the time. modules are also an elegant way of holding private state using closures.

Having a class with a single method is the Ruby way of categorizing this method within the domain.

I know this isn't the point of your comments, but I'll take the opportunity to point out that Ruby has a case statement that reads a lot better for code like this:

  when user.blacklisted?
    # ... code ...
  when grouper.full?
    # ... code ...
    # ... and so on ...

How is a Policy different than an ActiveRecord::Validator? Could this TicketPolicy not just be a validator extracted to it's own file (and tested on it's own)?


Policies (also known as strategies) aren't altogether different from Validators, though they're a bit more flexible. The real difference is that validation doesn't belong on your database interface, and its the chief reason why AR can only be used for domain driven design in the most basic of circumstances (NB: this isn't the same as saying that database constraints are a bad thing.) Validators are hardcoded into models; in that regard, they're only slightly more flexible than using validations directly on the model. Like Rails' notion of concerns, they're really about code geography, not architecture or design.

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.

related, Pundit provides some similar functionality in a very minimal package https://github.com/elabs/pundit

We've been using Pundit with great success in our organization. We use it along with a few other objects described below.

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.

Curious is you have some more details about this written up anywhere or some code on github?

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.

That would be a good idea for a writeup. I'll have to get that done. For now, I have a couple more details.

We're using the closure_tree gem to give a hierarchy to the permissions; every permission can have a parent, which may further limit which abilities are allowed on a managable by a manager. This means that if user A has abilities "1,2,3" on an object, when he grants a permission to user B, while he can create the permission with whatever abilities he wants (maybe abilities "2,3,4"), the parent of that user B's permission is user A's permission, which restricts the real abilities user B has so that user B effectively has only abilities "2,3" on that object. To do this, we get the whole ancestor chain and take the intersection of all the abilities of each permission.

We also have organization objects; these are the owners of everything in the system - they own the users, the roles, the permissions, etc. Organizations may also be a manager or a managable; in fact, an organization automatically gets a permission with itself as both the manger and the managable, with every ability. We also use closure_tree with organizations: when an organization is the managable part of a permission, that permission applies to everything the organization owns, including all descendants.

So we have permissions with a hierarchy where we look upward at its ancestors to limit its abilities, and organizations with a hierarchy where we look down to find the scope that permission operates on.

awesome. I'd definitely like to see a full writeup if you ever do one, email is in my profile.

We use pundit more often these days to cancan, I would recommend it.

agreed, it seems like the way to go today. But there has been some movement towards reviving cancan lately https://mojolingo.com/blog/2014/putting-the-can-in-cancan/ so we'll see where it goes.

While more interesting than the first part, it should be noted that the policy does not solve the same problem as the one they had originally, where different fail reasons led to different URLs.

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.

Yeah, I agree with you...I think the implication is that the original implementation was bad for various reasons, among them, redirecting to different URLs based on error. But if in fact, that kind of redirection needed to be done (which would seem to be a mild violation of best-OOP practices), then I'd agree, a Policy object that lived outside of the Model and the Controller would be needed.

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.

Yes - you're absolutely right. We perhaps could have made this trade-off more explicit.

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.

I would use CanCan ability.rb file for that: https://github.com/CanCanCommunity/cancancan

My problem with CanCan is that when you begin to have more complicated access logic ability.rb becomes a giant mess.

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 agree. The first app we wrote using CanCan, the `ability.rb` file (and the dozen files we factored out of that) grew to be... significant.

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...

This gem's function is so integral to an application I don't understand how it's not part of Rails.

that's exactly what i was thinking...

I think this is a responsibility that lies in the model. What if you decide to allocate tickets through a different model? You have to remember to include this policy everywhere. Another approach i think is reasonable is creating a class for this on the model level.


Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | DMCA | Apply to YC | Contact