From 226e3cbf020436bb5cca2fd78c008382343ed1fd Mon Sep 17 00:00:00 2001 From: Arpith Siromoney Date: Sun, 4 Dec 2016 15:20:32 +0530 Subject: [PATCH] Reactions backend: make endpoints more REST-ful. Adding a reaction is now a PUT request to /messages//emoji_reactions/ Similarly, removing a reaction is now a DELETE request to /messages//emoji_reactions/ This commit changes the url and updates the views and tests. This commit also adds a test for invalid emoji when removing reaction. --- zerver/tests/test_reactions.py | 92 ++++++++++++++++------------------ zerver/views/reactions.py | 10 ++-- zproject/urls.py | 7 +-- 3 files changed, 50 insertions(+), 59 deletions(-) diff --git a/zerver/tests/test_reactions.py b/zerver/tests/test_reactions.py index 9213ede384..4b4c087b32 100644 --- a/zerver/tests/test_reactions.py +++ b/zerver/tests/test_reactions.py @@ -16,28 +16,28 @@ class ReactionEmojiTest(ZulipTestCase): Sending reaction without emoji fails """ sender = 'hamlet@zulip.com' - result = self.client_post('/api/v1/reactions', {'message_id': 1}, - **self.api_auth(sender)) - self.assert_json_error(result, "Missing 'emoji' argument") + result = self.client_put('/api/v1/messages/1/emoji_reactions', + **self.api_auth(sender)) + self.assertEquals(result.status_code, 404) - def test_empty_emoji(self): - # type: () -> None - """ - Sending empty emoji fails - """ - sender = 'hamlet@zulip.com' - result = self.client_post('/api/v1/reactions', {'message_id': 1, 'emoji': ''}, - **self.api_auth(sender)) - self.assert_json_error(result, "Emoji '' does not exist") - - def test_invalid_emoji(self): + def test_add_invalid_emoji(self): # type: () -> None """ Sending invalid emoji fails """ sender = 'hamlet@zulip.com' - result = self.client_post('/api/v1/reactions', {'message_id': 1, 'emoji': 'foo'}, - **self.api_auth(sender)) + result = self.client_put('/api/v1/messages/1/emoji_reactions/foo', + **self.api_auth(sender)) + self.assert_json_error(result, "Emoji 'foo' does not exist") + + def test_remove_invalid_emoji(self): + # type: () -> None + """ + Removing invalid emoji fails + """ + sender = 'hamlet@zulip.com' + result = self.client_delete('/api/v1/messages/1/emoji_reactions/foo', + **self.api_auth(sender)) self.assert_json_error(result, "Emoji 'foo' does not exist") def test_valid_emoji(self): @@ -46,8 +46,8 @@ class ReactionEmojiTest(ZulipTestCase): Reacting with valid emoji succeeds """ sender = 'hamlet@zulip.com' - result = self.client_post('/api/v1/reactions', {'message_id': 1, 'emoji': 'smile'}, - **self.api_auth(sender)) + result = self.client_put('/api/v1/messages/1/emoji_reactions/smile', + **self.api_auth(sender)) self.assert_json_success(result) self.assertEqual(200, result.status_code) @@ -69,8 +69,8 @@ class ReactionEmojiTest(ZulipTestCase): self.assert_json_success(result) self.assertTrue(emoji_name in content["emoji"]) - result = self.client_post('/api/v1/reactions', {'message_id': 1, 'emoji': emoji_name}, - **self.api_auth(sender)) + result = self.client_put('/api/v1/messages/1/emoji_reactions/%s' % (emoji_name,), + **self.api_auth(sender)) self.assert_json_success(result) class ReactionMessageIDTest(ZulipTestCase): @@ -80,9 +80,9 @@ class ReactionMessageIDTest(ZulipTestCase): Reacting without a message_id fails """ sender = 'hamlet@zulip.com' - result = self.client_post('/api/v1/reactions', {'emoji': 'smile'}, - **self.api_auth(sender)) - self.assert_json_error(result, "Missing 'message_id' argument") + result = self.client_put('/api/v1/messages//emoji_reactions/smile', + **self.api_auth(sender)) + self.assertEquals(result.status_code, 404) def test_invalid_message_id(self): # type: () -> None @@ -90,10 +90,9 @@ class ReactionMessageIDTest(ZulipTestCase): Reacting to an invalid message id fails """ sender = 'hamlet@zulip.com' - message_id = -1 - result = self.client_post('/api/v1/reactions', {'message_id': message_id, 'emoji': 'smile'}, - **self.api_auth(sender)) - self.assert_json_error(result, "Bad value for 'message_id': " + str(message_id)) + result = self.client_put('/api/v1/messages/-1/emoji_reactions/smile', + **self.api_auth(sender)) + self.assertEquals(result.status_code, 404) def test_inaccessible_message_id(self): # type: () -> None @@ -111,8 +110,8 @@ class ReactionMessageIDTest(ZulipTestCase): self.assert_json_success(result) content = ujson.loads(result.content) pm_id = content['id'] - result = self.client_post('/api/v1/reactions', {'message_id': pm_id, 'emoji': 'smile'}, - **self.api_auth(reaction_sender)) + result = self.client_put('/api/v1/messages/%s/emoji_reactions/smile' % (pm_id,), + **self.api_auth(reaction_sender)) self.assert_json_error(result, "Invalid message(s)") class ReactionTest(ZulipTestCase): @@ -131,14 +130,13 @@ class ReactionTest(ZulipTestCase): **self.api_auth(pm_sender)) self.assert_json_success(pm) content = ujson.loads(pm.content) + pm_id = content['id'] - first = self.client_post('/api/v1/reactions', {'message_id': pm_id, - 'emoji': 'smile'}, - **self.api_auth(reaction_sender)) + first = self.client_put('/api/v1/messages/%s/emoji_reactions/smile' % (pm_id,), + **self.api_auth(reaction_sender)) self.assert_json_success(first) - second = self.client_post('/api/v1/reactions', {'message_id': pm_id, - 'emoji': 'smile'}, - **self.api_auth(reaction_sender)) + second = self.client_put('/api/v1/messages/%s/emoji_reactions/smile' % (pm_id,), + **self.api_auth(reaction_sender)) self.assert_json_error(second, "Reaction already exists") def test_remove_nonexisting_reaction(self): @@ -157,18 +155,15 @@ class ReactionTest(ZulipTestCase): self.assert_json_success(pm) content = ujson.loads(pm.content) pm_id = content['id'] - add = self.client_post('/api/v1/reactions', {'message_id': pm_id, - 'emoji': 'smile'}, - **self.api_auth(reaction_sender)) + add = self.client_put('/api/v1/messages/%s/emoji_reactions/smile' % (pm_id,), + **self.api_auth(reaction_sender)) self.assert_json_success(add) - first = self.client_delete('/api/v1/reactions', {'message_id': pm_id, - 'emoji': 'smile'}, + first = self.client_delete('/api/v1/messages/%s/emoji_reactions/smile' % (pm_id,), **self.api_auth(reaction_sender)) self.assert_json_success(first) - second = self.client_delete('/api/v1/reactions', {'message_id': pm_id, - 'emoji': 'smile'}, + second = self.client_delete('/api/v1/messages/%s/emoji_reactions/smile' % (pm_id,), **self.api_auth(reaction_sender)) self.assert_json_error(second, "Reaction does not exist") @@ -197,9 +192,8 @@ class ReactionEventTest(ZulipTestCase): events = [] # type: List[Dict[str, Any]] with tornado_redirected_to_list(events): - result = self.client_post('/api/v1/reactions', {'message_id': pm_id, - 'emoji': 'smile'}, - **self.api_auth(reaction_sender)) + result = self.client_put('/api/v1/messages/%s/emoji_reactions/smile' % (pm_id,), + **self.api_auth(reaction_sender)) self.assert_json_success(result) self.assertEqual(len(events), 1) @@ -234,15 +228,13 @@ class ReactionEventTest(ZulipTestCase): expected_recipient_emails = set([pm_sender, pm_recipient]) expected_recipient_ids = set([get_user_profile_by_email(email).id for email in expected_recipient_emails]) - add = self.client_post('/api/v1/reactions', {'message_id': pm_id, - 'emoji': 'smile'}, - **self.api_auth(reaction_sender)) + add = self.client_put('/api/v1/messages/%s/emoji_reactions/smile' % (pm_id,), + **self.api_auth(reaction_sender)) self.assert_json_success(add) events = [] # type: List[Dict[str, Any]] with tornado_redirected_to_list(events): - result = self.client_delete('/api/v1/reactions', {'message_id': pm_id, - 'emoji': 'smile'}, + result = self.client_delete('/api/v1/messages/%s/emoji_reactions/smile' % (pm_id,), **self.api_auth(reaction_sender)) self.assert_json_success(result) self.assertEqual(len(events), 1) diff --git a/zerver/views/reactions.py b/zerver/views/reactions.py index af4cbfd137..282706ae82 100644 --- a/zerver/views/reactions.py +++ b/zerver/views/reactions.py @@ -12,9 +12,8 @@ from zerver.lib.response import json_success from zerver.models import Reaction, UserProfile @has_request_variables -def add_reaction_backend(request, user_profile, emoji_name=REQ('emoji'), - message_id = REQ('message_id', converter=to_non_negative_int)): - # type: (HttpRequest, UserProfile, text_type, int) -> HttpResponse +def add_reaction_backend(request, user_profile, message_id, emoji_name): + # type: (HttpRequest, UserProfile, int, text_type) -> HttpResponse # access_message will throw a JsonableError exception if the user # cannot see the message (e.g. for messages to private streams). @@ -36,9 +35,8 @@ def add_reaction_backend(request, user_profile, emoji_name=REQ('emoji'), return json_success() @has_request_variables -def remove_reaction_backend(request, user_profile, emoji_name=REQ('emoji'), - message_id = REQ('message_id', converter=to_non_negative_int)): - # type: (HttpRequest, UserProfile, text_type, int) -> HttpResponse +def remove_reaction_backend(request, user_profile, message_id, emoji_name): + # type: (HttpRequest, UserProfile, int, text_type) -> HttpResponse # access_message will throw a JsonableError exception if the user # cannot see the message (e.g. for messages to private streams). diff --git a/zproject/urls.py b/zproject/urls.py index af64b5e338..192c4fe844 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -199,10 +199,11 @@ v1_api_and_json_patterns = [ {'POST': 'zerver.views.messages.update_message_flags'}), # reactions -> zerver.view.reactions - # POST adds a reaction to a message + # PUT adds a reaction to a message # DELETE removes a reaction from a message - url(r'^reactions$', rest_dispatch, - {'POST': 'zerver.views.reactions.add_reaction_backend', + url(r'^messages/(?P[0-9]+)/emoji_reactions/(?P[0-9a-zA-Z.\-_]+(? zerver.views.typing