Skip to content

Commit 74e83b6

Browse files
JohnetordoffJohnetordoff
committed
Merge branch 'refactor-notifications' of https://github.com/CenterForOpenScience/osf.io into add-new-notifications-data-model
* 'refactor-notifications' of https://github.com/CenterForOpenScience/osf.io: fix mail mocking issues for password reset mock mail for tests re-expose quickfiles migrated logs remove logs groups embedding remove logs groups embedding
2 parents 8bbe32f + 4c50c6c commit 74e83b6

11 files changed

Lines changed: 32 additions & 171 deletions

File tree

‎addons/boa/tests/test_tasks.py‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
from addons.boa import settings as boa_settings
1010
from addons.boa.boa_error_code import BoaErrorCode
11-
from addons.boa.tasks import submit_to_boa, handle_boa_error
11+
from addons.boa.tasks import submit_to_boa, submit_to_boa_async, handle_boa_error
1212
from osf.models import NotificationType
1313
from osf_tests.factories import AuthUserFactory, ProjectFactory
1414
from tests.base import OsfTestCase
@@ -96,7 +96,7 @@ def test_submit_to_boa_async_called(self):
9696
new_callable=AsyncMock,
9797
return_value=BoaErrorCode.NO_ERROR
9898
) as mock_submit_to_boa_async:
99-
return_value = submit_to_boa(
99+
return_value = await submit_to_boa_async(
100100
self.host,
101101
self.username,
102102
self.password,
@@ -186,7 +186,7 @@ async def test_download_error(self):
186186
mock.patch('osf.models.user.OSFUser.get_or_create_cookie', return_value=self.user_cookie), \
187187
mock.patch('urllib.request.urlopen', side_effect=http_404), \
188188
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
189-
return_value = submit_to_boa(
189+
return_value = await submit_to_boa_async(
190190
self.host,
191191
self.username,
192192
self.password,
@@ -217,7 +217,7 @@ async def test_login_error(self):
217217
mock.patch('boaapi.boa_client.BoaClient.login', side_effect=BoaException()) as mock_login, \
218218
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
219219
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
220-
return_value = submit_to_boa(
220+
return_value = await submit_to_boa_async(
221221
self.host,
222222
self.username,
223223
self.password,
@@ -251,7 +251,7 @@ async def test_data_set_error(self):
251251
mock.patch('boaapi.boa_client.BoaClient.get_dataset', side_effect=BoaException()) as mock_get_dataset, \
252252
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
253253
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
254-
return_value = submit_to_boa(
254+
return_value = await submit_to_boa_async(
255255
self.host,
256256
self.username,
257257
self.password,
@@ -286,7 +286,7 @@ async def test_submit_error(self):
286286
mock.patch('boaapi.boa_client.BoaClient.query', side_effect=BoaException()) as mock_query, \
287287
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
288288
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
289-
return_value = submit_to_boa(
289+
return_value = await submit_to_boa_async(
290290
self.host,
291291
self.username,
292292
self.password,
@@ -324,7 +324,7 @@ async def test_compile_error(self):
324324
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
325325
mock.patch('asyncio.sleep', new_callable=AsyncMock, return_value=None), \
326326
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
327-
return_value = submit_to_boa(
327+
return_value = await submit_to_boa_async(
328328
self.host,
329329
self.username,
330330
self.password,
@@ -362,7 +362,7 @@ async def test_execute_error(self):
362362
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
363363
mock.patch('asyncio.sleep', new_callable=AsyncMock, return_value=None), \
364364
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
365-
return_value = submit_to_boa(
365+
return_value = await submit_to_boa_async(
366366
self.host,
367367
self.username,
368368
self.password,
@@ -399,7 +399,7 @@ async def test_output_error_(self):
399399
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
400400
mock.patch('asyncio.sleep', new_callable=AsyncMock, return_value=None), \
401401
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
402-
return_value = submit_to_boa(
402+
return_value = await submit_to_boa_async(
403403
self.host,
404404
self.username,
405405
self.password,
@@ -437,7 +437,7 @@ async def test_upload_error_conflict(self):
437437
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
438438
mock.patch('asyncio.sleep', new_callable=AsyncMock, return_value=None), \
439439
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
440-
return_value = submit_to_boa(
440+
return_value = await submit_to_boa_async(
441441
self.host,
442442
self.username,
443443
self.password,
@@ -475,7 +475,7 @@ async def test_upload_error_other(self):
475475
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
476476
mock.patch('asyncio.sleep', new_callable=AsyncMock, return_value=None), \
477477
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
478-
return_value = submit_to_boa(
478+
return_value = await submit_to_boa_async(
479479
self.host,
480480
self.username,
481481
self.password,
@@ -506,7 +506,7 @@ async def test_file_too_large_error(self):
506506
with mock.patch('osf.models.user.OSFUser.objects.get', return_value=self.user), \
507507
mock.patch('osf.models.user.OSFUser.get_or_create_cookie', return_value=self.user_cookie), \
508508
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
509-
return_value = submit_to_boa(
509+
return_value = await submit_to_boa_async(
510510
self.host,
511511
self.username,
512512
self.password,
@@ -542,7 +542,7 @@ async def test_job_timeout_error(self):
542542
mock.patch('boaapi.boa_client.BoaClient.query', return_value=self.mock_job), \
543543
mock.patch('boaapi.boa_client.BoaClient.close', return_value=None) as mock_close, \
544544
mock.patch('addons.boa.tasks.handle_boa_error', return_value=None) as mock_handle_boa_error:
545-
return_value = submit_to_boa(
545+
return_value = await submit_to_boa_async(
546546
self.host,
547547
self.username,
548548
self.password,

‎api/institutions/authentication.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ def authenticate(self, request):
334334

335335
# Send confirmation email for all three: created, confirmed and claimed
336336
NotificationType.objects.get(
337-
name=NotificationType.Type.USER_WELCOME_OSF4I.value,
337+
name=NotificationType.Type.USER_WELCOME_OSF4I,
338338
).emit(
339339
user=user,
340340
message_frequency='instantly',

‎api/nodes/serializers.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1273,7 +1273,7 @@ def create(self, validated_data):
12731273
'preprint': NotificationType.Type.PREPRINT_CONTRIBUTOR_ADDED_DEFAULT if is_published else False,
12741274
}.get(email_pref, False)
12751275
contributor = OSFUser.load(user_id)
1276-
notification_type = notification_type if email or (contributor and contributor.is_registered) else False,
1276+
notification_type = notification_type if email or (contributor and contributor.is_registered) else False
12771277

12781278
try:
12791279
return resource.add_contributor_registered_or_not(

‎api_tests/users/views/test_user_settings.py‎

Lines changed: 2 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
1-
import urllib
21
from unittest import mock
32
import pytest
43

5-
from api.base.settings.defaults import API_BASE, CSRF_COOKIE_NAME
4+
from api.base.settings.defaults import API_BASE
65
from api.base.utils import hashids
76
from osf_tests.factories import (
87
AuthUserFactory,
98
UserFactory,
109
)
11-
from website import settings
12-
from django.middleware import csrf
1310
from osf.models import Email, NotableDomain, NotificationType
1411
from framework.auth.views import auth_email_logout
1512
from tests.utils import capture_notifications
@@ -132,8 +129,7 @@ def test_post(self, app, user_one, user_two, url, payload):
132129
assert res.status_code == 403
133130

134131
# Logged in
135-
with mock.patch.object(settings, 'USE_EMAIL', False):
136-
res = app.post_json_api(url, payload, auth=user_one.auth)
132+
res = app.post_json_api(url, payload, auth=user_one.auth)
137133
assert res.status_code == 204
138134
user_one.reload()
139135
assert user_one.check_password('password2')
@@ -172,141 +168,6 @@ def test_multiple_errors(self, app, user_one, url, payload):
172168
assert res.json['errors'][1]['detail'] == 'Password should be at least eight characters'
173169

174170

175-
@pytest.mark.django_db
176-
class TestResetPassword:
177-
178-
@pytest.fixture()
179-
def user_one(self):
180-
user = UserFactory()
181-
user.set_password('password1')
182-
user.auth = (user.username, 'password1')
183-
user.save()
184-
return user
185-
186-
@pytest.fixture()
187-
def url(self):
188-
return f'/{API_BASE}users/reset_password/'
189-
190-
@pytest.fixture
191-
def csrf_token(self):
192-
return csrf._mask_cipher_secret(csrf._get_new_csrf_string())
193-
194-
def test_get(self, app, url, user_one):
195-
encoded_email = urllib.parse.quote(user_one.email)
196-
url = f'{url}?email={encoded_email}'
197-
with capture_notifications() as notifications:
198-
res = app.get(url)
199-
assert res.status_code == 200
200-
201-
user_one.reload()
202-
assert len(notifications) == 1
203-
assert notifications[0]['kwargs']['user'].username == user_one.username
204-
205-
def test_get_invalid_email(self, app, url):
206-
url = f'{url}?email={'invalid_email'}'
207-
with capture_notifications() as notifications:
208-
res = app.get(url)
209-
assert res.status_code == 200
210-
assert not notifications
211-
212-
def test_post(self, app, url, user_one, csrf_token):
213-
app.set_cookie(CSRF_COOKIE_NAME, csrf_token)
214-
encoded_email = urllib.parse.quote(user_one.email)
215-
url = f'{url}?email={encoded_email}'
216-
res = app.get(url)
217-
user_one.reload()
218-
payload = {
219-
'data': {
220-
'attributes': {
221-
'uid': user_one._id,
222-
'token': user_one.verification_key_v2['token'],
223-
'password': 'password2',
224-
}
225-
}
226-
}
227-
228-
res = app.post_json_api(url, payload, headers={'X-CSRFToken': csrf_token})
229-
user_one.reload()
230-
assert res.status_code == 200
231-
assert user_one.check_password('password2')
232-
233-
def test_post_empty_payload(self, app, url, csrf_token):
234-
app.set_cookie(CSRF_COOKIE_NAME, csrf_token)
235-
payload = {
236-
'data': {
237-
'attributes': {
238-
}
239-
}
240-
}
241-
res = app.post_json_api(url, payload, expect_errors=True, headers={'X-CSRFToken': csrf_token})
242-
assert res.status_code == 400
243-
244-
def test_post_invalid_token(self, app, url, user_one, csrf_token):
245-
app.set_cookie(CSRF_COOKIE_NAME, csrf_token)
246-
payload = {
247-
'data': {
248-
'attributes': {
249-
'uid': user_one._id,
250-
'token': 'invalid_token',
251-
'password': 'password2',
252-
}
253-
}
254-
}
255-
res = app.post_json_api(url, payload, expect_errors=True, headers={'X-THROTTLE-TOKEN': 'test-token', 'X-CSRFToken': csrf_token})
256-
assert res.status_code == 400
257-
258-
def test_post_invalid_password(self, app, url, user_one, csrf_token):
259-
app.set_cookie(CSRF_COOKIE_NAME, csrf_token)
260-
encoded_email = urllib.parse.quote(user_one.email)
261-
url = f'{url}?email={encoded_email}'
262-
res = app.get(url)
263-
user_one.reload()
264-
payload = {
265-
'data': {
266-
'attributes': {
267-
'uid': user_one._id,
268-
'token': user_one.verification_key_v2['token'],
269-
'password': user_one.username,
270-
}
271-
}
272-
}
273-
274-
res = app.post_json_api(url, payload, expect_errors=True, headers={'X-THROTTLE-TOKEN': 'test-token', 'X-CSRFToken': csrf_token})
275-
assert res.status_code == 400
276-
277-
def test_throttle(self, app, url, user_one, csrf_token):
278-
encoded_email = urllib.parse.quote(user_one.email)
279-
url = f'{url}?email={encoded_email}'
280-
app.get(url)
281-
user_one.reload()
282-
payload = {
283-
'data': {
284-
'attributes': {
285-
'uid': user_one._id,
286-
'token': user_one.verification_key_v2['token'],
287-
'password': '12345',
288-
}
289-
}
290-
}
291-
292-
res = app.post_json_api(
293-
url,
294-
payload,
295-
headers={'X-CSRFToken': csrf_token},
296-
expect_errors=True
297-
)
298-
res = app.post_json_api(
299-
url,
300-
payload,
301-
headers={'X-CSRFToken': csrf_token},
302-
expect_errors=True
303-
)
304-
assert res.status_code == 429
305-
306-
res = app.get(url, expect_errors=True)
307-
assert res.json['message'] == 'You have recently requested to change your password. Please wait a few minutes before trying again.'
308-
309-
310171
@pytest.mark.django_db
311172
class TestUserEmailsList:
312173

‎framework/email/tasks.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ def send_email(
6262
6363
:return: True if successful
6464
"""
65-
if not settings.USE_EMAIL:
66-
return
6765
if settings.SENDGRID_API_KEY:
6866
return _send_with_sendgrid(
6967
from_addr=from_addr,

‎osf/models/notification.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,17 @@ def send(
3131
"""
3232
if not protocol_type == 'email':
3333
raise NotImplementedError(f'Protocol type {protocol_type}. Email notifications are only implemented.')
34-
3534
recipient_address = destination_address or self.subscription.user.username
35+
36+
logging.info(
37+
f"Attempting to send Notification:"
38+
f"\nto={getattr(self.subscription.user, 'username', destination_address)}"
39+
f"\nat={destination_address}"
40+
f"\ntype={self.subscription.notification_type}"
41+
f"\ncontext={self.event_context}"
42+
f"\nemail={email_context}"
43+
)
44+
3645
if protocol_type == 'email' and waffle.switch_is_active(features.ENABLE_MAILHOG):
3746
email.send_email_over_smtp(
3847
recipient_address,

‎osf/models/notification_subscription.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,12 @@ def emit(
7171
"""
7272
logging.info(
7373
f"Attempting to create Notification:"
74-
f"\nto={self.user.username}"
74+
f"\nto={getattr(self.user, 'username', destination_address)}"
7575
f"\ntype={self.notification_type.name}"
7676
f"\nmessage_frequency={self.message_frequency}"
7777
f"\ncontext={event_context}"
78+
f"\nemail={email_context}"
79+
7880
)
7981
if self.message_frequency == 'instantly':
8082
notification = Notification.objects.create(

‎pytest.ini‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
[pytest]
2-
addopts = --ds=osf_tests.settings --tb=short --allow-hosts=127.0.0.1,192.168.168.167,localhost
2+
addopts = --ds=osf_tests.settings --tb=short --reuse-db --allow-hosts=127.0.0.1,192.168.168.167,localhost
33
filterwarnings =
44
once::UserWarning
55
ignore:.*U.*mode is deprecated:DeprecationWarning

‎tests/framework_tests/test_email.py‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,6 @@
2424

2525
class TestEmail(unittest.TestCase):
2626

27-
@unittest.skipIf(not SERVER_RUNNING,
28-
"Mailserver isn't running. Run \"invoke mailserver\".")
29-
@unittest.skipIf(not settings.USE_EMAIL,
30-
'settings.USE_EMAIL is False')
31-
def test_sending_email(self):
32-
assert send_email('foo@bar.com', 'baz@quux.com', subject='no subject',
33-
message='<h1>Greetings!</h1>', ttls=False, login=False)
34-
3527
def setUp(self):
3628
settings.SENDGRID_WHITELIST_MODE = False
3729

‎tests/identifiers/test_datacite.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ def _assert_unordered_list_of_dicts_equal(actual_list_of_dicts, expected_list_of
2929

3030
@pytest.mark.django_db
3131
@pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
32-
@mock.patch('website.mails.settings.USE_EMAIL', False)
3332
class TestDataCiteClient:
3433

3534
@pytest.fixture()

0 commit comments

Comments
 (0)
⚡