odoo / odoo

Odoo. Open Source Apps To Grow Your Business.
https://www.odoo.com
Other
38.26k stars 24.84k forks source link

[11.0] base: _sql_constraints are not being overwritten #25661

Closed gustavovalverde closed 6 years ago

gustavovalverde commented 6 years ago

Impacted versions:

10.0 11.0

Steps to reproduce:

In account.invoice Odoo has:

    _sql_constraints = [
        ('number_uniq', 'unique(number, company_id, journal_id, type)', 'Invoice Number must be unique per Company!'),
    ]

Create a new module that inherits account.invoice with any of the following approaches:

class AccountInvoice(models.Model):
    _inherit = "account.invoice"
...

Option 1

    _sql_constraints = [
        ('number_uniq',
         'unique(number, company_id, partner_id, journal_id, type)',
         'Invoice Number must be unique per Company!'),
    ]

Option 2

    @api.model_cr_context
    def _auto_init(self):
        super(AccountInvoice, self)._auto_init()
        self._sql_constraints += [
            ('number_uniq',
             'unique(number, company_id, partner_id, journal_id, type)',
             'Invoice Number must be unique per Company!'),
        ]
        self._add_sql_constraints()

Option 3

    @api.model_cr
    def init(self):
        self._sql_constraints = [
            ('number_uniq',
             'unique(number, company_id, partner_id, journal_id, type)',
             'Invoice Number must be unique per Company!'),
        ]
        super(AccountInvoice, self).init()

Also tried a few combinations given in https://github.com/odoo/odoo/issues/20727 with no success.

Current behavior:

Expected behavior:

sswapnesh commented 6 years ago

Hello @gustavovalverde

I have tested same in V11 Community as mentioned in Option 1 and works fine. Steps: Installed account module Constraints definition in table --> "account_invoice_number_uniq" UNIQUE CONSTRAINT, btree (number, company_id, journal_id, type)

Created/Installed new module with

_sql_constraints = [
('number_uniq','unique(date_due)','whoooooooooo!'),
    ]

Constraints definition changed to--> "account_invoice_number_uniq" UNIQUE CONSTRAINT, btree (date_due) screenshot from 2018-07-08 21-43-57

sswapnesh commented 6 years ago

As mentioned here: https://github.com/odoo/odoo/blob/11.0/odoo/models.py#L2254=L2283

At the time of installing new module check _add_sql_constraints method

self._sql_constraints = [('number_uniq', 'unique(number, company_id, journal_id, type)', 'Invoice Number must be unique per Company!'), ('number_uniq', 'unique(date_due)', 'whoooooooooo!')]

So for the first constraints --> ('number_uniq', 'unique(number, company_id, journal_id, type)', 'Invoice Number must be unique per Company!') --> has_definition will be True and it has same Definition (custom constraints is not loaded in table yet) so It wont process anything.

For the second constraints --> ('number_uniq', 'unique(date_due)', 'whoooooooooo!') has_definition will be True So It will compare old Constraints definition with new one (In our case both are different) and old constraints will be dropped and new will be added.

gustavovalverde commented 6 years ago

That's weird.

Every time I update the constraint (no matter the option I use above); the core definition crashes because it can't create the index (could this be causing the odd behavior?)

I'll try another approach based on what you explained here: https://github.com/odoo/odoo/issues/25661#issuecomment-403298243

sswapnesh commented 6 years ago

I didn't found any crash in my case. (I will look for this in v10 and update here)

gustavovalverde commented 6 years ago

So what's going on here is this:

I have invoices which won't comply with Odoo's default constraint in my DB.

    _sql_constraints = [
        ('number_uniq', 'unique(number, company_id, journal_id, type)', 
          'Invoice Number must be unique per Company!'),
    ]

Because I have duplicates on in_invoice (and I should have it, based on our localization). The index can't be created because of this.

With that clear. I'm trying to add a more flexible constraint, which allows duplicates if the partner is not the same:

    _sql_constraints = [
        ('number_uniq', 'unique(number, company_id, partner_id, journal_id, type)',
         'Invoice Number must be unique per Company!'),
    ]

This won't work as the Odoo's core constraint tries to execute before this one.

If my data allows the Odoo's constraint to create its indexes, a more restrictive one (considering that the actual data allows it), will work without problem and override Odoo's constraint.

On your example: https://github.com/odoo/odoo/issues/25661#issuecomment-403298243 Odoo's constraint will successfully load, and so your constraint too.

On my example, Odoo's constraint won't load, but it won't either be overwritten as it continuously tries to create an index on a database that won't allow it.

gustavovalverde commented 6 years ago

My only workaround so far:

pedrobaeza commented 6 years ago

@gustavovalverde you will have to use your workaround, as this edge case can't be covered by standard Odoo, as you are not complying with the defined index, which is a requirement for the constraint.

Closing as no possible best solution.

gustavovalverde commented 6 years ago

So the only way to override an Odoo's constraints (with a module) is to first comply with it?

pedrobaeza commented 6 years ago

Not exactly, but in this case you have an index over that constraint.

gustavovalverde commented 6 years ago

This worked on v10, what wouldn't this keep working on v11?

If I'd like to override a constraint, it should first evaluate my constraint before the one in Odoo's core. It doesn't have to comply with the core constraint (or it index) to accomplish that.

pedrobaeza commented 6 years ago

Maybe on v10 there wasn't that index, but use the workaround and everything is OK.

nhomar commented 6 years ago

Create an unit script on your localization module that prestare the data then in every update it is fixed.

BUT I can’t find a functional reason which can brake this rule starting from there you are forcing your design so far.

BTW if you share here what are you doing (the module itself) it is easiest to help with the information given till now, this most be closed.

On Sun, Aug 12, 2018 at 10:31 Pedro M. Baeza notifications@github.com wrote:

Maybe on v10 there wasn't that index, but use the workaround and everything is OK.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/odoo/odoo/issues/25661#issuecomment-412350529, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUqyqu7vrnTKV82uzDVYKPLEGeJxKmuks5uQEpCgaJpZM4VGuYT .

-- Sent from my mobile.

gustavovalverde commented 6 years ago

Deleting/commenting code from Odoo's core shouldn't be a workaround. I've done this just for the sake of finding the root cause.

The issue still exists, and doing that workaround (which needs SQL intervention) and code removal is not is not manageable for most partners with 100+ clients, or to whom that have their infrastructure on Odoo.sh

pedrobaeza commented 6 years ago

But this is your specific problem, not the 100+ partners. You are trying to abuse the system on that way, and thus you have to deal with that problem.

gustavovalverde commented 6 years ago

@nhomar We might be overseeing something here: https://github.com/odoo-dominicana/l10n-dominicana/blob/11.0/ncf_manager/models/account_invoice.py#L140

@pedrobaeza This is from a localization used for the whole country, I'm not the only one using it.

In Dominican Republic our invoice number is used as Odoo's invoice number. And that's our move_name too. This number repeats between suppliers, so that's why we override Odoo's constraint.

I'll appreciate if there's another solution for this, but we're just trying a recommendation given here: https://github.com/odoo/odoo/issues/19722#issuecomment-339001565