From 642f6556118228609ef65d6e89c093232cd8ab85 Mon Sep 17 00:00:00 2001 From: tkostuch Date: Fri, 31 Jan 2020 13:51:06 +0100 Subject: [PATCH 1/5] refactor variant selection --- core/modules/catalog/helpers/index.ts | 34 +++++++++++++++++++-------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/core/modules/catalog/helpers/index.ts b/core/modules/catalog/helpers/index.ts index b7f0877a74..18c1ef90b9 100644 --- a/core/modules/catalog/helpers/index.ts +++ b/core/modules/catalog/helpers/index.ts @@ -5,6 +5,7 @@ import omit from 'lodash-es/omit' import remove from 'lodash-es/remove' import toString from 'lodash-es/toString' import union from 'lodash-es/union' +import isObject from 'lodash-es/union' // TODO: Remove this dependency import { optionLabel } from './optionLabel' import i18n from '@vue-storefront/i18n' @@ -42,6 +43,24 @@ const getVariantWithLowestPrice = (prevVariant, nextVariant) => ( nextVariant.price_incl_tax <= prevVariant.price_incl_tax // prev variant price is higher then next ) ? nextVariant : prevVariant +/** + * Counts how much coniguration match for specific variant + */ +const countMatchConfiguration = (configuration, variant): number => { + if (!variant || !configuration) return 0 + const configProperties = Object.keys(omit(configuration, ['price'])) + return configProperties + .map(configProperty => { + const filter = configuration[configProperty] + const configurationId = filter && isObject(filter) && filter.id + const variantPropertyId = variant[configProperty] + return (configurationId && variantPropertyId) && + (toString(configurationId) === toString(variantPropertyId)) + }) + .filter(Boolean) + .length +} + export function findConfigurableChildAsync ({ product, configuration = null, selectDefaultChildren = false, availabilityCheck = true }) { const regularProductPrice = product.original_price_incl_tax ? product.original_price_incl_tax : product.price_incl_tax const selectedVariant = product.configurable_children.reduce((prevVariant, nextVariant) => { @@ -66,17 +85,12 @@ export function findConfigurableChildAsync ({ product, configuration = null, sel return getVariantWithLowestPrice(prevVariant, nextVariant) } } else { - const matchConfiguration = Object.keys(omit(configuration, ['price'])).every((configProperty) => { - let configurationPropertyFilters = configuration[configProperty] || [] - if (!Array.isArray(configurationPropertyFilters)) configurationPropertyFilters = [configurationPropertyFilters] - const configurationIds = configurationPropertyFilters.map(filter => toString(filter.id)).filter(filterId => !!filterId) - if (!configurationIds.length) return true // skip empty - return configurationIds.includes(toString(nextVariant[configProperty])) - }) + const prevVariantMatch = countMatchConfiguration(configuration, prevVariant) + const nextVariantMatch = countMatchConfiguration(configuration, nextVariant) + // compare variants based on configuration match + const bestConfigVariant = nextVariantMatch > prevVariantMatch ? nextVariant : prevVariant - if (matchConfiguration) { - return getVariantWithLowestPrice(prevVariant, nextVariant) - } + return bestConfigVariant } } }, undefined) From 56be20396f22ecdad06210842d33e111fd49db89 Mon Sep 17 00:00:00 2001 From: tkostuch Date: Fri, 31 Jan 2020 14:04:45 +0100 Subject: [PATCH 2/5] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbf008ec13..e9211497d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix overlapping text in PersonalDetails component - @jakubmakielkowski (#4024) - Redirect from checkout to home with a proper store code - @Fifciu - Added back error notification when user selects invalid configuration - @1070rik (#4033) +- findConfigurableChildAsync - return best match for configurable variant - @gibkigonzo (#4042) ### Changed / Improved - Optimized `translation.processor` to process only enabled locale CSV files - @pkarw (#3950) From 557462b39f83c4accc863fdab5d7933ce7725895 Mon Sep 17 00:00:00 2001 From: tkostuch Date: Fri, 31 Jan 2020 14:42:20 +0100 Subject: [PATCH 3/5] update names, when equality then return next variant --- core/modules/catalog/helpers/index.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/modules/catalog/helpers/index.ts b/core/modules/catalog/helpers/index.ts index 18c1ef90b9..eca9424ce3 100644 --- a/core/modules/catalog/helpers/index.ts +++ b/core/modules/catalog/helpers/index.ts @@ -46,7 +46,7 @@ const getVariantWithLowestPrice = (prevVariant, nextVariant) => ( /** * Counts how much coniguration match for specific variant */ -const countMatchConfiguration = (configuration, variant): number => { +const getConfigurationMatchLevel = (configuration, variant): number => { if (!variant || !configuration) return 0 const configProperties = Object.keys(omit(configuration, ['price'])) return configProperties @@ -85,12 +85,12 @@ export function findConfigurableChildAsync ({ product, configuration = null, sel return getVariantWithLowestPrice(prevVariant, nextVariant) } } else { - const prevVariantMatch = countMatchConfiguration(configuration, prevVariant) - const nextVariantMatch = countMatchConfiguration(configuration, nextVariant) + const prevVariantMatch = getConfigurationMatchLevel(configuration, prevVariant) + const nextVariantMatch = getConfigurationMatchLevel(configuration, nextVariant) // compare variants based on configuration match - const bestConfigVariant = nextVariantMatch > prevVariantMatch ? nextVariant : prevVariant + const bestMatch = nextVariantMatch >= prevVariantMatch ? nextVariant : prevVariant - return bestConfigVariant + return bestMatch } } }, undefined) From 82fa7d7e64f0ab192a2762a7649690fa12ad0f47 Mon Sep 17 00:00:00 2001 From: tkostuch Date: Fri, 31 Jan 2020 14:48:20 +0100 Subject: [PATCH 4/5] get lowest price if match is equal --- core/modules/catalog/helpers/index.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/modules/catalog/helpers/index.ts b/core/modules/catalog/helpers/index.ts index eca9424ce3..74f17f0539 100644 --- a/core/modules/catalog/helpers/index.ts +++ b/core/modules/catalog/helpers/index.ts @@ -87,10 +87,12 @@ export function findConfigurableChildAsync ({ product, configuration = null, sel } else { const prevVariantMatch = getConfigurationMatchLevel(configuration, prevVariant) const nextVariantMatch = getConfigurationMatchLevel(configuration, nextVariant) - // compare variants based on configuration match - const bestMatch = nextVariantMatch >= prevVariantMatch ? nextVariant : prevVariant - return bestMatch + if (prevVariantMatch === nextVariantMatch) { + return getVariantWithLowestPrice(prevVariant, nextVariant) + } + + return nextVariantMatch > prevVariantMatch ? nextVariant : prevVariant } } }, undefined) From 2b880b14fa48ef3887e832094ddb80b3d7b73545 Mon Sep 17 00:00:00 2001 From: tkostuch Date: Fri, 31 Jan 2020 14:54:57 +0100 Subject: [PATCH 5/5] simplify code --- core/modules/catalog/helpers/index.ts | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/core/modules/catalog/helpers/index.ts b/core/modules/catalog/helpers/index.ts index 74f17f0539..ed0ed4314f 100644 --- a/core/modules/catalog/helpers/index.ts +++ b/core/modules/catalog/helpers/index.ts @@ -62,7 +62,6 @@ const getConfigurationMatchLevel = (configuration, variant): number => { } export function findConfigurableChildAsync ({ product, configuration = null, selectDefaultChildren = false, availabilityCheck = true }) { - const regularProductPrice = product.original_price_incl_tax ? product.original_price_incl_tax : product.price_incl_tax const selectedVariant = product.configurable_children.reduce((prevVariant, nextVariant) => { if (availabilityCheck) { if (nextVariant.stock && !config.products.listOutOfStockProducts) { @@ -80,20 +79,14 @@ export function findConfigurableChildAsync ({ product, configuration = null, sel if (configuration.sku && nextVariant.sku === configuration.sku) { // by sku or first one return nextVariant } else { - if (!configuration || (configuration && Object.keys(configuration).length === 0)) { // no configuration - return the first child cheaper than the original price - if found - if (nextVariant.price_incl_tax <= regularProductPrice) { - return getVariantWithLowestPrice(prevVariant, nextVariant) - } - } else { - const prevVariantMatch = getConfigurationMatchLevel(configuration, prevVariant) - const nextVariantMatch = getConfigurationMatchLevel(configuration, nextVariant) + const prevVariantMatch = getConfigurationMatchLevel(configuration, prevVariant) + const nextVariantMatch = getConfigurationMatchLevel(configuration, nextVariant) - if (prevVariantMatch === nextVariantMatch) { - return getVariantWithLowestPrice(prevVariant, nextVariant) - } - - return nextVariantMatch > prevVariantMatch ? nextVariant : prevVariant + if (prevVariantMatch === nextVariantMatch) { + return getVariantWithLowestPrice(prevVariant, nextVariant) } + + return nextVariantMatch > prevVariantMatch ? nextVariant : prevVariant } }, undefined) return selectedVariant