-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support for recognizing mailer many jobs: #2125
Conversation
got lucky with rubocop new release, which break CI.. opened #2126 to fix CI.. will rebased this branch after CI is fixed.. |
Can we close is then? |
16a6605
to
6d895ff
Compare
@@ -63,20 +68,18 @@ def expected_count_message | |||
"#{message_expectation_modifier} #{@expected_number} #{@expected_number == 1 ? 'time' : 'times'}" | |||
end | |||
|
|||
def mailer_args | |||
def job_match?(job) | |||
MAILER_JOBS.any? { |mailer_job| send(mailer_job, job) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd rather we just called legacy_mail?(mailer_job, job) || parameterized_mail?(mailer_job, job) || unified_mail?(mailer_job, job)
rather than constantising it and iterating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, updated.. I used constant because I thought it would be easier later IF we somehow need to add check for other job..
- parameterized mailer when RAILS_VERSION >= 5.1 - unified mailer job when RAILS_VERSION >= 6.0
@samphippen are you ok to merge this? 😊 |
per #2117 (comment)
this PR supersede #2121 with added support for recognizing
ActionMailer::MailDeliveryJob
freshly added into rails 6.0