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

[FIX] sale: unstored field compute in methode with a stored field #25908

Closed
wants to merge 1 commit into from

Conversation

fmdl
Copy link
Contributor

@fmdl fmdl commented Jul 20, 2018

Description of the issue/feature this PR addresses:
invoice_count, invoice_ids and invoice_status are computed by _get_invoiced, but only invoice_status is store. It doesn't make sens.

Perf Nota : for a sale order with 3000 lines, the time to compute is 3 secondes, and you need to wait 3 secondes to show the sale order.

Nota : this fix seems not good.

@nim-odoo
@odony

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

@fmdl
Copy link
Contributor Author

fmdl commented Jul 23, 2018

OPW 1868451

@guewen
Copy link
Contributor

guewen commented Jul 23, 2018

Speaking of this, invoice_status is used really often and fast to compute, invoice_ids/invoice_count less often (mostly for display) and are slow to compute, it's much more efficient to split in 2 compute methods, something like:

    # Separate 2 computes from the main module ; invoice_ids will not be
    # called 4 times during an invoice validation, and storing the invoices
    # link / count on the SO improves the speed on the form view
    @api.depends('name',
                 'order_line.invoice_lines.invoice_id.type',
                 'order_line.invoice_lines.invoice_id.origin',
                 'order_line.invoice_lines.invoice_id.number')
    def _get_invoice_ids(self):
        for order in self:
            invoice_ids = order.order_line.mapped(
                'invoice_lines'
            ).mapped(
                'invoice_id'
            ).filtered(lambda r: r.type in ['out_invoice', 'out_refund'])
            # Search for invoices which have been 'cancelled'
            # (filter_refund = 'modify' in 'account.invoice.refund')
            # use like as origin may contains multiple references
            # (e.g. 'SO01, SO02')
            refunds = invoice_ids.search(
                [('origin', 'like', order.name),
                 ('company_id', '=', order.company_id.id)]
            ).filtered(lambda r: r.type in ['out_invoice', 'out_refund'])
            invoice_ids |= refunds.filtered(
                lambda r: order.name in [origin.strip() for origin
                                         in r.origin.split(',')]
            )
            # Search for refunds as well
            refund_ids = self.env['account.invoice'].browse()
            if invoice_ids:
                for inv in invoice_ids:
                    refund_ids += refund_ids.search(
                        [('type', '=', 'out_refund'),
                         ('origin', '=', inv.number), ('origin', '!=', False),
                         ('journal_id', '=', inv.journal_id.id)]
                    )

            order.update({
                'invoice_count': len(set(invoice_ids.ids + refund_ids.ids)),
                'invoice_ids': invoice_ids.ids + refund_ids.ids
            })

    @api.depends('state', 'order_line.invoice_status')
    def _get_invoiced(self):
        for order in self:
            # Ignore the status of the deposit product
            payment_adv_model = self.env['sale.advance.payment.inv']
            deposit_product_id = payment_adv_model._default_product_id()
            line_invoice_status = [
                line.invoice_status for line in order.order_line
                if line.product_id != deposit_product_id
            ]

            if order.state not in ('sale', 'done'):
                invoice_status = 'no'
            elif any(invoice_status == 'to invoice'
                     for invoice_status in line_invoice_status):
                invoice_status = 'to invoice'
            elif all(invoice_status == 'invoiced'
                     for invoice_status in line_invoice_status):
                invoice_status = 'invoiced'
            elif all(invoice_status in ['invoiced', 'upselling']
                     for invoice_status in line_invoice_status):
                invoice_status = 'upselling'
            else:
                invoice_status = 'no'

            order.update({
                'invoice_status': invoice_status
            })

    invoice_count = fields.Integer(compute='_get_invoice_ids')
    invoice_ids = fields.Many2many(compute='_get_invoice_ids')
    # often used in searches, 'sales to invoice' menu... so it's better to have
    # it's computation detached from the invoice status, plus an index
    invoice_status = fields.Selection(compute='_get_invoiced', index=True)

@odony
Copy link
Contributor

odony commented Jul 23, 2018

@fmdl Indeed, the fix isn't good ;-) Changing a non-stored field into a stored field is one of the few things that are strictly forbidden by our stable policy. Splitting the compute method as proposed by @guewen could be an alternative. It is a significant change of API though, and it's difficult to tell if anyone is currently overriding the compute methods in production (could be better suited for master)

If you're seeing a performance problem due to this code in 11.0, have you considered adding indexes for the relevant queries, before attempting code changes?

@fmdl
Copy link
Contributor Author

fmdl commented Jul 25, 2018

@odony can you give a solution to reduce the time to show an sale order with 3000 lines. Thanks

@odony
Copy link
Contributor

odony commented Jul 25, 2018

That requires a proper analysis of all queries involved, to verify the various bottlenecks. If you can't do it yourself you can open a helpdesk ticket for performance investigation, and our dedicated performance consultant will investigate and evaluate possible optimizations. Maybe the solution will include some custom patches, like the one suggested above.

Keep in mind that a 3k line order is an extreme case. You cannot expect all operations to be as fast for 3k lines as they are for a normal order. Some optimizations are probably possible in standard, but for extreme cases some customizations may be needed, such as specific indexes, storing fields, etc.
These patches typically need to be maintained by the Odoo Partner/Integrator in charge of the implementation, but we can help writing them.

/cc @nseinlet

@fmdl fmdl closed this Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants