Skip to content

Commit 936a4e1

Browse files
JohnetordoffJohnetordoff
authored
Merge pull request #11295 from Ostap-Zherebetskyi/fix/file_event_emails
[ENG-8868] File Operation: No notifications sent when file upload failed
2 parents 054fe23 + 7fb32f1 commit 936a4e1

File tree

4 files changed

+176
-5
lines changed

4 files changed

+176
-5
lines changed

‎addons/base/views.py‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -584,12 +584,18 @@ def create_waterbutler_log(payload, **kwargs):
584584
event_context={
585585
'user_fullname': user.fullname,
586586
'action': payload['action'],
587-
'source_node': source_node,
588-
'destination_node': destination_node,
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,
589592
'source_path': payload['source']['materialized'],
590593
'source_addon': payload['source']['addon'],
591594
'destination_addon': payload['destination']['addon'],
592-
'osf_support_email': settings.OSF_SUPPORT_EMAIL
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,
593599
}
594600
)
595601
if payload.get('errors'):

‎api_tests/mailhog/test_create_wb_log.py‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import time
2+
import pytest
3+
from framework.auth import signing
4+
from waffle.testutils import override_switch
5+
from osf import features
6+
from osf_tests.factories import (
7+
AuthUserFactory,
8+
NodeFactory
9+
)
10+
from tests.base import (
11+
OsfTestCase,
12+
)
13+
from tests.utils import get_mailhog_messages, delete_mailhog_messages, assert_emails
14+
from importlib import import_module
15+
16+
from django.conf import settings as django_conf_settings
17+
import itsdangerous
18+
from framework.auth.core import Auth
19+
from addons.osfstorage.models import OsfStorageFile
20+
from tests.utils import capture_notifications
21+
from website import settings
22+
23+
SessionStore = import_module(django_conf_settings.SESSION_ENGINE).SessionStore
24+
25+
pytestmark = pytest.mark.django_db
26+
27+
28+
class TestCreateWBLog(OsfTestCase):
29+
30+
def setUp(self):
31+
super().setUp()
32+
self.user = AuthUserFactory()
33+
self.user_non_contrib = AuthUserFactory()
34+
self.auth_obj = Auth(user=self.user)
35+
self.node = NodeFactory(creator=self.user)
36+
self.file = OsfStorageFile.create(
37+
target=self.node,
38+
path='/testfile',
39+
_id='testfile',
40+
name='testfile',
41+
materialized_path='/testfile'
42+
)
43+
self.file.save()
44+
self.session = SessionStore()
45+
self.session['auth_user_id'] = self.user._id
46+
self.session.create()
47+
self.cookie = itsdangerous.Signer(settings.SECRET_KEY).sign(self.session.session_key)
48+
49+
self.source_dest = {
50+
'source': {
51+
'nid': self.node._id,
52+
'resource': self.node._id,
53+
'provider': 'osfstorage',
54+
'kind': 'file',
55+
'path': '/68c416d5993abb955649e39b',
56+
'name': 'file.png',
57+
'materialized': '/file.png',
58+
'extra': {}
59+
},
60+
'destination': {
61+
'nid': self.node._id,
62+
'resource': self.node._id,
63+
'extra': {
64+
'guid': None,
65+
'version': 1,
66+
'downloads': 0,
67+
'checkout': None,
68+
'latestVersionSeen': None,
69+
'hashes': {
70+
'md5': '4df1cc7556a50f437318bed256795b99',
71+
'sha256': 'd22324aa85762f3eeec36987b99a4e23067b492d8296ef246bb5d3eac0c21842'
72+
}
73+
},
74+
'kind': 'file',
75+
'name': 'file.png',
76+
'path': '/68c416d5993abb955649e39b',
77+
'provider': 'osfstorage',
78+
'materialized': '/file.png',
79+
'etag': '568e49bea15354e35105a828a1775351921f2be70dedfcb3482d56f574189b6e',
80+
'contentType': None,
81+
'modified': '2025-09-12T12:49:25.352543+00:00',
82+
'modified_utc': '2025-09-12T12:49:25.352543+00:00',
83+
'created_utc': '2025-09-12T12:49:25.352543+00:00',
84+
'size': 208287,
85+
'sizeInt': 208287
86+
}
87+
}
88+
89+
def build_payload(self, action, metadata, **kwargs):
90+
options = dict(
91+
auth={'id': self.user._id},
92+
action=action,
93+
provider='osfstorage',
94+
metadata=metadata,
95+
time=time.time() + 1000,
96+
**self.source_dest
97+
)
98+
options.update(kwargs)
99+
options = {
100+
key: value
101+
for key, value in options.items()
102+
if value is not None
103+
}
104+
message, signature = signing.default_signer.sign_payload(options)
105+
return {
106+
'payload': message,
107+
'signature': signature,
108+
}
109+
110+
@override_switch(features.ENABLE_MAILHOG, active=True)
111+
def test_log_move_file_error(self):
112+
path = 'pizza'
113+
url = self.node.api_url_for('create_waterbutler_log')
114+
payload = self.build_payload(action='move', metadata={'nid': self.node._id, 'materialized': path, 'kind': 'file', 'path': path}, errors=['some error'])
115+
delete_mailhog_messages()
116+
117+
with capture_notifications(passthrough=True) as notifications:
118+
self.app.put(url, json=payload)
119+
120+
mailhog = get_mailhog_messages()
121+
assert mailhog['count'] == len(notifications['emails'])
122+
assert_emails(mailhog, notifications)
123+
124+
@override_switch(features.ENABLE_MAILHOG, active=True)
125+
def test_log_copy_file_error(self):
126+
path = 'pizza'
127+
url = self.node.api_url_for('create_waterbutler_log')
128+
payload = self.build_payload(action='copy', metadata={'nid': self.node._id, 'materialized': path, 'kind': 'file', 'path': path}, errors=['some error'])
129+
delete_mailhog_messages()
130+
131+
with capture_notifications(passthrough=True) as notifications:
132+
self.app.put(url, json=payload)
133+
mailhog = get_mailhog_messages()
134+
assert mailhog['count'] == len(notifications['emails'])
135+
assert_emails(mailhog, notifications)
136+
137+
@override_switch(features.ENABLE_MAILHOG, active=True)
138+
def test_log_move_file_success(self):
139+
path = 'pizza'
140+
url = self.node.api_url_for('create_waterbutler_log')
141+
payload = self.build_payload(action='move', metadata={'nid': self.node._id, 'materialized': path, 'kind': 'file', 'path': path})
142+
delete_mailhog_messages()
143+
144+
with capture_notifications(passthrough=True) as notifications:
145+
self.app.put(url, json=payload)
146+
147+
mailhog = get_mailhog_messages()
148+
assert mailhog['count'] == len(notifications['emails'])
149+
messages = {'count': mailhog['count'], 'items': mailhog['items'][::-1]} # Reverse to get chronological order
150+
assert_emails(messages, notifications)
151+
152+
@override_switch(features.ENABLE_MAILHOG, active=True)
153+
def test_log_copy_file_success(self):
154+
path = 'pizza'
155+
url = self.node.api_url_for('create_waterbutler_log')
156+
payload = self.build_payload(action='copy', metadata={'nid': self.node._id, 'materialized': path, 'kind': 'file', 'path': path})
157+
delete_mailhog_messages()
158+
159+
with capture_notifications(passthrough=True) as notifications:
160+
self.app.put(url, json=payload)
161+
162+
mailhog = get_mailhog_messages()
163+
assert mailhog['count'] == len(notifications['emails'])
164+
messages = {'count': mailhog['count'], 'items': mailhog['items'][::-1]} # Reverse to get chronological order
165+
assert_emails(messages, notifications)

‎website/templates/file_operation_failed.html.mako‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
<th colspan="2" style="padding: 0px 15px 0 15px">
4747
<h3 style="padding: 0 15px 5px 15px; margin: 30px 0 0 0;border: none;list-style: none;font-weight: 300; border-bottom: 1px solid #eee; text-align: left;">
4848
${destination_node_title}
49-
%if destination_node.parent_node:
49+
%if destination_node_parent_node_title:
5050
<small style="font-size: 14px;color: #999;"> in ${destination_node_parent_node_title} </small>
5151
%endif
5252
</h3>

‎website/templates/file_operation_success.html.mako‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
<th colspan="2" style="padding: 0px 15px 0 15px">
4747
<h3 style="padding: 0 15px 5px 15px; margin: 30px 0 0 0;border: none;list-style: none;font-weight: 300; border-bottom: 1px solid #eee; text-align: left;">
4848
${destination_node_title}
49-
%if destination_node.parent_node:
49+
%if destination_node_parent_node_title:
5050
<small style="font-size: 14px;color: #999;"> in ${destination_node_parent_node_title} </small>
5151
%endif
5252
</h3>

0 commit comments

Comments
 (0)