Details
Bug
Major
Description
The "Provision Paid Pending Services" automation tasks runs every 5 minutes by default to provision paid pending services. A paid pending service is considered paid if any linked invoices are paid. If there are no linked invoices, it's considered paid.
During order, the service is created first, then the invoice. If there is a delay between service and invoice creation, and the automation task runs between this delay, a service may be provisioned though it has not been paid since the invoice was not yet created.
I'm not sure what the right solution is here. Ideally, I suppose, it'd be great if there were some kind of "lock" put on a service that is released once the invoice has been generated, and then the automation task ignores services that are locked. Or, a delay in provisioning paid pending services to allow sufficient time for an invoice to be generated, though that is arbitrary.
Activity
We can delay newly-created services from being provisioned within a certain time span. We assume 10 seconds is sufficient enough for the vast majority of cases. While the race condition can still be encountered (e.g. plugins that perform slow actions using the Services.add event), this work-around is good enough until much of the business logic surrounding service and invoice creation can be refactored, at which time this race condition would no longer be possible.
Open /app/controllers/cron.php and find:
if ($provision_services) {
// Fetch all paid pending services for this client group
$services = $this->Services->getAllPaidPending($client_group->id);
foreach ($services as $service) {
// Add service module fields
$module_fields = [];
foreach ($service->fields as $field) {
$module_fields[$field->key] = $field->value;
}
replace with:
if ($provision_services) {
// Fetch all paid pending services for this client group
$services = $this->Services->getAllPaidPending($client_group->id);
// Set a 10 second cutoff for provisioning services from the current time
$service_cutoff_time = $this->Date->toTime($this->Date->format('c')) - 10;
foreach ($services as $service) {
// Skip services that were added too recently. They may not have an invoice yet.
// This temporarily works around a race condition whereby this task provisions the service
// before it could be paid because it has no invoice yet
if ($this->Date->toTime($service->date_added . 'Z') >= $service_cutoff_time) {
continue;
}
// Add service module fields
$module_fields = [];
foreach ($service->fields as $field) {
$module_fields[$field->key] = $field->value;
}
Tyson Phillips (Inactive)
added a comment - We can delay newly-created services from being provisioned within a certain time span. We assume 10 seconds is sufficient enough for the vast majority of cases. While the race condition can still be encountered (e.g. plugins that perform slow actions using the Services.add event), this work-around is good enough until much of the business logic surrounding service and invoice creation can be refactored, at which time this race condition would no longer be possible.
Open /app/controllers/cron.php and find:
if ($provision_services) {
// Fetch all paid pending services for this client group
$services = $ this ->Services->getAllPaidPending($client_group->id);
foreach ($services as $service) {
// Add service module fields
$module_fields = [];
foreach ($service->fields as $field) {
$module_fields[$field->key] = $field->value;
}
replace with:
if ($provision_services) {
// Fetch all paid pending services for this client group
$services = $ this ->Services->getAllPaidPending($client_group->id);
// Set a 10 second cutoff for provisioning services from the current time
$service_cutoff_time = $ this ->Date->toTime($ this ->Date->format('c')) - 10;
foreach ($services as $service) {
// Skip services that were added too recently. They may not have an invoice yet.
// This temporarily works around a race condition whereby this task provisions the service
// before it could be paid because it has no invoice yet
if ($ this ->Date->toTime($service->date_added . 'Z') >= $service_cutoff_time) {
continue ;
}
// Add service module fields
$module_fields = [];
foreach ($service->fields as $field) {
$module_fields[$field->key] = $field->value;
}
A good solution to this would be to have the entire process of the service + invoice creation be in a database transaction. The problem, however, is that there already exists transactions that occur in parts of the process when adding/updating services and when adding invoices, and since you can't have nested database transactions, we would have to refactor the process. I already had plans to refactor models in CORE-2430, which is where this could be resolved.
How often does this occur? An order has to be placed at exactly the right time such that between when the service is created and the invoice is created the cron happens to run the provision paid pending services task to fetch those services. While possible, that race condition seems like a shot in the dark, and I'm sure there would be other race conditions with similar undesired effects.
We could delay the provision paid pending services task as you suggested by some arbitrary amount of time, and that may reduce the frequency tremendously, although it will still be possible to encounter the issue. That would probably be alright in the interim.