Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added sending, updating, and deleting order and refund transactions #9

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from

Conversation

jmauzyk
Copy link

@jmauzyk jmauzyk commented Mar 12, 2021

Would love to get our site switched over to TaxJar from a different tax service, so I worked on building out some of the features we need. This PR incorporates the following:

  • Adjuster requires line items to prevent unnecessary API calls
  • Tax amounts stored at line item level (except shipping-related taxes)
  • Send order transaction on order paid (via queue)
  • TaxJar actions available on order edit pages, including send/update, refund, and delete
  • Refund records added to DB for re-sending refund transactions and tracking refunded line items

I made some assumptions as far as the refund creation process and ran with it, hopefully it works with the vision you all have for this plugin. Thanks!

@lukeholder
Copy link
Member

Thanks for all the work on this. I can see the need for this, but it will take a little time to review.

I can also see there are parts that should probably be extracted into core commerce functionality like line item refunds.

Leaving it open while I review.

@jmauzyk
Copy link
Author

jmauzyk commented Jul 1, 2021

@lukeholder We've got a contract with another sales tax service expiring at the end of July at which point we'll be switching to TaxJar. We can use my forked version of the plugin if need be, but would obviously prefer to use an official version. Not sure if that's feasible as I'm sure you all are busy as ever, but if there's anything I can do to help please let me know!

@codyjames
Copy link

I would love to see this merged. Any chance we'll see that happen soon?

@jmauzyk jmauzyk requested a review from a team as a code owner July 27, 2021 03:39
@codyjames
Copy link

@jmauzyk How you feeling about this fork? Is it something others could use?

@jmauzyk
Copy link
Author

jmauzyk commented Aug 13, 2021

@codyjames It's been working well for us for a few weeks now, so I don't anticipate any issues for others who want to use it. One quick note though: My changes added a refund modal that creates a TaxJar refund, tracks refunded quantities, and creates a refund transaction for the appropriate amount. Data for these refunds are stored in the taxjar_refund_lineitems and taxjar_refunds tables. Depending on how refunds end up getting implemented by Craft, you may have to perform some database migrations in the future.

@codyjames
Copy link

@jmauzyk That's great to hear. Thanks so much for working on this and saving others some time!

@codyjames
Copy link

@jmauzyk Quick question! Does the tax refund stuff handle partial refunds?

Again, thanks for your work on this!!!

@codyjames
Copy link

@jmauzyk Any chance you'd be up for some consulting (a call or two) on a project that I'm doing that uses this? Would happily pay you whatever your hourly rate is for this. If not, no worries!

{
return [
'amount' => $order->getItemSubtotal() + $order->getTotalDiscount() + $order->getTotalShippingCost(),
'shipping' => $order->getTotalShippingCost(),

Choose a reason for hiding this comment

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

I want to note that doing this here doesn't account for other adjusters. We have a shipping adjuster in our custom module — but this seems to run before that, and so this thinks shipping is $0 — causing us to not pay the correct amount of tax.

Choose a reason for hiding this comment

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

Actually, I may have figured this out. So sorry for spamming this PR. Replacing the default shipping adjuster rather than just adding a new shipping adjuster did the trick.

Copy link
Author

Choose a reason for hiding this comment

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

Hey @codyjames, sorry for the slow response! Yes, the tax refund handles partial refunds. I'm happy to jump on a call if you have lingering questions regarding this fork. If so, let me know how to best get in touch and we can set up a time.

Choose a reason for hiding this comment

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

@jmauzyk Yes! A call would actually be very helpful. Want to send me an email at [email protected]?

@codyjames
Copy link

@lukeholder Any chance this will make it into the core plugin sometime soon? Really wanting to migrate to Craft 4 but this is holding me back. These features are super important for folks using TaxJar with Commerce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants