sethobey
commented
2 years ago Do we need to consider the use case where a merchant adds an additional nexus address that does not exist in TaxJar? Because the Magento plugin sends over all the nexus addresses in requests, I believe that adding additional nexus locations in Magento could be being used by merchants to override the settings in TaxJar and possibly to handle situations where they might have multiple nexus locations in a single state.
I think this approach would also remove all of the additional nexus states that have been manually added.
@dallendalton Yeah, I considered that during development and I'm open to any suggestions as I may have misjudged the impact of that decision.
The core issue is we have no way of knowing what nexus addresses were created by the API vs manually, and maybe that is something that should be addressed here. Although we allow administrators to create and edit addresses in Magento, as you know, those addresses also can't be synced back to TaxJar, so I would take the position that TaxJar should be the source of truth for nexus data. Our M2 extension only supports one nexus address per state/region, so I don't think that last concern of multiple addresses is an issue.
While the sync feature in this PR will remove additional manually configured nexus addresses (as we also do when disabling the extension), if we didn't remove all nexus addresses, whenever a merchant removes a state from their TaxJar account, we will continue collecting tax there until they manually remove the entry.
It could be argued (and rightly so) that there is inherently more liability in under-collecting than over-collecting, so maybe that should guide our decision, but in the case that the merchant has manually configured a nexus address in Magento for a given state/region and later tries to sync their TaxJar nexus addresses, creating a proper entry for that nexus region will also fail if a different address exists in TaxJar for the same state/region since the pre-existing manually created entry will take precedence, so we're still not collecting/calculating accurate tax.
Possible approaches
- Add an additional popup/dialog that informs the merchant that ALL addresses will be removed before syncing. I like this option because it is transparent and simple while making TaxJar the source of truth for nexus address data.
- Add a column (bool) to denote API vs manually created entities, so we can remove only API-created entities when syncing. There is still the potential for some kind of conflict if a manually created address contains a region that conflicts with the API response, so still the possibility of creating incomplete entries (missing region and country), but at least merchants won't have the negative UX of having to re-enter the same information multiple times.
- Perform some kind of diff/filter to only remove nexus addresses with a region/country conflict. Simple to implement, but manually created entries might still be deleted, so just barely a step up from this PR's implementation.
- Scrap this implementation, don't delete any nexus addresses, instead refactor the original broken upsert. I like this choice if the first approach is not an option and deleting all nexus addresses is a blocker. We will still overwrite any manual changes made by merchants to nexus entities for regions included in the API response, but we won't overwrite additional nexus addresses added by the merchant for non-TaxJar regions.
I'm not really sure how to quantify the impact of a change like we do/don't delete nexus addresses when syncing from TaxJar, so maybe for that reason alone the last approach mentioned above is the best/only option. Thoughts?
Context
Bug was discovered where existing nexus addresses are not being overwritten causing additional incomplete entries to be written.
Description
When syncing nexus addresses, all existing addresses will now be removed before writing the addresses from API response.
Performance
N/A
Testing
Versions