Fix save with state feat - #11
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix
save_with_stateand improve error handlingCorrections after rollback github.com//pull/7
Why
save_with_statecurrently returnstruewhen a transition is invalid, silently swallowing the failure. In Sidecare, if an operator tries to transition a payment procedure fromcanceledtoto_process, the form returns success but no@registered_callbacksever fires, so no transition happens — misleading the operator and producing stale data.This PR fixes the silent-failure bug, adds a proper error on the model, and tightens a few related rough edges in
has_one_state_machine.What changed
1.
save_with_statenow returnsfalseand adds a model error on invalid transitionsIf
#{virtual_attribute_name}differs from the current state butcan_transition_to?isfalse, the method now::invalid_transitionerror on thefield_nameattribute, andfalse.This makes form failures visible via
record.errors.full_messagesas users would expect.2. State-change detection no longer relies on
_changed?Replaced
if #{virtual_attribute_name}_changed?withif #{virtual_attribute_name}.to_s != #{field_name}_current_state.to_s.The previous check always returned
truebecause Rails sees the virtual attribute as a change fromnil → 'canceled', even when the effective value is unchanged. As a result, forms that re-submit the current state value (common in Sidecare) always failed to update. The string comparison reliably detects a real state change.3.
#{field_name}is now a public reader (with a guard)Previously
private, which broke form helpers likeform.text_field :user_statuswithNoMethodError: private method called. The reader is now public so form helpers, serializers, and otherpublic_send-style callers work.To avoid silently clobbering an existing column or user-defined method with the same name as
field_name, definition is now guarded:4. Error message is now i18n-friendly
The previous implementation passed a hardcoded English string via
message:, which:canceled,to_process) instead of their humanized translations.Now the macro uses Rails' standard i18n lookup with interpolation kwargs:
A default English translation ships with the gem in
lib/statesman/multi_state/locales/en.yml, and theRailtieadds it to Rails' i18n load path automatically. Host apps can override per-locale, per-model, or per-attribute through their own locale files.Example output (English, default):
Example output (French, with host-app
fr.yml):Tests
Added/updated tests:
save_with_statereturnsfalseand surfaces a proper error on invalid transition.save_with_statedoes not error when the form submits the current state (the real-world Sidecare scenario).has_one_state_machinedefines a publicfield_namereader (regression guard for the form-helper bug).has_one_state_machinedoes not clobber a pre-existing method named likefield_name.:invalid_transitionwith humanizedcurrent_state/target_stateoptions.All tests pass:
Migration notes
None for app code — the change is fully backwards-compatible. Apps that already had locale entries under
statesman.<field_name>_<model>will start seeing humanized state names in transition error messages.Apps that asserted on the exact text of the previous error message (
"cannot transition from user_pending to invalid_state") will need to update those assertions to match the new humanized format.