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] purchase_discount: Migration to 16.0 #1602

Merged
merged 72 commits into from
Oct 27, 2022

Conversation

ramiadavid
Copy link
Contributor

No description provided.

Pedro M. Baeza and others added 30 commits October 19, 2022 23:33
[FIX] purchase_discount: Supplier invoice view with discount

PEP8 errors corrected

[FIX] Duplicated field definition.
[IMP] self.pool[] instead of self.pool.get

Some changes from Guewen's review
[FIX] purchase_discount: Install errors.

[FIX] purchase_discount: digits_compute at discount field
[IMP] Change number to right-align, move report
[FIX] purchase_discount: Tests. Fixes OCA#187
* Fix xml_id as Odoo removed the referenced one (purchase.purchase_order_line_form --> purchase.purchase_order_line_form2)
* Add account_invoice_line.py: datas of the invoice are managed from the invoice/invoice_line with the onchange of purchase_order_id and no more from the source model of the invoice creation
* Pass _calc_line_base_price into api.multi as Odoo is now into new api
* Redefine _compute_amount to add a depends on new column 'discount'
* Remove  StockMove there is no more a _get_invoice_line_vals and invoice creation process from the stock.move
* Fix and improve tests
* Add fr translations
* OCA conventions update

[CHG] Code review
* Add Pedro copyright
* Fix Flake8 errors
* Remove useless api.multi

[FIX] use None instead of {} from the native signature
Update README.rst

Fix tests
- remove and rename fields
- fix license header

Don't rely on missing hooks to compute discount on purchase order line
[CHG] Authors company name
[CHG] Update image with 9.0 screens
OCA Transbot updated translations from Transifex

OCA Transbot updated translations from Transifex

OCA Transbot updated translations from Transifex

OCA Transbot updated translations from Transifex
[FIX] purchase_discount: Adapt code to upstream

Odoo has changed the way taxes are computed in
odoo/odoo@5afb5ec#diff-3bf7fa37ad269bad5d8df8214bd5206b
so this the adaptation for not depending on context values
Removing a state change in the purchase order that now is not needed anymore.
in the test, changing the currency of the company
is not nice and can cause other modules' test to
fail for no good reason.

Instead, make sure that the rate of the company
currency is 1.
* Avoid discount = NULL problem in BI

  In some cases, discount column can be NULL, making price_total to be empty (0)

* Avoid problems on integration tests

  For example, when you install *delivery* along with this one

* Make tests stronger:

  * Don't depend on invoice line order for determining which invoice line comes from each
    purchase line. With this, modules that modifies this order don't break these tests.

  * Don't depend on stock move order for determining which line comes from each
    purchase line. With this, modules that modifies this order don't break these tests.
This method is used for getting price cost and is using another logic when coming
from purchase instead of getting stock.move `price_unit` field. This commit fixes
it and also includes a unit test for checking properly the cost price update.

Closes OCA#562
We have to use po_line.price_unit, not self(stock.move).price_unit.
moves = picking.move_ids
self.po_line_1.discount = 25
self.po_line_2.discount = 50
self.po_line_3.discount = 10
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pedrobaeza this is the test

purchase_discount/models/purchase_order.py Outdated Show resolved Hide resolved
@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-1602-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 20, 2022
Signed-off-by pedrobaeza
@pedrobaeza
Copy link
Member

/ocabot migration purchase_discount

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Oct 20, 2022
@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). 🤖

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-1602-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 24, 2022
Signed-off-by pedrobaeza
@pedrobaeza
Copy link
Member

Don't get why this is not merged... cc @sbidoul

Let's try again:

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-1602-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 25, 2022
Signed-off-by pedrobaeza
@@ -0,0 +1 @@
../../../../purchase_discount
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be a regular file, instead of a symlink.
Due to this, the bot can't build a wheel and fails.

Why the bot does not report the error is unknown at this stage and tracked in OCA/oca-github-bot#221

Copy link
Member

Choose a reason for hiding this comment

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

@ramiadavid can you remove this file and run pre-commit run --all-files again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sbidoul
I work from Windows and pre-commit fails when it tries to create that file, I usually create it manually, but I don't know if it's correct.

Copy link
Member

Choose a reason for hiding this comment

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

@ramiadavid ha, symlinks on windows may be complicated. I could push the fix to your branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sbidoul Thanks,
I'll check to see how I can make it work, for other migrations.

@ramiadavid
Copy link
Contributor Author

Hi @pedrobaeza
the problem is already solved, can you retry the merge, please?

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-1602-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit a42ff35 into OCA:16.0 Oct 27, 2022
@OCA-git-bot
Copy link
Contributor

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

None yet