Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify its use within models #16

Closed
JasonBarnabe opened this issue Jul 27, 2021 · 3 comments
Closed

Clarify its use within models #16

JasonBarnabe opened this issue Jul 27, 2021 · 3 comments

Comments

@JasonBarnabe
Copy link

JasonBarnabe commented Jul 27, 2021

Does this gem not work within models or is it just generally not recommended because you can use AR's built-in stuff instead?

For instance-level stuff, I'm thinking of doing something like this (simplified):

class User < ApplicationRecord
  has_many :posts

  def activate
    self.activated_at = Time.now
    save
    after_commit { WebhookJob.perform_later(self) }
  end
end

user = User.find(1)
user.transaction do
  user.activate
  user.posts.make_public!
end  

Is there any problem with this? It could be rewritten to use Rails's after_commit but then that would have to inspect for changes to activated_at which just seems to make it more complicated.

How about in class methods in models? I've seen #13, so I know using after_commit does not do what might be expected here. But is there a way to use it properly?

class User < ApplicationRecord
  has_many :posts

  def self.activate_all
    where(activated_at: nil).find_each do |user|
      user.update(activated_at: Time.now)
      user.posts.make_public!
      # Not this, but maybe something else?
      # after_commit { WebhookJob.perform_later(self) }
    end
  end
end

User.transaction do
  User.activate_all
end
@Envek
Copy link
Owner

Envek commented Jul 27, 2021

For using it in models instance methods, you can just include this gem's main module into your model (or right into ApplicationRecord class):

class ApplicationRecord < ActiveRecord::Base
  include AfterCommitEverywhere
end

And that's it!


For class methods you can't extend it into model as it will effectively replace existing after_commit class-level methods that are used to define callbacks.

For now you can monkey-patch gem itself in such a way:

module AfterCommitEverywhere
  module_function :after_commit, :before_commit, :after_rollback
end

And then you will be able to call its methods directly:

AfterCommitEverywhere.after_commit { puts "Done!" }

May be it worth it to add support for these direct calls to gem API (however, method_function has some possibly dangerous side-effects like “The instance-method versions are made private”)


But my main concern for your examples is that model layer isn't appropriate place for stuff like this. It is much better to have separate classes that implements your business logic along with all callbacks. They can be called “actions”, “operations”, or “services”.

@JasonBarnabe
Copy link
Author

Thanks for the suggestions.

But my main concern for your examples is that model layer isn't appropriate place for stuff like this.

I'm working on an large, existing app to replace delayed_job with sidekiq. The code I'm on could use more refactoring, but I'm trying to limit the scope of the changes I'm making. Trying to not let "perfect" get in the way of "better".

Envek added a commit that referenced this issue Aug 5, 2021
@Envek Envek closed this as completed in b206b1e Aug 5, 2021
@Envek
Copy link
Owner

Envek commented Aug 5, 2021

Allowed to call after_commit and others directly on AfterCommitEverywhere module and added section to the README.

Released in 1.1.0. Upgrade and enjoy!

andrebarretofv added a commit to andrebarretofv/after_commit_everywhere that referenced this issue Jul 10, 2024
andrebarretofv added a commit to andrebarretofv/after_commit_everywhere that referenced this issue Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants