Skip to content
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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ogomezba
Copy link
Contributor

Changes required by the Magento plugin to multiprice search engines.

Decisions

  • The feed URL depends on the Store View. Hence, it looked like a bad idea to mix several Store Views into one Search Engine.
  • Also, it looks like the function of the Store Views is mainly translations (it allows for different languages to be set) and not multicurrency.
  • What we do right now is consider the Default Display Currency for a Store View and use it as the currency of the Search Engine.
  • However, it might be time to reconsider this as the real configuration for the allowed currencies for a Store View is actually in the Allowed Currencies field which can also be configured at Store View level.
    image
  • If we use this, now we would have a Search Engine per Store View (configuring different languages) and a number of allowed currencies, which would configure the prices for every product in that Search Engine (this would be much similar to what we are doing with the other platforms).

Open Items / Questions

  • How are we going to handle the retrocompatibility/deployment?
  • Is there any update needed in the script?
  • Is there any update needed due to changes in the Installation search engine mapping?
  • How are we going to determine which currency symbol to use?
  • Is there any update on the response of the Doomanager create store request?

@ogomezba ogomezba self-assigned this Aug 13, 2024
@ogomezba ogomezba linked an issue Aug 13, 2024 that may be closed by this pull request
Comment on lines +302 to +303
$priceHelper = $this->priceHelperFactory->create();
$store = $this->storeManager->getStore($storeId);
Copy link
Member

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?

Copy link
Contributor Author

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!

Comment on lines +369 to +374
$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));
Copy link
Member

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?

Copy link
Contributor Author

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?).

Copy link
Member

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?

Copy link
Contributor Author

@ogomezba ogomezba Aug 16, 2024

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.

Comment on lines 9 to 10
<attribute code="price" type="float"/>
<attribute code="special_price" type="float"/>
Copy link
Member

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)

Copy link
Contributor Author

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.

Comment on lines -356 to -360
$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);

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

genial, gracias Oscar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Magento2] Analysis of search engine with multi currency: plugin
2 participants