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

Allow noreply email address/from to be set in admin panel #14975

Merged
merged 7 commits into from Oct 10, 2019
Merged

Conversation

katebutler
Copy link

Fixes #12426

@katebutler katebutler added the Needs Review PRs that need a code review label Oct 8, 2019
@katebutler katebutler added this to the 3.12.0 milestone Oct 8, 2019
@DuplicatePR-bot
Copy link

Hi there! This pull request looks like it might be a duplicate of #14137, since it has the same issue number , similar commits, and similar changed files.

To improve our bot, you can help us out by clicking one of the options below:
- This pull request is a duplicate, so this comment was useful. check
- This pull request is not a duplicate, but this comment was useful nevertheless. check
- This pull request is not a duplicate, so this comment was not useful. check
- I do not need this service, so this comment was not useful. check

This bot is currently in its alpha stage, and we are only sending at most one comment per repository. If you are interested in using our bot in the future, please subscribe. If you would like to learn more, see our web page.

@tsteur
Copy link
Member

tsteur commented Oct 10, 2019

@katebutler it's not working for me. I think the problem is that you're trying to set a config value directly which doesn't work

Config::getInstance()->General['noreply_email_name'] = Common::unsanitizeInputValue(Common::getRequestVar('mailFromName', ''));

instead something like this:

$general = Config::getInstance()->General
$general['noreply_email_name'] = Common::unsanitizeInputValue(Common::getRequestVar('mailFromName', ''));
...
Config::getInstance()->General = $general;

@tsteur
Copy link
Member

tsteur commented Oct 10, 2019

fyi @katebutler merging now... works now... made a minor tweak that when no address is configured it will use the default value as otherwise sending mails likely fails and making sure to validate the configured address that it looks like an email etc.

@tsteur tsteur merged commit da92367 into 3.x-dev Oct 10, 2019
@tsteur tsteur deleted the 12426 branch October 10, 2019 22:20
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mail from must equal authorized user
4 participants