Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[XWIKI-20109] Notification headers for autoreplies #1907

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ public MimeMessage next()

updateFactoryParameters(templateDocumentReference);
message = this.factory.createMessage(templateDocumentReference, this.factoryParameters);
message.setHeader("Auto-Submitted", "auto-generated");
message.setHeader("X-Auto-Response-Suppress", "All");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to see this feature exist in the protocol.

However, I'm not sure this is the best location, I would expect something like that to be more somewhere in createMessage itself but looks like we don't really have any common code between message factories so that would mean adding it to each of them.

I guess another possibility would be to make it the default in ExtendedMimeMessage constructor, but it might be a bit too much.

WDYT @vmassol ?

Copy link
Member

@surli surli Aug 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICS only Auto-Submitted is defined in the RFC (https://www.rfc-editor.org/rfc/rfc5436.html#section-2.7.1) the other one is defined by Microsoft according to https://www.arp242.net/autoreply.html.

Would be good to maybe add some comments to at least explain that. Also reading a bit more the RFC it sounds like the Auto-Submitted field should include the owner-email:

The header field "Auto-Submitted: auto-notified" MUST include one or
both of the following parameters: [owner-email or owner-token]

[EDIT: actually it's only for auto-notified value, here the proposal is to use auto-generated which does not have any parameter]

I guess another possibility would be to make it the default in ExtendedMimeMessage constructor, but it might be a bit too much.

According to the RFC the header could contain auto-generated or auto-notified. IMO it makes to always have the header in the emails sent by XWiki, now it's not necessarly always auto-generated: it would makes sense to use auto-notified for notification system.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would makes sense to use auto-notified for notification system.

the RFC actually says:

Keyword value: auto-notified
Description: Indicates that a message was generated by a Sieve
notification system.

we're not in that case since it's not a Sieve notification system. So I guess we can just commit to keep auto-generated. I'm not yet entirely sure about putting that to ExtendedMimeMessage: what if some extensions / scripts reuse that to send mails that are not generated?
We could maybe go at least for a first version to keep it there only. WDYT?


List<EntityEvent> events = new ArrayList<>();
this.currentEvents.forEach(
Expand Down