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

feat: ORDER_FULLY_REFUNDED #927

Closed
wants to merge 26 commits into from
Closed

feat: ORDER_FULLY_REFUNDED #927

wants to merge 26 commits into from

Conversation

peelar
Copy link
Member

@peelar peelar commented Aug 28, 2023

Featuring:

  • Implementation of the ORDER_FULLY_REFUNDED webhook that refunds the entire AvaTax transaction.

Rel: #1166

Note:
Currently, no partial refunds are supported.

@peelar peelar self-assigned this Aug 28, 2023
@changeset-bot
Copy link

changeset-bot bot commented Aug 28, 2023

🦋 Changeset detected

Latest commit: 11d530e

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Aug 28, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
saleor-app-avatax ❌ Failed (Inspect) Feb 15, 2024 10:59am
saleor-app-taxes ✅ Ready (Inspect) Visit Preview Feb 15, 2024 10:59am
11 Ignored Deployments
Name Status Preview Comments Updated (UTC)
saleor-app-cms ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-cms-v2 ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-crm ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-data-importer ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-emails-and-messages ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-invoices ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-klaviyo ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-products-feed ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-search ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-segment ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am
saleor-app-slack ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:59am

@lkostrowski
Copy link
Member

plase coop with @korycins and write the RFC that will cover missing APIs for refunds

Copy link
Member

@witoszekdev witoszekdev left a comment

Choose a reason for hiding this comment

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

We have split Avatax and Taxjar into separate apps, the Taxes app is now deprecated. Please add the changes to apps/avatax

Comment on lines 16 to 19
export function resolveAvataxTransactionLineNumber(line: OrderLineFragment) {
return line.id.slice(-50);
}

Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Why not use Order.token instead of shortening the ID?

Here's the comparison in length between token (UUID from Saleor database) and ID (base64 of Order:)

❯ echo "951feaa9-d613-4e0c-bd6a-373134ec3dc1" | wc -c                       
      37
❯ echo "T3JkZXI6OTUxZmVhYTktZDYxMy00ZTBjLWJkNmEtMzczMTM0ZWMzZGMx" | wc -c                                    
      57

In the schema it's marked as deprecated, but since we still keep on finding use-cases for shorter IDs we might leave it (conversation: https://saleorcommerce.slack.com/archives/C0352SN0W3U/p1706698862183379)

Refactor AvataxDocumentCodeResolver to use a function instead of a class

Refactor avatax-customer-code-resolver.ts to be functions instead of class

Refactor AvataxCalculationDateResolver to use functions instead of class

Refactor AvataxOrderConfirmedPayloadLinesTransformer to use functions instead of class

Refactor AvataxCalculateTaxesPayloadLinesTransformer to a function

Refactor classes to functions; group resolvers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants