AaronLasseigne / active_interaction

:briefcase: Manage application specific business logic.
MIT License
2.06k stars 136 forks source link

`ActiveInteraction::Errors#merge!` does not merge custom options #554

Closed danila closed 1 year ago

danila commented 1 year ago

Hello there! I stumbled upon a behavior that I find unobvious. Although I'm not sure I cook errors the right way they are designed to be used. Please feel free to correct me if I'm wrong. And if not, I'll be happy to introduce missing (from my pov) logic.

I use errors in the following style: errors.add(:base, "Activity is completed", code: "activity_completed") So that I use the type part of an error as a holder for an error message, and then I add a code to the options hash. In case I use composition and want to merge errors of nested interaction with the errors from the current context, the nested error options are ignored, so I end up losing nested error codes. From what I see in the sources, the reason is type a String and therefore merge_detail! which considers options is not used during merging. Like that (execution happens inside interaction):

> result = resolve_activity_specific_interaction.run(user: user, activity: activity);
> result.errors
=> #<ActiveInteraction::Errors [#<ActiveModel::Error attribute=base, type=Challenge is completed, options={:code=>"activity_completed"}>]>
> errors
=> #<ActiveInteraction::Errors []>
> errors.merge!(result.errors)
=> #<ActiveInteraction::Errors [#<ActiveModel::Error attribute=base, type=Challenge is completed, options={}>]>

And I expected that errors.merge!(result.errors) would have produced

#<ActiveInteraction::Errors [#<ActiveModel::Error attribute=base, type=Challenge is completed, options={:code=>"activity_completed"}>]>

A possible workaround that would suit me could be an interface for ActiveInteraction::Errors which would allow pushing existing errors objects.

I will be very grateful for the clarifications.

AaronLasseigne commented 1 year ago

From what you've said that sounds like a bug. I'll have to do a bit of exploring to see what I can figure out.

danila commented 1 year ago

@AaronLasseigne thanks for looking into this! Since it sounds like a bug, I could also investigate this and suggest a possible fix in a PR, wdyt?

AaronLasseigne commented 1 year ago

A PR would be great!

AaronLasseigne commented 1 year ago

This is fixed in the 5.3.0 release. Let me know if you have any issues.