nystudio107 / craft-similar

Similar for Craft lets you find elements, Entries, Categories, Commerce Products, etc, that are similar, based on... other related elements.
https://nystudio107.com/
MIT License
26 stars 5 forks source link

$similarCounts not sanity checked on line 162 of src/services/Similar.php #29

Closed jamiewhitemccann closed 3 years ago

jamiewhitemccann commented 3 years ago

Describe the bug

2021-03-09 10:58:03 [-][3][de99c010b4cfe6c82fedeef4c2b2ddb0][error][yii\base\ErrorException:8] yii\base\ErrorException: Undefined index: 1-175221 in /app/vendor/nystudio107/craft-similar/src/services/Similar.php:162

To reproduce

We are seeing fatal errors when using this plugin, although we are not 100% on the exact scenario to recreate.

Expected behaviour

The plugin should not throw error exceptions in this manner

Versions

Suggested patch to sanity check the $similarCounts array using a simple isset on the index:

From 3767b2c3d1393956a9d3fbf199e9fcfd5978495b Mon Sep 17 00:00:00 2001
From: Jamie White
Date: Tue, 9 Mar 2021 16:27:27 +0000
Subject: [PATCH] Adding a sanity check for for the $similarCounts array

Fix for the errors we are seeing:
2021-03-09 16:01:29 [-][-][0e5667ba12611515c899ffcdf5691c09][error][yii\base\ErrorException:8] yii\base\ErrorException: Undefined index: 1-175221 in /srv/live/src/hiq/tyre-directory/vendor/nystudio107/craft-similar/src/services/Similar.php:162
---
 src/services/Similar.php | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/services/Similar.php b/src/services/Similar.php
index 1c5a629..ddd9780 100644
--- a/src/services/Similar.php
+++ b/src/services/Similar.php
@@ -159,7 +159,9 @@ class Similar extends Component
         foreach ($elements as $element) {
             // The `count` property is added dynamically by our CountBehavior behavior
             /** @noinspection PhpUndefinedFieldInspection */
-            $element->count = $similarCounts[$element->siteId . '-' . $element->id];
+            if (isset($similarCounts[$element->siteId . '-' . $element->id])) {
+                $element->count = $similarCounts[$element->siteId . '-' . $element->id];
+            }
         }

         return $elements;
-- 
2.27.0
khalwat commented 3 years ago

Fixed in https://github.com/nystudio107/craft-similar/commit/0d81f2f27cb698c469aefce68ad76379153ced0d

You can try it now by setting your semver in your composer.json to look like this:

    "nystudio107/craft-similar": "dev-develop as 1.1.1”,

Then do a composer update

jamiewhitemccann commented 3 years ago

Thank you very much Andrew!

Regards, Jamie

[signature_1389440161]

Jamie White Technical Lead McCann, Tower Wharf, Cheese Lane, Bristol, BS2 0JJ 0117 921 8102 www.mccannbristol.co.ukhttp://www.mccannbristol.co.uk/

From: Andrew Welch notifications@github.com Reply to: nystudio107/craft-similar reply@reply.github.com Date: Wednesday, 10 March 2021 at 04:08 To: nystudio107/craft-similar craft-similar@noreply.github.com Cc: Jamie White Jamie.White@mccann.com, Author author@noreply.github.com Subject: [EXTERNAL] Re: [nystudio107/craft-similar] $similarCounts not sanity checked on line 162 of src/services/Similar.php (#29)

Fixed in 0d81f2fhttps://urldefense.com/v3/__https:/github.com/nystudio107/craft-similar/commit/0d81f2f27cb698c469aefce68ad76379153ced0d__;!!N96JrnIq8IfO5w!ynyHQa5FUriURbaC9hHCIw7WCgPfOwkMOhzyOoXzHCZMwushDLQkSDdB8Hx3-m5H7g$

You can try it now by setting your semver in your composer.json to look like this:

"nystudio107/craft-similar": "dev-develop as 1.1.1”,

Then do a composer update

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/nystudio107/craft-similar/issues/29*issuecomment-794840694__;Iw!!N96JrnIq8IfO5w!ynyHQa5FUriURbaC9hHCIw7WCgPfOwkMOhzyOoXzHCZMwushDLQkSDdB8Hxe2M0ZzA$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AEKX6EA75RDFFTD7MPJCL2TTC3PCBANCNFSM4Y4AE7KQ__;!!N96JrnIq8IfO5w!ynyHQa5FUriURbaC9hHCIw7WCgPfOwkMOhzyOoXzHCZMwushDLQkSDdB8HyDDPHOhQ$.

This message contains information which may be confidential and privileged. Unless you are the intended recipient (or authorized to receive this message for the intended recipient), you may not use, copy, disseminate or disclose to anyone the message or any information contained in the message. If you have received the message in error, please advise the sender by reply e-mail, and delete the message. Thank you very much.

solutiondrop commented 3 years ago

Hi,

I was receiving the same error and set my semver and updated with fix. It works as expected when I've assigned tags to a product, but when there are no tags assigned it now show products that aren't tagged similarly. It used to show nothing, but now with the update it shows the most recently added products.

Here's how I'm calling them:

{% set limitCriteria = craft.products.limit(20) %} {% set similarEntriesByTags = craft.similar.find({ element: product, context: product.recommendationTags, criteria: limitCriteria }) %} ... {% for similarEntry in similarEntriesByTags %} ... {% endfor %}

I'm wondering if I'm doing something wrong here? Thanks!

jamiewhitemccann commented 3 years ago

My college has opened this ticket, which I think is actually the root cause of the error firing in the first place:

https://github.com/nystudio107/craft-similar/issues/31

khalwat commented 3 years ago

This issue is fixed in 1.1.1, which is released. https://github.com/nystudio107/craft-similar/releases/tag/1.1.1

@solutiondrop If you have a separate issue, please file a new issue. Might be related to: https://github.com/nystudio107/craft-similar/issues/31