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

13.0 mig purchase discount #844

Merged
merged 46 commits into from Feb 19, 2020
Merged

Conversation

sa3m
Copy link
Contributor

@sa3m sa3m commented Jan 14, 2020

Standard migration

Pedro M. Baeza and others added 30 commits January 14, 2020 12:11
[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.
@sa3m sa3m force-pushed the 13.0-mig-purchase_discount branch from a198c5a to 199589d Compare January 19, 2020 13:42
@sa3m
Copy link
Contributor Author

sa3m commented Jan 19, 2020

It seems that the price_unit of move2 is wrong because the tax is included in the total (total_void returned by compute_all) that is compared with 161. @pedrobaeza I saw you worked on _get_stock_move_price_unit method that is related to this issue. Any clue on this ?

@pedrobaeza
Copy link
Member

That should mean that methods have changed in v13 and you have to overwrite other things, but I can't say more without analyzing this in deep, which I'm not able to do it now.

@sa3m
Copy link
Contributor Author

sa3m commented Jan 19, 2020

Thanks for the feedback.

@sa3m sa3m force-pushed the 13.0-mig-purchase_discount branch 6 times, most recently from b91b015 to 8c68303 Compare January 21, 2020 11:37
@sa3m
Copy link
Contributor Author

sa3m commented Jan 21, 2020

I figured it out. You have to set invoice_repartition_line_ids with account_id in tax and it seems that you cannot set cost_method on product now. But still, the checks failed for reasons out of my comprehension.

@sa3m sa3m force-pushed the 13.0-mig-purchase_discount branch from 8c68303 to 0c1d935 Compare January 23, 2020 09:32
@sa3m
Copy link
Contributor Author

sa3m commented Jan 23, 2020

This was an error from pip in travis. They patched pip so now it works. But still the ci/runbot is in error, how does this work ? I don't see any error. Thanks

@sergio-teruel
Copy link
Contributor

Tested locally and Ok..

@younismahsud
Copy link

When this merge request is going to be merged with branch 13.0?

_inherit = "purchase.report"

discount = fields.Float(
string="Discount (%)", digits=dp.get_precision("Discount"), group_operator="avg"
Copy link

@juppe juppe Feb 7, 2020

Choose a reason for hiding this comment

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

This gives a "Deprecated call to decimal_precision.get_precision" warning in Odoo

Suggested change
string="Discount (%)", digits=dp.get_precision("Discount"), group_operator="avg"
string="Discount (%)", digits="Discount", group_operator="avg"

The import odoo.addons.decimal_precision as dp can also be removed when this is fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done ! Thanks for the feedback

@sa3m sa3m force-pushed the 13.0-mig-purchase_discount branch from 0c1d935 to deef176 Compare February 8, 2020 08:42
@andrp92
Copy link

andrp92 commented Feb 11, 2020

Hey, can we have this merged? @juppe

@juppe
Copy link

juppe commented Feb 12, 2020

Hey, can we have this merged? @juppe

I cannot comment on that, as I'm not a member of the OCA team. But hopefully soon. I'm also eagerly waiting for this to be merged.

@andrp92
Copy link

andrp92 commented Feb 12, 2020

@pedrobaeza can you help?

@MarkingSC
Copy link

Is it ready? Hope this to be merged soon... @pedrobaeza can you do it?
@sa3m thanks for migrating

@pedrobaeza pedrobaeza added this to the 13.0 milestone Feb 19, 2020
@pedrobaeza
Copy link
Member

/ocabot merge

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 13.0-ocabot-merge-pr-844-by-pedrobaeza-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 19, 2020
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot merged commit deef176 into OCA:13.0 Feb 19, 2020
@OCA-git-bot
Copy link
Contributor

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