This will need a documentation PR to add those configuration variables and an explanation on when they are used

Really lovely yaml here.

• Did you contemplate using this “dynamic delivery” option vs the new bulk-focused class? https://guides.rubyonrails.org/action_mailer_basics.html#sending-emails-with-dynamic-delivery-options - presumably you’d have smaller diff that way, though I see the advantage of the isolated class as well.
• I recall on some previous issue that there's a crowdin problem with moving/renaming strings this way, though I don't recall the issue details. Am I making that up?

• Did you contemplate using this “dynamic delivery” option vs the new bulk-focused class? https://guides.rubyonrails.org/action_mailer_basics.html#sending-emails-with-dynamic-delivery-options - presumably you’d have smaller diff that way, though I see the advantage of the isolated class as well.

We did, having everything in a specific class seemed cleaner to us.

• I recall on some previous issue that there's a crowdin problem with moving/renaming strings this way, though I don't recall the issue details. Am I making that up?

Yes, existing translations are unfortunately invalidated. Crowdin should suggest them for the renamed strings, though.

* Did you contemplate using this “dynamic delivery” option vs the new bulk-focused class? https://guides.rubyonrails.org/action_mailer_basics.html#sending-emails-with-dynamic-delivery-options - presumably you’d have smaller diff that way, though I see the advantage of the isolated class as well.

Well, the new BulkMailer does use the dynamic delivery stuff, but we decided it would be tidier to have a separate class anyway. Especially since UserMailer is already quite large.

* I recall on some previous issue that there's a crowdin problem with moving/renaming strings this way, though I don't recall the issue details. Am I making that up?

Yes, there is a problem. Crowdin only picks up the english locale and will notice new keys that will all have to be translated again in all other locales. We do not know of a way to tell Crowdin that those keys only moved in an automated way. Seems like this will require some manual work in Crowdin. Happy to hear if you know of a better way.

Sounds good. I don't have a strong opinion on which way is better. Glad it was considered.

Using delivery_method_options as an additional arg to the two mailers within the existing user mailer, along with a method/module/something to negotiate the "use bulk if present and standard if not" logic would probably minimize the diff and not require translation changes ... but breaking out the new class may be more clean and more future proof re: adding more bulk mailers, if any (though I see there is added coverage here for the bulk/optional behavior, which makes that less important).

Separately - on top of the crowdin changes, are there any implications to running/queued mailers that are in place before the rename? I know in previous PRs we'd resisted changing the method signature/naming of some of them out of concern for breaking retries.

Especially since UserMailer is already quite large.

If we could solve the "renames and signature changes might break queues" issue, I think a good refactor at some point would be to separate all of the "inherited from devise" vs "our own creations" emails within this mailer into two mailers. Would reduce the LOC of the large user mailer, and I suspect would also be a nice code cleanup on the helpers/params/subjects/i18n front, because those tend to behave consistently between the devise and non-devise methods.

Separately - on top of the crowdin changes, are there any implications to running/queued mailers that are in place before the rename? I know in previous PRs we'd resisted changing the method signature/naming of some of them out of concern for breaking retries.

Excellent point. It's a rather minor concern because those are uncommonly-used features and not part of any release yet (although they are part of betas!), but this is all true.

I rewrote this without introducing a new mailer. This removes the need to move i18n keys and solves the problem of previously enqueued jobs.

I moved the logic into its own concern, so it could be re-used in other mailers (sadly, UserMailer does not inherit from ApplicationMailer).

As Matt pointed out, this makes the diff smaller, a lot smaller. And we could still tackle the bigger refactor later, e.g. when we have a third type of "bulk mail".