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

[16.0] [MIG] sale_payment_sheet #2554

Closed
wants to merge 20 commits into from

Conversation

angelmoya
Copy link
Member

No description provided.

@angelmoya angelmoya force-pushed the 16.0-mig-sale_payment_sheet branch from bc00b9e to f749eb3 Compare June 6, 2023 14:00
Copy link
Member

@FrancoMaxime FrancoMaxime left a comment

Choose a reason for hiding this comment

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

Hi, code LGTM, minors comment

sale_payment_sheet/__manifest__.py Outdated Show resolved Hide resolved
sale_payment_sheet/tests/test_sale_payment_sheet.py Outdated Show resolved Hide resolved
@angelmoya angelmoya force-pushed the 16.0-mig-sale_payment_sheet branch from f749eb3 to 0d2fb7f Compare June 7, 2023 07:38
@angelmoya angelmoya force-pushed the 16.0-mig-sale_payment_sheet branch from 0d2fb7f to 8b79c7a Compare June 7, 2023 11:41
Copy link
Member

@FrancoMaxime FrancoMaxime left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Please squash your last commit with the migration one, indicating that it also improves tests, or put the version change in the migration commit if you want to preserve both commits.

@@ -60,18 +61,30 @@ def _create_invoice(self):
line_form.price_unit = 100.00
return invoice_form.save()

def _create_refund(self):
with Form(
Copy link
Member

Choose a reason for hiding this comment

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

Don't do with here, as you are saving 2 times with it (one exiting the context and the other on the explicit call).

Copy link
Contributor

Choose a reason for hiding this comment

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

@rousseldenis
Copy link
Contributor

/ocabot migration sale_payment_sheet

@pedrobaeza
Copy link
Member

@carolinafernandez-tecnativa please supersede this PR fixing my last commit to proceed with the merge.

@pedrobaeza
Copy link
Member

Superseded by #2699

@pedrobaeza pedrobaeza closed this Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants