Skip to content

Commit d1a4f01

Browse files
committed
ACP2E-3493: Expired persistent quotes are not cleaned up by a cron job sales_clean_quotes
- Addressed the CR comments.
1 parent 0ab6797 commit d1a4f01

File tree

4 files changed

+16
-79
lines changed

4 files changed

+16
-79
lines changed

app/code/Magento/Persistent/Model/DeleteExpiredQuote.php renamed to app/code/Magento/Persistent/Model/ResourceModel/DeleteExpiredQuote.php

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55
*/
66
declare(strict_types=1);
77

8-
namespace Magento\Persistent\Model;
8+
namespace Magento\Persistent\Model\ResourceModel;
99

1010
use Magento\Framework\Exception\LocalizedException;
11-
use Magento\Framework\Exception\NoSuchEntityException;
1211
use Magento\Persistent\Helper\Data;
1312
use Magento\Store\Model\StoreManagerInterface;
1413
use Magento\Framework\App\Config\ScopeConfigInterface;
1514
use Magento\Framework\App\ResourceConnection;
15+
use Magento\Store\Model\ScopeInterface;
1616

1717
/**
1818
* Delete expired quote model
@@ -34,23 +34,18 @@ public function __construct(
3434
/**
3535
* Delete expired persistent quote for the website
3636
*
37-
* @param int|null $websiteId
37+
* @param int $websiteId
3838
* @return void
3939
* @throws LocalizedException
40-
* @throws NoSuchEntityException
4140
*/
42-
public function deleteExpiredQuote(?int $websiteId): void
41+
public function deleteExpiredQuote(int $websiteId): void
4342
{
44-
if ($websiteId === null) {
45-
$websiteId = $this->storeManager->getStore()->getWebsiteId();
46-
}
47-
4843
$storeIds = $this->storeManager->getWebsite($websiteId)->getStoreIds();
4944

5045
$lifetime = $this->scopeConfig->getValue(
5146
Data::XML_PATH_LIFE_TIME,
52-
'website',
53-
(int)$websiteId
47+
ScopeInterface::SCOPE_WEBSITE,
48+
$websiteId
5449
);
5550

5651
if ($lifetime) {

app/code/Magento/Persistent/Observer/ClearExpiredCronJobObserver.php

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
namespace Magento\Persistent\Observer;
99

1010
use Magento\Cron\Model\Schedule;
11-
use Magento\Persistent\Model\DeleteExpiredQuoteFactory;
11+
use Magento\Persistent\Model\ResourceModel\DeleteExpiredQuote;
1212
use Magento\Persistent\Model\SessionFactory;
1313
use Magento\Store\Model\ResourceModel\Website\CollectionFactory;
1414

@@ -31,23 +31,23 @@ class ClearExpiredCronJobObserver
3131
/**
3232
* A property for delete expired quote factory
3333
*
34-
* @var DeleteExpiredQuoteFactory
34+
* @var DeleteExpiredQuote
3535
*/
36-
private DeleteExpiredQuoteFactory $deleteExpiredQuoteFactory;
36+
private DeleteExpiredQuote $deleteExpiredQuote;
3737

3838
/**
3939
* @param CollectionFactory $websiteCollectionFactory
4040
* @param SessionFactory $sessionFactory
41-
* @param DeleteExpiredQuoteFactory $deleteExpiredQuoteFactory
41+
* @param DeleteExpiredQuote $deleteExpiredQuote
4242
*/
4343
public function __construct(
4444
CollectionFactory $websiteCollectionFactory,
4545
SessionFactory $sessionFactory,
46-
DeleteExpiredQuoteFactory $deleteExpiredQuoteFactory
46+
DeleteExpiredQuote $deleteExpiredQuote
4747
) {
4848
$this->_websiteCollectionFactory = $websiteCollectionFactory;
4949
$this->_sessionFactory = $sessionFactory;
50-
$this->deleteExpiredQuoteFactory = $deleteExpiredQuoteFactory;
50+
$this->deleteExpiredQuote = $deleteExpiredQuote;
5151
}
5252

5353
/**
@@ -64,10 +64,9 @@ public function execute(Schedule $schedule)
6464
return $this;
6565
}
6666

67-
$deleteExpiredQuote = $this->deleteExpiredQuoteFactory->create();
6867
foreach ($websiteIds as $websiteId) {
6968
$this->_sessionFactory->create()->deleteExpired($websiteId);
70-
$deleteExpiredQuote->deleteExpiredQuote((int) $websiteId);
69+
$this->deleteExpiredQuote->deleteExpiredQuote((int) $websiteId);
7170
}
7271

7372
return $this;

app/code/Magento/Persistent/Test/Unit/Model/DeleteExpiredQuoteTest.php

Lines changed: 1 addition & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
use Magento\Framework\App\Config\ScopeConfigInterface;
1111
use Magento\Framework\App\ResourceConnection;
1212
use Magento\Persistent\Helper\Data;
13-
use Magento\Persistent\Model\DeleteExpiredQuote;
13+
use Magento\Persistent\Model\ResourceModel\DeleteExpiredQuote;
1414
use Magento\Store\Model\StoreManagerInterface;
1515
use Magento\Store\Model\Website;
1616
use Magento\Store\Api\Data\StoreInterface;
@@ -101,49 +101,6 @@ public function testDeleteExpiredQuote(): void
101101
$this->model->deleteExpiredQuote($websiteId);
102102
}
103103

104-
/**
105-
* Test deleting expired quotes when no websiteId is provided.
106-
*/
107-
public function testDeleteExpiredQuoteWithNullWebsiteId(): void
108-
{
109-
// Mock the store manager and store data
110-
$websiteId = 1;
111-
$storeIds = [1, 2];
112-
$websiteMock = $this->createMock(Website::class);
113-
$websiteMock->method('getStoreIds')->willReturn($storeIds);
114-
$this->storeManagerMock->method('getWebsite')->with($websiteId)->willReturn($websiteMock);
115-
$this->storeManagerMock->method('getStore')->willReturn($this->storeMock);
116-
$this->storeMock->method('getWebsiteId')->willReturn($websiteId);
117-
118-
// Mock the scope config to return a specific lifetime
119-
$lifetime = 60; // 1 hour in seconds
120-
$this->scopeConfigMock->method('getValue')
121-
->with(Data::XML_PATH_LIFE_TIME, 'website', $websiteId)
122-
->willReturn($lifetime);
123-
124-
// Mock the database connection
125-
$connectionMock = $this->createMock(\Magento\Framework\DB\Adapter\AdapterInterface::class);
126-
$this->resourceConnectionMock->method('getConnection')->willReturn($connectionMock);
127-
$connectionMock->expects($this->once())
128-
->method('delete')
129-
->with(
130-
$this->resourceConnectionMock->getTableName('quote'),
131-
$this->callback(function ($condition) use ($storeIds, $lifetime) {
132-
// Validate the delete condition
133-
$expiredBefore = gmdate('Y-m-d H:i:s', time() - $lifetime);
134-
return isset($condition['store_id in (?)']) &&
135-
$condition['store_id in (?)'] === $storeIds &&
136-
isset($condition['updated_at < ?']) &&
137-
$condition['updated_at < ?'] === $expiredBefore &&
138-
isset($condition['is_persistent']) &&
139-
$condition['is_persistent'] === 1;
140-
})
141-
);
142-
143-
// Call the method to test with null websiteId
144-
$this->model->deleteExpiredQuote(null);
145-
}
146-
147104
/**
148105
* Test when there is no lifetime configured.
149106
*/

app/code/Magento/Persistent/Test/Unit/Observer/ClearExpiredCronJobObserverTest.php

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
use Magento\Persistent\Observer\ClearExpiredCronJobObserver;
1414
use Magento\Store\Model\ResourceModel\Website\Collection;
1515
use Magento\Store\Model\ResourceModel\Website\CollectionFactory;
16-
use Magento\Persistent\Model\DeleteExpiredQuote;
17-
use Magento\Persistent\Model\DeleteExpiredQuoteFactory;
16+
use Magento\Persistent\Model\ResourceModel\DeleteExpiredQuote;
1817
use PHPUnit\Framework\MockObject\MockObject;
1918
use PHPUnit\Framework\TestCase;
2019

@@ -35,11 +34,6 @@ class ClearExpiredCronJobObserverTest extends TestCase
3534
*/
3635
protected $sessionFactoryMock;
3736

38-
/**
39-
* @var MockObject
40-
*/
41-
protected $deleteExpiredQuoteFactoryMock;
42-
4337
/**
4438
* @var MockObject
4539
*/
@@ -68,10 +62,6 @@ protected function setUp(): void
6862
SessionFactory::class,
6963
['create']
7064
);
71-
$this->deleteExpiredQuoteFactoryMock = $this->createPartialMock(
72-
DeleteExpiredQuoteFactory::class,
73-
['create']
74-
);
7565
$this->scheduleMock = $this->createMock(Schedule::class);
7666
$this->sessionMock = $this->createMock(Session::class);
7767
$this->websiteCollectionMock
@@ -82,7 +72,7 @@ protected function setUp(): void
8272
$this->model = new ClearExpiredCronJobObserver(
8373
$this->collectionFactoryMock,
8474
$this->sessionFactoryMock,
85-
$this->deleteExpiredQuoteFactoryMock
75+
$this->deleteExpiredQuoteMock
8676
);
8777
}
8878

@@ -97,10 +87,6 @@ public function testExecute()
9787
->expects($this->once())
9888
->method('create')
9989
->willReturn($this->sessionMock);
100-
$this->deleteExpiredQuoteFactoryMock
101-
->expects($this->once())
102-
->method('create')
103-
->willReturn($this->deleteExpiredQuoteMock);
10490
$this->sessionMock->expects($this->once())->method('deleteExpired')->with(1);
10591
$this->deleteExpiredQuoteMock->expects($this->once())->method('deleteExpiredQuote')->with(1);
10692
$this->model->execute($this->scheduleMock);

0 commit comments

Comments
 (0)