Do not overwrite secret token value when already present.

[morgoth]

user = User.create(token: "custom-secure-token")
user.token # => "custom-secure-token"

I'm aware that has_secure_token is a simple method that shouldn't fit in all possible use cases, but this change is very small and adds some flexibility.
Previously it was little unexpected to get User.create(token: "custom").token #=> "some-random-string"

[kaspth]

The callback used to generate a token is triggered before creating a record. Your test uses an existing record and the callback won't be triggered.

[morgoth]

nope - @user is a new instance of model User as you can see in setup: https://github.com/morgoth/rails/blob/do-not-overwrite-value-of-secret-token-when-present/activerecord/test/cases/secure_token_test.rb#L6

[kaspth]

Sorry, the diff hid that :)

[senny]

@morgoth this looks reasonable. Thank you for your contribution 💛

[morgoth]

@senny I just realized that has_secure_token was added in Rails 5.0 so maybe there is no need for my changelog as this feature is not available yet in stable release

[senny]

@morgoth you're right. I removed the entry 57a1d2b

[dhh]

Can you explain the specific use case you have for this?

[morgoth]

@dhh In example: discount coupons table with token as part of url. Admin can set fixed token for some specific users.
But, also previously it was little surprising writing: User.create(token: "custom").token #=> "some-random-string"

[dhh]

I see. I think that design muddies the waters of what "has_secure_token" means. These manually set values do not fulfill that description. It won't be what it says on the tin.

I would recommend that you have a separate method, like maybe slug, that will look for manual_slug || generated_slug where the latter is the secure token. Then it's clear what's happening.

Happy to consider other examples until we find one that can't be restated better without changing has_secure_token.

On Aug 22, 2015, at 01:39, Wojciech Wnętrzak notifications@github.com wrote:

@dhh In example: discount coupons table with token as part of url. Admin can set fixed token for some specific users.
But, also previously it was little surprising writing: User.create(token: "custom").token #=> "some-random-string"

—
Reply to this email directly or view it on GitHub.

[senny]

I don't have a specific use-case either that would require this behavior. One thing to think about is what happens when assigning the attributes of one-record to a new one. For example in the case of serialization or cloning.

existing_coupon = Coupon.last
Coupon.new(existing_coupon.attributes)

With this PR the token is preserved. If we revert, a new token is generated. Thinking about it, generating a new token might even be what you would expect.

Just some thoughts. I'm fine with a revert here.