[rt-devel] support for dynamic envelope sender addresses

[Sam Hanes]

On 07/28/2016 01:03 PM, Shawn M Moore wrote:
>> On Jul 28, 2016, at 12:50, Sam Hanes wrote:
>> Is it likely that a patch to add a callback capable of setting the
>> envelope sender in `RT::Interface::Email::SendEmail()` would be accepted
>> upstream?
>
> Yeah, we'd happily take such a patch. As you discovered, callbacks
> are Mason-specific, so can't be used for this. The way we make
> library code customizable is by factoring out a small, well-defined
> method, which a local overlay can then redefine.
>
>> Should such a callback be passed a reference to the array of `sendmail`
>> arguments so it can modify them as desired or should it be expected to
>> return an envelope sender address?
>
> A method that returns the list of sendmail arguments might be the best fit.

I did eventually put together a patch to add that hook:
https://github.com/bestpractical/rt/compare/4.4-trunk...Elemecca:4.4/sendmail-args-hook

That was months ago (Jul 31), but I didn't submit it at the time because
I hadn't figured out how to write tests for it yet, and then I had a few
months where I was too busy to work on it. In the intervening time, this
pull request was submitted and merged to 4.2-trunk:
https://github.com/bestpractical/rt/pull/192/files

That adds something akin to the functionality I need, and it conflicts
with my patch. Unfortunately it doesn't quite work for me: I need the
TransactionObj in addition to the TicketObj to do the address mangling I
want, and it only provides the TicketObj. I therefore still need to
submit a change of some kind in order for my use-case to work. Also, I'm
on 4.4.1 and it hasn't been merged up to 4.4-trunk or master yet.

How should I deal with the conflict?

I see a few paths:

- Abandon altogether the idea of messing with sendmail arguments.
   Merge the _OutgoingMailFrom patch up to 4.4-trunk or master, then
   land a further patch to pass TransactionObj and the other
   SendEmail parameters to that method.

- Amend my SendmailArguments patch to also provide the _OutgoingMailFrom
   method, then land it on 4.4-trunk or master. There are two ways I
   could handle the _OutgoingMailFrom method:

   - have _OutgoingMailFrom return undef by default, then add it to the
     sender address fallback chain introduced by my patch at a priority
     lower than SendmailArguments but above OverrideOutgoingMailFrom

   - have _OutgoingMailFrom take over the OverrideOutgoingMailFrom
     section of the fallback chain as it does in the original patch

- Land my SendmailArguments patch as-is on 4.4-trunk or master and just
   drop the _OutgoingMailFrom method as SendmailArguments provides a
   strict superset of its functionality.

Please let me know how to proceed so I can update my patch as necessary,
add tests, and submit it properly.

Thanks,
Sam Hanes