From 6c003a7802da4ba9250e011ff5f35848c0fa943e Mon Sep 17 00:00:00 2001 From: Riken Shah Date: Mon, 26 Jul 2021 10:43:55 +0000 Subject: [PATCH] refactor: Move emoji details related code to `emoji.js`. As `reaction.js` and `user_status.js` has similar code to get emoji details, it makes more sense to extract this as a single function. --- frontend_tests/node_tests/emoji.js | 66 ++++++++++++++ frontend_tests/node_tests/reactions.js | 7 +- frontend_tests/node_tests/user_status.js | 104 ----------------------- static/js/reactions.js | 21 +---- static/js/user_status.js | 41 --------- static/shared/js/emoji.js | 30 +++++++ 6 files changed, 102 insertions(+), 167 deletions(-) diff --git a/frontend_tests/node_tests/emoji.js b/frontend_tests/node_tests/emoji.js index 446cf06c05..6a96e33c4e 100644 --- a/frontend_tests/node_tests/emoji.js +++ b/frontend_tests/node_tests/emoji.js @@ -47,3 +47,69 @@ run_test("get_emoji_* API", () => { assert.equal(emoji.get_realm_emoji_url("spain"), "/some/path/to/spain.png"); }); + +run_test("get_emoji_details_by_name", () => { + let emoji_name = "smile"; + + let result = emoji.get_emoji_details_by_name(emoji_name); + assert.deepEqual(result, { + emoji_name: "smile", + emoji_code: "1f642", + reaction_type: "unicode_emoji", + }); + + // Test adding an unicode_emoji. + emoji_name = "smile"; + + result = emoji.get_emoji_details_by_name(emoji_name); + assert.deepEqual(result, { + emoji_name: "smile", + reaction_type: "unicode_emoji", + emoji_code: "1f642", + }); + + // Test adding zulip emoji. + emoji_name = "zulip"; + + result = emoji.get_emoji_details_by_name(emoji_name); + assert.deepEqual(result, { + emoji_name: "zulip", + reaction_type: "zulip_extra_emoji", + emoji_code: "zulip", + url: "/static/generated/emoji/images/emoji/unicode/zulip.png", + }); + + // Test adding realm emoji. + emoji_name = "spain"; + + emoji_name = emoji.get_emoji_details_by_name(emoji_name); + assert.deepEqual(emoji_name, { + emoji_name: "spain", + reaction_type: "realm_emoji", + emoji_code: "101", + url: "/some/path/to/spain.png", + }); + + // Test sending without emoji name. + assert.throws( + () => { + emoji.get_emoji_details_by_name(); + }, + { + name: "Error", + message: "Emoji name must be passed.", + }, + ); + + // Test sending an unknown emoji. + emoji_name = "unknown-emoji"; + assert.throws( + () => { + emoji.get_emoji_details_by_name(emoji_name); + }, + { + name: "Error", + message: "Bad emoji name: unknown-emoji", + }, + ); +}); diff --git a/frontend_tests/node_tests/reactions.js b/frontend_tests/node_tests/reactions.js index d5903657cb..64c44ddf48 100644 --- a/frontend_tests/node_tests/reactions.js +++ b/frontend_tests/node_tests/reactions.js @@ -331,12 +331,15 @@ test("sending", ({override}) => { reaction_type: "zulip_extra_emoji", emoji_name: "zulip", emoji_code: "zulip", + url: "/static/generated/emoji/images/emoji/unicode/zulip.png", }); } emoji_name = "unknown-emoji"; // Test sending an emoji unknown to frontend. - blueslip.expect("warn", "Bad emoji name: " + emoji_name); - reactions.toggle_emoji_reaction(message.id, emoji_name); + assert.throws(() => reactions.toggle_emoji_reaction(message.id, emoji_name), { + name: "Error", + message: "Bad emoji name: unknown-emoji", + }); }); test("set_reaction_count", () => { diff --git a/frontend_tests/node_tests/user_status.js b/frontend_tests/node_tests/user_status.js index 5e0bb9de68..69bb65440c 100644 --- a/frontend_tests/node_tests/user_status.js +++ b/frontend_tests/node_tests/user_status.js @@ -5,7 +5,6 @@ const {strict: assert} = require("assert"); const {mock_esm, zrequire} = require("../zjsunit/namespace"); const {run_test} = require("../zjsunit/test"); const blueslip = require("../zjsunit/zblueslip"); -const {page_params} = require("../zjsunit/zpage_params"); const channel = mock_esm("../../static/js/channel"); @@ -39,109 +38,6 @@ function initialize() { user_status.initialize(params); } -run_test("get_extra_emoji_info", () => { - page_params.emojiset = "text"; - - let emoji_info = {}; - - emoji_info = user_status.get_emoji_info(emoji_info); - assert.deepEqual(emoji_info, {}); - - emoji_info = {emoji_name: "smile"}; - - emoji_info = user_status.get_emoji_info(emoji_info); - assert.deepEqual(emoji_info, { - emoji_name: "smile", - emoji_alt_code: true, - }); - - page_params.emojiset = "google"; - - // Test adding an unicode_emoji. - emoji_info = {emoji_name: "smile", emoji_code: "1f642", reaction_type: "unicode_emoji"}; - - emoji_info = user_status.get_emoji_info(emoji_info); - assert.deepEqual(emoji_info, { - emoji_name: "smile", - emoji_alt_code: false, - reaction_type: "unicode_emoji", - emoji_code: "1f642", - }); - - // Test adding an unicode_emoji's name only. - // It should fill in other details automatically. - emoji_info = {emoji_name: "smile"}; - - emoji_info = user_status.get_emoji_info(emoji_info); - assert.deepEqual(emoji_info, { - emoji_name: "smile", - emoji_alt_code: false, - reaction_type: "unicode_emoji", - emoji_code: "1f642", - }); - - // Test adding zulip emoji. - emoji_info = {emoji_name: "zulip", emoji_code: "zulip", reaction_type: "zulip_extra_emoji"}; - - emoji_info = user_status.get_emoji_info(emoji_info); - assert.deepEqual(emoji_info, { - emoji_name: "zulip", - emoji_alt_code: false, - reaction_type: "zulip_extra_emoji", - emoji_code: "zulip", - url: "/static/generated/emoji/images/emoji/unicode/zulip.png", - }); - - // Test adding zulip emoji's name only. - emoji_info = {emoji_name: "zulip"}; - - emoji_info = user_status.get_emoji_info(emoji_info); - assert.deepEqual(emoji_info, { - emoji_name: "zulip", - emoji_alt_code: false, - reaction_type: "zulip_extra_emoji", - emoji_code: "zulip", - url: "/static/generated/emoji/images/emoji/unicode/zulip.png", - }); - - // Test adding realm_emoji emoji. - emoji_info = { - emoji_name: "realm_emoji", - emoji_code: "991", - reaction_type: "realm_emoji", - }; - - emoji_info = user_status.get_emoji_info(emoji_info); - assert.deepEqual(emoji_info, { - emoji_name: "realm_emoji", - emoji_alt_code: false, - reaction_type: "realm_emoji", - emoji_code: "991", - url: "/url/for/991", - }); - - // Test adding only realm_emoji's name only. - // It should fill in other details automatically. - emoji_info = { - emoji_name: "realm_emoji", - }; - - emoji_info = user_status.get_emoji_info(emoji_info); - assert.deepEqual(emoji_info, { - emoji_name: "realm_emoji", - emoji_alt_code: false, - reaction_type: "realm_emoji", - emoji_code: "991", - url: "/url/for/991", - }); - - // Test sending an unknown emoji. - emoji_info = {emoji_name: "unknown-emoji"}; - blueslip.expect("warn", "Bad emoji name: " + emoji_info.emoji_name); - emoji_info = user_status.get_emoji_info(emoji_info); - assert.deepEqual(emoji_info, {}); -}); - run_test("basics", () => { initialize(); assert.ok(user_status.is_away(2)); diff --git a/static/js/reactions.js b/static/js/reactions.js index 5037890cb0..9ef054648f 100644 --- a/static/js/reactions.js +++ b/static/js/reactions.js @@ -103,27 +103,8 @@ export function toggle_emoji_reaction(message_id, emoji_name) { // clicking on a reaction and that is handled by `process_reaction_click()` // method. This codepath is to be used only where there is no chance of an // user interacting with a deactivated realm emoji like emoji picker. - const reaction_info = { - emoji_name, - }; - - if (emoji.active_realm_emojis.has(emoji_name)) { - if (emoji_name === "zulip") { - reaction_info.reaction_type = "zulip_extra_emoji"; - } else { - reaction_info.reaction_type = "realm_emoji"; - } - reaction_info.emoji_code = emoji.active_realm_emojis.get(emoji_name).id; - } else { - const codepoint = emoji.get_emoji_codepoint(emoji_name); - if (codepoint === undefined) { - blueslip.warn("Bad emoji name: " + emoji_name); - return; - } - reaction_info.reaction_type = "unicode_emoji"; - reaction_info.emoji_code = codepoint; - } + const reaction_info = emoji.get_emoji_details_by_name(emoji_name); update_ui_and_send_reaction_ajax(message_id, reaction_info); } diff --git a/static/js/user_status.js b/static/js/user_status.js index 525283640e..bd42f85b7e 100644 --- a/static/js/user_status.js +++ b/static/js/user_status.js @@ -1,8 +1,5 @@ -import * as emoji from "../shared/js/emoji"; - import * as blueslip from "./blueslip"; import * as channel from "./channel"; -import {page_params} from "./page_params"; const away_user_ids = new Set(); const user_info = new Map(); @@ -45,44 +42,6 @@ export function revoke_away(user_id) { away_user_ids.delete(user_id); } -// This function will add missing/extra parameters to the emoji info object, -// that would need by template to render an emoji. -export function get_emoji_info(emoji_info) { - // To call this function you must pass at least an emoji name. - if (!emoji_info || !emoji_info.emoji_name) { - return {}; - } - - const status_emoji_info = {...emoji_info}; - - status_emoji_info.emoji_alt_code = page_params.emojiset === "text"; - if (status_emoji_info.emoji_alt_code) { - return status_emoji_info; - } - - if (emoji.active_realm_emojis.has(emoji_info.emoji_name)) { - if (!emoji_info.reaction_type) { - if (emoji_info.emoji_name === "zulip") { - status_emoji_info.reaction_type = "zulip_extra_emoji"; - } else { - status_emoji_info.reaction_type = "realm_emoji"; - } - } - const more_emoji_info = emoji.active_realm_emojis.get(emoji_info.emoji_name); - status_emoji_info.emoji_code = emoji_info.emoji_code || more_emoji_info.id; - status_emoji_info.url = more_emoji_info.emoji_url; - } else { - const codepoint = emoji.get_emoji_codepoint(emoji_info.emoji_name); - if (codepoint === undefined) { - blueslip.warn("Bad emoji name: " + emoji_info.emoji_name); - return {}; - } - status_emoji_info.reaction_type = emoji_info.reaction_type || "unicode_emoji"; - status_emoji_info.emoji_code = emoji_info.emoji_code || codepoint; - } - return status_emoji_info; -} - export function is_away(user_id) { return away_user_ids.has(user_id); } diff --git a/static/shared/js/emoji.js b/static/shared/js/emoji.js index 4477459596..5d7ef58747 100644 --- a/static/shared/js/emoji.js +++ b/static/shared/js/emoji.js @@ -174,6 +174,36 @@ export function update_emojis(realm_emojis) { build_emoji_data(active_realm_emojis); } +// This function will provide required parameters that would +// need by template to render an emoji. +export function get_emoji_details_by_name(emoji_name) { + // To call this function you must pass an emoji name. + if (!emoji_name) { + throw new Error("Emoji name must be passed."); + } + + const emoji_info = {emoji_name}; + + if (active_realm_emojis.has(emoji_name)) { + if (emoji_name === "zulip") { + emoji_info.reaction_type = "zulip_extra_emoji"; + } else { + emoji_info.reaction_type = "realm_emoji"; + } + const emoji_code_info = active_realm_emojis.get(emoji_name); + emoji_info.emoji_code = emoji_code_info.id; + emoji_info.url = emoji_code_info.emoji_url; + } else { + const codepoint = get_emoji_codepoint(emoji_name); + if (codepoint === undefined) { + throw new Error("Bad emoji name: " + emoji_name); + } + emoji_info.reaction_type = "unicode_emoji"; + emoji_info.emoji_code = codepoint; + } + return emoji_info; +} + export function initialize(params) { emoji_codes = params.emoji_codes;