Skip to content
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

Commerce problem with variant's their status (Looks like they are disabled but still show front end)) #926

Closed
bob-pixeldeluxe opened this issue Jul 8, 2019 · 9 comments
Assignees
Labels

Comments

@bob-pixeldeluxe
Copy link

Description

Problem with variants their status.
So it seems to me that the getAllVariantsByProductId returns all product variants no matter the status.
So for me front end variants get displayed even if their status is disabled (i can fix it by checking the status front end) however i though i should still report it because it seems to me its buggy as well back end.

Screenshot 2019-07-08 at 10 24 21

This means the filter does not work when i select enabled it shows all the "disabled items"
And if i choose disabled it shows nothing.
I think this has something to do with multi site.

Thanks in advance,

Steps to reproduce

  1. Make a craft commerce site (multi site be sure to have at least 2)
  2. use a product variant field to try to display product variant front end ( and play with their status).

Additional info

  • Craft version: 3.1.31
  • PHP version: 7.2.10
  • Plugins & versions: Commerce (2.1.7)
@lukeholder lukeholder added the 🔎 status: investigating Trying to reproduce label Jul 13, 2019
@bob-pixeldeluxe
Copy link
Author

@lukeholder

Any progress?

Thank you.

@lukeholder
Copy link
Member

lukeholder commented Aug 14, 2019

@bob-pixeldeluxe

Yes, I was unable to reproduce. Video: https://jmp.sh/FJ8cmkm

As for getAllVariantsByProductId yes it does return all variants regardless of status. so product.variants returns all variants. If you want to only return variants that are enabled use a variant query with: {% for variant in craft.variants.product(product).all() %}. Let me know if that helps on the front-end.

@ghost
Copy link

ghost commented Aug 14, 2019

@lukeholder I was able to reproduce this on multiple Craft Commerce installs.

Craft version: 3.2.8
Commerce version: 2.1.10

Steps to reproduce:

  1. Create a product with variants
  2. Disable the product
  3. Create a Commerce Variants field
  4. Open the field modal

It will display all variants as the variant elements itself are enabled but it shows the status of the product. When I enable the product I still see the variants but now they have an enabled status.

EDIT: I'm creating an env with only the latest version of Commerce for testing.

@ghost
Copy link

ghost commented Aug 14, 2019

Was able to reproduce this on an env with only Commerce installed.

Craft version: 3.2.10
Commerce version: 2.1.11

Schermafbeelding 2019-08-14 om 10 50 29
Schermafbeelding 2019-08-14 om 10 53 26

Hope this helps!

@ghost
Copy link

ghost commented Aug 14, 2019

Another update: I've been looking through the source and I think I know why this is happening. Variants override the getStatus() function and return STATUS_DISABLED when their product is disabled. The element indexes controller probably queries the DB directly and since the variant is enabled there its included in the query results. The modal then prints these results but because the frontend uses getStatus() it will show the variant as disabled.

The VariantQuery should check the product status instead of the variant status.

@lukeholder
Copy link
Member

Looks like you have found it. Thinking of a solution now.

@lukeholder lukeholder self-assigned this Aug 24, 2019
@lukeholder lukeholder added the bug label Aug 24, 2019
@lukeholder
Copy link
Member

Will push a fix for this later today.

@ghost
Copy link

ghost commented Aug 28, 2019

Will push a fix for this later today.

Thank you!

@lukeholder lukeholder removed the 🔎 status: investigating Trying to reproduce label Aug 28, 2019
@lukeholder
Copy link
Member

  • Variant status is no longer overriden by the product status in Variant::getStatus()
  • Product status and Variant status together define if the variant is available for sale (which was the original intent of having the variant status check the product status before we introduced isAvailable)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants