Further refactor of token creation #1098
Conversation
- Rewrite terse code. - Remove superfluous implicit calls to self. - Move instance method out of `included do...` block.
- Optionally accept extra attributes hash when calling `#create_token`. - Utilize token_extras hash for `#create_token` to DRY up `#create_new_auth_token`. - Synchronize the time used to set `expiry` and `updated_at`.
|
Hey @Evan-M thanks for this! Your refactorings make sense and I get what you're doing here. If one other person approves this PR I'll merge it. |
| end | ||
| def email_required?; false; end | ||
| def email_changed?; false; end | ||
| def will_save_change_to_email?; false; end |
There was a problem hiding this comment.
These unexplained devise overrides are the bane of my existence and the cause of a lot of hair pulling for a lot of developers... But alas, you aren't changing their functionality here.
That said, I'd just leave these alone, so we can preserve our git blame history on these three methods to help the next poor soul who encounters this gotcha while wondering what's magically overriding their email callback on their user object.
There was a problem hiding this comment.
@KelseyDH is there a pr to fix the devise overrides situation? If not, feel free to open so we can debate!
|
Could there be any reason for wanting to preserve our calls to If not, then this looks like a great a refactor overall. 👍 |
* Cleanup in ahead of refactor. - Rewrite terse code. - Remove superfluous implicit calls to self. - Move instance method out of `included do...` block. * Refactor `#create_new_auth_token`. - Optionally accept extra attributes hash when calling `#create_token`. - Utilize token_extras hash for `#create_token` to DRY up `#create_new_auth_token`. - Synchronize the time used to set `expiry` and `updated_at`.
I commented on #1061 after it was merged, but since I'm not sure I was very clear, I went ahead and further refactored things to DRY up the code a bit more.
Essentially, the
#create_tokenmethod can accept arbitrary additional keyword attributes that will be merged into the resultingtokens[client_id]hash. This allows#create_new_auth_tokento reuse the token creation logic, instead of duplicating it. See:e601951for details.Note the first commit of this PR is misc cleanup, and perhaps doesn't belong.
I didn't add additional tests, since there is no additional functionality.