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

Make sender display name in mail notifications configurable #40671

Merged
merged 8 commits into from
Mar 7, 2023
Merged

Make sender display name in mail notifications configurable #40671

merged 8 commits into from
Mar 7, 2023

Conversation

pako81
Copy link

@pako81 pako81 commented Mar 6, 2023 •
edited
Loading

Description

Make sender display name in mail notifications configurable.

Motivation and Context

In some cases mail notifications related to sharing activities are blocked by mail filters as they are flagged as email impersonation. In such cases it may be desirable for an oC admin to have a config option for removing the sender display name.

How Has This Been Tested?

  • Set remove_sender_display_name => true in config.php. The From address in the mail notifications is displayed as follow:

image

  • Set remove_sender_display_name => false in config.php / do not set it at all. The From address in the mail notifications is displayed as per current behaviour:

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised
  • Changelog item

@pako81 pako81 added this to the development milestone Mar 6, 2023
@pako81 pako81 self-assigned this Mar 6, 2023
@pako81
Copy link
Author

pako81 commented Mar 6, 2023

It would eventually need documentation.

@pako81
Copy link
Author

pako81 commented Mar 6, 2023

@hodyroff @phil-davis @jnweiger @jvillafanez does it make sense?

@jvillafanez
Copy link
Member

Looks good to me

@phil-davis
Copy link
Contributor

phil-davis commented Mar 7, 2023 •
edited
Loading

Looks reasonable. I pushed a fix for the unit tests. That should help CI to pass.

We can add:

  • changelog
  • config.sample.php entry
  • new unit test case for when remove_sender_display_name is set to true

@pako81 pako81 mentioned this pull request Mar 7, 2023
Closed
12 tasks
@pako81
Copy link
Author

pako81 commented Mar 7, 2023

Doc issue created: owncloud/docs-server#887

@owncloud owncloud deleted a comment from update-docs bot Mar 7, 2023
@pako81
Copy link
Author

pako81 commented Mar 7, 2023

@phil-davis is it not sufficient to have an issue in docs-server repo for getting new config options in config.sample.php?

@phil-davis
Copy link
Contributor

@phil-davis is it not sufficient to have an issue in docs-server repo for getting new config options in config.sample.php?

comfig.sample.php original source code is in the core repo. @mmattel will then do a "config to docs" run to update it in the appropriate docs repo.

jnweiger
jnweiger approved these changes Mar 7, 2023
View reviewed changes
Copy link
Contributor

@jnweiger jnweiger left a comment

Choose a reason for hiding this comment

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

Nice!

@jnweiger jnweiger changed the base branch from master to release-10.12.0 March 7, 2023 12:59
@pako81
Copy link
Author

pako81 commented Mar 7, 2023

comfig.sample.php original source code is in the core repo. @mmattel will then do a "config to docs" run to update it in the appropriate docs repo.

I was under the impression it was the other way round. Thanks for clarifying it!

@pako81
Copy link
Author

pako81 commented Mar 7, 2023

Is this to go in 10.12.0?

Yes, 10.12.0. @jnweiger just changed the target branch.

@phil-davis
Copy link
Contributor

Note: there are now some Transifex commits also, but actually that is good - we want the latest translations to also end up in 10.12 anyway.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 7, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@pako81 pako81 merged commit 27f7f61 into release-10.12.0 Mar 7, 2023
@delete-merged-branch delete-merged-branch bot deleted the make_getFrom_configurable branch March 7, 2023 13:53
@mmattel mmattel mentioned this pull request Mar 7, 2023
Merged
@jnweiger jnweiger mentioned this pull request Mar 8, 2023
Merged
jnweiger added a commit that referenced this pull request Mar 13, 2023
@phil-davis @jnweiger
@jvillafanez @michielbdejong @mrow4a @ownclouders @EParzefall
Merge back 10.12.0 (#40682)
0d817c8 
* prepare 10.12

* update translations

* Allow stream wrappers not to provide the underlying stream (#40659)

This is mainly for streams that don't wrap another one, or use
a stream not provided by the native fopen function such as
smbclient_open

* Fix issue if no version has been created yet (#40661)

* fix link in changelog.

* Revert .htaccess change

* OCM via ScienceMesh

Signed-off-by: Michiel de Jong <michiel@unhosted.org>

* Get plugin from \OC::$server->query()

* Improve changelog entry

Signed-off-by: Michiel de Jong <michiel@unhosted.org>

* improve changelog entry

Signed-off-by: Michiel de Jong <michiel@unhosted.org>

* Whitespace change to re-trigger CI

Signed-off-by: Michiel de Jong <michiel@unhosted.org>

* Upgrading phpseclib/phpseclib (3.0.18 => 3.0.19)

* changelog for phpseclib 3.0.19

* Add test for the new feature

* revert logic to expose free_space, but keep availableStorage config

* Make sender display name in mail notifications configurable (#40671)

* [tx] updated from transifex

* [tx] updated from transifex

* make getFrom configurable

* fix style

* Adjust unit tests for MailNotificationsTest

* add changelog

* Document remove_sender_display_name system setting

* Add unit test cases for when remove_sender_display_name is set to true

---------

Co-authored-by: ownClouders <devops@owncloud.com>
Co-authored-by: Phil Davis <phil@jankaritech.com>

* removing double to

* prepare 10.12.0 RC3

* Bump final 10.12.0 version in version.php

---------

Signed-off-by: Michiel de Jong <michiel@unhosted.org>
Co-authored-by: Juergen Weigert <jnweiger@gmail.com>
Co-authored-by: Juan Pablo Villafañez <jvillafanez@solidgeargroup.com>
Co-authored-by: Michiel de Jong <michiel@unhosted.org>
Co-authored-by: Piotr Mrówczyński <piotr@owncloud.com>
Co-authored-by: Pasquale Tripodi <ptripodi@owncloud.com>
Co-authored-by: ownClouders <devops@owncloud.com>
Co-authored-by: EParzefall <edith_parzefall@gmx.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phil-davis phil-davis phil-davis approved these changes

@jvillafanez jvillafanez jvillafanez approved these changes

@jnweiger jnweiger jnweiger approved these changes

Assignees

@pako81 pako81

Labels
3 - To Review
Projects
None yet
Milestone
development
Development

Successfully merging this pull request may close these issues.
5 participants
@pako81 @jvillafanez @phil-davis @jnweiger @ownclouders