13851 Added improvements for credit memo total fields #27343

sergiy-v · 2020-03-18T18:27:18Z

Description

Added improvements for credit memo (new page) total fields.

Related Pull Requests

#27492

Fixed Issues

#13851

Manual testing scenarios

See #13851 issue description for details.

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 are green)

m2-assistant · 2020-03-18T18:27:21Z

Hi @sergiy-v. 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 give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

sergiy-v · 2020-03-18T20:53:59Z

work in progress

lbajsarowicz

Please keep backwards compatible constructors :-)

lbajsarowicz · 2020-03-18T21:44:19Z

app/code/Magento/Sales/Model/Order/CreditmemoFactory.php

        \Magento\Framework\Serialize\Serializer\Json $serializer = null
    ) {
        $this->convertor = $convertOrderFactory->create();
        $this->taxConfig = $taxConfig;
+        $this->localeFormat = $localeFormat;
        $this->serializer = $serializer ?: \Magento\Framework\App\ObjectManager::getInstance()->get(


Remove backward compatible class loader and add your dependency as last.

The changes has been added, thank you.

lbajsarowicz

Only import the Json class and fix the Static Tests failure.

lbajsarowicz · 2020-03-19T17:54:37Z

app/code/Magento/Sales/Model/Order/CreditmemoFactory.php

    ) {
        $this->convertor = $convertOrderFactory->create();
        $this->taxConfig = $taxConfig;
-        $this->serializer = $serializer ?: \Magento\Framework\App\ObjectManager::getInstance()->get(
+        $this->serializer = $serializer ?: ObjectManager::getInstance()->get(
            \Magento\Framework\Serialize\Serializer\Json::class


You can also import that class. I recommend you to use alias JsonSerializer to be explicit what it does.

The changes has been added, thank you for recommendation.

sergiy-v · 2020-03-20T10:27:07Z

I believe the tests errors not related to the changes, @lbajsarowicz could you please check it.

Thank you.

lbajsarowicz · 2020-03-20T10:31:48Z

@magento run all tests
Let's restart them.

sergiy-v · 2020-03-24T18:39:17Z

@lbajsarowicz please check the changes once you will have a chance.

Thank you.

eduard13

The failing tests aren't related with this PR. I've linked the PR that should fix the failing Unit Test.
Thank you.

magento-engcom-team · 2020-03-30T10:04:42Z

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

eduard13 · 2020-03-31T07:31:42Z

@magento run all tests

engcom-Alfa · 2020-04-07T11:19:52Z

✔️ QA Passed

Manual testing scenario:

Create a Credit Memo for an order;
Enter an amount with decimals in the field Adjustment Refund;
Create an offline payment (finish credit memo)

Before:

After:

slavvka · 2020-04-09T18:47:22Z

@magento run all tests

sdzhepa · 2020-04-09T21:08:55Z

@engcom-Alfa

After reviewing with Product managers was decided to add the PR to the next 2.4.0 Magento release.
But this functionality affect's order management process and financial documents
So it was decided to do extended testing for this PR additionally to be sure that we do not brake or change any functionality around it.

I would request to check the next scenarios:

full credit memo
partial credit memo
cases with multiple currencies configured on the store(e.g. default/base and admin but different on a couple of websites/store-views)
Localization settings, like digit delimiter, is "," or "." (12,01 and 12.01)
reports about the credit memo shown in the email (numbers are correct?)
reports about credit memo shown on Storefront(My account) as Comments related to order
advanced ability "Notify Customer by email" from Admin Panel
Orders with taxes
Orders with discounts/reward points/store credits etc
...
any additional user flows and cases that I could miss in this list

As a QA report should be:

Short description of what was tested
If need more details about precondition or additional configuration
Confirmation that bugs if fixed and/or other functionality works as previously(expected)

…7343

m2-assistant · 2020-04-10T06:12:15Z

Hi @sergiy-v, 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.

sdzhepa · 2020-04-10T18:14:19Z

Hello @engcom-Alfa

I know that you have done additional and advanced testing for this PR before the merge.
We discussed it in today internal sync.
Could you please add here a detailed QA report(what was tested) to be transparent with all participants?

engcom-Alfa · 2020-04-10T20:58:13Z

Hello @sdzhepa

I checked the PR and everything works well.
Fixed the issue described in #13851 and other functionality works as previously (expected).

What was tested:

1. The issue described in #13851

Create a Credit Memo for an order
Enter an amount with decimals (by example 1.23) in the fields Adjustment Refund, Adjustment Fee
Create an offline payment (finish credit memo)

Before: ✖️

After: ✔️

2. Also checked additional cases:

[ ✔️ ] full credit memo;

[ ✔️ ] partial credit memo;

[ ✔️ ] credit memo on other locale (German locale in my case) with digit delimiter ",";

[ ✔️ ] multiple currencies configured on the store (in my case $ and €);

[ ✔️ ] credit memo with taxes (including and excluding);

[ ✔️ ] credit memo on Storefront(My account) with related to order comments

[ ✔️ ] ☑️ Email Copy of Credit Memo

[ ✔️ ] Print PDF credit memo

ghost added the Progress: pending review label Mar 18, 2020

magento-engcom-team added Area: Frontend Component: Sales Release Line: 2.4 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Mar 18, 2020

sergiy-v linked an issue Mar 18, 2020 that may be closed by this pull request

Credit doesn't recognize amount after comma #13851

Closed

lbajsarowicz suggested changes Mar 18, 2020

View reviewed changes

ghost assigned lbajsarowicz Mar 18, 2020

ghost added Progress: needs update and removed Progress: pending review labels Mar 18, 2020

sergiy-v force-pushed the 13851-new-credit-memo-total-fields-improvements branch from 4080a6c to c939a4b Compare March 19, 2020 07:02

lbajsarowicz suggested changes Mar 19, 2020

View reviewed changes

sergiy-v force-pushed the 13851-new-credit-memo-total-fields-improvements branch from 9f35810 to 60ce499 Compare March 20, 2020 06:10

sergiy-v requested a review from lbajsarowicz March 20, 2020 10:27

Added improvements for credit memo (new page) total fields

4046760

sergiy-v force-pushed the 13851-new-credit-memo-total-fields-improvements branch from 60ce499 to 4046760 Compare March 30, 2020 06:20

eduard13 approved these changes Mar 30, 2020

View reviewed changes

ghost assigned eduard13 Mar 30, 2020

ghost added Progress: ready for testing and removed Progress: needs update labels Mar 30, 2020

eduard13 added Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: bug fix labels Mar 30, 2020

engcom-Alfa self-assigned this Apr 7, 2020

ghost added Progress: testing in progress and removed Progress: ready for testing labels Apr 7, 2020

ghost added Progress: extended testing and removed Progress: testing in progress labels Apr 7, 2020

engcom-Echo self-assigned this Apr 8, 2020

ghost added Progress: accept and removed Progress: extended testing labels Apr 9, 2020

VladimirZaets added Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Severity: S1 Affects critical data or functionality and forces users to employ a workaround. labels Apr 9, 2020

VladimirZaets added this to the 2.4.0 milestone Apr 9, 2020

magento-engcom-team pushed a commit that referenced this pull request Apr 10, 2020

ENGCOM-7223: 13851 Added improvements for credit memo total fields #2…

6f7e07f

…7343

magento-engcom-team merged commit 2b605aa into magento:2.4-develop Apr 10, 2020

magento-engcom-team mentioned this pull request May 1, 2020

Credit doesn't recognize amount after comma #13851

Closed

sdzhepa mentioned this pull request May 9, 2022

tyutyu sdzhepa/magento2#5

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

13851 Added improvements for credit memo total fields #27343

13851 Added improvements for credit memo total fields #27343

sergiy-v commented Mar 18, 2020 •

edited by eduard13

Loading

m2-assistant bot commented Mar 18, 2020

sergiy-v commented Mar 18, 2020

lbajsarowicz left a comment

lbajsarowicz Mar 18, 2020

sergiy-v Mar 20, 2020

lbajsarowicz left a comment

lbajsarowicz Mar 19, 2020

sergiy-v Mar 20, 2020

sergiy-v commented Mar 20, 2020

lbajsarowicz commented Mar 20, 2020

sergiy-v commented Mar 24, 2020

eduard13 left a comment

magento-engcom-team commented Mar 30, 2020

eduard13 commented Mar 31, 2020

engcom-Alfa commented Apr 7, 2020

slavvka commented Apr 9, 2020

sdzhepa commented Apr 9, 2020

m2-assistant bot commented Apr 10, 2020

sdzhepa commented Apr 10, 2020 •

edited

Loading

engcom-Alfa commented Apr 10, 2020

13851 Added improvements for credit memo total fields #27343

13851 Added improvements for credit memo total fields #27343

Conversation

sergiy-v commented Mar 18, 2020 • edited by eduard13 Loading

Description

Related Pull Requests

Fixed Issues

Manual testing scenarios

Contribution checklist

m2-assistant bot commented Mar 18, 2020

sergiy-v commented Mar 18, 2020

lbajsarowicz left a comment

Choose a reason for hiding this comment

lbajsarowicz Mar 18, 2020

Choose a reason for hiding this comment

sergiy-v Mar 20, 2020

Choose a reason for hiding this comment

lbajsarowicz left a comment

Choose a reason for hiding this comment

lbajsarowicz Mar 19, 2020

Choose a reason for hiding this comment

sergiy-v Mar 20, 2020

Choose a reason for hiding this comment

sergiy-v commented Mar 20, 2020

lbajsarowicz commented Mar 20, 2020

sergiy-v commented Mar 24, 2020

eduard13 left a comment

Choose a reason for hiding this comment

magento-engcom-team commented Mar 30, 2020

eduard13 commented Mar 31, 2020

engcom-Alfa commented Apr 7, 2020

slavvka commented Apr 9, 2020

sdzhepa commented Apr 9, 2020

m2-assistant bot commented Apr 10, 2020

sdzhepa commented Apr 10, 2020 • edited Loading

engcom-Alfa commented Apr 10, 2020

What was tested:

sergiy-v commented Mar 18, 2020 •

edited by eduard13

Loading

sdzhepa commented Apr 10, 2020 •

edited

Loading