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.

Sign up for GitHub

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

Jump to bottom

Adding option to set Auto-Submitted email header #35876

Merged
merged 14 commits into from
Jan 4, 2023
Merged

Adding option to set Auto-Submitted email header #35876

merged 14 commits into from
Jan 4, 2023

Conversation

bennet0496
Copy link
Contributor

Related to issue #16538 and PR #27200

As previously discussed an proposed in the issue and PR, this PR add the option Auto-Submitted, which has the main purpose of stopping properly configured mail server from sending OOO-Auto-Replys in response to fully automatic Nextcloud notifications. However as @nickvergessen mentioned in #27200 (comment) it might not be ideal to hard code the header for any email sent by Nextcloud, for this reason I added the method setAutoSubmitted(string $value) to OC\Mail\Message, which allows apps to dynamically set this header if it is deemed appropriate. The current default is to not set the Header which is equivalent to Auto-Submitted: no. The set of valid values is available with OCP\Mail\AutoSubmittedValue.

Additionally I set this Auto-Submitted value for new user welcome mail (NewUserMailHelper in apps/settings) and CalDAV notifications (OCA\DAV\CalDAV\Reminder\NotificationProvider in apps/dav)

The image show the header being adding to the new user mail, but not the test mail. I was not able to trigger a DAV notification in my 5min test installation until now, but as the code is the same I expect similar results.
image

Unit tests for the new methods are added as well.

@szaimen szaimen added this to the Nextcloud 26 milestone Dec 23, 2022
@szaimen szaimen added enhancement 3. to review Waiting for reviews labels Dec 23, 2022
@szaimen szaimen requested review from a team, PVince81 and icewind1991 and removed request for a team December 23, 2022 13:30
bennet0496 and others added 7 commits December 23, 2022 19:26
@bennet0496
Adding API method to set the Auto-Submitted header to OCP\Mail\IMessage
b3d9b57 
and implementation in OC\Mail\Message

Signed-off-by: Bennet Becker <bbecker@pks.mpg.de>
@bennet0496
Setting Auto-Submitted header for calendar events and new user mail
7a9f4fb 
Signed-off-by: Bennet Becker <bbecker@pks.mpg.de>
@bennet0496
fix getAutoSubmitted return type and adding unit tests
c1c6d21 
Signed-off-by: Bennet Becker <bbecker@pks.mpg.de>
@bennet0496
ran build/autoloaderchecker.sh after adding AutoSubmittedValue class
77569ba 
Signed-off-by: Bennet Becker <bbecker@pks.mpg.de>
@bennet0496
Updating unit test for NewUserMailHelper, to test for autosumbitted
eec1ebb 
header

Signed-off-by: Bennet Becker <bbecker@pks.mpg.de>
@bennet0496 @ChristophWurst
Update PHPDoc annotation in lib/public/Mail/IMessage.php
4b992cd 
Co-authored-by: Christoph Wurst <ChristophWurst@users.noreply.github.com>
Signed-off-by: Bennet B <bennet0496@users.noreply.github.com>
@bennet0496
fix static-analysis error and formatting code
411467a 
Signed-off-by: Bennet Becker <dev@bennet.cc>
@bennet0496
revert .htaccess to commit previous to PR
f5bf251 
(was changed by autoloaderchecker.sh or composer?)

Signed-off-by: Bennet Becker <dev@bennet.cc>
tcitworld
tcitworld reviewed Dec 24, 2022
View reviewed changes
Copy link
Member

@tcitworld tcitworld left a comment

Choose a reason for hiding this comment

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

Please revert unneeded autoloader changes.

lib/private/Mail/Message.php Outdated Show resolved Hide resolved
lib/private/Mail/Message.php Outdated Show resolved Hide resolved
@bennet0496
Revert "ran build/autoloaderchecker.sh after adding AutoSubmittedValu…
04c3c87 
…e class"

This reverts commit 77569ba.

Signed-off-by: Bennet Becker <dev@bennet.cc>
@bennet0496
Copy link
Contributor Author

which of the autoloader changes are ones that need to stay? If any, probably only lib/composer/composer/autoload_static.php and lib/composer/composer/autoload_classmap.php right? I would that completly revert commit 77569ba and just commit that changes to these two files again. However, all the other changes came from running build/autoloaderchecker.sh as well and I committed the result of that like documented. Sorry if I missunderstood that.

@bennet0496
use constant as default value in value getter
9d9a0b7 
Signed-off-by: Bennet Becker <dev@bennet.cc>
bennet0496 added a commit to bennet0496/nextcloud-activity that referenced this pull request Dec 26, 2022
@bennet0496
setting Auto-Submitted for notification mails
ee71841 
uses method from nextcloud/server#35876 if it exists, falling back
setting it via the swift message

Signed-off-by: Bennet Becker <dev@bennet.cc>
@bennet0496 bennet0496 mentioned this pull request Dec 26, 2022
Merged
@nickvergessen
Copy link
Member

which of the autoloader changes are ones that need to stay?

bash build/autoloaderchecker.sh
git add lib/composer/composer/autoload_*

and then commit it.

@bennet0496
refactor AutoSubmittedValue to Headers\AutoSubmitted and adding
4dc0957 
header name to constants

Signed-off-by: Bennet Becker <dev@bennet.cc>
@bennet0496
updating autoloader files
bd846df 
Signed-off-by: Bennet Becker <dev@bennet.cc>
@bennet0496
update usages of AutoSubmittedValue to AutoSubmitted::VALUE_
5a1cc8d 
Signed-off-by: Bennet Becker <dev@bennet.cc>
@nickvergessen nickvergessen merged commit 348454c into nextcloud:master Jan 4, 2023
@welcome
Copy link

welcome bot commented Jan 4, 2023

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@nickvergessen
Copy link
Member

Good stuff @bennet0496

@bennet0496
Copy link
Contributor Author

thank you @nickvergessen

it's always cool if I can help and contribute, even with the limited amount of coding I usually do 😄

@nickvergessen nickvergessen mentioned this pull request Jan 8, 2023
Closed
@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Jan 9, 2023
@nickvergessen nickvergessen mentioned this pull request Jan 9, 2023
Closed
@DaphneMuller
Copy link

hello @bennet0496
Thank you so much for your work on this pull request! This ticket seems to have the tag 'pending documentation'.
Is there any chance you could clarify what documentation is missing? Is this for admins or for app developers?

@nickvergessen
Copy link
Member

Short docs were added in #34692 (comment)

@nickvergessen
Copy link
Member

All our emailing has no docs.
Should be added some time to the developer docs in https://docs.nextcloud.com//server/stable/developer_manual/digging_deeper/index.html

@nickvergessen nickvergessen removed the pending documentation This pull request needs an associated documentation update label Feb 21, 2023
artonge pushed a commit to bennet0496/nextcloud-activity that referenced this pull request Oct 4, 2023
@bennet0496 @artonge
setting Auto-Submitted for notification mails
a6d76de 
uses method from nextcloud/server#35876 if it exists, falling back
setting it via the swift message

Signed-off-by: Bennet Becker <dev@bennet.cc>
susnux pushed a commit to bennet0496/nextcloud-activity that referenced this pull request Oct 18, 2023
@bennet0496 @susnux
setting Auto-Submitted for notification mails
6ba198b 
uses method from nextcloud/server#35876 if it exists, falling back
setting it via the swift message

Signed-off-by: Bennet Becker <dev@bennet.cc>
susnux pushed a commit to bennet0496/nextcloud-activity that referenced this pull request Oct 18, 2023
@bennet0496 @susnux
feat: setting Auto-Submitted for notification mails
b1ea0fb 
uses method from nextcloud/server#35876

Signed-off-by: Bennet Becker <dev@bennet.cc>
@kesselb kesselb mentioned this pull request Nov 7, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst left review comments

@nickvergessen nickvergessen nickvergessen approved these changes

@tcitworld tcitworld tcitworld approved these changes

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

@miaulalala miaulalala Awaiting requested review from miaulalala miaulalala is a code owner

@PVince81 PVince81 Awaiting requested review from PVince81

@icewind1991 icewind1991 Awaiting requested review from icewind1991

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.
6 participants
@bennet0496 @nickvergessen @DaphneMuller @ChristophWurst @tcitworld @szaimen