Skip to content

Commit a179750

Browse files
authored
[ENG-8064] Add New Notifications Data Model (Refactor Notifications Phase 2) (#11151)
## Purpose This system is designed to formalize and consolidate OSF Notifications under one system in order to better facilitate collaboration between Product, Engineers and QA and end persistent problems with notification email related tech debt. This project is the result of an extensive audit of all OSF emails and combined email digests and seeks to move the `NotificationSubscription` model out of it's transitional state having been migrated from a document based model, into it's final data model which is more regular for a relation our current relational database (django's postgress db). In order to do this I have taken @mfraezz design documents and implemented them with minor changes. This means the responsibility for sending notifications in osf.io is shared by three new models, which will have the old data migrated into them via a migration script and management command. The models are: - `NotificationType` - Similar to RegistrationSchemas or Waffle flags these are in db instrances which are poulated from static config documents in this case yaml. - Since these are synced on migration with `notification.yaml` a developer will be able to see at a glance if where a notification template is and what it's purpose is. - `NotificationSubscription` - This model is somewhat analogous to the earlier `EmailDigest` model, this holds references to potentially many Notifcations models that can be compiled into a single digest this is sent at a periodic basis. - `Notification` - Holds message information and context - Unlike earlier implementations this will record each Notification creation and sending for metrics purposes. ## Changes - Combines worker with beat in docker-compose - creates `notifications.yaml` to contain all notification types info it is populated via postmigrate hook - - creates new `Notification` `NotificationSubscription` and `NotificationType` - adds migrations to rename legacy tables and a managment command to populate the new ones. - Deletes old `send_mails` method and replaces it with `emit` of `NotificationType` - adds tests and updates old mocking - updates views and permission classed - A slight change to `handle_boa_error` to pass the already decanted user object. - adds new data model for Notification, NotificationTypes and Subscriptions - creates a `notifications.yaml` for data dependency notificationtypes - add migrations - updates notifications to use NotificationTypes - updates Admin app to control email templates - updates metrics reporter to count notifications sent etc. - updates tests to all use `capture_notifications` mocking util - Removes `queued_mail` - Removes `EmailDigest` - Removes `detect_duplicates` for duplicate subscriptions - Removes unused management commands that sent notifications. ## Ticket https://openscience.atlassian.net/browse/ENG-8064
1 parent 48e37bc commit a179750

File tree

391 files changed

+13867
-14711
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

391 files changed

+13867
-14711
lines changed

addons/base/views.py

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
from framework.flask import redirect
3535
from framework.sentry import log_exception
3636
from framework.transactions.handlers import no_auto_transaction
37-
from website import mails
3837
from website import settings
3938
from addons.base import signals as file_signals
4039
from addons.base.utils import format_last_known_metadata, get_mfr_url
@@ -52,7 +51,7 @@
5251
DraftRegistration,
5352
Guid,
5453
FileVersionUserMetadata,
55-
FileVersion
54+
FileVersion, NotificationType
5655
)
5756
from osf.metrics import PreprintView, PreprintDownload
5857
from osf.utils import permissions
@@ -64,7 +63,7 @@
6463
from website.util import rubeus
6564

6665
# import so that associated listener is instantiated and gets emails
67-
from website.notifications.events.files import FileEvent # noqa
66+
from notifications.file_event_notifications import FileEvent # noqa
6867

6968
ERROR_MESSAGES = {'FILE_GONE': """
7069
<style>
@@ -226,8 +225,6 @@ def get_auth(auth, **kwargs):
226225
_check_resource_permissions(resource, auth, action)
227226

228227
provider_name = waterbutler_data['provider']
229-
waterbutler_settings = None
230-
waterbutler_credentials = None
231228
file_version = file_node = None
232229
if provider_name == 'osfstorage' or (not flag_is_active(request, features.ENABLE_GV)):
233230
file_version, file_node = _get_osfstorage_file_version_and_node(
@@ -576,20 +573,31 @@ def create_waterbutler_log(payload, **kwargs):
576573
params=payload
577574
)
578575

579-
if payload.get('email') is True or payload.get('errors'):
580-
mails.send_mail(
581-
user.username,
582-
mails.FILE_OPERATION_FAILED if payload.get('errors')
583-
else mails.FILE_OPERATION_SUCCESS,
584-
action=payload['action'],
585-
source_node=source_node,
586-
destination_node=destination_node,
587-
source_path=payload['source']['materialized'],
588-
source_addon=payload['source']['addon'],
589-
destination_addon=payload['destination']['addon'],
590-
osf_support_email=settings.OSF_SUPPORT_EMAIL
576+
if payload.get('email') or payload.get('errors'):
577+
if payload.get('email'):
578+
notification_type = NotificationType.Type.USER_FILE_OPERATION_SUCCESS.instance
579+
if payload.get('errors'):
580+
notification_type = NotificationType.Type.USER_FILE_OPERATION_FAILED.instance
581+
notification_type.emit(
582+
user=user,
583+
subscribed_object=node,
584+
event_context={
585+
'user_fullname': user.fullname,
586+
'action': payload['action'],
587+
'source_node': source_node._id,
588+
'source_node_title': source_node.title,
589+
'destination_node': destination_node._id,
590+
'destination_node_title': destination_node.title,
591+
'destination_node_parent_node_title': destination_node.parent_node.title if destination_node.parent_node else None,
592+
'source_path': payload['source']['materialized'],
593+
'source_addon': payload['source']['addon'],
594+
'destination_addon': payload['destination']['addon'],
595+
'osf_support_email': settings.OSF_SUPPORT_EMAIL,
596+
'logo': settings.OSF_LOGO,
597+
'OSF_LOGO_LIST': settings.OSF_LOGO_LIST,
598+
'OSF_LOGO': settings.OSF_LOGO,
599+
}
591600
)
592-
593601
if payload.get('errors'):
594602
# Action failed but our function succeeded
595603
# Bail out to avoid file_signals
@@ -603,10 +611,8 @@ def create_waterbutler_log(payload, **kwargs):
603611
target_node = AbstractNode.load(metadata.get('nid'))
604612
if target_node and payload['action'] != 'download_file':
605613
update_storage_usage_with_size(payload)
606-
607614
with transaction.atomic():
608615
file_signals.file_updated.send(target=node, user=user, event_type=action, payload=payload)
609-
610616
return {'status': 'success'}
611617

612618

addons/boa/tasks.py

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,9 @@
1414
from addons.boa.boa_error_code import BoaErrorCode
1515
from framework import sentry
1616
from framework.celery_tasks import app as celery_app
17-
from osf.models import OSFUser
17+
from osf.models import OSFUser, NotificationType
1818
from osf.utils.fields import ensure_str, ensure_bytes
1919
from website import settings as osf_settings
20-
from website.mails import send_mail, ADDONS_BOA_JOB_COMPLETE, ADDONS_BOA_JOB_FAILURE
2120

2221
logger = logging.getLogger(__name__)
2322

@@ -184,18 +183,19 @@ async def submit_to_boa_async(host, username, password, user_guid, project_guid,
184183

185184
logger.info('Successfully uploaded query output to OSF.')
186185
logger.debug('Task ends <<<<<<<<')
187-
await sync_to_async(send_mail)(
188-
to_addr=user.username,
189-
mail=ADDONS_BOA_JOB_COMPLETE,
190-
fullname=user.fullname,
191-
query_file_name=query_file_name,
192-
query_file_full_path=file_full_path,
193-
output_file_name=output_file_name,
194-
job_id=boa_job.id,
195-
project_url=project_url,
196-
boa_job_list_url=boa_settings.BOA_JOB_LIST_URL,
197-
boa_support_email=boa_settings.BOA_SUPPORT_EMAIL,
198-
osf_support_email=osf_settings.OSF_SUPPORT_EMAIL,
186+
NotificationType.Type.ADDONS_BOA_JOB_COMPLETE.instance.emit(
187+
user=user,
188+
event_context={
189+
'fullname': user.fullname,
190+
'query_file_name': query_file_name,
191+
'query_file_full_path': file_full_path,
192+
'output_file_name': output_file_name,
193+
'job_id': boa_job.id,
194+
'project_url': project_url,
195+
'boa_job_list_url': boa_settings.BOA_JOB_LIST_URL,
196+
'boa_support_email': boa_settings.BOA_SUPPORT_EMAIL,
197+
'osf_support_email': osf_settings.OSF_SUPPORT_EMAIL,
198+
}
199199
)
200200
return BoaErrorCode.NO_ERROR
201201

@@ -209,22 +209,24 @@ def handle_boa_error(message, code, username, fullname, project_url, query_file_
209209
sentry.log_message(message, skip_session=True)
210210
except Exception:
211211
pass
212-
send_mail(
213-
to_addr=username,
214-
mail=ADDONS_BOA_JOB_FAILURE,
215-
fullname=fullname,
216-
code=code,
217-
message=message,
218-
query_file_name=query_file_name,
219-
file_size=file_size,
220-
max_file_size=boa_settings.MAX_SUBMISSION_SIZE,
221-
query_file_full_path=query_file_full_path,
222-
output_file_name=output_file_name,
223-
job_id=job_id,
224-
max_job_wait_hours=boa_settings.MAX_JOB_WAITING_TIME / 3600,
225-
project_url=project_url,
226-
boa_job_list_url=boa_settings.BOA_JOB_LIST_URL,
227-
boa_support_email=boa_settings.BOA_SUPPORT_EMAIL,
228-
osf_support_email=osf_settings.OSF_SUPPORT_EMAIL,
212+
NotificationType.Type.ADDONS_BOA_JOB_FAILURE.instance.emit(
213+
destination_address=username,
214+
event_context={
215+
'user_fullname': fullname,
216+
'code': code,
217+
'query_file_name': query_file_name,
218+
'file_size': file_size,
219+
'message': message,
220+
'max_file_size': boa_settings.MAX_SUBMISSION_SIZE,
221+
'query_file_full_path': query_file_full_path,
222+
'output_file_name': output_file_name,
223+
'job_id': job_id,
224+
'max_job_wait_hours': boa_settings.MAX_JOB_WAITING_TIME / 3600,
225+
'project_url': project_url,
226+
'boa_job_list_url': boa_settings.BOA_JOB_LIST_URL,
227+
'boa_support_email': boa_settings.BOA_SUPPORT_EMAIL,
228+
'osf_support_email': osf_settings.OSF_SUPPORT_EMAIL,
229+
230+
}
229231
)
230232
return code

addons/boa/tests/test_tasks.py

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@
99
from addons.boa import settings as boa_settings
1010
from addons.boa.boa_error_code import BoaErrorCode
1111
from addons.boa.tasks import submit_to_boa, submit_to_boa_async, handle_boa_error
12+
from osf.models import NotificationType
1213
from osf_tests.factories import AuthUserFactory, ProjectFactory
1314
from tests.base import OsfTestCase
15+
from tests.utils import capture_notifications
1416
from website import settings as osf_settings
15-
from website.mails import ADDONS_BOA_JOB_COMPLETE, ADDONS_BOA_JOB_FAILURE
1617

1718
DEFAULT_REFRESH_JOB_INTERVAL = boa_settings.REFRESH_JOB_INTERVAL
1819
DEFAULT_MAX_JOB_WAITING_TIME = boa_settings.MAX_JOB_WAITING_TIME
@@ -38,9 +39,6 @@ def setUp(self):
3839
self.output_file_name = 'fake_boa_script_results.txt'
3940
self.job_id = '1a2b3c4d5e6f7g8'
4041

41-
from conftest import start_mock_send_grid
42-
self.mock_send_grid = start_mock_send_grid(self)
43-
4442
def tearDown(self):
4543
super().tearDown()
4644

@@ -55,27 +53,26 @@ def test_boa_error_code(self):
5553
assert BoaErrorCode.FILE_TOO_LARGE_ERROR == 6
5654
assert BoaErrorCode.JOB_TIME_OUT_ERROR == 7
5755

58-
@mock.patch('website.mails.settings.USE_EMAIL', True)
59-
@mock.patch('website.mails.settings.USE_CELERY', False)
6056
def test_handle_boa_error(self):
61-
with mock.patch('addons.boa.tasks.sentry.log_message', return_value=None) as mock_sentry_log_message, \
62-
mock.patch('addons.boa.tasks.logger.error', return_value=None) as mock_logger_error:
63-
return_value = handle_boa_error(
64-
self.error_message,
65-
BoaErrorCode.UNKNOWN,
66-
self.user_username,
67-
self.user_fullname,
68-
self.project_url,
69-
self.file_full_path,
70-
query_file_name=self.query_file_name,
71-
file_size=self.file_size,
72-
output_file_name=self.output_file_name,
73-
job_id=self.job_id
74-
)
75-
self.mock_send_grid.assert_called()
76-
mock_sentry_log_message.assert_called_with(self.error_message, skip_session=True)
77-
mock_logger_error.assert_called_with(self.error_message)
78-
assert return_value == BoaErrorCode.UNKNOWN
57+
with mock.patch('addons.boa.tasks.sentry.log_message', return_value=None) as mock_sentry_log_message:
58+
with mock.patch('addons.boa.tasks.logger.error', return_value=None) as mock_logger_error:
59+
with capture_notifications() as notifications:
60+
return_value = handle_boa_error(
61+
self.error_message,
62+
BoaErrorCode.UNKNOWN,
63+
self.user_username,
64+
self.user_fullname,
65+
self.project_url,
66+
self.file_full_path,
67+
file_size=self.file_size,
68+
output_file_name=self.output_file_name,
69+
job_id=self.job_id
70+
)
71+
assert len(notifications['emits']) == 1
72+
assert notifications['emits'][0]['type'] == NotificationType.Type.ADDONS_BOA_JOB_FAILURE
73+
mock_sentry_log_message.assert_called_with(self.error_message, skip_session=True)
74+
mock_logger_error.assert_called_with(self.error_message)
75+
assert return_value == BoaErrorCode.UNKNOWN
7976

8077

8178
class TestSubmitToBoa(OsfTestCase):
@@ -154,14 +151,6 @@ def setUp(self):
154151
boa_settings.REFRESH_JOB_INTERVAL = DEFAULT_REFRESH_JOB_INTERVAL
155152
boa_settings.MAX_JOB_WAITING_TIME = DEFAULT_MAX_JOB_WAITING_TIME
156153

157-
from conftest import start_mock_send_grid
158-
self.mock_send_grid = start_mock_send_grid(self)
159-
160-
def tearDown(self):
161-
super().tearDown()
162-
163-
@mock.patch('website.mails.settings.USE_EMAIL', True)
164-
@mock.patch('website.mails.settings.USE_CELERY', False)
165154
async def test_submit_success(self):
166155
with mock.patch('osf.models.user.OSFUser.objects.get', return_value=self.user), \
167156
mock.patch('osf.models.user.OSFUser.get_or_create_cookie', return_value=self.user_cookie), \
@@ -190,7 +179,6 @@ async def test_submit_success(self):
190179
assert self.mock_job.refresh.call_count == 4
191180
assert mock_async_sleep.call_count == 4
192181
mock_close.assert_called()
193-
self.mock_send_grid.assert_called()
194182
mock_handle_boa_error.assert_not_called()
195183

196184
async def test_download_error(self):

addons/osfstorage/tests/test_models.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from osf import models
2525
from addons.osfstorage import utils
2626
from addons.osfstorage import settings
27+
from tests.utils import capture_notifications
2728
from website.files.exceptions import FileNodeCheckedOutError, FileNodeIsPrimaryFile
2829

2930
SessionStore = import_module(django_conf_settings.SESSION_ENGINE).SessionStore
@@ -745,7 +746,8 @@ def test_after_fork_copies_versions(self, node, node_settings, auth_obj):
745746
version = factories.FileVersionFactory()
746747
record.add_version(version)
747748

748-
fork = node.fork_node(auth_obj)
749+
with capture_notifications():
750+
fork = node.fork_node(auth_obj)
749751
fork_node_settings = fork.get_addon('osfstorage')
750752
fork_node_settings.reload()
751753

@@ -757,7 +759,8 @@ def test_fork_reverts_to_node_storage_region(self, user2, region, region2, node,
757759
"""
758760
Despite different user regions defaults, the forked node always stay in the same region as it's orginal node.
759761
"""
760-
fork = node.fork_node(Auth(user2))
762+
with capture_notifications():
763+
fork = node.fork_node(Auth(user2))
761764
assert fork.get_addon('osfstorage').region_id == region.id
762765

763766
# don't inherit or override region

addons/wiki/tests/test_wiki.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from framework.auth import Auth
3030
from django.utils import timezone
3131
from addons.wiki.utils import to_mongo_key
32+
from tests.utils import capture_notifications
3233

3334
from .config import EXAMPLE_DOCS, EXAMPLE_OPS
3435

@@ -818,7 +819,8 @@ def test_uuids_differ_between_forks(self):
818819
assert project_res.status_code == 200
819820
self.project.reload()
820821

821-
fork = self.project.fork_node(Auth(self.user))
822+
with capture_notifications():
823+
fork = self.project.fork_node(Auth(self.user))
822824
assert fork.is_fork_of(self.project)
823825
fork_url = fork.web_url_for('project_wiki_view', wname=self.wname)
824826
fork_res = self.app.get(fork_url, auth=self.user.auth)
@@ -1084,7 +1086,8 @@ def test_get_sharejs_uuid(self):
10841086
# Differs across projects and forks
10851087
project = ProjectFactory()
10861088
assert sharejs_uuid != get_sharejs_uuid(project, wname)
1087-
fork = self.project.fork_node(Auth(self.project.creator))
1089+
with capture_notifications():
1090+
fork = self.project.fork_node(Auth(self.project.creator))
10881091
assert sharejs_uuid != get_sharejs_uuid(fork, wname)
10891092

10901093
def test_generate_share_uuid(self):

admin/common_auth/views.py

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from django.contrib.auth import login, REDIRECT_FIELD_NAME, authenticate, logout
1111

1212
from osf.models.user import OSFUser
13-
from osf.models import AdminProfile, AbstractProvider
13+
from osf.models import AdminProfile
1414
from admin.common_auth.forms import LoginForm, UserRegistrationForm, DeskUserForm
1515

1616

@@ -69,16 +69,6 @@ def form_valid(self, form):
6969

7070
# create AdminProfile for this new user
7171
profile, created = AdminProfile.objects.get_or_create(user=osf_user)
72-
73-
for group in form.cleaned_data.get('group_perms'):
74-
osf_user.groups.add(group)
75-
split = group.name.split('_')
76-
group_type = split[0]
77-
if group_type == 'reviews':
78-
provider_id = split[2]
79-
provider = AbstractProvider.objects.get(id=provider_id)
80-
provider.notification_subscriptions.get(event_name='new_pending_submissions').add_user_to_subscription(osf_user, 'email_transactional')
81-
8272
osf_user.save()
8373

8474
if created:

admin/nodes/views.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
from admin.base.utils import change_embargo_date
2222
from admin.base.views import GuidView
2323
from admin.base.forms import GuidForm
24-
from admin.notifications.views import detect_duplicate_notifications, delete_selected_notifications
24+
from admin.notifications.views import delete_selected_notifications
2525

2626
from api.share.utils import update_share
2727
from api.caching.tasks import update_storage_usage_cache
@@ -100,7 +100,6 @@ def get_context_data(self, **kwargs):
100100
context = super().get_context_data(**kwargs)
101101
node = self.get_object()
102102

103-
detailed_duplicates = detect_duplicate_notifications(node_id=node.id)
104103
children = node.get_nodes(is_node_link=False)
105104
# Annotate guid because django templates prohibit accessing attributes that start with underscores
106105
children = AbstractNode.objects.filter(
@@ -111,7 +110,6 @@ def get_context_data(self, **kwargs):
111110
'STORAGE_LIMITS': settings.StorageLimits,
112111
'node': node,
113112
'children': children,
114-
'duplicates': detailed_duplicates
115113
})
116114

117115
return context

0 commit comments

Comments
 (0)