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

Support before_reset callback in CurrentAttributes #35063

Merged
merged 1 commit into from Feb 4, 2019

Conversation

rosa
Copy link
Member

@rosa rosa commented Jan 27, 2019

Summary

This PR adds a before_reset callback in CurrentAttributes. The existing resets callback runs after reset has been called on the instance, which means we can't use it to do work that depends on the instance values, only to reset global independent collaborators like Time.zone.


I think this might need something added to the CHANGELOG, I'll add it if you think this before_reset is useful and can be added to Rails.

@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Jan 28, 2019

I think this is good. Can you add the CHANGELOG entry?

@kaspth
Copy link
Contributor

@kaspth kaspth commented Jan 28, 2019

On the other hand, would it be better to have resets imply before_reset?

If not, then perhaps we should also add a after_reset alias to resets (or the other way around) for readability.

@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Jan 28, 2019

I thought about changing resets but some times you will need both so add a after_reset alias too would be nice.

@rosa
Copy link
Member Author

@rosa rosa commented Jan 29, 2019

Thank you @rafaelfranca and @kaspth! Adding an after_reset alias is a good idea! I'll add that and the CHANGELOG entry.

@rosa rosa force-pushed the current-before-reset-callback branch from ac79e97 to 75b987c Compare Jan 29, 2019
This is useful when we need to do some work associated to `Current.reset`
but that work depends on the values of the current attributes themselves.
This cannot be done in the supported `resets` callback because when the
block is executed, CurrentAttributes's instance has already been reset.

For symmetry, `after_reset` is defined as alias of `resets`.
@rosa rosa force-pushed the current-before-reset-callback branch from 75b987c to 2ce8455 Compare Jan 30, 2019
@rafaelfranca rafaelfranca merged commit 2ce8455 into rails:master Feb 4, 2019
2 checks passed
rafaelfranca added a commit that referenced this issue Feb 4, 2019
Support before_reset callback in CurrentAttributes
suketa added a commit to suketa/rails_sandbox that referenced this issue Jul 27, 2019
Support before_reset callback in CurrentAttributes
rails/rails#35063
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