-
Notifications
You must be signed in to change notification settings - Fork 9
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
First draft changes for the plugin #338
base: master
Are you sure you want to change the base?
First draft changes for the plugin #338
Conversation
$priceHelper = $this->priceHelperFactory->create(); | ||
$store = $this->storeManager->getStore($storeId); |
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.
@ogomezba esto lo has importado aquí con la intención de pedir todos los precios después? o para qué? no veo que lo utilices, no?
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.
@sofia-doofinder Pues como inicialmente pensaba que iba a tener que hacer el cálculo como customAttribute, movía la lógica aquí y, cuando más tarde vi que se podía hacer en los extensionAttributes, moví el código de vuelta de nuevo pero se me olvidó quitar este. No se usa así que lo quito. Gracias!
$currency = $store->getCurrentCurrency()->getCode(); | ||
$price = round($priceHelper->getProductPrice($product, 'regular_price'), 2); | ||
$specialPrice = round($priceHelper->getProductPrice($product, 'final_price'), 2); | ||
|
||
$prices = [$currency => ["price" => $price, "sale_price" => $specialPrice]]; | ||
$extensionAttributes->setPrices(json_encode($prices)); |
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.
tal y como se plantea aquí la respuesta de la API va a devolver sólo el precio para una moneda no? no habría que traerse todas las monedas para la store actual y rellener el array con todas las monedas posibles?
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.
Sí, si queremos realmente usar el campo de "Allowed Currencies" como propongo en el comentario de la PR, la idea sería efectivamente obtener la lista de currencies permitidas y obtener los precios en cada currency. No está puesto porque ahora mismo no usamos el campo "Allowed Currencies", sino la "Default Currency" del Store View. Ahora respetaría la configuración tal y como la usamos actualmente (entiendo que empezar a usar el otro campo es un cambio no solamente interno nuestro y habría que validarlo con producto?).
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.
sí, lo del "Allowed Currencies" mejor comentarlo con producto antes que nada. Creo que no me he explicado bien. Ahora mismo tenemos por ejemplo search engine Español EURO y Español PESOS, tenemos dos search engines con el mismo idioma y diferentes monedas, esto significa que tenemos en magento dos store views una español - euro y otra español - pesos, que están dentro del mismo store group. Ahora con este cambio vamos a tener sólo un search engine que va a ser español, y a la hora de pedir los datos debería indexar en euro y en pesos, pero con el cambio que veo aquí arriba sólo lo estaríamos indexando en una de las dos monedas. O me estoy perdiendo algo?
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.
Esto es justo lo que comentaba en el primer comentario de la PR. Creo que el mapeo de Store View a Search Engine lo deberíamos mantener porque la URL base la marca el Store View (de hecho, cuando se instala el plugin, el feed_url que asignamos a cada Search Engine es diferente por Store View o puede serlo. Por eso, creo que es mejor mantener que cada Store View genere un Search Engine porque la idea de los Store Views (tal y como se comenta en la documentación de Magento) es tener diferentes lenguajes (u otros motivos, pero no por currencies). Para habilitar múltiples currencies, se debería usar el "Allowed Currencies" y no un nuevo Store View.
<attribute code="price" type="float"/> | ||
<attribute code="special_price" type="float"/> |
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.
estos habría que quitarlos y dejar solo prices
no? porque si se ha quitado el código de arriba (le he escrito a José para confirmar que ya estos campos no se esperarán)
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.
Correcto, el qué vamos a hacer con los otros precios era un punto pendiente por confirmar. Habría que ver el tema de la retrocompatibilidad cómo va a ser y ver si los podemos quitar.
$price = round($priceHelper->getProductPrice($product, 'regular_price'), 2); | ||
$specialPrice = round($priceHelper->getProductPrice($product, 'final_price'), 2); | ||
$extensionAttributes->setPrice($price); | ||
($price == $specialPrice || $specialPrice == 0) ?: $extensionAttributes->setSpecialPrice($specialPrice, 2); | ||
|
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.
para mantener la retrocompatibilidad creo que esto no podemos borrarlo. Tendremos que detectar de alguna forma si el cliente está utilizando search engines con multicurrencies o no y en función de lo que tenga devolver el precio como hasta ahora o de la forma nueva.
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.
Totalmente de acuerdo. El approach que he seguido para estas PRs es hacer el código asumiendo la nueva forma de funcionar y validar que es viable (y tener el código también para futuro como referencia). Pero efectivamente, en una PR real futura, estas partes habría que mantenerlas.
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.
genial, gracias Oscar
Changes required by the Magento plugin to multiprice search engines.
Decisions
Open Items / Questions