taxjar / taxjar-magento2-extension

Magento 2 Sales Tax Extension by TaxJar
http://www.taxjar.com/guides/integrations/magento2/
Open Software License 3.0
22 stars 30 forks source link

fix: Invalid transaction backfill CLI command date parameters #240

Closed lbajsarowicz closed 2 years ago

lbajsarowicz commented 2 years ago

Context

When you call bin/magento taxjar:transactions:sync -f 2022/02/10 the date argument is ignored (as it expects from and to, while from_date and to_date are provided)

https://github.com/taxjar/taxjar-magento2-extension/blob/2346140c538a5151441f07335e1e245a6785f433/Observer/BackfillTransactions.php#L163-L180

TaxJar developers forgot to test that command after "refactoring".

Description

This Pull Request changes the property name to make source & target date supported

Performance

No change to the performance

Testing

  1. Call bin/magento taxjar:transactions:sync -f 2022/02/10
  2. Expect all the transactions since 2022/02/10 to be synchronized

Versions

sethobey commented 2 years ago

Hey @lbajsarowicz, thanks for the contribution!

sethobey commented 2 years ago
image

1st output is current develop branch 2nd output is this PR

sethobey commented 2 years ago

@dallendalton & @jordan-k-johnson - On second thought, maybe this bug should be addressed downstream in the BackfillTransactions observer class by updating the lookup key (e.g. from to from_date) instead so that we don't change the observer object's params?

These observer date params have existed for awhile, and as such, some customers may have come to rely on them.

lbajsarowicz commented 2 years ago

@sethobey I agree with this thought. Let me know if you'd like to change that the other way.

sethobey commented 2 years ago

@sethobey I agree with this thought. Let me know if you'd like to change that the other way.

@lbajsarowicz - Yes, that change would be great! We will include this fix in our next release, and thank you again for contributing.

lbajsarowicz commented 2 years ago

@sethobey As per our discussion, the new version is committed.