-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Workflows with XState #1019
Workflows with XState #1019
Conversation
source/SIL.AppBuilder.Portal/src/routes/(authenticated)/admin/workflows/+page.server.ts
Outdated
Show resolved
Hide resolved
...IL.AppBuilder.Portal/src/routes/(authenticated)/admin/workflows/[product_id]/+page.server.ts
Outdated
Show resolved
Hide resolved
...IL.AppBuilder.Portal/src/routes/(authenticated)/admin/workflows/[product_id]/+page.server.ts
Outdated
Show resolved
Hide resolved
...e/SIL.AppBuilder.Portal/src/routes/(authenticated)/admin/workflows/[product_id]/+page.svelte
Outdated
Show resolved
Hide resolved
...e/SIL.AppBuilder.Portal/src/routes/(authenticated)/admin/workflows/[product_id]/+page.svelte
Outdated
Show resolved
Hide resolved
source/SIL.AppBuilder.Portal/src/routes/(authenticated)/tasks/[product_id]/+page.server.ts
Outdated
Show resolved
Hide resolved
source/SIL.AppBuilder.Portal/src/routes/(authenticated)/tasks/[product_id]/+page.svelte
Outdated
Show resolved
Hide resolved
source/SIL.AppBuilder.Portal/common/workflow/default-workflow.ts
Outdated
Show resolved
Hide resolved
source/SIL.AppBuilder.Portal/common/workflow/default-workflow.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good; well done.
Here's a first-look review. I haven't reviewed the default-workflow.ts
stuff at all, and I haven't tested anything, but here's some comments I have looking through it rather quickly. Some questions are probably somewhat ignorant because I'm not entirely clear what your goal is, but I think they'll be helpful.
One larger thing: you're handling the state machines with a lot of functional programming. You have a lot of functions that take WorkflowMachine
or WorkflowContext
as (often the first) argument, which makes me wonder if it could be done in a more object-oriented way, making things more intuitive to write.
source/SIL.AppBuilder.Portal/src/routes/(authenticated)/tasks/[product_id]/+page.server.ts
Outdated
Show resolved
Hide resolved
I have created a wrapper class for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, it does look a lot better and easier to use. Let me know what you think of my feedback.
9047c04
to
3dd3423
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, let's try it out!
e3094e9
to
6f1e44c
Compare
673a738
to
5458627
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch to using svelte-kit superforms in a few pages
...e/SIL.AppBuilder.Portal/src/routes/(authenticated)/admin/workflows/[product_id]/+page.svelte
Outdated
Show resolved
Hide resolved
source/SIL.AppBuilder.Portal/src/routes/(authenticated)/tasks/[product_id]/+page.svelte
Outdated
Show resolved
Hide resolved
Why? The productId passed into the factory method should be the productId to use, regardless of what may be stored in the snapshot. Technically, the productId doesn't need to be stored in the snapshot; it's only in there because it's part of the state machine context. It also isn't used from the context either, but it will be in the future because it is necessary for the build engine hooks that will be added in a future PR.
Even better, just don't write the productId (or the other dynamically calculated fields) in the first place
There is no i18n yet for actions or individual instructions.
7441cc0
to
5bc28bf
Compare
- link to go back to My Tasks - breadcrumb link to go back to project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Uses the XState library to create FSM instances that model the desired workflow behavior.
Features:
WorkflowInstances
) that stores enough information to restore to the current state.UserTasks
table with the appropriate information.ProductTransitions
table with the appropriate information./tasks/[product_id]
and are handled by the FSM./admin/workflows
./admin/workflows/[product_id]
Future Priorities:
/admin/workflows