Skip to content
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

Jump to bottom

[ProductBundle] Fix default repository for sylius product association type #10379

Closed
wants to merge 4 commits into from
Closed

[ProductBundle] Fix default repository for sylius product association type #10379

wants to merge 4 commits into from

Conversation

loic425
Copy link
Member

@loic425 loic425 commented May 13, 2019

Q A
Branch? 1.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets mentioned in #10350
License MIT

@loic425 loic425 requested a review from a team as a code owner May 13, 2019 09:28
@loic425 loic425 changed the title Fix default repository for sylius product association type [ProductBundle] Fix default repository for sylius product association type May 13, 2019
@Zales0123 Zales0123 added the Potential Bug Potential bugs or bugfixes, that needs to be reproduced. label May 14, 2019
@lchrusciel
Copy link
Member

The base of this pull-request was changed, you need fetch and reset your local branch
if you want to add new commits to this pull request. Reset before you pull, else commits
may become messed-up.

Unless you added new commits (to this branch) locally that you did not push yet,
execute git fetch origin && git reset "fix/default-repositories" to update your local branch.

Feel free to ask for assistance when you get stuck 👍

@lchrusciel lchrusciel changed the base branch from master to 1.3 May 15, 2019 06:49
@lchrusciel lchrusciel force-pushed the fix/default-repositories branch from 92946e1 to ca57b7b Compare May 15, 2019 06:49
@lchrusciel
Copy link
Member

Can you rebase one more time to 1.3? #10386 has a fix for the build

@loic425 loic425 force-pushed the fix/default-repositories branch from 467d6ba to 3c52a52 Compare May 15, 2019 12:01
@loic425
Copy link
Member Author

loic425 commented May 15, 2019

Can you rebase one more time to 1.3? #10386 has a fix for the build

Done

oallain
oallain approved these changes May 15, 2019
View reviewed changes
Copy link
Member

@oallain oallain left a comment

Choose a reason for hiding this comment

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

👍

Zales0123
Zales0123 reviewed May 16, 2019
View reviewed changes
src/Sylius/Bundle/ProductBundle/Resources/config/services/integrations/doctrine/orm.xml
@@ -14,6 +14,7 @@
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
<parameters>
<parameter key="sylius.repository.product.class">Sylius\Bundle\ProductBundle\Doctrine\ORM\ProductRepository</parameter>
<parameter key="sylius.repository.product_association_type.class">Sylius\Bundle\ProductBundle\Doctrine\ORM\ProductAssociationTypeRepository</parameter>
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we then remove this configuration? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

@lchrusciel @GSadee Ok with that ?

Copy link
Member

Choose a reason for hiding this comment

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

we should :)

Copy link
Member Author

@loic425 loic425 May 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@lchrusciel @Zales0123
This doesn't work... we forgot something...
This line returns EntityRepository.

However this following command

bin/console debug:container | grep repository.product_association_type

returns
sylius.repository.product_association_type Sylius\Bundle\ProductBundle\Doctrine\ORM\ProductAssociationTypeRepository
sylius.repository.product_association_type_translation Sylius\Bundle\ResourceBundle\Doctrine\ORM\EntityRepository

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, this is the right repository on ResourceController.

@stale
Copy link

stale bot commented May 30, 2020

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Issues and PRs with no recent activity, about to be closed soon. label May 30, 2020
@stale stale bot closed this Jun 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lchrusciel lchrusciel lchrusciel approved these changes

@Zales0123 Zales0123 Zales0123 left review comments

@Schnax Schnax Schnax approved these changes

@oallain oallain oallain approved these changes

@GSadee GSadee GSadee approved these changes

Assignees
No one assigned
Labels
Potential Bug Potential bugs or bugfixes, that needs to be reproduced. Stale Issues and PRs with no recent activity, about to be closed soon.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@loic425 @lchrusciel @GSadee @Zales0123 @oallain @Schnax