solidusio / solidus_avatax

Avatax integration with Solidus
BSD 3-Clause "New" or "Revised" License
5 stars 23 forks source link

Add tax before payment step. #2

Closed adammathys closed 9 years ago

adammathys commented 9 years ago

Instead of waiting for the confirm step, add tax before the payment step. This nicely solves the issue with store credits not covering the entire amount. (Not to mention regular payments that would need to be adjusted later on.)

adammathys commented 9 years ago

Original conversation here: https://github.com/adammathys/solidus_avatax/pull/1

athal7 commented 9 years ago

I'm interested to measure the amount of API calls with this strategy vs the previous one in some bonobos test flows, though based on the code alone this looks like a wise approach to me.

adammathys commented 9 years ago

Just did a quick round of testing using this with spree-backend. It correctly adjusts the store credits in all tested scenarios. (Enough credits to cover the full order, no credits, half-half.)

jordan-brough commented 9 years ago

@adammathys super. did you also already test these things? --

In those cases does it correctly update the taxes for the new address and amounts?

athal7 commented 9 years ago

That's fantastic. Also curious about if we are making more calls as compared to the currently deployed bonobos flow since that is how avalara prices.

jordan-brough commented 9 years ago

if we are making more calls as compared to the currently deployed bonobos flow

If we can easily revert this then the best way to test that will be to deploy this to production and measure. @adammathys will we be able to revert this? (Does the solidus upgrade depend on it?)

athal7 commented 9 years ago

i don't think revert is possible or required here. can we simply measure the amount of calls on master vs on this branch with a few different flows?

jordan-brough commented 9 years ago

can we simply measure the amount of calls on master vs on this branch with a few different flows?

not really. what we have to measure is:

  1. how often do people abandon their orders at the "payment" state vs the "confirm" state
  2. how often do people leave the "payment" state and go back to an earlier state and then returning to "payment", vs leaving the "confirm" state and go back to an earlier state and then returning to "confirm".

You can't measure that by clicking around on staging. We can try to analyze our prod data but it would be far easier to just deploy this, compare today's usage to yesterday's usage and roll it back if needed. But if there is no rollback then we don't have that option. It's always unfortunate to deploy something that doesn't have a rollback, are we sure that's the case here?

adammathys commented 9 years ago

@jordan-brough Just tested adding items and changing the shipping address at different steps and it all worked correctly on my locally running version.

Unfortunately, there's no rollback strategy for this change right now. This extension doesn't work with Solidus without this change. (In the case when there's enough store credits to cover the order.) Furthermore, there isn't an easy way to rollback the Solidus upgrade once it goes to production.

As @jordan-brough mentioned, there's also no easy way to measure the difference in calls because it is entirely dependant on customer behaviour. In a happy-path checkout, it is the exact same number of calls as before. Do you guys maybe have some analytics on conversion rates from different checkout steps? If the conversion rate from "payment" is the same as from "confirm" then there won't be any difference in the number of calls.

athal7 commented 9 years ago

@jordan-brough that makes sense, thanks for the explanation.

@zacfrank might be able to provide some insight here on the analytics side

magnusvk commented 9 years ago

Chatted about this with @philbirt and @athal7 offline just now.

Doing some more thinking about where this change here makes a difference in terms of number of Avatax calls:

  1. A user that abandons at payment vs a user that abandons at confirm
  2. A user that restarts checkout from payment vs from confirm Am I missing something?

Based off all this, we would want to go ahead and merge this change and just keep an eye on the number of Avatax calls per order. As long as it stays in the same rough range as before the change, we're good. And if it's much more, it'll be a good excuse to prioritize ripping this out of state machine hooks and making a proper extension point for it.

@jordan-brough is that ok with you?

jordan-brough commented 9 years ago

:+1: sounds great!