[16.0][MIG] stock_picking_invoicing: Migration to 16.0

[jguenat]

Added dependency on base_view_inheritance_extension to avoid rewriting view field context

Simplify invoice creation wizard (wip)

ToDo:

• Get supplier price for purchase invoices
• Get customer price from customer pricelist : maybe use account_invoice_pricelist instead of reimplementing it?
• Remove demo data (stock demo data is sufficient to try this module)
• Remove test dependency of demo data, this is not good practice right?
• Clean commit history (administrative commits)

[rvalyi]

cc @mbcosta @renatonlima

[mbcosta]

Hi @jguenat thanks for migration, please consider to do review of migration to v15 #1452 , unfortunally we not did it before your PR, after merge you can made a rebase in your branch based in v15, this is important for us to keep the history of module and make be possible to use the module in v15, if someone need, by doing this we also will be able to make backports or fowardports between the different versions, about your questions:

• "Simplify invoice creation wizard (wip)"

If the wizard are working in v16 should be better just make the migration here and after in another PR you propose the change.

• "Get supplier price for purchase invoices
  Get customer price from customer pricelist : maybe use account_invoice_pricelist instead of reimplementing it?"

The main goal of this module are "create invoices directly from picking, without having to use sale or purchase orders." https://github.com/OCA/account-invoicing/blob/14.0/stock_picking_invoicing/readme/DESCRIPTION.rst, so in my view should be use the Price informed in stock.move, to integrate this module with Sale or Purchase the better approuch can be create new modules sale_stock_picking_invoicing and purchase_stock_picking_invoicing where we can inherit the methods and create the Invoice with the Price informed in the Sale or Purchase Order, by doing this we keep the logic of Price in Sale and Purchase modules instead to put it in stock.picking or stock.move objects.

This implementation are used in Brazilian Localization because we have cases, where by the Fiscal rules, even when the company move a product to different address, not Sale or Purchase, the company are obrigated to generate an Invoice, but we also use this module when we want to force the creation of the Invoice from the Picking that was created by a Sale or Purchase, you can see here an example of how can be this implementation https://github.com/OCA/l10n-brazil/tree/14.0/l10n_br_sale_stock I have made a PR in this repo based in l10n_br_sale_stock module for v14 #1025 and https://github.com/OCA/l10n-brazil/tree/14.0/l10n_br_purchase_stock

But it's important to hear from you, and other people, about the use and the view of this module, because my focus are based just in the Brazilian case.

• "Remove demo data (stock demo data is sufficient to try this module)"

I'm recommend keep demo data, when we are testing the module a lot of times, we need to delete and create databases so it's important after install the module we can be able to just make the tests without need to create new Data over and over, during a Review or Homologation process other developers or non technical people can test, even at runboat, the same case in order to avoid the problem of someone created some Data with a specific Field or Configurateion that cause errors that don't happening for the others.

• "Remove test dependency of demo data, this is not good practice right?"

I'm not see any problem by using demo data in the tests and never heard about this being a problem or not good practice, do you have a reference about it?

[jguenat]

Hi @mbcosta , thanks for your feedback.

I needed this module on V16 and did not have the time to do the V15 migration. But now that you are doing it I totally agree with you let's do the V15 first and redo this one after.

Well I had some issues making the wizard work so a rework was needed anyway. Lots of stuff are now automatically computed by odoo like taxes and fiscal position. Lots of onchange are a think of the past.

For the supplier/customer price computation I totally agree that this is not this module job to make a link with sale or purchase app. It's just that the V14 wizard is using the partner property_product_pricelist to get the price and this cannot be done easily like that in V16 : product.with_context(pricelist....).price doesn't work anymore.

The use case I have is for a Music LP distribution company who have regular clients (music retailers) with a lots of returns. It's way easier for them to simply create a return picking (they use the barcodes to create it) and then, with this module, they could create a credit note.

Concerning the demo data I noticed a 'Module stock_picking_invoicing demo data failed to install' warning when testing the module but that was a mistake on my part (testing with -i instead of -u). Also I found it quite easy to try this module with the base stock demo data.
Demo and testing seems two different thing for me thus my interrogation. But if it's all right like that no need to change it.

I'll review your PR soon.

[mbcosta]

@jguenat I'm glad to know the module are being use for Music, by LP you say Long Plays Vinyl, right? This media has the best sound, with valves sound system even better. Can you say the name of the company? No problem if you can't, I just has curiosity to see what LPs the company has.

About the code, I'm just start to see v16 and will test your PR, I belive the wizard can be refactoring but it's important keep the separeted methods to allow other modules inherit and make changes, we can try to keep the method to get Price from Price List, let will see.

The PR of v15 #1452 now it's back to green, all tests has passed, with the changes that you has recommend.

[jguenat]

@mbcosta Yes it's Long Plays Vinyl :) you can check their stuff there https://irascible.ch/.

I close this PR as it is continued there #1465