[17.0][MIG] sale_timesheet_invoice_description

[MarioLM-23]

Standard migration with changes of old references

[bosd]

Is it me or is there more going on in your first commit then just isort,prettier, black and pre-commit?

[bosd]

Some minor improvements

[MarioLM-23]

Is it me or is there more going on in your first commit then just isort,prettier, black and pre-commit?

Yes sorry I changed the version in manifest, in the test file the super setUpClass and added contributors

[MarioLM-23]

@bosd

I did the changes and improve the reference in xpath of configuration view, could you check it and let me know if I have done it correctly?
Thanks for the review!

[bosd]

I've encountered an error, while testing on the runboat the default description feature.
Steps to reproduce.
On a SO set the Description to empty.

Then create an invoice:

Error occurs:

Traceback (most recent call last):
  File "/opt/odoo/odoo/http.py", line 1768, in _serve_db
    return service_model.retrying(self._serve_ir_http, self.env)
  File "/opt/odoo/odoo/service/model.py", line 133, in retrying
    result = func()
  File "/opt/odoo/odoo/http.py", line 1795, in _serve_ir_http
    response = self.dispatcher.dispatch(rule.endpoint, args)
  File "/opt/odoo/odoo/http.py", line 1999, in dispatch
    result = self.request.registry['ir.http']._dispatch(endpoint)
  File "/opt/odoo/odoo/addons/base/models/ir_http.py", line 222, in _dispatch
    result = endpoint(**request.params)
  File "/opt/odoo/odoo/http.py", line 725, in route_wrapper
    result = endpoint(self, *args, **params_ok)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 28, in call_button
    action = self._call_kw(model, method, args, kwargs)
  File "/opt/odoo/addons/web/controllers/dataset.py", line 20, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 468, in call_kw
    result = _call_kw_multi(method, model, args, kwargs)
  File "/opt/odoo/odoo/api.py", line 453, in _call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/opt/odoo/addons/sale/wizard/sale_make_invoice_advance.py", line 180, in create_invoices
    invoices = self._create_invoices(self.sale_order_ids)
  File "/opt/odoo/addons/sale_timesheet/wizard/sale_make_invoice_advance.py", line 46, in _create_invoices
    return sale_orders.with_context(
  File "/mnt/data/odoo-addons-dir/sale_timesheet_invoice_description/models/sale.py", line 131, in _create_invoices
    desc_list = self._get_timesheet_description_list(ts_ids, desc_rule)
  File "/mnt/data/odoo-addons-dir/sale_timesheet_invoice_description/models/sale.py", line 41, in _get_timesheet_description_list
    details = self._get_timesheet_details(timesheet_id, desc_rule)
  File "/mnt/data/odoo-addons-dir/sale_timesheet_invoice_description/models/sale.py", line 29, in _get_timesheet_details
    if desc_rule[0] == "1":
TypeError: 'bool' object is not subscriptable

The above server error caused the following client error:
RPC_ERROR: Odoo Server Error
    at makeErrorFromResponse (http://oca-account-invoicing-17-0-pr1713-fd442d552117.runboat.odoo-community.org/web/assets/0ab7b66/web.assets_web.min.js:2870:163)
    at XMLHttpRequest.<anonymous> (http://oca-account-invoicing-17-0-pr1713-fd442d552117.runboat.odoo-community.org/web/assets/0ab7b66/web.assets_web.min.js:2874:13)

I was expecting that the default method would be used insted. Which s configured in configuration--> settings.

EDIT: My Bad, for using incorrect workflow.
When creating a a new SO. The correct description is applied. (the one defined under config/settings).
According to this error it is not allowed to have the empty value assigned.

[bosd]

My last test actually leads me to reproduce another error.
After installation of this module. The default Timesheet description is on the empty value.
When creating a new invoice. This empty value is assigned. Leading to afforementioned error.

Proposal:
Remove the Empy value from the Invoice methods.
By default assign the value [000] none

[MarioLM-23]

@bosd Hello and thank you for checking those errors
The default value was already set to none, to solve the error of setting the description to empty I set the field in the views as required, with this it is solved?
I first tried to set the field in the model as required but it was only applied in the order view

[bosd]

@mariobinhex Thanks for your patience and working on this bug.

I just discovered that this bug was already present in earlier versions.
So preferrebly the changes should be placed in a seperate commit for easy backport 🤗

The required flag works as inteded. It is no longer possible to assign the empty False value.
But, right after installation of this module. In configuration settings the value is still False.
We should set a default in python code. (just like on SO)
add
default="000"

at
https://github.com/OCA/account-invoicing/pull/1713/files#diff-3f0a25bc7aaab05ba3a84450130c78677a449cd2abbc4644e07786804d690a76R9-R13

If this module is installed in a DB with exsisting SO's. The error could still occur. (Because the value is False).
Or possibly when the SO is created via non UI.
For robustness we should adopt the code to allow for a False situation, I think changing this
https://github.com/OCA/account-invoicing/pull/1713/files#diff-26928443132d5ac74840f1c074ae427e24bdd060867bab3437730a4cc3e7c763R27-R35
to:

    def _get_timesheet_details(self, timesheet, desc_rule):
        details = []
        if desc_rule is False:
        return details
        if desc_rule[0] == "1":
            details.append(fields.Date.to_string(timesheet.date))
        if desc_rule[1] == "1":
            details.append(f"{timesheet.unit_amount} {timesheet.product_uom_id.name}")
        if desc_rule[2] == "1":
            details.append(timesheet.name)
        return details

Should do the trick.

[MarioLM-23]

@bosd Done! Please check my last commit when you can and tell me if any further changes are necessary

[bosd]

@mariobinhex Thanks, please move the required flags to the last commit.
(For easy cherry picking, backports)

Please change the commit message according to OCA guidelines
[IMP] sale_timesheet_invoice_description: Handle default description
Or a better description 😄

[MarioLM-23]

@bosd Done, thanks for your patience looking for my mistakes and improving the code

[bosd]

LGTM ✨ Thanks for contributing

[Dranyel-Bosd]

LGTM 👍 🎇

[remi-filament]

LGTM thank you for your work @MarioLM-23

[OCA-git-bot]

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

[pedrobaeza]

Please squash last 2 commits into one, as both belongs to the migration itself.

[bosd]

Please squash last 2 commits into one, as both belongs to the migration itself.

I've actually requested to have the fix of the bug I've discovered to be in a seperate commit.
(Bug was present in V15+, likely in 14 as well)
For easier cherry picking / backporting. (Which is now in the last commit)
@pedrobaeza Is that ok to you?

[pedrobaeza]

That commit contains changes in the README, which doesn't seem correct.