algoliasearch-magento-2 icon indicating copy to clipboard operation
algoliasearch-magento-2 copied to clipboard

Algolia's usage of `queryId` on the add to cart before/after event observers is potentially problematic.

Open dfelton opened this issue 3 years ago • 0 comments

Empty Commit for purposes of PR, in order to report issue on GitHub, because they have Issues disabled on the repository

Summary

Explain the motivation for making this change. What existing problem does the pull request solve? Are there any linked issues?

  • Explain the motivation for making this change:
    • To report a problem
  • What existing problem does the pull request solve?:
    • None, pointing out a problem via PR because the repository has Issues
  • Are there any linked issues?
    • If you guys would re-enable issues on your repository, I would have reported this there instead.

Summary of Problem:

The Algolia\AlgoliaSearch\Model\Observer\Insights\CheckoutCartProductAddBefore and Algolia\AlgoliaSearch\Model\Observer\Insights\CheckoutCartProductAddAfter class relies on performing a $product->setData('queryId', $requestInfo['queryID']) and $product->getData('queryId') to pass data back and forth between each other.

This is potentially problematic because:

  • it does not account for a scenario where a client or other third party module has actually added an EAV product attribute queryId and utilizes it for their own purposes.
  • In such an event, Algolia's _before observer would pollute the EAV attribute with bad data.
  • AlgoliaSearch, does not bother to register any EAV queryId catalog_product attribute, but because it is treating it like it does have this attribute, this could make debugging the problematic scenario above somewhat difficult for developers who may encounter this situation.

Possible suggestions for a better approach would be to:

  1. Algolia creates a Algolia\AlgoliaSearch\Model\Observer\Insights\QueryId class.
  • This class has two methods:
    • setQueryId(?string $queryId = null): void
    • getQueryId(): ?string
  • This QueryId class is then injected into CheckoutCartProductAddBefore, and this class performs the setQueryId() call to store the current value.
  • This QueryId class is also injected into CheckoutCartProductAddAfter and uses the getQueryId() method to fetch the current value.
  1. Algolia revises the CheckoutCartProductAddBefore and CheckoutCartProductAddAfter classes to utilize a SessionInterface object to store & fetch queryId.

dfelton avatar Aug 04 '22 20:08 dfelton