laravel-model-states
laravel-model-states copied to clipboard
Equalize DefaultTransition and custom Transitions constructors
Hello again.
This should be the last pull request i need to do. :smile:
I'm sorta confused by the behaviour of resolveTransitionClass()
currently.
Why doesn't a custom transition gets the same parameters as the default one and vice versa?
Also i don't see currently how a custom transition can advance to the newState as it's not passed to it. I suppose this is because it's possibly not intended to write a generic custom transition? As in a custom transition should have an hardcoded target state.
Could you shed some light on this? maybe i'm taking the wrong approach. Feel free to look at my proposed change.
This if it gets merged should possibly require a major version since i'm changing the constructors and could break existing people's code.
Thanks again for your help.
Hello. Here's my proposal.
I created a TransitionContext object wich will hold all relevant information that a Transition will need to be able to both handle generic and specific usages.
Code from resolveTransitionClass():
$transition = new $transitionClass(
new TransitionContext(
$this->model,
$this->field,
$newState,
),
...$transitionArgs
);
Should the user want to override the landing state after handle() execution they can do so since access to newState is now given. Or create a generic transition to be applied ondemand (wich i think is more flexible than overriding the default one and is my usecase).
The only problem now is that the Transition constructor itself needs to be standardized to handle when $model is a TransitionContext.
I decided this could be a good tradeoff since it doesn't require users wich want to apply transitions using the transition()
to provide a TransitionContext but can provide directly the model itself wich i suppose is something you would want to do always anyways.
if ($model instanceof TransitionContext) {
$this->model = $model->model;
} else {
$this->model = $model;
}
I'm still unsure wether the code involving $model derivation should be part of the Transition abstract class. Mostly for the reason that i've seen it defined in DefaultTransition using protected and in tests using private, and i think it could be better to allow the user to choose.
Let me know what you think. Thanks for your time. :smile:
I have squashed the commits. This should be ready for review now.
Dear contributor,
because this pull request seems to be inactive for quite some time now, I've automatically closed it. If you feel this pull request deserves some attention from my human colleagues feel free to reopen it.