compose_box: Fix order of group pm recipient pills on focus.

Earlier when compose box was opened or narrowed to pm, the recipient
pills were sorted according to user email strings instead of their
full names which was inconsistent with compose box placeholder text,
recipient row and message header.

This commit fixes the behaviour by introducing a `sort_email`
function to sort emails according to their full names and display
sorted pills.

Fixes: zulip#27375.

Co-authored-by: richardshaju <richardshaju66@gmail.com>
This commit is contained in:
Pratik Chanda 2024-06-10 23:44:45 +05:30 committed by Tim Abbott
parent c180b25a68
commit 64a9f83473
7 changed files with 29 additions and 12 deletions

View File

@ -158,8 +158,8 @@ async function test_send_multirecipient_pm_from_cordelia_pm_narrow(page: Page):
await pm.click();
await page.waitForSelector("#compose-textarea", {visible: true});
const recipient_internal_emails = [
await common.get_internal_email_from_name(page, common.fullname.othello),
await common.get_internal_email_from_name(page, common.fullname.cordelia),
await common.get_internal_email_from_name(page, common.fullname.othello),
].join(",");
await common.pm_recipient.expect(page, recipient_internal_emails);
}

View File

@ -212,7 +212,7 @@ async function test_restore_private_message_draft_via_draft_overlay(page: Page):
page,
common.fullname.hamlet,
);
await common.pm_recipient.expect(page, `${hamlet_internal_email},${cordelia_internal_email}`);
await common.pm_recipient.expect(page, `${cordelia_internal_email},${hamlet_internal_email}`);
assert.strictEqual(
await common.get_text_from_selector(page, "title"),
"Cordelia, Lear's daughter, King Hamlet - Zulip Dev - Zulip",

View File

@ -354,9 +354,10 @@ export function restore_message(draft: LocalStorageDraft): ComposeArguments {
const recipient_emails = draft.private_message_recipient
.split(",")
.filter((email) => people.is_valid_email_for_compose(email));
const sorted_recipient_emails = people.sort_emails_by_username(recipient_emails);
return {
type: "private",
private_message_recipient: recipient_emails.join(","),
private_message_recipient: sorted_recipient_emails.join(","),
content: draft.content,
};
}

View File

@ -1211,6 +1211,8 @@ export function narrow_by_recipient(target_id, opts) {
opts = {then_select_id: target_id, ...opts};
// don't use message_lists.current as it won't work for muted messages or for out-of-narrow links
const message = message_store.get(target_id);
const emails = message.reply_to.split(",");
const reply_to = people.sort_emails_by_username(emails);
switch (message.type) {
case "private":
@ -1224,7 +1226,7 @@ export function narrow_by_recipient(target_id, opts) {
// in the new view.
unread_ops.notify_server_message_read(message);
}
show([{operator: "dm", operand: message.reply_to}], opts);
show([{operator: "dm", operand: reply_to.join(",")}], opts);
break;
case "stream":

View File

@ -179,6 +179,24 @@ export function update_email(user_id: number, new_email: string): void {
// still work correctly.
}
// A function that sorts Email according to the user's full name
export function sort_emails_by_username(emails: string[]): (string | undefined)[] {
const user_ids = emails.map((email) => get_user_id(email));
const name_email_dict = user_ids.map((user_id, index) => ({
name:
user_id !== undefined && people_by_user_id_dict.has(user_id)
? people_by_user_id_dict.get(user_id)?.full_name
: "?",
email: emails[index],
}));
const sorted_emails = name_email_dict
.sort((a, b) => util.strcmp(a.name!, b.name!))
.map(({email}) => email);
return sorted_emails;
}
export function get_visible_email(user: User): string {
if (user.delivery_email) {
return user.delivery_email;
@ -268,9 +286,7 @@ export function user_ids_string_to_emails_string(user_ids_string: string): strin
emails = emails.map((email) => email.toLowerCase());
emails.sort();
return emails.join(",");
return sort_emails_by_username(emails).join(",");
}
export function user_ids_string_to_ids_array(user_ids_string: string): number[] {
@ -465,9 +481,7 @@ export function pm_reply_to(message: Message): string | undefined {
return person.email;
});
emails.sort();
const reply_to = emails.join(",");
const reply_to = sort_emails_by_username(emails).join(",");
return reply_to;
}

View File

@ -920,7 +920,7 @@ test_people("message_methods", () => {
};
assert.equal(people.pm_with_url(message), "#narrow/dm/301,302-group");
assert.equal(people.pm_perma_link(message), "#narrow/dm/30,301,302-group");
assert.equal(people.pm_reply_to(message), "Athens@example.com,charles@example.com");
assert.equal(people.pm_reply_to(message), "charles@example.com,Athens@example.com");
assert.equal(people.small_avatar_url(message), "http://charles.com/foo.png");
message = {

View File

@ -48,7 +48,7 @@ run_test("blueslip", () => {
blueslip.expect("error", "Unknown user_id");
people.get_actual_name_from_user_id(9999);
blueslip.expect("error", "Unknown email for get_user_id");
blueslip.expect("error", "Unknown email for get_user_id", 2);
people.get_user_id(unknown_email);
blueslip.expect("warn", "No user_id provided");