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

Fix GenerateInvoicesCommand selects order without number #148

Merged
merged 1 commit into from
Oct 15, 2019
Merged

Fix GenerateInvoicesCommand selects order without number #148

merged 1 commit into from
Oct 15, 2019

Conversation

tom10271
Copy link
Contributor

Supposedly those are cart orders and should not be selected for invoice generation

Copy link

@firstred firstred left a comment

Choose a reason for hiding this comment

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

I can confirm these changes solved the problem for me 😄

Thanks @tom10271 !

@tom10271
Copy link
Contributor Author

There is still one more thing to fix, I called createQueryBuilder which lived in EntityRepository, but the type set for $orderRepository is OrderRepositoryInterfacemaking CI cannot pass.

Which interface or class should we set?

@Zales0123 Zales0123 added the Bug Confirmed bugs or bugfixes. label Aug 12, 2019
@Zales0123
Copy link
Member

The best solution would be creating a custom OrderRepository, but it requires a custom interface, class, and configuration, which is probably not something we would like to introduce in such a simple fix :/ Maybe you can use one of the existing methods from order repository, like createListQueryBuilder, and then apply the not-null-number criteria on the result? It should do the trick with not much work. Thanks for the contribution!

@tom10271
Copy link
Contributor Author

I have updated the commit. Please check again. Thank you everyone

@Zales0123
Copy link
Member

👍 The last thing that should be done is adding a not-placed order in the background of this feature file to check does this query for sure works as it should work :) Do you think you would be able to do it, @tom10271?

@pamil
Copy link
Contributor

pamil commented Oct 15, 2019

Thank you, @tom10271! 🥇

@diimpp
Copy link
Member

diimpp commented Feb 20, 2020

That's a might weird solution. Why not check for Order->paymentState instead? Unpaid/cancelled orders have number, but don't qualify to have invoice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bugs or bugfixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants