OrchardCMS / OrchardCore.Commerce

The commerce module for Orchard Core.
MIT License
218 stars 90 forks source link

PriceVariantProduct issues (OCC-262) #461

Open sarahelsaig opened 4 months ago

sarahelsaig commented 4 months ago

Discussed in https://github.com/OrchardCMS/OrchardCore.Commerce/discussions/460

Originally posted by **DotCat1985** July 8, 2024 Hi, I installed Orchard Core Commerce and so I launched **Orchard Core Commerce - Content - Product** recipe for trying the **PriceVariantProduct** type. 1 As you can see on the screenshots below, I set the predefined settings **M**, **L**, **XL** and a default value of the **Size** field. 2 3 But, when I open the page for creating a **PriceVariantProduct**: - a **Price** variant is written as **X-L Price** instead of **XL Price** - a **Inventory** variant is written as **L Inventory** instead of **XL Inventory** 4 Also, when I try to create a **PriceVariantProduct**, I receive the error message **SKU must not empty** even if I set **SKU=CLOTHING**. 5 I get the same result setting **Medium**, **Large**, **Extra Large** and a default value of the Size field. I have no problem setting **S**, **M**, **L** and a default value of the Size field. How can I solve this issue?
\n\n[Jira issue](https://lombiq.atlassian.net/browse/OCC-262)
DotCat1985 commented 4 months ago

Hi, During my debugging session of OrchardCore.Commerce solution, I discovered that, in row 84 of OrchardCore.Commerce.Drivers.PriceVariantsPartDisplayDriver.cs, attr.HtmlClassify().ToUpperInvariant() transforms the item XL into X-L. I don't know the use of HtmlClassify(), but this bug could be fixed by removing HtmlClassify(). What do you think about it?

Screenshot 2024-07-16 at 20 04 05
sarahelsaig commented 4 months ago

I will look into this and double check later, but I think .HtmlClassify().ToUpperInvariant() was intended to force the variant key to respect the format constraints of SKUs (all upper case, no spaces). This is because the variant keys are appended to the end of the SKU in the order summary (and you'd use that complex SKU in invoicing). Looks like .HtmlClassify() was not a perfect fit fir this task because it splits the word along before capital letters so XL becomes X-L and LARGE becomes L-A-R-G-E. So ironically now it only works when the predefined values are lower case such as m, l, xl.