Skip to content

[FIX] keep _register_hook's signature and call super #352

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.

Sign up for GitHub

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

Merged
hbrunn merged 3 commits into from
Feb 13, 2016
Merged

[FIX] keep _register_hook's signature and call super #352

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a28aa2
[FIX] keep _register_hook's signature and call super
StefanRijnhart Jan 26, 2016
7fcf65c
[FIX] Odoolint
StefanRijnhart Jan 26, 2016
ed4a62b
REF using _patch_method for name_create
eLBati Feb 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions base_optional_quick_create/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,41 +23,43 @@
from openerp.tools.translate import _


class ir_model(orm.Model):
class IrModel(orm.Model):
_inherit = 'ir.model'

_columns = {
'avoid_quick_create': fields.boolean('Avoid quick create'),
}

def _wrap_name_create(self, old_create, model):
def wrapper(cr, uid, name, context=None):
raise orm.except_orm(
_('Error'),
_("Can't create quickly. Opening create form"))
return wrapper
def _patch_quick_create(self, cr, ids):

def _wrap_name_create():
def wrapper(self, cr, uid, name, context=None):
raise orm.except_orm(
_('Error'),
_("Can't create quickly. Opening create form"))
return wrapper

def _register_hook(self, cr, ids=None):
if ids is None:
ids = self.search(cr, SUPERUSER_ID, [])
for model in self.browse(cr, SUPERUSER_ID, ids):
if model.avoid_quick_create:
model_name = model.model
model_obj = self.pool.get(model_name)
if model_obj and not hasattr(model_obj, 'check_quick_create'):
model_obj.name_create = self._wrap_name_create(
model_obj.name_create, model_name)
model_obj._patch_method('name_create', _wrap_name_create())
model_obj.check_quick_create = True
return True

def _register_hook(self, cr):

@yajo yajo Feb 8, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you use _patch_method() instead?

@eLBati eLBati Feb 10, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a suggestion about how to do it in this case? (Where _register_hook has 2 parameters and _patch_quick_create 3)

@StefanRijnhart StefanRijnhart Feb 10, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @yajo means to replace the contents of _patch_quick_create with a call to _patch_method. It basically does the same thing. You'd still trigger _patch_quick_create from _register_hook.

@StefanRijnhart StefanRijnhart Feb 10, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check base_action_rule for an example of how to use _patch_method.

@yajo yajo Feb 11, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I meant that. It seems the standard way to do it.

@eLBati eLBati Feb 11, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks. I modified it

self._patch_quick_create(cr, self.search(cr, SUPERUSER_ID, []))
return super(IrModel, self)._register_hook(cr)

def create(self, cr, uid, vals, context=None):
res_id = super(ir_model, self).create(cr, uid, vals, context=context)
self._register_hook(cr, [res_id])
res_id = super(IrModel, self).create(cr, uid, vals, context=context)
self._patch_quick_create(cr, [res_id])

@yajo yajo Feb 8, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make any sense to patch the creation method after the record has been created?

@StefanRijnhart StefanRijnhart Feb 10, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is on a different level. After the creation of the model, we patch the model's name_create method which applies when records of that model are being created through the quick create option.

@eLBati eLBati Feb 10, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure to understand.
Consider that this is the create of a new model. That is: when a new model is created, patch its name_create method

Edit: I did not see @StefanRijnhart answer 😏

@yajo yajo Feb 11, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still it does not seem a good trigger point to do it. If you quick-create a record before slow-creating one, it would still enable it.

The logical flow IMHO should be like:

_inherit = 'ir.model'
@api.cr
def _register_hook(self):
  self.search([("avoid_quick_create", "=", True)])._patch_quick_create()
  return super(...)._register_hook()

@api.multi
def _patch_quick_create(self):
  @api.model
  def new_name_create_method(self):
    ...
  for s in self:
    self.env[s.model]._patch_method("name_create", new_name_create)

And add a constraint to avoid_quick_create column that patches or restores the method depending on what changed.

@eLBati eLBati Feb 11, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still it does not seem a good trigger point to do it. If you quick-create a record before slow-creating one, it would still enable it

Why? I don't think this happens.

Consider these are the create and write methods of ir.model model, not of the model itself.

For example: when the account.invoice (record of ir.model) is modified, setting avoid_quick_create = True, patch name_create of account.invoice model

@yajo yajo Feb 12, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope you get my point with eLBati#3. I did not test it, just showing what I mean.

return res_id

def write(self, cr, uid, ids, vals, context=None):
if isinstance(ids, (int, long)):
ids = [ids]
res = super(ir_model, self).write(cr, uid, ids, vals, context=context)
self._register_hook(cr, ids)
res = super(IrModel, self).write(cr, uid, ids, vals, context=context)
self._patch_quick_create(cr, ids)

@yajo yajo Feb 8, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the relation between patching name_create and write()?

@StefanRijnhart StefanRijnhart Feb 10, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name_create should be patched when the model has avoid_quick_create set. Strictly speaking, patching should only take place when this field is being written.

@eLBati eLBati Feb 10, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/OCA/server-tools/pull/352/files#r52468995

return res