Skip to content

Commit

Permalink
left-sidebar: Use same logic as of Topics view to render PMs.
Browse files Browse the repository at this point in the history
Change the logic for rendering PM threads in PM section to
be in the same as that of topics view --
In default view, only recent 5 PM threads would be shown
and append the active conversation as the 6th one at last
if not present in those 5.

In PM section with unreads, a maximum of 8 conversations
would be shown and rest of them would be hidden behind
the 'more conversations' li-item, clicking on which takes
to the zoomedIn view of PM section where all the present
PM threads would be visible.

Co-authored-by: Tim Abbott <[email protected]>
  • Loading branch information
jai2201 and timabbott committed Apr 6, 2022
1 parent 3388eaf commit c6f2b9c
Show file tree
Hide file tree
Showing 2 changed files with 332 additions and 5 deletions.
260 changes: 260 additions & 0 deletions frontend_tests/node_tests/pm_list_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,46 @@ const me = {
user_id: 103,
full_name: "Me Myself",
};
const zoe = {
email: "[email protected]",
user_id: 104,
full_name: "Zoe",
};
const cardelio = {
email: "[email protected]",
user_id: 105,
full_name: "Cardelio",
};
const shiv = {
email: "[email protected]",
user_id: 106,
full_name: "Shiv",
};
const desdemona = {
email: "[email protected]",
user_id: 107,
full_name: "Desdemona",
};
const lago = {
email: "[email protected]",
user_id: 108,
full_name: "Lago",
};
const aaron = {
email: "[email protected]",
user_id: 109,
full_name: "Aaron",
};
const jai = {
email: "[email protected]",
user_id: 110,
full_name: "Jai",
};
const shivam = {
email: "[email protected]",
user_id: 111,
full_name: "Shivam",
};
const bot_test = {
email: "[email protected]",
user_id: 314,
Expand All @@ -44,6 +84,14 @@ const bot_test = {
people.add_active_user(alice);
people.add_active_user(bob);
people.add_active_user(me);
people.add_active_user(zoe);
people.add_active_user(cardelio);
people.add_active_user(shiv);
people.add_active_user(desdemona);
people.add_active_user(lago);
people.add_active_user(aaron);
people.add_active_user(jai);
people.add_active_user(shivam);
people.add_active_user(bot_test);
people.initialize_current_user(me.user_id);

Expand All @@ -55,6 +103,10 @@ function test(label, f) {
});
}

function get_list_info(zoomed) {
return pm_list_data.get_list_info(zoomed);
}

test("get_conversations", ({override}) => {
pm_conversations.recent.insert([alice.user_id, bob.user_id], 1);
pm_conversations.recent.insert([me.user_id], 2);
Expand Down Expand Up @@ -179,3 +231,211 @@ test("is_all_privates", () => {
narrow_state.set_current_filter(private_filter());
assert.equal(pm_list_data.is_all_privates(), true);
});

test("get_list_info", ({override}) => {
let list_info;
assert.equal(narrow_state.filter(), undefined);

// Initialize an empty list to start.
const empty_list_info = get_list_info();

assert.deepEqual(empty_list_info, {
conversations_to_be_shown: [],
more_conversations_unread_count: 0,
});

// TODO: We should just initialize a Filter object with `new
// Filter` rather than creating a mock.
function set_filter_result(emails) {
const active_filter = {
operands: (operand) => {
assert.equal(operand, "pm-with");
return emails;
},
};
narrow_state.set_current_filter(active_filter);
}
set_filter_result([]);
assert.equal(pm_list_data.get_active_user_ids_string(), undefined);

// Mock to arrange that each user has exactly 1 unread.
const num_unread_for_person = 1;
override(unread, "num_unread_for_person", () => num_unread_for_person);

// Initially, we append 2 conversations and check for the
// `conversations_to_be_shown` returned in list_info.
pm_conversations.recent.insert([alice.user_id, bob.user_id], 1);
pm_conversations.recent.insert([me.user_id], 2);

list_info = get_list_info(false);
const expected_list_info = [
{
is_active: false,
is_group: false,
is_zero: false,
recipients: "Me Myself",
status_emoji_info: {
emoji_code: 20,
},
unread: 1,
url: "#narrow/pm-with/103-me",
user_circle_class: "user_circle_empty",
user_ids_string: "103",
},
{
recipients: "Alice, Bob",
user_ids_string: "101,102",
unread: 1,
is_active: false,
url: "#narrow/pm-with/101,102-group",
user_circle_class: undefined,
status_emoji_info: undefined,
is_group: true,
is_zero: false,
},
];

assert.deepEqual(list_info, {
conversations_to_be_shown: expected_list_info,
more_conversations_unread_count: 0,
});

// Now, add additional converstaions until we exceed
// `max_conversations_to_show_with_unreads`.

pm_conversations.recent.insert([zoe.user_id], 3);
pm_conversations.recent.insert([cardelio.user_id], 4);
pm_conversations.recent.insert([zoe.user_id, cardelio.user_id], 5);
pm_conversations.recent.insert([shiv.user_id], 6);
pm_conversations.recent.insert([cardelio.user_id, shiv.user_id], 7);
pm_conversations.recent.insert([desdemona.user_id], 8);

// We've now added a total of 8 conversations, which is the value
// of `max_conversations_to_show_with_unreads` so even now, the
// number of unreads in `more conversations` li-item should be 0.
list_info = get_list_info(false);

assert.deepEqual(list_info.conversations_to_be_shown.length, 8);
assert.deepEqual(list_info.more_conversations_unread_count, 0);

// Verify just the ordering of the conversations with unreads.
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
[
"Desdemona",
"Cardelio, Shiv",
"Shiv",
"Cardelio, Zoe",
"Cardelio",
"Zoe",
"Me Myself",
"Alice, Bob",
],
);

// After adding one more conversation, there will be 10
// conversations, which exceeds
// `max_conversations_to_show_with_unreads`. Verify that the
// oldest conversations are not shown and their unreads counted in
// more_conversations_unread_count.

pm_conversations.recent.insert([lago.user_id], 9);
pm_conversations.recent.insert([zoe.user_id, lago.user_id], 10);
list_info = get_list_info(false);
assert.deepEqual(list_info.conversations_to_be_shown.length, 8);
assert.deepEqual(list_info.more_conversations_unread_count, 2);
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
[
"Lago, Zoe",
"Lago",
"Desdemona",
"Cardelio, Shiv",
"Shiv",
"Cardelio, Zoe",
"Cardelio",
"Zoe",
],
);

// If we are narrowed to an older topic, then that one gets
// included despite not being among the 8 most recent.

set_filter_result(["[email protected],[email protected]"]);
list_info = get_list_info(false);
assert.deepEqual(list_info.conversations_to_be_shown.length, 9);
assert.deepEqual(list_info.more_conversations_unread_count, 1);
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
[
"Lago, Zoe",
"Lago",
"Desdemona",
"Cardelio, Shiv",
"Shiv",
"Cardelio, Zoe",
"Cardelio",
"Zoe",
"Alice, Bob",
],
);

// Verify that if the list is zoomed, we'll include all 9
// conversations in the correct order.

list_info = get_list_info(true);
assert.deepEqual(list_info.conversations_to_be_shown.length, 10);
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
[
"Lago, Zoe",
"Lago",
"Desdemona",
"Cardelio, Shiv",
"Shiv",
"Cardelio, Zoe",
"Cardelio",
"Zoe",
"Me Myself",
"Alice, Bob",
],
);

// We now test some no unreads cases.
override(unread, "num_unread_for_person", () => 0);
pm_conversations.clear_for_testing();
pm_conversations.recent.insert([alice.user_id], 1);
pm_conversations.recent.insert([bob.user_id], 2);
pm_conversations.recent.insert([me.user_id], 3);
pm_conversations.recent.insert([zoe.user_id], 4);
pm_conversations.recent.insert([cardelio.user_id], 5);
pm_conversations.recent.insert([shiv.user_id], 6);
pm_conversations.recent.insert([alice.user_id, bob.user_id], 7);

// We have 7 conversations in total, but only most recent 5 are visible.
list_info = get_list_info(false);
assert.deepEqual(list_info.conversations_to_be_shown.length, 5);

// If we set the oldest conversation as active, it will be
// included as well as the 5 most recent.

set_filter_result(["[email protected]"]);
assert.equal(pm_list_data.get_active_user_ids_string(), "101");
list_info = get_list_info(false);
assert.deepEqual(list_info.conversations_to_be_shown.length, 6);
assert.deepEqual(list_info.conversations_to_be_shown[5], {
recipients: "Alice",
user_ids_string: "101",
unread: 0,
is_zero: true,
is_active: true,
url: "#narrow/pm-with/101-alice",
status_emoji_info: {emoji_code: 20},
user_circle_class: "user_circle_empty",
is_group: false,
});
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
["Alice, Bob", "Shiv", "Cardelio", "Zoe", "Me Myself", "Alice"],
);
});
77 changes: 72 additions & 5 deletions static/js/pm_list_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import * as pm_conversations from "./pm_conversations";
import * as unread from "./unread";
import * as user_status from "./user_status";

// Maximum number of conversation threads to show in default view.
const max_conversations_to_show = 5;

// Maximum number of conversation threads to show in default view with unreads.
const max_conversations_to_show_with_unreads = 8;

export function get_active_user_ids_string() {
const filter = narrow_state.filter();

Expand All @@ -25,17 +31,17 @@ export function get_active_user_ids_string() {
export function get_conversations() {
const private_messages = pm_conversations.recent.get();
const display_objects = [];

// The user_ids_string for the current view, if any.
const active_user_ids_string = get_active_user_ids_string();

for (const private_message_obj of private_messages) {
const user_ids_string = private_message_obj.user_ids_string;
for (const conversation of private_messages) {
const user_ids_string = conversation.user_ids_string;
const reply_to = people.user_ids_string_to_emails_string(user_ids_string);
const recipients_string = people.get_recipients(user_ids_string);

const num_unread = unread.num_unread_for_person(user_ids_string);

const is_group = user_ids_string.includes(",");

const is_active = user_ids_string === active_user_ids_string;

let user_circle_class;
Expand All @@ -47,8 +53,10 @@ export function get_conversations() {
const recipient_user_obj = people.get_by_user_id(user_id);

if (recipient_user_obj.is_bot) {
// Bots do not have status emoji, and are modeled as
// always present. We may want to use this space for a
// bot icon in the future.
user_circle_class = "user_circle_green";
// bots do not have status emoji
} else {
status_emoji_info = user_status.get_status_emoji(user_id);
}
Expand Down Expand Up @@ -80,3 +88,62 @@ export function is_all_privates() {

return filter.operands("is").includes("private");
}

// Designed to closely match topic_list_data.get_list_info().
export function get_list_info(zoomed) {
const conversations = get_conversations();

if (zoomed || conversations.length <= max_conversations_to_show) {
return {
conversations_to_be_shown: conversations,
more_conversations_unread_count: 0,
};
}

const conversations_to_be_shown = [];
let more_conversations_unread_count = 0;
for (const [idx, conversation] of conversations.entries()) {
function should_show_conversation(conversation) {
// We always show the active conversation; see the similar
// comment in topic_list_data.js.
if (conversation.is_active) {
return true;
}

// We don't need to filter muted users here, because
// pm_conversations.js takes care of this for us.

// We include the most recent max_conversations_to_show
// conversations, regardless of whether they have unread
// messages.
if (idx < max_conversations_to_show) {
return true;
}

// We include older conversations with unread messages up
// until max_conversations_to_show_with_unreads total
// topics have been included.
if (
conversation.unread > 0 &&
conversations_to_be_shown.length < max_conversations_to_show_with_unreads
) {
return true;
}

// Otherwise, this conversation should only be visible in
// the unzoomed view.
return false;
}

if (should_show_conversation(conversation)) {
conversations_to_be_shown.push(conversation);
} else {
more_conversations_unread_count += conversation.unread;
}
}

return {
conversations_to_be_shown,
more_conversations_unread_count,
};
}

0 comments on commit c6f2b9c

Please sign in to comment.