Tierprice can t save float percentage value 18651 #19584

novikor · 2018-12-05T14:43:18Z

Description (*)

See #18651

Fixed Issues (if relevant)

Tierprice can't save float percentage value #18651: Tierprice can't save float percentage value

Manual testing scenarios (*)

Steps to reproduce (*)

Add new product
Add new tierprice with "discount" value "12.5"
Save the product

Expected result (*)

Saving the tierprice as you set it "12.5"

Actual result (*)

It's converted to INT then save it

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 · 2018-12-05T14:43:26Z

Hi @novikor. 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.3-develop instance - deploy vanilla Magento instance

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

novikor · 2018-12-05T14:43:52Z

@magento-engcom-team give me test instance

magento-engcom-team · 2018-12-05T14:44:03Z

Hi @novikor. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2018-12-05T14:44:33Z

Hi @novikor, here is your new Magento instance.
Admin access: https://pr-19584.instances.magento-community.engineering/admin
Login: admin Password: 123123q

orlangur · 2018-12-05T14:47:11Z

app/code/Magento/Catalog/Model/Product/Attribute/Backend/TierPrice/SaveHandler.php

     */
-    private function getPercentage(array $priceRow): ?int
+    private function getPercentage(array $priceRow): ?float


As mentioned in previous incarnation of this PR - type hinting needs to be removed so that any of int/float is preserved. Type casting within function should be done with + 0 or * 1.

Also, please squash all changes into a single commit.

@orlangur , okay, but is there a real reason to avoid float type hint?
As I saw, there were errors in tests because of removed isset statement

Please review.

a real reason to avoid float type hint?

Just that there is no reason to convert int.

magento-engcom-team · 2018-12-05T15:14:41Z

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

orlangur

QA report

Problem: Incorrect percentage value of discount on product page.
Manual testing scenario:

Add new product
Add new tierprice with "discount" value "12.5"
Save the product
Go to Product Detail Page
 Message "Buy 500 for $87.50 each and save 13%" is shown instead of "Buy 500 for $87.50 each and save 12.5%"

novikor · 2018-12-22T14:05:05Z

@orlangur , please check my fix for displaying correct percentage on frontend.
Also, it seems that someone already fixed this issue in 2.3-develop using explicit float conversion and ?float type hint...

orlangur · 2018-12-26T15:38:12Z

@novikor as I see it's still ?int in mainline. Please check one more time and resolve conflicts if it makes sense.

swnsma · 2019-01-08T07:17:10Z

Merged with MAGETWO-96016.
Refactored to use decomposition instead of inheritance.
Tests coverage added.
PR still contains fix for "Problem: Incorrect percentage value of discount on product page." (should be re-checked).

@orlangur, can you please make review? It will be nice to have it released with MAGETWO-96016 to do not broke backward compatibility.

orlangur · 2019-01-10T23:15:54Z

@swnsma nice idea of refactoring but need some time to checkout code locally and review, hope to find some spare time this weekend. Please stay tuned!

swnsma · 2019-01-30T20:10:22Z

Hi @orlangur
Any updates?)

sivaschenko · 2019-02-15T14:14:54Z

Hi @novikor can you please adjust the fix to keep existing classes/hierarchy for backward compatibility reasons.

Display percentage on PDP in case it is set explicitly.

novikor · 2019-02-16T13:31:58Z

@sivaschenko - done. Please recheck.

Fixed invalid zeros trimming.

novikor · 2019-05-05T16:48:20Z

Fixed.
@nmalevanec , please recheck.

ghost · 2019-05-05T16:48:56Z

@novikor unfortunately, only members of the maintainers team are allowed to assign developers to the pull request

soleksii · 2019-05-08T08:27:47Z

✔️ QA Passed

Result:

m2-assistant · 2019-05-14T15:06:34Z

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

…o#19584

magento-engcom-team added Partner: ISM eCompany Pull Request is created by partner ISM eCompany partners-contribution Pull Request is created by Magento Partner Component: Catalog Release Line: 2.3 labels Dec 5, 2018

novikor force-pushed the Tierprice-can-t-save-float-percentage-value-18651 branch from 4c0bce9 to 48f77bd Compare December 5, 2018 14:44

orlangur self-assigned this Dec 5, 2018

orlangur requested changes Dec 5, 2018

View reviewed changes

novikor force-pushed the Tierprice-can-t-save-float-percentage-value-18651 branch 3 times, most recently from 03028de to 9f71ffd Compare December 5, 2018 15:09

orlangur approved these changes Dec 5, 2018

View reviewed changes

magento-engcom-team added this to the Release: 2.3.1 milestone Dec 5, 2018

orlangur previously requested changes Dec 11, 2018

View reviewed changes

okorshenko removed this from the Release: 2.3.1 milestone Jan 28, 2019

sivaschenko added the Progress: needs update label Feb 15, 2019

sivaschenko self-assigned this Feb 15, 2019

Tierprice can't save float percentage value magento#18651.

4f318e5

Display percentage on PDP in case it is set explicitly.

novikor force-pushed the Tierprice-can-t-save-float-percentage-value-18651 branch from 84cd8cc to 4f318e5 Compare February 16, 2019 13:30

ghost unassigned nmalevanec and VasylShvorak May 5, 2019

Tierprice can't save float percentage value magento#18651.

c4c76d1

Fixed invalid zeros trimming.

novikor assigned nmalevanec May 5, 2019

ghost unassigned nmalevanec May 5, 2019

magento-engcom-team assigned nmalevanec May 6, 2019

ghost added Progress: ready for testing and removed Progress: needs update labels May 6, 2019

soleksii self-assigned this May 8, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels May 8, 2019

ghost added Progress: extended testing Progress: accept and removed Progress: testing in progress Progress: extended testing labels May 8, 2019

magento-engcom-team merged commit c4c76d1 into magento:2.3-develop May 14, 2019

magento-engcom-team pushed a commit to okorshenko/magento2 that referenced this pull request May 14, 2019

ENGCOM-5066: Tierprice can t save float percentage value 18651 magent…

a9618c1

…o#19584

magento-engcom-team added this to the Release: 2.3.3 milestone May 14, 2019

novikor mentioned this pull request May 16, 2019

[Backport] Tierprice can t save float percentage value 18651 novikor/magento2#1

Closed

4 tasks

novikor pushed a commit to novikor/magento2 that referenced this pull request May 16, 2019

magento#19584: Fixed static tests

b239b04

novikor pushed a commit to novikor/magento2 that referenced this pull request May 16, 2019

magento#19584: Fixed static tests

d1a6a83

This was referenced May 16, 2019

[Backport] Tierprice can t save float percentage value 18651 novikor/magento2#2

Closed

[Backport] Tierprice can t save float percentage value 18651 #22936

Merged

novikor pushed a commit to novikor/magento2 that referenced this pull request May 18, 2019

magento#19584: Fixed static tests

f8022f8

orlangur assigned orlangur and sivaschenko May 20, 2019

Tierprice can t save float percentage value 18651 #19584

Tierprice can t save float percentage value 18651 #19584

Uh oh!

Conversation

novikor commented Dec 5, 2018

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Steps to reproduce (*)

Expected result (*)

Actual result (*)

Contribution checklist (*)

Uh oh!

magento-engcom-team commented Dec 5, 2018

Uh oh!

novikor commented Dec 5, 2018

Uh oh!

magento-engcom-team commented Dec 5, 2018

Uh oh!

magento-engcom-team commented Dec 5, 2018

Uh oh!

orlangur Dec 5, 2018

Choose a reason for hiding this comment

Uh oh!

novikor Dec 5, 2018

Choose a reason for hiding this comment

Uh oh!

novikor Dec 5, 2018

Choose a reason for hiding this comment

Uh oh!

orlangur Dec 5, 2018

Choose a reason for hiding this comment

Uh oh!

magento-engcom-team commented Dec 5, 2018

Uh oh!

orlangur left a comment

Choose a reason for hiding this comment

Uh oh!

novikor commented Dec 22, 2018

Uh oh!

orlangur commented Dec 26, 2018

Uh oh!

swnsma commented Jan 8, 2019

Uh oh!

orlangur commented Jan 10, 2019

Uh oh!

swnsma commented Jan 30, 2019

Uh oh!

sivaschenko commented Feb 15, 2019

Uh oh!

novikor commented Feb 16, 2019

Uh oh!

novikor commented May 5, 2019

Uh oh!

ghost commented May 5, 2019

Uh oh!

soleksii commented May 8, 2019

Uh oh!

m2-assistant bot commented May 14, 2019

Uh oh!

Uh oh!