fix template and notification capture issues

Author: Johnetordoff

addons/base/tests/views.py

@@ -183,7 +183,13 @@ def test_import_auth_cant_write_node(self):
         user.save()
 
         node = ProjectFactory(creator=self.user)
-        node.add_contributor(user, permissions=permissions.READ, auth=self.auth, save=True)
+        node.add_contributor(
+            user,
+            permissions=permissions.READ,
+            auth=self.auth,
+            save=True,
+            notification_type=False
+        )
         node.add_addon(self.ADDON_SHORT_NAME, auth=self.auth)
         node.save()
         url = node.api_url_for(f'{self.ADDON_SHORT_NAME}_import_auth')
@@ -219,7 +225,13 @@ def test_get_config(self):
     def test_get_config_unauthorized(self):
         url = self.project.api_url_for(f'{self.ADDON_SHORT_NAME}_get_config')
         user = AuthUserFactory()
-        self.project.add_contributor(user, permissions=permissions.READ, auth=self.auth, save=True)
+        self.project.add_contributor(
+            user,
+            permissions=permissions.READ,
+            auth=self.auth,
+            save=True,
+            notification_type=False
+        )
         res = self.app.get(url, auth=user.auth, )
         assert res.status_code == http_status.HTTP_403_FORBIDDEN
 

addons/bitbucket/tests/test_views.py

@@ -79,7 +79,6 @@ def test_set_config(self, mock_account, mock_repo):
 class TestBitbucketViews(OsfTestCase):
 
     def setUp(self):
-        super().setUp()
         self.user = AuthUserFactory()
         self.consolidated_auth = Auth(user=self.user)
 

addons/osfstorage/tests/test_models.py

@@ -24,6 +24,7 @@
 from osf import models
 from addons.osfstorage import utils
 from addons.osfstorage import settings
+from tests.utils import capture_notifications
 from website.files.exceptions import FileNodeCheckedOutError, FileNodeIsPrimaryFile
 
 SessionStore = import_module(django_conf_settings.SESSION_ENGINE).SessionStore
@@ -866,7 +867,8 @@ def setUp(self):
 
     def test_checkout_logs(self):
         non_admin = factories.AuthUserFactory()
-        self.node.add_contributor(non_admin, permissions=WRITE)
+        with capture_notifications():
+            self.node.add_contributor(non_admin, permissions=WRITE)
         self.node.save()
         self.file.check_in_or_out(non_admin, non_admin, save=True)
         self.file.reload()

osf/models/notification.py

@@ -39,7 +39,7 @@ def send(
                 f"\ncontext={self.event_context}"
                 f"\nemail={email_context}"
             )
-        elif protocol_type == 'email' and settings.DEV_MODE:
+        if protocol_type == 'email' and settings.DEV_MODE:
             email.send_email_over_smtp(
                 recipient_address,
                 self.subscription.notification_type,

osf/models/preprint.py

@@ -1026,35 +1026,29 @@ def _add_creator_as_contributor(self):
     def _send_preprint_confirmation(self, auth):
         # Send creator confirmation email
         recipient = self.creator
-        if self.provider._id == 'osf':
-            logo = settings.OSF_PREPRINTS_LOGO
-        else:
-            logo = self.provider._id
-
-        context = {
-            'domain': settings.DOMAIN,
-            'reviewable_title': self.title,
-            'reviewable_absolute_url': self.absolute_url,
-            'reviewable_provider_name': self.provider.name,
-            'workflow': self.provider.reviews_workflow,
-            'provider_url': '{domain}preprints/{provider_id}'.format(
-                            domain=self.provider.domain or settings.DOMAIN,
-                            provider_id=self.provider._id if not self.provider.domain else '').strip('/'),
-            'provider_contact_email': self.provider.email_contact or settings.OSF_CONTACT_EMAIL,
-            'provider_support_email': self.provider.email_support or settings.OSF_SUPPORT_EMAIL,
-            'no_future_emails': False,
-            'is_creator': True,
-            'provider_name': 'OSF Preprints' if self.provider.name == 'Open Science Framework' else self.provider.name,
-            'logo': logo,
-            'document_type': self.provider.preprint_word
-        }
-
         NotificationType.objects.get(
             name=NotificationType.Type.PROVIDER_REVIEWS_SUBMISSION_CONFIRMATION
         ).emit(
             subscribed_object=self.provider,
             user=recipient,
-            event_context=context,
+            event_context={
+                'domain': settings.DOMAIN,
+                'user_fullname': recipient.fullname,
+                'referrer_fullname': recipient.fullname,
+                'reviewable_title': self.title,
+                'reviewable_absolute_url': self.absolute_url,
+                'reviewable_provider_name': self.provider.name,
+                'workflow': self.provider.reviews_workflow,
+                'provider_url': f'{self.provider.domain or settings.DOMAIN}preprints/'
+                                f'{(self.provider._id if not self.provider.domain else '').strip('/')}',
+                'provider_contact_email': self.provider.email_contact or settings.OSF_CONTACT_EMAIL,
+                'provider_support_email': self.provider.email_support or settings.OSF_SUPPORT_EMAIL,
+                'no_future_emails': False,
+                'is_creator': True,
+                'provider_name': 'OSF Preprints' if self.provider.name == 'Open Science Framework' else self.provider.name,
+                'logo': settings.OSF_PREPRINTS_LOGO if self.provider._id == 'osf' else self.provider._id,
+                'document_type': self.provider.preprint_word
+            },
         )
 
     # FOLLOWING BEHAVIOR NOT SPECIFIC TO PREPRINTS

osf_tests/factories.py

@@ -796,7 +796,10 @@ def _create(cls, target_class, *args, **kwargs):
             instance.set_subjects(subjects, auth=auth)
             if license_details:
                 instance.set_preprint_license(license_details, auth=auth)
-            instance.set_published(is_published, auth=auth)
+            from tests.utils import capture_notifications
+
+            with capture_notifications():
+                instance.set_published(is_published, auth=auth)
             create_task_patcher = mock.patch('website.identifiers.utils.request_identifiers')
             mock_create_identifier = create_task_patcher.start()
             if is_published and set_doi:

tests/base.py

@@ -161,7 +161,11 @@ class OsfTestCase(DbTestCase, AppTestCase, SearchTestCase):
     application. Note: superclasses must call `super` in order for all setup and
     teardown methods to be called correctly.
     """
-    pass
+    def setUp(self):
+        from tests.utils import capture_notifications
+
+        with capture_notifications():
+            return super().setUp()
 
 
 class ApiTestCase(DbTestCase, ApiAppTestCase, SearchTestCase):

tests/test_adding_contributor_views.py

@@ -27,7 +27,6 @@
 )
 from tests.utils import capture_notifications
 from website.profile.utils import add_contributor_json, serialize_unregistered
-from website import settings
 from website.project.views.contributor import (
     deserialize_contributors,
     notify_added_contributor,

tests/utils.py

@@ -255,23 +255,38 @@ def run_celery_tasks():
 
 
 @contextlib.contextmanager
-def capture_notifications():
+def capture_notifications(capture_email: bool = True):
     """
-    Context manager to capture NotificationType emits without interfering with ORM calls.
-    Yields a list of captured emits:
-        [{'type': <NotificationType.Type>, 'args': ..., 'kwargs': ...}, ...]
+    Context manager to capture NotificationType emits without interfering with ORM calls
+    and (optionally) stub out actual email sending so tests don't open sockets.
+
+    Yields a dict with two lists:
+      {
+        "emits": [
+          {"type": <str name>, "args": tuple, "kwargs": dict}, ...
+        ],
+        "emails": [
+          {
+            "protocol": "smtp" | "sendgrid",
+            "to": <str or user object>,
+            "notification_type": <NotificationType or str>,
+            "context": <dict>,
+            "email_context": <dict>
+          }, ...
+        ]
+      }
     """
     NotificationType = apps.get_model('osf', 'NotificationType')
     real_get = NotificationType.objects.get  # Save the real .get()
 
-    captured = []
+    captured = {'emits': [], 'emails': []}
 
     def side_effect(*args, **kwargs):
         notifier = real_get(*args, **kwargs)  # Call the real .get()
         original_emit = notifier.emit
 
         def wrapped_emit(*emit_args, **emit_kwargs):
-            captured.append({
+            captured['emits'].append({
                 'type': notifier.name,
                 'args': emit_args,
                 'kwargs': emit_kwargs
@@ -281,7 +296,34 @@ def wrapped_emit(*emit_args, **emit_kwargs):
         notifier.emit = wrapped_emit
         return notifier
 
-    with mock.patch('osf.models.notification_type.NotificationType.objects.get', side_effect=side_effect):
-        yield captured
+    patches = [
+        mock.patch('osf.models.notification_type.NotificationType.objects.get', side_effect=side_effect),
+    ]
+    if capture_email:
+        def _fake_send_over_smtp(to_email, notification_type, context, email_context):
+            captured['emails'].append({
+                'protocol': 'smtp',
+                'to': to_email,
+                'notification_type': notification_type,
+                'context': context,
+                'email_context': email_context,
+            })
 
+        def _fake_send_with_sendgrid(user, notification_type, context, email_context):
+            captured['emails'].append({
+                'protocol': 'sendgrid',
+                'to': user,  # keeping the object for tests that assert user props
+                'notification_type': notification_type,
+                'context': context,
+                'email_context': email_context,
+            })
 
+        patches.extend([
+            mock.patch('osf.email.send_email_over_smtp', new=_fake_send_over_smtp),
+            mock.patch('osf.email.send_email_with_send_grid', new=_fake_send_with_sendgrid),
+        ])
+
+    with contextlib.ExitStack() as stack:
+        for p in patches:
+            stack.enter_context(p)
+        yield captured

website/project/views/contributor.py

@@ -625,17 +625,19 @@ def notify_added_contributor(node, contributor, notification_type, auth=None, *a
     throttle = kwargs.get('throttle', settings.CONTRIBUTOR_ADDED_EMAIL_THROTTLE)
     if notification_type and check_email_throttle(contributor, notification_type, throttle=throttle):
         return
-
+    referrer_name = getattr(getattr(auth, 'user', None), 'fullname', '') if auth else ''
     NotificationType.objects.get(
         name=notification_type
     ).emit(
         user=contributor,
         event_context={
-            'user': contributor.id,
-            'referrer_name': getattr(getattr(auth, 'user', None), 'fullname', '') if auth else '',
+            'user_fullname': contributor.fullname,
+            'referred_text': referrer_name + ' has added you' if referrer_name else 'You have been add',
+            'referrer_name': referrer_name,
             'is_initiator': getattr(getattr(auth, 'user', None), 'id', None) == contributor.id if auth else False,
-            'all_global_subscriptions_none': False,
             'branded_service': getattr(getattr(node, 'provider', None), 'id', None),
+            'node_title': node.title,
+            'node_absolute_url': node.absolute_url,
             'can_change_preferences': False,
             'logo': logo,
             'osf_contact_email': settings.OSF_CONTACT_EMAIL,