[#46800] Created new task-type for inheriting webform in flow #92
Conversation
mBoegvald
force-pushed
the
change/46800_webform_inherit_task_type
branch
from
62a1f84 to
867db77
Compare
|
This MR is ready for review i'd say. |
|
I'll look at it then! |
agger-magenta
left a comment
agger-magenta
left a comment
There was a problem hiding this comment.
Nogle få kommentarer, generelt ser det fint ud!
mBoegvald
force-pushed
the
change/46800_webform_inherit_task_type
branch
from
867db77 to
e179ae4
Compare
|
The PR has been updated with the mentioned improvements, I'm removing the Draft status and await final approval. I have attached the flow and the webforms I've used to test it. Hjemmearbejde_flow.txt |
|
Ja, nu ser det godt ud! |
rimi-itk
left a comment
rimi-itk
left a comment
There was a problem hiding this comment.
Looks good. A few changes are required. Note that I've added a comment on line 108, but this comment is hidden inside the previously resolved conversation on this line.
mBoegvald
force-pushed
the
change/46800_webform_inherit_task_type
branch
from
e179ae4 to
991b561
Compare
rimi-itk
left a comment
rimi-itk
left a comment
There was a problem hiding this comment.
I missed the missing error handling in my first review.
Please don't force push changes! This makes it harder to review changes.
| if (!isset($webform_submission)) { | ||
| \Drupal::logger('os2forms_forloeb')->error( | ||
| "Predecessors must have submissions with webforms attached." | ||
| ); | ||
| } | ||
| // Copy the fields of the webform submission to the values array. | ||
| foreach ($webform_submission->getData() as $key => $value) { |
There was a problem hiding this comment.
You need to handle the case when $webform_submission is not set, i.e. by returning early. Just logging the error is not enough.
There was a problem hiding this comment.
I changed line 109-111 to:
\Drupal::logger('os2forms_forloeb')->error( "Predecessors must have submissions with webforms attached." ); return FALSE
I'm wondering in which scenario we would use this task-type without having a submission. I don't see how we would ever get to an WebformInheritTask without having posted a submission earlier in the flow (as the whole functionality of this task-type is to get earlier submission's data).
There was a problem hiding this comment.
I think you're missing a git push …
There was a problem hiding this comment.
Yes, sorry. I haven't pushed yet, I didn't test it before writing the comment, so it got a bit delayed, my bad.
This is not thoroughly tested, but this is at least a first iteration of the new task-type.