-
-
Notifications
You must be signed in to change notification settings - Fork 370
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.
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
[10.0][ADD] Add default analytic account for accounts #152
[10.0][ADD] Add default analytic account for accounts #152
Conversation
Add the ability to set default analytic account on accounts. This is useful when, e.g. you use the account_analytic_required and have multi-currencies, thus need to create an automatic currency exchange difference entry.
README.md
Outdated
@@ -18,6 +18,7 @@ Available addons | |||
---------------- | |||
addon | version | summary | |||
--- | --- | --- | |||
[account_analytic_default_account](account_analytic_default_account/) | 10.0.1.0.0 | Add default analytic account for accounts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skip it: this part is updated automatically ;)
Configuration | ||
============= | ||
|
||
For example, if you want to have an analytic account whenever you use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skip the 1st part. Just state "Go to Accounting..."
account_id = fields.Many2one( | ||
'account.account', string='Account', | ||
ondelete='cascade', | ||
help="Select an account which will use analytic account specified in " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you rephrase this? The way I get is: "This account will be used by default for sale orders or invoices" or something similar :)
) | ||
|
||
@api.model | ||
def account_get(self, product_id=None, partner_id=None, user_id=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd split this in more methods and simplify it a bit, for instance:
account_get_domain_keys = (
'product_id', 'partner_id', 'user_id',
'date', 'company_id', 'account_id',
)
def _account_get_domain(self, **kw):
"""Build account.analytic.default domain.
:param kw: filter keys.
Available filter keys are defined into `account_get_domain_keys`.
:return: a search domain with all required keys' leaves.
Each key will be searched for both False and specified value.
For instance: [
('product_id'), '=', False), '|', [('product_id'), '=', 100)
]
"""
domain = []
account_get_domain_keys = self.account_get_domain_keys[:]
# date will be handled differently
date_key = account_get_domain_keys.pop('date', None)
for key in account_get_domain_keys:
if kw.get(key):
domain += ['|', (key, '=', kw.get(key))]
domain += [(key, '=', False)]
if date_key and kw.get(date_key):
domain += ['|', ('date_start', '<=', kw.get(date_key)),
('date_start', '=', False)]
domain += ['|', ('date_stop', '>=', kw.get(date_key)),
('date_stop', '=', False)]
return domain
def account_get(self, **kw):
domain = self._account_get_domain(**kw)
best_index = -1
res = self.env['account.analytic.default']
for rec in self.search(domain):
index = 0
for k in kw.keys():
if rec[k]:
index += 1
if index > best_index:
res = rec
best_index = index
return res
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't change the method signature so drastically as it already exists in the Odoo addons. I can't guarantee that the calls are made with kwargs everywhere. That's the main reason I copied the method and updated it for my needs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine, you are right. Keep the signature as it is... but respect it in your calls if possible :)
Then you can still address the rest I guess ;)
def _onchange_product_id(self): | ||
res = super(AccountInvoiceLine, self)._onchange_product_id() | ||
rec = self.env['account.analytic.default'].account_get( | ||
self.product_id.id, self.invoice_id.partner_id.id, self.env.uid, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the method is supposed to accept keyword args
def _set_additional_fields(self, invoice): | ||
if not self.account_analytic_id: | ||
rec = self.env['account.analytic.default'].account_get( | ||
self.product_id.id, self.invoice_id.partner_id.id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
would you mind adding some tests for this? |
bcde952
to
deac001
Compare
@simahawk Thanks for the suggestions! I also added some tests |
"""Returns a set of original account_get arg names - self + new ones""" | ||
func = super(AccountAnalyticDefaultAccount, self).account_get | ||
return ( | ||
list(func.func_code.co_varnames[1:func.func_code.co_argcount]) + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
???
('date_stop', '=', False)] | ||
return domain | ||
|
||
def _account_get(self, **kw): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docstring explaining how you get to the right account
@api.model | ||
def account_get(self, *args, **kwargs): | ||
"""Wrapper method to convert *args to **kwargs""" | ||
kwargs.update(zip(self._account_get_domain_keys(), args)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use black magic. If the original method has some specific keys, use them.
cb00ce9
to
d117e7a
Compare
d117e7a
to
9fd8dfc
Compare
@api.model | ||
def account_get(self, product_id=None, partner_id=None, user_id=None, | ||
date=None, company_id=None, account_id=None): | ||
"""Wrapper method to convert positional args to kwargs""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need any wrapping:
@api.model
def account_get(self, product_id=None, partner_id=None, user_id=None,
date=None, company_id=None, account_id=None):
filters = {
'product_id': product_id,
'partner_id': partner_id,
'user_id': user_id,
'date': date,
'company_id': company_id,
'account_id': account_id,
}
domain = self._account_get_domain(**filters)
keys = filters.keys()
[...]
] | ||
""" | ||
domain = [] | ||
account_get_domain_keys = self.account_get_domain_keys[:] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need this anymore as the keys are controlled by the main method signature
def _account_get_domain(self, **kw):
"""Build account.analytic.default domain.
:param kw: filter keys.
Available filter keys are defined into `account_get_domain_keys`.
:return: a search domain with all required keys' leaves.
Each key will be searched for both False and specified value.
For instance: [
('product_id'), '=', False), '|', [('product_id'), '=', 100)
]
"""
domain = []
date_val = kw.pop('date', None)
for key, value in kw.items():
if value is not None:
domain += ['|', (key, '=', value)]
domain += [(key, '=', False)]
if date_val:
domain += ['|', ('date_start', '<=', date_val),
('date_start', '=', False)]
domain += ['|', ('date_stop', '>=', date_val),
('date_stop', '=', False)]
return domain
7c8decb
to
e4a1770
Compare
remember to squash before merge |
cannot test it - runbot does not initate as normally :\ |
@pedrobaeza can you please metge this, i tested on tunbot as well.. |
Add the ability to set default analytic account on accounts. This is useful when, e.g. you use the account_analytic_required and have multi-currencies, thus need to create an automatic currency exchange difference entry.
Add the ability to set default analytic account on accounts. This is useful when, e.g. you use the account_analytic_required and have multi-currencies, thus need to create an automatic currency exchange difference entry.
Add the ability to set default analytic account on accounts. This is useful when, e.g. you use the account_analytic_required and have multi-currencies, thus need to create an automatic currency exchange difference entry.
Add the ability to set default analytic account on accounts.
This is useful when, e.g. you use the account_analytic_required
and have multi-currencies, thus need to create an automatic
currency exchange difference entry.