[16.0][FIX] sale_invoice_policy: re-structure module so that there is no need to change the original invoice_policy field

[ACheung-FactorLibre]

FIX for: #2725

[rousseldenis]

@ACheung-FactorLibre The fix should not be as this simple as the code depends on the context which is not suitable for stored values.

[ACheung-FactorLibre]

Yes @rousseldenis, you're right.

[ACheung-FactorLibre]

Hi @rousseldenis, I think modifying the fields on the model product.template is not necessary. You can calculate the qty_to_invoice per line without modifying the fields.

Here is a suggestion for _compute_qty_to_invoice():

    @api.depends(
        "qty_invoiced",
        "qty_delivered",
        "product_uom_qty",
        "state",
        "order_id.invoice_policy",
    )
    def _compute_qty_to_invoice(self):
        if not self.order_id.invoice_policy:
            return super()._compute_qty_to_invoice()
        if self.order_id.invoice_policy == 'order':
            for line in self:
                line.qty_to_invoice = line.product_uom_qty - line.qty_invoiced
        else:
            for line in self:
                line.qty_to_invoice = line.qty_delivered - line.qty_invoiced
        return True

PD: Are you working on the FIX? If you are not, I'll commit my changes on this PR, otherwise, this is just a suggestion and I'll let you FIX it.

[rousseldenis]

Hi @rousseldenis, I think modifying the fields on the model product.template is not necessary. You can calculate the qty_to_invoice per line without modifying the fields.

Here is a suggestion for _compute_qty_to_invoice():

    @api.depends(
        "qty_invoiced",
        "qty_delivered",
        "product_uom_qty",
        "state",
        "order_id.invoice_policy",
    )
    def _compute_qty_to_invoice(self):
        if not self.order_id.invoice_policy:
            return super()._compute_qty_to_invoice()
        if self.order_id.invoice_policy == 'order':
            for line in self:
                line.qty_to_invoice = line.product_uom_qty - line.qty_invoiced
        else:
            for line in self:
                line.qty_to_invoice = line.qty_delivered - line.qty_invoiced
        return True

PD: Are you working on the FIX? If you are not, I'll commit my changes on this PR, otherwise, this is just a suggestion and I'll let you FIX it.

Nope, not so much time for that. Wanted to fix the main branch as priority. You can go on with your proposal

[ACheung-FactorLibre]

Hello @rousseldenis @pedrobaeza,

Could you review this?

Thank you

[pedrobaeza]

Don't remove the depends_context if still used.

[ACheung-FactorLibre]

Hi Pedro,
depends_context is used to return a decorator that specifies the context dependencies of a non-stored "compute" method. Since the field is stored on the database, it will not run the method even if invoice_policy is changed by context.

[pedrobaeza]

Yeah, but you are still using a context value, so both are incompatible. You can't depend on a context if it's stored.

[pedrobaeza]

Why still this method if the field is not computed? And IMO, this should be adjusted on the sales order fields, not here.

[ACheung-FactorLibre]

Hi Pedro,

Yes, the field is computed, here is Odoo's declaration of the field:

    invoice_policy = fields.Selection(
        [('order', 'Ordered quantities'),
         ('delivery', 'Delivered quantities')], string='Invoicing Policy',
        compute='_compute_invoice_policy', store=True, readonly=False, precompute=True,
        help='Ordered Quantity: Invoice quantities ordered by the customer.\n'
             'Delivered Quantity: Invoice quantities delivered to the customer.')

[rafaelbn]

Hello @ACheung-FactorLibre ,

thank you for this PR and this effors. Just to have the situation:

In your opinion, which is the state of this PR and what would you like to happend with it?

I kindly ask just to figure out how to help 😄 ❤️

Thanks in advance
Best regards

[ACheung-FactorLibre]

Hello @rafaelbn,

Well, in my opinion, this PR is ready, but there are some changes that have been requested that have not been applied yet.
If anyone wants to open a PR with those changes, it would be great.

Thank you @rafaelbn
Best regards

[ACheung-FactorLibre]

Hi @pedrobaeza,

Changes have been applied, could you please review them?

Thank you for your time

[pedrobaeza]

There are still some red tests.

[ACheung-FactorLibre]

There are still some red tests.

Hello @pedrobaeza, corrected.

Thank you

[pedrobaeza]

There's a merge commit that shouldn't be there. You should rebase. And the technique used writing on the product is a very weak one. What you should do is to intercept the to invoice quantity compute methods to take into account the invoice policy.

[ACheung-FactorLibre]

Hi @pedrobaeza,
Could you please explain this?

And the technique used writing on the product is a very weak one. What you should do is to intercept the to invoice quantity compute methods to take into account the invoice policy.

Thank you

[pedrobaeza]

Override the compute method for the quantity to invoice and put that quantity according the invoice policy set in the order.

[pedrobaeza]

Supersede by #3102