search

rubysherpas / forem

The best Rails 3 and Rails 4 forum engine. Ever.
http://forem.herokuapp.com
MIT License
1.55k stars 424 forks source link

Should not attempt to send subscription notifications when email not present #650

Open comp615 opened 9 years ago

comp615 commented 9 years ago

Right now when a user creates a post the system will attempt to send notifications to all previous posters (subscribers): https://github.com/radar/forem/blob/rails4/app/models/forem/post.rb#L106

There is a bug here, however, in that at no point during the process does the system actually check if the user it's attempting to notify has an email (it's entirely plausible that they might not). This line should check to see if they have an email address that is valid before attempting to send: https://github.com/radar/forem/blob/rails4/app/mailers/forem/subscription_mailer.rb#L10

radar commented 9 years ago

Hi @comp615,

It sure sounds like you know what the problem is and how to fix it. Could you please submit a PR that fixes this? Personally I would not put the check in the SubscriptionMailer, but on the method that calls the mailer.

comp615 commented 9 years ago

Yeah that was my plan, this is sort of a marker for me as well :-P I'll take a stab at it when I have a chance

On Mon, Mar 30, 2015 at 1:33 PM, Ryan Bigg notifications@github.com wrote:

Hi @comp615 https://github.com/comp615,

It sure sounds like you know what the problem is and how to fix it. Could you please submit a PR that fixes this? Personally I would not put the check in the SubscriptionMailer, but on the method that calls the mailer.

— Reply to this email directly or view it on GitHub https://github.com/radar/forem/issues/650#issuecomment-87824186.

krainboltgreene commented 8 years ago

@comp615 Any progress on this?