This question has been flagged
2 Replies
9781 Views
columnrelatedvaluedropdropped
Avatar
Weste

Hi all,

I have a problem trying to override a fields.related. It's originally defined in product module, product_supplierinfo class:

_columns = {
...
    'product_uom': fields.related('product_id', 'uom_po_id', type='many2one', relation='product.uom', string="Supplier Unit of Measure", readonly="1", help="This comes from the product form."),
...
}

I create a module, that depends on product, to put a many2one instead:

class product_supplierinfo(Model):
    _inherit = 'product.supplierinfo'
    _columns = {
        'product_uom': fields.many2one('product.uom', "Supplier Unit of Measure", required=True, help="This comes from the product form."),
    }

I add a function to populate the field and call it at installation

def _init_seller_uom(self, cr, uid, ids=None, context=None):
    psi_ids = self.search(cr, SUPERUSER_ID, [], context=context)
    for psi in self.browse(cr, SUPERUSER_ID, psi_ids, context=context):
        uom_id = psi.product_id.uom_po_id.id
        self.write(cr, SUPERUSER_ID, psi.id, {'product_uom': uom_id}, context=context)
    return psi_ids

At this point, my column is correctly set and I can modify the values as I'd like. Perfect!

Now, the problem comes when I restart my server with --update=all. It seems the system first parses the original class in product module, sees that the field is a not-stored related field, and thus drops the column. I can indeed see that this query is executed:

ALTER TABLE "product_supplierinfo" DROP COLUMN "product_uom" CASCADE

Later in the update, it parses my module and execute this query:

ALTER TABLE "product_supplierinfo" ADD COLUMN "product_uom" int4

At the end of the update, my column is still updatable, but all values are lost!

Is there a solution? Thanks.

Avatar
Discard
Avatar
Weste
Author Best Answer

The working solution is finally:

class product_supplierinfo(Model):
    _inherit = 'product.supplierinfo'

    def _get_product_uom(self, cr, uid, ids, field_name, arg, context):
        res = {}
        for psi in self.browse(cr, uid, ids, context=context):
            res[psi.id] = psi.product_uom_stored.id
        return res

    def _set_product_uom(self, cr, uid, ids, field_name, field_value, arg, context):
        psi = self.browse(cr, uid, ids, context=context)
        psi.write({'product_uom_stored': field_value})
        return ids

    _columns = {
        'product_uom': fields.function(_get_product_uom, fnct_inv=_set_product_uom, type="many2one", relation="product.uom", help="The supplier UoM for this product."),
        'product_uom_stored': fields.many2one('product.uom', "Supplier Unit of Measure", required=True, help="This comes from the product form."),
    }
Avatar
Discard
Avatar
Olivier Dony (odo)
Best Answer

Short answer: that's the expected behavior, and you're doing something that is considered a bad practice.

Why does this happen: For many technical and logical reasons, during installations/upgrades the framework needs to handle each module and its dependencies as an isolated set of modules. For instance the product module will be upgraded before loading the modules that depend on product. This is necessary to maintain the modularity of OpenERP and let each module deal with the database and registry state that it should know about.

Why is it a bad practice: Overriding a field to change its type (or make it stored when it wasn't) is one of the few things that will break the encapsulation principle, by seriously changing the original behavior of the parent module rather than simply extending it. The extent of that change means the original module and other modules based on it may suddenly have surprises that could break their logic completely. The OpenERP API contains enough entry points for extending the behavior of other modules that you can always do what you want in a different manner, without breaking this rule. Here you could probably add a new m2o field and override some methods to use it as you need. It will also force you to review the cases where the original field is used and perhaps help you detect cases where you wrongly assumed you could replace the related field by an arbitrary value. For example what happens if the UOM you pick is not compatible with the product UOM, and what if some code elsewhere directly takes the Purchase UOM on the product because it does not expect the one of the supplierinfo to be different?

Some similarly bad practice:

  • Storing a field that is normally not stored (this is what you're doing)
  • Changing the required flag of a stored field from a parent module (at model level, not view level)
  • Overriding methods without calling super()
  • Monkey-patching code from parent modules

You can inherit a model and re-declare some of its fields, but in that case you should not alter anything that will significantly change the model. You can change view-level attributes (such as states, readonly, string, etc.). You can even change its type if the new type is 100% compatible at model level, e.g. replace a char field by a stored function field of type char, with a proper setter and getter that guarantee it will at least keep its old behavior. Anything else is usually a bad idea.

There have been a few cases were official OpenERP modules used to do this (changing a field type), because it often looks easier at first sight, but it was a bad decision in every case, and most of them have been removed now.

The framework gives you a lot of freedom, including the option to shoot yourself in the foot if you really ask for it, but the cases where you'll need that are not very frequent, and you should be ready to deal with the consequences if you're asking for that.

That may not be the answer you expected but I hope it helps a bit... ;-)

Avatar
Discard
Weste
Author

Thanks for the explanations.

So in my exemple, I should:

  • create a new fields.many2one named product_uom_stored.

  • in place of the existing product_uom field, put a function field, not stored, with a proper setter and getter, that would return the value stored in product_uom_stored.

Would that be the correct way to do?

Olivier Dony (odo)

@Weste: yes that solution would probably work without the problems you were facing :)