Merge pull request #9242 from madhavajay/madhava/fix_emails

Tweaked email templates

packages/syft/src/syft/server/routes.py

@@ -198,23 +198,28 @@ def handle_forgot_password(email: str, server: AbstractServer) -> Response:
             context = UnauthedServiceContext(server=server)
             result = method(context=context, email=email)
         except SyftException as e:
-            result = SyftError(message=e.public_message)
+            result = SyftError.from_public_exception(e)
 
         if isinstance(result, SyftError):
             logger.debug(f"Forgot Password Error: {result.message}. user={email}")
 
-        response = result
         return Response(
-            serialize(response, to_bytes=True),
+            serialize(result, to_bytes=True),
             media_type="application/octet-stream",
         )
 
     def handle_reset_password(
         token: str, new_password: str, server: AbstractServer
     ) -> Response:
-        method = server.get_service_method(UserService.reset_password)
-        context = UnauthedServiceContext(server=server)
-        result = method(context=context, token=token, new_password=new_password)
+        try:
+            method = server.get_service_method(UserService.reset_password)
+            context = UnauthedServiceContext(server=server)
+            result = method(context=context, token=token, new_password=new_password)
+        except SyftException as e:
+            result = SyftError.from_public_exception(e)
+
+        if isinstance(result, SyftError):
+            logger.debug(f"Reset Password Error: {result.message}. token={token}")
 
         return Response(
             serialize(result, to_bytes=True),

packages/syft/src/syft/service/notification/email_templates.py

@@ -342,6 +342,20 @@ def email_body(notification: "Notification", context: AuthedServiceContext) -> s
                         <p><strong>Changes:</strong>
                             {request_changes}
                         </p>
+
+                        <p>Use:<br />
+                        <code style="color: #FF8C00;background-color: #f0f0f0;font-size: 12px;">
+                            request = client.api.services.request.get_by_uid(uid=sy.UID("{request_id}"))
+                        </code><br />
+                            to get this specific request.
+                        </p>
+
+                        <p>Or you can view all requests with: <br />
+                        <code style="color: #FF8C00;background-color: #f0f0f0;font-size: 12px;">
+                            client.requests
+                        </code>
+                        </p>
+
                         </div>
                     </div>
                     <p>If you did not expect this request or have concerns about it,
@@ -501,6 +515,19 @@ def email_body(notification: "Notification", context: AuthedServiceContext) -> s
                             <strong>Changes:</strong>
                             {request_changes}
                             </p>
+
+                            <p>Use:<br />
+                            <code style="color: #FF8C00;background-color: #f0f0f0;font-size: 12px;">
+                                request = client.api.services.request.get_by_uid(uid=sy.UID("{request_id}"))
+                            </code><br />
+                                to get this specific request.
+                            </p>
+
+                            <p>Or you can view all requests with: <br />
+                            <code style="color: #FF8C00;background-color: #f0f0f0;font-size: 12px;">
+                                client.requests
+                            </code>
+                            </p>
                         </div>
                     </div>
                     <p>If you did not expect this request or have concerns about it,

packages/syft/src/syft/service/notification/notification_service.py

@@ -106,13 +106,17 @@ def settings(
         path="notifications.activate",
         name="activate",
         roles=DATA_SCIENTIST_ROLE_LEVEL,
+        unwrap_on_success=False,
     )
     def activate(
         self,
         context: AuthedServiceContext,
     ) -> Notification:
         notifier_service = context.server.get_service("notifierservice")
         result = notifier_service.activate(context)
+        if isinstance(result, OkErr) and result.is_ok():
+            # sad, TODO: remove upstream Ok
+            result = result.ok()
         return result
 
     @service_method(

packages/syft/src/syft/service/notifier/notifier.py

@@ -100,7 +100,9 @@ def send(
     ) -> SyftSuccess | SyftError:
         subject = None
         receiver_email = None
+        sender = None
         try:
+            sender = self.sender
             user_service = context.server.get_service("userservice")
             receiver = user_service.get_by_verify_key(
                 notification.to_user_verify_key
@@ -126,12 +128,14 @@ def send(
                 receiver_email = [receiver_email]
 
             self.smtp_client.send(  # type: ignore
-                sender=self.sender, receiver=receiver_email, subject=subject, body=body
+                sender=sender, receiver=receiver_email, subject=subject, body=body
             )
-            print(f"> Sent email: {subject} to {receiver_email}")
+            print(f"> Sent email: {subject} to {receiver_email} from: {sender}")
             return SyftSuccess(message="Email sent successfully!")
-        except Exception:
-            print(f"> Error sending email: {subject} to {receiver_email}")
+        except Exception as e:
+            message = f"> Error sending email: {subject} to {receiver_email} from: {sender}. {e}"
+            print(message)
+            logger.error(message)
             return SyftError(message="Failed to send an email.")
             # raise SyftException.from_exception(
             #     exc,

packages/syft/src/syft/service/notifier/notifier_service.py

@@ -12,6 +12,7 @@
 from ...store.document_store_errors import NotFoundException
 from ...store.document_store_errors import StashException
 from ...types.errors import SyftException
+from ...types.result import OkErr
 from ...types.result import as_result
 from ..context import AuthedServiceContext
 from ..notification.email_templates import PasswordResetTemplate
@@ -216,7 +217,11 @@ def activate(
         This will only work if the datasite owner has enabled notifications.
         """
         user_service = context.server.get_service("userservice")
-        return user_service.enable_notifications(context, notifier_type=notifier_type)
+        result = user_service.enable_notifications(context, notifier_type=notifier_type)
+        if isinstance(result, OkErr) and result.is_ok():
+            # sad, TODO: remove upstream Ok
+            result = result.ok()
+        return result
 
     @as_result(SyftException)
     def deactivate(

packages/syft/src/syft/service/notifier/smtp_client.py

@@ -1,6 +1,7 @@
 # stdlib
 from email.mime.multipart import MIMEMultipart
 from email.mime.text import MIMEText
+import logging
 import smtplib
 
 # third party
@@ -12,6 +13,8 @@
 
 SOCKET_TIMEOUT = 5  # seconds
 
+logger = logging.getLogger(__name__)
+
 
 class SMTPClient(BaseModel):
     server: str
@@ -44,7 +47,8 @@ def send(self, sender: str, receiver: list[str], subject: str, body: str) -> Non
                 text = msg.as_string()
                 server.sendmail(sender, ", ".join(receiver), text)
                 return None
-        except Exception:
+        except Exception as e:
+            logger.error(f"Unable to send email. {e}")
             raise SyftException(
                 public_message="Ops! Something went wrong while trying to send an email."
             )

packages/syft/src/syft/service/user/user_service.py

@@ -258,7 +258,7 @@ def reset_password(
         #
         if user is None:
             raise SyftException(
-                public_message="Failed to reset user password. Token is invalid or expired!"
+                public_message="Failed to reset user password. Token is invalid or expired."
             )
         now = datetime.now()
         if user.reset_token_date is not None:
@@ -277,9 +277,10 @@ def reset_password(
 
         if not validate_password(new_password):
             raise SyftException(
-                public_message="Your new password must have at least 8 \
-                characters, Upper case and lower case characters\
-                and at least one number."
+                public_message=(
+                    "Your new password must have at least 8 characters, an upper case "
+                    "and lower case character; and at least one number."
+                )
             )
 
         salt, hashed = salt_and_hash_password(new_password, 12)
@@ -293,7 +294,7 @@ def reset_password(
             credentials=root_context.credentials, obj=user, has_permission=True
         ).unwrap()
 
-        return SyftSuccess(message="User Password updated successfully!")
+        return SyftSuccess(message="User Password updated successfully.")
 
     def generate_new_password_reset_token(
         self, token_config: PwdTokenResetConfig