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

Add total_duties to customer account order page #1368

Merged

Conversation

minh-tran-shopify
Copy link
Contributor

@minh-tran-shopify minh-tran-shopify commented Feb 16, 2022

Why are these changes introduced?

  • Duties is a new feature for merchants selling internationally. We expose the amount of duties on an order view through the total_duties attribute. This PR updates the Dawn theme to display the duties total on the customer account order page.

What approach did you take?

  • Since total_duties is an attribute on order object, I used this attribute to display the total duties. In cases where duties are not applicable (merchants do not collect duties, or the order is domestic), the duties line is not rendered.
Here's an example of duties being rendered Screen Shot 2022-02-16 at 3 56 26 PM

Testing steps/scenarios
Enable the duties and taxes incoterm feature. Set up shipping to an international country. In taxes and duties setting page, collect duties in that international country. Sign up / Log into an account. Checkout.

  • An international order in a country that the store has enabled duties for, and the order total is above a country's de minimis (e.g: > $800 for orders in the US) -> Duties should show up
  • An international order in a country that the store has enabled duties for, but the order total is below a country's de minimis (e.g: < $800 for orders in the US) -> Duties should show up but the value is $0
  • A domestic order -> Duties should not show up as it is not applicable

Demo links
Please include a link to a demo store that includes preconfigured sections and settings to allow reviewers to easily test the features you are working on.

Checklist

@minh-tran-shopify minh-tran-shopify changed the title WIP Add total_duties to customer account order page Add total_duties to customer account order page Feb 17, 2022
@minh-tran-shopify minh-tran-shopify marked this pull request as ready for review February 17, 2022 00:07
@minh-tran-shopify minh-tran-shopify force-pushed the add-total_duties-to-customer-account-order-page branch from 3c421e1 to 8ddab10 Compare February 17, 2022 00:20
KaichenWang
KaichenWang previously approved these changes Feb 17, 2022
Copy link
Contributor

@KaichenWang KaichenWang left a comment

Choose a reason for hiding this comment

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

Looks good. Just awaiting translations

jstncarvalho
jstncarvalho previously approved these changes Feb 18, 2022
Copy link

@jstncarvalho jstncarvalho left a comment

Choose a reason for hiding this comment

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

🚀

martinamarien
martinamarien previously approved these changes Feb 18, 2022
Copy link
Contributor

@martinamarien martinamarien left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks Minh.
I'll approve again once all translations come back.

@ghost ghost added the cla-needed label Feb 18, 2022
Copy link
Contributor

@melissaperreault melissaperreault left a comment

Choose a reason for hiding this comment

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

Looks good!
Unrelated, At checkout, I was wondering why we see Taxes prior to Duties, it was interesting to witness. (Video reference)

Copy link
Contributor

@martinamarien martinamarien left a comment

Choose a reason for hiding this comment

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

Hey Minh, looks like a bunch of files were modified by an app on your store and sent back as commits on your branch. Would you mind reverting those two commits from your shop? Once that's done, I'll re-approve. Thanks!

@minh-tran-shopify minh-tran-shopify force-pushed the add-total_duties-to-customer-account-order-page branch from 40d44c7 to d18a554 Compare February 22, 2022 17:52
@ghost ghost removed the cla-needed label Feb 22, 2022
@minh-tran-shopify
Copy link
Contributor Author

Hey Minh, looks like a bunch of files were modified by an app on your store and sent back as commits on your branch. Would you mind reverting those two commits from your shop? Once that's done, I'll re-approve. Thanks!

@martinamarien Hey Martina, thanks for letting me know. I have removed the 2 commits from the branch

@melissaperreault melissaperreault self-requested a review February 22, 2022 20:36
Copy link
Contributor

@melissaperreault melissaperreault left a comment

Choose a reason for hiding this comment

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

🎉

Copy link
Contributor

@martinamarien martinamarien left a comment

Choose a reason for hiding this comment

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

Just re-tested everything. Seems good to me.
Thanks for adding this Minh.

@minh-tran-shopify minh-tran-shopify merged commit 988bb22 into main Feb 24, 2022
@minh-tran-shopify minh-tran-shopify deleted the add-total_duties-to-customer-account-order-page branch February 24, 2022 00:14
phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants