Skip to content

Commit c14fa3f

Browse files
KrasnoshchokBohdanKrasnoshchokBohdan
committed
#39905 Product Removed on Mobile Still Appears in Web's Mini Compare Section Until Re-login
Introduced `CompareCookieManager` enhancements with better cookie management through visibility modifiers and documentation changes. Suppressed warnings for PHPMD where relevant and added comprehensive unit tests for cookie handling, including edge cases like size limits and failures.
1 parent a70613e commit c14fa3f

File tree

5 files changed

+249
-8
lines changed

5 files changed

+249
-8
lines changed

‎app/code/Magento/CompareListGraphQl/Model/Resolver/AddProductsToCompareList.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
/**
2626
* Add products item to compare list
27+
*
28+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2729
*/
2830
class AddProductsToCompareList implements ResolverInterface
2931
{
@@ -118,7 +120,6 @@ public function resolve(
118120
throw new GraphQlInputException(__($exception->getMessage()));
119121
}
120122

121-
122123
if (!$listId) {
123124
throw new GraphQlInputException(__('"uid" value does not exist'));
124125
}

‎app/code/Magento/CompareListGraphQl/Model/Resolver/CreateCompareList.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525

2626
/**
2727
* Class for creating compare list
28+
*
29+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2830
*/
2931
class CreateCompareList implements ResolverInterface
3032
{
@@ -70,6 +72,7 @@ class CreateCompareList implements ResolverInterface
7072
* @param GetCompareList $getCompareList
7173
* @param CreateCompareListService $createCompareList
7274
* @param Compare|null $productCompareHelper
75+
* @param CompareCookieManager|null $compareCookieManager
7376
*/
7477
public function __construct(
7578
Random $mathRandom,

‎app/code/Magento/CompareListGraphQl/Model/Resolver/RemoveProductsFromCompareList.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
/**
2626
* Remove items from compare list
27+
*
28+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2729
*/
2830
class RemoveProductsFromCompareList implements ResolverInterface
2931
{

‎app/code/Magento/CompareListGraphQl/Model/Service/CompareCookieManager.php‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,25 @@
1616

1717
/**
1818
* Service for managing compare list cookies
19+
*
20+
* @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
1921
*/
2022
class CompareCookieManager
2123
{
2224
/**
2325
* Name of cookie that holds compare products section data
2426
*/
25-
const COOKIE_COMPARE_PRODUCTS = 'section_data_ids';
27+
public const COOKIE_COMPARE_PRODUCTS = 'section_data_ids';
2628

2729
/**
28-
* Cookie path
30+
* The path for which the cookie will be available
2931
*/
30-
const COOKIE_PATH = '/';
32+
public const COOKIE_PATH = '/';
3133

3234
/**
33-
* Cookie lifetime value
35+
* Cookie lifetime value in seconds (86400 = 24 hours)
3436
*/
35-
const COOKIE_LIFETIME = 86400;
37+
public const COOKIE_LIFETIME = 86400;
3638

3739
/**
3840
* @var CookieManagerInterface
@@ -56,8 +58,8 @@ class CompareCookieManager
5658
*/
5759
public function __construct(
5860
CookieManagerInterface $cookieManager,
59-
CookieMetadataFactory $cookieMetadataFactory,
60-
LoggerInterface $logger
61+
CookieMetadataFactory $cookieMetadataFactory,
62+
LoggerInterface $logger
6163
) {
6264
$this->cookieManager = $cookieManager;
6365
$this->cookieMetadataFactory = $cookieMetadataFactory;

‎app/code/Magento/CompareListGraphQl/Test/Unit/Model/Service/CompareCookieManagerTest.php‎

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
<?php
2+
/**
3+
* Copyright 2025 Adobe
4+
* All Rights Reserved.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\CompareListGraphQl\Test\Unit\Model\Service;
9+
10+
use Magento\Framework\Exception\InputException;
11+
use Magento\Framework\Stdlib\Cookie\CookieMetadataFactory;
12+
use Magento\Framework\Stdlib\Cookie\CookieSizeLimitReachedException;
13+
use Magento\Framework\Stdlib\Cookie\FailureToSendException;
14+
use Magento\Framework\Stdlib\Cookie\PublicCookieMetadata;
15+
use Magento\Framework\Stdlib\CookieManagerInterface;
16+
use Magento\CompareListGraphQl\Model\Service\CompareCookieManager;
17+
use PHPUnit\Framework\MockObject\Exception;
18+
use PHPUnit\Framework\MockObject\MockObject;
19+
use PHPUnit\Framework\TestCase;
20+
use Psr\Log\LoggerInterface;
21+
22+
/**
23+
* Unit test for CompareCookieManager
24+
*/
25+
class CompareCookieManagerTest extends TestCase
26+
{
27+
/**
28+
* @var CookieManagerInterface|MockObject
29+
*/
30+
private $cookieManagerMock;
31+
32+
/**
33+
* @var CookieMetadataFactory|MockObject
34+
*/
35+
private $cookieMetadataFactoryMock;
36+
37+
/**
38+
* @var LoggerInterface|MockObject
39+
*/
40+
private $loggerMock;
41+
42+
/**
43+
* @var PublicCookieMetadata|MockObject
44+
*/
45+
private $publicCookieMetadataMock;
46+
47+
/**
48+
* @var CompareCookieManager
49+
*/
50+
private $compareCookieManager;
51+
52+
/**
53+
* Set up test environment
54+
*
55+
* @return void
56+
* @throws Exception
57+
*/
58+
protected function setUp(): void
59+
{
60+
$this->cookieManagerMock = $this->getMockForAbstractClass(CookieManagerInterface::class);
61+
$this->cookieMetadataFactoryMock = $this->createMock(CookieMetadataFactory::class);
62+
$this->loggerMock = $this->getMockForAbstractClass(LoggerInterface::class);
63+
$this->publicCookieMetadataMock = $this->createMock(PublicCookieMetadata::class);
64+
65+
$this->compareCookieManager = new CompareCookieManager(
66+
$this->cookieManagerMock,
67+
$this->cookieMetadataFactoryMock,
68+
$this->loggerMock
69+
);
70+
}
71+
72+
/**
73+
* Test invalidate method successfully sets cookie
74+
*
75+
* @return void
76+
*/
77+
public function testInvalidateSuccess(): void
78+
{
79+
$cookieValue = json_encode(['compare-products' => time()]);
80+
81+
$this->cookieMetadataFactoryMock->expects($this->once())
82+
->method('createPublicCookieMetadata')
83+
->willReturn($this->publicCookieMetadataMock);
84+
85+
$this->publicCookieMetadataMock->expects($this->once())
86+
->method('setDuration')
87+
->with(CompareCookieManager::COOKIE_LIFETIME)
88+
->willReturnSelf();
89+
90+
$this->publicCookieMetadataMock->expects($this->once())
91+
->method('setPath')
92+
->with(CompareCookieManager::COOKIE_PATH)
93+
->willReturnSelf();
94+
95+
$this->publicCookieMetadataMock->expects($this->once())
96+
->method('setHttpOnly')
97+
->with(false)
98+
->willReturnSelf();
99+
100+
$this->cookieManagerMock->expects($this->once())
101+
->method('setPublicCookie')
102+
->with(
103+
CompareCookieManager::COOKIE_COMPARE_PRODUCTS,
104+
$this->callback(function ($value) {
105+
$decodedValue = json_decode($value, true);
106+
return isset($decodedValue['compare-products']) && is_int($decodedValue['compare-products']);
107+
}),
108+
$this->publicCookieMetadataMock
109+
);
110+
111+
$this->compareCookieManager->invalidate();
112+
}
113+
114+
/**
115+
* Test invalidate method logs exception when cookie setting fails
116+
*
117+
* @return void
118+
*/
119+
public function testInvalidateWithException(): void
120+
{
121+
$exception = new InputException(__('Error setting cookie'));
122+
123+
$this->cookieMetadataFactoryMock->expects($this->once())
124+
->method('createPublicCookieMetadata')
125+
->willReturn($this->publicCookieMetadataMock);
126+
127+
$this->publicCookieMetadataMock->expects($this->once())
128+
->method('setDuration')
129+
->willReturnSelf();
130+
131+
$this->publicCookieMetadataMock->expects($this->once())
132+
->method('setPath')
133+
->willReturnSelf();
134+
135+
$this->publicCookieMetadataMock->expects($this->once())
136+
->method('setHttpOnly')
137+
->willReturnSelf();
138+
139+
$this->cookieManagerMock->expects($this->once())
140+
->method('setPublicCookie')
141+
->willThrowException($exception);
142+
143+
$this->loggerMock->expects($this->once())
144+
->method('error')
145+
->with($this->stringContains('Error invalidating compare products cookie'));
146+
147+
$this->compareCookieManager->invalidate();
148+
}
149+
150+
/**
151+
* Test cookie creation with CookieSizeLimitReachedException
152+
*
153+
* @return void
154+
*/
155+
public function testInvalidateWithCookieSizeLimitReachedException(): void
156+
{
157+
$exception = new CookieSizeLimitReachedException(__('Cookie size limit reached'));
158+
159+
$this->cookieMetadataFactoryMock->expects($this->once())
160+
->method('createPublicCookieMetadata')
161+
->willReturn($this->publicCookieMetadataMock);
162+
163+
$this->publicCookieMetadataMock->expects($this->once())
164+
->method('setDuration')
165+
->willReturnSelf();
166+
167+
$this->publicCookieMetadataMock->expects($this->once())
168+
->method('setPath')
169+
->willReturnSelf();
170+
171+
$this->publicCookieMetadataMock->expects($this->once())
172+
->method('setHttpOnly')
173+
->willReturnSelf();
174+
175+
$this->cookieManagerMock->expects($this->once())
176+
->method('setPublicCookie')
177+
->willThrowException($exception);
178+
179+
$this->loggerMock->expects($this->once())
180+
->method('error')
181+
->with($this->stringContains('Error invalidating compare products cookie'));
182+
183+
$this->compareCookieManager->invalidate();
184+
}
185+
186+
/**
187+
* Test cookie creation with FailureToSendException
188+
*
189+
* @return void
190+
*/
191+
public function testInvalidateWithFailureToSendException(): void
192+
{
193+
$exception = new FailureToSendException(__('Failed to send cookie'));
194+
195+
$this->cookieMetadataFactoryMock->expects($this->once())
196+
->method('createPublicCookieMetadata')
197+
->willReturn($this->publicCookieMetadataMock);
198+
199+
$this->publicCookieMetadataMock->expects($this->once())
200+
->method('setDuration')
201+
->willReturnSelf();
202+
203+
$this->publicCookieMetadataMock->expects($this->once())
204+
->method('setPath')
205+
->willReturnSelf();
206+
207+
$this->publicCookieMetadataMock->expects($this->once())
208+
->method('setHttpOnly')
209+
->willReturnSelf();
210+
211+
$this->cookieManagerMock->expects($this->once())
212+
->method('setPublicCookie')
213+
->willThrowException($exception);
214+
215+
$this->loggerMock->expects($this->once())
216+
->method('error')
217+
->with($this->stringContains('Error invalidating compare products cookie'));
218+
219+
$this->compareCookieManager->invalidate();
220+
}
221+
222+
/**
223+
* Test that constants have the expected values
224+
*
225+
* @return void
226+
*/
227+
public function testConstants(): void
228+
{
229+
$this->assertEquals('section_data_ids', CompareCookieManager::COOKIE_COMPARE_PRODUCTS);
230+
$this->assertEquals('/', CompareCookieManager::COOKIE_PATH);
231+
$this->assertEquals(86400, CompareCookieManager::COOKIE_LIFETIME);
232+
}
233+
}

0 commit comments

Comments
 (0)