Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions InventoryCatalog/Model/GetStockIdForCurrentWebsite.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
namespace Magento\InventoryCatalog\Model;

use Magento\Framework\App\ObjectManager;
use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Framework\HTTP\PhpEnvironment\Request;
use Magento\InventorySalesApi\Api\Data\SalesChannelInterface;
use Magento\InventorySalesApi\Api\StockResolverInterface;
use Magento\Store\Model\StoreManagerInterface;
use Magento\Framework\HTTP\PhpEnvironment\Request;

/**
* Service for get stock id for current website.
Expand Down Expand Up @@ -56,7 +57,11 @@ public function __construct(
public function execute(): int
{
$storeId = $this->request->getParam('store');
$websiteId = $this->storeManager->getStore($storeId)->getWebsiteId();
try {
$websiteId = $this->storeManager->getStore($storeId)->getWebsiteId();
} catch (NoSuchEntityException $e) {
$websiteId = $this->storeManager->getStore()->getWebsiteId();
}
$websiteCode = $this->storeManager->getWebsite($websiteId)->getCode();

$stock = $this->stockResolver->execute(SalesChannelInterface::TYPE_WEBSITE, $websiteCode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

namespace Magento\InventoryCatalog\Test\Unit\Model;

use Magento\Framework\Exception\NoSuchEntityException;
use Magento\Framework\HTTP\PhpEnvironment\Request;
use Magento\InventoryApi\Api\Data\StockInterface;
use Magento\InventoryCatalog\Model\GetStockIdForCurrentWebsite;
Expand Down Expand Up @@ -95,4 +96,52 @@ public function testExecute(): void

$this->assertSame($stockId, $this->model->execute());
}

/**
* @return void
*/
public function testExecuteWithInvalidStoreFallsBackToCurrentStore(): void
{
$invalidStoreCode = 'noneExistent';
$websiteId = 1;
$websiteCode = 'base';
$stockId = 1;

$this->request->expects($this->once())->method('getParam')
->with('store')
->willReturn($invalidStoreCode);

$currentStore = $this->createMock(StoreInterface::class);
$currentStore->expects($this->once())
->method('getWebsiteId')
->willReturn($websiteId);

$this->storeManager->expects($this->exactly(2))
->method('getStore')
->willReturnCallback(function ($storeId = null) use ($invalidStoreCode, $currentStore) {
if ($storeId === $invalidStoreCode) {
throw new NoSuchEntityException(__('The store that was requested wasn\'t found.'));
}
return $currentStore;
});

$website = $this->createMock(WebsiteInterface::class);
$website->expects($this->once())
->method('getCode')
->willReturn($websiteCode);
$this->storeManager->expects($this->once())
->method('getWebsite')
->with($websiteId)
->willReturn($website);

$stock = $this->createMock(StockInterface::class);
$stock->expects($this->once())->method('getStockId')
->willReturn($stockId);
$this->stockResolver->expects($this->once())
->method('execute')
->with(SalesChannelInterface::TYPE_WEBSITE, $websiteCode)
->willReturn($stock);

$this->assertSame($stockId, $this->model->execute());
}
}
18 changes: 16 additions & 2 deletions InventoryInStorePickup/Model/ExtractPickupLocationAddressData.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace Magento\InventoryInStorePickup\Model;

use Magento\Directory\Model\RegionFactory;
use Magento\Directory\Model\ResourceModel\Region as RegionResource;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\DataObject\Copy;
use Magento\InventoryApi\Api\Data\SourceInterface;
Expand All @@ -28,6 +29,11 @@ class ExtractPickupLocationAddressData
*/
private $regionFactory;

/**
* @var RegionResource
*/
private $regionResource;

/**
* @var array
*/
Expand All @@ -36,14 +42,18 @@ class ExtractPickupLocationAddressData
/**
* @param Copy $copyService
* @param RegionFactory|null $regionFactory
* @param RegionResource|null $regionResource
*/
public function __construct(
Copy $copyService,
?RegionFactory $regionFactory = null
?RegionFactory $regionFactory = null,
?RegionResource $regionResource = null
) {
$this->objectCopyService = $copyService;
$this->regionFactory = $regionFactory ?:
ObjectManager::getInstance()->get(RegionFactory::class);
$this->regionResource = $regionResource ?:
ObjectManager::getInstance()->get(RegionResource::class);
}

/**
Expand Down Expand Up @@ -78,7 +88,11 @@ private function retrieveRegion(PickupLocationInterface $pickupLocation, array $

if (!isset($this->regions[$cacheKey])) {
$region = $this->regionFactory->create();
$region->loadByName($pickupLocation->getRegion(), $pickupLocation->getCountryId());
if ($pickupLocation->getRegionId()) {
$this->regionResource->load($region, $pickupLocation->getRegionId());
} else {
$region->loadByName($pickupLocation->getRegion(), $pickupLocation->getCountryId());
}
$this->regions[$cacheKey] = $region->getName() ?: $pickupLocation->getRegion();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use Magento\Directory\Model\Region;
use Magento\Directory\Model\RegionFactory;
use Magento\Directory\Model\ResourceModel\Region as RegionResource;
use Magento\Framework\DataObject\Copy;
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
use Magento\InventoryInStorePickup\Model\ExtractPickupLocationAddressData;
Expand Down Expand Up @@ -36,6 +37,11 @@ class ExtractPickupLocationAddressDataTest extends TestCase
*/
private $regionMock;

/**
* @var RegionResource|MockObject
*/
private $regionResourceMock;

/**
* @inheritDoc
*/
Expand All @@ -50,6 +56,12 @@ protected function setUp(): void
->getMock();
$this->regionMock->method('loadByName')->willReturnSelf();

$this->regionResourceMock = $this->getMockBuilder(RegionResource::class)
->disableOriginalConstructor()
->onlyMethods(['load'])
->getMock();
$this->regionResourceMock->method('load')->willReturnSelf();

$this->objectCopyServiceMock = $this->getMockBuilder(Copy::class)
->disableOriginalConstructor()
->onlyMethods(['getDataFromFieldset'])
Expand All @@ -68,6 +80,7 @@ protected function setUp(): void
[
'objectCopyService' => $this->objectCopyServiceMock,
'regionFactory' => $regionFactoryMock,
'regionResource' => $this->regionResourceMock,
]
);
}
Expand Down Expand Up @@ -117,4 +130,81 @@ public static function executeDataProvider(): array
],
];
}

/**
* When region_id is set, region is loaded via ResourceModel by ID and loadByName is not called.
*/
public function testExecuteLoadsRegionByIdWhenRegionIdIsSet(): void
{
$this->objectCopyServiceMock->method('getDataFromFieldset')
->willReturn(['region' => 'stale_name']);
$this->regionMock->method('getName')->willReturn('current_name');

$pickupLocation = $this->createMock(PickupLocation::class);
$pickupLocation->method('getCountryId')->willReturn('CH');
$pickupLocation->method('getRegionId')->willReturn(42);
$pickupLocation->method('getRegion')->willReturn('stale_name');

$this->regionResourceMock->expects($this->once())
->method('load')
->with($this->regionMock, 42);
$this->regionMock->expects($this->never())->method('loadByName');

$result = $this->model->execute($pickupLocation);

$this->assertEquals(['region' => 'current_name'], $result);
}

/**
* When region_id is null, region is resolved via loadByName and ResourceModel load is not called.
*/
public function testExecuteFallsBackToLoadByNameWhenRegionIdIsNull(): void
{
$this->objectCopyServiceMock->method('getDataFromFieldset')
->willReturn(['region' => 'some_region']);
$this->regionMock->method('getName')->willReturn('some_region');

$pickupLocation = $this->createMock(PickupLocation::class);
$pickupLocation->method('getCountryId')->willReturn('US');
$pickupLocation->method('getRegionId')->willReturn(null);
$pickupLocation->method('getRegion')->willReturn('some_region');

$this->regionResourceMock->expects($this->never())->method('load');
$this->regionMock->expects($this->once())
->method('loadByName')
->with('some_region', 'US')
->willReturnSelf();

$result = $this->model->execute($pickupLocation);

$this->assertEquals(['region' => 'some_region'], $result);
}

/**
* After a Directory data patch renames a subdivision, the current name is resolved
* live from Directory by region_id, not from the stale string in inventory_source.region.
*/
public function testExecuteReturnsCurrentRegionNameAfterDirectoryRename(): void
{
// inventory_source.region still holds the old name from before the data patch
$this->objectCopyServiceMock->method('getDataFromFieldset')
->willReturn(['region' => 'Friburg']);

// ResourceModel returns the updated name after the rename
$this->regionMock->method('getName')->willReturn('Renamed Friburg');

$pickupLocation = $this->createMock(PickupLocation::class);
$pickupLocation->method('getCountryId')->willReturn('CH');
$pickupLocation->method('getRegionId')->willReturn(42);
$pickupLocation->method('getRegion')->willReturn('Friburg');

$this->regionResourceMock->expects($this->once())
->method('load')
->with($this->regionMock, 42);

$result = $this->model->execute($pickupLocation);

// Must reflect the live Directory name, not the stale inventory_source value
$this->assertEquals(['region' => 'Renamed Friburg'], $result);
}
}
Loading