Skip to content

Commit

Permalink
Remove duplicated check is payment invoiceable
Browse files Browse the repository at this point in the history
Method `InvoiceGenerator->renderInvoiceMailAttachment()` was checking if
user or admin disabled invoicing for account. This is redundant (and not
needed in this case). Few lines later is called `InvoiceGenerator->generate()`
which contains `isPaymentInvoiceable()` check.

Side effect: Sending invoice in case it was generated before user disabled
invoicing. Previous condition would stop this email. But invoice was already
generated (link stored in `$payment->invoice_id`). If invoice exists, it
should be mailed to user.

---

Added handling (where it was missing) for exceptions thrown by
`InvoiceGenerator->renderInvoiceMailAttachment()`.

- `PaymentNotInvoiceableException` is not logged in
  `PaymentInvoiceProviderManager` (and rest of places) because this is
  valid state. Some payments are not invoiceable, some users disabled
  invoicing. This shouldn't be logged as error.
- `InvoiceGenerationException` is always logged.

In most of the cases neither of these exceptions should stop processing.
Eg. email should be sent (about new subscription or recurrent payment).
It just will be sent without attachment.

---

Not catching/logging exceptions in `SendInvoiceCommand`. This command is
always run manually and exception message & trace are accessible (and
helpful) directly in command output.

---

remp/crm#1267
  • Loading branch information
markoph committed Jul 29, 2021
1 parent a2edddb commit 6d9579f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/dataproviders/PaymentInvoiceProviderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,10 @@

interface PaymentInvoiceProviderInterface
{
/**
* @param ActiveRow $payment
* @throws \Crm\InvoicesModule\InvoiceGenerationException
* @throws \Crm\InvoicesModule\PaymentNotInvoiceableException If payment is not invoiceable from valid reasons (eg. outside of invoiceable period).
*/
public function provide(ActiveRow $payment);
}
4 changes: 4 additions & 0 deletions src/dataproviders/PaymentInvoiceProviderManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Crm\PaymentsModule\DataProvider;

use Crm\InvoicesModule\PaymentNotInvoiceableException;
use Nette\Database\Table\ActiveRow;
use Tracy\Debugger;

Expand Down Expand Up @@ -38,6 +39,9 @@ public function getAttachments(ActiveRow $payment): array
foreach ($this->getProviders() as $provider) {
try {
$attachment = $provider->provide($payment);
} catch (PaymentNotInvoiceableException $e) {
// do nothing, no invoice attachment; exception may be raised for valid payments that are not invoiceable
continue;
} catch (\Exception $e) {
Debugger::log($e, Debugger::ERROR);
continue;
Expand Down

0 comments on commit 6d9579f

Please sign in to comment.