Skip to content
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

Specify when to generate has_secure_token #47420

Merged
merged 1 commit into from Jul 18, 2023

Conversation

seanpdoyle
Copy link
Contributor

Motivation / Background

When a model defines has_secure_token, that value isn't available at initialization-time.

Detail

Extend has_secure_token to accept an on: option to specify the callback. Continues to default to before_create.

The callback when the value is generated. When called with on: :initialize, the value is generated in an after_initialize callback, otherwise the value will be used in a before_ callback. It will default to :create.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@p8 p8 changed the title Sepcify when to generate has_secure_token Specify when to generate has_secure_token Feb 17, 2023
activerecord/CHANGELOG.md Outdated Show resolved Hide resolved
@seanpdoyle seanpdoyle force-pushed the has-secure-token-callback branch 2 times, most recently from 0d21192 to 3a02994 Compare February 21, 2023 17:47
The callback when the value is generated. When called with `on:
:initialize`, the value is generated in an `after_initialize` callback,
otherwise the value will be used in a `before_` callback. It will
default to `:create`.
@seanpdoyle seanpdoyle requested a review from zzak July 18, 2023 21:16
@guilleiguaran guilleiguaran merged commit 2884d00 into rails:main Jul 18, 2023
9 checks passed
@seanpdoyle seanpdoyle deleted the has-secure-token-callback branch July 18, 2023 21:40
@seanpdoyle
Copy link
Contributor Author

@guilleiguaran thank you for reviewing and merging this work! As a follow-up, I've opened #48912 to propose on: :initialize as the new default in Rails 7.1, with the ability to configure the value to on: :create to preserve backwards compatibility.

rafaelfranca pushed a commit to seanpdoyle/rails that referenced this pull request Sep 1, 2023
Follow-up to [rails#47420][]

With the changes made in [rails#47420][], `has_secure_token` declarations can
be configured to execute in an `after_initialize` callback. This commit
proposed a new Rails 7.1 default: generate all `has_secure_token` values
when their corresponding models are initialized.

To preserve pre-7.1 behavior, applications can set
`config.active_record.generate_secure_token_on = :create`.

By default, generate the value when the model is initialized:

```ruby
class User < ApplicationRecord
  has_secure_token
end

record = User.new
record.token # => "fwZcXX6SkJBJRogzMdciS7wf"
```

With `config.active_record.generate_secure_token_on = :create`, generate
the value when the model is created:

```ruby
 # config/application.rb
config.active_record.generate_secure_token_on = :create

 # app/models/user.rb
class User < ApplicationRecord
  has_secure_token on: :create
end

record = User.new
record.token # => nil
record.save!
record.token # => "fwZcXX6SkJBJRogzMdciS7wf"
```

[rails#47420]: rails#47420

Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>
kamipo added a commit that referenced this pull request Sep 5, 2023
…n: :create`

Follow-up to #47420.

Whereas the original behavior (`on: :create`) is invoked only once
before a record is persisted, the new behavior (`on: :initialize`) is
invoked not only new record but also persisted records.

It should be invoked only once for new record consistently.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants