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.

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

[Enhancement] Refactor and deprecate Invoice repository class #186

Merged
merged 7 commits into from
Oct 23, 2020

Conversation

Prometee
Copy link
Contributor

@Prometee Prometee commented Oct 15, 2020

The actual Sylius\InvoicingPlugin\Repository\DoctrineInvoiceRepository has been deprecated and a new class has been defined in the src/Doctrine/ORM folder with a name following the entity name.
The corresponding interface has been also deprecated and the new one is owning the only required method.

I propose this PR because the actual repository was not correctly used : it was an override of sylius_invoicing_plugin.custom_repository.invoice instead of a default value in the resource plugin configuration.

@Prometee Prometee requested a review from a team as a code owner October 15, 2020 10:29
@Prometee
Copy link
Contributor Author

@pamil @GSadee I fix the ECS error, but this fix should maybe part of another PR I guess :

[ERROR] Class "Symplify\PackageBuilder\Console\HelpfulApplicationTrait" not found while loading        
         "Symplify\EasyCodingStandard\Console\EasyCodingStandardConsoleApplication".  

Copy link
Member

@GSadee GSadee left a comment

Choose a reason for hiding this comment

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

I've extracted your commit with fix to a separate PR #190 to make the build green again 😃

src/Repository/InvoiceRepository.php Outdated Show resolved Hide resolved
GSadee added a commit that referenced this pull request Oct 22, 2020
…he build (Prometee)

This PR was merged into the 1.0-dev branch.

Discussion
----------

Extracted commit of @Prometee from #186 

Commits
-------

977c71b Fix ecs error
@@ -1,3 +1,11 @@
### UPGRADE FROM 0.11.0 TO 0.11.1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### UPGRADE FROM 0.11.0 TO 0.11.1
### UPGRADE FROM 0.11.0 TO 0.12.0

1. The custom repository has been removed :

- removed the repository class `DoctrineInvoiceRepository` and replaced by `Sylius\InvoicingPlugin\Doctrine\ORM\InvoiceRepository`
- removed the related service `sylius_invoicing_plugin.custom_repository.invoice` use `sylius_invoicing_plugin.repository.invoice` instead
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- removed the related service `sylius_invoicing_plugin.custom_repository.invoice` use `sylius_invoicing_plugin.repository.invoice` instead
- removed the related service `sylius_invoicing_plugin.custom_repository.invoice`, use `sylius_invoicing_plugin.repository.invoice` instead


- removed the repository class `DoctrineInvoiceRepository` and replaced by `Sylius\InvoicingPlugin\Doctrine\ORM\InvoiceRepository`
- removed the related service `sylius_invoicing_plugin.custom_repository.invoice` use `sylius_invoicing_plugin.repository.invoice` instead
- removed the related interface `InvoiceRepository` use `Sylius\InvoicingPlugin\Doctrine\ORM\InvoiceRepositoryInterface` instead
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- removed the related interface `InvoiceRepository` use `Sylius\InvoicingPlugin\Doctrine\ORM\InvoiceRepositoryInterface` instead
- removed the related interface `Sylius\InvoicingPlugin\Repository\InvoiceRepository`, use `Sylius\InvoicingPlugin\Doctrine\ORM\InvoiceRepositoryInterface` instead


1. The custom repository has been removed :

- removed the repository class `DoctrineInvoiceRepository` and replaced by `Sylius\InvoicingPlugin\Doctrine\ORM\InvoiceRepository`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- removed the repository class `DoctrineInvoiceRepository` and replaced by `Sylius\InvoicingPlugin\Doctrine\ORM\InvoiceRepository`
- removed the repository class `Sylius\InvoicingPlugin\Repository\DoctrineInvoiceRepository` and replaced by `Sylius\InvoicingPlugin\Doctrine\ORM\InvoiceRepository`

@GSadee GSadee merged commit 53a586d into Sylius:master Oct 23, 2020
@GSadee
Copy link
Member

GSadee commented Oct 23, 2020

Thanks, Francis! 🎉

lchrusciel added a commit that referenced this pull request Oct 23, 2020
This PR was merged into the 1.0-dev branch.

Discussion
----------

After #186 

Commits
-------

7770810 Update upgrade to v0.12.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants