-
Notifications
You must be signed in to change notification settings - Fork 6
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 to adapt the plugin #232
base: master
Are you sure you want to change the base?
Conversation
@@ -62,6 +62,7 @@ | |||
|
|||
foreach (Language::getLanguages(true, $context->shop->id) as $lang) { | |||
$lang = Tools::strtoupper($lang['iso_code']); | |||
//TODO: Is this still needed? | |||
$currency = dfTools::getCurrencyForLanguage($lang); |
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 creo que es para cuando se hace una petición a la config del plugin que aparezca todos los idiomas y monedas que el cliente tiene configurados. Creo que está bien mantenerlo
* | ||
* @return string | ||
*/ | ||
public function getHashId($id_lang, $id_currency) | ||
public function getHashId($id_lang) |
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.
creo que hay que tener cuidado con este cambio porque para los clientes que hagan una instalación nueva si que se les va a crear un search engine por idioma pero para los antiguos tenemos que seguir siendo capaces de buscar el hashid teniendo en cuenta la moneda. Esta función tiene que ser retrocompatible con una instalación "antigua" por lo que no podemos hacer este cambio directamente.
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 mismo que comentaba para Magento, la idea de las PRs era verificar que la nueva forma de funcionar era viable. Cuando tengamos claro cómo lo vamos a hacer retrocompatible en general, el código deberá poder distinguir entre los dos casos.
echo ($product_price && $onsale_price && $product_price != $onsale_price) | ||
? Tools::convertPrice($onsale_price, $currency) : ''; | ||
|
||
echo json_encode($prices) . TXT_SEPARATOR; |
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.
Te dejo por aquí lo que comentó José de cómo deberíamos devolver los precios en el caso de un fichero de texto:
id|title|pricesxxx|t1|price_EUR=33/sale_price_USD=20/price_rupia=33000/sale_price_EUR=22
```
feeds/product.php
Outdated
|
||
$prices = []; | ||
|
||
//TODO: What do we do when the prices should not be indexed? |
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.
ahora mismo si el show_price está deshabilitado lo que hace es mandar el campo price a false? o es que no se manda directamente ese campo en el csv?
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.
Ahora mismo, sí el show_price está deshabilitado, no se manda el campo directamente
@@ -498,16 +496,19 @@ function array_merge_by_id_product($array1 = [], $array2 = []) | |||
$product_price = dfTools::get_price($product_id, $cfg_prices_w_taxes); | |||
$onsale_price = dfTools::get_onsale_price($product_id, $cfg_prices_w_taxes); | |||
|
|||
if (!$row['show_price']) { | |||
$product_price = false; | |||
$onsale_price = false; |
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.
aquí pasa un poco igual que antes, no se si deberíamos sacar esta parte a una función y dependiendo de si el cliente esté utilizando el nuevo sistema que calcule el precio de la forma nueva pero que si es de los antiguos se mantenga el código antiguo
@@ -264,10 +262,13 @@ private function getAvailability($product) | |||
} | |||
} | |||
|
|||
private function getPrice($product, $salePrice = false) | |||
private function getPrices($product) |
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.
y aquí lo mismo también
b7f0984
to
cce9947
Compare
$lang_code = $lang['language_code']; | ||
$feed_url = $this->buildFeedUrl($shopId, $lang['iso_code']); | ||
$store_data['search_engines'][] = [ | ||
'language' => $lang_code, |
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.
Changes required by the prestashop plugin to adapt to multiprice search engines.
Decisions
Open Items / Questions