4 ms·

Rails – The Missing Parts – Policies

• danso 13y ago
  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” end -- 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.
  • briantakita 13y ago
    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.
    • phillmv 13y ago
      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.
      • briantakita 13y ago
        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.
        • phillmv 13y ago
          :%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.
          • briantakita 13y ago
            > :%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.
  • mjbellantoni 13y ago
    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: case when user.blacklisted? # ... code ... when grouper.full? # ... code ... # ... and so on ... end
  • tomblomfield 13y ago
    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).
    • phillmv 13y ago
      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.
• dpeck 13y ago
  related, Pundit provides some similar functionality in a very minimal package https://github.com/elabs/pundit https://github.com/elabs/pundit
  • matthewcford 13y ago
    We use pundit more often these days to cancan, I would recommend it.
    • dpeck 13y ago
      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/ https://mojolingo.com/blog/2014/putting-the-can-in-cancan/ so we'll see where it goes.
  • planckscnst 13y ago
    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.
    • dpeck 13y ago
• mjbellantoni 13y ago
  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.)
  • jonathanwallace 13y ago
    Similar to http://words.steveklabnik.com/rails-has-two-default-stacks http://words.steveklabnik.com/rails-has-two-default-stacks ?
    • mjbellantoni 13y ago
      Ha! Yes. Exactly so.
    • ritchiea 13y ago
      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.
• Dorian-Marie 13y ago
  I would use CanCan ability.rb file for that: https://github.com/CanCanCommunity/cancancan https://github.com/CanCanCommunity/cancancan
  • mikesilvis 13y ago
    that's exactly what i was thinking...
  • Dirlewanger 13y ago
    This gem's function is so integral to an application I don't understand how it's not part of Rails.
  • bwilliams 13y ago
    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.
    • Jeff_Dickey 13y ago
      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...
• yxhuvud 13y ago
  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.
  • danso 13y ago
    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.
    • tomblomfield 13y ago
      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.
• dhh 13y ago
  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 https://gist.github.com/dhh/9672827
  • dhh 13y ago
    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 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.
    • pothibo 13y ago
      >> 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.
• swanson 13y ago
  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)? http://api.rubyonrails.org/classes/ActiveModel/Validator.html http://api.rubyonrails.org/classes/ActiveModel/Validator.htm...
  • karmajunkie 13y ago
    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.
• jvans 12y ago
  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. https://gist.github.com/jvans1/9745395 https://gist.github.com/jvans1/9745395