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.

Sign up for GitHub

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

Jump to bottom

[Backport] Full Tax Summary display wrong numbers #21252

Closed
wants to merge 1 commit into from
Closed

[Backport] Full Tax Summary display wrong numbers #21252

wants to merge 1 commit into from

Conversation

mage2pratik
Copy link
Contributor

@mage2pratik mage2pratik commented Feb 15, 2019 •
edited
Loading

#20682

Description (*)

The full tax summary is displaying total tax, instead of showing individual tax value.

Fixed Issues (if relevant)

  1. Magento 2.3 Shopping Cart Taxes Missing Calc Line #19701: Magento 2.3 Shopping Cart Taxes Missing Calc Line
  2. Full Tax Summary display wrong numbers. #11358: Full Tax Summary display wrong numbers.

Manual testing scenarios (*)

  1. Manual Testing

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-engcom-team
Copy link
Contributor

Hi @mage2pratik. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.2-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@VladimirZaets VladimirZaets self-assigned this Feb 15, 2019
@magento-engcom-team
Copy link
Contributor

Hi @VladimirZaets, thank you for the review.
ENGCOM-4294 has been created to process this Pull Request

sivaschenko
sivaschenko requested changes Feb 17, 2019
View reviewed changes
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Hi @mage2pratik , thanks for the backport. Please see my code review notes. Please rearrange the branch of this pull request by cherry-picking commits from original pull request, to preserve authorship. This method also helps to avoid any typos/mistakes.

app/code/Magento/Tax/view/frontend/web/js/view/checkout/summary/tax.js Outdated
'use strict';

var isTaxDisplayedInGrandTotal = window.checkoutConfig.includeTaxInGrandTotal,
isFullTaxSummaryDisplayed = window.checkoutConfig.isFullTaxSummaryDisplayed,
isZeroTaxDisplayed = window.checkoutConfig.isZeroTaxDisplayed;
totalPercentage = 0,
Copy link
Member

Choose a reason for hiding this comment

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

Comma-separated variables declaration continues after a semicolon, it should be replaced with a comma.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sivaschenko I've updated code, next time I will do with cherry-pick. Thanks 👍

app/code/Magento/Tax/view/frontend/web/js/view/checkout/summary/tax.js
* @return {*|String}
*/
getTaxAmount: function (parent, percentage) {
var totalPercentage = 0;
Copy link
Member

Choose a reason for hiding this comment

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

totalPercentage variable is declared twice in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sivaschenko I've updated code. Thanks 👍

@sivaschenko sivaschenko self-assigned this Feb 17, 2019
@magento-engcom-team
Copy link
Contributor

Hi @sivaschenko, thank you for the review.
ENGCOM-4294 has been created to process this Pull Request

sivaschenko
sivaschenko requested changes Feb 18, 2019
View reviewed changes
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Hi @mage2pratik , thanks for updates, however, the code still does not match the original PR. Please see the Travis build results.

@mage2pratik
Copy link
Contributor Author

Hi @sivaschenko, I've updated code.

@amol2jcommerce
Copy link
Contributor

amol2jcommerce commented Feb 20, 2019 via email

@nilesh2jcommerce
Copy link

nilesh2jcommerce commented Feb 20, 2019 via email

@ajay2jcommerce
Copy link

ajay2jcommerce commented Feb 20, 2019 via email

@nainesh2jcommerce
Copy link
Contributor

nainesh2jcommerce commented Feb 20, 2019 via email

@priti2jcommerce
Copy link
Contributor

priti2jcommerce commented Feb 20, 2019 via email

@dipti2jcommerce
Copy link

dipti2jcommerce commented Feb 20, 2019 via email

@prakash2jcommerce
Copy link

prakash2jcommerce commented Feb 20, 2019 via email

@nilesh2jcommerce
Copy link

nilesh2jcommerce commented Feb 20, 2019 via email

@amol2jcommerce
Copy link
Contributor

amol2jcommerce commented Feb 20, 2019 via email

@dipti2jcommerce
Copy link

dipti2jcommerce commented Feb 20, 2019 via email

@priti2jcommerce
Copy link
Contributor

priti2jcommerce commented Feb 20, 2019 via email

@prakash2jcommerce
Copy link

prakash2jcommerce commented Feb 20, 2019 via email

@nainesh2jcommerce
Copy link
Contributor

nainesh2jcommerce commented Feb 20, 2019 via email

@nilesh2jcommerce
Copy link

nilesh2jcommerce commented Feb 20, 2019 via email

@dipti2jcommerce
Copy link

dipti2jcommerce commented Feb 20, 2019 via email

@priti2jcommerce
Copy link
Contributor

priti2jcommerce commented Feb 20, 2019 via email

@dipti2jcommerce
Copy link

dipti2jcommerce commented Feb 20, 2019 via email

@nilesh2jcommerce
Copy link

nilesh2jcommerce commented Feb 20, 2019 via email

@amol2jcommerce
Copy link
Contributor

amol2jcommerce commented Feb 20, 2019 via email

@ajay2jcommerce
Copy link

ajay2jcommerce commented Feb 20, 2019 via email

@priti2jcommerce
Copy link
Contributor

priti2jcommerce commented Feb 20, 2019 via email

@dipti2jcommerce
Copy link

dipti2jcommerce commented Feb 20, 2019 via email

@amol2jcommerce
Copy link
Contributor

amol2jcommerce commented Feb 20, 2019 via email

@prakash2jcommerce
Copy link

prakash2jcommerce commented Feb 20, 2019 via email

@priti2jcommerce
Copy link
Contributor

priti2jcommerce commented Feb 20, 2019 via email

@nilesh2jcommerce
Copy link

nilesh2jcommerce commented Feb 20, 2019 via email

@ajay2jcommerce
Copy link

ajay2jcommerce commented Feb 20, 2019 via email

@amol2jcommerce
Copy link
Contributor

amol2jcommerce commented Feb 20, 2019 via email

@priti2jcommerce
Copy link
Contributor

priti2jcommerce commented Feb 20, 2019 via email

@ajay2jcommerce
Copy link

ajay2jcommerce commented Feb 20, 2019 via email

@sivaschenko
Copy link
Member

Hi @mage2pratik thanks for the update, do you know why all these comments where added to this pull request?

@mage2pratik
Copy link
Contributor Author

Hi @sivaschenko. It seems because of github system glitch. I don't have exact idea what is going on surprised...!!!

@magento-engcom-team
Copy link
Contributor

Hi @sivaschenko, thank you for the review.
ENGCOM-4294 has been created to process this Pull Request

sivaschenko
sivaschenko requested changes Feb 27, 2019
View reviewed changes
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

Hi @mage2pratik we are getting a JS error on the shopping cart page, can you please check it?

image

@sidolov
Copy link
Contributor

sidolov commented Mar 14, 2019

@mage2pratik , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

@sidolov sidolov closed this Mar 14, 2019
@ghost
Copy link

ghost commented Mar 14, 2019

Hi @mage2pratik, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@mage2pratik mage2pratik deleted the back-port-pull-20682 branch March 26, 2021 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sivaschenko sivaschenko sivaschenko requested changes

@VladimirZaets VladimirZaets VladimirZaets approved these changes

Assignees

@sivaschenko sivaschenko

@VladimirZaets VladimirZaets

Labels
Area: Frontend Component: Tax Partner: Krish TechnoLabs partners-contribution Pull Request is created by Magento Partner Progress: needs update Release Line: 2.2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.