-
Notifications
You must be signed in to change notification settings - Fork 1
Task A: Implemented redirect route for task console to get better UX for users #130
Conversation
rimi-itk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good! Nice and clean solution.
Some comments and suggestions added.
os2forms_forloeb.routing.yml
Outdated
| @@ -0,0 +1,9 @@ | |||
| os2forms_forloeb.forloeb_task_console_controller_execute: | |||
| path: 'execute-foloeb-task' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: “foloeb” should be “forloeb”.
Maybe we should use the module name in the path to get some namespacing, e.g. os2forms_forloeb/execute-forloeb-task, say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
| else { | ||
| $this->messenger()->addWarning($this->t('No tasks found to execute.')); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider an early return if no queue record is found to make the code a little easier to read, i.e.
if (!$queueRecord) {
$this->messenger()->addWarning($this->t('No tasks found to execute.'));
return new RedirectResponse($redirect_to->toString());
}
$handler = $queueRecord->handler->getString();
…There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added early return
| public function getQueueIdByWebformSubmissionToken($token = '') { | ||
| $engine = new MaestroEngine(); | ||
| // Fetch the user's queue items. | ||
| $queueIDs = $engine->getAssignedTaskQueueIds(\Drupal::currentUser()->id()); | ||
|
|
||
| foreach ($queueIDs as $queueID) { | ||
| $this->entityTypeManager->getStorage('maestro_queue')->resetCache([$queueID]); | ||
| /** @var \Drupal\maestro\Entity\MaestroQueue $queueRecord */ | ||
| $queueRecord = $this->entityTypeManager->getStorage('maestro_queue')->load($queueID); | ||
| $processID = $engine->getProcessIdFromQueueId($queueID); | ||
| $templateMachineName = $engine->getTemplateIdFromProcessId($processID); | ||
|
|
||
| // Get user input from 'inherit_webform_unique_id' | ||
| $taskMachineName = $engine->getTaskIdFromQueueId($queueID); | ||
| $task = $engine->getTemplateTaskByID($templateMachineName, $taskMachineName); | ||
|
|
||
| // Load its corresponding webform submission. | ||
| $sid = $engine->getEntityIdentiferByUniqueID($processID, $task['data']['inherit_webform_unique_id'] ?? ''); | ||
| $webform_submission = $sid ? WebformSubmission::load($sid) : NULL; | ||
|
|
||
| if ($webform_submission && $webform_submission->getToken() != $token) { | ||
| return $queueRecord; | ||
| } | ||
| } | ||
|
|
||
| return NULL; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to get the queue record directly from the token without having to load and iterate over all queues and submissions, i.e. something like
// Warning: Pseudocode ahead!
$submission = $this->entityTypeManager->getStorage('webform_submission')->findByToken($token);
return NULL !== $submission
? $this->entityTypeManager->getStorage('maestro_queue')->findBySubmission($submission)
: NULL;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's possible in an easy way. I can not find a direct connection between maestro_queue and webform_submission to implement this kind of lookup.
By idea, maestro_queue can have different handlers and reference to webform submission is on of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I finally found a way to get it f92e668
rimi-itk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good work!
|
Hi @rimi-itk |
|
Sorry, @andriyun, I don't have permissions to merge, but maybe @madsnorgaard or @agger-magenta can help us? |
|
It's strange, but I can't seem to merge this either. @madsnorgaard, can you do it? |
|
Squashed and merged. |
Custom path:
/execute-foloeb-taskThe ability to refer to the specific task in email is possible via token, for example
/execute-foloeb-task?token=XebF7BbNosDZNM-GNDP7ADH0vaPbR74pLSf8pnXCalEThe code is available for testing as a composer package https://packagist.org/packages/os2forms/os2forms_forloeb#dev-AARHUS-ITK-A