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

Disallow calling `#deliver_later` after local message modifications. #24457

Merged
merged 1 commit into from Apr 9, 2016

Conversation

Projects
None yet
3 participants
@jeremy
Member

jeremy commented Apr 7, 2016

Prevents a common, hard-to-find bug where local message changes aren't enqueued with the delivery job:

message = Notifier.welcome(user, foo)
message.message_id = my_generated_message_id
message.deliver_later

The message_id is silently lost here! Only the mailer arguments are passed to the delivery job.

This raises an exception now.

Make modifications to the message within the mailer method or use a custom Active Job to manage delivery instead of using #deliver_later.

@rafaelfranca

This comment has been minimized.

Member

rafaelfranca commented Apr 7, 2016

:shipit:

Disallow calling `#deliver_later` after local message modifications.
They would be lost when the delivery job is enqueued, otherwise.
Prevents a common, hard-to-find bug like:

```ruby
message = Notifier.welcome(user, foo)
message.message_id = my_generated_message_id
message.deliver_later
```

The message_id is silently lost here! *Only the mailer arguments are
passed to the delivery job.*

This raises an exception now.

Make modifications to the message within the mailer method or use a
custom Active Job to manage delivery instead of using #deliver_later.

@jeremy jeremy force-pushed the jeremy:mailer/dont-deliver-later-after-message-is-loaded branch to 95e06e6 Apr 7, 2016

@jeremy jeremy merged commit 2080ff2 into rails:master Apr 9, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@jeremy jeremy deleted the jeremy:mailer/dont-deliver-later-after-message-is-loaded branch Apr 9, 2016

weiqingtoh added a commit to coursemology-collab/coursemology2 that referenced this pull request Sep 26, 2017

jeremyyap added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 9, 2017

jeremyyap added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 10, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 12, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 12, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 13, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 13, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 14, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 14, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 14, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 14, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 16, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 17, 2017

allenwq added a commit to coursemology-collab/coursemology2 that referenced this pull request Oct 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment