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

[MERGED] ActiveModel.reload has to clear the internal cache [MERGED] #507

Closed
wants to merge 4 commits into from

Conversation

zerc
Copy link
Contributor

@zerc zerc commented Apr 3, 2023

In #417 the state.reset method was added and suggested to be used together with active_record.reload.

In this PR, I make sure that we apply this logic automatically if Statesman::Adapters::ActiveRecordQueries included to the model definition so the users don't have to do this themself.

@zerc zerc self-assigned this Apr 3, 2023
@zerc zerc marked this pull request as ready for review April 3, 2023 14:17
CHANGELOG.md Outdated Show resolved Hide resolved
@zerc zerc force-pushed the reload-should-clear-the-cache branch 3 times, most recently from c6c2b17 to 375bb11 Compare April 21, 2023 12:56
Otherwise, one will get stale cache on the freshly reloaded model e.g.
`current_state` would return outdated result.
@zerc zerc force-pushed the reload-should-clear-the-cache branch from 375bb11 to 843cee3 Compare April 21, 2023 13:18
Unrelated to the changes made.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR it's just rubocop started to complain about this for some reason and I had to make the change.

Comment on lines +52 to +59

define_method(:reload) do |*a|
instance = super(*a)
if instance.respond_to?(:state_machine, true)
instance.state_machine.reset
end
instance
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: this will only work if

include Statesman::Adapters::ActiveRecordQueries

is applied to the target model.

@zerc zerc changed the title reload should clear the cache ActiveModel.reload has to clear the internal cache Apr 21, 2023
@zerc zerc requested a review from stephenbinns April 21, 2023 14:19
Copy link
Contributor

@stephenbinns stephenbinns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@zerc
Copy link
Contributor Author

zerc commented Apr 21, 2023

PR is merged but for some reason it's not marked as such on Github 😭

Screenshot 2023-04-21 at 15 46 23

@zerc zerc closed this Apr 21, 2023
@zerc zerc deleted the reload-should-clear-the-cache branch April 21, 2023 15:43
@zerc zerc changed the title ActiveModel.reload has to clear the internal cache [MERGED] ActiveModel.reload has to clear the internal cache [MERGED] Apr 21, 2023
davegudge pushed a commit to skillstream/statesman that referenced this pull request May 26, 2023
…-the-cache

ActiveModel.reload has to clear the internal cache
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants