javrasya / django-river

Django workflow library that supports on the fly changes ⛵
BSD 3-Clause "New" or "Revised" License
740 stars 105 forks source link

Workflow pattern issue (circular not working) #188

Open JohnieBraaf opened 3 years ago

JohnieBraaf commented 3 years ago

I'm running into strange issues with a particular workflow design. As soon as I go back a step, the transition in the opposite direction does not re-enable.

Say, I move from bewerken to 4 ogen controle and then back to bewerken, I cannot move to 4 ogen controle afterwards. Any thoughts?

image

JohnieBraaf commented 3 years ago

This simplified design works just fine:

image

javrasya commented 3 years ago

Hi @JohnieBraaf, thanks for reporting this. Does it mean that the simplified version is something you can go with?

Regardless, I will look into the first flow and try to reproduce it as soon as possible.

JohnieBraaf commented 3 years ago

Hi @javrasya, yes for now I have worked around this issue by using the second workflow. If I have time, I will create a demo project over the weekend to demonstrate the issue.

JohnieBraaf commented 3 years ago

I have not yet found the time to create a sample project. But I do suspect this piece of code to be the culprit:

https://github.com/javrasya/django-river/blob/master/river/core/instanceworkflowobject.py#L57-L59

JohnieBraaf commented 3 years ago

@javrasya, it seems that you are initializing the complete workflow upon creation of an workflow instance object. Why not choose for the option of instantiating only the next available transitions/approvals and repeat after each approve?

I'm measuring database read/write and those are huge. Like about 100 per single case creation. I have created a workaround for this as described above. Please let me know your thoughts and if I need to consider an PR.

JohnieBraaf commented 3 years ago

I have created a commit that:

https://github.com/JohnieBraaf/django-river/commit/84159da874c217da92c7778feac96f9ea593db50 must be used together with https://github.com/JohnieBraaf/django-river/commit/0d0db850eae3b8535c9833d9c06a280104346f05

These changes alone slashed the total number of database operations in half. But still there are a lot of database calls going on.

javrasya commented 3 years ago

@javrasya, it seems that you are initializing the complete workflow upon creation of an workflow instance object. Why not choose for the option of instantiating only the next available transitions/approvals and repeat after each approve?

I'm measuring database read/write and those are huge. Like about 100 per single case creation. I have created a workaround for this as described above. Please let me know your thoughts and if I need to consider an PR.

Hi @JohnieBraaf , thanks for giving a thought to this. The thing with that is that it allows customizing the workflow for a specific object. So it is like that by design. I don't think we should change the behavior unless it still supports this.

The only way to make django-river work that way would be to introduce something like overriding rules as part of the workflow which I have been thinking about but never had the time to invest in that. Then you use a workflow template and never have to re-create the entire workflow on and on but customize it for a specific object by defining overriding rules.

javrasya commented 3 years ago

You don't need a full-fledged project to re-produce it btw. Here is an example of reproducing a scenario in a human-readable cucumber test;

https://github.com/javrasya/django-river/commit/72cdd6d148d158eeb4f2e8b36da81b96fd4f1517

JohnieBraaf commented 3 years ago

Thanks for your reply and pointers on creating the test scenario for this.

I do not fully understand what you mean with the current design allowing customization of a a specific object. The proposed solution increases flexibility and allows for ad-hoc workflow changes, I guess that you try to achieve the opposite.

pupubird commented 3 years ago

Facing the same issue, I have the following patterns:

Reviewing -> Reverted -> Reviewing -> Reverted...

The Reverted state after 1 round of circulation was disappeared in Available Approvals list