Patch #16006

Forum post notification does not include attachments

Added by T. Hauptman almost 6 years ago. Updated about 2 hours ago.

Status:NewStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Email notifications
Target version:Candidate for next major release

Description

The send_notification is called on message creation, before attachments are added. In my case, I need links to attachments to be sent in the notification email.

My app/views/mailer/message_posted.text.erb do do this is:

<%= @message.content %>

<% if @message.attachments.any? -%>
---<%= l(:label_attachment_plural).ljust(37, '-') %>
<% @message.attachments.each do |attachment| -%>
<%=attachment.filename%>: <%=send(:named_attachment_path, attachment, attachment.filename,:only_path=>false) %> (<%= number_to_human_size(attachment.filesize) %>)
<% end -%>
<% end -%>
<%= @author.firstname + " " + @author.lastname + " - "+ @author.mail %>

If the forum message is created by an incoming email (with attachments) then no attachements are shown in outgoing notifications. If the forum message is created via the web form, the attachments are shown.

I fixed this by changing the send_notification trigger to after_commit. Patch attached.

notify.patch Magnifier (1.04 KB) T. Hauptman, 2014-02-01 20:37

forum_notification_include_attachments.png (200 KB) Yuichi HARADA, 2019-10-29 06:30

16006_forum_notification_include_attachments.patch Magnifier (3.08 KB) Yuichi HARADA, 2019-10-29 06:35

no-attachment-in-reply@2x.png (91 KB) Go MAEDA, 2019-12-19 13:06

attachment-in-reply.png (370 KB) Yuichi HARADA, 2019-12-26 01:55

16006_forum_notification_include_attachments_v2.patch Magnifier (4.76 KB) Yuichi HARADA, 2020-01-23 06:07


Related issues

Related to Redmine - Patch #1616: Allow email to create and reply to forum messages New 2008-07-10

History

#1 Updated by Toshi MARUYAMA almost 6 years ago

  • Category set to Email notifications

#2 Updated by Toshi MARUYAMA almost 6 years ago

  • Tracker changed from Patch to Defect

#3 Updated by Toshi MARUYAMA almost 6 years ago

  • Tracker changed from Defect to Patch

#4 Updated by Toshi MARUYAMA almost 6 years ago

  • Related to Patch #1616: Allow email to create and reply to forum messages added

#5 Updated by Yuichi HARADA 3 months ago

T. Hauptman wrote:

I fixed this by changing the send_notification trigger to after_commit.

Already improved by source:trunk/app/models/message.rb@17588#L48 . However, the attachments are not included in forum post notification.
I created a patch to include attachments in forum post notification.

#6 Updated by Go MAEDA 3 months ago

  • Target version set to Candidate for next major release

#7 Updated by Go MAEDA about 1 month ago

Thank you for posting the patch. I tried out the patch but I am experiencing a problem that attachments are included only when a topic is created. No attachments information is in a notification when a reply is added.

#8 Updated by Yuichi HARADA 28 days ago

Go MAEDA wrote:

Thank you for posting the patch. I tried out the patch but I am experiencing a problem that attachments are included only when a topic is created. No attachments information is in a notification when a reply is added.

It cannot be reproduced. Attachments information is in a notification when a reply is added.

#9 Updated by Go MAEDA 20 days ago

I still see the problem when adding a reply.

I found that container_type and container_id have not been set to the attachment when sending notifications. Those values are set after the delivery is finished. So, @message.attachments.any? in app/views/mailer/message_posted.html.erb returns false when generating notification emails.

[ActiveJob] Enqueued ActionMailer::DeliveryJob (Job ID: ce613aa3-4241-4bc4-b59d-009475839967) to Inline(mailers) with arguments: "Mailer", "message_posted", "deliver_now", #<GlobalID:0x00007ff1b32ac090 @uri=#<URI::GID gid://redmine-app/User/1>>, #<GlobalID:0x00007ff1b0f1b680 @uri=#<URI::GID gid://redmine-app/Message/9>>
  Attachment Load (0.3ms)  SELECT  "attachments".* FROM "attachments" WHERE "attachments"."id" = ? AND "attachments"."digest" = ? LIMIT ?  [["id", 29], ["digest", "632a392af1de2e3de4d8b5ad62da7275550ee0004cdf9f54fa3de05cd28d0aac"], ["LIMIT", 1]]
   (1.0ms)  begin transaction
  CACHE User Load (0.0ms)  SELECT  "users".* FROM "users" WHERE "users"."type" IN ('User', 'AnonymousUser') AND "users"."id" = ? LIMIT ?  [["id", 1], ["LIMIT", 1]]
  Attachment Update (1.1ms)  UPDATE "attachments" SET "container_id" = ?, "description" = ?, "container_type" = ? WHERE "attachments"."id" = ?  [["container_id", 9], ["description", ""], ["container_type", "Message"], ["id", 29]]
   (1.2ms)  commit transaction

#10 Updated by Yuichi HARADA about 2 hours ago

Go MAEDA wrote:

I still see the problem when adding a reply.

Perhaps I think that Message's callback method after_create_commit ( Message#send_notification ) fired before saving the attachments.
Add the following patch to 16006_forum_notification_include_attachments.patch .

diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb
index bbc6a35d4..2a0e341b6 100644
--- a/app/controllers/messages_controller.rb
+++ b/app/controllers/messages_controller.rb
@@ -77,10 +77,10 @@ class MessagesController < ApplicationController
     @reply.author = User.current
     @reply.board = @board
     @reply.safe_attributes = params[:reply]
+    @reply.save_attachments(params[:attachments])
     @topic.children << @reply
     if !@reply.new_record?
       call_hook(:controller_messages_reply_after_save, { :params => params, :message => @reply})
-      attachments = Attachment.attach_files(@reply, params[:attachments])
       render_attachment_warning_if_needed(@reply)
     end
     flash[:notice] = l(:notice_successful_update)
@@ -91,12 +91,14 @@ class MessagesController < ApplicationController
   def edit
     (render_403; return false) unless @message.editable_by?(User.current)
     @message.safe_attributes = params[:message]
-    if request.post? && @message.save
-      attachments = Attachment.attach_files(@message, params[:attachments])
-      render_attachment_warning_if_needed(@message)
-      flash[:notice] = l(:notice_successful_update)
-      @message.reload
-      redirect_to board_message_path(@message.board, @message.root, :r => (@message.parent_id && @message.id))
+    if request.post?
+      @message.save_attachments(params[:attachments])
+      if @message.save
+        render_attachment_warning_if_needed(@message)
+        flash[:notice] = l(:notice_successful_update)
+        @message.reload
+        redirect_to board_message_path(@message.board, @message.root, :r => (@message.parent_id && @message.id))
+      end
     end
   end

</diff>

Also available in: Atom PDF