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

[15.0][MIG] stock_picking_invoicing: Migration to 15.0 #1452

Merged
merged 123 commits into from
May 20, 2023

Conversation

mbcosta
Copy link
Contributor

@mbcosta mbcosta commented Apr 28, 2023

Standard migration of stock_picking_invoicing module, #1022

cc @rvalyi @renatonlima

@mbcosta mbcosta force-pushed the 15.0-mig-stock_picking_invoicing branch 3 times, most recently from cf998cc to 62bcc20 Compare April 28, 2023 15:46
Copy link
Member

@jguenat jguenat left a comment

Choose a reason for hiding this comment

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

Tested it and it works as expected :)

Nevertheless I question the necessity of computing some invoice & invoice lines values because they are also computed by odoo. I tried to comment them and the generated invoice was the same.

Comment on lines 369 to 373
values.update(
{
"invoice_origin": ", ".join(pickings.mapped("name")),
"user_id": self.env.user.id,
"partner_id": partner_id,
"invoice_payment_term_id": payment_term,
"move_type": inv_type,
"fiscal_position_id": partner.property_account_position_id.id,
"company_id": company.id,
"currency_id": currency.id,
"journal_id": journal.id,
"picking_ids": [(4, p.id, False) for p in pickings],
}
)
Copy link
Member

Choose a reason for hiding this comment

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

What is the idea behind defining user_id, invoice_payment_term_id, fiscal_position_id and company_id because those fields are already computed by odoo with the onchange ?

Comment on lines 474 to 460
values.update(
{
"name": name,
"account_id": account.id,
"product_id": product.id,
"product_uom_id": product.uom_id.id,
"quantity": quantity,
"price_unit": price,
"tax_ids": [(6, 0, taxes.ids)],
"move_line_ids": move_line_ids,
"move_id": invoice.id,
}
)
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as before : account_id, product_uom_id and tax_ids are already computed by odoo onchange.

@mbcosta
Copy link
Contributor Author

mbcosta commented May 11, 2023

Thanks @jguenat for review, I removed the fields pointed and included in the tests to check if those fields are filled, to be able to check Fiscal Position was need to included a simple demo data.

The errors in the tests are not related with this module, the problem came from recently update in v15 and affect modules in this repo

2023-05-11 14:27:14,545 254 ERROR odoo odoo.addons.sale_timesheet_invoice_description.tests.test_sale_timesheet_description: FAIL: TestSaleTimesheetDescription.test_three_timesheets_same_date_split
Traceback (most recent call last):
File "/__w/account-invoicing/account-invoicing/sale_timesheet_invoice_description/tests/test_sale_timesheet_description.py", line 217, in test_three_timesheets_same_date_split
self.assertEqual(aml_ids[-1].quantity, 1.3)
AssertionError: 1.29 != 1.3

I'm trying to understand the problem, it's seems a rounding error, but I'm not sure if it can be ignored and we should just change the value for 1.29, any idea to solve will be appreciated.

@mbcosta
Copy link
Contributor Author

mbcosta commented May 19, 2023

The problem in sale_timesheet_invoice_description was solved in #1458 , now just waiting the merge of #1457

@mbcosta mbcosta force-pushed the 15.0-mig-stock_picking_invoicing branch from d8cf1b9 to 76f2c30 Compare May 19, 2023 13:35
@rvalyi rvalyi requested review from jguenat and renatonlima May 19, 2023 15:39
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@rvalyi
Copy link
Member

rvalyi commented May 20, 2023

/ocabot migration stock_picking_invoicing

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone May 20, 2023
@rvalyi
Copy link
Member

rvalyi commented May 20, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-1452-by-rvalyi-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 3090435 into OCA:15.0 May 20, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at c7fd954. Thanks a lot for contributing to OCA. ❤️

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.