-
Notifications
You must be signed in to change notification settings - Fork 9.4k
magento/magento2#39169: Special Price To Date is wrongly validated on applySpecialPrice #39690
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
base: 2.4-develop
Are you sure you want to change the base?
magento/magento2#39169: Special Price To Date is wrongly validated on applySpecialPrice #39690
Conversation
Hi @KrasnoshchokBohdan. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
@magento run all tests |
@magento run all tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @KrasnoshchokBohdan,
Thanks for the contribution!
Please fix the failed static test and also please add an automated test in accordance to the DOD.
Thanks
…cialPrice - changing Copyright
@magento run all tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -274,7 +274,7 @@ public function isScopeDateInInterval($scope, $dateFrom = null, $dateTo = null) | |||
$scopeTimeStamp = $this->scopeTimeStamp($scope); | |||
$fromTimeStamp = strtotime($dateFrom); | |||
$toTimeStamp = strtotime($dateTo); | |||
if ($dateTo) { | |||
if ($dateTo && date('H:i:s', strtotime($dateTo)) === '00:00:00') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will have the same +1 day result if use '00:00:00' in the API query. I'm not sure that it's correct solution
pls provide more information if we update special price with
#39169 from description how I tested it https://app.magento.test/rest/default/V1/products/special-price (app.magento.test replace with your base_url) if we check with POST /rest/<store_code>/V1/products/special-price-information but we should triger \Magento\Framework\Stdlib\DateTime\Timezone::isScopeDateInInterval |
@KrasnoshchokBohdan I mean
After the fix I haven't checked the source code more deeply, but I don't rule out that somewhere the time value may be thrown off and date become |
but it was the same before fix fix is only cover
#39169 from description { sorry it seems I still don't understand the question |
@magento run Integration Tests |
…cialPrice - more targeted solution to cover only the question described in the issue
@magento run all tests |
@magento run Integration Tests |
Thank you for your contribution! As I can see that, you are working on the review comments and tests failures, also marked the PR as draft hence I am moving it to on Hold status. Please feel free to Open it once you are done with implementation. |
It seems that bot moved it to Review bucket, moving it back to On Hold |
…cialPrice - cover additional isScopeDateInInterval method usage for special price interval checking
@magento run all tests |
@magento run all tests |
@magento run all tests |
@magento run all tests |
@magento run Functional Tests B2B, Integration Tests, Magento Health Index |
@Den4ik @engcom-Hotel |
Hello @KrasnoshchokBohdan, After reviewing the changes in the PR, it appears to offer a reasonable solution to the described issue by limiting the scope to custom prices and addressing the incorrect handling of the time component in However, the duplication of logic (extract the @Den4ik, looking forward to your thoughts on this. Thanks |
Hello @KrasnoshchokBohdan, Is this PR is ready for review? |
Description (*)
As the author of the issue mentioned, if we add a special price using POST /rest/<store_code>/V1/products/special-price specifying in the request body date like "price_to": "2017-07-16 23:59:59" i.e. time including hours, minutes and seconds, then as a result we will get that the special price will be valid for one day longer than the specified date
this happens in the method \Magento\Framework\Stdlib\DateTime\Timezone::isScopeDateInInterval where there is code that literally adds +1 day
according to the example from the documentation (where "price_to": "2017-07-16 23:59:59" including hours is specified) we get this execution

therefore in this pull request I propose to skip adding 1 day in case in "price_to" specifies the time including hours
Related Pull Requests
--
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)