-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Fix for the issue #24547 Magento\Customer\Model\Account\Redirect::setRedirectCookie() not properly working #24612
Fix for the issue #24547 Magento\Customer\Model\Account\Redirect::setRedirectCookie() not properly working #24612
Conversation
Hi @sashas777. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
…ct::setRedirectCookie() not properly working
@paliarush can you advise how to avoid these errors, please? /var/www/html/app/code/Magento/Customer/Model/Account/Redirect.php:28 The class Magento\Customer\Model\Account\Redirect uses sessions or cookies while not being a part of HTML Presentation layer
/var/www/html/app/code/Magento/Customer/Model/Account/Redirect.php:103 The method __construct has 10 parameters. Consider reducing the number of parameters to less than 10.
/var/www/html/app/code/Magento/Customer/Model/RedirectCookieManager.php:14 The class Magento\Customer\Model\RedirectCookieManager uses sessions or cookies while not being a part of HTML Presentation layer
Failed asserting that 2 matches expected 0.``` |
Hello @sashas777
is common and can be suppressed by adding
can be suppressed by adding Although, changing the constructor signature isn't backward compatible. Please refer to Backward compatible development for your issues. |
*/ | ||
public function setRedirectCookie($route, StoreInterface $store) | ||
{ | ||
$cookieMetadata = $this->cookieMetadataFactory->createPublicCookieMetadata() |
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.
Public cookie metadata is used. What will be the behavior and is it appropriate in the case if connection is secured?
Hello @sashas777! |
Merge from 2.4-develop
…ookieManager, setCookieManager, getRedirectCookie, setRedirectCookie, clearRedirectCookie
…ookieManager, setCookieManager, getRedirectCookie, setRedirectCookie, clearRedirectCookie
@dyushkin Updated. Does it look correct now? |
…ookieManager, setCookieManager, getRedirectCookie, setRedirectCookie, clearRedirectCookie
@sashas777, thank you for quick updates, looks fine for me. |
@magento run all tests |
Hi @lenaorobei, thank you for the review. |
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.
All changes are correct.
✔️ QA passed |
…Redirect::setRedirectCookie() not properly working #24612
Hi @sashas777, thank you for your contribution! |
Description (*)
Added metadata to the setRedirectCookie and clearRedirectCookie functions.
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)