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

Even more flexible task handlers #161

Open
jonathanbp opened this issue Oct 12, 2023 · 9 comments
Open

Even more flexible task handlers #161

jonathanbp opened this issue Oct 12, 2023 · 9 comments
Labels
enhancement New feature or request hacktoberfest help wanted Extra attention is needed

Comments

@jonathanbp
Copy link

I have a use case where the logic that determines how to execute a given task depends on how that task is configured, i.e. which attributes it has. Would you consider adding a method on the newTaskHandlerCommand type which takes a predicate func to determine if a given handler can be invoked. It would look something like:

engine.NewTaskHandler().Matching(func(t *BPMN20.TaskElement) bool { <some logic goes here> }).Handler(myHandler);

I realise there was some work done in #57 and #58 to make the handler assignment more dynamic (with task types) but for my case there wouldn't be a specific type to distinguish between tasks rather a couple of other extension properties.

@jinjaghost
Copy link

What if no logic match a handler ? Do you imagine a default handler of last-resort ?

@nitram509
Copy link
Owner

@jonathanbp
thanks for proposing that feature ...
just another detail question, on that:

  • would the current BPMN20.Taskelement have enough attributes, to fulfill your needs?

@nitram509
Copy link
Owner

@jinjaghost thanks for chiming in ... IMHO, the "default" handler is a concept, not present yet - and also would help for the other handlers as well.
I would suggest not to couple this issue with the general problem , but rather consider them separate feature.
What do you think?

@nitram509
Copy link
Owner

@jonathanbp May I ask, is this just a feature request, or do you consider participating the Hacktoberfest and prepare a PR as well?

@jinjaghost
Copy link

@jinjaghost thanks for chiming in ... IMHO, the "default" handler is a concept, not present yet - and also would help for the other handlers as well. I would suggest not to couple this issue with the general problem , but rather consider them separate feature. What do you think?

This issue is about adding some logic to task handler selection, i think that the "default" handler concept is part of it because in any case, it will be necessary to ultimately select a handler.

As in #149, jsonlogic would be a perfect fit to represent that logic.

@jonathanbp
Copy link
Author

In my use-case the task element would probably have enough attributes to select the handler, but having a predicate would mean the decision as to which handler is selected can be controlled in the handler itself at runtime. I'd be happy to do a PR - I just wanted to check whether the idea meshed well with the other approaches or if there were other reasons not to do this or other and better approaches.

@jinjaghost
Copy link

If i am not missing something, why don't you run the decision logic from the main handler and call the right handler from there ?

@jonathanbp
Copy link
Author

That is what I'm doing at the moment, but I thought this would a more appropriate approach and a nice addition to the library.

@nitram509
Copy link
Owner

@jinjaghost thanks for reminding me about #149 (and sorry for my late reply).

Since this issue and #149 do address the same need, of providing more freedom/flexibility in implementing handlers,
I do consider this one more simple. A generic handler function can implement anything.
Adding jsonlogic on top I don't have seen so often and don't consider that much of a benefit.

About the default handler ... I do consider this feature would already address this and helps us to get default handlers implemented. @jonathanbp would you add this logic as well, please (looks like a quick win to me)?

Happy to review any PR @jonathanbp and also adding "hacktoberfest-accepted" label with pleasure, in case you're participating.

Some implementation hints...

  • add some example code in the documentation
  • ensure documentation covers this new option
  • unit tests are present
  • for the default == CatchAll this could look like engine.NewTaskHandler().CatchAll().Handler(myHandler); internally, this function could simply use the new Matching() function which always returns true == hence the quick win ;)

@nitram509 nitram509 added enhancement New feature or request help wanted Extra attention is needed hacktoberfest labels Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hacktoberfest help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants