Add notification when mail send fail #1842

alexandrediasldev · 2021-09-09T12:05:59Z

No description provided.

codecov · 2021-09-09T12:43:20Z

Codecov Report

Merging #1842 (11101a1) into master (2ba4ad0) will increase coverage by 0.18%.
The diff coverage is 76.25%.

❗ Current head 11101a1 differs from pull request most recent head a163668. Consider uploading reports for the commit a163668 to get more accurate results

@@            Coverage Diff             @@
##           master    #1842      +/-   ##
==========================================
+ Coverage   87.38%   87.57%   +0.18%     
==========================================
  Files         244      244              
  Lines       23636    24233     +597     
==========================================
+ Hits        20654    21221     +567     
- Misses       2982     3012      +30

Impacted Files	Coverage Δ
parsec/api/protocol/__init__.py	`100.00% <ø> (ø)`
parsec/core/gui/custom_dialogs.py	`68.25% <33.33%> (-1.30%)`	⬇️
parsec/core/cli/invitation.py	`89.53% <66.66%> (+2.52%)`	⬆️
parsec/core/gui/greet_device_widget.py	`82.40% <66.66%> (-0.66%)`	⬇️
parsec/core/gui/users_widget.py	`83.65% <70.00%> (-0.66%)`	⬇️
parsec/backend/invite.py	`92.16% <88.00%> (-0.35%)`	⬇️
parsec/core/logged_core.py	`86.03% <88.88%> (-0.10%)`	⬇️
parsec/api/protocol/invite.py	`100.00% <100.00%> (ø)`
parsec/core/gui/devices_widget.py	`97.22% <100.00%> (+0.05%)`	⬆️
parsec/backend/cli/run.py	`69.95% <0.00%> (-2.86%)`	⬇️
... and 30 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 2ba4ad0...a163668. Read the comment docs.

touilleMan · 2021-09-10T08:28:34Z

parsec/core/gui/tr/parsec_en.po

@@ -1851,6 +1858,10 @@ msgstr "This user is already a member of this organization."
 msgid "TEXT_INVITE_USER_INVITE_ERROR"
 msgstr "Could not invite the user."

+msgid "TEXT_INVITE_USER_EMAIL_NOT_SENT_directlink"
+msgstr "Could not send the invite by mail to the user, here is the link the "


"the invitation"

"the user needs"

touilleMan · 2021-09-10T08:39:05Z

parsec/core/logged_core.py

@@ -281,11 +283,13 @@ async def new_device_invitation(self, send_email: bool) -> BackendInvitationAddr
        )
        if rep["status"] != "ok":
            raise BackendConnectionError(f"Backend error: {rep}")
+        email_sent = not ("email_sent" in rep)


Should return a tuple containing the addr AND the email send result

touilleMan · 2021-09-10T08:39:29Z

parsec/core/types/backend_address.py

@@ -438,19 +438,21 @@ class BackendInvitationAddr(BackendActionAddr):
    (e.g. ``parsec://parsec.example.com/my_org?action=claim_user&token=3a50b191122b480ebb113b10216ef343``)
    """

-    __slots__ = ("_organization_id", "_invitation_type", "_token")
+    __slots__ = ("_organization_id", "_invitation_type", "_token", "_email_sent")


no need to modify addr given it is not related to the success of email sending operation

touilleMan · 2021-09-10T08:41:20Z

parsec/core/invite/greeter.py

-            raise InviteAlreadyUsedError()
-        elif rep["status"] != "ok":
-            raise InviteError(f"Backend error during step 1: {rep}")
+        _check_rep(rep, step_name="step 1")


keep this refacto for another PR, as the current one should not have to deal with greeter/claimer (but only with invitation creation)

parsec/backend/invite.py

touilleMan · 2021-09-10T08:49:58Z

parsec/backend/invite.py

@@ -78,6 +78,10 @@ class InvitationAlreadyMemberError(InvitationError):
    pass


+class InvitationEmailError(InvitationError):


should define InvitationEmailConfigError & InvitationEmailRecipientError

touilleMan · 2021-09-10T08:55:40Z

parsec/api/protocol/invite.py

@@ -60,6 +60,7 @@ def get_obj_type(self, obj: Dict[str, object]) -> InvitationType:

 class InviteNewRepSchema(BaseRepSchema):
    token = fields.UUID(required=True)
+    email_sent = fields.Boolean(required=False)


should be an enum with different possible outcomes:

success

not_available

bad_recipient

with required=False & allow_none=False and a nice comment about the introduction of this new field ;-)

Ironicbay · 2021-09-10T15:27:03Z

parsec/api/protocol/invite.py

@@ -58,8 +58,19 @@ def get_obj_type(self, obj: Dict[str, object]) -> InvitationType:
        return cast(InvitationType, obj["type"])


+class InvitationEmailSentStatus(Enum):
+    SUCESS = "SUCESS"


Typo here : SUCCESS

Ironicbay · 2021-09-10T15:38:45Z

parsec/core/gui/greet_device_widget.py

+            self.button_send_email.setText(_("TEXT_GREET_DEVICE_EMAIL_SENT"))
+            self.button_send_email.setDisabled(True)
+        else:
+            show_info(


Would be nice to tell in show_info the error type (recipient or sender's fault)

Ironicbay · 2021-09-10T15:40:41Z

parsec/core/gui/users_widget.py

+        if email_sent_status == InvitationEmailSentStatus.SUCESS:
+            show_info(self, _("TEXT_USER_INVITE_SUCCESS_email").format(email=email))
+        else:
+            show_info(


Same comment as previous

alexandrediasldev · 2021-09-13T11:34:05Z

The window opened when an email can't be sent

Max-7 · 2021-09-13T11:46:42Z

parsec/core/gui/greet_device_widget.py

+            if email_sent_status == InvitationEmailSentStatus.BAD_RECIPIENT:
+                show_info_copy_link(
+                    self,
+                    "Email bad recipient",


No hard-coded strings, it should be translated (same thing for all the other ones).

In case you didn't know, you can use

python setup.py extract_translations

to add the all the translations to the .po file (instead of doing it manually), makes it easier to find the missing ones.

…corresponding test

…vitationEmailRecipientError

alexandrediasldev linked an issue Sep 9, 2021 that may be closed by this pull request

Return send mail status in invitation creation api #1751

Closed

alexandrediasldev marked this pull request as ready for review September 9, 2021 14:35

alexandrediasldev changed the title ~~WIP: Add notification when mail send fail~~ Add notification when mail send fail Sep 9, 2021

touilleMan requested changes Sep 10, 2021

View reviewed changes

Ironicbay requested changes Sep 10, 2021

View reviewed changes

Max-7 requested changes Sep 13, 2021

View reviewed changes

Max-7 approved these changes Sep 22, 2021

View reviewed changes

alexandrediasldev force-pushed the add-notification_when_mail_send_fail branch 3 times, most recently from b5456bb to a5b3107 Compare September 22, 2021 13:17

alexandrediasldev added 11 commits September 22, 2021 15:26

Invite added exception when mail is failing to be sent and added the …

78fb0fa

…corresponding test

Added exception for the client when an email is not sent

6c492eb

Added test for device invitation

176848a

Error window opening on the gui when email could not be sent

5f56e95

Popup added when failing to invite a device

b404e24

Added custom text for device failing to send an email

b50f087

changed the BackendInvitationAddr to remove the mail sent status

54771a2

Modified email sent from boolean to enum

6983fbd

Separated InvitationEmailError into InvitationEmailConfigError and In…

47e8ea0

…vitationEmailRecipientError

Added cli support for failing to send mail

401c968

Added copy link to clipboard to the error message

a163668

alexandrediasldev force-pushed the add-notification_when_mail_send_fail branch from a5b3107 to a163668 Compare September 22, 2021 13:28

alexandrediasldev merged commit a163668 into master Sep 22, 2021

alexandrediasldev deleted the add-notification_when_mail_send_fail branch September 22, 2021 14:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add notification when mail send fail #1842

Add notification when mail send fail #1842

alexandrediasldev commented Sep 9, 2021

codecov bot commented Sep 9, 2021 •

edited

touilleMan Sep 10, 2021

touilleMan Sep 10, 2021

touilleMan Sep 10, 2021

touilleMan Sep 10, 2021

touilleMan Sep 10, 2021

touilleMan Sep 10, 2021

Ironicbay Sep 10, 2021

Ironicbay Sep 10, 2021

Ironicbay Sep 10, 2021

alexandrediasldev commented Sep 13, 2021

Max-7 Sep 13, 2021

		@@ -78,6 +78,10 @@ class InvitationAlreadyMemberError(InvitationError):
		pass


		class InvitationEmailError(InvitationError):

Add notification when mail send fail #1842

Add notification when mail send fail #1842

Conversation

alexandrediasldev commented Sep 9, 2021

codecov bot commented Sep 9, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexandrediasldev commented Sep 13, 2021

Choose a reason for hiding this comment

codecov bot commented Sep 9, 2021 •

edited