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

[14.0][FIX] stock_picking_invoicing: Inform field invoice_state in Stock Rule #1782

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

mbcosta
Copy link
Contributor

@mbcosta mbcosta commented Aug 23, 2024

Inform field invoice_state in Stock Rule

https://github.com/OCA/OCB/blob/14.0/addons/stock/models/stock_rule.py#L259

    def _get_custom_move_fields(self):
        """ The purpose of this method is to be override in order to easily add
        fields from procurement 'values' argument to move data.
        """
        return []

Review of #1025

Also remove License Header in init file with only imports

cc @rvalyi @renatonlima @kevinkhao

Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

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

sorry for the delay. The why can be seen here https://github.com/OCA/account-invoicing/pull/1025/files#r1686701138
BTW this is future proof as _get_custom_move_fields is present even in v18.

@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 Oct 22, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-1782-by-rvalyi-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 22, 2024
Signed-off-by rvalyi
@OCA-git-bot
Copy link
Contributor

@rvalyi your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-1782-by-rvalyi-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@mbcosta
Copy link
Contributor Author

mbcosta commented Oct 22, 2024

The error in the merge command seems not related with this PR

https://github.com/OCA/account-invoicing/actions/runs/11469170392/job/31915851268#step:8:726

2024-10-22 22:03:10,750 564 ERROR odoo odoo.addons.account_invoice_pricelist.tests.test_account_move_pricelist: FAIL: TestAccountMovePricelist.test_account_invoice_fixed_pricelist_without_discount_secondary_currency
Traceback (most recent call last):
  File "/__w/account-invoicing/account-invoicing/account_invoice_pricelist/tests/test_account_move_pricelist.py", line 319, in test_account_invoice_fixed_pricelist_without_discount_secondary_currency
    self.assertAlmostEqual(invoice_line.price_unit, 65.41)
AssertionError: 100.0 != 65.41 within 7 places

2024-10-22 22:03:10,958 564 ERROR odoo odoo.addons.account_invoice_pricelist.tests.test_account_move_pricelist: FAIL: TestAccountMovePricelist.test_account_invoice_pricelist_with_discount_secondary_currency
Traceback (most recent call last):
  File "/__w/account-invoicing/account-invoicing/account_invoice_pricelist/tests/test_account_move_pricelist.py", line 295, in test_account_invoice_pricelist_with_discount_secondary_currency
    self.assertAlmostEqual(invoice_line.price_unit, 58.87)
AssertionError: 90.00000000000004 != 58.87 within 7 places

2024-10-22 22:03:11,360 564 ERROR odoo odoo.addons.account_invoice_pricelist.tests.test_account_move_pricelist: FAIL: TestAccountMovePricelist.test_account_invoice_pricelist_without_discount_secondary_currency
Traceback (most recent call last):
  File "/__w/account-invoicing/account-invoicing/account_invoice_pricelist/tests/test_account_move_pricelist.py", line 303, in test_account_invoice_pricelist_without_discount_secondary_currency
    self.assertAlmostEqual(invoice_line.price_unit, 65.41)
AssertionError: 100.0 != 65.41 within 7 places

@rvalyi
Copy link
Member

rvalyi commented Oct 22, 2024

@mbcosta it's not related indeed, I could confirm it here #1820

However we still need to get it fixed to merge anything in 14.0... EDIT: we have the same issue on 15.0 and 16.0...
Any chance you could take a look please?

@mbcosta mbcosta force-pushed the 14.0-FIX-invoice_state_stock_rule branch from 49c98c1 to 17a76f4 Compare October 23, 2024 02:42
@mbcosta
Copy link
Contributor Author

mbcosta commented Oct 23, 2024

@rvalyi after testing for while suddenly the error stop to happening, I'm not sure the reason but suspect it could be related by the DATE, the lines of the object product.pricelist has the fields date_start and date_end, the tests of the module don't inform the values, but this information is used in methods like _compute_price_rule_get_items and others, but as the error stop for now it's not possible be sure about.

@rvalyi
Copy link
Member

rvalyi commented Oct 23, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-1782-by-rvalyi-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 0db1369 into OCA:14.0 Oct 23, 2024
9 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 0a2b7d9. 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.

4 participants