[Git][noosfero/noosfero][master] 3 commits: Fixes reply_scrap_on_self notification info

[Rodrigo Souto]

Rodrigo Souto pushed to branch master at Noosfero / noosfero


Commits:
1664c4c4 by Gabriel Silva at 2017-04-02T23:11:00+00:00
Fixes reply_scrap_on_self notification info

Signed-off-by: Gabriel Silva <gabriel93.silva at gmail.com>

- - - - -
8cb7a782 by Gabriel Silva at 2017-04-02T23:50:04+00:00
Ignores erorrs when rendering the content summary

Signed-off-by: Gabriel Silva <gabriel93.silva at gmail.com>

- - - - -
e8b2fb35 by Rodrigo Souto at 2017-04-10T17:25:36+00:00
Merge branch 'reply_scrap_notification' into 'master'

Improvements for PersonNotifier::Mailer's content_summary

See merge request !1159
- - - - -


7 changed files:

- app/helpers/action_tracker_helper.rb
- app/models/person_notifier.rb
- app/models/scrap.rb
- app/views/person_notifier/mailer/content_summary.html.erb
- + db/migrate/20170401104432_fixes_tracked_notifications_receiver_info.rb
- test/unit/person_notifier_test.rb
- test/unit/scrap_test.rb


Changes:

=====================================
app/helpers/action_tracker_helper.rb
=====================================
--- a/app/helpers/action_tracker_helper.rb
+++ b/app/helpers/action_tracker_helper.rb
@@ -61,15 +61,19 @@ module ActionTrackerHelper
       tag(:br, style: 'clear: both')
   end
 
-  def reply_scrap_description ta
+  def leave_scrap_description ta
     _('sent a message to %{receiver}: <br /> "%{message}"') % {
       receiver: link_to(ta.get_receiver_name, ta.get_receiver_url),
       message: auto_link_urls(ta.get_content)
     }
   end
 
-  alias :leave_scrap_description :reply_scrap_description
-  alias :reply_scrap_on_self_description :reply_scrap_description
+  def reply_scrap_on_self_description ta
+    _('replied to a scrap from %{receiver}: <br /> "%{message}"') % {
+      receiver: link_to(ta.get_receiver_name, ta.get_receiver_url),
+      message: auto_link_urls(ta.get_content)
+    }
+  end
 
   def leave_scrap_to_self_description ta
     _('wrote: <br /> "%{text}"') % {


=====================================
app/models/person_notifier.rb
=====================================
--- a/app/models/person_notifier.rb
+++ b/app/models/person_notifier.rb
@@ -67,12 +67,8 @@ class PersonNotifier
     end
 
     def failure(job)
-      begin
-        person = Person.find(person_id)
-        person.notifier.dispatch_notification_mail
-      rescue
-        Rails.logger.error "PersonNotifier::NotifyJob: Cannot recover from failure"
-      end
+      person = Person.find(person_id)
+      person.notifier.dispatch_notification_mail
     end
 
   end
@@ -80,6 +76,17 @@ class PersonNotifier
   class Mailer < ApplicationMailer
 
     helper ActionTrackerHelper
+    helper do
+      def render_activity(activity)
+        begin
+          render activity.verb, activity: activity
+        rescue => error
+          Delayed::Worker.logger.warn "PersonNotifier::NotifyJob: Cannot "\
+                                      "render template for #{activity.verb} "\
+                                      "notification: #{error}"
+        end
+      end
+    end
 
     def session
       {:user_theme => nil}


=====================================
app/models/scrap.rb
=====================================
--- a/app/models/scrap.rb
+++ b/app/models/scrap.rb
@@ -32,7 +32,7 @@ class Scrap < ApplicationRecord
 
   track_actions :leave_scrap_to_self, :after_create, :keep_params => ['sender.name', 'content'], :if => Proc.new{|s| s.sender == s.receiver}, :custom_user => :sender
 
-  track_actions :reply_scrap_on_self, :after_create, :keep_params => ['sender.name', 'content'], :if => Proc.new{|s| s.sender != s.receiver && s.sender == s.top_root.receiver}, :custom_user => :sender
+  track_actions :reply_scrap_on_self, :after_create, :keep_params => ['sender.name', 'content', 'receiver.name', 'receiver.url'], :if => Proc.new{|s| s.sender != s.receiver && s.sender == s.top_root.receiver}, :custom_user => :sender
 
   after_create :send_notification
 


=====================================
app/views/person_notifier/mailer/content_summary.html.erb
=====================================
--- a/app/views/person_notifier/mailer/content_summary.html.erb
+++ b/app/views/person_notifier/mailer/content_summary.html.erb
@@ -25,7 +25,7 @@
           <% @notifications.each do |activity| %>
               <div style="border-bottom:1px solid #e2e2e2;padding:15px 0;width:600px">
               <table style="width:100%;table-layout:fixed;">
-                <%= render activity.verb, activity: activity %>
+                <%= render_activity(activity) %>
               </table>
             </div>
           <% end %>


=====================================
db/migrate/20170401104432_fixes_tracked_notifications_receiver_info.rb
=====================================
--- /dev/null
+++ b/db/migrate/20170401104432_fixes_tracked_notifications_receiver_info.rb
@@ -0,0 +1,13 @@
+class FixesTrackedNotificationsReceiverInfo < ActiveRecord::Migration
+  def up
+    ActionTracker::Record.where(verb: 'reply_scrap_on_self').find_each do |n|
+      n.params['receiver_name'] ||= n.target.receiver.name
+      n.params['receiver_url'] ||= n.target.receiver.url
+      n.save
+    end
+  end
+
+  def down
+    say "This migration can't be reverted"
+  end
+end


=====================================
test/unit/person_notifier_test.rb
=====================================
--- a/test/unit/person_notifier_test.rb
+++ b/test/unit/person_notifier_test.rb
@@ -160,9 +160,9 @@ class PersonNotifierTest < ActiveSupport::TestCase
     Comment.create!(:author => @admin, :title => 'test comment', :body => 'body!', :source => @article)
     ActionTracker::Record.any_instance.stubs(:verb).returns("some_invalid_verb")
     process_delayed_job_queue
-    assert_raise ActionView::Template::Error do
-      notify
-    end
+
+    Logger.any_instance.expects(:warn).with(regexp_matches(/#{:some_invalid_verb}/)).at_least(1)
+    notify
   end
 
   Targets = {
@@ -286,6 +286,16 @@ class PersonNotifierTest < ActiveSupport::TestCase
     assert_match /href=".*\/myprofile\/member\/tasks"/, sent.body.to_s
   end
 
+  should 'send the content summary even if one notification raise an exception during render' do
+    @community.add_member(@member)
+    process_delayed_job_queue
+
+    ActionTracker::Record.any_instance.stubs(:get_resource_name).raises(StandardError)
+    assert_difference 'ActionMailer::Base.deliveries.count' do
+      notify
+    end
+  end
+
   private
 
   def notify


=====================================
test/unit/scrap_test.rb
=====================================
--- a/test/unit/scrap_test.rb
+++ b/test/unit/scrap_test.rb
@@ -303,8 +303,11 @@ class ScrapTest < ActiveSupport::TestCase
     assert_difference 'ActionTracker::Record.count', 1 do
       reply = create(Scrap, :sender => r, :receiver => s, :scrap_id => root.id, :content => 'sample')
     end
+
     activity = ActionTracker::Record.last
     assert_equal 'reply_scrap_on_self', activity.verb.to_s
+    assert_equal s.name, activity.get_receiver_name
+    assert_equal s.url, activity.get_receiver_url
   end
 
   should 'create profile activity' do



View it on GitLab: https://gitlab.com/noosfero/noosfero/compare/40c9530a09f05f8567a2bdfa6ba95bb99a0112fe...e8b2fb35592942b988c9bc8c99d656bc59c415c0
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listas.softwarelivre.org/pipermail/noosfero-dev/attachments/20170410/8042d8cb/attachment-0001.html>