From b4eddad9a549b60765f1ce6d9fe6410bd667bb06 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Tue, 25 Oct 2022 18:08:45 +0530 Subject: [PATCH] people: Use names in PM urls instead of emails. We change the "pm-with" and "sender" narrow urls to be of "{user-id}-{encoded_name}" form instead of using email. This change improves performance of changing between PM views since parseOneAddress function was slow and we remove its usage now by using name instead of email. The name is encoded such that the characters that would be encoded by the browsers are replaced by "-". --- frontend_tests/node_tests/activity.js | 6 +++--- frontend_tests/node_tests/buddy_data.js | 6 +++--- frontend_tests/node_tests/filter.js | 6 ++---- frontend_tests/node_tests/hash_util.js | 4 ++-- frontend_tests/node_tests/hashchange.js | 4 ++-- frontend_tests/node_tests/narrow.js | 4 ++-- frontend_tests/node_tests/people.js | 6 +++--- frontend_tests/node_tests/pm_list_data.js | 8 ++++---- frontend_tests/node_tests/popovers.js | 4 ++-- static/js/people.js | 8 ++++---- 10 files changed, 27 insertions(+), 29 deletions(-) diff --git a/frontend_tests/node_tests/activity.js b/frontend_tests/node_tests/activity.js index e6f57a548b..0fbd01b5c6 100644 --- a/frontend_tests/node_tests/activity.js +++ b/frontend_tests/node_tests/activity.js @@ -383,7 +383,7 @@ test("first/prev/next", ({override, mock_template}) => { rendered_alice = true; assert.deepEqual(data, { faded: true, - href: "#narrow/pm-with/1-alice", + href: "#narrow/pm-with/1-Alice-Smith", is_current_user: false, name: "Alice Smith", num_unread: 0, @@ -401,7 +401,7 @@ test("first/prev/next", ({override, mock_template}) => { case fred.user_id: rendered_fred = true; assert.deepEqual(data, { - href: "#narrow/pm-with/2-fred", + href: "#narrow/pm-with/2-Fred-Flintstone", name: "Fred Flintstone", user_id: fred.user_id, is_current_user: false, @@ -449,7 +449,7 @@ test("insert_one_user_into_empty_list", ({override, mock_template}) => { user_settings.user_list_style = 2; mock_template("presence_row.hbs", true, (data, html) => { assert.deepEqual(data, { - href: "#narrow/pm-with/1-alice", + href: "#narrow/pm-with/1-Alice-Smith", name: "Alice Smith", user_id: 1, is_current_user: false, diff --git a/frontend_tests/node_tests/buddy_data.js b/frontend_tests/node_tests/buddy_data.js index 74b89666a9..cb2073db7f 100644 --- a/frontend_tests/node_tests/buddy_data.js +++ b/frontend_tests/node_tests/buddy_data.js @@ -508,7 +508,7 @@ test("get_items_for_users", () => { assert.deepEqual(buddy_data.get_items_for_users(user_ids), [ { faded: false, - href: "#narrow/pm-with/1001-self", + href: "#narrow/pm-with/1001-Human-Myself", is_current_user: true, name: "Human Myself", num_unread: 0, @@ -520,7 +520,7 @@ test("get_items_for_users", () => { }, { faded: false, - href: "#narrow/pm-with/1002-alice", + href: "#narrow/pm-with/1002-Alice-Smith", is_current_user: false, name: "Alice Smith", num_unread: 0, @@ -532,7 +532,7 @@ test("get_items_for_users", () => { }, { faded: false, - href: "#narrow/pm-with/1003-fred", + href: "#narrow/pm-with/1003-Fred-Flintstone", is_current_user: false, name: "Fred Flintstone", num_unread: 0, diff --git a/frontend_tests/node_tests/filter.js b/frontend_tests/node_tests/filter.js index 70f4b5eb15..82ebe95bbb 100644 --- a/frontend_tests/node_tests/filter.js +++ b/frontend_tests/node_tests/filter.js @@ -1628,14 +1628,12 @@ test("navbar_helpers", () => { const redirect_edge_cases = [ { operator: sender_me, - redirect_url_with_search: - "/#narrow/sender/" + me.user_id + "-" + parseOneAddress(me.email).local, + redirect_url_with_search: "/#narrow/sender/" + me.user_id + "-Me-Myself", is_common_narrow: false, }, { operator: sender_joe, - redirect_url_with_search: - "/#narrow/sender/" + joe.user_id + "-" + parseOneAddress(joe.email).local, + redirect_url_with_search: "/#narrow/sender/" + joe.user_id + "-joe", is_common_narrow: false, }, ]; diff --git a/frontend_tests/node_tests/hash_util.js b/frontend_tests/node_tests/hash_util.js index 807bb1da9f..a2cb7506df 100644 --- a/frontend_tests/node_tests/hash_util.js +++ b/frontend_tests/node_tests/hash_util.js @@ -37,7 +37,7 @@ run_test("hash_util", () => { let operator = "sender"; let operand = hamlet.email; - encode_decode_operand(operator, operand, "15-hamlet"); + encode_decode_operand(operator, operand, "15-Hamlet"); operator = "stream"; operand = "frontend"; @@ -185,7 +185,7 @@ run_test("test_search_public_streams_notice_url", () => { assert.equal( hash_util.search_public_streams_notice_url(get_operators("#narrow/sender/15")), - "#narrow/streams/public/sender/15-hamlet", + "#narrow/streams/public/sender/15-Hamlet", ); }); diff --git a/frontend_tests/node_tests/hashchange.js b/frontend_tests/node_tests/hashchange.js index ce17d5751c..1dbcbe891e 100644 --- a/frontend_tests/node_tests/hashchange.js +++ b/frontend_tests/node_tests/hashchange.js @@ -106,13 +106,13 @@ run_test("people_slugs", () => { people.add_active_user(alice); operators = [{operator: "sender", operand: "alice@example.com"}]; hash = hash_util.operators_to_hash(operators); - assert.equal(hash, "#narrow/sender/42-alice"); + assert.equal(hash, "#narrow/sender/42-Alice-Smith"); const narrow = hash_util.parse_narrow(hash.split("/")); assert.deepEqual(narrow, [{operator: "sender", operand: "alice@example.com", negated: false}]); operators = [{operator: "pm-with", operand: "alice@example.com"}]; hash = hash_util.operators_to_hash(operators); - assert.equal(hash, "#narrow/pm-with/42-alice"); + assert.equal(hash, "#narrow/pm-with/42-Alice-Smith"); }); function test_helper({override, change_tab}) { diff --git a/frontend_tests/node_tests/narrow.js b/frontend_tests/node_tests/narrow.js index 644b8af2c2..16f3a85ec9 100644 --- a/frontend_tests/node_tests/narrow.js +++ b/frontend_tests/node_tests/narrow.js @@ -177,13 +177,13 @@ run_test("uris", () => { people.initialize_current_user(me.user_id); let url = hash_util.pm_with_url(ray.email); - assert.equal(url, "#narrow/pm-with/22-ray"); + assert.equal(url, "#narrow/pm-with/22-Raymond"); url = hash_util.huddle_with_url("22,23"); assert.equal(url, "#narrow/pm-with/22,23-group"); url = hash_util.by_sender_url(ray.email); - assert.equal(url, "#narrow/sender/22-ray"); + assert.equal(url, "#narrow/sender/22-Raymond"); let emails = hash_util.decode_operand("pm-with", "22,23-group"); assert.equal(emails, "alice@example.com,ray@example.com"); diff --git a/frontend_tests/node_tests/people.js b/frontend_tests/node_tests/people.js index 8405231384..1edb9d6fc9 100644 --- a/frontend_tests/node_tests/people.js +++ b/frontend_tests/node_tests/people.js @@ -756,7 +756,7 @@ test_people("message_methods", () => { display_recipient: [{id: maria.user_id}, {id: me.user_id}], avatar_url: "legacy.png", }; - assert.equal(people.pm_with_url(message), "#narrow/pm-with/302-athens"); + assert.equal(people.pm_with_url(message), "#narrow/pm-with/302-Maria-Athens"); assert.equal(people.pm_perma_link(message), "#narrow/pm-with/30,302-pm"); assert.equal(people.pm_reply_to(message), "Athens@example.com"); assert.equal(people.small_avatar_url(message), "http://zulip.zulipdev.com/legacy.png?s=50"); @@ -793,7 +793,7 @@ test_people("message_methods", () => { type: "private", display_recipient: [{id: me.user_id}], }; - assert.equal(people.pm_with_url(message), "#narrow/pm-with/30-me"); + assert.equal(people.pm_with_url(message), "#narrow/pm-with/30-Me-Myself"); assert.equal(people.pm_perma_link(message), "#narrow/pm-with/30-pm"); message = {type: "stream"}; @@ -906,7 +906,7 @@ test_people("slugs", () => { people.add_active_user(debbie); const slug = people.emails_to_slug(debbie.email); - assert.equal(slug, "501-debbie71"); + assert.equal(slug, "501-Debra-Henton"); const email = people.slug_to_emails(slug); assert.equal(email, "debbie71@example.com"); diff --git a/frontend_tests/node_tests/pm_list_data.js b/frontend_tests/node_tests/pm_list_data.js index 66d6bd1e33..4b5d39d91d 100644 --- a/frontend_tests/node_tests/pm_list_data.js +++ b/frontend_tests/node_tests/pm_list_data.js @@ -121,7 +121,7 @@ test("get_conversations", ({override}) => { is_zero: false, recipients: "Me Myself", unread: 1, - url: "#narrow/pm-with/103-me", + url: "#narrow/pm-with/103-Me-Myself", user_circle_class: "user_circle_empty", user_ids_string: "103", status_emoji_info: { @@ -172,7 +172,7 @@ test("get_conversations bot", ({override}) => { unread: 1, is_zero: false, is_active: false, - url: "#narrow/pm-with/314-outgoingwebhook", + url: "#narrow/pm-with/314-Outgoing-webhook", status_emoji_info: undefined, user_circle_class: "user_circle_green", is_group: false, @@ -277,7 +277,7 @@ test("get_list_info", ({override}) => { emoji_code: 20, }, unread: 1, - url: "#narrow/pm-with/103-me", + url: "#narrow/pm-with/103-Me-Myself", user_circle_class: "user_circle_empty", user_ids_string: "103", }, @@ -428,7 +428,7 @@ test("get_list_info", ({override}) => { unread: 0, is_zero: true, is_active: true, - url: "#narrow/pm-with/101-alice", + url: "#narrow/pm-with/101-Alice", status_emoji_info: {emoji_code: 20}, user_circle_class: "user_circle_empty", is_group: false, diff --git a/frontend_tests/node_tests/popovers.js b/frontend_tests/node_tests/popovers.js index ba4a188801..f48dc39cf2 100644 --- a/frontend_tests/node_tests/popovers.js +++ b/frontend_tests/node_tests/popovers.js @@ -188,8 +188,8 @@ test_ui("sender_hover", ({override, mock_template}) => { user_circle_class: "user_circle_empty", user_last_seen_time_status: "translated: Last active: translated: More than 2 weeks ago", - pm_with_url: "#narrow/pm-with/42-alice", - sent_by_uri: "#narrow/sender/42-alice", + pm_with_url: "#narrow/pm-with/42-Alice-Smith", + sent_by_uri: "#narrow/sender/42-Alice-Smith", private_message_class: "respond_personal_button", show_email: false, show_manage_menu: true, diff --git a/static/js/people.js b/static/js/people.js index 4037767004..318a356265 100644 --- a/static/js/people.js +++ b/static/js/people.js @@ -1,6 +1,5 @@ import md5 from "blueimp-md5"; import {format, utcToZonedTime} from "date-fns-tz"; -import {parseOneAddress} from "email-addresses"; import * as typeahead from "../shared/js/typeahead"; @@ -528,8 +527,8 @@ export function pm_with_url(message) { suffix = "group"; } else { const person = get_by_user_id(user_ids[0]); - if (person && person.email) { - suffix = parseOneAddress(person.email).local.toLowerCase(); + if (person && person.full_name) { + suffix = person.full_name.replace(/[ "%/<>`\p{C}]+/gu, "-"); } else { blueslip.error("Unknown people in message"); suffix = "unk"; @@ -603,7 +602,8 @@ export function emails_to_slug(emails_string) { const emails = emails_string.split(","); if (emails.length === 1) { - slug += parseOneAddress(emails[0]).local.toLowerCase(); + const name = get_by_email(emails[0]).full_name; + slug += name.replace(/[ "%/<>`\p{C}]+/gu, "-"); } else { slug += "group"; }