Skip to content

Commit 7344336

Browse files
committed
support PM delete opration on Recipient side only
1 parent 90ce14b commit 7344336

16 files changed

+749
-4
lines changed

r2/r2/controllers/api.py

+25
Original file line numberDiff line numberDiff line change
@@ -1723,6 +1723,31 @@ def POST_report(self, form, jquery, thing, reason, site_reason, other_reason):
17231723
parent_div.removeClass("active")
17241724
parent_div.html("")
17251725

1726+
@require_oauth2_scope("privatemessages")
1727+
@noresponse(
1728+
VUser(),
1729+
VModhash(),
1730+
thing=VByName('id'),
1731+
)
1732+
@api_doc(api_section.messages)
1733+
def POST_del_msg(self, thing):
1734+
"""Delete messages from the recipient's view of their inbox."""
1735+
if not thing:
1736+
return
1737+
1738+
if not isinstance(thing, Message):
1739+
return
1740+
1741+
if thing.to_id != c.user._id:
1742+
return
1743+
1744+
thing.del_on_recipient = True
1745+
thing._commit()
1746+
1747+
# report the message deletion to data pipeline
1748+
g.events.message_event(thing, "ss.delete_message",
1749+
request=request, context=c)
1750+
17261751
@require_oauth2_scope("privatemessages")
17271752
@noresponse(
17281753
VUser(),

r2/r2/controllers/listingcontroller.py

+5
Original file line numberDiff line numberDiff line change
@@ -1128,6 +1128,11 @@ def keep(item):
11281128
if item.author_id in c.user.enemies:
11291129
return False
11301130

1131+
# do not show messages which were deleted on recipient
1132+
if (isinstance(item, Message) and
1133+
item.to_id == c.user._id and item.del_on_recipient):
1134+
return False
1135+
11311136
if item.author_id == c.user._id:
11321137
return wouldkeep
11331138

r2/r2/lib/eventcollector.py

+3-2
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ def modmail_event(self, message, request=None, context=None):
571571

572572
event = Event(
573573
topic="message_events",
574-
event_type="ss.send_message",
574+
event_type=event_type,
575575
time=message._date,
576576
request=request,
577577
context=context,
@@ -624,7 +624,8 @@ def modmail_event(self, message, request=None, context=None):
624624

625625
@squelch_exceptions
626626
@sampled("events_collector_message_sample_rate")
627-
def message_event(self, message, request=None, context=None):
627+
def message_event(self, message, event_type="ss.send_message",
628+
request=None, context=None):
628629
"""Create a 'message' event for event-collector.
629630
630631
message: An r2.models.Message object

r2/r2/lib/pages/things.py

+3
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,8 @@ def __init__(self, thing, delete = False, report = True):
279279
can_block = True
280280
can_mute = False
281281
is_admin_message = False
282+
del_on_recipient = (isinstance(thing, Message) and
283+
thing.del_on_recipient)
282284

283285
if not was_comment:
284286
first_message = thing
@@ -322,6 +324,7 @@ def __init__(self, thing, delete = False, report = True):
322324
can_block = can_block,
323325
can_mute = can_mute,
324326
is_admin_message = is_admin_message,
327+
del_on_recipient=del_on_recipient,
325328
)
326329

327330

r2/r2/models/builder.py

+6
Original file line numberDiff line numberDiff line change
@@ -1853,6 +1853,12 @@ def _viewable_message(self, message):
18531853

18541854
if message.author_id in self.user.enemies:
18551855
return False
1856+
1857+
# do not show messages which were deleted on recipient
1858+
if (hasattr(message, "del_on_recipient") and
1859+
message.to_id == c.user._id and message.del_on_recipient):
1860+
return False
1861+
18561862
return super(UserMessageBuilder, self)._viewable_message(message)
18571863

18581864
def get_tree(self):

r2/r2/models/link.py

+8-1
Original file line numberDiff line numberDiff line change
@@ -1996,6 +1996,7 @@ class Message(Thing, Printable):
19961996
display_to=None,
19971997
email_id=None,
19981998
sent_via_email=False,
1999+
del_on_recipient=False,
19992000
)
20002001
_data_int_props = Thing._data_int_props + ('reported',)
20012002
_essentials = ('author_id',)
@@ -2471,7 +2472,13 @@ def wrapped_cache_key(wrapped, style):
24712472
return s
24722473

24732474
def keep_item(self, wrapped):
2474-
return c.user_is_admin or not wrapped.enemy
2475+
if c.user_is_admin:
2476+
return True
2477+
# do not keep message which were deleted on recipient
2478+
if (isinstance(self, Message) and
2479+
self.to_id == c.user._id and self.del_on_recipient):
2480+
return False
2481+
return not wrapped.enemy
24752482

24762483

24772484
class _SaveHideByAccount(tdb_cassandra.DenormalizedRelation):

r2/r2/templates/message.html

+8
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,14 @@
8383
${"[%s]" % ("+" if thing.collapsed else "–")}
8484
</a>
8585

86+
%if c.user_is_admin:
87+
%if not thing.was_comment and thing.del_on_recipient:
88+
<em>${_("deleted message by")}</em>&#32;
89+
%endif
90+
${WrappedUser(thing.to, thing.attribs, thing)}
91+
&#32;
92+
%endif
93+
8694
<%
8795
substitutions = {}
8896

r2/r2/templates/printablebuttons.html

+6
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,12 @@
544544
</li>
545545
%endif
546546
%if thing.user_is_recipient:
547+
## only allow message deleting on recipient side now
548+
%if not (thing.was_comment or thing.thing.del_on_recipient):
549+
<li>
550+
${ynbutton(_("delete"), _("deleted"), "del_msg", "hide_thing", access_required=False, event_action="delete_message")}
551+
</li>
552+
%endif
547553
%if thing.can_block:
548554
${self.banbuttons()}
549555
%if thing.thing.author_id != c.user._id and thing.thing.author_id not in c.user.enemies:

r2/r2/tests/__init__.py

+48-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@
3737
import paste.script.appinstall
3838
from paste.deploy import loadapp
3939

40-
__all__ = ['RedditTestCase']
40+
from routes.util import url_for
41+
from r2.lib.utils import query_string
42+
43+
44+
__all__ = ['RedditTestCase', 'RedditControllerTestCase']
4145

4246
here_dir = os.path.dirname(os.path.abspath(__file__))
4347
conf_dir = os.path.dirname(os.path.dirname(here_dir))
@@ -214,6 +218,8 @@ def set_multi(self, *a, **kw):
214218

215219

216220
class RedditControllerTestCase(RedditTestCase):
221+
CONTROLLER = None
222+
ACTIONS = {}
217223

218224
def setUp(self):
219225
super(RedditControllerTestCase, self).setUp()
@@ -240,6 +246,47 @@ def setUp(self):
240246
cache=NonCache(),
241247
)
242248

249+
# mock out for controllers UTs which use
250+
# r2.lib.controllers as part of the flow.
251+
self.autopatch(g.events, "queue_production", MockEventQueue())
252+
self.autopatch(g.events, "queue_test", MockEventQueue())
253+
254+
self.simple_event = self.autopatch(g.stats, "simple_event")
255+
256+
self.user_agent = "Hacky McBrowser/1.0"
257+
self.device_id = None
258+
243259
# Lastly, pull the app out of test mode so it'll load controllers on
244260
# first use
245261
RedditApp.test_mode = False
262+
263+
def do_post(self, action, params, headers=None, expect_errors=False):
264+
265+
assert self.CONTROLLER is not None
266+
267+
body = self.make_qs(**params)
268+
269+
headers = headers or {}
270+
headers.setdefault('User-Agent', self.user_agent)
271+
if self.device_id:
272+
headers.setdefault('Client-Vendor-ID', self.device_id)
273+
for k, v in self.additional_headers(headers, body).iteritems():
274+
headers.setdefault(k, v)
275+
headers = {k: v for k, v in headers.iteritems() if v is not None}
276+
return self.app.post(
277+
url_for(controller=self.CONTROLLER,
278+
action=self.ACTIONS.get(action, action)),
279+
extra_environ={"REMOTE_ADDR": "1.2.3.4"},
280+
headers=headers,
281+
params=body,
282+
expect_errors=expect_errors,
283+
)
284+
285+
def make_qs(self, **kw):
286+
"""Convert the provided kw into a kw string suitable for app.post."""
287+
return query_string(kw).lstrip("?")
288+
289+
def additional_headers(self, headers, body):
290+
"""Additional generated headers to be added to the request.
291+
"""
292+
return {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
# The contents of this file are subject to the Common Public Attribution
2+
# License Version 1.0. (the "License"); you may not use this file except in
3+
# compliance with the License. You may obtain a copy of the License at
4+
# http://code.reddit.com/LICENSE. The License is based on the Mozilla Public
5+
# License Version 1.1, but Sections 14 and 15 have been added to cover use of
6+
# software over a computer network and provide for limited attribution for the
7+
# Original Developer. In addition, Exhibit A has been modified to be consistent
8+
# with Exhibit B.
9+
#
10+
# Software distributed under the License is distributed on an "AS IS" basis,
11+
# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License for
12+
# the specific language governing rights and limitations under the License.
13+
#
14+
# The Original Code is reddit.
15+
#
16+
# The Original Developer is the Initial Developer. The Initial Developer of
17+
# the Original Code is reddit Inc.
18+
#
19+
# All portions of the code written by reddit are Copyright (c) 2006-2015 reddit
20+
# Inc. All Rights Reserved.
21+
###############################################################################
22+
import contextlib
23+
24+
from r2.tests import RedditControllerTestCase
25+
from mock import patch, MagicMock
26+
from r2.lib.validator import VByName, VUser, VModhash
27+
28+
from r2.models import Link, Message, Account
29+
30+
from pylons import app_globals as g
31+
32+
33+
class DelMsgTest(RedditControllerTestCase):
34+
CONTROLLER = "api"
35+
36+
def setUp(self):
37+
super(DelMsgTest, self).setUp()
38+
39+
self.id = 1
40+
41+
def test_del_msg_success(self):
42+
"""Del_msg succeeds: Returns 200 and sets del_on_recipient."""
43+
message = MagicMock(spec=Message)
44+
message.name = "msg_1"
45+
message.to_id = self.id
46+
message.del_on_recipient = False
47+
48+
with self.mock_del_msg(message):
49+
res = self.do_del_msg(message.name)
50+
51+
self.assertEqual(res.status, 200)
52+
self.assertTrue(message.del_on_recipient)
53+
54+
def test_del_msg_failure_with_link(self):
55+
"""Del_msg fails: Returns 200 and does not set del_on_recipient."""
56+
link = MagicMock(spec=Link)
57+
link.del_on_recipient = False
58+
link.name = "msg_2"
59+
60+
with self.mock_del_msg(link):
61+
res = self.do_del_msg(link.name)
62+
63+
self.assertEqual(res.status, 200)
64+
self.assertFalse(link.del_on_recipient)
65+
66+
def test_del_msg_failure_with_null_msg(self):
67+
"""Del_msg fails: Returns 200 and does not set del_on_recipient."""
68+
message = MagicMock(spec=Message)
69+
message.name = "msg_3"
70+
message.to_id = self.id
71+
message.del_on_recipient = False
72+
73+
with self.mock_del_msg(message, False):
74+
res = self.do_del_msg(message.name)
75+
76+
self.assertEqual(res.status, 200)
77+
self.assertFalse(message.del_on_recipient)
78+
79+
def test_del_msg_failure_with_sender(self):
80+
"""Del_msg fails: Returns 200 and does not set del_on_recipient."""
81+
message = MagicMock(spec=Message)
82+
message.name = "msg_3"
83+
message.to_id = self.id + 1
84+
message.del_on_recipient = False
85+
86+
with self.mock_del_msg(message):
87+
res = self.do_del_msg(message.name)
88+
89+
self.assertEqual(res.status, 200)
90+
self.assertFalse(message.del_on_recipient)
91+
92+
def mock_del_msg(self, thing, ret=True):
93+
"""Context manager for mocking del_msg."""
94+
95+
return contextlib.nested(
96+
patch.object(VByName, "run", return_value=thing if ret else None),
97+
patch.object(VModhash, "run", side_effect=None),
98+
patch.object(VUser, "run", side_effect=None),
99+
patch.object(thing, "_commit", side_effect=None),
100+
patch.object(Account, "_id", self.id, create=True),
101+
patch.object(g.events, "message_event", side_effect=None),
102+
)
103+
104+
def do_del_msg(self, name, **kw):
105+
return self.do_post("del_msg", {"id": name}, **kw)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# The contents of this file are subject to the Common Public Attribution
2+
# License Version 1.0. (the "License"); you may not use this file except in
3+
# compliance with the License. You may obtain a copy of the License at
4+
# http://code.reddit.com/LICENSE. The License is based on the Mozilla Public
5+
# License Version 1.1, but Sections 14 and 15 have been added to cover use of
6+
# software over a computer network and provide for limited attribution for the
7+
# Original Developer. In addition, Exhibit A has been modified to be consistent
8+
# with Exhibit B.
9+
#
10+
# Software distributed under the License is distributed on an "AS IS" basis,
11+
# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License for
12+
# the specific language governing rights and limitations under the License.
13+
#
14+
# The Original Code is reddit.
15+
#
16+
# The Original Developer is the Initial Developer. The Initial Developer of
17+
# the Original Code is reddit Inc.
18+
#
19+
# All portions of the code written by reddit are Copyright (c) 2006-2015 reddit
20+
# Inc. All Rights Reserved.
21+
###############################################################################
22+
from r2.tests import RedditControllerTestCase
23+
from common import LoginRegBase
24+
25+
26+
class LoginRegTests(LoginRegBase, RedditControllerTestCase):
27+
CONTROLLER = "api"
28+
29+
def setUp(self):
30+
RedditControllerTestCase.setUp(self)
31+
LoginRegBase.setUp(self)
32+
33+
def assert_success(self, res):
34+
self.assertEqual(res.status, 200)
35+
self.assertTrue("error" not in res)
36+
37+
def assert_failure(self, res, code=None):
38+
self.assertEqual(res.status, 200)
39+
self.assertTrue("error" in res)
40+
self.assertTrue(code in res)

0 commit comments

Comments
 (0)