Redmine

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.

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.

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.

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

Yuichi HARADA wrote:

Go MAEDA wrote:

However, it may insert an orphaned row in the attachments table if Redmine fails to save a Message object for some reason. A possible cause is validation error when saving the Message object.

The following is an example of an orphaned row. Since Redmine failed to save a Message object, container_id and container_type of the Attachment object is nil.

[...]

Thank you for pointing out.
However, I think running the rake task redmine:attachments:prune on a regular basis should solve it.

You are right and I was wrong. Such orphaned attachments may also be created when attaching files to issues and the orphaned files are treated by rake redmine:attachments:prune. So, the patch doesn't have to consider the case and your patch 16006_forum_notification_include_attachments_v2.patch can be merged into the trunk as is.

Now I am setting the target version to 4.2.0.

Committed the patch. Thank you for your contribution.