diff --git a/frontend_tests/node_tests/dispatch.js b/frontend_tests/node_tests/dispatch.js index fcda618171..dbe687a146 100644 --- a/frontend_tests/node_tests/dispatch.js +++ b/frontend_tests/node_tests/dispatch.js @@ -190,6 +190,7 @@ const event_fixtures = { op: 'add', message_id: 128, emoji_name: 'anguished_pig', + user_id: "1", user: { id: "1", }, @@ -200,6 +201,7 @@ const event_fixtures = { op: 'remove', message_id: 256, emoji_name: 'angery', + user_id: "1", user: { id: "1", }, diff --git a/frontend_tests/node_tests/reactions.js b/frontend_tests/node_tests/reactions.js index 6b2efdf4a0..bc6255457e 100644 --- a/frontend_tests/node_tests/reactions.js +++ b/frontend_tests/node_tests/reactions.js @@ -74,15 +74,15 @@ people.add(cali); const message = { id: 1001, reactions: [ - {emoji_name: 'smile', user: {id: 5}, reaction_type: 'unicode_emoji', emoji_code: '263a'}, - {emoji_name: 'smile', user: {id: 6}, reaction_type: 'unicode_emoji', emoji_code: '263a'}, - {emoji_name: 'frown', user: {id: 7}, reaction_type: 'unicode_emoji', emoji_code: '1f641'}, - {emoji_name: 'inactive_realm_emoji', user: {id: 5}, reaction_type: 'realm_emoji', + {emoji_name: 'smile', user_id: 5, reaction_type: 'unicode_emoji', emoji_code: '263a'}, + {emoji_name: 'smile', user_id: 6, reaction_type: 'unicode_emoji', emoji_code: '263a'}, + {emoji_name: 'frown', user_id: 7, reaction_type: 'unicode_emoji', emoji_code: '1f641'}, + {emoji_name: 'inactive_realm_emoji', user_id: 5, reaction_type: 'realm_emoji', emoji_code: '992'}, // add some bogus user_ids - {emoji_name: 'octopus', user: {id: 8888}, reaction_type: 'unicode_emoji', emoji_code: '1f419'}, - {emoji_name: 'frown', user: {id: 9999}, reaction_type: 'unicode_emoji', emoji_code: '1f641'}, + {emoji_name: 'octopus', user_id: 8888, reaction_type: 'unicode_emoji', emoji_code: '1f419'}, + {emoji_name: 'frown', user_id: 9999, reaction_type: 'unicode_emoji', emoji_code: '1f641'}, ], }; @@ -298,9 +298,7 @@ run_test('add_and_remove_reaction', () => { reaction_type: 'unicode_emoji', emoji_name: '8ball', emoji_code: '1f3b1', - user: { - user_id: alice.user_id, - }, + user_id: alice.user_id, }; const message_reactions = $.create('our-reactions'); @@ -355,9 +353,7 @@ run_test('add_and_remove_reaction', () => { reaction_type: 'unicode_emoji', emoji_name: '8ball', emoji_code: '1f3b1', - user: { - user_id: bob.user_id, - }, + user_id: bob.user_id, }; const count_element = $.create('count-element'); @@ -402,9 +398,7 @@ run_test('add_and_remove_reaction', () => { reaction_type: 'realm_emoji', emoji_name: 'realm_emoji', emoji_code: '991', - user: { - user_id: cali.user_id, - }, + user_id: cali.user_id, }; template_called = false; @@ -430,9 +424,7 @@ run_test('add_and_remove_reaction', () => { reaction_type: 'realm_emoji', emoji_name: 'realm_emoji', emoji_code: '991', - user: { - user_id: alice.user_id, - }, + user_id: alice.user_id, }; message_reactions.find = function (selector) { @@ -495,9 +487,7 @@ run_test('with_view_stubs', () => { reaction_type: 'unicode_emoji', emoji_name: '8ball', emoji_code: '1f3b1', - user: { - user_id: alice.user_id, - }, + user_id: alice.user_id, }; const bob_8ball_event = { @@ -505,9 +495,7 @@ run_test('with_view_stubs', () => { reaction_type: 'unicode_emoji', emoji_name: '8ball', emoji_code: '1f3b1', - user: { - user_id: bob.user_id, - }, + user_id: bob.user_id, }; const cali_airplane_event = { @@ -515,9 +503,7 @@ run_test('with_view_stubs', () => { reaction_type: 'unicode_emoji', emoji_name: 'airplane', emoji_code: '2708', - user: { - user_id: cali.user_id, - }, + user_id: cali.user_id, }; test_view_calls({ @@ -627,9 +613,7 @@ run_test('error_handling', () => { reaction_type: 'realm_emoji', emoji_name: 'realm_emoji', emoji_code: '991', - user: { - user_id: 99, - }, + user_id: 99, }; const original_func = reactions.current_user_has_reacted_to_emoji; @@ -653,9 +637,7 @@ run_test('remove spurious user', () => { emoji_name: 'frown', emoji_code: '1f641', message_id: message.id, - user: { - user_id: alice.user_id, - }, + user_id: alice.user_id, }; reactions.remove_reaction(event); @@ -676,9 +658,7 @@ run_test('remove last user', () => { emoji_name: 'frown', emoji_code: '1f641', message_id: message.id, - user: { - user_id: cali.user_id, - }, + user_id: cali.user_id, }; reactions.remove_reaction(event); @@ -745,8 +725,7 @@ run_test('code coverage', () => { }; reactions.remove_reaction({ - message_id: 42, - user: {}, + message_id: 42, // TODO: REACTIONS API }); }); @@ -754,8 +733,8 @@ run_test('duplicates', () => { const dup_reaction_message = { id: 1001, reactions: [ - {emoji_name: 'smile', user: {id: 5}, reaction_type: 'unicode_emoji', emoji_code: '263a'}, - {emoji_name: 'smile', user: {id: 5}, reaction_type: 'unicode_emoji', emoji_code: '263a'}, + {emoji_name: 'smile', user_id: 5, reaction_type: 'unicode_emoji', emoji_code: '263a'}, + {emoji_name: 'smile', user_id: 5, reaction_type: 'unicode_emoji', emoji_code: '263a'}, ], }; diff --git a/static/js/reactions.js b/static/js/reactions.js index 355740ce47..a576a5de2d 100644 --- a/static/js/reactions.js +++ b/static/js/reactions.js @@ -41,10 +41,7 @@ function get_message(message_id) { function create_reaction(message_id, reaction_info) { return { message_id: message_id, - user: { - user_id: page_params.user_id, - id: page_params.user_id, - }, + user_id: page_params.user_id, local_id: exports.get_local_reaction_id(reaction_info), reaction_type: reaction_info.reaction_type, emoji_name: reaction_info.emoji_name, @@ -218,7 +215,7 @@ exports.add_reaction = function (event) { exports.set_clean_reactions(message); const local_id = exports.get_local_reaction_id(event); - const user_id = event.user.user_id; + const user_id = event.user_id; const r = message.clean_reactions.get(local_id); @@ -326,7 +323,7 @@ exports.remove_reaction = function (event) { const emoji_name = event.emoji_name; const emoji_code = event.emoji_code; const message_id = event.message_id; - const user_id = event.user.user_id; + const user_id = event.user_id; const message = message_store.get(message_id); const local_id = exports.get_local_reaction_id(event); @@ -441,7 +438,7 @@ exports.set_clean_reactions = function (message) { for (const reaction of message.reactions) { const local_id = exports.get_local_reaction_id(reaction); - const user_id = reaction.user.id; + const user_id = reaction.user_id; if (!people.is_known_user_id(user_id)) { blueslip.warn('Unknown user_id ' + user_id + diff --git a/templates/zerver/api/get-messages.md b/templates/zerver/api/get-messages.md index dd4ce2f55c..0184a2e10e 100644 --- a/templates/zerver/api/get-messages.md +++ b/templates/zerver/api/get-messages.md @@ -103,6 +103,20 @@ present in all Zulip API responses). displayed sorted by ID. * `is_me_message`: Whether the message is a [/me status message][status-messages] * `reactions`: Data on any reactions to the message. + * `emoji_code`: An encoded version of the emoji's unicode codepoint. + * `emoji_name`: Name of the emoji. + * `reaction_type`: If the reaction uses a [custom emoji](/help/add-custom-emoji), + `reaction_type` will be set to `realm_emoji`. + * `user_id`: The ID of the user who added the reaction. + **Changes**: New in Zulip 2.2. The `user` object is + deprecated and will be removed in the future. + * `user`: Dictionary with data on the user who added the reaction, including + the user ID as the `id` field. **Note**: In the [events + API](/api/get-events-from-queue), this `user` dictionary + confusing had the user ID in a field called `user_id` + instead. We recommend ignoring fields other than the user + ID. **Deprecated** and to be removed in a future release + once core clients have migrated to use the `user_id` field. * `recipient_id`: A unique ID for the set of users receiving the message (either a stream or group of users). Useful primarily for hashing. diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 9932c62c66..210fbabdda 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -1670,6 +1670,10 @@ def notify_reaction_update(user_profile: UserProfile, message: Message, event: Dict[str, Any] = { 'type': 'reaction', 'op': op, + 'user_id': user_profile.id, + # TODO: We plan to remove this redundant user_dict object once + # clients are updated to support accessing use user_id. See + # https://github.com/zulip/zulip/pull/14711 for details. 'user': user_dict, 'message_id': message.id, 'emoji_name': reaction.emoji_name, diff --git a/zerver/lib/message.py b/zerver/lib/message.py index 2d259c4ad9..eb237bdf75 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -556,9 +556,17 @@ class ReactionDict: return {'emoji_name': row['emoji_name'], 'emoji_code': row['emoji_code'], 'reaction_type': row['reaction_type'], + # TODO: We plan to remove this redundant user dictionary once + # clients are updated to support accessing use user_id. See + # https://github.com/zulip/zulip/pull/14711 for details. + # + # When we do that, we can likely update the `.values()` query to + # not fetch the extra user_profile__* fields from the database + # as a small performance optimization. 'user': {'email': row['user_profile__email'], 'id': row['user_profile__id'], - 'full_name': row['user_profile__full_name']}} + 'full_name': row['user_profile__full_name']}, + 'user_id': row['user_profile__id']} def access_message(user_profile: UserProfile, message_id: int) -> Tuple[Message, Optional[UserMessage]]: diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index e43420f6b1..cd6d1372d5 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -846,6 +846,7 @@ class EventsRegisterTest(ZulipTestCase): ('emoji_name', check_string), ('emoji_code', check_string), ('reaction_type', check_string), + ('user_id', check_int), ('user', check_dict_only([ ('email', check_string), ('full_name', check_string), @@ -871,6 +872,7 @@ class EventsRegisterTest(ZulipTestCase): ('emoji_name', check_string), ('emoji_code', check_string), ('reaction_type', check_string), + ('user_id', check_int), ('user', check_dict_only([ ('email', check_string), ('full_name', check_string), @@ -897,6 +899,7 @@ class EventsRegisterTest(ZulipTestCase): ('emoji_name', check_string), ('emoji_code', check_string), ('reaction_type', check_string), + ('user_id', check_int), ('user', check_dict_only([ ('email', check_string), ('full_name', check_string), @@ -951,6 +954,7 @@ class EventsRegisterTest(ZulipTestCase): ('emoji_name', check_string), ('emoji_code', check_string), ('reaction_type', check_string), + ('user_id', check_int), ('user', check_dict_only([ ('email', check_string), ('full_name', check_string), diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index 82466ab656..e42b5423a2 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -1527,6 +1527,7 @@ class MessageDictTest(ZulipTestCase): msg_dict = MessageDict.build_dict_from_raw_db_row(row) self.assertEqual(msg_dict['reactions'][0]['emoji_name'], reaction.emoji_name) + self.assertEqual(msg_dict['reactions'][0]['user_id'], sender.id) self.assertEqual(msg_dict['reactions'][0]['user']['id'], sender.id) self.assertEqual(msg_dict['reactions'][0]['user']['email'],