changeStatus() forces PDF attachment even when "Attach PDF" is disabled on the order status

[Nidzo]

Phoca Cart version: 6.1.8 (also present on current master)
Joomla version: 6.1.3

When calling PhocacartOrderStatus::changeStatus() with fewer than 11 arguments (i.e. relying on the defaults for the trailing parameters, as any external integration naturally would), the notification email always gets a PDF attached — even when "Attach PDF" is explicitly set to No on that order status. In our case this caused a fatal error during email generation, so the customer never received the status-change notification at all.

Root cause

In admin/libraries/phocacart/order/status.php:

Code: Select all

public static function changeStatus($orderId, $statusId, $orderToken = '',
    $notifyUser = 99, $notifyOthers = 99, $emailSend = 99,
    $stockMovements = '99', $changeUserGroup = '99', $changePointsNeeded = '99', $changePointsReceived = '99', $emailSendFormat = '99'
) {
    ...
    if ($emailSendFormat === 99) {
        $emailSendFormat = $status['email_send_format'];
    }
    ...
    $notificationResult = self::sendOrderEmail(..., !!$emailSendFormat);

$emailSendFormat's default value is the string '99', but the fallback check compares it with strict === 99 (int). A string is never strictly equal to an int, so this check never passes for any caller that doesn't explicitly supply all 11 arguments. $emailSendFormat stays the string '99', and !!'99' evaluates to true — so $attachPDF ends up true regardless of what's actually configured on the order status.

Impact

Any external caller using the short form — changeStatus($orderId, $statusId, $orderToken), which is the natural way to call it from a payment gateway webhook or a custom integration — is affected. In our environment this surfaced as a fatal error further downstream (Pdf::renderPdf() → PhocaPDFRender::renderPDF('', ...) → the Phoca PDF phocacart plugin's onBeforeDisplayPDFPhocaCart(), which calls $document->getBuffer() on that hardcoded empty string), which aborted changeStatus() entirely and silently skipped the customer notification email.

Suggested fix

Either change the default to the int 99:

Code: Select all

$emailSendFormat = 99

or change the comparison to loose/type-coerced, consistent with how the other trailing parameters ($stockMovements, $changeUserGroup, etc.) are already checked with == rather than ===.

Workaround (for anyone hitting this in the meantime): explicitly pass all 11 arguments, with 0 (or your desired value) for $emailSendFormat, to bypass the broken default.