Project

General

Profile

Bug #12155

Setting email_reply_address is not used without restart of Foreman

Added by Bryan Kearney almost 4 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Category:
E-Mail
Target version:
Difficulty:
Triaged:
Bugzilla link:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

Cloned from https://bugzilla.redhat.com/show_bug.cgi?id=1271129
Description of problem:
Changing the setting email_reply_address the new value is not used until Foreman is restarted

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:
1. Change the email_reply_address setting to foobar
2. Trigger a mail sending
3.

Actual results:
The from address is still using the original from address and not the updated foobar

Expected results:
Mail from address is changed to foobar

Additional info:

Associated revisions

Revision 4fdf2f97 (diff)
Added by Stephen Benjamin almost 4 years ago

Fixes #12155 - register changes in reply address without restart

Problem -
ApplicationMailer uses the Setting :email_reply_address. If an user
changes this setting, Foreman still use the original one, because it's
evaluated only once, at initialization.

Solution -
Use a lambda to evaluate on every send what's the email_reply_address
setting, instead of just on initialization

Revision 791e1829 (diff)
Added by Daniel Lobato Garcia almost 4 years ago

Refs #12155 - Use Proc instead of lambda on mailer for Rails 4

Problem:
On Rails 3, using a lambda with no arguments works fine in an
ActionMailer default. However on Rails 4, the lambda will whine because
there is no handling of arguments and ActionMailer is trying to pass a
HostMailer.

Solution:
Use Procs instead of lambda to allow it to work with and without arguments.

How to reproduce the error:
Checkout https://github.com/eLobato/foreman/tree/rails4_from_scratch,
change the -> by ->(args) and put a debug breakpoint inside. Verify
ActionMailer is passed. On Rails 3 this is not the case.

Revision 11613e53 (diff)
Added by Stephen Benjamin over 3 years ago

Fixes #12155 - register changes in reply address without restart

Problem -
ApplicationMailer uses the Setting :email_reply_address. If an user
changes this setting, Foreman still use the original one, because it's
evaluated only once, at initialization.

Solution -
Use a lambda to evaluate on every send what's the email_reply_address
setting, instead of just on initialization

(cherry picked from commit 4fdf2f97ad63898dee899f8fe77dd9e47221ffca)

Revision 0f94d81f (diff)
Added by Daniel Lobato Garcia over 3 years ago

Refs #12155 - Use Proc instead of lambda on mailer for Rails 4

Problem:
On Rails 3, using a lambda with no arguments works fine in an
ActionMailer default. However on Rails 4, the lambda will whine because
there is no handling of arguments and ActionMailer is trying to pass a
HostMailer.

Solution:
Use Procs instead of lambda to allow it to work with and without arguments.

How to reproduce the error:
Checkout https://github.com/eLobato/foreman/tree/rails4_from_scratch,
change the -> by ->(args) and put a debug breakpoint inside. Verify
ActionMailer is passed. On Rails 3 this is not the case.

(cherry picked from commit 791e18293f73150d6a5fdd99154a8b149ae3319f)

History

#1 Updated by The Foreman Bot almost 4 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/2819 added
  • Pull request deleted ()

#2 Updated by Anonymous almost 4 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

#3 Updated by Dominic Cleal almost 4 years ago

  • Assignee set to Stephen Benjamin
  • Legacy Backlogs Release (now unused) set to 104

#4 Updated by Stephen Benjamin almost 4 years ago

@Dominic - Be aware this commit removes a deprecation that was targetted for removal in 1.11 not 1.10

#5 Updated by Dominic Cleal almost 4 years ago

Stephen Benjamin wrote:

@Dominic - Be aware this commit removes a deprecation that was targetted for removal in 1.11 not 1.10

Thanks for the heads up, but it doesn't look like it's in the final develop commit.

It looks like what happened is that you removed it in your PR, as it was based on an older parent commit (baaf6b4), but since we had already removed it in the newer c1755e0, the actual commit that Daniel merged to develop didn't contain the deprecation removal proposed in the PR. The git 3-way merge process must have skipped over it as it had already been done.

#6 Updated by Stephen Benjamin almost 4 years ago

Oh weird, thanks, I wish GitHub was more explicit in the PR that it would need a 3-way merge to merge it.

Also available in: Atom PDF