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
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions Controller/Adminhtml/Integration/CreateStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,13 @@ public function generateSearchEngineData($storeGroupId)

foreach ($this->storeConfig->getStoreGroupStores($storeGroupId) as $store) {
$language = $this->storeConfig->getLanguageFromStore($store);
$currency = strtoupper($store->getCurrentCurrency()->getCode());
$store_id = $store->getId();
$base_url = $store->getBaseUrl();

// store_id field refers to store_view's id.
$searchEngineConfig[] = [
"name" => $store->getName(),
"language" => $language,
"currency" => $currency,
"site_url" => $base_url,
"callback_url" => $base_url . 'doofinderfeed/setup/processCallback?storeId=' . $store_id,
"options" => [
Expand All @@ -156,7 +154,7 @@ public function generateSearchEngineData($storeGroupId)
]
];

$storesConfig[$language][$currency] = (int)$store_id;
$storesConfig[$language] = (int)$store_id;
}
return [
"searchEngineConfig" => $searchEngineConfig,
Expand Down Expand Up @@ -201,26 +199,24 @@ private function saveInstallationConfig($storeGroupId, $installationId, $script)
* "search_engines":{"de":{"USD":"024d8eb1caa649775d08f3f69ddf333a"},"en":{"USD":"c3981a773ac987e5828c94677cda237f"}}
* We're going to iterate over the search_engines because there is the data created in doofinder. May occour that some
* of the data that we've in storeConfig has some invalid parameter and will be bypass during the creation.
*
*
* @param $storesConfig
* @param $searchEngines
*/
private function saveSearchEngineConfig($storesConfig, $searchEngines)
{
foreach ($searchEngines as $language => $values) {
foreach ($values as $currency => $hashid) {
$storeId = $storesConfig[$language][$currency];
$this->storeConfig->setHashId($hashid, $storeId);
$this->setIndexationStatus($storeId);
}
foreach ($searchEngines as $language => $hashid) {
$storeId = $storesConfig[$language];
$this->storeConfig->setHashId($hashid, $storeId);
$this->setIndexationStatus($storeId);
}
}

/**
* Function to store the status of the SE indexation.
*
*
* By default we set this value to "STARTED" and will be updated when we receive the callback from doofinder
*
*
* @param $storeId
*/
private function setIndexationStatus($storeId)
Expand Down
1 change: 1 addition & 0 deletions Helper/StoreConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ public function getDisplayLayer(): ?string

if (!empty($displayLayerScript) && 1 !== preg_match('/dfLayerOptions/', $displayLayerScript) && 1 !== preg_match('/doofinderApp/', $displayLayerScript)) {
$store = $this->getCurrentStore();
//TODO: Adapt in case layer needs the info in a different format
$currency = $store->getCurrentCurrency()->getCode();
$language_country = $this->getLanguageFromStore($store);
$lang_parts = explode('-', $language_country);
Expand Down
26 changes: 16 additions & 10 deletions Model/ProductRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public function get($sku, $editMode = false, $storeId = null, $forceReload = fal
$product->load($productId);
$this->appEmulation->startEnvironmentEmulation($storeId, Area::AREA_FRONTEND, true);
$this->setExtensionAttributes($product, $storeId);
$this->setCustomAttributes($product);
$this->setCustomAttributes($product, $storeId);
$this->appEmulation->stopEnvironmentEmulation();
// End Custom code here
$this->cacheProduct($cacheKey, $product);
Expand All @@ -143,7 +143,7 @@ public function getList(SearchCriteriaInterface $searchCriteria)
}

$this->setExtensionAttributes($product, $storeId);
$this->setCustomAttributes($product);
$this->setCustomAttributes($product, $storeId);
}
$this->appEmulation->stopEnvironmentEmulation();

Expand Down Expand Up @@ -293,11 +293,13 @@ private function prepareSku(string $sku): string
* Here we will update also the value of the custom attribute (id of the option selected) by the option text.
*
* @param ProductInterface $product
* @param int $storeId
* @return void
*/
private function setCustomAttributes($product): void
private function setCustomAttributes($product, $storeId): void
{
$productHelper = $this->productHelperFactory->create();
$priceHelper = $this->priceHelperFactory->create();
$store = $this->storeManager->getStore($storeId);
Comment on lines +301 to +302
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!

$customAttributes = $this->storeConfig->getCustomAttributes($product->getStoreId());

foreach ($customAttributes as $customAttribute) {
Expand All @@ -318,6 +320,7 @@ private function setCustomAttributes($product): void
$product->setCustomAttribute('thumbnail', $thumbnailImageUrl);
$smallImageUrl = $this->getImage($product, 'product_small_image')->getUrl();
$product->setCustomAttribute('small_image', $smallImageUrl);

$this->removeExcludedCustomAttributes($product);
}

Expand All @@ -330,10 +333,11 @@ private function setCustomAttributes($product): void
*/
private function setExtensionAttributes($product, $storeId): void
{
$priceHelper = $this->priceHelperFactory->create();
$productHelper = $this->productHelperFactory->create();
$inventoryHelper = $this->inventoryHelperFactory->create();
$storeCode = $this->storeManager->getStore($storeId)->getCode();
$store = $this->storeManager->getStore($storeId);
$priceHelper = $this->priceHelperFactory->create();

/** @var ProductExtension $extensionAttributes */
$extensionAttributes = $product->getExtensionAttributes();
Expand All @@ -353,11 +357,6 @@ private function setExtensionAttributes($product, $storeId): void
$extensionAttributes->setCategoryLinks($this->getCategoriesInformation($categories));
}

$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);

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

$extensionAttributes->setImage($productHelper
->getProductImageUrl(
$product,
Expand All @@ -366,6 +365,13 @@ private function setExtensionAttributes($product, $storeId): void
->getValueFromConfig("doofinder_config_config/doofinder_image/doofinder_image_size")
));

$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));
Comment on lines +368 to +373
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.


$product->setExtensionAttributes($extensionAttributes);
}

Expand Down
1 change: 1 addition & 0 deletions etc/extension_attributes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
<attribute code="image" type="string"/>
<attribute code="price" type="float"/>
<attribute code="special_price" type="float"/>
Comment on lines 9 to 10
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.

<attribute code="prices" type="string"/>
</extension_attributes>
</config>