Remove ActiveRecord::Base.suppress

[radar]

From my comment on #18847.

---

I agree with @rafaelfranca.

This feature will be mis-used in quite a lot of Rails applications. Suppressing callbacks in this way should be something that the framework actively discourages, rather than encourages. I have worked with enough live Rails applications (that aren't just simple CRUD apps) to know that this feature is going to cause issues in the long run.

From the examples given in the PR, this is what I understand it to be doing:

1. There is an after_create callback on the Comment model which creates new Notification objects every time a comment is created. This is a pretty "traditional" callback in Rails-land.
2. To stop notifications being created you wrap it in a suppress block:

Notification.suppress do
  # code that calls something like Comment.create
end

This problem could just as easily be solved by not having the after_create callback at all. As other people have suggested, there's two better solutions:

1. Move the logic of creating the comment + sending a notification out to a "service class". This is the direction the Rails community has been headed recently, and the choice that I personally would pick, regardless of any BDFL's quips.
2. If it's just one place where I care about both creating a comment and sending a notification (probably something like CommentsController), have this code:

def create
  comment = Comment.new(comment_params)
  if comment.save
    Notification.create(thing: comment)
    # etc.
  end
end

Both of these solutions are more preferable (to me, at least) than adding more code to the Rails framework which encourages "magical" behaviour. Both of the solutions I've provided make it explicit that when the comment is created, the notification is created. If Notification.create isn't called after Comment.create, then there will be no notifications. This is particularly handy in tests, when we don't necessarily want to create a Comment and a Notification together because of tight coupling.

Consider reverting this feature. It is going to cause headaches because it will be misused in Rails applications.

[rails-bot]

r? @senny

(@rails-bot has picked a reviewer for you, use r? to override)

[BJClark]

This is the direction the Rails community has been headed recently

By recently he means since around 2009 at least.

+1

[rafaelfranca]

I defer this decision to who really care about it (r? @dhh), but I have to say my opinion is still the same. I still think this can lead to problema with people misusing it. I also understand that even that people can hurt themselves with a feature it is not a good direction to design. Basically people could hunt themselves even with best design you could think.

[senny]

r? @dhh

[lukerollans]

If there are those whom wish to follow this pattern (they probably shouldnt), I think it would be much better suited as a gem

[iGEL]

I agree with the reasoning behind this. It's also weird that the feature is targeted at callbacks, but hooks into saving. If you really want something like this, shouldn't it rather suppress the callback itself? The gem idea sounds also good.

[dhh]

We discussed this at length a year ago. No new arguments have been introduced. Protecting programmers from themselves is explicitly not in the charter for Rails when it comes to providing features that have a valid use case but could be abused.

Now you may well think that this use case isn't valid for your style and code. That's totally fine. Nobody is forcing you to use this. I consider this a valid use case, have this feature employed in a production app, and I'm thrilled with the results.

As a general point: Opening a pull request to restate existing arguments that were considered a year ago doesn't add anything but antagonism to the proceedings. You know what the outcome is going to be.

If you want to vent your frustrations about a feature of Rails you don't like, and most people have at least a few of those, may I suggest an indignant tweet? If you really want to add oomph to the outrage engine, you could also post a medium article about how this is making you consider not using Rails (or was the final straw that made you leave). Both are popular choices for discharging bile, and I encourage them over reopening old tickets.

Rails has thousands of individual features. Almost nobody is going to agree about how to best create applications using such a broad scope of services. That's wonderful! It means we accept a level of pluralism inside the framework that's necessary to support a big tent.

Finding a feature you particularly don't like isn't a novel discovery that you need to open a ticket for. It's part of A Developer's Life. Enjoy yours!

[sevenseacat]

I don't think that level of condescension was called for, on a well-thought out issue that has received this much agreement in such a short timeframe.

[dhh]

Given that @radar helpfully invited a +1 pile-on via Twitter, this issue has been locked. If you want to vent or express support, feel free to use a forum like Twitter that has pitchfork ergonomics as a design goal. This issue tracker is where we address bugs.