[Association] Associated products #3165
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| public function thereAreFollowingAssociationTypes(TableNode $table) | ||
| { | ||
| foreach ($table->getHash() as $row) { | ||
| $associationType = new AssociationType(); |
There was a problem hiding this comment.
We should use the factory method here.
lchrusciel
changed the title
| ->createQueryBuilder('p') | ||
| ->setMaxResults(1) | ||
| ->getQuery() | ||
| ->getOneOrNullResult() |
There was a problem hiding this comment.
Either setMaxResults(1) or getOneOrNullResult()
lchrusciel
changed the title
| */ | ||
| class AssociationInheritanceMetadataSubscriber implements EventSubscriber | ||
| { | ||
| const ASSOCIATION_CLASS_NAME = 'Sylius\Component\Association\Model\Association'; |
lchrusciel
force-pushed
the
associated-products
branch
from
3360843
to
17919de
Compare
| /** | ||
| * @author Łukasz Chruściel <lukasz.chrusciel@lakion.com> | ||
| */ | ||
| interface AssociatableInterface |
There was a problem hiding this comment.
I think Associable without ta is correct! ?
There was a problem hiding this comment.
Yes it is indeed associable.
lchrusciel
force-pushed
the
associated-products
branch
3 times, most recently
from
dd9ce5e
to
73f1cba
Compare
| @@ -21,6 +21,10 @@ Feature: Products | |||
| | Black T-Shirt | 19.99 | T-Shirt size | T-Shirt fabric:Cotton | | |||
| | Mug | 5.99 | | | | |||
| | Sticker | 10.00 | | | | |||
| And there are following association types: | |||
| | name | code | | |||
| | Cross sell | PAs1 | | |||
There was a problem hiding this comment.
Can we automatically generate the code as it doesn't have any specific role in the scenarios?
There was a problem hiding this comment.
The convention in a file is to write them explicitly. I think that it will be better to be consistent.
lchrusciel
force-pushed
the
associated-products
branch
from
73f1cba
to
d0a029e
Compare
|
All review comments were applied. Waiting for green and it is ready to be merged. |
| */ | ||
| private function createSubjectMapping($associationEntity, $subject, ClassMetadata $associationEntityMetadata) | ||
| { | ||
| $subjectMapping = ['fieldName' => 'owner', 'targetEntity' => $associationEntity, 'inversedBy' => 'associations', 'joinColumns' => [['name' => $subject . '_id', 'referencedColumnName' => $associationEntityMetadata->fieldMappings['id']['columnName'], 'nullable' => false, 'onDelete' => 'CASCADE',]],]; |
lchrusciel
force-pushed
the
associated-products
branch
2 times, most recently
from
4eb76b4
to
0b92f1a
Compare
lchrusciel
force-pushed
the
associated-products
branch
from
0b92f1a
to
a18726e
Compare
pjedrzejewski
pushed a commit
that referenced
this pull request
Jan 29, 2016
[Association] Associated products
|
Great work, thanks Łukasz and all reviewers! 👍 I will setup repositories later today. :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is extended #2098 PR. @l3l0 has done awsome work so I only extracted it to separated bundle.
This PR was reviewed once, but feel free to point me out more problems ;)
Update
Important change
Due to extraction to different budnle the previous configuration does not apply any more.
Previous config:
After feedback it was decided to change a configuration to more dynamic. Current version is inspired by AttributesBundle configuration.
Now it has to be done in separated tree, where you have to precise a subject of association. The most basic config looks like that:
Because of assigning this PR to v0.16 milestone it was associated with @pamil #3110 PR
TODOs:
Update
Since last time I have worked on this PR plenty things changed in Sylius core, so new dynamic configuration has to be adjusted to them. For now, new association configuration looks like this: