-
-
Notifications
You must be signed in to change notification settings - Fork 577
[18.0][MIG] account_payment_order_return #1436
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
[18.0][MIG] account_payment_order_return #1436
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor change 👍 . I'd also add in a separate commit the reference to the dependency at OCA/account-payment#792, as seen here in the documentation: https://github.com/OCA/maintainer-tools/wiki/Use-temporary-reference%28s%29-to-another-pull-request%28s%29
| (4, self.invoice.journal_id.id), | ||
| ], | ||
| "partner_ids": [(4, self.partner_a.id)], | ||
| "allow_blocked": True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "allow_blocked": True, |
This field was removed in the migration of the dependency account_payment_order, here: 43c010f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, Thanks
02a0869 to
fac3a87
Compare
|
Hi @JasminSForgeFlow, in order to make testing possible, please add the dependency (account-payment/#792) in the test-requirements in a separate commit, as described in https://github.com/OCA/maintainer-tools/wiki/Use-temporary-reference%28s%29-to-another-pull-request%28s%29 |
9409119 to
72cf94c
Compare
Done, thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
test-requirements.txt
Outdated
| @@ -0,0 +1 @@ | |||
| odoo-addon-account_payment_return@git+https://github.com/OCA/account-payment.git@refs/pull/792/head#subdirectory=account_payment_return | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| odoo-addon-account_payment_return@git+https://github.com/OCA/account-payment.git@refs/pull/792/head#subdirectory=account_payment_return | |
| odoo-addon-account_payment_return@git+https://github.com/OCA/account-payment.git@refs/pull/833/head#subdirectory=account_payment_return |
Hi @JasminSForgeFlow. It seems that this is the dependency (OCA/account-payment#833) that's active now. The other PR is closed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, Thanks
72cf94c to
67ad6f1
Compare
|
Hi @JasminSForgeFlow, I see that account_payment_return is merged now. You can now rebase and remove the commit [DON'T MERGE] test-requirements.txt, so that all the tests can be in green |
67ad6f1 to
62da763
Compare
Done, Thanks |
:100644 100644 d46b131 a23b471 M account_payment_order_return/README.rst :100644 100644 340d1c7 1235bcb M account_payment_order_return/__manifest__.py :100644 100644 a341cc9 be7d7bb M account_payment_order_return/i18n/account_payment_order_return.pot :100644 100644 60be6ab e6b32b3 M account_payment_order_return/readme/CONTRIBUTORS.rst :100644 100644 a8591d0 826c26d M account_payment_order_return/readme/USAGE.rst :100644 100644 450f2d9 a5f5d97 M account_payment_order_return/static/description/index.html
fix domain in payment_order_return in order to avoid problems with moves without associated invoice as discused at OCA#718
Don't post the invoice if it is already posted. See odoo/odoo@80c2818
When the test hour is around midnight, and demo data using CET/CEST timezones, current code fails due to some asserts not having this into consideration. This fix avoids such problem, and irons the execution no matter the hour.
- Include context keys for avoiding mail operations overhead.
Currently translated at 100.0% (2 of 2 strings) Translation: bank-payment-16.0/bank-payment-16.0-account_payment_order_return Translate-URL: https://translation.odoo-community.org/projects/bank-payment-16-0/bank-payment-16-0-account_payment_order_return/it/
62da763 to
a632115
Compare
| cls.env = cls.env( | ||
| context=dict( | ||
| cls.env.context, | ||
| mail_create_nolog=True, | ||
| mail_create_nosubscribe=True, | ||
| mail_notrack=True, | ||
| no_reset_password=True, | ||
| tracking_disable=True, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please import this from https://github.com/odoo/odoo/blob/de678f78869d6eec894f36662e19c2bae1355610/odoo/addons/base/tests/common.py#L11C1-L11C22
from odoo.addons.base.tests.common import DISABLED_MAIL_CONTEXT
| "invoice_line_ids": [ | ||
| ( | ||
| 0, | ||
| 0, | ||
| { | ||
| "product_id": cls.product_a.id, | ||
| "price_unit": 1000.0, | ||
| "tax_ids": [], | ||
| }, | ||
| ) | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "invoice_line_ids": [ | |
| ( | |
| 0, | |
| 0, | |
| { | |
| "product_id": cls.product_a.id, | |
| "price_unit": 1000.0, | |
| "tax_ids": [], | |
| }, | |
| ) | |
| ], | |
| "invoice_line_ids": [ | |
| Command.create( | |
| { | |
| "product_id": cls.product_a.id, | |
| "price_unit": 1000.0, | |
| "tax_ids": [], | |
| }, | |
| ) | |
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented your suggestions @carlos-lopez-tecnativa. Thanks for the review! :)
a632115 to
d482412
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
TT54089
@pedrobaeza could you please review this?
|
This PR has the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
18.0 migration diff reviewed.
/ocabot migration account_payment_order_return
/ocabot merge nobump
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at 2af41f0. Thanks a lot for contributing to OCA. ❤️ |
Standard Migration
Depends on:
@ForgeFlow