elasticsuite
elasticsuite copied to clipboard
Smile-SA
Add option to index child attributes or not
While working with bundle products on a website, the merchant wants to only include attributes of the parent/bundle product not any of its options/children. This configuration option adds the ability for website administrators to make this decision. By default, the existing behaviour is preserved.
Hi @fredden,
having such a config globally is not enough for me, I'd like to be able to also control which attributes are mergeable, and which are not, as it's done "hardcoded" actually, but with the ability to inject the list through DI.
Can you rework this PR ? Anyway, thank you for proposing it.
Best regards
Hi @romainruaud. Thanks for the feedback.
Making the list of forbidenChildrenAttributeCodes configurable sounds like a good idea. If that feature existed, we may have been able to use that instead of the approach here.
I think having the setting exposed in the admin makes more sense than via dependency injection though. This way, if a website administrator adds a new attribute, they'll be able to select its index status right away, and not require a deployment to set this.
With an admin configuration option per attribute, I can see it being a chore marking each and every attribute individually as indexable or not; having a global toggle would help then - which is what's being introduced here.
What do you think about putting this global toggle in for now, and working on getting the list configurable as a separate task?
@fredden Leaving this here for notes, but following the merge of #2465, this file requires it's constructor modifying to also accommodate for the new Config parameter.
Thanks @robaimes. I've updated this pull request to be compatible with the changes introduced in #2465.
@romainruaud I still think that this should be merged, and the list of attributes should be set in admin configuration (not via dependency injection) as a separate task / feature.