#655 Created "mark group received" runner #700
Conversation
|
@jensschuppe Could you please have a brief look to see you can spot any major blunders? |
| LIMIT %5", [ | ||
| 1 => [$this->txgroup['id'], 'Integer'], | ||
| 2 => [implode(',', $this->origin_status_ids), 'Integer'], | ||
| 3 => [$status_inProgress, 'Integer'], |
There was a problem hiding this comment.
Placeholder %3 seems obsolete now.
| CRM_Utils_System::setTitle(E::ts('Mark SEPA group received')); | ||
|
|
||
| // get the group ID | ||
| $group_id = (int) $_REQUEST['group_id'] ?? 0; |
There was a problem hiding this comment.
This should use \CRM_Utils_Request::retrieve().
| * | ||
| */ | ||
|
|
||
| require_once 'CRM/Core/Page.php'; |
There was a problem hiding this comment.
This shouldn't be necessary, as the class loader will take care of it.
| $queue = CRM_Queue_Service::singleton()->create([ | ||
| 'type' => 'Sql', | ||
| 'name' => 'sdd_close', | ||
| 'reset' => TRUE, | ||
| )); | ||
| ]); |
There was a problem hiding this comment.
According to the docs, this should use the new Civi::queue() syntax.
| $queue->createItem(new CRM_Sepa_Logic_Queue_Close('update_contribution', $txgroup, $target_contribution_status, $offset)); | ||
| } | ||
|
|
||
| // finally: render XML and mark the group | ||
| $queue->createItem(new CRM_Sepa_Logic_Queue_Close('create_xml', $txgroup, $target_group_status)); | ||
| if ($is_received_runner) { | ||
| $queue->createItem(new CRM_Sepa_Logic_Queue_Close('create_xml', $txgroup, $target_group_status)); |
There was a problem hiding this comment.
According to the docs, a queue should be filled with CRM_Queue_Task objects when it is supposed to be run using CiviCRM's queue runners. Not sure how much BC there is for the concept being used here, though …
| $queue->createItem(new CRM_Sepa_Logic_Queue_Close('update_contribution', $txgroup, $target_contribution_status, $offset)); | ||
| } | ||
|
|
||
| // finally: render XML and mark the group | ||
| $queue->createItem(new CRM_Sepa_Logic_Queue_Close('create_xml', $txgroup, $target_group_status)); | ||
| if ($is_received_runner) { | ||
| $queue->createItem(new CRM_Sepa_Logic_Queue_Close('create_xml', $txgroup, $target_group_status)); |
There was a problem hiding this comment.
Adding items should only be done when the queue is empty or doesn't exist. This should be checked with $queue->existsQueue() before adding items. Otherwise, loading the CRM_Sepa_Page_MarkGroupReceived page again would load the existing queue and add items for all contributions again, and also starting the runner again (which might stall either runner due to the next item being already claimed by the other).
I agree, but the version as-is has been tested by 3 clients now, and I'm going to merge it the way it is. I created #703 for your suggestions. |
This additional runner will avoid timeout issues when marking large SEPA groups as "received".
systopia-reference: 19531