search

pimcore / ecommerce-framework-bundle

Ecommerce Framework community bundle provides e-commerce functionality such as product listing and filtering, pricing, carts and checkouts for Pimcore.
https://pimcore.com/docs/platform/Ecommerce_Framework/
Other
11 stars 33 forks source link

fix: .map(Number) creates problems if preselect is not an id but a string #206

Closed MercuryKojo closed 3 weeks ago

MercuryKojo commented 1 month ago

this checks if filterGroups in fieldConfig contains 'relation' and only maps to Number. This could still cause an issue if a field contains IDs and strings.
I couldn't find a use case for this, but something to think about. Another solution would be to cast the key in FilterGroupHelper->getGroupValuesForFilterGroup to string and we can remove the map function altogether. The function is only used in the pimcore_ecommerceframework_index_getvaluesforfilterfield route and therefore should not cause other problems

mattamon commented 1 month ago

Hey @MercuryKojo thanks for the PR .

Could you maybe also describe a way to reproduce it?

MercuryKojo commented 1 month ago

Hey @MercuryKojo thanks for the PR .

Could you maybe also describe a way to reproduce it?

Sure thing: https://demo.pimcore.com/admin/login/deeplink?object_563_object

add a FilterMultiSelect Precondition select field 'Name' for example (any field with strings as values) preselect a Value -> at the moment there seems to be a loading animation which won´t go away, but if you scroll a little you can still select one save and reload preselect is gone because it tries to map a string to a number in Javascript

mattamon commented 1 month ago

@MercuryKojo Thank you!

I do get it now and it was introduced here https://github.com/pimcore/ecommerce-framework-bundle/pull/194. Sorry for that.

Your solution works fine, but maybe since it is 4 times the same code of getting the data out of the preselect, we could introduce a method for this and then just call the method inside of setValue?

mattamon commented 4 weeks ago

@MercuryKojo Thank you!

I do get it now and it was introduced here #194. Sorry for that.

Your solution works fine, but maybe since it is 4 times the same code of getting the data out of the preselect, we could introduce a method for this and then just call the method inside of setValue?

Alternatively I can do it too for you since I already would have it preprared. WDYT?

MercuryKojo commented 4 weeks ago

@MercuryKojo Thank you! I do get it now and it was introduced here #194. Sorry for that. Your solution works fine, but maybe since it is 4 times the same code of getting the data out of the preselect, we could introduce a method for this and then just call the method inside of setValue?

Alternatively I can do it too for you since I already would have it preprared. WDYT?

would be apreciated

mattamon commented 4 weeks ago

@MercuryKojo Thank you! I do get it now and it was introduced here #194. Sorry for that. Your solution works fine, but maybe since it is 4 times the same code of getting the data out of the preselect, we could introduce a method for this and then just call the method inside of setValue?

Alternatively I can do it too for you since I already would have it preprared. WDYT?

would be apreciated

Done! :) Could you confirm that this still works for you?

MercuryKojo commented 3 weeks ago

@MercuryKojo Thank you! I do get it now and it was introduced here #194. Sorry for that. Your solution works fine, but maybe since it is 4 times the same code of getting the data out of the preselect, we could introduce a method for this and then just call the method inside of setValue?

Alternatively I can do it too for you since I already would have it preprared. WDYT?

would be apreciated

Done! :) Could you confirm that this still works for you?

sry for the late reply can confirm, still works for me