-
Notifications
You must be signed in to change notification settings - Fork 7
feat: [SFCC-466] Fallback for simple products for the VARIATION_GROUP_LEVEL record model #242
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
feat: [SFCC-466] Fallback for simple products for the VARIATION_GROUP_LEVEL record model #242
Conversation
e900f66 to
83c7fba
Compare
sbellone
left a comment
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.
Logic looks good for the simple products, I would only suggest to keep the algoliaLocalizedProductModel as agnostic as possible and not pass the recordModel to it.
But for products that are neither master, variation group or simple product, for example bundles, those changes are removing important information from the records: the attributes defined as variantAttribute: true are not indexed anymore: price (pricebooks), in_stock
We probably want to compute the variants for any kind of product?
cartridges/int_algolia/cartridge/scripts/algolia/model/algoliaLocalizedProduct.js
Outdated
Show resolved
Hide resolved
cartridges/int_algolia/cartridge/scripts/algolia/model/algoliaLocalizedProduct.js
Outdated
Show resolved
Hide resolved
cartridges/int_algolia/cartridge/scripts/algolia/model/algoliaLocalizedProduct.js
Outdated
Show resolved
Hide resolved
cartridges/int_algolia_sfra/cartridge/static/default/js/algolia/instantsearch-config.js
Show resolved
Hide resolved
e3de939 to
cdd5f96
Compare
Removed
Following our discussion, we are not going to exclude option products, bundles and product sets, these are special cases of simple (standard) products. Renamed
Right, but only if the model is not |
|
|
||
| /** | ||
| * Returns true if supplied product is a standard product, false otherwise | ||
| * A standard product is a product that is neither a master nor a variant |
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.
The naming will create confusion, as SFCC explicitly states the following:
Set products and bundled products aren’t a standard product.
https://help.salesforce.com/s/articleView?id=cc.b2c_product_types.htm&type=5
Why do we need this helper, isn't the simple else enough?
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.
It's enough in this case, I modified this line.
Regarding the naming convention, it's a mess, really.
SFCC is really inconsistent between their documentation and the API; I'm honestly not surprised that so many people are confused by SFCC product types. Someone reading the Salesforce documentation will walk away with the wrong impression if they don't take a look at and experiment with the API as well.
Let's take the different naming conventions that the API (and its documentation) and the Salesforce documentation use: what the API docs call a "master" is called a "base product" by the documentation. A "simple product" in the API documentation is a "standard product" in the documentation.
Since buying out Demandware, Salesforce is likely trying to retcon the naming conventions, probably to fit better with their other products, but they can't change the API anymore, so all of this leads to a schizophrenic situation with no clear name for these product types.
Exhibit A: product types in the API documentation:

Exhibit B: product types in the Salesforce documentation:

Please note that there's no mention of the alternate names in either of the documentations and neither of the docs draw a correlation between them.
Technically there are only three product types and two classes (Product and Variant, the latter extends the former). All products that are not variants or masters are simple products with optional product flags that show whether they are a product set, a bundle, part of a bundle or a set, if they have options, etc.
Since there's no isStandardProduct() or isSimpleProduct() method and no separate class for standard products, they only way to verify whether a product is a standard product is to make sure it's not a master, not a variant, not a variation group, not a set product, not a product set, not a product bundle and not a bundled product (not to mention option products, which is, again, a misnomer since an option product is a not a product in and of itself which is attached to another product, but a product that has options), which is a lot to verify. One could say that there should be a build-in API method for this.
Since we want to continue including product sets and bundled products in the index, but want to make a distinction between them and variants and masters, we need to call the set that includes simple products, product bundles and product sets somehow, but there's no official name for this (again, technically, from the API's standpoint, these are just standard products that have a flag set, but again, the Salesforce documentation does not match this, it views this differently).
Everything above is further complicated by:
- a product set being a product that is a product set, while
- a product set product being a product that is part of a product set, and
- a product bundle being a product that is a bundle itself, and
- a bundled product being a product that is part of a product bundle,
these are all product flags that can be retrieved (and separate product types according to the Salesforce documentation), so a product that is not either of these types, not an option product, not a variant, not a master and not a variation group is left to be a standard product.
TL;DR
Going forward, I'm going to bypass all this nonsense and just work with three types: masters, variants and simple products (this category including bundles, bundled products, product sets, product set products and option products, but we already include these, so no change there) and their definition is going to be:
- master:
product.isMaster() - variant:
product.isVariant() - simple product: products that are neither masters nor variants, and also not variation groups -- this is going to include bundles, sets, etc. This way we don't leave out any products.
I'm also going to keep using the naming convention from the API reference as that's the one coders are working with (and which was also the first): master, variant, simple product.
cartridges/int_algolia/cartridge/scripts/algolia/model/algoliaLocalizedProduct.js
Outdated
Show resolved
Hide resolved
sbellone
left a comment
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.
Much simpler, looks good now 👍
…-product-fallback
In order to stay consistent with masters and records grouped by a variation attribute, simple products now have a
variantsarray when using theVARIATION_GROUP_LEVELrecord model, just like masters do. Thevariantsarray contains the same attributes that a master's variant would have.This change was required in order for the general shape of the records to be the same and to make faceting work with both masters and simple products alike.
algoliaProductIndex.jsjob script to handle simple productsalgoliaProductDeltaIndex.jsjob script to handle simple productsalgoliaLocalizedProduct.jswith the new modelisSimpleProductinjobHelper.js