raoul2000 / yii2-workflow

A simple workflow engine for Yii2
BSD 3-Clause "New" or "Revised" License
171 stars 48 forks source link

Bug in validation #3

Closed abdullah-almesbahi closed 9 years ago

abdullah-almesbahi commented 9 years ago

I'm doing GUI for workflow and about to finish it. Workflow is working great with me except I found a bug. In creating new record page, if the user change the status using developer tools in chrome and make the value of status empty. The validation of workflow dose not catch it.

screen shot 2015-03-28 at 12 40 42 am

raoul2000 commented 9 years ago

Hi Cadr-sa, I will be on holidays for the next 10 days and will check this when I'm back. What you did is very impressive... !!

bye

raoul2000 commented 9 years ago

@cadr-sa : validation that occurs on server side is by definition not affected by any change done on the page (directly or through developer tools). When you write

The validation of workflow dose not catch it.

Are you refering to the client-side validation ?

abdullah-almesbahi commented 9 years ago

Sorry for the late reply , No, I'm not referring to client-side validation. for example when the user wants to add new record . The user must choose initial status so he can be able to create new record. The bug that the user can add new record without choosing any status by deleting from status dropdown. You need to add to your workflow validation to check if status not is set in $_POST , display error.

<select id="test-status" class="form-control" name="Test[status]">
<option value="TestWorkflow/one">One</option>
</select>
abdullah-almesbahi commented 9 years ago

Btw , I have fixed the problem by changing the status in MYSQL that can not be null

abdullah-almesbahi commented 9 years ago

I was wondering if you have any idea how to work with two workflows. First you need to look at the workflow in blew URL so you can understand my explanation.

https://s3.amazonaws.com/fvd-data/notes/401879/1428539107-LKw1Fq/screen.png

I was trying to use one workflow in Order but is not possible. The problem there is On Hold status can be at any level at workflow

for example if the order was In Queue status , for any reason we want to change it to On Hold. then again returned to the previous status which is In Queue. Again when the order reach to Waiting for payment status , for any reason we want also to change it to On hold and then returned back to previous status which is Waiting for payment.

The problem here if we worked with one workflow status are : 1- When the status is On hold , we will not know in which level of workflow the status changed to On hold. So we can't changed to previous status. 2- In each level there are a group of employees and their supervisor. So if the status changed from In Queue to On Hold , A supervisor of In Queue section will check the order. The same thing if the order was in different level and changed to On hold , the supervisor of that level will check the order.

What I'm trying to accomplish is to get all results for supervisor of In Queue level if the order changed from In Queue - On hold and the same thing with other levels.

raoul2000 commented 9 years ago

to reply to your first question, yii2-workflow does not force you to assign a status to a model, there is no obligation so that's the developer who must take care of that. In the use case you're describing, I see no reason why the user should choose a status when a record is created : maybe it should simply be inserted in the initial status automatically in the controller.

In the previous version there was an autoInsert options that automatically set the initial status on record creation, but this feature is not currently available in yii2-workflow.

What you did with the required rule on the status attribute is the good way to go ...

raoul2000 commented 9 years ago

To reply to your second comment ... well I must say that it is a little bit complex (the image of the workflow is not very clear). Tell me if I understand correctly : you want to be able to send a model in its previous status right ?

abdullah-almesbahi commented 9 years ago

yes , To make it clear for you , I'll add it by your style

[ 
    'initialStatusId' => 'pendingConfirm', 
    'status' => [
        //waiting for user to confirm the order
        'pendingConfirm'  => ['transition' => 'inQueue'],
        //add to in queue if the order is confirmed from user side
        'inQueue'  => ['transition' => 'assignedForPickup'],
        // we will assign the order to an agent to pickup packages for order, if one package is missing then change the status to [pickup problem]
        'assignedForPickup'  => ['transition' => 'pickedUp, pickupProblem'], 
        //if package found then change status to [processingOrderNow] otherwise to [pickupReview]
        'pickupProblem'  => ['transition' => 'processingOrderNow, pickupReview'],
        //if one of the packages is missing , remove the package from order and refund money for user for the missing package and then continue to process the order  
        'pickupReview'  => ['transition' => 'processingOrderNow'],
        //change to [pickedUp] when all packages of order is picked up
        'pickedUp'  => ['transition' => 'processingOrderNow'],
        'processingOrderNow'  => ['transition' => 'waitingForPayment'],
        //if the user paid change to [shippingSoon] otherwise if the user didn't pay for 2 months then change to [pendingDisposal]
        'waitingForPayment'  => ['transition' => 'shippingSoon, pendingDisposal'],
        'shippingSoon'  => ['transition' => 'shipped'],
        'shipped' , // end of workflow
        //Dispose the order
        'pendingDisposal'  => ['transition' => 'disposed'],
        'disposed', // end of workflow
    ]
]

This is my simple way to explain my workflow , now let make it more complex. in the image blew we will add transition permissions for each users Roles , so every user can change the status depending on his role https://cloud.githubusercontent.com/assets/1207996/6877564/2056002c-d4e3-11e4-9448-7dc8ba5ebbe8.png

now let make more complex : - //employee here can't transit from onHold to [assignedForPickup] or [shippingSoon] 1- How to transit from [inQueue] to [onHold] , then only from [onHold] to [inQueue] //employee here can't transit from onHold to [inQueue] or [shippingSoon] 2- How to transit from [assignedForPickup] to [onHold] , then only from [onHold] to [assignedForPickup] //employee here can't transit from onHold to [inQueue] or [assignedForPickup] 3- How to transit from [shippingSoon] to [onHold] , then only from [onHold] to [shippingSoon]

I hope I made it clear.

raoul2000 commented 9 years ago

I understand better, thanks. One question : what is "onHold" ? In your workflow definition there is no status "onHold"

I have drawned your workflow here ... tell me if it is correct

abdullah-almesbahi commented 9 years ago

onHold is status same as pendingConfirm and inQueue and the rest , I didn't add it to workflow because I do not know where or how to add it as I described in pervious comment.

Your drawned of workflow is correct , and Only one thing is left is to add onHold status to each level. and this is my problem

raoul2000 commented 9 years ago

Ok so, onHold can be reached from any other status, and it must be possible to go from onHold to the previous status. Maybe onHold should simply not be a status but an attribute of the model.....don't you think ?

abdullah-almesbahi commented 9 years ago

How about if we have 6 other statuses like onHold? I provided you a simple workflow, so your solution will be good with simple one. See here another complex workflow https://nimbus.everhelper.me/client/notes/share/191708/EJdwWVgZOE17eUnPPB6sVl6RqWKcDkfe/

if you checked the statuses in image , you will see I put many status with hyphen like Received - No Account , TO shelf - No Account , Not allowed - pending disposal

I do not think it will be good idea if we use it as attribute of the model with the last workflow. I don't feel what I did is a typical. That's why I was asking you any idea about using 2 workflows together , So we will have two columns in MYSQL , Main status and Sub status. What do you think?

raoul2000 commented 9 years ago

Theorically, using 2 workflow could indeed be a solution for your problem. Let me do some basic verification and test to see if 2 status can be used for the same model...

raoul2000 commented 9 years ago

I have made some modification in the code (version 0.0.6) to allow a model to be attached to more than one workflow. You need 2 columns (for example status and _substatus) and declare 2 behaviors. Please take a look to the CHANGE.md for more info.

I hope it will help you...

abdullah-almesbahi commented 9 years ago

@raoul2000 What you did is impressive , One thing is left and then it will be perfect. We need to make 2 workflows related in validation . For example: we can only transit from SecondaryWorkflow/success to SecondaryWorkflow/onHold if the main status was transiting from PostWorkflow/ready to PostWorkflow/published

https://github.com/raoul2000/yii2-workflow/blob/master/tests/codeception/unit/models/Item08Workflow1.php https://github.com/raoul2000/yii2-workflow/blob/master/tests/codeception/unit/models/Item08Workflow2.php

raoul2000 commented 9 years ago

I have to think how to achieve related workflow validation ... that's not simple.

One question : in the example you give, what happend if the main status was NOT transiting from 'readt' to 'published' ?

  1. both transition (primary and secondary) fail
  2. the secondary transition fails and the primary succeeds
abdullah-almesbahi commented 9 years ago

I know is not simple , that's why I'm here to discuss with you for a solution as you are an expert in workflow . Regarding to the question , currently I need the first choice , but the secondary choice should be an option to help others.

raoul2000 commented 9 years ago

I will need some more time to come with a good solution ... let me think about it...

raoul2000 commented 9 years ago

Ok, I have a possible solution based on attribute validation. Basically, when _statusex attribute is validated, a standalone validator is responsible for :

Have a look to the code here and in particular the RelatedWorkflowValidator that implements the related validation logic.

This is just a draft and can surely be optimized.

philippfrenzel commented 9 years ago

hmm, sounds like a way to go;) maybe you can work with events?

raoul2000 commented 9 years ago

Yes philip, going with event is an equivalent solution but I don't think it would be more simple ....

I still suspect there is something wrong with using 2 worklows on the same model... it's too complex, in particular when you introduce dependencies between both workflows... it can become a nightmare to maintain.

@cadr-sa based on this representation of the workflow, why not create 3 workflows (red, blue, orange) and link them together. Did you know it is possible for a model to change workflow ?

class PostWorkflow implements IWorkflowDefinitionProvider 
{
    public function getDefinition() {
        return [ 
            'initialStatusId' => 'success',
            'status' => [
                'success' => [
                    'transition' => ['onHold']
                ],
                'onHold' => [
// a model is status PostWorkflow/onHold is able to go to anotherWorkflow/statusName
                    'transition' => ['success','anotherWorkflow/statusName']
                ],
            ]
        ];
    }
}
vkonde commented 8 years ago

hi, i want to implement same scenario for document uploading.i want to add multiple workflow, it change by user role and document type vise so can u suggest me any tutorial please.

vkonde commented 8 years ago

I have User role : 1) System Admin 2) Manger 3) Client Manger 4) Client

Document Type: 1) A 2) B : : 3) n

And also custom actions..

raoul2000 commented 8 years ago

hi, I don't know any tutorial on this subject but I can give you some advices... First in a simple scenario, your document can go through many workflow, but at a certain time is it assign to only one workflow. You could for example create all your workflows and link them together. You can also create condition based on user role to allow or forbid transitions between statuses. This can be achieved using status constraints ( see documentation).

A more complex solution is to implement multi-workflow for your documents, which means that at a certain time a document belongs to more than one workflow. I would not recommend to go this way as it may become quite complex to manage. @cadr-sa had the same kind of demand and I came with a solution ( see example) but I don't know if in the end it was implemented. Anyway, before going for this solution, you should ask yourself if it is appropriate...

vkonde commented 8 years ago

thank you very much for replay.. i am wandering around a day for that, I prefer your first solution

vkonde commented 8 years ago

so as per multiple type document and according user role can you guess how many workflow classes and is it for my single Upload Model class

vkonde commented 8 years ago

I read Workflow Events but can't understand how these implementing in my scenario.. please can you tell me raw structure.

raoul2000 commented 8 years ago

I need more detail about your specifications to be able to reply.

Moreover note that I am currently travelling so I can't reply quickly ...

vkonde commented 8 years ago

simply dynamically creating multiple workflows for my user role having different document types with custom actions such as (CRUD). that having access of that document. for example.

Client Manager => Document A => after create => Validate. a Client manager can edit or validate Document Type A And assigned to Manager for Approval.

raoul2000 commented 8 years ago

What do you mean by "dynamically creating multiple worklows" ? Do you want to create many workflow definition ? It is not clear to me what you want to achieve.

The idea I had is that you create one workflow definition (for example in DocumentWorkflow.php), and then, attach "before" event handlers to the transition you want to control.

The concept of Document Type is not the same as Document Status right ? The document type doesn't change when the document status does change as it evolves inside its workflow.

Do you have a graphical representation of your Document workflow ? that could really help me understand ...

vkonde commented 8 years ago

hi, sorry for late replay.. but i have done with Generic workflow i have UploadresultWorkflow: public function getDefinition() { return [ 'initialStatusId' => 'draft', 'status' => [ 'draft' => [ 'transition' => ['assigned','deleted'] ], 'assigned' => [ 'transition' => ['approval','approve','disapprove','deleted'] ], 'approval' => [ 'transition' => ['approve','disapprove','deleted'] ], 'approve' => [ 'transition' => ['draft','deleted'] ], 'disapprove' => [ 'transition' => ['draft','deleted'] ], 'deleted' => [ 'transition' => ['deleted'] ] ] ]; } so, how can i make this All status dynamically changeable , reordering by admin.

vkonde commented 8 years ago

can we save status in the DB?

philippfrenzel commented 8 years ago

Should be written into the field status?

vkonde commented 8 years ago

Thank's yes i know but, my question is ,can we fetch all status form DB in our workflow getDefinition() and also, all transitions?

vkonde commented 8 years ago

Please can any one direct me???

raoul2000 commented 8 years ago

Hi I am currently away from computer, on holiday with no good internet connections, ans back in one week-end. There is no db worflow source component TO allow you store jour worflow définition in db, but you can easely write tout own. I write à prototype some time ago... Ciao

raoul2000 commented 8 years ago

Please open a new issue si i can follow it more easy