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

Master no onchange fp #32323

Draft
wants to merge 32 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@fpodoo
Copy link
Contributor

commented Apr 2, 2019

POC moved to branch: master-nochange-fp

Support for compute, with readonly=False as an alternative to onchange & conditional defaults

Most onchange includes business logic (ex: changing a partner on an invoice sets the
fiscal position, and the payment terms). But the business logic should not be exclusive to
the user interface. Thus, most of these onchanges should actually be compute fields with
store=True and readonly=False.

Compute fields are usually orthogonal between each others, thus simpler to implement and
inherits. Example: on an invoice, it's easy to implement _compute_fiscal_position (it's a
related partner_id.fiscal_position_id), but very complex to implement _onchange_partner_id
(it has impact on lots of different concepts depending on installed modules: fiscal positions,
payment terms, address, ...). By splitting this big onchange into smaller compute, it should
simplify the code and inheritability.

USE CASE 1:

If onchanges are replaced by compute fields, the code to create an invoice, or automated tests
it is simplified as you can let the business logic do its job:

self.env['account.invoice'].create({
    'partner_id': 1
    'line_ids': [{
        'product_id': 1
    })
)}

-> the compute fields will compute the right fiscal position, taxes, customer address, etc. It's
not the role of the sale order to implement this logic anymore.

USE CASE 2:

Let's say the module l10n_mx want to add a TAXCODE field on an invoice,
there is two way to do it:

@api.onchange['partner_id')
def _change_partner_id(self):
    super()
    self.taxcode = self.partner_id.taxcode

or, the new approach:

@api.depends('partner_id')
def _compute_taxcode(self):
    self.taxcode = self.partner_id.taxcode

The onchange approach creates a big issue: you will have to manage this logic by creating
bridge modules for every module that creates an invoice (e.g. l10n_mx_sale and
l10n_mx_subsription).

With the new approach, the TAXCODE is delegated to the invoice, and handled automatically
at creation, unless provided explicitly. Every module who create an invoice will have the new
business logic applied automatically: no need to create bridge modules.

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

fpodoo added some commits Mar 30, 2019

[IMP] base: support for compute, with store=True as a better alternat…
…ive to onchange

Rationale:

Most onchange includes business logic (ex: changing a partner on an
invoice sets the fiscal position, and the payment terms). But the
business logic should not be exclusive to the user interface. Thus,
most of these onchanges should actually be compute with store=True
and readonly=False.

Compute fields are usually orthogonal between each others, thus simpler
to implement and inherits. Example: on an invoice, it's easy to
implement _compute_fiscal_position (it's a related
partner_id.fiscal_position_id), but very complex to implement
_onchange_partner_id (it has impact on lots of different concepts
depending on installed modules: fiscal positions, payment terms,
address, ...). By splitting this big onchange into smaller compute,
it should simplify the code and inheritability.

USE CASE 1:

If onchanges are replaced by compute fields, the code to create an
invoice is simplified as you can let the business logic do its job:

self.env['account.invoice'].create({
    'partner_id': 1
    'line_ids': [{
        'product_id': 1
    })
)}

-> the compute fields will compute the right fiscal position, taxes,
customer address, etc. It's not the role of the sale order to implement
this logic anymore.

This should simplify tests too as you cover more business logic code
with less data to provide.

USE CASE 2:

Let's say the module l10n_mx want to add a TAXCODE field on an invoice,
there is two way to do it:

@api.onchange['partner_id')
def _change_partner_id(self):
    super()
    self.taxcode = self.partner_id.taxcode

or, the new approach:

@api.depends('partner_id')
def _compute_taxcode(self):
    self.taxcode = self.partner_id.taxcode

The onchange approach creates a big issue: you will have to create modules
like l10n_mx_sale and l10n_mx_subsription. Because, when these modules create
an invoice, they will have to call the code to set the taxcode.

With the new approach, there is nothing to do. Every module who create
an invoice will have the new business logic applied automatically: no
need to create glue modules.
fix

@robodoo robodoo added the seen 🙂 label Apr 2, 2019

@C3POdoo C3POdoo added the RD label Apr 2, 2019

fpodoo added some commits Apr 11, 2019

fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.