[12.0][IMP] purchase_landed_cost workflow

[clementmbr]

These 3 commits aim to improve the purchase_landed_cost workflow and fix small bugs.

The basic idea is to add the possibility to create/open a Cost Distribution directly from the Purchase Order/Picking.
It adds a boolean on the PO and the Picking that allows the user to create/access the related Cost Distribution from there. It also adds an option on res_partner in order to pre-fill this boolean on PO and Pickings linked with some specified vendors :

Beside that, the Fix helps linking Invoice Lines with Cost Distribution Expenses even when the user fill the invoice_line field directly from the Cost Distribution Expense o2m list without using the button "Import Invoice Line".

What do you think about it @percevaq @pedrobaeza @cubells @ernestotejeda ?

---

Starting from here, I would like to open a discussion about renaming some variable names in order to be more meaningful and help users and other developers to understand how the module works.

For instance, objects from 'purchase.cost.distribution.line' are called in too many different ways :

• cost_lines in the model purchase.cost.distribution
• distribution_line in the model purchase.cost.distribution.line.expense
• affected_lines or imported_lines in the model purchase.cost.distribution.expense
• dist_line when needed inside its own model.

... while in fact they all refer to some kind of stock.move objects. Thus names like moves, distribution_move, affected_moves and imported_moves would make more sense, wouldn't they ?
But it may involve to rename the model 'purchase.cost.distribution.move' and other modifications if you all agree with it.

---

Another renaming work would be on these 'purchase.cost.distribution.line' fields :

• standard_price_old doesn't refer to what would be "the product's standard_price before the picking was done" as we can imagine, but refers to the Product move's purchase price (via his definition through dist_line.move_id._get_price_unit()).
  Thus it looks like a duplicate of the other field product_price_unit while the real information about "the product's standard_price before the picking was done" seems to be quite relevant to be calculated and displayed to the user.
• standard_price_new doesn't refer to what would be the "new product's standard_price afther the cost update", but refer to the (sought-after) product's "Landed Cost".
  I would suggest to call it landed_price_unit if you agree.

WIP in #901

---

Finally, it looks like some improvements are needed in the link between Cost Distribution's Expenses and their related Invoice Lines.

Firs of all I would suggest to rename the title "Supplier Invoice line" into something more relevant like "Expense Invoice line" to avoid confusions with the Invoice line linked to the purchase of the product we are updating the price.

Then, the button "Import from Pickings" in the Invoice form is not really obvious to use. Maybe a more explicit name like "Link Invoice Line to Cost Distribution Expense" would be better, imagining that it would display a wizard allowing to chose a real Cost Distribution Expense object and not a Stock Picking.

---

Thank you very much for your reading,
I hope you found these inputs interesting. Of course I am available to work on the suggestions I made if you agree with some of them and I'm opened to other ideas in order to improve this useful module.

cc @rvalyi

[pedrobaeza]

It would desirable to have each improvement in a different PR, so the discussion can flow independently and direct things can be directly merged without waiting for the rest. Would you mind doing that way?

[clementmbr]

It would desirable to have each improvement in a different PR, so the discussion can flow independently and direct things can be directly merged without waiting for the rest. Would you mind doing that way?

Ok, I'll make 3 more PR so and let's talk about this one here.
Did this improvement in the workflow make sense for you ?

[pedrobaeza]

In a quick glance, yes, but that's why I want to discuss them by parts: for refining each separately.

[clementmbr]

FYI @pedrobaeza I made a second PR about new variables names #901

Cc @rvalyi

[clementmbr]

Following this idea to improve the purchase_landed_cost workflow, I add 3 commits in order to :

• Hide the core module stock_landed_cost configuration option (and avoid the user to use both modules simultaneously)
• Block the Update Cost if some products costing method is not "average" (as the module only update something in this case)
• Add a message on importing Pickings and allow the user to import pickings in state "assigned" (event if the Update Cost will be authorized only if all the pickings are in state "done")

cc @pedrobaeza @rvalyi

[pedrobaeza]

But why are you still adding more and more stuff to the same PR? This is becoming a difficult piece to be reviewable. It's advisable to put each change in a separate PR for being reviewed quickly those that are straight-forward, and retain those that not. This way, all are retained, and more because Travis is red.

[clementmbr]

@pedrobaeza I've just corrected some old flake8 problems and now Travis is green.

I'm sorry if this PR is quite long to be reviewed...
These 3 last commits are all very small (just adding some error messages and correct flake8) and they made sense to be added in this PR from my point of view because they are part of this "improving workflow" idea.
Moreover, I'm already working on 3 other PR to be proposed on this module... I thought creating a 4th one would confuse more than help reviewers in understanding what was being proposed globally.

By the way, I just added the migration script you asked on the other PR aiming to rename price variables : #901

I hope these 2 PR are now ready to be reviewed in your point of view. Let me know if I can improve other things.

I'm still working on another PR aiming to improve the way to link invoice_lines to Cost Distribution Expenses. I'll publish it as soon as is will be ready.

cc @rvalyi

[clementmbr]

@pedrobaeza @rvalyi
I made the rebase with the #907 cleaning the pep8 problems and Travis is green now.
I hope the PR looks ok. I know it's 7 commits but only the first one is consequent and need a particular attention for the review, the others are really small and might not create any discussion.

Thank you very much for your reviews ! ✨

[tonygalmiche]

Hi @clementmbr,
I tried to test by following a document in French :
https://odoo-doc.gitbook.io/fonctionnel/-LxRKpKPmg6E8oIFl-4d/

I don't see the 'Show History' link in the product :

I created this order

with these costs

I don't understand this result

I did not go further in my tests :
thanks in advance for the explanations

[pedrobaeza]

About all the state propagation, I have 2 doubts:

• Why is needed? Why do you need to add all that overhead for putting picking and PO states in other places?
• Why not doing it with computed fields?

[clementmbr]

@pedrobaeza @tonygalmiche thank you very much for your interest !

@pedrobaeza , I think I don't understand what you are saying...
When you talk about "state propagation" you mean "Why are these cost_distribution_state fields are needed in PO and picking" ?

This cost_distribution_state is a computed field indeed and is here to help filtering Purchase Orders or pickings by the "the lowest state of the Cost Distributions they are linked to". In order to help the user find quickly all the PO or pickings that need to be related to a Cost Distribution or that are linked to a Cost Distribution that need to be calculated...

And once they are here I use also these fields to hide/display the "Landed Cost" and "Register Landed Cost" buttons too. Let me know if you think it could be possible to do this link between Cost Distributions and their related PO/pickings in a shorter way...

[pedrobaeza]

OK about the reasoning of adding them, but not about the implementation. When I say "computed", I mean in the ORM sense of computed: a computed field with proper depends, not something that you have to patch and call manually to a _compute_x method for readjusting things.

[clementmbr]

@tonygalmiche

I don't see the 'Show History' link in the product :

As this tutorial I shared with you was made for an Akretion client, the "Show history" is displayed with an Akretion module called product_usability :
https://github.com/akretion/odoo-usability/tree/12.0/product_usability

Sorry not to have mentioned it when we talked about it. This "Show history" button helps to visualize how the product's standard_price changed after the picking is done and after the Cost Distribution updated this standard_price.

I don't understand this result

If you don't understand the meaning of these "Picking lines" columns maybe you will like my other PR that try to set more explicit names, not only for the variables used in the code but also for these columns titles : #901

But if I can help you understanding these names :

• Quantity is your Stock move product quantity
• Unit Price is a field related to the stock move's price_unit which is usually equal to the purchase order's price_unit (100 here). That's why it's a bit strange to see it null here... Is your product "storable" and not "consumable" ?
• Amount line would be Quantity * Unit Price
• Cost amount will be the sum of all the part of expenses that will be added to our Stock Move.
• Unit Cost is this Cost amount sum divided by the stock move's product quantity. It will be calculated when clicking on "Calculate" in the Cost Distribution but as your expenses are distributed by "Amount of the line" and "Product Price" over the stock moves and these two fields are equal to zero, your stock move will receive no added expenses.
• Previous cost is a calculated field that take the stock_move's price_unit value. It duplicates the column 'Unit Price', that's why I use only this field in my PR [12.0][REF] purchase_landed_cost: Rename variables #901 and I delete Unit price. In your case it did have the 100 value of your purchase order... which is even more confusing with the fact that your your 'Unit Price' is zero. You can ping me on framateam if you want me to have a closer look.
• New Cost is the sought-after 'Landed Cost', the unit cost total including the unit price from the purchase order and the part of the expenses added to this unit price.

Is it clearer ?

[clementmbr]

OK about the reasoning of adding them, but not about the implementation. When I say "computed", I mean in the ORM sense of computed: a computed field with proper depends, not something that you have to patch and call manually to a _compute_x method for readjusting things.

I'm sorry I think I still don't understand what do you mean by "the ORM sense of computed"...
You mean that I can use better depends fields in the _compute definitions ?

Or do you mean I should decorate this _propagate_state_in_order_picking with an @api.depends instead of calling it in the write ?

[tonygalmiche]

* **Unit Price** is a field related to the stock move's `price_unit` which is usually equal to the purchase order's `price_unit` (100 here). That's why it's a bit strange to see it null here... Is your product "storable" and not "consumable" ?

Thank you for this detailed answer but it's a storable product and that's why I don't understand this result :

I just did a new test and I still have the same result

[clementmbr]

@tonygalmiche then it's because your actual product cost (standard_price) is zero and your product cost method might be Standard (you define/change it in your product's category).
With this cost method stock move's price_unit will catch the product's standard_price in order to evaluate your stock with always the same fixed standard_price instead of a price depending on the purchase value of the incoming products.

See this documentation for more understanding.

Anyway, I added this warning when you try to Update a product cost which has not an "Average Cost (AVCO)" cost method :
https://github.com/OCA/purchase-workflow/pull/897/files#diff-a8f21f0a7db0dbe7ada673c97ff108b9R288

You would have noticed it if you've tried to click on "Update Cost".
But yes, it will be necessary to explicit this limitation to "products with Average cost method" on the module's README once the improvements will be merged (if they will).

[tonygalmiche]

Thank you @clementmbr for all these explanations.
I followed and checked everything indicated in this document :
https://odoo-doc.gitbook.io/fonctionnel/-LxRKpKPmg6E8oIFl-4d/

Everything works as indicated without anomaly. Very good

[tonygalmiche]

in terms of working, it's ok for me

[rvalyi]

@clementmbr not sure but may be this is related to what @pedrobaeza was trying to explain you OCA/odoo-community.org#50 (comment)
I have no time to check this now, just linking here for reference.

[pedrobaeza]

Well, my explanation is for v13

[github-actions]

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.