timriley
commented
6 years ago Hi @Hugo-Hache, thanks for putting this together! This pretty closely matches what I was thinking when I described the idea in https://github.com/dry-rb/dry-transaction/issues/99#issuecomment-387569170.
Taking a look over it, the only thing I'm not certain about his how you're using that defined? check to determine whether an adapter offers a failure class.
I feel a better approach might be for all adapters to offer a #failure(step, value) method, so we need any conditionals around building failures. However, that would mean that the adapter classes now need to know about the step objects, which is an increase in their responsibilities, so I'm not fully sure. Might be a sign that we need to rework things further to keep things nicely arranged.
Anyway, I know @flash-gordon was interested in taking a look, so I'll ping him here and see if he has any thoughts in the meantime. Otherwise I might come back to this after thinking it over for a few days 😅
99
Based on @timriley suggestion,
Aroundstep adapter now has its ownStepFailuresubclass. This subclass unwraps the step failure contained in value and adds anaround_stepinstance variable to keep track of the failed around step.Matching the failing step of a transaction using an around step adapter is now tested, and working.
It's my first PR on dry-rb and I am quite new to the functional world, so please don't hesitate to point my possible misunderstandings of the codebase or paradigm.