-
Notifications
You must be signed in to change notification settings - Fork 118
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
Update FormFlow.php #235
base: master
Are you sure you want to change the base?
Update FormFlow.php #235
Conversation
Includes a flag to determine if we should expire the flow on a POST/PUT request, so we can create a flow from a POST/PUT request like this workflow Load Route We do some stuff, for example in AngularJS Once finished our stuff, we submit data to the same controller (or another controller). That controller, handles the data and creates a flow according to submitted data. Flow is rendered. This workflow won't work right now, because every POST/PUT request expires the flow.
I wonder why there's no stored data available to the flow, which in turn causes the flow to expire. Could you elaborate on this? |
@craue I don't understand what do you mean? This is my controller, if you want to see my workflow public function formAction(Request $request)
{
$product = new Product();
if($category = $request->get('category')) {
$product->setCategory($category);
}
$flow = $this->container->get('product_flow');
$flow->bind($product);
$form = $flow->createForm();
if($category || $flow->isValid($form)) {
$flow->saveCurrentStepData($form);
if ($flow->nextStep()) {
$form = $flow->createForm();
} else {
$data = $flow->getDataManager()->load($flow);
$this->manager()->save($product, $data);
$flow->reset();
return $this->redirectToRoute('success');
}
return $this->render('form.html.twig',array(
'form' => $form->createView(),
'flow' => $flow,
));
}
return $this->render('preindex.html.twig',array('flow' => $flow));
} You may wonder why I need |
I mean |
It won't be processed even with invalid data, as $category will be null always but the first time (when we set the data to the product that has the form), then, it's always empty as the POST request won't have any 'category' stuff. The workaround is because before the commit that came with the POST expiration, our flows were working perfectly, now, they're expired... We may be confused and wrong about how to make our flow (in this particular case, with a POST request), in that case I don't know how to proceed. About I've also tried to create the flow ONLY after the post, so no flow is created before we submit our angularjs stuff, and it's expired too. EG. public function formAction(Request $request)
{
$product = new Product();
if($category = $request->get('category')) {
$product->setCategory($category);
}
if($category || $product->getCategory()) {
$flow = $this->container->get('product_flow');
$flow->bind($product);
$form = $flow->createForm();
if($flow->isValid($form)) {
$flow->saveCurrentStepData($form);
if ($flow->nextStep()) {
$form = $flow->createForm();
} else {
$data = $flow->getDataManager()->load($flow);
$this->manager()->save($product, $data);
$flow->reset();
return $this->redirectToRoute('success');
}
}
return $this->render('form.html.twig',array(
'form' => $form->createView(),
'flow' => $flow,
));
}
return $this->render('preindex.html.twig',array());
} |
Would it work to create the flow with a GET request? Sending a POST before the flow's storage has been initialized seems to be the issue here. |
Using a GET request won't work as desired. Using GET, we get querystring The bug is also expiring forms in other controller actions, as if we send the POST request to And all that, is why I thought using a flag for expiring the form is needed in our case, and as it is harmless I made the PR, if you don't set it to |
@Nairebis, do you have an opinion on this? |
@robhunt3r, I thought about using GET only for the first request to properly start the flow. You don't need to send the I'm still not sure if the issue is in the bundle or in your code handling the flow, so I'm not yet convinced that the bundle needs another option. Would it help to change your action to this? (Edit: Never mind, that's silly, since the instance id is not set when public function formAction(Request $request) {
$product = new Product();
$flow = $this->container->get('product_flow');
if ($category = $request->get('category')) {
$product->setCategory($category);
$flow->getDataManager()->save($flow, array());
}
if ($category || $product->getCategory()) {
$flow->bind($product);
$form = $flow->createForm();
... If not, could you set up a reasonable test (or project) that passes with your introduced |
I will upload a test on Monday, as I am leaving the office now and it's holydays until monday, and I don't have the project at home. |
I'm not sure I completely have a handle on how things are being used here, but it sounds like the form is initialized through a POST request? But the expiration detection won't trigger unless it already has an instance ID generated (the creation sets the newInstance flag, which blocks the expiration action). robhunt3r mentioned needing "flow in my preindex template". I'm thinking that might be generating the instance ID somehow, and then perhaps bind() isn't getting called, which is what initializes the storage slot. Then POSTing into the form unknown to the session triggers the expiration, which is what it should do. Assuming the above is what's really going on, I can see three ways to handle it, depending on what makes sense with what robhunt3r is doing:
Of course, #2 is attractive, since that wouldn't require any client code changes. Just casually looking at the code, the slot initialization could be done in determineInstanceId() where it's actually generated. I think the reason I didn't do it that way in the first place was: 1) it architecturally made sense to do it in bind() where all the rest of the magic happens, and 2) I was thinking it might create unused slots in the session if instance IDs were casually created multiple times, which I wasn't sure about. |
@craue Load data fixtures to have some data. There are two URL http://localhost/app_dev.php/create [GET] There's also http://localhost/app_dev.php/noextra, in that URL, try to submit the form without filling any input, you will get a This value is not valid. error, if you then fillt he data an try to submit again, you will have the expired flow error. In that URL i'm also trying to recreate another error I got randomly in my project, but I will open another PR if I recreate it successfuly. @Nairebis ping. |
With these changes, the diff --git a/src/AppBundle/Controller/DefaultController.php b/src/AppBundle/Controller/DefaultController.php
index 4264e8a..8d33a3e 100644
--- a/src/AppBundle/Controller/DefaultController.php
+++ b/src/AppBundle/Controller/DefaultController.php
@@ -76,7 +76,7 @@
$flow->saveCurrentStepData($form);
if($flow->nextStep()) {
-
+ $form = $flow->createForm();
} else {
$em = $this->getDoctrine()->getManager();
$em->persist($formData);
diff --git a/src/AppBundle/Entity/Vehicle.php b/src/AppBundle/Entity/Vehicle.php
index 05ab26e..2c4006c 100644
--- a/src/AppBundle/Entity/Vehicle.php
+++ b/src/AppBundle/Entity/Vehicle.php
@@ -32,7 +32,7 @@
/**
* @var integer
*/
- private $price;
+ private $price = 0;
/**
* @var \AppBundle\Entity\Category
diff --git a/src/AppBundle/Resources/views/preCreate.html.twig b/src/AppBundle/Resources/views/preCreate.html.twig
index f9d93d6..6f1a701 100644
--- a/src/AppBundle/Resources/views/preCreate.html.twig
+++ b/src/AppBundle/Resources/views/preCreate.html.twig
@@ -24,6 +24,8 @@
{% block render_form %}
<form method="POST" name="testForm" action="">
+ <input type="hidden" name="{{ flow.getInstanceKey() }}" value="{{ flow.getInstanceId() }}" />
+ <input type="hidden" name="{{ flow.getFormStepKey() }}" value="{{ flow.getCurrentStepNumber() }}" />
<select name="product" class="form-control">
<option value="1">Product 1</option>
<option value="2">Product 2</option> |
Great, so I just need to add theese two lines
I can't set |
The issue is that this custom form was sent without these fields identifying the current flow instance. Even better if this also fixes your real project. 😏 There's still something odd after a validation error occured since the instance and step field have no value which in turn leads to the "flow expired" message again when submitting. I just had to set a non-null value for the price to avoid an error when persisting in order to make the example code work. I did't take a look at the |
Okay I will try, about the noextra stuff, I will upload "soon" (I'm very very busy right now) a full example, it does some weird stuff, but I "fixed" in my project removing some angular/javascript and stuff. |
@robhunt3r, any news? |
@craue sorry we were so busy at work because project needs to launch asap and I forgot about this, I will postit so tomorrow can test and give you feedback. |
Includes a flag to determine if we should expire the flow on a POST/PUT request, so we can create a flow from a POST/PUT request like this workflow
Load Route
We do some stuff, for example in AngularJS
Once finished our stuff, we submit data to the same controller (or another controller).
That controller, handles the data and creates a flow according to submitted data.
Flow is rendered.
This workflow won't work right now, because every POST/PUT request expires the flow.