Commit 8e0ac476620bc7c71ba815f43b18fbcdabbc182e
1 parent
692325d6
Exists in
master
and in
29 other branches
Subscribe users to be notified about new comments on article
(ActionItem2375)
Showing
12 changed files
with
156 additions
and
3 deletions
Show diff stats
app/controllers/public/content_viewer_controller.rb
@@ -99,6 +99,14 @@ class ContentViewerController < ApplicationController | @@ -99,6 +99,14 @@ class ContentViewerController < ApplicationController | ||
99 | @images = @images.paginate(:per_page => per_page, :page => params[:npage]) unless params[:slideshow] | 99 | @images = @images.paginate(:per_page => per_page, :page => params[:npage]) unless params[:slideshow] |
100 | end | 100 | end |
101 | 101 | ||
102 | + @unfollow_form = params[:unfollow] && params[:unfollow] == 'true' | ||
103 | + if params[:unfollow] && params[:unfollow] == 'commit' && request.post? | ||
104 | + @page.followers -= [params[:email]] | ||
105 | + if @page.save | ||
106 | + session[:notice] = _("Notification of new comments to '%s' canceled") % params[:email] | ||
107 | + end | ||
108 | + end | ||
109 | + | ||
102 | @comments = @page.comments(true).as_thread | 110 | @comments = @page.comments(true).as_thread |
103 | @comments_count = @page.comments.count | 111 | @comments_count = @page.comments.count |
104 | if params[:slideshow] | 112 | if params[:slideshow] |
app/models/article.rb
@@ -34,6 +34,7 @@ class Article < ActiveRecord::Base | @@ -34,6 +34,7 @@ class Article < ActiveRecord::Base | ||
34 | settings_items :display_hits, :type => :boolean, :default => true | 34 | settings_items :display_hits, :type => :boolean, :default => true |
35 | settings_items :author_name, :type => :string, :default => "" | 35 | settings_items :author_name, :type => :string, :default => "" |
36 | settings_items :allow_members_to_edit, :type => :boolean, :default => false | 36 | settings_items :allow_members_to_edit, :type => :boolean, :default => false |
37 | + settings_items :followers, :type => Array, :default => [] | ||
37 | 38 | ||
38 | belongs_to :reference_article, :class_name => "Article", :foreign_key => 'reference_article_id' | 39 | belongs_to :reference_article, :class_name => "Article", :foreign_key => 'reference_article_id' |
39 | 40 |
app/models/comment.rb
@@ -75,11 +75,30 @@ class Comment < ActiveRecord::Base | @@ -75,11 +75,30 @@ class Comment < ActiveRecord::Base | ||
75 | article.comments_updated if article.kind_of?(Article) | 75 | article.comments_updated if article.kind_of?(Article) |
76 | end | 76 | end |
77 | 77 | ||
78 | - after_create do |comment| | ||
79 | - if comment.source.kind_of?(Article) && comment.article.notify_comments? && !comment.article.profile.notification_emails.empty? | ||
80 | - Comment::Notifier.deliver_mail(comment) | 78 | + after_create :new_follower |
79 | + def new_follower | ||
80 | + if source.kind_of?(Article) | ||
81 | + article.followers += [author_email] | ||
82 | + article.followers -= article.profile.notification_emails | ||
83 | + article.followers.uniq! | ||
84 | + article.save | ||
85 | + end | ||
86 | + end | ||
87 | + | ||
88 | + after_create :notify_by_mail | ||
89 | + def notify_by_mail | ||
90 | + if source.kind_of?(Article) && article.notify_comments? | ||
91 | + if !article.profile.notification_emails.empty? | ||
92 | + Comment::Notifier.deliver_mail(self) | ||
93 | + end | ||
94 | + emails = article.followers - [author_email] | ||
95 | + if !emails.empty? | ||
96 | + Comment::Notifier.deliver_mail_to_followers(self, emails) | ||
97 | + end | ||
81 | end | 98 | end |
99 | + end | ||
82 | 100 | ||
101 | + after_create do |comment| | ||
83 | if comment.source.kind_of?(Article) | 102 | if comment.source.kind_of?(Article) |
84 | comment.article.create_activity if comment.article.activity.nil? | 103 | comment.article.create_activity if comment.article.activity.nil? |
85 | if comment.article.activity | 104 | if comment.article.activity |
@@ -138,6 +157,22 @@ class Comment < ActiveRecord::Base | @@ -138,6 +157,22 @@ class Comment < ActiveRecord::Base | ||
138 | :environment => profile.environment.name, | 157 | :environment => profile.environment.name, |
139 | :url => profile.environment.top_url | 158 | :url => profile.environment.top_url |
140 | end | 159 | end |
160 | + def mail_to_followers(comment, emails) | ||
161 | + profile = comment.article.profile | ||
162 | + bcc emails | ||
163 | + from "#{profile.environment.name} <#{profile.environment.contact_email}>" | ||
164 | + subject _("[%s] %s commented on a content of %s") % [profile.environment.name, comment.author_name, profile.short_name] | ||
165 | + body :recipient => profile.nickname || profile.name, | ||
166 | + :sender => comment.author_name, | ||
167 | + :sender_link => comment.author_link, | ||
168 | + :article_title => comment.article.name, | ||
169 | + :comment_url => comment.url, | ||
170 | + :unsubscribe_url => comment.article.view_url.merge({:unfollow => true}), | ||
171 | + :comment_title => comment.title, | ||
172 | + :comment_body => comment.body, | ||
173 | + :environment => profile.environment.name, | ||
174 | + :url => profile.environment.top_url | ||
175 | + end | ||
141 | end | 176 | end |
142 | 177 | ||
143 | def rejected? | 178 | def rejected? |
app/models/user.rb
@@ -114,6 +114,7 @@ class User < ActiveRecord::Base | @@ -114,6 +114,7 @@ class User < ActiveRecord::Base | ||
114 | 114 | ||
115 | # Activates the user in the database. | 115 | # Activates the user in the database. |
116 | def activate | 116 | def activate |
117 | + return false unless self.person | ||
117 | self.activated_at = Time.now.utc | 118 | self.activated_at = Time.now.utc |
118 | self.activation_code = nil | 119 | self.activation_code = nil |
119 | self.person.visible = true | 120 | self.person.visible = true |
@@ -0,0 +1,22 @@ | @@ -0,0 +1,22 @@ | ||
1 | +<%= _('Hi!') %> | ||
2 | + | ||
3 | +<%= word_wrap(_('%{sender} (%{sender_link}) commented on the content "%{article_title}".') % { :sender => @sender, :sender_link => url_for(@sender_link), :article_title => @article_title }) %> | ||
4 | + | ||
5 | +<%= word_wrap(_('Title: %s') % @comment_title) %> | ||
6 | + | ||
7 | +<%= _("Comment:") %> | ||
8 | +------------------------------------------------------------------------------- | ||
9 | +<%= word_wrap(@comment_body) %> | ||
10 | +------------------------------------------------------------------------------- | ||
11 | + | ||
12 | +<%= _('Access the address below to view this comment:') %> | ||
13 | +<%= url_for @comment_url %> | ||
14 | + | ||
15 | +<%= _('Access the address below to cancel comments notification:') %> | ||
16 | +<%= url_for @unsubscribe_url %> | ||
17 | + | ||
18 | +<%= _("Greetings,") %> | ||
19 | + | ||
20 | +-- | ||
21 | +<%= _('%s team.') % @environment %> | ||
22 | +<%= url_for @url %> |
@@ -0,0 +1,12 @@ | @@ -0,0 +1,12 @@ | ||
1 | +<% if @unfollow_form %> | ||
2 | +<div class='unfollow-article'> | ||
3 | + <h1><%= _('Cancel notification of new comments') %></h1> | ||
4 | + <% form_tag(@page.view_url.merge({:only_path => true}), {:method => 'post', :class => 'comment_form'}) do %> | ||
5 | + <%= hidden_field_tag(:unfollow, 'commit') %> | ||
6 | + <%= labelled_form_field(_('Enter your e-Mail'), text_field_tag(:email, nil, {:size => 40})) %> | ||
7 | + <% button_bar do %> | ||
8 | + <%= submit_button(:ok, _('Cancel notifications for e-mail above') ) %> | ||
9 | + <% end %> | ||
10 | + <% end %> | ||
11 | +</div> | ||
12 | +<% end %> |
app/views/content_viewer/view_page.rhtml
@@ -6,6 +6,8 @@ | @@ -6,6 +6,8 @@ | ||
6 | 6 | ||
7 | <div id="article" class="<%= @page.css_class_name %>"> | 7 | <div id="article" class="<%= @page.css_class_name %>"> |
8 | 8 | ||
9 | +<%= render :partial => 'confirm_unfollow' %> | ||
10 | + | ||
9 | <div id="article-toolbar"></div> | 11 | <div id="article-toolbar"></div> |
10 | 12 | ||
11 | <script type="text/javascript"> | 13 | <script type="text/javascript"> |
public/stylesheets/application.css
@@ -5896,6 +5896,10 @@ h1#agenda-title { | @@ -5896,6 +5896,10 @@ h1#agenda-title { | ||
5896 | 5896 | ||
5897 | /* }}} */ | 5897 | /* }}} */ |
5898 | 5898 | ||
5899 | +.unfollow-article { | ||
5900 | + margin-bottom: 40px; | ||
5901 | +} | ||
5902 | + | ||
5899 | #alt-beautify { | 5903 | #alt-beautify { |
5900 | background-color: #EDEDED; | 5904 | background-color: #EDEDED; |
5901 | border: 1px solid #666666; | 5905 | border: 1px solid #666666; |
test/unit/article_test.rb
@@ -1705,6 +1705,11 @@ class ArticleTest < ActiveSupport::TestCase | @@ -1705,6 +1705,11 @@ class ArticleTest < ActiveSupport::TestCase | ||
1705 | assert !a.allow_edit?(nil) | 1705 | assert !a.allow_edit?(nil) |
1706 | end | 1706 | end |
1707 | 1707 | ||
1708 | + should 'has a empty list of followers by default' do | ||
1709 | + a = Article.new | ||
1710 | + assert_equal [], a.followers | ||
1711 | + end | ||
1712 | + | ||
1708 | should 'get first image from lead' do | 1713 | should 'get first image from lead' do |
1709 | a = fast_create(Article, :body => '<p>Foo</p><p><img src="bar.png" />Bar<img src="foo.png" /></p>', | 1714 | a = fast_create(Article, :body => '<p>Foo</p><p><img src="bar.png" />Bar<img src="foo.png" /></p>', |
1710 | :abstract => '<p>Lead</p><p><img src="leadbar.png" />Bar<img src="leadfoo.png" /></p>') | 1715 | :abstract => '<p>Lead</p><p><img src="leadbar.png" />Bar<img src="leadfoo.png" /></p>') |
test/unit/comment_notifier_test.rb
@@ -65,6 +65,21 @@ class CommentNotifierTest < ActiveSupport::TestCase | @@ -65,6 +65,21 @@ class CommentNotifierTest < ActiveSupport::TestCase | ||
65 | end | 65 | end |
66 | end | 66 | end |
67 | 67 | ||
68 | + should "deliver mail to followers" do | ||
69 | + author = create_user('follower_author').person | ||
70 | + follower = create_user('follower').person | ||
71 | + @article.followers += [follower.email] | ||
72 | + @article.save! | ||
73 | + @article.comments << Comment.new(:source => @article, :author => author, :title => 'comment title', :body => 'comment body') | ||
74 | + assert_includes ActionMailer::Base.deliveries.map(&:bcc).flatten, follower.email | ||
75 | + end | ||
76 | + | ||
77 | + should "not deliver follower's mail about new comment to comment's author" do | ||
78 | + follower = create_user('follower').person | ||
79 | + @article.comments << Comment.new(:source => @article, :author => follower, :title => 'comment title', :body => 'comment body') | ||
80 | + assert_not_includes ActionMailer::Base.deliveries.map(&:bcc).flatten, follower.email | ||
81 | + end | ||
82 | + | ||
68 | private | 83 | private |
69 | 84 | ||
70 | def read_fixture(action) | 85 | def read_fixture(action) |
test/unit/comment_test.rb
@@ -355,6 +355,48 @@ class CommentTest < ActiveSupport::TestCase | @@ -355,6 +355,48 @@ class CommentTest < ActiveSupport::TestCase | ||
355 | assert c.rejected? | 355 | assert c.rejected? |
356 | end | 356 | end |
357 | 357 | ||
358 | + should 'subscribe user as follower of an article on new comment' do | ||
359 | + owner = create_user('owner_of_article').person | ||
360 | + person = create_user('follower').person | ||
361 | + article = fast_create(Article, :profile_id => owner.id) | ||
362 | + assert_not_includes article.followers, person.email | ||
363 | + article.comments.create!(:source => article, :author => person, :title => 'new comment', :body => 'new comment') | ||
364 | + assert_includes article.reload.followers, person.email | ||
365 | + end | ||
366 | + | ||
367 | + should 'subscribe guest user as follower of an article on new comment' do | ||
368 | + article = fast_create(Article, :profile_id => create_user('article_owner').person.id) | ||
369 | + assert_not_includes article.followers, 'follower@example.com' | ||
370 | + article.comments.create!(:source => article, :name => 'follower', :email => 'follower@example.com', :title => 'new comment', :body => 'new comment') | ||
371 | + assert_includes article.reload.followers, 'follower@example.com' | ||
372 | + end | ||
373 | + | ||
374 | + should 'keep unique emails in list of followers' do | ||
375 | + article = fast_create(Article, :profile_id => create_user('article_owner').person.id) | ||
376 | + article.comments.create!(:source => article, :name => 'follower one', :email => 'follower@example.com', :title => 'new comment', :body => 'new comment') | ||
377 | + article.comments.create!(:source => article, :name => 'follower two', :email => 'follower@example.com', :title => 'another comment', :body => 'new comment') | ||
378 | + assert_equal 1, article.reload.followers.select{|v| v == 'follower@example.com'}.count | ||
379 | + end | ||
380 | + | ||
381 | + should 'not subscribe owner as follower of an article on new comment' do | ||
382 | + owner = create_user('owner_of_article').person | ||
383 | + article = fast_create(Article, :profile_id => owner.id) | ||
384 | + article.comments.create!(:source => article, :author => owner, :title => 'new comment', :body => 'new comment') | ||
385 | + assert_not_includes article.reload.followers, owner.email | ||
386 | + end | ||
387 | + | ||
388 | + should 'not subscribe admins as follower of an article on new comment' do | ||
389 | + owner = fast_create(Community) | ||
390 | + follower = create_user('follower').person | ||
391 | + admin = create_user('admin_of_community').person | ||
392 | + owner.add_admin(admin) | ||
393 | + article = fast_create(Article, :profile_id => owner.id) | ||
394 | + article.comments.create!(:source => article, :author => follower, :title => 'new comment', :body => 'new comment') | ||
395 | + article.comments.create!(:source => article, :author => admin, :title => 'new comment', :body => 'new comment') | ||
396 | + assert_not_includes article.reload.followers, admin.email | ||
397 | + assert_includes article.followers, follower.email | ||
398 | + end | ||
399 | + | ||
358 | should 'update article activity when add a comment' do | 400 | should 'update article activity when add a comment' do |
359 | profile = create_user('testuser').person | 401 | profile = create_user('testuser').person |
360 | article = create(TinyMceArticle, :profile => profile) | 402 | article = create(TinyMceArticle, :profile => profile) |
test/unit/user_test.rb
@@ -486,6 +486,12 @@ class UserTest < ActiveSupport::TestCase | @@ -486,6 +486,12 @@ class UserTest < ActiveSupport::TestCase | ||
486 | assert new_user.activated? | 486 | assert new_user.activated? |
487 | end | 487 | end |
488 | 488 | ||
489 | + should 'cancel activation if user has no person associated' do | ||
490 | + user = new_user | ||
491 | + user.stubs(:person).returns(nil) | ||
492 | + assert !user.activate | ||
493 | + end | ||
494 | + | ||
489 | protected | 495 | protected |
490 | def new_user(options = {}) | 496 | def new_user(options = {}) |
491 | user = User.new({ :login => 'quire', :email => 'quire@example.com', :password => 'quire', :password_confirmation => 'quire' }.merge(options)) | 497 | user = User.new({ :login => 'quire', :email => 'quire@example.com', :password => 'quire', :password_confirmation => 'quire' }.merge(options)) |