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

Add recipients feature for defining recipients inside the notifier #459

Merged
merged 5 commits into from
Jun 27, 2024

Conversation

excid3
Copy link
Owner

@excid3 excid3 commented Jun 26, 2024

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
Copy link

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
Copy link
Owner Author

excid3 commented Jun 27, 2024

@misterhtmlcss I'm not following?

@excid3 excid3 merged commit 30e02db into main Jun 27, 2024
45 checks passed
@excid3 excid3 deleted the recipients branch June 27, 2024 07:33
ghiculescu added a commit to ghiculescu/noticed that referenced this pull request Aug 27, 2024
This adds info about excid3#459 to the README.
@ghiculescu ghiculescu mentioned this pull request Aug 27, 2024
excid3 pushed a commit that referenced this pull request Aug 27, 2024
This adds info about #459 to the README.
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

Successfully merging this pull request may close these issues.

2 participants