craue / CraueFormFlowBundle

Multi-step forms for your Symfony project.
MIT License
736 stars 118 forks source link

How to handle unique constraints properly #302

Open tkleinhakisa opened 6 years ago

tkleinhakisa commented 6 years ago

Hello,

I've got a case that i can't figure out for now, it seems a common use case:

If the db throws a unique constraint error, i would like to add an error message on the field in step 1 and show it to the user so that he can change the value

It is similar to #82 but instead of a flash message i'd like to have a form error.

The only way i can see to do it is to set some session data, use the same redirection as in #82, and add the error to the form if i have a specific parameter set in the session

In an ideal world i'd like to be able to do something like:

$flow->goToStep(1);
$form = $flow->getForm();
$form->addError(...)

$this->render(...)

Any suggestion on how to achieve that ? Thank you

craue commented 6 years ago

Instead of trying to catch the database error, you should add a proper validation constraint on the entity itself using the correct validation group.

tkleinhakisa commented 6 years ago

Hi and thank you for the answer. I already have a validation constraint on the form but there can be concurency issue between the check of the validation and the insert itself so i need to find a way to handle this edge case. (this is also true when using a simple form without steps).

For example, see discussion in https://github.com/symfony/symfony/issues/20692

craue commented 6 years ago

True. So the issue is how to catch the error and add it to the form nicely? You could actually just do exactly that:

$flow->bind(...);
$form = $flow->createForm();
try {
    if ($flow->isValid($form)) {
        throw new \RuntimeException('test');
    }
} catch (\Exception $e) {
    $form->addError(new \Symfony\Component\Form\FormError($e->getMessage()));
}

This would show the error in the current step, which is not what you want. But by default, the validation for step 1 is also triggered when submitting step 2, so at least if the unique constraint is already violated when submitting step 2, you should be fine. If it happens between validating the flow (after clicking finish) and persisting the data, you need to rely on catching the exception.

tkleinhakisa commented 6 years ago

As we like to say: "This should not happen!" but this piece of the system is critical so we want optimize UX

I'm pretty much stuck at the same point. Correct me if i'm wrong:

The only idea i got so far was catching the exception, adding the error to the session, redirect to the appropriate step, have the init of the flow somehow check the session to add the error back to the form. Not a good balance between hassle and likelihood :(

As the form is really simple i could afford to just reset everything and redirect to the first step with a flash message but that's kind of disapointing ^^

Do you see any other possibilities ? Do you think it would require many changes to handle this use case ? (again, very unlikely, but valid for every unique constraint bound to a form and enforced at the db level)

thanks for the help

mcgoode commented 3 years ago

@craue

True. So the issue is how to catch the error and add it to the form nicely? You could actually just do exactly that:

$flow->bind(...);
$form = $flow->createForm();
try {
  if ($flow->isValid($form)) {
      throw new \RuntimeException('test');
  }
} catch (\Exception $e) {
  $form->addError(new \Symfony\Component\Form\FormError($e->getMessage()));
}

This would show the error in the current step, which is not what you want. But by default, the validation for step 1 is also triggered when submitting step 2, so at least if the unique constraint is already violated when submitting step 2, you should be fine. If it happens between validating the flow (after clicking finish) and persisting the data, you need to rely on catching the exception.

I am doing something very similar here but seems validation still gets set to true.

        // charge the CC
        if( $flow->getLastStepNumber() == $flow->getCurrentStepNumber() ){
            try {
                $token = $flow->getRequest()->request->get('createOrderStepFive')['token'];
                $stripe->createCharge(123.00, $token);
            } catch (\Exception $e){
                dump($e);
                $form->addError(new FormError($e->getMessage()));
            }
        }

        dump($flow->isValid($form));

image

craue commented 3 years ago

@mcgoode, this doesn't work since the errors are cleared while calling $flow->isValid($form).

mcgoode commented 3 years ago

@craue Do you have a suggested way to add form errors to inputs?