-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Fix SQL error throwed when websiteId is NULL #36802
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
Closed
Sekiphp
wants to merge
3
commits into
magento:2.4-develop
from
Sekiphp:fix/add-website-group-date-filter
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
In my opinion there is not solving core of issue because it's looks like unpredictable application behavior.
Everywhere we expect to receive integer value of website id and we need to solve core of this problem specifically why website id is null.
@Sekiphp could you please add total list of steps for reproducing the error
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.
@Den4ik It is truth. I was not able to simulate this error, but this part is problematic and I just use same solution, which is used in method, which is also called inside this method. In our website there are a lot of diferrent salesrules and probably not all are configured well and throws this error in some combination of steps.
Uh oh!
There was an error while loading. Please reload this page.
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.
Hi @Sekiphp,
I just analyzed where this code is used - looks like it might be only in
magento2/app/code/Magento/SalesRule/Block/Rss/Discounts.php
Line 97 in 3218b12
While website id is getting retrieved from store. In case when store doesn't have linked website - this will trigger such error.
magento2/app/code/Magento/SalesRule/Block/Rss/Discounts.php
Line 73 in 3218b12
In this case, I think direct type conversion is good