Fix actionmailer lambda default #30391
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbourassa
force-pushed
the
fix-actionmailer-lambda-default
branch
2 times, most recently
from
03c847d to
c2c96ab
Compare
|
Thanks! Don't think we need to deprecate it; it's just a bug fix. That way we can also backport it to 5-1-stable 😄 Remember to squash your commits down to 1. |
I was suggesting a deprecation because I think it's pointless to keep both forever, it just makes things unnecessarily complicated and provides no value. The deprecation allows us to remove this complexity in the near future. I'll update the PR but still keep the deprecated version stashed on a branch somewhere, just in case! |
jbourassa
force-pushed
the
fix-actionmailer-lambda-default
branch
from
c2c96ab to
04714e3
Compare
PR rails#29270 changed the number of arguments that gets passed to Procs defined in ActionMail::Base.default. With this changeset, Procs can now have 1 or 0 arguments Also adds test coverage for AM::Base.default Proc arity.
jbourassa
force-pushed
the
fix-actionmailer-lambda-default
branch
from
04714e3 to
fbb2fc8
Compare
kamipo
added a commit
that referenced
this pull request
Jan 23, 2018
Fix actionmailer lambda default
kamipo
added a commit
that referenced
this pull request
Jan 25, 2018
Fix actionmailer lambda default
bogdanvlviv
added a commit
to bogdanvlviv/rails
that referenced
this pull request
Mar 18, 2018
- Remove
```
* Namespace error pages' CSS selectors to stop the styles from bleeding
into other pages when using Turbolinks.
([Pull Request](rails#28814))
```
since it was backported to
`5-1-stable` by 50d5baf
and
`5-0-stable` by d1c4a39.
- Remove
```
* Allow irb options to be passed from `rails console` command.
([Pull Request](rails#29010))
```
since it was backported to
`5-1-stable` by e91b483.
- Remove
```
* Load environment file in `dbconsole` command.
([Pull Request](rails#29725))
```
since it was backported to
`5-1-stable` by 7f93428.
- Remove
```
* Gemfile for new apps: upgrade redis-rb from ~> 3.0 to 4.0.
([Pull Request](rails#30748))
```
since it was backported to
`5-1-stable` by 3789531.
- Remove
```
* Fix minitest rails plugin.
The custom reporters are added only if needed.
This will fix conflicts with others plugins.
([Commit](rails@ac99916))
```
since it was backported to
`5-1-stable` by caa7695.
- Remove
```
* Add support for compatibility with redis-rb gem for 4.0 version.
([Pull Request](rails#30748))
```
since it was backported to
`5-1-stable` by 3789531.
- Remove
```
* Add `action_controller_api` and `action_controller_base` load hooks to be
called in `ActiveSupport.on_load`.
([Pull Request](rails#28402))
```
since it was backported to
`5-1-stable` by b9a5fd7.
- Remove
```
* `driven_by` now registers poltergeist and capybara-webkit.
([Pull Request](rails#29315))
```
since it was backported to
`5-1-stable` by c5dd451.
- Remove
```
* Fallback `ActionController::Parameters#to_s` to `Hash#to_s`.
([Pull Request](rails#29630))
```
since it was backported to
`5-1-stable` by c1014e4
and
`5-0-stable` by 0e71fc3.
- Remove
```
* Make `take_failed_screenshot` work within engine.
([Pull Request](rails#30421))
```
since it was backported to
`5-1-stable` by 595a231.
- Remove
```
* Fix optimized url helpers when using relative url root.
([Pull Request](rails#31261))
```
since it was backported to
`5-1-stable` by e9b7795.
- Remove
```
* Update `distance_of_time_in_words` helper to display better error messages
for bad input.
([Pull Request](rails#20701))
```
since it was backported to
`5-1-stable` by 2c97fbf.
- Remove
```
* Generate field ids in `collection_check_boxes` and
`collection_radio_buttons`.
([Pull Request](rails#29412))
```
since it was backported to
`5-1-stable` by 2d8c10a.
- Remove
```
* Fix issues with scopes and engine on `current_page?` method.
([Pull Request](rails#29503))
```
since it was backported to
`5-1-stable` by 2135daf.
- Remove
```
* Bring back proc with arity of 1 in `ActionMailer::Base.default` proc
since it was supported in Rails 5.0 but not deprecated.
([Pull Request](rails#30391))
```
since it was backported to
`5-1-stable` by b2bedb1.
- Remove
```
* Add type caster to `RuntimeReflection#alias_name`.
([Pull Request](rails#28961))
```
since it was backported to
`5-1-stable` by f644e7a.
- Remove
```
* Loading model schema from database is now thread-safe.
([Pull Request](rails#29216))
```
since it was backported to
`5-1-stable` by 02926cf.
and
`5-0-stable` by 84bcfe5
- Remove
```
* Fix destroying existing object does not work well when optimistic locking
enabled and `locking_column` is null in the database.
([Pull Request](rails#28926))
```
since it was backported to
`5-1-stable` by e498052.
- Remove
```
* `ActiveRecord::Persistence#touch` does not work well
when optimistic locking enabled and `locking_column`,
without default value, is null in the database.
([Pull Request](rails#28914))
```
since it was backported to
`5-1-stable` by 1e2f63d.
- Remove
```
* Previously, when building records using a `has_many :through` association,
if the child records were deleted before the parent was saved,
they would still be persisted. Now, if child records are deleted
before the parent is saved on a `has_many :through` association,
the child records will not be persisted.
([Pull Request](rails#29593))
```
since it was backported to
`5-1-stable` by a22c39e.
- Remove
```
* Query cache was unavailable when entering the `ActiveRecord::Base.cache`
block without being connected.
([Pull Request](rails#29609))
```
since it was backported to
`5-1-stable` by fd6c8cd
and
`5-0-stable` by 9f2532b.
- Remove
```
* `Relation#joins` is no longer affected by the target model's
`current_scope`, with the exception of `unscoped`.
([Commit](rails@5c71000))
```
since it was backported to
`5-1-stable` by 3630d63.
- Remove
```
* Fix `unscoped(where: [columns])` removing the wrong bind values.
([Pull Request](rails#29780))
```
since it was backported to
`5-1-stable` by d378fcb.
- Remove
```
* When a `has_one` association is destroyed by `dependent: destroy`,
`destroyed_by_association` will now be set to the reflection, matching the
behaviour of `has_many` associations.
([Pull Request](rails#29855))
```
since it was backported to
`5-1-stable` by 8254a8b.
- Remove
```
* Fix `COUNT(DISTINCT ...)` with `ORDER BY` and `LIMIT`
to keep the existing select list.
([Pull Request](rails#29848))
```
since it was backported to
`5-1-stable` by 0e8d4ed.
- Remove
```
* Ensure `sum` honors `distinct` on `has_many :through` associations.
([Commit](rails@566f1fd))
```
since it was backported to
`5-1-stable` by c0a1dc2.
- Remove
```
* Fix `COUNT(DISTINCT ...)` for `GROUP BY` with `ORDER BY` and `LIMIT`.
([Commit](rails@5668dc6))
```
since it was backported to
`5-1-stable` by 87ca68e.
- Remove
```
* MySQL: Don't lose `auto_increment: true` in the `db/schema.rb`.
([Commit](rails@9493d45))
```
since it was backported to
`5-1-stable` by 8b6e694.
- Remove
```
* Fix longer sequence name detection for serial columns.
([Pull Request](rails#28339))
```
since it was backported to
`5-1-stable` by af9c170
and
`5-0-stable` by 7025b1d.
- Remove
```
* Fix `bin/rails db:setup` and `bin/rails db:test:prepare` create wrong
ar_internal_metadata's data for a test database.
([Pull Request](rails#30579))
```
since it was backported to
`5-1-stable` by bb67b5f
and
`5-0-stable` by 60437e6.
- Remove
```
* Fix conflicts `counter_cache` with `touch: true` by optimistic locking.
([Pull Request](rails#31405))
```
since it was backported to
`5-1-stable` by 5236dda.
- Remove
```
* Fix `count(:all)` to correctly work `distinct` with custom SELECT list.
([Commit](rails@c6cd9a5))
```
since it was backported to
`5-1-stable` by 6beb4de.
- Remove
```
* Fix to invoke callbacks when using `update_attribute`.
([Commit](rails@732aa34))
```
since it was backported to
`5-1-stable` by 6346683.
- Remove
```
* Use `count(:all)` in `HasManyAssociation#count_records` to prevent invalid
SQL queries for association counting.
([Pull Request](rails#27561))
```
since it was backported to
`5-1-stable` by eef3c89.
- Remove
```
* Fix `count(:all)` with eager loading and having an order other than
the driving table.
([Commit](rails@ebc09ed))
```
since it was backported to
`5-1-stable` by 6df9b69.
- Remove
```
* PostgreSQL: Allow pg-1.0 gem to be used with Active Record.
([Pull Request](rails#31671))
```
since it was backported to
`5-1-stable` by a9c06f6.
- Remove
```
* Fix that after commit callbacks on update does not triggered
when optimistic locking is enabled.
([Commit](rails@7f9bd03))
```
since it was backported to
`5-1-stable` by aaee10e.
- Remove
```
* Fix regression in numericality validator when comparing Decimal and Float
input values with more scale than the schema.
([Pull Request](rails#28584))
```
since it was backported to
`5-1-stable` by 5b1c3e5.
Note that there was incorrect link to PR,
original PR is rails#29249.
- Remove
```
* Fix to working before/after validation callbacks on multiple contexts.
([Pull Request](rails#31483))
```
since it was backported to
`5-1-stable` by 0f7046a.
- Remove
```
* Fix implicit coercion calculations with scalars and durations.
([Pull Request](rails#29163),
[Pull Request](rails#29971))
```
since it was backported to
`5-1-stable` by 51ea27c,
4d82e2a.
- Remove
```
* Fix modulo operations involving durations.
([Commit](rails@a54e13b))
```
since it was backported to
`5-1-stable` by 233fa7e.
- Remove
```
* Return all mappings for a timezone identifier in `country_zones`.
([Commit](rails@cdce6a7))
```
since it was backported to
`5-1-stable` by 0222ebb.
- Remove
```
* Add support for compatibility with redis-rb gem for 4.0 version.
([Pull Request](rails#30748))
```
since it was backported to
`5-1-stable` by 3789531.
Related to rails#32252.
Related to rails#32222, rails#32222 (comment).
Follow up a489cc8.
This was referenced Mar 20, 2018
louim
added a commit
to louim/devise
that referenced
this pull request
May 5, 2023
Rails allow procs and lambda with either zero or more argument. Devise however always tried to call instance_eval on those values, which does always pass one argument: self. There was a PR to fix this specific problem in Devise heartcombo#4627, before the arity check was fixed in rails itself: rails/rails#30391. But even if the problem was fixed in Rails, Devise was still calling the proc/lambas with instance_eval. That meant the fix added to Rails did not apply to Devise. The fix is to let Rails handle the :from and :reply_to defaults. We do that by unsetting the headers instead of trying to replicate Rails handling in Devise. This lets Rails handle it when setting up the mailer.
rafaelfranca
pushed a commit
to louim/devise
that referenced
this pull request
Jun 9, 2023
Rails allow procs and lambda with either zero or more argument. Devise however always tried to call instance_eval on those values, which does always pass one argument: self. There was a PR to fix this specific problem in Devise heartcombo#4627, before the arity check was fixed in rails itself: rails/rails#30391. But even if the problem was fixed in Rails, Devise was still calling the proc/lambas with instance_eval. That meant the fix added to Rails did not apply to Devise. The fix is to let Rails handle the :from and :reply_to defaults. We do that by unsetting the headers instead of trying to replicate Rails handling in Devise. This lets Rails handle it when setting up the mailer.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR #29270 changed the number of arguments that gets passed to Procs defined in
ActionMail::Base.default, thus introducing a breaking change. This change impacted Devise (heartcombo/devise#4604).With this PR, Procs can have an arity of 0 or 1. I added a deprecation for Procs with arity of 1 because it doesn't really make sense to keep this around – I brought it back for the sake of deprecating it before it is removed.
This is where the breaking change was introduced:
instance_evalpassesselfto the block, butinstance_execdoes not. In Rails < 5.1, this was the "correct" way, but it now crashes in 5.1. This breaking change was not caught by the test suite because the Proc were not defined with thelambdasyntax. I added a test case for that as well.Thoughts?