Skip to content

Commit

Permalink
Reply to comment by email feature (#2669)
Browse files Browse the repository at this point in the history
* Added mailman script

* Added migration file for adding comment_via column to comment table

* Added reverse_markdown gem for converting html to markdown format

* Added migration file for adding message_id column to comment table

* Added function to handle received mail to create comment

* Added icon for comment added via email to distinguish between the normal comment and reply by email comment

* Added node_id in Mailer subjects to be used in reply by email feature

* Minor changes

* Updated schema.rb.example file

* Improved tests

* Added migrations

* Added migrations

* Added migrations

* Corrected tests

* Added enviroment variable for server address

* Added sql sphefic conditions

* Unwanted changes in schema.rb.example file is removed

* Gem added

* Log file is moved to public folder

* Corrected tests

* forward-date timestamps

* Rename 20180605190014_add_message_id_column_to_comments.rb to 20180605010014_add_message_id_column_to_comments.rb

* Update schema.rb.example
  • Loading branch information
namangupta01 authored and jywarren committed Jun 5, 2018
1 parent a71cce4 commit fad88fd
Show file tree
Hide file tree
Showing 16 changed files with 127 additions and 28 deletions.
6 changes: 6 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ gem 'sidekiq'
# Whenever provides a clear syntax for writing and deploying cron jobs
gem 'whenever', require: false

# To implement incoming mail processing microframework
gem 'mailman', require: false

# To convert html to markdown
gem 'reverse_markdown'

# run with `bundle install --without production` or `bundle install --without mysql` to exclude this
group :mysql, :production do
gem 'mysql2', '~> 0.3.20'
Expand Down
23 changes: 23 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ GEM
debug_inspector (>= 0.0.1)
builder (3.2.3)
byebug (10.0.2)
celluloid (0.16.0)
timers (~> 4.0.0)
chronic (0.10.2)
ci_reporter (2.0.0)
builder (>= 2.1.2)
Expand Down Expand Up @@ -147,6 +149,7 @@ GEM
hashie (3.5.7)
highline (1.7.10)
hike (1.2.3)
hitimes (1.2.6)
http-cookie (1.0.3)
domain_name (~> 0.5)
http_accept_language (2.1.1)
Expand Down Expand Up @@ -186,11 +189,22 @@ GEM
letter_opener (1.6.0)
launchy (~> 2.2)
libv8 (3.16.14.19)
listen (2.10.1)
celluloid (~> 0.16.0)
rb-fsevent (>= 0.9.3)
rb-inotify (>= 0.9)
loofah (2.2.2)
crass (~> 1.0.2)
nokogiri (>= 1.5.9)
mail (2.7.0)
mini_mime (>= 0.1.1)
maildir (2.2.1)
mailman (0.7.3)
activesupport (>= 2.3.4)
i18n (>= 0.4.1)
listen (~> 2.2)
mail (>= 2.0.3)
maildir (>= 0.5.0)
metaclass (0.0.4)
mime-types (3.1)
mime-types-data (~> 3.2015)
Expand Down Expand Up @@ -315,6 +329,9 @@ GEM
thor (>= 0.18.1, < 2.0)
rainbow (3.0.0)
rake (10.5.0)
rb-fsevent (0.10.3)
rb-inotify (0.9.10)
ffi (>= 0.5.0, < 2)
rb-readline (0.5.5)
rdiscount (2.2.0.1)
recaptcha (4.7.0)
Expand All @@ -330,6 +347,8 @@ GEM
http-cookie (>= 1.0.2, < 2.0)
mime-types (>= 1.16, < 4.0)
netrc (~> 0.8)
reverse_markdown (1.1.0)
nokogiri
rspec (3.7.0)
rspec-core (~> 3.7.0)
rspec-expectations (~> 3.7.0)
Expand Down Expand Up @@ -399,6 +418,8 @@ GEM
thread_safe (0.3.6)
tilt (1.4.1)
timecop (0.9.1)
timers (4.0.4)
hitimes
tins (1.16.3)
tool (0.2.3)
turbolinks (5.1.0)
Expand Down Expand Up @@ -465,6 +486,7 @@ DEPENDENCIES
json_expressions
less-rails (~> 2.6)
letter_opener
mailman
minitest-reporters (~> 1.1.19)
mocha (~> 1.1)
mustermann (~> 0.4)
Expand Down Expand Up @@ -494,6 +516,7 @@ DEPENDENCIES
recaptcha
responders (~> 2.0)
rest-client
reverse_markdown
rspec
rubocop (~> 0.52.1)
ruby-openid
Expand Down
11 changes: 6 additions & 5 deletions app/mailers/comment_mailer.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class CommentMailer < ActionMailer::Base
helper :application
require 'byebug'
include ApplicationHelper
default from: "do-not-reply@#{ActionMailer::Base.default_url_options[:host]}"

Expand All @@ -8,14 +9,14 @@ def notify(user, comment)
@user = user
@comment = comment
@footer = feature('email-footer')
mail(to: user.email, subject: "New comment on '" + comment.parent.title + "'")
mail(to: user.email, subject: "New comment on #{comment.parent.title} (##{comment.parent.id}) ")
end

def notify_note_author(user, comment)
@user = user
@comment = comment
@footer = feature('email-footer')
mail(to: user.email, subject: "New comment on '" + comment.node.title + "'")
mail(to: user.email, subject: "New comment on #{comment.node.title} (##{comment.node.id}) ")
end

# user is awarder, not awardee
Expand All @@ -30,21 +31,21 @@ def notify_callout(comment, user)
@user = user
@comment = comment
@footer = feature('email-footer')
mail(to: user.email, subject: 'You were mentioned in a comment.')
mail(to: user.email, subject: "You were mentioned in a comment. (##{comment.node.id}) ")
end

def notify_tag_followers(comment, user)
@user = user
@comment = comment
@footer = feature('email-footer')
mail(to: user.email, subject: 'A tag you follow was mentioned in a comment.')
mail(to: user.email, subject: "A tag you follow was mentioned in a comment. (##{comment.node.id}) ")
end

def notify_answer_author(user, comment)
@user = user
@comment = comment
@footer = feature('email-footer')
mail(to: user.email, subject: "New comment on your answer on '" + comment.parent.title + "'")
mail(to: user.email, subject: "New comment on your answer on #{comment.parent.title} (##{comment.parent.id}) ")
end

def notify_coauthor(user, note)
Expand Down
3 changes: 1 addition & 2 deletions app/mailers/subscription_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ class SubscriptionMailer < ActionMailer::Base
default from: "do-not-reply@#{ActionMailer::Base.default_url_options[:host]}"

def notify_node_creation(node)
subject = '[PublicLab] ' + (node.has_power_tag('question') ? 'Question: ' : '') +
node.title
subject = '[PublicLab] ' + (node.has_power_tag('question') ? 'Question: ' : '') + node.title + " (##{node.id}) "
@node = node
@tags = node.tags.collect(&:name).join(',')
@footer = feature('email-footer')
Expand Down
16 changes: 15 additions & 1 deletion app/models/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class Comment < ActiveRecord::Base

attr_accessible :pid, :nid, :uid, :aid,
:subject, :hostname, :comment,
:status, :format, :thread, :timestamp
:status, :format, :thread, :timestamp, :comment_via, :message_id

belongs_to :node, foreign_key: 'nid', touch: true, counter_cache: true
# dependent: :destroy, counter_cache: true
Expand Down Expand Up @@ -203,4 +203,18 @@ def likers
User.where(id: likes.pluck(:user_id))
end

def self.receive_mail(message)
node_id = message.subject[/#([\d]+)/, 1] #This took out the node ID from the subject line
unless node_id.nil?
node = Node.find(node_id)
user = User.find_by(email: message.from.first)
if user.present? && node_id.present?
message_markdown = ReverseMarkdown.convert message.html_part.body.decoded
message_id = message.message_id
comment = node.add_comment(uid: user.uid, body: message_markdown, comment_via: 1, message_id: message_id)
comment.notify user
end
end
end

end
9 changes: 8 additions & 1 deletion app/models/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,11 @@ def add_comment(params = {})
else
'01/'
end
if params[:comment_via].nil?
comment_via_status = 0
else
comment_via_status = params[:comment_via].to_i
end
c = Comment.new(pid: 0,
nid: nid,
uid: params[:uid],
Expand All @@ -582,7 +587,9 @@ def add_comment(params = {})
status: 1,
format: 1,
thread: thread,
timestamp: DateTime.now.to_i)
timestamp: DateTime.now.to_i,
comment_via: comment_via_status,
message_id: params[:message_id])
c.save
c
end
Expand Down
6 changes: 5 additions & 1 deletion app/views/notes/_comment.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,19 @@
<% end %>
-->
<% end %>

<a rel="tooltip" title="Flag as spam" class="btn btn-sm btn-default btn-flag-spam-<%= comment.id %>" href="mailto:moderators@publiclab.org?subject=Reporting+spam+on+Public+Lab&body=Hi,+I+found+this+comment+that+looks+like+spam+or+needs+to+be+moderated:+https://publiclab.org/<%= comment.parent.path %>#c<%= comment.cid %>+by+https://publiclab.org/profile/<% if comment.author %><%= comment.author.name %><% end %>+Thanks!">
<i class="fa fa-flag"></i>
</a>
<% if comment.comment_via == 1 %>
<a href="#" onclick="return false;" class="btn btn-default btn-sm"><i class="fa fa-envelope"></i></a>
<% end %>

<% if current_user && comment.uid == current_user.uid %>
<a class="btn btn-default btn-sm" href="javascript:void(0)" onClick="$('#c<%= comment.cid %>edit').toggle();$('#c<%= comment.cid %>show').toggle();setInit(<%= comment.cid %>);">
<i class="fa fa-pencil"></i>
</a>
<% end %>

<% if current_user && (current_user.role == "admin" || current_user.role == "moderator" || comment.uid == current_user.uid || comment.parent.uid == current_user.uid) %>
<a data-remote="true" data-confirm="<%= t('notes._comment.are_you_sure') %> <% if current_user && comment.uid != current_user.uid %><%= t('notes._comment.please_exercise_caution') %><% end %>" href="/comment/delete/<%= comment.cid %>" class="btn btn-default btn-sm" id="c<%= comment.cid %>delete-btn">
<i class='icon fa fa-trash'></i>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddCommentViaColumnToComment < ActiveRecord::Migration
def change
add_column :comments, :comment_via, :integer, :default => 0
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddMessageIdColumnToComments < ActiveRecord::Migration
def change
add_column :comments, :message_id, :string, :default => nil
end
end
6 changes: 4 additions & 2 deletions db/schema.rb.example
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20180525220718) do
ActiveRecord::Schema.define(version: 20180605010014) do

create_table "answer_selections", force: true do |t|
t.integer "user_id"
Expand Down Expand Up @@ -49,6 +49,8 @@ ActiveRecord::Schema.define(version: 20180525220718) do
t.string "mail", limit: 64
t.string "homepage"
t.integer "aid", default: 0, null: false
t.integer "comment_via", limit: 4, default: 0
t.string "message_id", limit: 255
end

add_index "comments", ["comment"], name: "index_comments_on_comment", type: :fulltext if ActiveRecord::Base.connection.adapter_name == 'Mysql2'
Expand Down Expand Up @@ -450,4 +452,4 @@ ActiveRecord::Schema.define(version: 20180525220718) do
add_index "users", ["name"], name: "index_users_name", using: :btree
add_index "users", ["uid"], name: "index_users_uid", using: :btree

end
end
29 changes: 29 additions & 0 deletions script/mailman_server
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/usr/bin/env ruby
require "rubygems"
require "bundler/setup"
require "mailman"

Mailman.config.logger = Logger.new("public/mailman.log")

# Email Configration for mailman
Mailman.config.pop3 = {
server: ENV['SERVER_ADDRESS'], # example pop.example.com
port: 995,
ssl: true,
username: ENV['USERNAME'],
password: ENV['EMAIL_PASSWORD']
}

Mailman::Application.run do
# routes are written here
# route for mail having node with id in their subject will only be accepted for processing
default do
begin
Comment.receive_mail(message)
rescue Exception => e
Mailman.logger.error "Exception occurred while receiving message:\n#{message}"
Mailman.logger.error [e, *e.backtrace].join("\n")
end
end

end
2 changes: 1 addition & 1 deletion test/functional/admin_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ def teardown
# test general subscription notices
# (we test the final one, but there are many)
email = ActionMailer::Base.deliveries.last
assert_equal '[PublicLab] ' + node.title, email.subject
assert_equal "[PublicLab] #{node.title} (##{node.id}) ", email.subject
end

test "moderator user should not be able to publish a note if it's already published" do
Expand Down
9 changes: 6 additions & 3 deletions test/functional/comment_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ def teardown
UserSession.create(users(:bob))
assert_difference 'Comment.count' do
xhr :post, :create,
id: nodes(:one).nid,
body: 'Notes comment'
{
id: nodes(:one).nid,
body: 'Notes comment'
}

end
assert_response :success
assert_not_nil :comment
Expand Down Expand Up @@ -257,7 +260,7 @@ def teardown
id: nodes(:wiki_page).nid,
body: 'A comment by Jeff on a wiki page of author bob',
type: 'page'
assert ActionMailer::Base.deliveries.collect(&:subject).include?("New comment on 'Wiki page title'")
assert ActionMailer::Base.deliveries.collect(&:subject).include?("New comment on Wiki page title (#11) ")
end

test 'should prompt user if comment includes question mark' do
Expand Down
10 changes: 5 additions & 5 deletions test/functional/notes_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ def teardown
end

email = ActionMailer::Base.deliveries.last
assert_equal '[PublicLab] ' + title, email.subject
assert_equal '[PublicLab] ' + title + ' (#' + Node.last.id.to_s + ') ', email.subject
assert_equal 1, Node.last.status
assert_redirected_to '/notes/' + users(:jeff).username + '/' + Time.now.strftime('%m-%d-%Y') + '/' + title.parameterize
end
Expand Down Expand Up @@ -708,7 +708,7 @@ def test_get_rss_feed
assert_redirected_to '/notes/' + users(:jeff).username + '/' + Time.now.strftime('%m-%d-%Y') + '/' + node.title.parameterize

email = ActionMailer::Base.deliveries.last
assert_equal '[PublicLab] ' + node.title, email.subject
assert_equal '[PublicLab] ' + node.title + " (##{node.id}) ", email.subject
end

test 'draft author can publish the draft' do
Expand All @@ -727,7 +727,7 @@ def test_get_rss_feed
assert_redirected_to '/notes/' + users(:jeff).username + '/' + Time.now.strftime('%m-%d-%Y') + '/' + node.title.parameterize

email = ActionMailer::Base.deliveries.last
assert_equal '[PublicLab] ' + node.title, email.subject
assert_equal '[PublicLab] ' + node.title + " (##{node.id}) ", email.subject
end

test 'co-author can publish the draft' do
Expand All @@ -746,7 +746,7 @@ def test_get_rss_feed
assert_redirected_to '/notes/' + users(:jeff).username + '/' + Time.now.strftime('%m-%d-%Y') + '/' + node.title.parameterize

email = ActionMailer::Base.deliveries.last
assert_equal '[PublicLab] ' + node.title, email.subject
assert_equal '[PublicLab] ' + node.title + " (##{node.id}) ", email.subject
end

test 'Normal user should not be allowed to publish the draft' do
Expand Down Expand Up @@ -808,7 +808,7 @@ def test_get_rss_feed
draft: "true"

email = ActionMailer::Base.deliveries.last
assert_equal '[PublicLab] ' + title, email.subject
assert_equal '[PublicLab] ' + title + " (##{Node.last.id}) ", email.subject
assert_equal 3, Node.last.status
assert_equal I18n.t('notes_controller.saved_as_draft'), flash[:notice]
assert_redirected_to '/notes/' + users(:jeff).username + '/' + Time.now.strftime('%m-%d-%Y') + '/' + title.parameterize
Expand Down
Loading

0 comments on commit fad88fd

Please sign in to comment.