Skip to content

Commit

Permalink
unread: Indicate which topics have unread @-mentions.
Browse files Browse the repository at this point in the history
Fixes #21637.
  • Loading branch information
jai2201 committed Aug 23, 2022
1 parent a0f0a6e commit 068a654
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 4 deletions.
35 changes: 33 additions & 2 deletions frontend_tests/node_tests/topic_list_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ test("get_list_info w/real stream_topic_history", ({override}) => {

assert.deepEqual(empty_list_info, {
items: [],
more_topics_have_unread_mention_messages: false,
more_topics_unreads: 0,
num_possible_topics: 0,
});
Expand Down Expand Up @@ -77,9 +78,11 @@ test("get_list_info w/real stream_topic_history", ({override}) => {
list_info = get_list_info();
assert.equal(list_info.items.length, 5);
assert.equal(list_info.more_topics_unreads, 0);
assert.equal(list_info.more_topics_have_unread_mention_messages, false);
assert.equal(list_info.num_possible_topics, 7);

assert.deepEqual(list_info.items[0], {
contains_unread_mention: false,
is_active_topic: true,
is_muted: false,
is_zero: true,
Expand All @@ -91,6 +94,7 @@ test("get_list_info w/real stream_topic_history", ({override}) => {
});

assert.deepEqual(list_info.items[1], {
contains_unread_mention: false,
is_active_topic: false,
is_muted: false,
is_zero: true,
Expand All @@ -108,6 +112,7 @@ test("get_list_info w/real stream_topic_history", ({override}) => {
list_info = get_list_info(zoomed);
assert.equal(list_info.items.length, 7);
assert.equal(list_info.more_topics_unreads, 0);
assert.equal(list_info.more_topics_have_unread_mention_messages, false);
assert.equal(list_info.num_possible_topics, 7);

add_topic_message("After Brooklyn", 1008);
Expand All @@ -117,6 +122,7 @@ test("get_list_info w/real stream_topic_history", ({override}) => {
list_info = get_list_info(zoomed);
assert.equal(list_info.items.length, 2);
assert.equal(list_info.more_topics_unreads, 0);
assert.equal(list_info.more_topics_have_unread_mention_messages, false);
assert.equal(list_info.num_possible_topics, 2);
});

Expand Down Expand Up @@ -144,6 +150,20 @@ test("get_list_info unreads", ({override}) => {
);
}

function add_unreads_with_mention(topic, count) {
unread.process_loaded_messages(
Array.from({length: count}, () => ({
id: (message_id += 1),
stream_id: general.stream_id,
topic,
type: "stream",
unread: true,
mentioned: true,
mentioned_me_directly: true,
})),
);
}

/*
We have 15 topics, but we only show up
to 8 topics, depending on how many have
Expand All @@ -156,9 +176,18 @@ test("get_list_info unreads", ({override}) => {
add_unreads("topic 8", 8);
add_unreads("topic 9", 9);

/*
We added 9 unread messages in 'topic 9',
but now we would add a unread message
with `mention` for user, to test
`more_topics_have_unread_mention_messages`.
*/
add_unreads_with_mention("topic 9", 1);

list_info = get_list_info();
assert.equal(list_info.items.length, 7);
assert.equal(list_info.more_topics_unreads, 0);
assert.equal(list_info.more_topics_have_unread_mention_messages, false);
assert.equal(list_info.num_possible_topics, 15);

assert.deepEqual(
Expand All @@ -171,7 +200,8 @@ test("get_list_info unreads", ({override}) => {

list_info = get_list_info();
assert.equal(list_info.items.length, 8);
assert.equal(list_info.more_topics_unreads, 9);
assert.equal(list_info.more_topics_unreads, 10);
assert.equal(list_info.more_topics_have_unread_mention_messages, true);
assert.equal(list_info.num_possible_topics, 15);

assert.deepEqual(
Expand All @@ -190,7 +220,8 @@ test("get_list_info unreads", ({override}) => {

list_info = get_list_info();
assert.equal(list_info.items.length, 8);
assert.equal(list_info.more_topics_unreads, 9 + 13);
assert.equal(list_info.more_topics_unreads, 10 + 13);
assert.equal(list_info.more_topics_have_unread_mention_messages, true);
assert.equal(list_info.num_possible_topics, 15);

assert.deepEqual(
Expand Down
29 changes: 29 additions & 0 deletions frontend_tests/node_tests/unread.js
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,35 @@ test("mention updates", () => {
test_counted(true);
});

test("topics with unread mentions", () => {
const message_with_mention = {
id: 98,
type: "stream",
stream_id: 999,
topic: "topic with mention",
mentioned: true,
mentioned_me_directly: true,
unread: true,
};

const message_without_mention = {
id: 99,
type: "stream",
stream_id: 999,
topic: "topic without mention",
mentioned: false,
mentioned_me_directly: false,
unread: true,
};

unread.process_loaded_messages([message_with_mention, message_without_mention]);
assert.equal(unread.get_topics_with_unread_mentions(999).size, 1);
assert.deepEqual(unread.get_topics_with_unread_mentions(999), new Set(["topic with mention"]));
unread.mark_as_read(message_with_mention.id);
assert.equal(unread.get_topics_with_unread_mentions(999).size, 0);
assert.deepEqual(unread.get_topics_with_unread_mentions(999), new Set([]));
});

test("starring", () => {
// We don't need any setup here, because we just hard code
// this to [] in the code.
Expand Down
7 changes: 5 additions & 2 deletions static/js/topic_list.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,11 @@ export function keyed_topic_li(conversation) {
};
}

export function more_li(more_topics_unreads) {
export function more_li(more_topics_unreads, more_topics_have_unread_mention_messages) {
const render = () =>
render_more_topics({
more_topics_unreads,
more_topics_have_unread_mention_messages,
});

const eq = (other) => other.more_items && more_topics_unreads === other.more_topics_unreads;
Expand Down Expand Up @@ -142,6 +143,8 @@ export class TopicListWidget {

const num_possible_topics = list_info.num_possible_topics;
const more_topics_unreads = list_info.more_topics_unreads;
const more_topics_have_unread_mention_messages =
list_info.more_topics_have_unread_mention_messages;

const is_showing_all_possible_topics =
list_info.items.length === num_possible_topics &&
Expand All @@ -154,7 +157,7 @@ export class TopicListWidget {
if (spinner) {
nodes.push(spinner_li());
} else if (!is_showing_all_possible_topics) {
nodes.push(more_li(more_topics_unreads));
nodes.push(more_li(more_topics_unreads, more_topics_have_unread_mention_messages));
} else if (zoomed) {
// In the zoomed topic view, we need to add the input
// for filtering through list of topics.
Expand Down
10 changes: 10 additions & 0 deletions static/js/topic_list_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const max_topics_with_unread = 8;
export function get_list_info(stream_id, zoomed) {
let topics_selected = 0;
let more_topics_unreads = 0;
let more_topics_have_unread_mention_messages = false;

let active_topic = narrow_state.topic();

Expand All @@ -29,12 +30,16 @@ export function get_list_info(stream_id, zoomed) {

const items = [];

const topics_with_unread_mentions = unread.get_topics_with_unread_mentions(stream_id);

for (const [idx, topic_name] of topic_names.entries()) {
const num_unread = unread.num_unread_for_topic(stream_id, topic_name);
const is_active_topic = active_topic === topic_name.toLowerCase();
const is_topic_muted = user_topics.is_topic_muted(stream_id, topic_name);
const [topic_resolved_prefix, topic_display_name] =
resolved_topic.display_parts(topic_name);
// Important: Topics are lower-case in this set.
const contains_unread_mention = topics_with_unread_mentions.has(topic_name.toLowerCase());

if (!zoomed) {
function should_show_topic(topics_selected) {
Expand Down Expand Up @@ -85,6 +90,9 @@ export function get_list_info(stream_id, zoomed) {
// stream-level counts, only counts messages
// on unmuted topics.
more_topics_unreads += num_unread;
if (contains_unread_mention) {
more_topics_have_unread_mention_messages = true;
}
}
continue;
}
Expand All @@ -102,6 +110,7 @@ export function get_list_info(stream_id, zoomed) {
is_muted: is_topic_muted,
is_active_topic,
url: hash_util.by_stream_topic_url(stream_id, topic_name),
contains_unread_mention,
};

items.push(topic_info);
Expand All @@ -111,5 +120,6 @@ export function get_list_info(stream_id, zoomed) {
items,
num_possible_topics: topic_names.length,
more_topics_unreads,
more_topics_have_unread_mention_messages,
};
}
30 changes: 30 additions & 0 deletions static/js/unread.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,32 @@ class UnreadTopicCounter {

return id_set.size !== 0;
}

get_topics_with_unread_mentions(stream_id) {
// Returns the set of lower cased topics with unread mentions
// in the given stream.
const result = new Set();
const per_stream_bucketer = this.bucketer.get_bucket(stream_id);

if (!per_stream_bucketer) {
return result;
}

for (const message_id of unread_mentions_counter) {
// Because bucket keys in per_stream_bucketer are topics,
// we can just directly use reverse_lookup to find the
// topic in this stream containing a given unread message
// ID. If it's not in this stream, we'll get undefined.
const topic_match = per_stream_bucketer.reverse_lookup.get(message_id);
if (topic_match !== undefined) {
// Important: We lower-case topics here before adding them
// to this set, to support case-insensitive checks.
result.add(topic_match.toLowerCase());
}
}

return result;
}
}
const unread_topic_counter = new UnreadTopicCounter();

Expand Down Expand Up @@ -601,6 +627,10 @@ export function topic_has_any_unread(stream_id, topic) {
return unread_topic_counter.topic_has_any_unread(stream_id, topic);
}

export function get_topics_with_unread_mentions(stream_id) {
return unread_topic_counter.get_topics_with_unread_mentions(stream_id);
}

export function num_unread_for_person(user_ids_string) {
return unread_pm_counter.num_unread(user_ids_string);
}
Expand Down
5 changes: 5 additions & 0 deletions static/templates/more_topics.hbs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
<li class="topic-list-item show-more-topics bottom_left_row {{#unless more_topics_unreads}}zero-topic-unreads{{/unless}}">
<span class='topic-box'>
<a class="topic-name" tabindex="0">{{t "more topics" }}</a>
{{#if more_topics_have_unread_mention_messages}}
<span class="unread_mention_info">
@
</span>
{{/if}}
<span class="unread_count {{#unless more_topics_unreads}}zero_count{{/unless}}">
{{more_topics_unreads}}
</span>
Expand Down
5 changes: 5 additions & 0 deletions static/templates/topic_list_item.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
<a href='{{url}}' class="topic-name" title="{{topic_name}}">
{{topic_display_name}}
</a>
{{#if contains_unread_mention}}
<span class="unread_mention_info">
@
</span>
{{/if}}
<span class="unread_count {{#if is_zero}}zero_count{{/if}}">
{{unread}}
</span>
Expand Down

0 comments on commit 068a654

Please sign in to comment.