noosfero | 6 new commits pushed to repository

Bráulio Bhavamitra gitlab at gitlab.com
Mon Feb 23 14:55:12 BRT 2015


Bráulio Bhavamitra pushed to refs/heads/master at <a href="https://gitlab.com/noosfero/noosfero">Noosfero / noosfero</a>

Commits:
<a href="https://gitlab.com/noosfero/noosfero/commit/bd55ad6081281be89bcdc90136c341fe89dce0f6">bd55ad60</a> by Victor Costa
Set the asset host for person notifier mailing

- - - - -
<a href="https://gitlab.com/noosfero/noosfero/commit/64b73a348a316646d3445a65ece9920c9b76d508">64b73a34</a> by Victor Costa
Avoid loop in person notification job

- - - - -
<a href="https://gitlab.com/noosfero/noosfero/commit/8b11b55de9144608de7621251c1289cbba793d51">8b11b55d</a> by Victor Costa
Small improvements on layout of person notifier email

- - - - -
<a href="https://gitlab.com/noosfero/noosfero/commit/5012a8e49bdb5348efc32d8972af636a0211811d">5012a8e4</a> by Victor Costa
Display pending tasks in notification email

- - - - -
<a href="https://gitlab.com/noosfero/noosfero/commit/7cf05dde765cd0a583b024f5c54c7de2584e3507">7cf05dde</a> by Victor Costa
Style improvements on person notifier email

- - - - -
<a href="https://gitlab.com/noosfero/noosfero/commit/a39b90c074ae13db605d89acc8d9898c3d8852ec">a39b90c0</a> by Bráulio Bhavamitra
Merge branch 'fix_person_notifier' into 'master'

Improve person notifier email layout

Improve person notifier email layout

Also:
 - Include pending tasks in notification email;
 - Fix image path problem;
 - Fix loop bug.

See merge request !446

- - - - -


Changes:

=====================================
app/models/person_notifier.rb
=====================================
--- a/app/models/person_notifier.rb
+++ b/app/models/person_notifier.rb
@@ -22,12 +22,17 @@ class PersonNotifier
     schedule_next_notification_mail
   end
 
+  def notify_from
+    @person.last_notification || DateTime.now - @person.notification_time.hours
+  end
+
   def notify
     if @person.notification_time && @person.notification_time > 0
-      from = @person.last_notification || DateTime.now - @person.notification_time.hours
-      notifications = @person.tracked_notifications.find(:all, :conditions => ["created_at > ?", from])
+      notifications = @person.tracked_notifications.find(:all, :conditions => ["created_at > ?", notify_from])
+      tasks = Task.to(@person).without_spam.pending.where("created_at > ?", notify_from).order_by('created_at', 'asc')
+
       Noosfero.with_locale @person.environment.default_language do
-        Mailer::content_summary(@person, notifications).deliver unless notifications.empty?
+        Mailer::content_summary(@person, notifications, tasks).deliver unless notifications.empty? && tasks.empty?
       end
       @person.settings[:last_notification] = DateTime.now
       @person.save!
@@ -59,8 +64,12 @@ class PersonNotifier
     end
 
     def failure(job)
-      person = Person.find(person_id)
-      person.notifier.dispatch_notification_mail
+      begin
+        person = Person.find(person_id)
+        person.notifier.dispatch_notification_mail
+      rescue
+        Rails.logger.error "PersonNotifier::NotifyJob: Cannot recover from failure"
+      end
     end
 
   end
@@ -73,18 +82,24 @@ class PersonNotifier
       {:theme => nil}
     end
 
-    def content_summary(person, notifications)
+    def content_summary(person, notifications, tasks)
+      if person.environment
+        ActionMailer::Base.asset_host = person.environment.top_url
+        ActionMailer::Base.default_url_options[:host] = person.environment.default_hostname
+      end
+
       @current_theme = 'default'
       @profile = person
       @recipient = @profile.nickname || @profile.name
       @notifications = notifications
+      @tasks = tasks
       @environment = @profile.environment.name
       @url = @profile.environment.top_url
       mail(
         content_type: "text/html",
         from: "#{@profile.environment.name} <#{@profile.environment.noreply_email}>",
         to: @profile.email,
-        subject: _("[%s] Network Activity") % [@profile.environment.name]
+        subject: _("[%s] Notifications") % [@profile.environment.name]
       )
     end
   end

=====================================
app/views/person_notifier/mailer/_comment.html.erb
=====================================
--- a/app/views/person_notifier/mailer/_comment.html.erb
+++ b/app/views/person_notifier/mailer/_comment.html.erb
@@ -19,7 +19,7 @@
         <span style="font-size: 12px;"><%= comment.title %></span><br/>
       <% end %>
       <span style="font-size: 10px;"><%= txt2html comment.body %></span><br/>
-      <span style="font-size: 8px; color: #444444"><%= time_ago_as_sentence(comment.created_at) %></span>
+      <span style="font-size: 8px; color: #929292"><%= time_ago_as_sentence(comment.created_at) %></span>
   <br style="clear: both;" />
 
   <% unless comment.replies.blank? %>

=====================================
app/views/person_notifier/mailer/_create_article.html.erb
=====================================
--- a/app/views/person_notifier/mailer/_create_article.html.erb
+++ b/app/views/person_notifier/mailer/_create_article.html.erb
@@ -1,13 +1,12 @@
-<table>
 <tr>
-  <td>
+  <td style="width: 11%">
     <%= link_to(profile_image(activity.user, :minor), activity.user.url) %>
   </td>
   <td>
-    <p style="width:550px">
+    <p>
     <span style="font-size: 14px;"><%= link_to activity.user.short_name(20), activity.user.url %></span>
     <span style="font-size: 14px;"><%= _("has published on community %s") % link_to(activity.target.profile.short_name(20), activity.target.profile.url, :style => "color: #333; font-weight: bold; text-decoration: none;") if activity.target.profile.is_a?(Community) %></span>
-    <span style="font-size: 10px; color: #444444; float:right;"><%= time_ago_as_sentence(activity.created_at) %></span>
+    <span style="font-size: 10px; color: #929292; float:right;"><%= time_ago_as_sentence(activity.created_at) %></span>
     </p>
     <p>
     <span style="font-size: 14px;"><%= link_to(activity.params['name'], activity.params['url'], :style => "color: #333; font-weight: bold; text-decoration: none;") %></span>
@@ -24,4 +23,3 @@
     <%= render :partial => 'profile_comments', :locals => { :activity => activity } %>
   </td>
 </tr>
-</table>

=====================================
app/views/person_notifier/mailer/_default_activity.html.erb
=====================================
--- a/app/views/person_notifier/mailer/_default_activity.html.erb
+++ b/app/views/person_notifier/mailer/_default_activity.html.erb
@@ -1,12 +1,11 @@
-<table>
 <tr>
-  <td>
+  <td style="width: 11%">
     <%= link_to(profile_image(activity.user, :minor), activity.user.url) %>
   </td>
   <td>
-    <p style="width:550px">
+    <p>
       <span style="font-size: 14px;"><%= link_to activity.user.name, activity.user.url %> <%= describe activity %></span>
-      <span style="font-size: 10px; color: #444444; float: right;"><%= time_ago_as_sentence(activity.created_at) %></span>
+      <span style="font-size: 10px; color: #929292; float: right;"><%= time_ago_as_sentence(activity.created_at) %></span>
     </p>
   </td>
 </tr>
@@ -16,4 +15,3 @@
     <%= render :partial => 'profile_comments', :locals => { :activity => activity } %>
   </td>
 </tr>
-</table>

=====================================
app/views/person_notifier/mailer/_task.html.erb
=====================================
--- /dev/null
+++ b/app/views/person_notifier/mailer/_task.html.erb
@@ -0,0 +1,20 @@
+<div style="border-bottom:1px solid #e2e2e2;padding:15px 0;width:600px">
+  <table style="width:100%;">
+    <tr>
+      <td style="width: 11%">
+        <%= profile_image(task.requestor, :minor) %>
+      </td>
+      <td>
+        <div>
+          <strong><%= link_to task.title, url_for(:controller => 'tasks', :profile => @profile.identifier, :action => 'index', :only_path => false) %></strong>
+        </div>
+        <div style="font-size: 14px">
+          <span style="font-size: 14px">
+            <%= task_information(task) %>
+          </span>
+          <span style="font-size: 10px; color: #929292; float: right;"><%= time_ago_as_sentence(task.created_at) %></span>
+        </div>
+      </td>
+    </tr>
+  </table>
+</div>

=====================================
app/views/person_notifier/mailer/_upload_image.html.erb
=====================================
--- a/app/views/person_notifier/mailer/_upload_image.html.erb
+++ b/app/views/person_notifier/mailer/_upload_image.html.erb
@@ -1,15 +1,13 @@
-<table>
   <tr>
-    <td>
+    <td style="width: 11%">
       <%= link_to(profile_image(activity.user, :minor), activity.user.url) %>
     </td>
     <td>
-      <p style="width:550px">
+      <p>
         <span style="font-size: 14px;"><%= link_to activity.user.name, activity.user.url %> <%= describe activity %></span>
-        <span style="font-size: 10px; color: #444444; float:right;"><%= time_ago_as_sentence(activity.created_at) %></span>
+        <span style="font-size: 10px; color: #929292; float:right;"><%= time_ago_as_sentence(activity.created_at) %></span>
       </p>
   </td>
 </tr>
-</table>
 <div title='<%= activity.target.class.short_description %>' class='profile-activity-icon icon-new icon-newgallery'></div>
 <br/>

=====================================
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
@@ -1,18 +1,42 @@
-<h3><%= _("%s's network activity") % @profile.name %></h3>
-<br/>
-<div>
-<% @notifications.each do |activity| %>
-  <div style="border-left:none;border-right:none;border-top:1px solid #ccc;border-bottom:none;padding:10px;width:600px">
-    <%= render :partial => activity.verb, :locals => { :activity => activity } rescue "cannot render notification for #{activity.verb}" %>
-  </div>
-<% end %>
-</div>
+<div style="background-color: #EEEEEE">
+  <table style="width: 100%;"><tbody><tr><td align="center">
+  <div style="display: table; background-color: white; margin: 26px 0;">
+    <div style="padding: 25px 20px 20px 20px;text-align: left;">
+      <%= link_to @url, :style => "text-decoration: none;" do %>
+        <span style="font-weight:bold;font-size: 28px;margin: 0;color: white;background-color: #AAAAAA;padding: 5px;"><%= @environment %></span>
+      <% end %>
+      <span style="font-weight:bold;color: #333;font-size:19px;margin-left: 8px;"><%= _("%s's Notifications") % @profile.name %></h3>
+    </div>
+    <div style="margin: 0 20px 20px 20px;border-top:1px solid #e2e2e2;">
+      <% if @tasks.present? %>
+        <div style="border-top: 1px solid #e2e2e2;">
+          <div style="border-bottom: 1px solid #CBCBCB;padding-top: 15px;font-weight: bold;text-align: right;color: #7c7c7c;float: right;min-width: 140px;">
+            <%= _('Tasks') %>
+          </div>
+          <%= render :partial => 'task', :collection => @tasks %>
+        </div>
+      <% end %>
+
+      <% if @notifications.present? %>
+        <div style="border-top: 1px solid #e2e2e2;">
+          <div style="border-bottom: 1px solid #CBCBCB;padding-top: 15px;font-weight: bold;text-align: right;color: #7c7c7c;float: right;min-width: 140px;">
+            <%= _('Network Activity') %>
+          </div>
+          <% @notifications.each do |activity| %>
+              <div style="border-bottom:1px solid #e2e2e2;padding:15px 0;width:600px">
+              <table style="width:100%;">
+                <%= render :partial => activity.verb, :locals => { :activity => activity } rescue "cannot render notification for #{activity.verb}" %>
+              </table>
+            </div>
+          <% end %>
+        </div>
+      <% end %>
 
-<div style="color:#444444;font-size:11px;">
-<p><%= _("Greetings,") %></p>
-<br/>
-<p>--</p>
-<p><%= _('%s team.') % @environment %></p>
-<p><%= url_for @url %></p>
+      <div style="color:#444444;font-size:11px;margin-bottom: 20px;">
+        <p style="margin:0"><%= _("Greetings,") %></p>
+        <p style="margin:0"><%= _('%s team.') % @environment %></p>
+        <p style="margin:0"><%= link_to @url, url_for(@url) %></p>
+      </div>
+    </div>
+  </td></tr></tbody></table>
 </div>
-<br/>

=====================================
test/unit/person_notifier_test.rb
=====================================
--- a/test/unit/person_notifier_test.rb
+++ b/test/unit/person_notifier_test.rb
@@ -225,6 +225,35 @@ class PersonNotifierTest < ActiveSupport::TestCase
     assert !jobs.select {|j| !j.failed? && j.last_error.nil? }.empty?
   end
 
+  should 'render image tags for both internal and external src' do
+    @community.add_member(@member)
+    process_delayed_job_queue
+    notify
+    sent = ActionMailer::Base.deliveries.last
+    assert_match /src="\/\/www.gravatar.com\/avatar.*"/, sent.body.to_s
+    assert_match /src="http:\/\/.*\/images\/icons-app\/community-icon.png.*"/, sent.body.to_s
+  end
+
+  should 'do not raise errors in NotifyJob failure to avoid loop' do
+    Delayed::Worker.max_attempts = 1
+    Delayed::Job.enqueue(PersonNotifier::NotifyJob.new(@member.id))
+
+    PersonNotifier.any_instance.stubs(:notify).raises('error')
+    PersonNotifier.any_instance.stubs(:dispatch_notification_mail).raises('error')
+
+    process_delayed_job_queue
+    jobs = PersonNotifier::NotifyJob.find(@member.id)
+    assert jobs.select {|j| !j.failed? && j.last_error.nil? }.empty?
+  end
+
+  should 'list tasks in notification mail' do
+    task = @member.tasks.create!
+    process_delayed_job_queue
+    notify
+    sent = ActionMailer::Base.deliveries.last
+    assert_match /href=".*\/myprofile\/member\/tasks"/, sent.body.to_s
+  end
+
   private
 
   def notify

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listas.softwarelivre.org/pipermail/noosfero-dev/attachments/20150223/d3e697e9/attachment-0001.html>


More information about the Noosfero-dev mailing list