craftcms / commerce

Fully integrated ecommerce for Craft CMS.
https://craftcms.com/commerce
Other
215 stars 170 forks source link

[4.x]: Sales category match, doesn't respect purchasable site #3328

Closed vandres closed 9 months ago

vandres commented 10 months ago

What happened?

Description

We came across a "bug", where sales were calculated wrongly, on a newly created site.

The product in use, has different active categories per site. Our sales are calculating the promotion based on these categories.

Site Shop EUR: Category EUR -45% Site Shop CHF: Category CHF -45% Site Blog: Category EUR -45%, Category CHF -45%

Now we have a different site, where products are queryied from one of the shop sites. The price, descriptions, categories are all correct. But when I query the sales, it always take that different site. That happens to have both categories. If I disable both categories for that site, there is no promotion at all.

Steps to reproduce

  1. Create sites "A" and "B"
  2. Create category "Site A -45%", which is only enabled on site A
  3. Create category "Site B -45%", which is only enabled on site B
  4. Create a product, with both categories enabled
  5. Depending on which site you look at it, only one of the categories is active
  6. Create 2 sales "A -45%" und "B -45%" depending on those categories
  7. Create site "C"
  8. Both categories should be visible in the product on that site
  9. Now query the product on site C for site A in the Frontend and check the sales (craft.products().siteId(siteA.id).all())

Expected behavior

Only the sale, enabled for site A should be applied

Actual behavior

Sales from site C are applied

Additional information

I hotfixed vendor/craftcms/commerce/src/services/Sales.php, lines 459 and 460. It seems to work. But I am not 100% sure, if it is totally correct.

from

$relatedCategories = Category::find()->id($saleCategories)->relatedTo($relatedTo)->ids();
$relatedEntries = Entry::find()->id($saleCategories)->relatedTo($relatedTo)->ids();

to

$relatedCategories = Category::find()->id($saleCategories)->relatedTo($relatedTo)->siteId($purchasable->siteId)->ids();
$relatedEntries = Entry::find()->id($saleCategories)->relatedTo($relatedTo)->siteId($purchasable->siteId)->ids();

Craft CMS version

4.5.9

Craft Commerce version

4.3.2 Pro

PHP version

8.2

Operating system and version

CentOS

Database type and version

MariaDB 10.5

Image driver and version

-

Installed plugins and versions

    "carlcs/craft-footnote": "^3.0",
    "craftcms/cms": "^4.3.8",
    "craftcms/commerce": "^4.1.0",
    "craftcms/commerce-paypal-checkout": "dev-develop",
    "craftcms/contact-form": "^3.0",
    "craftcms/contact-form-honeypot": "^2.0",
    "craftcms/redactor": "^3.0",
    "mmikkel/cp-field-inspect": "^1.4",
    "nystudio107/craft-retour": "^4.1",
    "nystudio107/craft-vite": "^4.0",
    "php-http/curl-client": "^2.2",
    "putyourlightson/craft-campaign": "^2.7",
    "putyourlightson/craft-elements-panel": "^2.0",
    "putyourlightson/craft-sprig": "^2.1",
    "sebastianlenz/linkfield": "^2.1",
    "spatie/craft-ray": "^2.0",
    "spicyweb/craft-neo": "^3.2",
    "ttempleton/craft-nocache": "^3.0",
    "typesense/typesense-php": "^4.8",
    "vaersaagod/geomate": "^2.1.0",
    "verbb/expanded-singles": "^2.0",
    "verbb/field-manager": "^3.0",
    "verbb/tablemaker": "^4.0",
    "verbb/wishlist": "^2.0",
    "vlucas/phpdotenv": "^5.4.0",
    "wbrowar/adminbar": "^3.2",
    "webburospring/craft-spring-pixabay": "^1.2"
vandres commented 10 months ago

I think, the problem also occurs on Product/VariantQueries like this:

        $products = Product::find()
            ->siteId($shopSite->id)
            ->hasVariant([
                'hasSales' => true,
            ])
            ->limit(2)
            ->orderBy('RAND()')
            ->all();

It shows products, even there is no sale on that "$shopSite"

lukeholder commented 8 months ago

Thanks for reporting that! I’ve just fixed it for the next release.

To get the fix early, change your craftcms/commerce requirement in composer.json to:

"require": {
  "craftcms/commerce": "dev-develop#ad6803123e4dbe5127fb7780a9ed650dda27ac29",
  "...": "..."
}

Then run composer update.

We will update you here once the release is out. Thanks.