Commit 8531b160 authored by Benjamin Neff's avatar Benjamin Neff Committed by Dennis Schubert

gracefully handle when a like is already deleted again

closes #5983
parent 68f7208f
......@@ -13,6 +13,7 @@
* Improved handling of reshares with deleted roots [#5968](https://github.com/diaspora/diaspora/pull/5968)
* Remove two unused methods [#5970](https://github.com/diaspora/diaspora/pull/5970)
* Refactored the Logger to add basic logrotating and more useful timestamps [#5975](https://github.com/diaspora/diaspora/pull/5975)
* Gracefully handle mailer failures if a like is already deleted again [#5983](https://github.com/diaspora/diaspora/pull/5983)
## Bug fixes
* Disable auto follow back on aspect deletion [#5846](https://github.com/diaspora/diaspora/pull/5846)
......@@ -37,6 +38,7 @@
* Fix a freeze in new post parsing [#5965](https://github.com/diaspora/diaspora/pull/5965)
* Add case insensitive unconfirmed email addresses as authentication key [#5967](https://github.com/diaspora/diaspora/pull/5967)
* Fix liking on single post views when accessed via GUID [#5978](https://github.com/diaspora/diaspora/pull/5978)
* Gracefully handle mailer failures when a like is already deleted again [#5983](https://github.com/diaspora/diaspora/pull/5983)
## Features
* Hide post title of limited post in comment notification email [#5843](https://github.com/diaspora/diaspora/pull/5843)
......
......@@ -8,8 +8,8 @@ module NotificationMailers
def initialize(recipient_id, sender_id=nil, *args)
@headers = {}
@recipient = User.find_by_id(recipient_id)
@sender = Person.find_by_id(sender_id) if sender_id.present?
@recipient = User.find(recipient_id)
@sender = Person.find(sender_id) if sender_id.present?
log_mail(recipient_id, sender_id, self.class.to_s.underscore)
......@@ -29,11 +29,12 @@ module NotificationMailers
end
private
def default_headers
headers = {
:from => AppConfig.mail.sender_address.get,
:host => "#{AppConfig.pod_uri.host}",
:to => name_and_address(@recipient.name, @recipient.email)
from: AppConfig.mail.sender_address.get,
host: "#{AppConfig.pod_uri.host}",
to: name_and_address(@recipient.name, @recipient.email)
}
headers[:from] = "\"#{@sender.name} (diaspora*)\" <#{AppConfig.mail.sender_address}>" if @sender.present?
......@@ -46,14 +47,15 @@ module NotificationMailers
end
def log_mail(recipient_id, sender_id, type)
log_string = "event=mail mail_type=#{type} recipient_id=#{recipient_id} sender_id=#{sender_id}"
if @recipient && @sender
log_string << "models_found=true sender_handle=#{@sender.diaspora_handle} recipient_handle=#{@recipient.diaspora_handle}"
else
log_string << "models_found=false"
end
log_string = "event=mail mail_type=#{type} recipient_id=#{recipient_id} sender_id=#{sender_id} " \
" recipient_handle=#{@recipient.diaspora_handle}"
log_string << " sender_handle=#{@sender.diaspora_handle}" if sender_id.present?
logger.info(log_string)
end
Rails.logger.info(log_string)
def logger
@logger ||= ::Logging::Logger[self]
end
end
end
......@@ -2,9 +2,12 @@ module Workers
module Mail
class Liked < Base
sidekiq_options queue: :mail
def perform(recipient_id, sender_id, like_id)
Notifier.liked(recipient_id, sender_id, like_id).deliver_now
rescue ActiveRecord::RecordNotFound => e
logger.warn("failed to send liked notification mail: #{e.message}")
raise e unless e.message.start_with?("Couldn't find Like with")
end
end
end
......
require "spec_helper"
describe Workers::Mail::Liked do
describe "#perfom" do
it "should call .deliver_now on the notifier object" do
sm = FactoryGirl.build(:status_message, author: bob.person, public: true)
like = FactoryGirl.build(:like, author: alice.person, target: sm)
mail_double = double
expect(mail_double).to receive(:deliver_now)
expect(Notifier).to receive(:liked).with(bob.id, like.author.id, like.id).and_return(mail_double)
Workers::Mail::Liked.new.perform(bob.id, like.author.id, like.id)
end
it "should not fail if the like is not found" do
sm = FactoryGirl.build(:status_message, author: bob.person, public: true)
like = FactoryGirl.build(:like, author: alice.person, target: sm)
expect(Notifier).to receive(:liked).with(bob.id, like.author.id, like.id)
.and_raise(ActiveRecord::RecordNotFound.new("Couldn't find Like with 'id'=42"))
Workers::Mail::Liked.new.perform(bob.id, like.author.id, like.id)
end
it "should fail if the sender is not found" do
sm = FactoryGirl.build(:status_message, author: bob.person, public: true)
like = FactoryGirl.build(:like, author: alice.person, target: sm)
expect(Notifier).to receive(:liked).with(bob.id, like.author.id, like.id)
.and_raise(ActiveRecord::RecordNotFound.new("Couldn't find Person with 'id'=42"))
expect {
Workers::Mail::Liked.new.perform(bob.id, like.author.id, like.id)
}.to raise_error(ActiveRecord::RecordNotFound)
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment