Prepend `X-Original-To` with destination data

[dnamsons]

Outlook seems to omit the BCC recipient's emails from the To: header, therefore it is not possible to easily get the SES email that the message was sent to.

The solution I found was to extract the recipient from the SNS payload - https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notification-contents.html#mail-object

This solution was inspired by a PR for the ActionMailbox Mailgun integration, which had a similar issue - https://github.com/rails/rails/pull/38738

[bobf]

Hi, @dnamsons , thanks a lot for the PR. The concept looks sound to me but I would like to a) make a couple of tiny tweaks (just minor refactoring, no change to functionality), b) leave this conversation open for a couple of days in case anybody else has some input here.

If it's okay with you, I'll make a follow-up commit to this PR with my suggested changes, and then if you could verify that everything still works correctly then we can merge. I would also like to have a regression test added for this specific case - if you are able to add that then that would be perfect, otherwise I can try to put something together (in which case an example JSON document would be helpful so I could use it as a fixture).

Either way, I see no problem getting this functionality added to the gem and I can do a release shortly after merging the final PR.

Thanks again for taking the time to look at this and for finding a solution.

[dnamsons]

Hey @bobf! Thank you for the response!

I will write a test for this feature and I am also open to improvements to the code.

[bobf]

Hi, @dnamsons , sorry it's taken me so long to get round to this, I've been unwell the last week but I am doing much better now so I will reserve some time this weekend to give you some feedback. The changes to the code I was looking at are really minor so I think the easiest way to do it is for me to add a commit to this PR with my proposed changes and you can let me know if you disagree with them. We'll get this PR merged soon for sure.

[bobf]

@dnamsons I've pushed a branch to the main repository which just does a quick refactor of the changes you made:

https://github.com/bobf/action_mailbox_amazon_ingress/commit/d66a0b2c5796f2d313a170752c8366d67bc065d4

I believe it to be functionally equivalent to your code and I did not change any of the logic you implemented, I just moved it into a few smaller methods.

Could you please add this code to your PR (feel free to suggest any improvements if you see any) and, if possible, write a test to cover the new functionality ?

If so, I am happy to merge your changes into master and push a new release of the gem. If the test is too awkward to write then I guess we can live without it but i would always prefer a test if it is possible to add one.

Thanks again for your contribution - we should be able to get this wrapped up pretty soon now. I'm happy to support you if you have any questions.

[bobf]

@dnamsons Is this one ready to go now ? Thanks for the recent updates to the PR.

[dnamsons]

@bobf Yes, this is done. I added a test case to ensure that the destination email will be extracted from the SNS payload.

I didn't add a new email fixture file because there is no real visible difference - Outlook sends the email directly to each BCC recipient, omitting the BCC data.

[bobf]

@dnamsons Thanks a lot for these changes. I've merged the PR and will do a release in the next few days if you don't make any more changes. :+1: