Feature #16006
closed
Include attachments in forum post notifications
Added by T. Hauptman over 10 years ago. Updated over 4 years ago.
0%
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.
Files
| notify.patch (1.04 KB) notify.patch | T. Hauptman, 2014-02-01 20:37 | ||
| forum_notification_include_attachments.png (200 KB) forum_notification_include_attachments.png | Yuichi HARADA, 2019-10-29 06:30 | ||
| 16006_forum_notification_include_attachments.patch (3.08 KB) 16006_forum_notification_include_attachments.patch | Yuichi HARADA, 2019-10-29 06:35 | ||
| no-attachment-in-reply@2x.png (91 KB) no-attachment-in-reply@2x.png | Go MAEDA, 2019-12-19 13:06 | ||
| attachment-in-reply.png (370 KB) attachment-in-reply.png | Yuichi HARADA, 2019-12-26 01:55 | ||
| 16006_forum_notification_include_attachments_v2.patch (4.76 KB) 16006_forum_notification_include_attachments_v2.patch | Yuichi HARADA, 2020-01-23 06:07 |
Related issues
Updated by Toshi MARUYAMA about 10 years ago
- Category set to Email notifications
Updated by Toshi MARUYAMA about 10 years ago
- Tracker changed from Patch to Defect
Updated by Toshi MARUYAMA about 10 years ago
- Tracker changed from Defect to Patch
Updated by Toshi MARUYAMA about 10 years ago
- Related to Patch #1616: Allow email to create and reply to forum messages added
Updated by Yuichi HARADA over 4 years ago
- File forum_notification_include_attachments.png forum_notification_include_attachments.png added
- File 16006_forum_notification_include_attachments.patch 16006_forum_notification_include_attachments.patch added
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.
Updated by Go MAEDA over 4 years ago
- Target version set to Candidate for next major release
Updated by Go MAEDA over 4 years 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.
Updated by Yuichi HARADA over 4 years ago
- File attachment-in-reply.png attachment-in-reply.png 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.
Updated by Go MAEDA over 4 years 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
Updated by Yuichi HARADA over 4 years ago
- File 16006_forum_notification_include_attachments_v2.patch 16006_forum_notification_include_attachments_v2.patch 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
Updated by Go MAEDA over 4 years ago
Yuichi HARADA wrote:
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 .
Thank you for updating the patch. The latest patch works fine.
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.
[2] pry(main)> Attachment.last Attachment Load (0.3ms) SELECT "attachments".* FROM "attachments" ORDER BY "attachments"."id" DESC LIMIT ? [["LIMIT", 1]] => #<Attachment:0x00007ffa37fa58f0 id: 28, container_id: nil, container_type: nil, filename: "lenna.jpg", disk_filename: "200130122359_lenna.jpg", filesize: 20401, content_type: "image/jpeg", digest: "632a392af1de2e3de4d8b5ad62da7275550ee0004cdf9f54fa3de05cd28d0aac", downloads: 0, author_id: 1, created_on: Thu, 30 Jan 2020 08:06:16 UTC +00:00, description: nil, disk_directory: "2020/01">
Updated by Yuichi HARADA over 4 years ago
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_idandcontainer_typeof the Attachment object isnil.[...]
Thank you for pointing out.
However, I think running the rake task redmine:attachments:prune on a regular basis should solve it.
Updated by Go MAEDA over 4 years ago
- Target version changed from Candidate for next major release to 4.2.0
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_idandcontainer_typeof the Attachment object isnil.[...]
Thank you for pointing out.
However, I think running the rake taskredmine:attachments:pruneon 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.
Updated by Go MAEDA over 4 years ago
- Tracker changed from Patch to Feature
- Subject changed from Forum post notification does not include attachments to Include attachments in forum post notifications
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the patch. Thank you for your contribution.
Updated by Go MAEDA over 4 years ago
- Related to Feature #33002: Include attachments in news post notifications added