From fc1bd590b69ee17f3c1db21e082af7d35708889a Mon Sep 17 00:00:00 2001 From: evykassirer Date: Wed, 27 Dec 2023 20:02:26 -0800 Subject: [PATCH] reactions: Remove view namespace. This will make a migration to typescript easier, and was unnecessary extra complexity. --- web/src/reactions.js | 30 +++++----- web/tests/reactions.test.js | 110 +++++++++++++++++++----------------- 2 files changed, 72 insertions(+), 68 deletions(-) diff --git a/web/src/reactions.js b/web/src/reactions.js index 03fc00867b..729f386b3b 100644 --- a/web/src/reactions.js +++ b/web/src/reactions.js @@ -13,9 +13,7 @@ import * as people from "./people"; import * as spectators from "./spectators"; import {user_settings} from "./user_settings"; -export const view = { - waiting_for_server_request_ids: new Set(), -}; // function namespace +const waiting_for_server_request_ids = new Set(); export function get_local_reaction_id(reaction_info) { return [reaction_info.reaction_type, reaction_info.emoji_code].join(","); @@ -68,7 +66,7 @@ function update_ui_and_send_reaction_ajax(message_id, reaction_info) { // unique request ID combining the message ID and the local ID, // which identifies just which emoji to use. const reaction_request_id = [message_id, local_id].join(","); - if (view.waiting_for_server_request_ids.has(reaction_request_id)) { + if (waiting_for_server_request_ids.has(reaction_request_id)) { return; } @@ -82,10 +80,10 @@ function update_ui_and_send_reaction_ajax(message_id, reaction_info) { url: "/json/messages/" + message_id + "/reactions", data: reaction_info, success() { - view.waiting_for_server_request_ids.delete(reaction_request_id); + waiting_for_server_request_ids.delete(reaction_request_id); }, error(xhr) { - view.waiting_for_server_request_ids.delete(reaction_request_id); + waiting_for_server_request_ids.delete(reaction_request_id); if (xhr.readyState !== 0) { if ( xhr.responseJSON?.code === "REACTION_ALREADY_EXISTS" || @@ -100,7 +98,7 @@ function update_ui_and_send_reaction_ajax(message_id, reaction_info) { }, }; - view.waiting_for_server_request_ids.add(reaction_request_id); + waiting_for_server_request_ids.add(reaction_request_id); if (operation === "add") { channel.post(args); } else if (operation === "remove") { @@ -251,7 +249,7 @@ export function add_reaction(event) { if (clean_reaction_object) { clean_reaction_object.user_ids.push(user_id); update_user_fields(clean_reaction_object, message.clean_reactions); - view.update_existing_reaction(clean_reaction_object, message, user_id); + update_existing_reaction(clean_reaction_object, message, user_id); } else { clean_reaction_object = make_clean_reaction({ local_id, @@ -263,11 +261,11 @@ export function add_reaction(event) { message.clean_reactions.set(local_id, clean_reaction_object); update_user_fields(clean_reaction_object, message.clean_reactions); - view.insert_new_reaction(clean_reaction_object, message, user_id); + insert_new_reaction(clean_reaction_object, message, user_id); } } -view.update_existing_reaction = function (clean_reaction_object, message, acting_user_id) { +export function update_existing_reaction(clean_reaction_object, message, acting_user_id) { // Our caller ensures that this message already has a reaction // for this emoji and sets up our user_list. This function // simply updates the DOM. @@ -285,9 +283,9 @@ view.update_existing_reaction = function (clean_reaction_object, message, acting } update_vote_text_on_message(message); -}; +} -view.insert_new_reaction = function (clean_reaction_object, message, user_id) { +export function insert_new_reaction(clean_reaction_object, message, user_id) { // Our caller ensures we are the first user to react to this // message with this emoji. We then render the emoji/title/count // and insert it before the add button. @@ -323,7 +321,7 @@ view.insert_new_reaction = function (clean_reaction_object, message, user_id) { $new_reaction.insertBefore($reaction_button_element); update_vote_text_on_message(message); -}; +} export function remove_reaction(event) { const message_id = event.message_id; @@ -358,10 +356,10 @@ export function remove_reaction(event) { const should_display_reactors = check_should_display_reactors(message.clean_reactions); update_user_fields(clean_reaction_object, should_display_reactors); - view.remove_reaction(clean_reaction_object, message, user_id); + remove_reaction_from_view(clean_reaction_object, message, user_id); } -view.remove_reaction = function (clean_reaction_object, message, user_id) { +export function remove_reaction_from_view(clean_reaction_object, message, user_id) { const local_id = get_local_reaction_id(clean_reaction_object); const $reaction = find_reaction(message.id, local_id); const reaction_count = clean_reaction_object.user_ids.length; @@ -387,7 +385,7 @@ view.remove_reaction = function (clean_reaction_object, message, user_id) { } update_vote_text_on_message(message); -}; +} export function get_emojis_used_by_user_for_message_id(message_id) { const user_id = page_params.user_id; diff --git a/web/tests/reactions.test.js b/web/tests/reactions.test.js index 2c3e50c17d..e2913a670d 100644 --- a/web/tests/reactions.test.js +++ b/web/tests/reactions.test.js @@ -269,7 +269,7 @@ test("reactions from unknown users", () => { test("unknown realm emojis (add)", () => { assert.throws( () => - reactions.view.insert_new_reaction( + reactions.insert_new_reaction( { reaction_type: "realm_emoji", emoji_name: "false_emoji", @@ -288,7 +288,7 @@ test("unknown realm emojis (add)", () => { test("unknown realm emojis (insert)", () => { assert.throws( () => - reactions.view.insert_new_reaction( + reactions.insert_new_reaction( { reaction_type: "realm_emoji", emoji_name: "fake_emoji", @@ -560,10 +560,7 @@ test("emoji_reaction_title", ({override}) => { ); }); -test("add_reaction/remove_reaction", ({override}) => { - // This function tests reaction events by mocking out calls to - // the view. - +test("add_reaction/remove_reaction", ({override, override_rewire}) => { const message = { id: 2001, reactions: [], @@ -573,21 +570,21 @@ test("add_reaction/remove_reaction", ({override}) => { override(message_store, "get", () => message); - let view_calls = []; + let function_calls = []; - override(reactions.view, "insert_new_reaction", (clean_reaction_object, message, user_id) => { - view_calls.push({ + override_rewire(reactions, "insert_new_reaction", (clean_reaction_object, message, user_id) => { + function_calls.push({ name: "insert_new_reaction", clean_reaction_object, message, user_id, }); }); - override( - reactions.view, + override_rewire( + reactions, "update_existing_reaction", (clean_reaction_object, message, user_id) => { - view_calls.push({ + function_calls.push({ name: "update_existing_reaction", clean_reaction_object, message, @@ -595,16 +592,25 @@ test("add_reaction/remove_reaction", ({override}) => { }); }, ); - override(reactions.view, "remove_reaction", (clean_reaction_object, message, user_id) => { - view_calls.push({name: "remove_reaction", clean_reaction_object, message, user_id}); - }); + override_rewire( + reactions, + "remove_reaction_from_view", + (clean_reaction_object, message, user_id) => { + function_calls.push({ + name: "remove_reaction_from_view", + clean_reaction_object, + message, + user_id, + }); + }, + ); - function test_view_calls(test_params) { - view_calls = []; + function test_function_calls(test_params) { + function_calls = []; test_params.run_code(); - assert.deepEqual(view_calls, test_params.expected_view_calls); + assert.deepEqual(function_calls, test_params.expected_function_calls); assert.deepEqual( new Set(reactions.get_emojis_used_by_user_for_message_id(message.message_id)), new Set(test_params.alice_emojis), @@ -648,11 +654,11 @@ test("add_reaction/remove_reaction", ({override}) => { user_ids: [alice.user_id], vote_text: "translated: You", }; - test_view_calls({ + test_function_calls({ run_code() { reactions.add_reaction(alice_8ball_event); }, - expected_view_calls: [ + expected_function_calls: [ { name: "insert_new_reaction", clean_reaction_object: clean_reaction_object_alice, @@ -671,11 +677,11 @@ test("add_reaction/remove_reaction", ({override}) => { }); // Add redundant reaction. - test_view_calls({ + test_function_calls({ run_code() { reactions.add_reaction(alice_8ball_event); }, - expected_view_calls: [], + expected_function_calls: [], alice_emojis: ["8ball"], }); @@ -692,11 +698,11 @@ test("add_reaction/remove_reaction", ({override}) => { user_ids: [alice.user_id, bob.user_id], vote_text: "translated: You, Bob van Roberts", }; - test_view_calls({ + test_function_calls({ run_code() { reactions.add_reaction(bob_8ball_event); }, - expected_view_calls: [ + expected_function_calls: [ { name: "update_existing_reaction", clean_reaction_object: clean_reaction_object_bob, @@ -727,11 +733,11 @@ test("add_reaction/remove_reaction", ({override}) => { user_ids: [cali.user_id], vote_text: "Cali", }; - test_view_calls({ + test_function_calls({ run_code() { reactions.add_reaction(cali_airplane_event); }, - expected_view_calls: [ + expected_function_calls: [ { name: "insert_new_reaction", clean_reaction_object: clean_reaction_object_cali, @@ -750,13 +756,13 @@ test("add_reaction/remove_reaction", ({override}) => { alice_emojis: ["8ball"], }); - test_view_calls({ + test_function_calls({ run_code() { reactions.remove_reaction(bob_8ball_event); }, - expected_view_calls: [ + expected_function_calls: [ { - name: "remove_reaction", + name: "remove_reaction_from_view", clean_reaction_object: clean_reaction_object_alice, message: { clean_reactions: new Map( @@ -773,13 +779,13 @@ test("add_reaction/remove_reaction", ({override}) => { alice_emojis: ["8ball"], }); - test_view_calls({ + test_function_calls({ run_code() { reactions.remove_reaction(alice_8ball_event); }, - expected_view_calls: [ + expected_function_calls: [ { - name: "remove_reaction", + name: "remove_reaction_from_view", clean_reaction_object: { count: 0, class: "message_reaction", @@ -808,16 +814,16 @@ test("add_reaction/remove_reaction", ({override}) => { }); // Test redundant remove. - test_view_calls({ + test_function_calls({ run_code() { reactions.remove_reaction(alice_8ball_event); }, - expected_view_calls: [], + expected_function_calls: [], alice_emojis: [], }); }); -test("view.insert_new_reaction (me w/unicode emoji)", ({mock_template}) => { +test("insert_new_reaction (me w/unicode emoji)", ({mock_template}) => { const clean_reaction_object = { class: "message_reaction", count: 1, @@ -863,7 +869,7 @@ test("view.insert_new_reaction (me w/unicode emoji)", ({mock_template}) => { insert_called = true; }; - reactions.view.insert_new_reaction( + reactions.insert_new_reaction( clean_reaction_object, { id: message_id, @@ -881,7 +887,7 @@ test("view.insert_new_reaction (me w/unicode emoji)", ({mock_template}) => { assert.ok(insert_called); }); -test("view.insert_new_reaction (them w/zulip emoji)", ({mock_template}) => { +test("insert_new_reaction (them w/zulip emoji)", ({mock_template}) => { const clean_reaction_object = { class: "message_reaction", count: 1, @@ -929,7 +935,7 @@ test("view.insert_new_reaction (them w/zulip emoji)", ({mock_template}) => { insert_called = true; }; - reactions.view.insert_new_reaction( + reactions.insert_new_reaction( clean_reaction_object, { id: message_id, @@ -947,7 +953,7 @@ test("view.insert_new_reaction (them w/zulip emoji)", ({mock_template}) => { assert.ok(insert_called); }); -test("view.update_existing_reaction (me)", () => { +test("update_existing_reaction (me)", () => { const clean_reaction_object = { class: "message_reaction", count: 2, @@ -965,7 +971,7 @@ test("view.update_existing_reaction (me)", () => { const $reaction_count = $.create("reaction-count-stub"); $our_reaction.set_find_results(".message_reaction_count", $reaction_count); - reactions.view.update_existing_reaction( + reactions.update_existing_reaction( clean_reaction_object, { id: message_id, @@ -994,7 +1000,7 @@ test("view.update_existing_reaction (me)", () => { ); }); -test("view.update_existing_reaction (them)", () => { +test("update_existing_reaction (them)", () => { const clean_reaction_object = { class: "message_reaction", count: 4, @@ -1012,7 +1018,7 @@ test("view.update_existing_reaction (them)", () => { const $reaction_count = $.create("reaction-count-stub"); $our_reaction.set_find_results(".message_reaction_count", $reaction_count); - reactions.view.update_existing_reaction( + reactions.update_existing_reaction( clean_reaction_object, { id: message_id, @@ -1053,7 +1059,7 @@ test("view.update_existing_reaction (them)", () => { ); }); -test("view.remove_reaction (me)", () => { +test("remove_reaction_from_view (me)", () => { const clean_reaction_object = { class: "message_reaction", count: 2, @@ -1072,7 +1078,7 @@ test("view.remove_reaction (me)", () => { const $reaction_button = $.create("reaction-button-stub"); $message_reactions.find = () => $reaction_button; - reactions.view.remove_reaction( + reactions.remove_reaction_from_view( clean_reaction_object, { id: message_id, @@ -1101,7 +1107,7 @@ test("view.remove_reaction (me)", () => { ); }); -test("view.remove_reaction (them)", () => { +test("remove_reaction_from_view (them)", () => { const clean_reaction_object = { class: "message_reaction", count: 1, @@ -1120,7 +1126,7 @@ test("view.remove_reaction (them)", () => { const $reaction_button = $.create("reaction-button-stub"); $message_reactions.find = () => $reaction_button; - reactions.view.remove_reaction( + reactions.remove_reaction_from_view( clean_reaction_object, { id: message_id, @@ -1143,7 +1149,7 @@ test("view.remove_reaction (them)", () => { ); }); -test("view.remove_reaction (last person)", () => { +test("remove_reaction_from_view (last person)", () => { const clean_reaction_object = { class: "message_reaction", count: 1, @@ -1163,7 +1169,7 @@ test("view.remove_reaction (last person)", () => { $our_reaction.remove = () => { removed = true; }; - reactions.view.remove_reaction( + reactions.remove_reaction_from_view( clean_reaction_object, {id: message_id, reactions: []}, bob.user_id, @@ -1208,11 +1214,11 @@ test("remove spurious user", ({override}) => { reactions.remove_reaction(event); }); -test("remove last user", ({override}) => { +test("remove last user", ({override, override_rewire}) => { const message = {...sample_message}; override(message_store, "get", () => message); - override(reactions.view, "remove_reaction", noop); + override_rewire(reactions, "remove_reaction_from_view", noop); function assert_names(names) { assert.deepEqual( @@ -1244,8 +1250,8 @@ test("local_reaction_id", () => { assert.equal(local_id, "unicode_emoji,1f44d"); }); -test("process_reaction_click", ({override}) => { - override(reactions.view, "remove_reaction", noop); +test("process_reaction_click", ({override, override_rewire}) => { + override_rewire(reactions, "remove_reaction_from_view", noop); const message = {...sample_message}; override(message_store, "get", () => message);