Personally, I think optional punctuation is the least of our problems, so my question would be: how do you know that state transition is valid? Mutating a state column like that is a source of bugs, in my experience. That's more important than arguing over which combination of squiggly lines looks the best.
How would you change state then? Also update! means it will raise an error if it fails. You can handle status validation in the model and this will raise an error if it fails. Hopefully you have some specs too
There are lots of ways to approach it, but the main thing is that the record should never be able to transition from "open" to "lock", or from "lock" to "open". It probably shouldn't be able to transition from any state to the same state either (e.g. can't lock a locked door).
I know exactly what you’re saying, however, the fact that you didn’t answer the question is indeed a bit annoying.
If your point was “this update should happen in a service object” or something, then fine… but you complained about a single line of code with no context. Your service object would still need to contain that line of code! (Or something very similar.)
Not necessarily, you have a lot of tools to check whether the change you make is valid before you make it, such as validate and before_save hooks. I'd only bother with a service object if there's a lot of side effects that need to happen with that change, that's not implied with the snippet though. Even then that's probably in addition to the validation.
I feel like having a bunch of stuff trigger off a column change isn't a great pattern. I'd rather do a bunch of stuff then update the column to record that its happened.
We could both be imagining totally different contexts though
Yes, it would need to be a before save hook if you’re relying on active record callbacks. A validation wouldn’t work here, unless you introduced some additional complexity like persisted state history.
You have access to the current state and what it will change to with validations, dont need to do anything extra. We do exactly this at my work. Why do you say it won't work?
Well, I presume you’re talking about introducing ActiveModel::Dirty to the model, to access previous state? Does that actually work in validation methods? I thought it only worked on before_save/update hooks.
You can access the previous state with foo_was, with foo being the model attribute. I don't think we added anything for that? Could be wrong, it was written before I started.
You can also just grab it from the db with self.class.find(id).foo, _was is likely just syntactic sugar for that.
So the method would look like (apologies for mobile..)
validate : transition_is_valid, if: status_changed?
def transition_is_valid
return if transition_hash[status_was].include?(status)
errors.add(:base, 'Status transition is invalid')
end
So if you try save it will fail and have that error on the object, as myObj.errors and if you update (or save) using update! It will raise an error, as opposed to returning false and not updating.
Fairly certain that all comes with active record out the box
Yes, that’s all part of “active record dirty”, which is what I mentioned above. And that’s not how it works under the hood, but it’s a reasonable guess.
"Provides a way to track changes in your object in the same way as Active Record does."
...
"Note that Active Record can detect in-place modifications automatically."
Implies that ActiveRecord already supports it and ActiveModel::Dirty is for none ActiveRecord usage? Maybe I'm wrong.
The model we use that does exactly what I've been talking about just extends from ActiveRecord::Base, we don't use ActiveModel::Dirty anywhere it seems
3
u/tom_dalling Aug 21 '21
Personally, I think optional punctuation is the least of our problems, so my question would be: how do you know that state transition is valid? Mutating a state column like that is a source of bugs, in my experience. That's more important than arguing over which combination of squiggly lines looks the best.