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

Relate Invoice with Order #226

Merged
merged 5 commits into from
Jun 14, 2021
Merged

Conversation

Zales0123
Copy link
Member

Similar to Sylius/RefundPlugin#306 and Sylius/RefundPlugin#307.

Should be easier to operate on the Invoice with a proper relation rather than just an order number

@Zales0123 Zales0123 added the Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). label Jun 11, 2021
@Zales0123 Zales0123 requested a review from a team as a code owner June 11, 2021 08:20
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.

Could you add a note in UPGRADE file?

@Zales0123
Copy link
Member Author

@GSadee UPGRADE file updated 🖖

UPGRADE.md Outdated Show resolved Hide resolved

1. `orderNumber` field on `Sylius\InvoicingPlugin\Entity\Invoice` has been removed and replaced with relation to `Order` entity.
1. `Sylius\InvoicingPlugin\Entity\InvoiceInterface::orderNumber` function is left due to easier and smoother upgrades,
but is also deprecated and will be removed in the `v1.0.0` release. Use `Sylius\InvoicingPlugin\Entity\InvoiceInterface::order` instead.
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 remove that method earlier than v1.0.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

I chose v1.0.0 because I had no better idea which version it should be :D alpha? beta? rc? I think we can rethink that, but also we can just change this decision later, so it does not block the merge 💃

@GSadee GSadee merged commit 1015a34 into Sylius:master Jun 14, 2021
@GSadee
Copy link
Member

GSadee commented Jun 14, 2021

Thank you, Mateusz! 🎉

SirDomin added a commit to Sylius/PayPalPlugin that referenced this pull request Jul 9, 2021
This PR was merged into the 1.0-dev branch.

Discussion
----------

Before [this change](Sylius/InvoicingPlugin#226), it was impossible to place the PayPal order from the Payment page, due to some problems with Invoice generation (missing number on order). Therefore, we should require at least SyliusInvoicingPlugin v0.16.0 and release a new version asap 🖖 🚀 

Commits
-------

5df3399 Conflict with SyliusInvoicingPlugin below 0.16.0
76dfc7c Fix build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants