Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to have multiple pending transitions for same column #14

Open
binaryweavers opened this issue Feb 3, 2021 · 9 comments
Open

Comments

@binaryweavers
Copy link

Summary

@asantibanez, Thank you for the awesome and simple package.
I have a use case when there are multiple pending transitions that need to run one after another. Currently when transitionTo(...) method deletes all the pending transitions of the column, so the first pending transition i.e. call to transitionTo(...) would wipe the rest.

Proposal

I think there might be some sort of opt-in boolean type flag default to true on the pending transition itself which can be configured when pushing a pending transition. e.g. if it should be canceled or not after the latest transition.

@asantibanez
Copy link
Owner

Hi @binaryweavers !

Thanks for your interest in the package.

I've also thought about the use case you mention. I think adding a cancellable flag of some sorts to the pending transitions might work. Will give it a try.

Thanks for the suggestion.

@binaryweavers
Copy link
Author

Hi @binaryweavers !

Thanks for your interest in the package.

I've also thought about the use case you mention. I think adding a cancellable flag of some sorts to the pending transitions might work. Will give it a try.

Thanks for the suggestion.

Pleasure @asantibanez , in fact thank you for building for community.
I am currently using the solution as I mentioned above by overriding migration and couple of methods in StateMachine.

@binaryweavers
Copy link
Author

One more thing I felt in v4, not sure if i should open a new issue or so, but in transition hooks, specicially ,afterTransitionHooks, they are triggered for the previous state. I think afterTransitionHooks should be triggered for new state a field has been transitioned to 🤔
https://github.com/asantibanez/laravel-eloquent-state-[machines/blob/f2a70b554473825833b227bbb65617f6cedc495d/src/StateMachines/StateMachine.php#L132]

@asantibanez
Copy link
Owner

Thanks @binaryweavers for the feedback.

It felt confusing for me that before transitions key were the "previous state" and after transitions key were the "new transitioned state". That's why I left both on the "previous state" for key.

Definitely a breaking change but made more sense to me. What do you think? Did the previous approach make more sense?

@binaryweavers
Copy link
Author

Yes, I think "previous state" for before is fine. But for after, they should be triggered based on 'new transitioned state'. I have a use case where some actions are after performed based on "new transitioned state", in v4 it requires if checks for the passed '$to' state to figure out what the new state is.

@asantibanez
Copy link
Owner

Yeah. You have to add checks now. I think it makes sense to have that change reversed. 🤔

@binaryweavers
Copy link
Author

Yes, That covers most of the use cases 👍

@asantibanez
Copy link
Owner

asantibanez commented Feb 21, 2021 via email

@binaryweavers
Copy link
Author

Sounds good. Did you try the new changed attributes feature?
On Sun, 21 Feb 2021 at 13:57 Wali Razzaq @.***> wrote: Yes, That covers most of the use cases 👍 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHDT6CWCO4RDUAV6ZXFWVDTAFJPZANCNFSM4XA6KQ2Q .
-- Saludos, Andrés Santibáñez B

Unfortunately, I couldn't get to that yet. But that feature is a good addition though 👍.
I'll definitely give a feedback on that as soon I use that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant