feat: [SFCC-523][KTLO] Changed direct product property accesses to getters and setters#278
Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 3 high |
🟢 Metrics 10 complexity · 5 duplication
Metric Results Complexity 10 Duplication 5
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull request overview
This PR normalizes Salesforce Commerce Cloud (SFCC) Script API usage across the Algolia integration by replacing direct property access (e.g., product.ID, product.master, pli.product, money.value) with the corresponding getter methods (e.g., getID(), isMaster(), getProduct(), getValue()), and updates the Jest mocks/tests to match the new access patterns.
Changes:
- Updated indexing steps, filters, helpers, and recommend utilities to use SFCC getters (
getID,isMaster,getVariants,getCount, etc.). - Updated SFRA controllers/hooks to use getters on product line items and products (
getProduct(),getQuantityValue(),getMasterProduct(), etc.). - Extended unit-test mocks with Money- and MediaFile-like helpers and collection
get()support to keep tests aligned with SFCC API usage.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/int_algolia/scripts/hooks/order/algoliaHooks.test.js | Adjusts shipment/PLI fixtures to use getProduct() instead of .product. |
| test/unit/int_algolia/scripts/algolia/recommend/utils.test.js | Updates product fixtures to use getID(), getMasterProduct(), getVariationModel(), etc. |
| test/mocks/helpers/moneyHelper.js | Adds a Money-like mock exposing both fields and getter methods. |
| test/mocks/helpers/mediaFileHelper.js | Adds a MediaFile-like decorator exposing both fields and getters. |
| test/mocks/helpers/collectionHelper.js | Extends collection mocks with .get(index) to better match SFCC-style lists. |
| test/mocks/dw/catalog/Variant.js | Updates variant mock pricing and price model to provide Money-like getters and pricebook info getters. |
| test/mocks/dw/catalog/ProductVariationModel.js | Ensures variation attributes/images can be accessed via getters and returns MediaFile-like mocks. |
| test/mocks/dw/catalog/ProductVariationAttributeValue.js | Adds getID(), getValue(), getDisplayValue() to match API usage. |
| test/mocks/dw/catalog/MasterProduct.js | Updates master product mock to return Money-like objects and adds online/searchable/bundled accessors. |
| jest.setup.js | Aligns global mocks with new getter usage (e.g., getCount(), pricebook getters, allowed currencies get()). |
| cartridges/int_algolia/cartridge/scripts/algolia/steps/algoliaProductIndex.js | Uses products.getCount() and product.isMaster() / product.getID() in job logic and logging. |
| cartridges/int_algolia/cartridge/scripts/algolia/steps/algoliaProductDeltaIndex.js | Uses product.isMaster(), product.getID(), and product.getVariants() in delta indexing. |
| cartridges/int_algolia/cartridge/scripts/algolia/steps/algoliaContentIndex.js | Uses siteLocales.get(l) when iterating locales. |
| cartridges/int_algolia/cartridge/scripts/algolia/recommend/utils.js | Uses getMasterProduct(), getDefaultVariant(), and getID() for anchor product logic. |
| cartridges/int_algolia/cartridge/scripts/algolia/model/algoliaLocalizedProduct.js | Replaces many direct accesses (promotion/product/category/value) with getters across attribute handlers. |
| cartridges/int_algolia/cartridge/scripts/algolia/model/algoliaLocalizedCategory.js | Uses category.getID() for category IDs/URLs. |
| cartridges/int_algolia/cartridge/scripts/algolia/helper/modelHelper.js | Uses variation value getters and MediaFile getters for image group creation. |
| cartridges/int_algolia/cartridge/scripts/algolia/helper/jobHelper.js | Uses getters for logging and setSelectedAttributeValue(...) inputs. |
| cartridges/int_algolia/cartridge/scripts/algolia/filters/productFilter.js | Uses isOnline(), isSearchable(), isBundled(), getVariants() for filter logic. |
| cartridges/int_algolia_sfra/cartridge/scripts/hooks/order/algoliaHooks.js | Uses pli.getProduct() for inventory update hook processing. |
| cartridges/int_algolia_sfra/cartridge/controllers/Order.js | Uses product/PLI getters for Insights payload composition. |
| cartridges/int_algolia_sfra/cartridge/controllers/Algolia.js | Uses Money getters when comparing promotional/default prices. |
Comments suppressed due to low confidence (2)
cartridges/int_algolia/cartridge/scripts/algolia/steps/algoliaProductDeltaIndex.js:535
product.getVariants()returns an SFCC collection/list, so indexing it withvariants[v]is not reliable in production (it works in tests because the mock is a plain array). Iterate viaiterator()or usevariants.get(v)to avoid runtime issues during delta indexing cleanup.
let variants = product.getVariants();
if (variants && variants.size() > 0) {
for (let v = 0; v < variants.size(); ++v) {
let variant = variants[v];
if (!productFilter.isInclude(variant)) {
cartridges/int_algolia/cartridge/scripts/algolia/model/algoliaLocalizedProduct.js:440
- This still relies on direct
Moneyproperty access (price.currencyCode/price.value). To align with the getter-based SFCC API usage (and avoid failures if the script engine doesn’t expose those fields), usegetCurrencyCode()/getValue()here as well.
if (price) {
if (!promotionalPrice) { promotionalPrice = {}; }
promotionalPrice[price.currencyCode] = price.value;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (promotionPrice.getValue() && promotionPrice.getValue() < minPrice ) { | ||
| minPrice = promotionPrice.getValue(); | ||
| activePromotion = promotion; | ||
| } |
There was a problem hiding this comment.
This is valid, addressing issue in PR #279.
Various fixes for some of the issues flagged by the automated reviewer on PR #278. Details for each fix in the PR comments.
Normalized SFCC API usage across the cartridge. The code now uses setters and getters instead of direct property access, as it is recommended by Salesforce.