pos
pos copied to clipboard
[ADD] 12.0 pos_mail_receipt module
Is this a backport of v13 functionality?
I did not know v13 included this. So this is not a backport.
This module allows you to send an e-mail to a customer without creating a partner, this is better to not force it to be more RGPD compliant. Also in this module you send the ticket after validating the payment and not before, which is more user friendly.
Thank you for this module !
- CSS for the email input should be changed a little to be centered
margin-left: 50px;
-
I didn't received mail from runbot (maybe that's normal ?)
-
And it was not clear if the mail was sent after clicking "OK". A green warming message "The email was sent" would be nice !
I "fixed" the textinput
You can see the failed sent mails in Settings -> Technical -> Email -> Emails (you need to be in debug mode)
I'll change the button color once clicked to avoid having to handle notifications (unless it is easy but I don't know how)
@legalsylvain I've addressed the problems you saw and added tests
@legalsylvain @quentinDupont I think this is in good shape now. I had trouble applying good CSS to the PDF generated by the backend, but in the end I got it to work
@PierrickBrun Could you explain why the force=True
option is used by default when the mail is sent, and when the create_from_ui()
with force=False
is used? I'm worried the session could be hindered when the email can't be sent.
@PierrickBrun Could you explain why the
force=True
option is used by default when the mail is sent, and when thecreate_from_ui()
withforce=False
is used? I'm worried the session could be hindered when the email can't be sent.
When the request is immediately sent (ie. The PoS is connected to the server), there is a force=True
to ensure the e-mail is sent right away. This is useful because you may need it to prove you did not steal or to have a free parking for instance.
In that case, since the record is already created when we decide to send the email or not, the email is created from a separated endpoint.
When the request is sent afterwards (ie. The PoS gets back connection) there is no force=True
because in that case it does not matter to send the email quickly.
In that case the email is sent with the same request as the record creation, in the create_from_ui
credit to @vvrossem for helping with this find!
Hello all, Hello @mclaeysb and @PierrickBrun I tested the last version on one of our test database.
- Send ticket to a customer who has an email adress. I clicked on the send receipt button, which became green. The email was sent correctly.
I configured some text on the header/footer in the pos config. Le sent ticket in attachment contains the modifications.
- Selling without selecting any customer. When I click on the button to sent the ticket, a pop up opens to indicate an email adress. The email was sent correclty.
Something somewhat surpising : it is possible to re-click on the button, and enter a new email adress. But no ticket will be sent again. The best would be to prevent the user to click again on the button. But still we can live with that at the moment.
- I cut the Wifi and made a sale. I was able to click on the button to sent the receipt. There was no error. The wifi signal was red with the number 1 (to inform there is 1 non synchronised sale). I re-activated the Wifi. I clicked on the 1 number. Everything was synchronised and the email was in the queue.
=> for me everyting works fine.
We need to upload it to PyPi. Is it ok? https://github.com/coopiteasy/pos/pull/36 @mclaeysb
Hi @cesarlr! For this PR we are waiting for the OCA (@legalsylvain @chienandalu ?) to consider it :)
some rebase should be done on this branch. (conflict, a lot of file modified, ...)
The branch is still messy unfortunately. Make sure you push with --force
after rebasing.
Ideally, for this PR, only 1 commit should be here: [ADD] pos_mail_receipt
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.
Hi @PierrickBrun , any chance to make a push rebase so that everyone could benefit from the huge amount of energy you spent on this?
Regards
Hi @flotho , done. I quickly tested it and it looks like nothing's been broken in the process
@legalsylvain This won't merge because travis is failing.
@PierrickBrun : could you take a look on travis ?
/ocabot rebase
@legalsylvain The rebase process failed, because command git push --force coopiteasy tmp-pr-428:12-pos_mail_receipt
failed with output:
remote: Permission to coopiteasy/pos.git denied to OCA-git-bot.
fatal: unable to access 'https://github.com/coopiteasy/pos/': The requested URL returned error: 403
@legalsylvain I'll rebase.
still red !
@legalsylvain The travis CI is red because of a timeout. Do you have an idea on how to fix this ?
@victor-champonnois : not sure that is a time out problem. when executing test of pos_mail_receipt
something seems blocked.
No output has been received in the last 10m0s
...
@PierrickBrun what do you think ?
Note to @carmenbianca : I did not explicitly resolve the promise in the code.
- I am not sure it is needed, since the screen is locked until the mail is sent (or raise an errors)
- Also after some trials I did not find a way to do it
- await does not work (I have an error telling me it must be used inside an async function/module/class, although I specifically added async to the function)
- using .resolve() in the
email
function does not work either (.resolve() is not a function
error)
@victor-champonnois : not sure that is a time out problem. when executing test of
pos_mail_receipt
something seems blocked.
No output has been received in the last 10m0s
...@PierrickBrun what do you think ?
@legalsylvain You're right, apparently the problem is related to a call to _wkhtmltopdf
in the tests.
@legalsylvain This is tricky. The timeout happens when the test call send_mail_receipt()
, itself calling _run_wkhtmltopdf
to convert the receipt to a pdf.
_run_wkhtmltopdf
run wkhtmltopdf in a subprocess.
I tried to run the command via the command line and my conclusion is that it is stuck because of the head:
<link type="text/css" rel="stylesheet" href="/web/content/572-85cd6e9/web.report_assets_pdf.0.css"/>
<link type="text/css" rel="stylesheet" href="/web/content/569-a6cdbb7/web.report_assets_common.0.css"/>
<link type="text/css" rel="stylesheet" href="/web/content/570-a6cdbb7/web.report_assets_common.1.css"/>
When I remove it everything is fine. I think these links cannot be accessed in the test environment. I don't know how to fix this though. It seems impossible to remove the header this high in the call stack. Maybe we need to simply remove this test ?
I dont know. @ivantodorovich any idea on that topic ?
I've never encountered this myself but I see something that bugs me here: https://github.com/OCA/pos/blob/df17842e7cdce02eae825c1133c905dc2ae884f5/.travis.yml#L16
I'm not sure that's the right way of adding wkhtmltopdf
dependency, as Odoo requires very specific versions of it.
On the oca repo template we have this: https://github.com/OCA/oca-addons-repo-template/blob/8f81ed76dd5ad0e315fb3b9729e3d7c9eeb4c867/src/%7B%25%20if%20ci%20%3D%3D%20'Travis'%20%25%7D.travis.yml%7B%25%20else%20%25%7D.t2d.yml%7B%25%20endif%20%25%7D.jinja#L67
Sadly the 12.0
branch is not using copier at all, but maybe you could try adding the travis dependency in the same way.
Not sure it'll fix the issue, but maybe it's worth trying.
I tried to run the command via the command line and my conclusion is that it is stuck because of the head ... https://github.com/OCA/pos/pull/428#issuecomment-1164544986
Maybe wkhtmltopdf
doesn't have access to the odoo server on the test environment? Kind of reminds me of the issues you can get due to misconfiguration and report.url
system parameter
@ivantodorovich Thanks a lot, it works :)
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). 🤖