Fix SQL error throwed when websiteId is NULL #36802

Sekiphp · 2023-02-03T09:50:40Z

Description (*)

In code is sometimes situation when $websiteId passed to addWebsiteGroupDateFilter is NULL.

Throwed error in GraphQL

Code

Problematic part:

Inside addWebsiteGroupDateFilter is called $this->addWebsiteFilter($websiteId);, but NULL for $websiteId is not problem there, because inside this method is this variable casted to int.

Manual testing scenarios (*)

...

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)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

Resolved issues:

resolves [Issue] Fix SQL error throwed when websiteId is NULL #37284: Fix SQL error throwed when websiteId is NULL

m2-assistant · 2023-02-03T09:50:43Z

Hi @Sekiphp. Thank you for your contribution
Here are 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

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

Sekiphp · 2023-02-03T09:58:46Z

@magento run all tests

magento-automated-testing · 2023-02-03T09:58:54Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

Sekiphp · 2023-02-06T13:04:11Z

@magento run Magento Health Index

magento-automated-testing · 2023-02-06T13:04:18Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

ihor-sviziev

hi @Sekiphp,
Your changes looks good. Could you please cover your change with some automated tests?

app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php

Den4ik · 2023-02-07T21:53:02Z

app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php

@@ -332,13 +332,9 @@ public function addWebsiteGroupDateFilter($websiteId, $customerGroupId, $now = n
                'customer_group_ids.' .
                $entityInfo['entity_id_field'] .
                ' = cgw.' .
-                $entityInfo['entity_id_field'] . ' AND ' . $websiteId . ' = cgw.website_id',
+                $entityInfo['entity_id_field'] . ' AND ' . (int) $websiteId . ' = cgw.website_id',


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

@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.

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

foreach ($this->rssModel->getDiscountCollection($websiteId, $customerGroupId) as $rule) {

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

$websiteId = $storeModel->getWebsiteId();

In this case, I think direct type conversion is good

AshishKumarPundeer · 2023-02-13T07:23:27Z

@magento run Integration Tests

magento-automated-testing · 2023-02-13T07:23:34Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

engcom-Lima · 2023-03-23T13:26:19Z

@magento create issue

engcom-Lima · 2023-03-23T13:26:34Z

@magento run all tests

magento-automated-testing · 2023-03-23T13:26:43Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

ihor-sviziev · 2023-10-05T11:45:55Z

@engcom-Charlie any updates on this?

aissyass · 2023-12-01T20:02:25Z

Hello @engcom-Lima @Sekiphp @Den4ik @ihor-sviziev
Any update please? We had the bug too!

engcom-Charlie · 2024-09-23T06:58:10Z

@magento run all tests

engcom-Charlie · 2024-09-24T06:36:16Z

Hi @Sekiphp,

Thank you for your contribution!

Code wise the changes looks okay but we want to reproduce the issue in order to proceed further on the same.

In magento its not possible to have a store without a websiteId attached to it. We tried to reproduce this issue on local setup with different sections and tried to see when addWebsiteGroupDateFilter is getting used but no luck.

Can you please provide some more details on issue reproduction steps, till then moving this PR to On hold.

Thank you!

engcom-Charlie · 2024-10-08T09:36:44Z

As mentioned here, we are unable to reproduce this issue hence closing this PR now.

Please feel free to reopen it for any further updates.

Thank you!

Fix SQL error throwed when websiteId is NULL

ac23fb8

m2-community-project bot added the Progress: pending review label Feb 3, 2023

ihor-sviziev requested changes Feb 7, 2023

View reviewed changes

m2-community-project bot assigned ihor-sviziev Feb 7, 2023

m2-community-project bot added Progress: needs update and removed Progress: pending review labels Feb 7, 2023

ihor-sviziev added the Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. label Feb 7, 2023

Den4ik requested changes Feb 7, 2023

View reviewed changes

m2-community-project bot assigned Den4ik Feb 7, 2023

m2-community-project bot added Progress: review and removed Progress: needs update labels Feb 13, 2023

ihor-sviziev approved these changes Mar 17, 2023

View reviewed changes

m2-community-project bot removed the Progress: review label Mar 17, 2023

ihor-sviziev added Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests Progress: review labels Mar 17, 2023

m2-community-project bot added Progress: ready for testing and removed Progress: ready for testing Progress: review labels Mar 17, 2023

engcom-Lima self-assigned this Mar 23, 2023

m2-community-project bot added Progress: testing in progress and removed Progress: testing in progress labels Mar 23, 2023

m2-community-project bot added Progress: ready for testing Progress: testing in progress and removed Progress: ready for testing labels Mar 23, 2023

Merge branch '2.4-develop' into fix/add-website-group-date-filter

dbf6d86

m2-assistant bot mentioned this pull request Mar 23, 2023

[Issue] Fix SQL error throwed when websiteId is NULL #37284

Closed

5 tasks

m2-community-project bot added Progress: ready for testing and removed Progress: testing in progress labels Mar 23, 2023

m2-community-project bot added Progress: testing in progress and removed Progress: ready for testing labels Mar 23, 2023

engcom-Lima removed their assignment Mar 24, 2023

engcom-Lima added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Mar 24, 2023

m2-community-project bot added Progress: ready for testing and removed Progress: testing in progress labels Mar 24, 2023

engcom-Charlie self-assigned this Mar 28, 2023

Merge branch '2.4-develop' into fix/add-website-group-date-filter

cb22121

thai2301 pushed a commit to thai2301/magento2 that referenced this pull request Sep 24, 2024

magento#36802: fixing calculator has not been init yet

975e86d

engcom-Charlie closed this Oct 8, 2024

m2-community-project bot removed the Progress: ready for testing label Oct 8, 2024

Fix SQL error throwed when websiteId is NULL #36802

Fix SQL error throwed when websiteId is NULL #36802

Uh oh!

Conversation

Sekiphp commented Feb 3, 2023 • edited by m2-assistant bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Throwed error in GraphQL

Code

Manual testing scenarios (*)

Contribution checklist (*)

Resolved issues:

Uh oh!

m2-assistant bot commented Feb 3, 2023

Uh oh!

Sekiphp commented Feb 3, 2023

Uh oh!

magento-automated-testing bot commented Feb 3, 2023

Uh oh!

Sekiphp commented Feb 6, 2023

Uh oh!

magento-automated-testing bot commented Feb 6, 2023

Uh oh!

ihor-sviziev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Den4ik Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

Sekiphp Feb 13, 2023

Choose a reason for hiding this comment

Uh oh!

ihor-sviziev Feb 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AshishKumarPundeer commented Feb 13, 2023

Uh oh!

magento-automated-testing bot commented Feb 13, 2023

Uh oh!

engcom-Lima commented Mar 23, 2023

Uh oh!

engcom-Lima commented Mar 23, 2023

Uh oh!

magento-automated-testing bot commented Mar 23, 2023

Uh oh!

ihor-sviziev commented Oct 5, 2023

Uh oh!

aissyass commented Dec 1, 2023

Uh oh!

engcom-Charlie commented Sep 23, 2024

Uh oh!

engcom-Charlie commented Sep 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

engcom-Charlie commented Oct 8, 2024

Uh oh!

Uh oh!

Sekiphp commented Feb 3, 2023 •

edited by m2-assistant bot

Loading

ihor-sviziev Feb 16, 2023 •

edited

Loading

engcom-Charlie commented Sep 24, 2024 •

edited

Loading