Problem/Motivation

Implement an invoice/payment data type plugin for backwards-compatibility with previous version's use case.

Proposed resolution

  1. Write a data type plugin to create an Invoice from the Order.
  2. Write a processor plugin for Payments. This probably has to be a "send" plugin, but maybe ordered first so it saves the Order, then adds the Payments later?
  3. Write a kernel test for each plugin. Invoices with payments are probably way too complex for the xero data test trait and it lacks support for those data types anyway.

Remaining tasks

  • Write a patch.
  • Manually test

API changes

Maybe.

Data model changes

Probably not.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mradcliffe created an issue. See original summary.

mradcliffe’s picture

Title: Implement invoice/payment data type plugin » Implement invoice data type plugin, payment processor plugin
Issue summary: View changes
mradcliffe’s picture

Status: Active » Needs work
StatusFileSize
new26.03 KB

I started work on this, but it's not complete.

I think I can do one invoice plugin, then mutate it into a payment depending on if another plugin is enabled. That will require some API changes to the plugins included in this patch.

It also requires a change to xero which is in the 8.x-1.x branch, but no tag yet.

mradcliffe’s picture

Also found that with commerce_tax enabled it'll be required to add on either the tax plugin or maybe even do tax calculations in the invoice CX data type plugin. Xero will default to using whatever the organization's setup with unless TaxType/TaxAmount is set correctly and/or a special "tax" line item is provided. That latter thing wasn't available when I was working with Drupal 7.

mradcliffe’s picture

Status: Needs work » Needs review
StatusFileSize
new19.04 KB
new43.66 KB

Adds test coverage for the new plugins.

mradcliffe’s picture

Fixes some coding standards caused by copy/paste in PHPStorm.

mradcliffe’s picture

Worked some more on this today.

Found some bugs with invoice lookup and recording invoice payments since Payment type didn't have PaymentID. Updated xero to 1.2-alpha7.

This patch doesn't have any additional test coverage.

jeff veit’s picture

A few notes on this.

Commerce has just introduced a number pattern. The issue was that the order ids are not sequential with no gaps, especially with Commerce Recurring. This is a legal requirement for many invoice numbers.

https://www.drupal.org/node/2730131
https://www.drupal.org/node/3080247

I think this will be used on Commerce receipts and the final purchase pane.

-----

It looks to me like in your code you are only looking at order level adjustments but Commerce allows adjustments at both line level and order level.

I think, if tax is not included in the entered product price, then the tax is included at line level.

Promotions are also handled as adjustments, so they can also be at line level, or on the whole order. Or both.

I think it's possible for tax to be at both, depending on the store locality.

-----

When I started looking at Commerce Xero, I didn't understand enough about how it was working, so I've upgraded an old 7.x Commerce Invoice module. The guts of it are going to be very similar to the invoice code here, so when I have it working, I'll paste that part as a comment. I'd like to be able to switch to Commerce Xero because it's more general.

There are a few big differences:

* No strategies; it's just to send orders as invoices. Some of it's likely to be specific to the client it's for.
* I started using Queue, but switched to Advanced Queue because Commerce Recurring uses Advanced. I don't know their reasoning but I didn't want the end users to have to worry about Queues and Advanced Queues which have separate user interfaces.
* When I'm finished, it's likely to bundle up a few invoices and send them together, to reduce the Xero call count.

-----

mradcliffe’s picture

Thank you for the review, I appreciate it.

The issue with taxes - if the tax calculation doesn't come in as Xero expects/calculates it, then Xero will reject the entire thing. Commerce and Xero never calculate the same way and there have been slight differences :(

The idea here architecturally (there should be a markdown file with more detail) is that this handles the data type plugin and posting to xero, but #2918799: Implement tax processor plugin (or other contrib/custom) could come in with a process plugin and change the data to be what's expected. Having something similar to add other adjustments such as promotions makes sense too.

jeff veit’s picture

Aha about the tax processor! One thing I noticed was the fixed account code of 460 for all order level adjustments, and I was thinking that it needed something to be able to vary the account code, depending on the adjustment (promotion, tax, etc) and the user or buyer; 460 doesn't exist in the Xero Demo chart of accounts. But it makes much more sense if there's a processor for it.

jonathanshaw’s picture

The new processor plugins introduced in this patch specify for their 'types' annotation "commerce_xero_invoice" which is the id of the invoice type plugin. I believe they should specify 'xero_invoice' which is the actual name of the type, specified on the commerce_xero_invoice plugin.

jonathanshaw’s picture

+++ b/src/OrderDataTransformationTrait.php
@@ -0,0 +1,44 @@
+  protected function getDefaultContactValues(OrderInterface $order) {
+    $profile = $order->getBillingProfile();
+    if ($profile->hasField('address')) {

This bit me. Let's not assume that orders always have profiles.

Better
if ($profile && $profile->hasField('address')) {

jonathanshaw’s picture

StatusFileSize
new100.54 KB
new11.27 KB

Fixes #11 and #13

jonathanshaw’s picture

Encoding issues with patch should be fixed here.

jonathanshaw’s picture

StatusFileSize
new48.99 KB

Straight reroll.

Status: Needs review » Needs work

The last submitted patch, 15: 3023730-15.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jonathanshaw’s picture

StatusFileSize
new49.09 KB

Better reroll, I missed a use statement.

jonathanshaw’s picture

Status: Needs work » Needs review
jonathanshaw’s picture

StatusFileSize
new51.15 KB
new7.54 KB

Fix test fails. Also we should use the Xero Query factory. And also need the user module as a kernel test dependency because of using the user data service for tokens in XeroTokenManager.

mattjones86 made their first commit to this issue’s fork.

mradcliffe’s picture

Status: Needs review » Fixed

This was merged in a while ago.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.