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

Storing State on Model - Enhancements? #380

Open
ckorhonen opened this issue Dec 7, 2019 · 1 comment
Open

Storing State on Model - Enhancements? #380

ckorhonen opened this issue Dec 7, 2019 · 1 comment

Comments

@ckorhonen
Copy link

I've recently implemented the approach recommended in the readme for storing the current state on my mode:

after_transition do |model, transition|
  model.state = transition.to_state
  model.save!
end

Very pleased with the results - based on benchmarks, I've seen a 40-50% improvement in my queries.

In doing so I had to change a lot of code which was using Model.in_state(:foo) to use the new column (Model.where(state: 'foo')) and ended up doing some meta programming to automatically create the scopes based on the state machine states.

I was wondering if you had ever considered incorporating something like this into the gem, and supporting this usage in a more robust way? Given the performance gains we have seen I feel like it could be of interest.

In terms of what I'm thinking would be ideal:

  1. In the State Machine class you would specify the column on the model which should be used to store the current state.
  2. If declared, the column value is automatically set after a transition.
  3. If declared, the in_state method would automatically use this column for querying.

Thoughts? If we feel this is useful, happy to work on a PR.

@ckorhonen ckorhonen changed the title in_state & storing states on model Storing State on Model - Enhancements? Dec 7, 2019
@lawrencejones
Copy link
Contributor

Hey @ckorhonen! Thanks for opening this issue.

At GoCardless, I'm not sure if we would have a desire to use this type of denormalising. We normally query against the transition tables themselves rather than the models, and rely on the (most_recent, to_state) indexes to optimise our queries, or so it was when I worked in the app teams. @danwakefield should be able to confirm if this might be able to help us?

That said, it may well be useful for people other than us, so if you wanted to submit a PR that adds this as an optional performance add-on then I don't see that as an issue.

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

No branches or pull requests

2 participants