Skip to content

#11936:required attribute set id filter on attribute group repository getList #12105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 22, 2017
Merged

#11936:required attribute set id filter on attribute group repository getList #12105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
433c3cd
#11936:required attribute set id filter on attribute group repository…
tzyganu Nov 8, 2017
7be92a8
mark method retrieveAttributeSetIdFromSearchCriteria as deprecated
tzyganu Dec 20, 2017
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
11 changes: 1 addition & 10 deletions app/code/Magento/Eav/Model/Attribute/GroupRepository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,6 @@ public function save(\Magento\Eav\Api\Data\AttributeGroupInterface $group)
*/
public function getList(\Magento\Framework\Api\SearchCriteriaInterface $searchCriteria)
{
$attributeSetId = $this->retrieveAttributeSetIdFromSearchCriteria($searchCriteria);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maghamed Since you requested the attribute set to be optional in #11936 (comment): are you fine with it being removed completely?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @mzeis and @tzyganu
I am fine with removing this check,
The only thing we need to add here is to mark method
protected function retrieveAttributeSetIdFromSearchCriteria(
as @deprecated because we no need it any more, but because it's protected we can't just remove it

if (!$attributeSetId) {
throw InputException::requiredField('attribute_set_id');
}
try {
$this->setRepository->get($attributeSetId);
} catch (\Exception $exception) {
throw NoSuchEntityException::singleField('attributeSetId', $attributeSetId);
}

/** @var \Magento\Eav\Model\ResourceModel\Entity\Attribute\Group\Collection $collection */
$collection = $this->groupListFactory->create();
$this->joinProcessor->process($collection);
Expand Down Expand Up @@ -188,6 +178,7 @@ public function deleteById($groupId)
/**
* @param \Magento\Framework\Api\SearchCriteriaInterface $searchCriteria
* @return null|string
* @deprecated
*/
protected function retrieveAttributeSetIdFromSearchCriteria(
\Magento\Framework\Api\SearchCriteriaInterface $searchCriteria
Expand Down
58 changes: 2 additions & 56 deletions app/code/Magento/Eav/Test/Unit/Model/Attribute/GroupRepositoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,33 +266,25 @@ public function testSaveThrowExceptionIfProvidedGroupDoesNotExist()
*/
public function testGetList()
{
$attributeSetId = 'filter';

$filterInterfaceMock = $this->getMockBuilder(\Magento\Framework\Api\Search\FilterGroup::class)
->disableOriginalConstructor()
->setMethods([
'getField',
'getValue',
])
->getMock();
$filterInterfaceMock->expects($this->once())
->method('getField')
->willReturn('attribute_set_id');
$filterInterfaceMock->expects($this->once())
->method('getValue')
->willReturn($attributeSetId);

$filterGroupMock = $this->getMockBuilder(\Magento\Framework\Api\Search\FilterGroup::class)
->disableOriginalConstructor()
->getMock();
$filterGroupMock->expects($this->once())
$filterGroupMock->expects($this->any())
->method('getFilters')
->willReturn([$filterInterfaceMock]);

$searchCriteriaMock = $this->getMockBuilder(\Magento\Framework\Api\SearchCriteriaInterface::class)
->disableOriginalConstructor()
->getMock();
$searchCriteriaMock->expects($this->once())
$searchCriteriaMock->expects($this->any())
->method('getFilterGroups')
->willReturn([$filterGroupMock]);

Expand Down Expand Up @@ -324,52 +316,6 @@ public function testGetList()
$this->assertEquals($searchResultsMock, $this->model->getList($searchCriteriaMock));
}

/**
* Test get list with invalid input exception
*
* @expectedException \Magento\Framework\Exception\InputException
* @expectedExceptionMessage attribute_set_id is a required field.
* @throws \Magento\Framework\Exception\InputException
* @throws \Magento\Framework\Exception\NoSuchEntityException
* @return void
*/
public function testGetListWithInvalidInputException()
{
$searchCriteriaMock = $this->createMock(\Magento\Framework\Api\SearchCriteriaInterface::class);
$searchCriteriaMock->expects($this->once())->method('getFilterGroups')->willReturn([]);
$this->model->getList($searchCriteriaMock);
}

/**
* Test get list with no such entity exception
*
* @expectedException \Magento\Framework\Exception\NoSuchEntityException
* @expectedExceptionMessage No such entity with attributeSetId = filter
* @throws \Magento\Framework\Exception\InputException
* @throws \Magento\Framework\Exception\NoSuchEntityException
* @return void
*/
public function testGetListWithNoSuchEntityException()
{
$attributeSetId = 'filter';
$searchCriteriaMock = $this->createMock(\Magento\Framework\Api\SearchCriteriaInterface::class);
$filterGroupMock = $this->createMock(\Magento\Framework\Api\Search\FilterGroup::class);
$filterInterfaceMock = $this->createMock(\Magento\Framework\Api\Filter::class);

$searchCriteriaMock->expects($this->once())->method('getFilterGroups')->willReturn([$filterGroupMock]);

$filterGroupMock->expects($this->once())->method('getFilters')->willReturn([$filterInterfaceMock]);
$filterInterfaceMock->expects($this->once())->method('getField')->willReturn('attribute_set_id');
$filterInterfaceMock->expects($this->once())->method('getValue')->willReturn($attributeSetId);

$searchCriteriaMock->expects($this->once())->method('getFilterGroups')->willReturn([]);
$this->setRepositoryMock->expects($this->once())
->method('get')
->with($attributeSetId)
->willThrowException(new \Exception());
$this->model->getList($searchCriteriaMock);
}

/**
* Test get
*
Expand Down