Skip to content

[Sales] add getMailer and Email-Queues - #5625

Draft
Hanmac wants to merge 14 commits into
OpenMage:mainfrom
Hanmac:feature/sales_mailer_helper
Draft

[Sales] add getMailer and Email-Queues#5625
Hanmac wants to merge 14 commits into
OpenMage:mainfrom
Hanmac:feature/sales_mailer_helper

Conversation

@Hanmac

@Hanmac Hanmac commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Description (*)

  • adds getMailer helper function to be easier overwritten.
  • also adds Email-Queue for other Sales related E-Mails.

While touching this part, maybe we could discuss other Sales E-Mails related ideas?

  • like sending Invoice and Shipment E-Mails to their E-Mail in their order_address? (instead/in addition)?

Related Pull Requests

  • see OpenMage/magento-lts#<issue_number>

Fixed Issues (if relevant)

  • fixes OpenMage/magento-lts#<issue_number>

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All automated tests passed successfully (all builds are green, SonarCloud checks are not required to merge)

@Hanmac
Hanmac requested a review from sreichel June 2, 2026 15:16
@Hanmac Hanmac added the Component: Sales Relates to Mage_Sales label Jun 2, 2026
@github-actions github-actions Bot added the Component: Payment Relates to Mage_Payment label Jun 3, 2026
@github-actions github-actions Bot added Component: Checkout Relates to Mage_Checkout phpstan labels Jun 3, 2026
@Hanmac

Hanmac commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

@sreichel i'm curious about this part:

$paymentBlock = Mage::helper('payment')->getInfoBlock($order->getPayment())

and their ignored errors:

Parameter #1 $info of method Mage_Payment_Helper_Data::getInfoBlock() expects Mage_Payment_Model_Info, Mage_Sales_Model_Order_Payment|false given.

But shouldn't this cause a big PHP Error crash?
Or do i miss something there?

Edit: I'm dumb, Mage_Sales_Model_Order_Payment extends Mage_Payment_Model_Info, it complains about the possible false

@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
26.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@Hanmac

Hanmac commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@sreichel your comments on this? Is this MR ready? or should there be more/less?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors Sales email sending to centralize mailer/payment-block generation and expands usage of the Email Queue to additional Sales emails (invoice, shipment, credit memo), while also tightening PHPStan typing and generics across Sales/Payment/Checkout.

Changes:

  • Introduces Mage_Sales_Model_Abstract::getMailer() and getPaymentBlockHtml() and migrates Sales email senders to use them.
  • Queues invoice/shipment/credit memo emails via Mage_Core_Model_Email_Queue (similar to existing order email queueing) and adds per-entity email event constants.
  • Improves PHPStan signal by refining docblocks/generics, return types, and removing several baseline ignores.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
app/code/core/Mage/Sales/Model/Resource/Order/Comment/Collection/Abstract.php Narrows collection generic template type to the new Sales comment abstract base.
app/code/core/Mage/Sales/Model/Order/Shipment/Comment.php Moves shipment comments to the shared comment abstract base; adds strict types.
app/code/core/Mage/Sales/Model/Order/Shipment.php Uses shared mailer/payment HTML helper and queues shipment emails with new event constants.
app/code/core/Mage/Sales/Model/Order/Invoice/Comment.php Moves invoice comments to the shared comment abstract base.
app/code/core/Mage/Sales/Model/Order/Invoice.php Uses shared mailer/payment HTML helper and queues invoice emails with new event constants.
app/code/core/Mage/Sales/Model/Order/Creditmemo/Comment.php Moves credit memo comments to the shared comment abstract base.
app/code/core/Mage/Sales/Model/Order/Creditmemo.php Uses shared mailer/payment HTML helper and queues credit memo emails with new event constants.
app/code/core/Mage/Sales/Model/Order/Comment/Abstract.php Adds a new abstract base class to consolidate comment model magic-method PHPDocs.
app/code/core/Mage/Sales/Model/Order.php Uses shared mailer/payment HTML helper and modernizes copy-to checks.
app/code/core/Mage/Sales/Model/Abstract.php Adds getMailer(), getPaymentBlockHtml(), and consolidates _getEmails() implementation.
app/code/core/Mage/Sales/Block/Order/Comments.php Specifies generics for comment collections to satisfy static analysis.
app/code/core/Mage/Payment/Helper/Data.php Tightens return type documentation for getInfoBlock().
app/code/core/Mage/Payment/Block/Info.php Fixes docblock wording and adds missing magic-method annotations.
app/code/core/Mage/Checkout/Helper/Data.php Improves typing for _getEmails() and avoids boolean-ish checks on arrays.
.phpstan.dist.baselines/notEqual.notAllowed.php Removes now-resolved ignore entries related to loose comparisons.
.phpstan.dist.baselines/missingType.iterableValue.php Removes resolved iterable-value-type ignores for updated _getEmails() docs.
.phpstan.dist.baselines/missingType.generics.php Removes resolved generics-related ignores after doc updates.
.phpstan.dist.baselines/foreach.nonIterable.php Removes resolved foreach-type ignores after copy-to checks were tightened.
.phpstan.dist.baselines/empty.notAllowed.php Removes/reduces ignores after replacing empty() with stricter checks in touched areas.
.phpstan.dist.baselines/booleanNot.exprNotBoolean.php Removes resolved ignores after avoiding negation of array
.phpstan.dist.baselines/booleanAnd.leftNotBoolean.php Removes resolved ignores after avoiding && with array
.phpstan.dist.baselines/argument.type.php Removes resolved ignores after tightening types around payment info blocks/emulation.

Comment on lines +504 to +508
$emailQueue = Mage::getModel('core/email_queue');
$emailQueue->setEntityId($this->getId())
->setEntityType(self::ENTITY)
->setEventType(self::EMAIL_EVENT_NAME_NEW_SHIPMENT)
->setIsForceCheck(true);
Comment on lines +583 to +587
$emailQueue = Mage::getModel('core/email_queue');
$emailQueue->setEntityId($this->getId())
->setEntityType(self::ENTITY)
->setEventType(self::EMAIL_EVENT_NAME_UPDATE_SHIPMENT)
->setIsForceCheck(true);
Comment on lines +872 to +876
$emailQueue = Mage::getModel('core/email_queue');
$emailQueue->setEntityId($this->getId())
->setEntityType(self::ENTITY)
->setEventType(self::EMAIL_EVENT_NAME_NEW_INVOICE)
->setIsForceCheck(true);
Comment on lines +956 to +960
$emailQueue = Mage::getModel('core/email_queue');
$emailQueue->setEntityId($this->getId())
->setEntityType(self::ENTITY)
->setEventType(self::EMAIL_EVENT_NAME_UPDATE_INVOICE)
->setIsForceCheck(true);
Comment on lines +844 to +848
$emailQueue = Mage::getModel('core/email_queue');
$emailQueue->setEntityId($this->getId())
->setEntityType(self::ENTITY)
->setEventType(self::EMAIL_EVENT_NAME_NEW_CREDITMEMO)
->setIsForceCheck(true);
Comment on lines +929 to +933
$emailQueue = Mage::getModel('core/email_queue');
$emailQueue->setEntityId($this->getId())
->setEntityType(self::ENTITY)
->setEventType(self::EMAIL_EVENT_NAME_UPDATE_CREDITMEMO)
->setIsForceCheck(true);
* @method $this setParentId(int $value)
* @method $this setStoreId(int $value)
*/
abstract class Mage_Sales_Model_Order_Comment_Abstract extends Mage_Sales_Model_Abstract {}
@Hanmac

Hanmac commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@sreichel your opinion on setIsForceCheck for the Sales E-Mails?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Checkout Relates to Mage_Checkout Component: Payment Relates to Mage_Payment Component: Sales Relates to Mage_Sales phpstan

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants