[IMP] l10n_id: show DPP value #202832
[IMP] l10n_id: show DPP value #202832
Conversation
|
|
3ed0db6 to
e8ce284
Compare
e8ce284 to
b5245ec
Compare
There was a problem hiding this comment.
Hello,
I did not look much into it from the technical side.
But I did a first functional test and have some feedback and questions.
(0) module
Should we not do this in l10n_id directly?
Since it affects all users of the localization.
(1) tax name
Should we not also rename the 11% tax?
So that it says 12% on the invoice?
Because if we can just leave 11% why do we go through all the effort to display the adjusted base amount?
(2) tax groups
Should the 11% and 12% tax not have a different tax group? (according to the task)
This way they would be displayed separately in the widget
(See also point (4)).
(3) multiple tax groups on single invoice
I think it does not work correctly in case we mix different tax groups.
I.e. consider the case of 1 line with 11% and 1 line with 20% on the same invoice:
There is a traceback when trying to print (or trying to open the "Send & Print") dialog:
RPC_ERROR
Odoo Server Error
Traceback (most recent call last):
File "<667>", line 143, in template_667
File "<667>", line 58, in template_667_content
KeyError: 'hide_base_amount'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/odoo/src/odoo/odoo/http.py", line 1658, in _serve_db
return service_model.retrying(self._serve_ir_http, self.env)
File "/home/odoo/src/odoo/odoo/service/model.py", line 152, in retrying
result = func()
File "/home/odoo/src/odoo/odoo/http.py", line 1686, in _serve_ir_http
response = self.dispatcher.dispatch(rule.endpoint, args)
File "/home/odoo/src/odoo/odoo/http.py", line 1890, in dispatch
result = self.request.registry['ir.http']._dispatch(endpoint)
File "/home/odoo/src/odoo/odoo/addons/base/models/ir_http.py", line 154, in _dispatch
result = endpoint(**request.params)
File "/home/odoo/src/odoo/odoo/http.py", line 734, in route_wrapper
result = endpoint(self, *args, **params_ok)
File "/home/odoo/src/odoo/addons/web/controllers/dataset.py", line 43, in call_kw
return self._call_kw(model, method, args, kwargs)
File "/home/odoo/src/odoo/addons/web/controllers/dataset.py", line 34, in _call_kw
return call_kw(Model, method, args, kwargs)
File "/home/odoo/src/odoo/odoo/api.py", line 484, in call_kw
result = _call_kw_multi(method, model, args, kwargs)
File "/home/odoo/src/odoo/odoo/api.py", line 469, in _call_kw_multi
result = method(recs, *args, **kwargs)
File "/home/odoo/src/odoo/odoo/models.py", line 6658, in onchange
record._onchange_eval(name, field_onchange[name], result)
File "/home/odoo/src/odoo/odoo/models.py", line 6369, in _onchange_eval
method_res = method(self)
File "/home/odoo/src/odoo/addons/account/wizard/account_invoice_send.py", line 95, in onchange_is_email
self.composer_id._onchange_template_id_wrapper()
File "/home/odoo/src/odoo/addons/mail/wizard/mail_compose_message.py", line 178, in _onchange_template_id_wrapper
values = self._onchange_template_id(self.template_id.id, self.composition_mode, self.model, self.res_id)['value']
File "/home/odoo/src/odoo/addons/mail/wizard/mail_compose_message.py", line 598, in _onchange_template_id
values = self.generate_email_for_composer(
File "/home/odoo/src/odoo/addons/mail/wizard/mail_compose_message.py", line 732, in generate_email_for_composer
template_values = self.env['mail.template'].with_context(tpl_partners_only=True).browse(template_id).generate_email(res_ids, fields)
File "/home/odoo/src/odoo/addons/account_edi/models/mail_template.py", line 39, in generate_email
res = super().generate_email(res_ids, fields)
File "/home/odoo/src/odoo/addons/mail/models/mail_template.py", line 286, in generate_email
result, report_format = self.env['ir.actions.report']._render_qweb_pdf(report, [res_id])
File "/home/odoo/src/odoo/addons/account/models/ir_actions_report.py", line 61, in _render_qweb_pdf
return super()._render_qweb_pdf(report_ref, res_ids=res_ids, data=data)
File "/home/odoo/src/odoo/odoo/addons/base/models/ir_actions_report.py", line 907, in _render_qweb_pdf
collected_streams = self._render_qweb_pdf_prepare_streams(report_ref, data, res_ids=res_ids)
File "/home/odoo/src/enterprise/account_followup/models/ir_actions_report.py", line 12, in _render_qweb_pdf_prepare_streams
res = super()._render_qweb_pdf_prepare_streams(report_ref, data, res_ids)
File "/home/odoo/src/odoo/addons/account_edi_ubl_cii/models/ir_actions_report.py", line 65, in _render_qweb_pdf_prepare_streams
collected_streams = super()._render_qweb_pdf_prepare_streams(report_ref, data, res_ids=res_ids)
File "/home/odoo/src/odoo/addons/account_edi/models/ir_actions_report.py", line 14, in _render_qweb_pdf_prepare_streams
collected_streams = super()._render_qweb_pdf_prepare_streams(report_ref, data, res_ids=res_ids)
File "/home/odoo/src/odoo/addons/account/models/ir_actions_report.py", line 20, in _render_qweb_pdf_prepare_streams
return super()._render_qweb_pdf_prepare_streams(report_ref, data, res_ids=res_ids)
File "/home/odoo/src/odoo/odoo/addons/base/models/ir_actions_report.py", line 758, in _render_qweb_pdf_prepare_streams
html = self.with_context(**additional_context)._render_qweb_html(report_ref, all_res_ids_wo_stream, data=data)[0]
File "/home/odoo/src/odoo/odoo/addons/base/models/ir_actions_report.py", line 982, in _render_qweb_html
return self._render_template(report.report_name, data), 'html'
File "/home/odoo/src/odoo/odoo/addons/base/models/ir_actions_report.py", line 661, in _render_template
return view_obj._render_template(template, values).encode()
File "/home/odoo/src/odoo/odoo/addons/base/models/ir_ui_view.py", line 2135, in _render_template
return self.env['ir.qweb']._render(template, values)
File "/home/odoo/src/odoo/odoo/tools/profiler.py", line 294, in _tracked_method_render
return method_render(self, template, values, **options)
File "/home/odoo/src/odoo/odoo/addons/base/models/ir_qweb.py", line 593, in _render
result = ''.join(rendering)
File "<669>", line 90, in template_669
File "<669>", line 72, in template_669_content
File "<669>", line 60, in template_669_t_call_0
File "<665>", line 2153, in template_665
File "<665>", line 2135, in template_665_content
File "<665>", line 1442, in template_665_t_call_0
File "<666>", line 155, in template_666
File "<666>", line 87, in template_666_content
File "<667>", line 149, in template_667
odoo.addons.base.models.ir_qweb.QWebException: Error while render the template
KeyError: 'hide_base_amount'
Template: account.tax_groups_totals
Path: /t/t/tr/t[1]/td[1]/span[2]
Node: <span t-if="not amount_by_group[\'hide_base_amount\']" class="text-nowrap"/>
Compiled code:
code = None
template = 'account.tax_groups_totals'
template_functions = {}
def template_667_content(self, values):
# element: '/t' , '<t/>'
attrs = values['__qweb_attrs__'] = {}
# element: '/t/t' , '<t t-foreach="tax_totals[\'groups_by_subtotal\'][subtotal_to_show]" t-as="amount_by_group"/>'
yield '\n '
template_667_t_foreach_0 = ((values.get('tax_totals')['groups_by_subtotal'][values.get('subtotal_to_show')])) or []
if isinstance(template_667_t_foreach_0, Sized):
values['amount_by_group_size'] = template_667_size_1 = len(template_667_t_foreach_0)
elif (template_667_t_foreach_0).__class__ == int:
values['amount_by_group_size'] = template_667_size_1 = template_667_t_foreach_0
template_667_t_foreach_0 = range(template_667_size_1)
else:
template_667_size_1 = None
template_667_has_value_2 = False
if isinstance(template_667_t_foreach_0, Mapping):
template_667_t_foreach_0 = template_667_t_foreach_0.items()
template_667_has_value_2 = True
for index, item in enumerate(template_667_t_foreach_0):
values['amount_by_group_index'] = index
if template_667_has_value_2:
values['amount_by_group'], values['amount_by_group_value'] = item
else:
values['amount_by_group'] = values['amount_by_group_value'] = item
values['amount_by_group_first'] = values['amount_by_group_index'] == 0
if template_667_size_1 is not None:
values['amount_by_group_last'] = index + 1 == template_667_size_1
values['amount_by_group_odd'] = index % 2
values['amount_by_group_even'] = not values['amount_by_group_odd']
values['amount_by_group_parity'] = 'odd' if values['amount_by_group_odd'] else 'even'
attrs = values['__qweb_attrs__'] = {}
# element: '/t/t/tr/t[1]' , '<t t-if="tax_totals[\'display_tax_base\']"/>'
yield '\n <tr>'
if ((values.get('tax_totals')['display_tax_base'])):
attrs = values['__qweb_attrs__'] = {}
# element: '/t/t/tr/t[1]/td[1]/span[1]' , '<span t-esc="amount_by_group[\'tax_group_name\']"/>'
attrs = values['__qweb_attrs__'] = {}
yield '\n <td>\n '
content = ((values.get('amount_by_group')['tax_group_name']))
if content is not None and content is not False:
yield '<span'
attrs = values.pop('__qweb_attrs__', None)
if attrs:
tagName = 'span'
attrs = self._post_processing_att(tagName, attrs)
for name, value in attrs.items():
if value or isinstance(value, str):
yield f' {escape(str(name))}="{escape(str(value))}"'
yield '>'
yield str(escape(content))
yield '</span>'
# element: '/t/t/tr/t[1]/td[1]/span[2]' , '<span t-if="not amount_by_group[\'hide_base_amount\']" class="text-nowrap"/>'
yield ''
if ((not values.get('amount_by_group')['hide_base_amount'])):
attrs = values['__qweb_attrs__'] = {}
attrs['class'] = 'text-nowrap'
yield '\n <span'
attrs = values.pop('__qweb_attrs__', None)
if attrs:
tagName = 'span'
attrs = self._post_processing_att(tagName, attrs)
for name, value in attrs.items():
if value or isinstance(value, str):
yield f' {escape(str(name))}="{escape(str(value))}"'
# element: '/t/t/tr/t[1]/td[1]/span[2]/t' , '<t t-esc="amount_by_group[\'formatted_tax_group_base_amount\']"/>'
attrs = values['__qweb_attrs__'] = {}
yield '> on\n '
content = ((values.get('amount_by_group')['formatted_tax_group_base_amount']))
if content is not None and content is not False:
yield str(escape(content))
yield '\n </span>'
# element: '/t/t/tr/t[1]/td[2]/span' , '<span class="text-nowrap" t-esc="amount_by_group[\'formatted_tax_group_amount\']"/>'
attrs = values['__qweb_attrs__'] = {}
attrs['class'] = 'text-nowrap'
yield '\n </td>\n <td class="text-end o_price_total">\n '
content = ((values.get('amount_by_group')['formatted_tax_group_amount']))
if content is not None and content is not False:
yield '<span'
attrs = values.pop('__qweb_attrs__', None)
if attrs:
tagName = 'span'
attrs = self._post_processing_att(tagName, attrs)
for name, value in attrs.items():
if value or isinstance(value, str):
yield f' {escape(str(name))}="{escape(str(value))}"'
yield '>'
yield str(escape(content))
yield '</span>'
yield '\n </td>'
else:
# element: '/t/t/tr/t[2]' , '<t t-else="" t-else-valid="True"/>'
attrs = values['__qweb_attrs__'] = {}
# element: '/t/t/tr/t[2]/td[1]/span' , '<span class="text-nowrap" t-esc="amount_by_group[\'tax_group_name\']"/>'
attrs = values['__qweb_attrs__'] = {}
attrs['class'] = 'text-nowrap'
yield '\n <td>'
content = ((values.get('amount_by_group')['tax_group_name']))
if content is not None and content is not False:
yield '<span'
attrs = values.pop('__qweb_attrs__', None)
if attrs:
tagName = 'span'
attrs = self._post_processing_att(tagName, attrs)
for name, value in attrs.items():
if value or isinstance(value, str):
yield f' {escape(str(name))}="{escape(str(value))}"'
yield '>'
yield str(escape(content))
yield '</span>'
# element: '/t/t/tr/t[2]/td[2]/span' , '<span class="text-nowrap" t-esc="amount_by_group[\'formatted_tax_group_amount\']"/>'
attrs = values['__qweb_attrs__'] = {}
attrs['class'] = 'text-nowrap'
yield '</td>\n <td class="text-end o_price_total">\n '
content = ((values.get('amount_by_group')['formatted_tax_group_amount']))
if content is not None and content is not False:
yield '<span'
attrs = values.pop('__qweb_attrs__', None)
if attrs:
tagName = 'span'
attrs = self._post_processing_att(tagName, attrs)
for name, value in attrs.items():
if value or isinstance(value, str):
yield f' {escape(str(name))}="{escape(str(value))}"'
yield '>'
yield str(escape(content))
yield '</span>'
yield '\n </td>'
yield '\n </tr>'
yield ''
def template_667(self, values):
try:
if '__qweb_loaded_values' not in values:
values['__qweb_loaded_values'] = {}
values['__qweb_root_values'] = values.copy()
values['xmlid'] = 'account.tax_groups_totals'
values['viewid'] = 667
values['__qweb_loaded_values'].update(template_functions)
yield from template_667_content(self, values)
except QWebException:
raise
except Exception as e:
if isinstance(e, TransactionRollbackError):
raise
raise QWebException("Error while render the template",
self, template, ref=667, code=code) from e
template_functions['template_667_content'] = template_667_content
The above server error caused the following client error:
null
(4) adjusted and unadjusted base amounts on single invoice
Does it work as intended for the cases where only some of the base amount is adjusted?
If I understand it correctly the base is only adjusted for some cases.
In these cases we would use the 11% tax in odoo (but in reality it is a 12% tax on 11/12 of the base).
So in case I use the 11% and 12% taxes together (1 line with 11% and 1 line with 12%) the DPP value is currently not really useful.
(It is adapted for both base amounts but should only be adjusted for one of them. So we cannot easily deduce any of the base amounts.)
Should the DPP value not only reflect the reduced base amounts?
Or maybe we should have 2 values? One "normal" and one adjusted?
On invoice PDFs we already display different base amounts (in case of different tax groups I think):
It is handled by the 'display_tax_base' key in prepare_tax_totals
Maybe we could also just "hijack" that instead?
I think currently it would display sth. like "11% on {100% base} amount" in such a case.
So we would maybe have to adapt it anyway? To say "12% on {11/12 base} amount"?
(I am not sure)
Currently we do not display the different base amounts in the multiple base amounts case on the move form view if I am not mistaken.
Maybe we could do a more general improvement of the widget to show base amounts (so that we can use it here)?
Could also be overkill^^
(5) runbot
The runbot is red. Could you please check?
(May be unrelated though but it is an accounting module)
|
@svfu-odoo thank you for the review and insight! I will have a look here and let you know if there is any update or question from my side |
b5245ec to
8440553
Compare
8440553 to
0b9b627
Compare
|
@svfu-odoo we're currently still doing internal testing but thank you for the early reviews! I will re-request for review when it's been functionally validated! |
d5e4cb9 to
7e96bfc
Compare
ea3089d to
b6e7cfb
Compare
b6e7cfb to
96a4858
Compare
|
@svfu-odoo I've also made this upgrade PR https://github.com/odoo/upgrade/pull/7637 to add the tax groups, I'm not sure if there's anything else needed to be updated. |
|
@svfu-odoo btw for some reason, when I push branch 16.0-dpp_change-nni in upgrade, they say invalid version |
There was a problem hiding this comment.
Thanks for the update.
The "normal" code looks good to me 👍
But I think the migration script does not work correctly currently (wrong xmlids of the taxes).
Also I am not sure whether it is okay to just set the tax group in all cases. The user may have modified the tax and we should probably not mess with that.
I left some comments.
But I will also check with the team on how to handle this case.
|
|
||
| # Ensure tax_ST1 and tax_PT1 is set to the corret tax group to allow the 11/12 tax base calculation | ||
| tax_group_id = env.ref("l10n_id.l10n_id_tax_group_non_luxury_goods").id | ||
| for tax in ["tax_ST1", "tax_PT1"]: |
There was a problem hiding this comment.
Those are the names of the tax templates and not the taxes itself (in 16.0 the taxes are created from the templates).
The xmlid of the taxes contain the company id.
So we have to update the taxes on a per company basis. Or at least account for that somehow
You can check the xmlids of all taxes like this e.g.
[f'{data.module}.{data.name}' for data in env['ir.model.data'].search([('model', '=', 'account.tax')])]So for company with id 1 it would be:
l10n_id.1_tax_ST1l10n_id.1_tax_PT1
The xmlids would look sth like this
id_chart = env.ref('l10n_id.l10n_id_chart')
companies = env['res.company'].search([('chart_template_id', 'child_of', id_chart.id)])
for company in companies:
tax_xmlids = (
f'l10n_id.{company.id}_tax_ST1',
f'l10n_id.{company.id}_tax_PT1',
)Note on FW-ports / 17.0+
The tax groups will also have a company_id and be handled "the same as taxes" in 17.0+
They will also have to updated on per company basis
I think in 17.0+ the module part of the xmlid of the taxes and tax groups will be account for both taxes and tax groups.
See this helper function
odoo/addons/account/models/chart_template.py
Line 1077 in fdb8bc8
|
Also please upate the commit / PR title / message. The module should be |
7d87475 to
1d5bd2b
Compare
10000
addons/l10n_id/migrations/1.2/end-migrate_update_taxes.py
Outdated
Show resolved
Hide resolved
1d5bd2b to
cb4bbab
Compare
|
Thanks for the feedbacks! @svfu-odoo |
|
LGTM except for the ci/style issues on the runbot. |
Due to government regulations, tax base amount is multiplied by factor of 11/12 and tax value of 12% resulting to essentially 11% of tax. We need to display this DPP value both on invoice form view and reports 4485693
cb4bbab to
a82122f
Compare
|
Hello @svfu-odoo , I'm wondering if there's been anyone else available to take a look yet? Few of our customers in Indonesia have asked updates about this feature 😅 |
| for subtotal_group in move.tax_totals['groups_by_subtotal'].values(): | ||
| for group in subtotal_group: | ||
| if group['tax_group_id'] == non_luxury_tax_group.id: | ||
| dpp = group['tax_group_base_amount'] * (11 / 12) |
There was a problem hiding this comment.
Still wondering if there is no way to make it more generic by putting the 11/12 in the factor_percent of the tax repartition line...
There was a problem hiding this comment.
I found a crazy way to do this in odoo standard.
-
Tax: DPP- (This tax reduces the base amount)
Amount: -8.333%
Sequence: 0
Tax Group: DPP
Affect Base of subsequent: True -
Tax: 12% (This is a standard 12% tax, and uses the base affected by DPP-)
Amount: 12%
Sequence: 1
Tax Group: 12%
Base Affected by previous: True -
Tax: DPP+ (This adds back the first DPP tax)
Amount: 8.333%
Sequence: 2
Tax Group: DPP
Base Affected by previous: False
End Result:
IMO neither solution is ideal
- This PR has an 11% tax pretending to be 12% - I think it's going to cause problems in the long term, and is not ideal for a user who chooses 11% but ends up with 12%
- Standard solution involves too many taxes and will result in a poor UX
| @@ -1,12 +1,13 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <odoo> | |||
| <odoo noupdate="1"> | |||
| @@ -34,8 +35,9 @@ | |||
| ]"/> | |||
| </record> | |||
| <record id="tax_PT1" model="account.tax.template"> | |||
| <field name="description">11%</field> | |||
| <field name="description">12%</field> | |||
There was a problem hiding this comment.
Having a 12% description on an 11% tax is super weird and very confusing
| @@ -1,7 +1,23 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <odoo noupdate="1"> | |||
There was a problem hiding this comment.
if a user deletes the tax group - the feature is broken and can't be fixed even if they update the module
Due to government regulations, tax base amount is multiplied by factor of 11/12 and tax value of 12% resulting to essentially 11% of tax. We need to display this DPP value both on invoice form view and reports 4485693 closes #202832 Signed-off-by: Habib Ayob (ayh) <ayh@odoo.com>
Due to government regulations, tax base amount is multiplied by factor of 11/12 and tax value of 12% resulting to essentially 11% of tax. We need to display this DPP value both on invoice form view and reports
4485693
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr