beam-community / bamboo

Testable, composable, and adapter based Elixir email library for devs that love piping.
https://hex.pm/packages/bamboo
MIT License
1.91k stars 345 forks source link

feat!: Use interceptor before stripping the recipients with the formatter #682

Open tarzan opened 1 month ago

tarzan commented 1 month ago

I would like to use the interceptor to detect the preferences of the rich struct that is set as a receiver on the 'to:'

The (implemented) formatter strips all info from this struct and just set a {name, email} tuple instead which I cannot use at the interceptor level anymore.

I decided to swap them: interceptor first normalization and validation second - can't think of a reason this would result in unwanted behavior.

Could you take this under consideration?

tarzan commented 1 month ago

Thank you @tarzan! I looked this over and left a quick comment on the test changes.

While I can't immediately think of a negative impact here, I do suspect this could have unintended consequences to current users of Bamboo and would require a breaking changes. If so, that would warrant some more conversation 🤔

You are absolutely right, this is a breaking change. Any custom interceptors might expect a tuple [{name, email}] as the value of to: in the Bamboo.Email struct. With this change it will now contain the value that was actually set during the construction of the Bamboo.Email before formatting takes place.

In our case the value on to: is a %User{} struct that holds values of a user in our app, one of its fields is some preferences on whether the user would want to receive certain emails. Setting this fully populated User struct as the receiver of the email allows us to abstract checking of preferences to an Interceptor - which I think is conceptually the goal of an interceptor.

I can totally understand that current users that have spend time in constructing their own interceptors would not be to thrilled about this change. Would it help if I wrote some simple instructings on what to look for and how to apply these changes - ie something like a (mini) migration guide?

tarzan commented 1 month ago

hey @doomspork any updates on how you would propose we move forward with this? or would like to keep it on hold?