excid3 / noticed

Notifications for Ruby on Rails applications
MIT License
2.4k stars 169 forks source link

Add recipients feature for defining recipients inside the notifier #459

Closed excid3 closed 2 months ago

excid3 commented 2 months ago

Currently, Noticed requires you to precompute the recipients of a Notifier.

recipients = comment.post.commenters.excluding(comment.user).uniq
CommentNotifier.with(comment: @comment).deliver(recipients)

Instead, we could move this into the Notifier and compute it internally

class CommentNotifier < ApplicationNotifier
  recipients ->{ params[:comment].post.commenters.excluding(params[:comment].user).distinct }

  # or 
  recipients do 
    params[:comment].post.commenters.excluding(params[:comment].user).distinct 
  end

  # or
  recipients :fetch_recipients

  def fetch_recipients
     # ...
  end
end

This greatly simplifies the code required in controllers or other locations where Notifiers are triggered.

CommentNotifier.with(comment: @comment).deliver
misterhtmlcss commented 2 months ago

Is this better than using this? Just wondering if there are advantages I’m not aware of.

class CommentNotifier < ApplicationNotifier
  def initialize(comment)
    @comment = comment
  end

  def recipients
      @comment.post.commenters.excluding(@comment.user).uniq
  end

  def deliver
    # implementation here
  end
end

CommentNotifier.new(@comment).deliver
excid3 commented 2 months ago

@misterhtmlcss I'm not following?