diff --git a/packages/syft/src/syft/service/notifier/notifier_service.py b/packages/syft/src/syft/service/notifier/notifier_service.py index 57ef154b2f0..d667d289331 100644 --- a/packages/syft/src/syft/service/notifier/notifier_service.py +++ b/packages/syft/src/syft/service/notifier/notifier_service.py @@ -67,24 +67,16 @@ def user_settings( app=notifications[NOTIFIERS.APP], ) - def set_notifier_active_to_true(self, context: AuthedServiceContext) -> SyftSuccess: + def _set_notifier(self, context: AuthedServiceContext, active: bool) -> SyftSuccess: notifier = self.stash.get(credentials=context.credentials).unwrap( public_message="Notifier settings not found." ) - notifier.active = True - self.stash.update(credentials=context.credentials, settings=notifier).unwrap() - return SyftSuccess(message="notifier.active set to true.") - def set_notifier_active_to_false( - self, context: AuthedServiceContext - ) -> SyftSuccess: - """ - Essentially a duplicate of turn_off method. - """ - notifier = self.stash.get(credentials=context.credentials).unwrap() - notifier.active = False + notifier.active = active self.stash.update(credentials=context.credentials, settings=notifier).unwrap() - return SyftSuccess(message="notifier.active set to false.") + + active_s = "active" if active else "inactive" + return SyftSuccess(message=f"Notifier set to {active_s}") @as_result(SyftException) def turn_on( diff --git a/packages/syft/src/syft/service/settings/settings_service.py b/packages/syft/src/syft/service/settings/settings_service.py index 4e552ba26cc..3282d8b72ea 100644 --- a/packages/syft/src/syft/service/settings/settings_service.py +++ b/packages/syft/src/syft/service/settings/settings_service.py @@ -1,5 +1,6 @@ # stdlib from string import Template +from typing import cast # relative from ...abstract_server import ServerSideType @@ -9,6 +10,7 @@ from ...store.document_store_errors import StashException from ...types.errors import SyftException from ...types.result import as_result +from ...types.syft_metaclass import Empty from ...util.assets import load_png_base64 from ...util.experimental_flags import flags from ...util.misc_objs import HTMLObject @@ -20,6 +22,7 @@ from ..context import AuthedServiceContext from ..context import UnauthedServiceContext from ..notifier.notifier_enums import EMAIL_TYPES +from ..notifier.notifier_service import NotifierService from ..response import SyftSuccess from ..service import AbstractService from ..service import service_method @@ -31,6 +34,14 @@ from .settings import ServerSettingsUpdate from .settings_stash import SettingsStash +# for testing purpose +_NOTIFICATIONS_ENABLED_WIHOUT_CREDENTIALS_ERROR = ( + "Failed to enable notification. " + "Email credentials are invalid or have not been set. " + "Please use `enable_notifications` from `user_service` " + "to set the correct email credentials." +) + @serializable(canonical_name="SettingsService", version=1) class SettingsService(AbstractService): @@ -118,23 +129,44 @@ def _update( context.credentials, settings=new_settings ).unwrap() - notifier_service = context.server.get_service("notifierservice") + notifier_service = cast( + NotifierService, context.server.get_service("notifierservice") + ) # If notifications_enabled is present in the update, we need to update the notifier settings - if settings.notifications_enabled is True: - if not notifier_service.settings(context): + if settings.notifications_enabled is not Empty: # type: ignore[comparison-overlap] + notifier_settings_res = notifier_service.settings(context) + if ( + not notifier_settings_res.is_ok() + or (notifier_settings := notifier_settings_res.ok()) is None + ): raise SyftException( - public_smessage="Create notification settings using enable_notifications from user_service" + public_message=( + "Notification has not been enabled. " + "Please use `enable_notifications` from `user_service`." + ) + ) + + if settings.notifications_enabled and ( + not ( + notifier_settings.email_username + and notifier_settings.email_password ) - notifier_service = context.server.get_service("notifierservice") - notifier_service.set_notifier_active_to_true(context) - elif settings.notifications_enabled is False: - if not notifier_service.settings(context): + or not notifier_settings.validate_email_credentials( + notifier_settings.email_username, + notifier_settings.email_password, + notifier_settings.email_server, + notifier_settings.email_port, + ) + ): raise SyftException( - public_message="Create notification settings using enable_notifications from user_service" + public_message=_NOTIFICATIONS_ENABLED_WIHOUT_CREDENTIALS_ERROR ) - notifier_service = context.server.get_service("notifierservice") - notifier_service.set_notifier_active_to_false(context) + + notifier_service.set_notifier( + context, active=settings.notifications_enabled + ) + return update_result else: raise NotFoundException(public_message="Server settings not found") diff --git a/packages/syft/tests/syft/settings/settings_service_test.py b/packages/syft/tests/syft/settings/settings_service_test.py index d7e5d81d853..0a26e51f260 100644 --- a/packages/syft/tests/syft/settings/settings_service_test.py +++ b/packages/syft/tests/syft/settings/settings_service_test.py @@ -23,6 +23,9 @@ from syft.service.service import _SIGNATURE_ERROR_MESSAGE from syft.service.settings.settings import ServerSettings from syft.service.settings.settings import ServerSettingsUpdate +from syft.service.settings.settings_service import ( + _NOTIFICATIONS_ENABLED_WIHOUT_CREDENTIALS_ERROR, +) from syft.service.settings.settings_service import SettingsService from syft.service.settings.settings_stash import SettingsStash from syft.service.user.user import UserPrivateKey @@ -462,3 +465,12 @@ def test_invalid_args_error_message(root_datasite_client: DatasiteClient) -> Non settings = root_datasite_client.api.services.settings.get() assert settings.name == update_args["name"] assert settings.organization == update_args["organization"] + + +def test_notifications_enabled_without_emails_credentials_not_allowed( + root_datasite_client: DatasiteClient, +) -> None: + with pytest.raises(SyftException) as exc: + root_datasite_client.api.services.settings.update(notifications_enabled=True) + + assert _NOTIFICATIONS_ENABLED_WIHOUT_CREDENTIALS_ERROR in exc.value.public_message