[FW][FIX] point_of_sale: keeping failed to sync orders as an attachment

[fw-bot]

(wrote by LSE)

Before this commit:
If an error happened when trying to synchronise a PoS order nothing is saved on the backend to inform the user regarding the error.
Note: Odoo logs would contain the information, but it is out of reach for certain clients (on odoo online for instance).

In theory, we can't lose any information as, if the sync process raise an exception, the order is still on the PoS browser cache that will then try to be resync when another order sync happen.
But, in practice, the support received some cases of "missing PoS orders". Which can happen as we fully rely on the client browser cache that can be cleared or use another computer/browser/session.

After this commit:
If an order can not be processed in the backend:

• the PoS order data is saved in the PoS session attachments
• a scheduled activity is created in the PoS session

As an un-synced keep being tried to be sync (and will likely fail each time), we compare it with the already attached one to avoid having the content repeated multiple times.
If the order was modified in between, a new attachment with the same name is created.

Note: draft orders that will fail to validate are NOT stored

The attachment and activity are automatically removed when the order of same reference is validated

opw-3650239

Forward-Port-Of: #155401
Forward-Port-Of: #147130

[robodoo]

.

[fw-bot]

@pedrambiria @caburj cherrypicking of pull request #147130 failed.

stdout:

Auto-merging addons/point_of_sale/models/pos_order.py
CONFLICT (content): Merge conflict in addons/point_of_sale/models/pos_order.py
Auto-merging addons/point_of_sale/models/pos_session.py
Auto-merging addons/point_of_sale/tests/__init__.py
Auto-merging addons/point_of_sale/tests/common.py

stderr:

16:40:17.729473 git.c:463               trace: built-in: git cherry-pick 823db558340d60c881b117352280bcc4a098d4d3
error: could not apply 823db558340d... [FIX] point_of_sale: keeping failed to sync orders as an attachment
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

⚠️ after resolving this conflict, you will need to merge it via @robodoo.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

[lse-odoo]

@xmo-odoo security override due to conflict (once once once again) please 🙏

@pedrambiria I propose to r+ the PR later when you will more available as 17.0 is used a lot

[xmo-odoo]

@lse-odoo

security override due to conflict (once once once again) please 🙏

https://docs.python.org/3/library/traceback.html#traceback.format_exception

Changed in version 3.5: The etype argument is ignored and inferred from the type of value.

As far as I can see from 3.5 onwards if there are 3 arguments the first one is just ignored, so you can pas None as first parameter, no need for the type() call.

[lse-odoo]

https://docs.python.org/3/library/traceback.html#traceback.format_exception

Changed in version 3.5: The etype argument is ignored and inferred from the type of value.

As far as I can see from 3.5 onwards if there are 3 arguments the first one is just ignored, so you can pas None as first parameter, no need for the type() call.

Indeed nice catch!
And I think we can go even further, as Odoo requires Python >= 3.10 to run, we can benefit from the new syntax of this function:

Changed in version 3.10: This function’s behavior and signature were modified to match [print_exception()](https://docs.python.org/3/library/traceback.html#traceback.print_exception).

On which we can just give the exception as a parameter

[lse-odoo]

@xmo-odoo is there still some security check that you would like to check ?

[lse-odoo]

TODO: see how we handle the order remove from the front end after failed to sync (as no call to the backend is done). Probably garbage collect them

[xmo-odoo]

@robodoo override=ci/security

[lse-odoo]

TODO 2: may be able to use: #161250 to distinguish for TODO1

[lse-odoo]

TODO-main: discussed with JCB: only orders that are NOT converted to draft interest us to keep. Others can be not stored (so just ignored like restaurant draft orders) as they are considered not fully validated. Depends on PEBR previous PR, so wait for it to be merged

[lse-odoo]

@pedrambiria would you mind reviewing the PR please ? :)

I used the option that you introduced on this part of the code to forward the information to the backend:
https://github.com/odoo/odoo/pull/157119/files#diff-86ab5d4b3803e1a7fa3373b7225bc610cfdcfc70fb413673b71d52a752706137R1266
( I could have used it in the orm call (like the draft parameter), but it was sounding quite unstable)

[lse-odoo]

@xmo-odoo would you mind giving me your opinion on this part of the code:

https://github.com/odoo/odoo/pull/157119/files#diff-86db45bc09231ecbc49a64405de92e0a022a6833980f46a9111501f8c8832760R2394

I'm pretty sure there is a cleaner/better way to do so. Would it be better to make the whole message a Markup maybe ?

[xmo-odoo]

would you mind giving me your opinion on this part of the code:

https://github.com/odoo/odoo/pull/157119/files#diff-86db45bc09231ecbc49a64405de92e0a022a6833980f46a9111501f8c8832760R2394

I'm pretty sure there is a cleaner/better way to do so. Would it be better to make the whole message a Markup maybe ?

You can use nl2br_enclose(message, 'code'), it will perform newline to <br> conversion (safely, by first escaping the message) then enclose that in a <code> tag.

[pedrambiria]

@lse-odoo you can add it to the context in _getCreateOrderContext

[lse-odoo]

would you mind giving me your opinion on this part of the code:
https://github.com/odoo/odoo/pull/157119/files#diff-86db45bc09231ecbc49a64405de92e0a022a6833980f46a9111501f8c8832760R2394
I'm pretty sure there is a cleaner/better way to do so. Would it be better to make the whole message a Markup maybe ?

You can use nl2br_enclose(message, 'code'), it will perform newline to <br> conversion (safely, by first escaping the message) then enclose that in a <code> tag.

I was not able to find a good way to use nl2br_enclose according to its code it break line for the string content you give it to it which will be encapsulated into the given tag. In my case there is no point as it only contains the string of the PoS reference which will never contain breaklines. I used instead nl2br on the overall Markup which looks more relevant. What do you think of this ?

[xmo-odoo]

I was not able to find a good way to use nl2br_enclose according to its code it break line for the string content you give it to it which will be encapsulated into the given tag. In my case there is no point as it only contains the string of the PoS reference which will never contain breaklines. I used instead nl2br on the overall Markup which looks more relevant. What do you think of this ?

I might have misread the order of operation in the original, sorry.

Looks good to me.

[lse-odoo]

@pedrambiria could you forward bot r+ this PR if you agree with the changes ? :)

[pedrambiria]

robodoo r+