Skip to content
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

Invitation email #1177

Merged
merged 9 commits into from Jul 7, 2020
Merged

Invitation email #1177

merged 9 commits into from Jul 7, 2020

Conversation

Ironicbay
Copy link
Contributor

@Ironicbay Ironicbay commented Jun 8, 2020

I've written quite a lot of comments to highlight the issues I'm facing, mostly :

  • I'm unsure about the place I've put the modifications in, if it should be inside the class or outside
  • The email HTML text is huge and I'm not sure where it should be, clearly it's not ideal right now
  • I don't know what email address will be used to send it, and I don't know how to secure its password while keeping the process automated

@codecov
Copy link

codecov bot commented Jun 8, 2020

Codecov Report

Merging #1177 into master will increase coverage by 0.27%.
The diff coverage is 45.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1177      +/-   ##
==========================================
+ Coverage   80.21%   80.49%   +0.27%     
==========================================
  Files         283      284       +1     
  Lines       24082    25195    +1113     
==========================================
+ Hits        19318    20281     +963     
- Misses       4764     4914     +150     
Impacted Files Coverage Δ
parsec/backend/invite.py 84.59% <30.18%> (-10.56%) ⬇️
parsec/backend/cli/run.py 70.80% <45.00%> (-0.63%) ⬇️
parsec/backend/client_context.py 94.52% <66.66%> (-2.50%) ⬇️
parsec/backend/config.py 93.54% <100.00%> (+1.39%) ⬆️
parsec/backend/memory/factory.py 100.00% <100.00%> (ø)
parsec/backend/postgresql/factory.py 100.00% <100.00%> (ø)
...c/backend/postgresql/realm_queries/update_roles.py 94.59% <0.00%> (-1.16%) ⬇️
parsec/core/gui/trio_thread.py 90.55% <0.00%> (-1.12%) ⬇️
parsec/core/fs/workspacefs/entry_transactions.py 96.47% <0.00%> (-0.59%) ⬇️
parsec/core/gui/files_widget.py 74.86% <0.00%> (-0.54%) ⬇️
... and 67 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f343c58...7ad5a1d. Read the comment docs.

parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite_mail.tmpl.html Outdated Show resolved Hide resolved
parsec/backend/invite_mail.tmpl.html Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite_mail.tmpl.html Outdated Show resolved Hide resolved
@vxgmichel vxgmichel changed the title First email draft to check WIP: Invitation email Jun 11, 2020
@Ironicbay Ironicbay marked this pull request as ready for review June 17, 2020 12:18
@Ironicbay Ironicbay requested a review from c4ffein June 17, 2020 12:19
Copy link
Contributor

@c4ffein c4ffein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
parsec/backend/invite.py Outdated Show resolved Hide resolved
@@ -173,21 +192,29 @@ async def send_invite_email(
)

# Plain-text version used as backup if the html doesn't work for the client
text = f"{line1}{parsec_url}\n{line2}\n{invite_link}\n{line4}"
text = f"{body[0]}{parsec_url}\n{body[1]}\n{invite_link}\n{body[3]}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems too fragile (for instance why do we skip body[2] ? ^^).
If we want to change the place invite_link is we have to change this part, the body list (for each language) and the html formating too... If we consider the body as a list of paragraph, we just have to move the {invite_link} part in the body (well still for each language).

And the formatting becomes trivial:

body = MAIL_TEXT[lang]['body']

text_body = '\n'.join(body).format(sender_name=sender_name, parsec_url=parsec_url, invite_link=invite_link)

html_body = '<br>'.join(body).format(sender_name=sender_name, parsec_url=parsec_url, invite_link=invite_link)
html_template = importlib_resources.read_text(parsec.backend.mail, "invite_mail.tmpl.html")
html = html_template.format(body=html_body, preheader=MAIL_TEXT[lang]['preheader'], ...)

@touilleMan
Copy link
Member

It seems you can add LANGUAGE headers to email messages

RFC1766 §4.1 seems to present an example of multi-language message (https://tools.ietf.org/html/rfc1766):

    Content-Type: multipart/alternative; differences=Content-Language;
              boundary="limit"
    Content-Language: en, fr, de

    --limit
    Content-Language: fr

    Le renard brun et agile saute par dessus le chien paresseux
    --limit
    Content-Language: de
    Content-Type: text/plain; charset=iso-8859-1
    Content-Transfer-encoding: quoted-printable

    Der schnelle braune Fuchs h=FCpft =FCber den faulen Hund
    --limit
    Content-Language: en

    The quick brown fox jumps over the lazy dog
    --limit--

see also https://tools.ietf.org/html/rfc2231.html

Have you tried this kind of stuff in your email ?

@touilleMan
Copy link
Member

touilleMan commented Jul 2, 2020

I've made a couple of tests for multilingual mails

import smtplib, ssl
from email.mime.text import MIMEText
from email.mime.multipart import MIMEMultipart

sender_email = "no-reply@parsec.cloud"
# receiver_email = "yyy@gmail.com"
receiver_email = "xxx@gmail.com"
password = "xxxxxxxxxxxxx"

message = MIMEMultipart("alternative")
message["Subject"] = "Bonjour 2"
message["From"] = sender_email
message["To"] = receiver_email

# Create the plain-text and HTML version of your message
text_fr = """\
Salut,
Comment ça va ?
Real Python has many great tutorials:
www.realpython.com"""

html_fr = """\
<html>
  <body>
    <p>Salut,<br>
       Comment ça va ?<br>
       <a href="http://www.realpython.com">Real Python</a> 
       has many great tutorials.
    </p>
  </body>
</html>
"""

text_en = """\
Hi,
How are you?
Real Python has many great tutorials:
www.realpython.com"""

html_en = """\
<html>
  <body>
    <p>Hi,<br>
       How are you?<br>
       <a href="http://www.realpython.com">Real Python</a> 
       has many great tutorials.
    </p>
  </body>
</html>
"""

# Turn these into plain/html MIMEText objects
part_text_fr = MIMEText(text_fr, "plain")
part_html_fr = MIMEText(html_fr, "html")
part_text_en = MIMEText(text_en, "plain")
part_html_en = MIMEText(html_en, "html")

part_text_fr.add_header("Content-Language", "fr-FR")
part_html_fr.add_header("Content-Language", "fr-FR")
part_text_en.add_header("Content-Language", "en-US")
part_html_en.add_header("Content-Language", "en-US")

# Add HTML/plain-text parts to MIMEMultipart message
# The email client will try to render the last part first
message.attach(part_text_en)
message.attach(part_html_en)
message.attach(part_text_fr)
message.attach(part_html_fr)

# Create secure connection with server and send email
context = ssl.create_default_context()
with smtplib.SMTP("mail.gandi.net", 587) as server:
    tls_ready = server.starttls(context=context)
    if tls_ready[0] != 220:
        print("TLS error:", tls_ready)
    server.login(sender_email, password)
    server.sendmail(
        sender_email, receiver_email, message.as_string()
    )

which create this kind of email body:

[...]
Content-Type: multipart/alternative; boundary="===============2245090318428771878=="
MIME-Version: 1.0
Subject: Bonjour 1
From: no-reply@parsec.cloud
To: xxx@gmail.com

--===============2245090318428771878==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Language: fr

U2FsdXQsCkNvbW1lbnQgw6dhIHZhID8KUmVhbCBQeXRob24gaGFzIG1hbnkgZ3JlYXQgdHV0b3Jp
YWxzOgp3d3cucmVhbHB5dGhvbi5jb20=
--===============2245090318428771878==
Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Language: fr

PGh0bWw+CiAgPGJvZHk+CiAgICA8cD5TYWx1dCw8YnI+CiAgICAgICBDb21tZW50IMOnYSB2YSA/
PGJyPgogICAgICAgPGEgaHJlZj0iaHR0cDovL3d3dy5yZWFscHl0aG9uLmNvbSI+UmVhbCBQeXRo
b248L2E+IAogICAgICAgaGFzIG1hbnkgZ3JlYXQgdHV0b3JpYWxzLgogICAgPC9wPgogIDwvYm9k
eT4KPC9odG1sPgo=
--===============2245090318428771878==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Language: en

Hi,
How are you?
Real Python has many great tutorials:
www.realpython.com
--===============2245090318428771878==
Content-Type: text/html; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Language: en

<html>
  <body>
    <p>Hi,<br>
       How are you?<br>
       <a href="http://www.realpython.com">Real Python</a> 
       has many great tutorials.
    </p>
  </body>
</html>

--===============2245090318428771878==--

However this doesn't seems to be handled correctly with mail clients (I've tried with gmail, the last html part is always used no matter it content-language type and the language of the gmail client) :'-(

@Ironicbay
Copy link
Contributor Author

Mail preview
Screenshot from 2020-07-02 17-22-53

Copy link
Member

@touilleMan touilleMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@touilleMan touilleMan merged commit 77b5f71 into master Jul 7, 2020
@touilleMan touilleMan deleted the send-invite-email branch July 7, 2020 12:25
@touilleMan touilleMan changed the title WIP: Invitation email Invitation email Jul 7, 2020
@Ironicbay Ironicbay linked an issue Jul 15, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send an automated email with parsec link when on-boarding user
3 participants