[Admin][ProductAssociation] Adding associated products to a product #6642
Conversation
GSadee
commented
Nov 7, 2016
GSadee
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Related tickets | - |
| License | MIT |
GSadee
changed the title
GSadee
force-pushed
the
admin-associated-products
branch
from
b8d568e
to
fc0dd4a
Compare
| And I remove an associated product "LG earphones" from "Accessories" | ||
| And I add it | ||
| Then I should be notified that it has been successfully created | ||
| And this product should have an association "Accessories" with product "LG headphones" |
There was a problem hiding this comment.
Missing And this product should not have an association "Accessories" with product "LG earphones"
| And I remove an associated product "LG earphones" from "Accessories" | ||
| And I save my changes | ||
| Then I should be notified that it has been successfully edited | ||
| And this product should have an association "Accessories" with product "LG headphones" |
There was a problem hiding this comment.
And this product should not have an association "Accessories" with product "LG earphones"
| '%association%' => $productAssociationType->getName(), | ||
| '%item%' => $productName, | ||
| ]); | ||
| $item->find('css', 'i.delete')->click(); |
There was a problem hiding this comment.
find can return null we should assert before click
|
@GSadee rebase required. |
GSadee
force-pushed
the
admin-associated-products
branch
from
fc0dd4a
to
f3c6d09
Compare
GSadee
changed the title
GSadee
force-pushed
the
admin-associated-products
branch
2 times, most recently
from
e35193c
to
3286609
Compare
| $this->setProductAssociations($productAssociations); | ||
|
|
||
| if (null === $productAssociations) { | ||
| return null; |
There was a problem hiding this comment.
By convention of DataTransformerInterface, this should be return '';
| /** | ||
| * @Given /^the (product "[^"]+") has(?:| also) an (association "[^"]+") with (product "[^"]+")$/ | ||
| */ | ||
| public function theProductHasAnAssociationWithProduct( |
There was a problem hiding this comment.
This method could be merged with the method below if we would use variadics
GSadee
force-pushed
the
admin-associated-products
branch
from
3286609
to
8a0a3c9
Compare
| ) { | ||
| /** @var UpdateSimpleProductPageInterface|UpdateConfigurableProductPageInterface $currentPage */ | ||
| $currentPage = $this->currentPageResolver->getCurrentPageWithForm([ | ||
| $this->createSimpleProductPage, |
There was a problem hiding this comment.
Do the assosciation work only on simple product page?
There was a problem hiding this comment.
I don't think that testing that also on configurable product page has a business value, because the logic is the same.
There was a problem hiding this comment.
Sure, but we have generic steep, which can be used with both with simple or configurable product.
| /** | ||
| * @var FactoryInterface | ||
| */ | ||
| protected $productAssociationFactory; |
There was a problem hiding this comment.
Class is final so methods should be private
…ociations of a product
… changing associations
GSadee
force-pushed
the
admin-associated-products
branch
from
8a0a3c9
to
a029676
Compare
|
Good job Grzesiu! 👍 |
