Skip to content

Commit

Permalink
recent: Sort PM recipients by their last sent message id.
Browse files Browse the repository at this point in the history
  • Loading branch information
amanagr authored and timabbott committed Dec 6, 2022
1 parent 28c73c6 commit 8a0344f
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 4 deletions.
1 change: 1 addition & 0 deletions frontend_tests/node_tests/message_store.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ mock_esm("../../static/js/stream_topic_history", {

mock_esm("../../static/js/recent_senders", {
process_stream_message: noop,
process_private_message: noop,
});

set_global("document", "document-stub");
Expand Down
38 changes: 38 additions & 0 deletions frontend_tests/node_tests/recent_senders.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ function make_stream_message({stream_id, topic, sender_id}) {
mock_esm("../../static/js/message_store", {
get: (message_id) => messages.get(message_id),
});
mock_esm("../../static/js/people", {
my_current_user_id: () => 1,
});

const rs = zrequire("recent_senders");
zrequire("message_util.js");
Expand Down Expand Up @@ -310,3 +313,38 @@ test("process_stream_message", () => {
true,
);
});

test("process_pms", () => {
const sender1 = 1; // Current user id
const sender2 = 2;
const sender3 = 3;

const user_ids_string = "2,3,4";
rs.process_private_message({
to_user_ids: user_ids_string,
sender_id: sender2,
id: 1,
});
rs.process_private_message({
to_user_ids: user_ids_string,
sender_id: sender3,
id: 2,
});
rs.process_private_message({
to_user_ids: user_ids_string,
sender_id: sender1,
id: 3,
});

// Recent topics displays avatars in the opposite order to this since
// that was simpler to implement in HTML.
assert.deepEqual(rs.get_pm_recent_senders(user_ids_string), {
participants: [1, 3, 2],
non_participants: [4],
});
// PM doesn't exist.
assert.deepEqual(rs.get_pm_recent_senders("1000,2000"), {
participants: [],
non_participants: [],
});
});
1 change: 1 addition & 0 deletions static/js/message_helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export function process_new_message(message) {

pm_conversations.process_message(message);

recent_senders.process_private_message(message);
if (people.is_my_user_id(message.sender_id)) {
for (const recip of message.display_recipient) {
message_user_ids.add_user_id(recip.id);
Expand Down
39 changes: 39 additions & 0 deletions static/js/recent_senders.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import _ from "lodash";

import {FoldDict} from "./fold_dict";
import * as message_store from "./message_store";
import * as people from "./people";

// This class is only exported for unit testing purposes.
// If we find reuse opportunities, we should just put it into
Expand Down Expand Up @@ -46,6 +47,9 @@ const stream_senders = new Map();
// topic_senders[stream_id][topic_id][sender_id] = IdTracker
const topic_senders = new Map();

// pm_senders[user_ids_string][user_id] = IdTracker
const pm_senders = new Map();

export function clear_for_testing() {
stream_senders.clear();
topic_senders.clear();
Expand Down Expand Up @@ -211,3 +215,38 @@ export function get_topic_recent_senders(stream_id, topic) {
}
return recent_senders;
}

export function process_private_message({to_user_ids, sender_id, id}) {
const sender_dict = pm_senders.get(to_user_ids) || new Map();
const id_tracker = sender_dict.get(sender_id) || new IdTracker();
pm_senders.set(to_user_ids, sender_dict);
sender_dict.set(sender_id, id_tracker);
id_tracker.add(id);
}

export function get_pm_recent_senders(user_ids_string) {
const user_ids = user_ids_string.split(",").map((id) => Number.parseInt(id, 10));
const sender_dict = pm_senders.get(user_ids_string);
const pm_senders_info = {participants: [], non_participants: []};
if (!sender_dict) {
return pm_senders_info;
}

function compare_pm_user_ids_by_recency(user_id1, user_id2) {
const max_id1 = sender_dict.get(user_id1)?.max_id() || -1;
const max_id2 = sender_dict.get(user_id2)?.max_id() || -1;
return max_id2 - max_id1;
}

// Add current user to user_ids.
user_ids.push(people.my_current_user_id());
pm_senders_info.non_participants = user_ids.filter((user_id) => {
if (sender_dict.get(user_id)) {
pm_senders_info.participants.push(user_id);
return false;
}
return true;
});
pm_senders_info.participants.sort(compare_pm_user_ids_by_recency);
return pm_senders_info;
}
15 changes: 11 additions & 4 deletions static/js/recent_topics_ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,6 @@ function format_conversation(conversation_data) {
context.pm_url = last_msg.pm_with_url;
context.is_group = last_msg.display_recipient.length > 2;

// Display in most recent sender first order
all_senders = last_msg.display_recipient;
senders = all_senders.slice(-MAX_AVATAR).map((sender) => sender.id);

if (!context.is_group) {
const user_id = Number.parseInt(last_msg.to_user_ids, 10);
const user = people.get_by_user_id(user_id);
Expand All @@ -444,6 +440,17 @@ function format_conversation(conversation_data) {
}
}

// Since the css for displaying senders in reverse order is much simpler,
// we provide our handlebars with senders in opposite order.
// Display in most recent sender first order.
// To match the behavior for streams, we display the set of users who've actually
// participated, with the most recent participants first. It could make sense to
// display the other recipients on the PM conversation with different styling,
// but it's important to not destroy the information of "who's actually talked".
all_senders = recent_senders
.get_pm_recent_senders(context.user_ids_string)
.participants.reverse();
senders = all_senders.slice(-MAX_AVATAR);
// Collect extra senders fullname for tooltip.
extra_sender_ids = all_senders.slice(0, -MAX_AVATAR);
displayed_other_senders = extra_sender_ids
Expand Down

0 comments on commit 8a0344f

Please sign in to comment.