Skip to content
This repository has been archived by the owner on Feb 13, 2024. It is now read-only.

RFC: Add oca.decorators.onwrite #6

Open
yajo opened this issue Oct 31, 2017 · 8 comments
Open

RFC: Add oca.decorators.onwrite #6

yajo opened this issue Oct 31, 2017 · 8 comments
Labels

Comments

@yajo
Copy link
Member

yajo commented Oct 31, 2017

This is something I've been waiting for a long time, but now that we have this, I don't know which one would be the way to go.

Read the motivation and discussion on implementation from odoo/odoo#11042.

Summarizing, basically I'd love to legally abuse @api.constrains to have a method that gets executed whenever write() or create() are called and include the defined fields.

One way to do it would be just onwrite = odoo.api.constrains, and leave further work when a incompatibility arises, given the decorator can actually be abused.

Another way to do it would be to do it fine from the beginning. The decorator would be something like:

def onwrite(*fields):
    ...

When no fields come in, it is called on every write and create. When there are fields, it is called only when write and create dicts include keys those fields. Recursion should be prevented automatically.

The doubt is how to overload both methods automatically when all I have is a decorator... 🤔

Ideas?

@legalsylvain
Copy link

Hi @yajo. No anwser but another question :

  • do you know if @api.constrains is raised when an item is unlinked ?
  • did you planned to call @onwrite if item are unlink. (automatically or by option).

I regurlarly some needs when I have to overload create / write / unlink function to realize some checks.

regards.

@yajo
Copy link
Member Author

yajo commented Oct 31, 2017

the answer is no to both.

Unlink is a different scenario, where most checks usually don't matter, except the integrity checks from SQL itself. You also don't get vals when unlinking.

So at the best option it would be a different decorator, and you should be able to apply both to your method, but in most cases it wouldn't make much sense 🤔

@hbrunn
Copy link
Member

hbrunn commented Oct 31, 2017

I'm a friend of doing it right in the first place ;-) For how to set this up: I think this must come with a mixin, so that you can override https://github.com/OCA/OCB/blob/11.0/odoo/models.py#L501 et al there. Or the library detects when it's loaded into odoo and then patches BaseModel with everything the it needs.

@legalsylvain no, this can also become tricky when records are deleted by the database (ondelete='cascade'). So I think the decorator also shouldn't attempt to do anything there.

@yajo
Copy link
Member Author

yajo commented Oct 31, 2017

If that's the case, I'm wondering if this should be an addon instead 🤔

@lasley
Copy link
Contributor

lasley commented Oct 31, 2017

Hmmm yeah if it's a patch on the base model, then an addon would make sense because we have our environment isolation. Usage is a more tricky in that case though, yeah? Would have to be something like:

class A(models.Model):
    @models.BaseModel.onwrite('something')
    def meth(self):
        pass

^-- cuz no self and:

>>> class A(object):
...     a = A
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 2, in A
NameError: name 'A' is not defined

@yajo
Copy link
Member Author

yajo commented Nov 3, 2017

Yikes, so much time waiting for this repo, and it turns out it should have been an addon all the time!

@lasley
Copy link
Contributor

lasley commented Nov 3, 2017

The usage still seems a bit clunky and non-obvious to me in an addon IMO.

@yajo
Copy link
Member Author

yajo commented Nov 7, 2017

Then can we have a sane way of doing it without being an addon? I guess the best would be to have it in core. @rco-odoo said he's planning on adding it, maybe he could please report about proggress on that matter?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants