Take care deleting variants products

[sofia-doofinder]

Required by: https://github.com/doofinder/support/issues/3108

Estamos teniendo problemas en el update on save porque al borrar productos variantes se estamos eliminando del feed el producto padre en vez de la variante, esto viene por este PR: #334. Se ha añadido la condición de que añadimos a la base de datos el producto padre en vez de la variante sólo si estamos editando / creando, pero no para el borrado.

Además, haciendo pruebas he visto que con el cambio que se hizo en este PR (#358) para no incluir al feed los productos sin visibilidad esta provoando que si un producto simple se edita directamente lo marcamos como que hay que borrarlo en vez de actualizar su producto padre. Por eso a la condición de que si no tiene la visibildiad esperada lo borramos se le ha añadido la condición de que además no tenga producto padre, para excluir de esa eliminación a las variantes.

[ogomezba]

Buenas Sofía! Una duda por confirmar que he entendido bien.

Con estos cambios, cuando una variante se cambie su estado a uno de los "desactivados", añadiríamos a la tabla del update on save sus padres con la operación "UDPATE", es correcto? Porque imagino que con el update del padre, tanto el padre como las variables que se quedan se actualizan pero, la entrada de la otra variante se borra en Elastic?

[sofia-doofinder]

perdona Óscar que creo que no me expliqué muy bien.

• Si una variante cambia su estado a desactivado entraría por el if

if ($product->getStatus() == Status::STATUS_DISABLED) {
   $this->setOperationType(ChangedItemInterface::OPERATION_TYPE_DELETE);

y se setearía la operación a deleted y después como es una variante y va a tener producto padre en vez de borrar el padre se pondría en base de datos para borrar ella misma:

$parentProducts = $this->configurableProductType->getParentIdsByChild($itemId);
    if (count($parentProducts) > 0 && $this->getOperationType() != ChangedItemInterface::OPERATION_TYPE_DELETE) {

• Si una variante cambia en cualquier otra cosa que no sea el estado (edición) en lugar de insertar en la tabla el id de la variante se incluiría el del producto en la base de datos para que con el update on save del padre se actualicen todas sus variantes.
• Si lo que se está editando es un producto con una visibilidad diferente a Visibility::VISIBILITY_IN_SEARCH, visibility::VISIBILITY_BOTH] lo vamos a eliminar porque son productos que no deberían estar en el feed, a no ser que sean variantes que en este caso si que tenemos indexarlas. (Y antes sin querer se estaba marcando como para borrar y después borrabamos el producto padre).
  (Código antiguo)

} elseif (!in_array($product->getVisibility(), $this->visibilityAllowed)){
                    $this->setOperationType(ChangedItemInterface::OPERATION_TYPE_DELETE);
............
if (count($parentProducts) > 0) {

Igual si que el primer caso en vez de borrar directamente la variante se podría incluir el producto padre como para actualizar en vez de para borrar y así se le borrarían las variantes que se han quitado 🤔 crees que es mejor solución? También puede ser que me haya equivocado en algún caos y por eso te ha llevado a confusión?

[ogomezba]

Nada, todo claro! Lo veo genial.

[brunovesar]

Lo unico que no me cierra es eso de que cuando una variante no es visible actualizamos el padre para que borre las variantes que no debería indexar.
Este comportamiento parece raro, realmente estamos borrando las variantes que no debemos indexar cada vez que se actualiza el producto padre o alguna variante?
De todas formas en el caso del borrado de la variante entiendo que esta bien borrarla, ya que el padre no va a saber que existía la variante para borrarla del índice, no?

[brunovesar]

This change is intended?
Seems that the operation type updated in the if above won´t be taken into account on this if, and seems like it should

[sofia-doofinder]

no entiendo bien esto, lo he cambiado porque en todos los otros casos se hace la comparación directamente con operationType porque se define arriba $operationType = $this->getOperationType(); y era por tener todos igual

[sofia-doofinder]

gracias!! no lo había visto

[brunovesar]

Thanks for clarifiying how visibilities works on magento, now it's clear

[mursisoy]

LGTM!