search

SuperGoodSoft / solidus_taxjar

Support for using TaxJar to handle tax calculations in Solidus
BSD 3-Clause "New" or "Revised" License
12 stars 13 forks source link

Add support for the new event system in Solidus 4.x #238

Closed forkata closed 1 year ago

forkata commented 1 year ago

What is the goal of this PR?

We want to support Solidus 4.x in time for launching version 1.0 of Solidus TaxJar! Prior to this the extension was using the compatibility layer which allowed us to not have to migrate to the new Omnes API. That however was removed in Solidus 4.x.

This refactor tackles compatibility with the new Omnes event bus by moving our legacy subscriber into a separate module and introducing a new Omnes version!

This change required moving some things around and introducing a new initializer file. We are not set on the naming for that and look for some input on wether having both an omnes.rb and solidus_taxjar_legacy_events.rb is the best way to do this.

This change depends on #226 being merged first, as that refactor allows us to reduce the duplication of having two subscribers.

Huge thanks to @camerond594 and @benjaminwil for their help with working through this 🎉

How do you manually test these changes? (if applicable)

  1. Do a thing
    • [ ] Assert a result

Merge Checklist

Screenshots

forkata commented 1 year ago

Cherry-picked the update to the Ruby version from https://github.com/SuperGoodSoft/solidus_taxjar/pull/239 as this was failing the required CI job 😩

forkata commented 1 year ago

Based on our testing here, it appears that sometimes the order we're creating for the refund feature spec doesn't get taxes on the line items (only the shipment) which results in the spec failing during the reimbursement due to the difference in the tax amount.

We should consider pending that feature spec so we can unblock the merging of this and the dependent PR at least for now, if we are not concerned about this being a legitimate failure.