diff --git a/frontend_tests/node_tests/search_suggestion.js b/frontend_tests/node_tests/search_suggestion.js index 83eb717d34..b9779e360c 100644 --- a/frontend_tests/node_tests/search_suggestion.js +++ b/frontend_tests/node_tests/search_suggestion.js @@ -36,7 +36,7 @@ init(); set_global('narrow', {}); -topic_data.populate_for_tests({}); +topic_data.reset(); (function test_basic_get_suggestions() { var query = 'fred'; @@ -575,25 +575,24 @@ init(); } }; - var recent_data = {}; - topic_data.populate_for_tests(recent_data); + topic_data.reset(); suggestions = search.get_suggestions('te'); expected = [ "te", ]; assert.deepEqual(suggestions.strings, expected); - recent_data[devel_id] = [ - {name: 'REXX'}, - ]; + topic_data.add_message({ + stream_id: devel_id, + topic_name: 'REXX', + }); - recent_data[office_id] = [ - {name: 'team'}, - {name: 'ignore'}, - {name: 'test'}, - ]; - - topic_data.populate_for_tests(recent_data); + _.each(['team', 'ignore', 'test'], function (topic_name) { + topic_data.add_message({ + stream_id: office_id, + topic_name: topic_name, + }); + }); suggestions = search.get_suggestions('te'); expected = [ @@ -678,7 +677,7 @@ init(); return; }; - topic_data.populate_for_tests({}); + topic_data.reset(); var suggestions = search.get_suggestions(query); @@ -698,7 +697,7 @@ init(); return; }; - topic_data.populate_for_tests({}); + topic_data.reset(); var query = 'stream:of'; var suggestions = search.get_suggestions(query); @@ -750,13 +749,7 @@ init(); people.add(alice); - topic_data.populate_for_tests({ - office: [ - {name: 'team'}, - {name: 'ignore'}, - {name: 'test'}, - ], - }); + topic_data.reset(); var suggestions = search.get_suggestions(query); @@ -882,7 +875,7 @@ init(); return; }; - topic_data.populate_for_tests({}); + topic_data.reset(); // test allowing spaces with quotes surrounding operand var query = 'stream:"dev he"'; diff --git a/frontend_tests/node_tests/stream_data.js b/frontend_tests/node_tests/stream_data.js index ddbf382f15..cafa98b2ce 100644 --- a/frontend_tests/node_tests/stream_data.js +++ b/frontend_tests/node_tests/stream_data.js @@ -252,37 +252,53 @@ var people = global.people; stream_data.add_sub('Rome', rome); - var message = { + var message1 = { stream_id: stream_id, - timestamp: 101, + id: 101, subject: 'toPic1', }; - topic_data.process_message(message); + topic_data.process_message(message1); var history = topic_data.get_recent_names(rome.stream_id); assert.deepEqual(history, ['toPic1']); - message = { + var message2 = { stream_id: stream_id, - timestamp: 102, + id: 102, subject: 'Topic1', }; - topic_data.process_message(message); + topic_data.process_message(message2); history = topic_data.get_recent_names(rome.stream_id); assert.deepEqual(history, ['Topic1']); - message = { + var message3 = { stream_id: stream_id, - timestamp: 103, + id: 103, subject: 'topic2', }; - topic_data.process_message(message); + topic_data.process_message(message3); history = topic_data.get_recent_names(rome.stream_id); assert.deepEqual(history, ['topic2', 'Topic1']); - topic_data.process_message(message, true); + // Removing first topic1 message has no effect. + topic_data.process_message(message1, true); history = topic_data.get_recent_names(rome.stream_id); - assert.deepEqual(history, ['Topic1']); + assert.deepEqual(history, ['topic2', 'Topic1']); + + // Remove second topic1 message removes the topic. + topic_data.process_message(message2, true); + history = topic_data.get_recent_names(rome.stream_id); + assert.deepEqual(history, ['topic2']); + + // Test that duplicate remove does not crash us. + topic_data.process_message(message2, true); + history = topic_data.get_recent_names(rome.stream_id); + assert.deepEqual(history, ['topic2']); + + // get to 100% coverage for defensive code + topic_data.remove_message({ + stream_id: 9999999, + }); }()); (function test_is_active() { @@ -306,7 +322,7 @@ var people = global.people; var message = { stream_id: 222, - timestamp: 108, + id: 108, subject: 'topic2', }; topic_data.process_message(message); diff --git a/frontend_tests/node_tests/topic_list.js b/frontend_tests/node_tests/topic_list.js index 84ef81139c..51ef658ed1 100644 --- a/frontend_tests/node_tests/topic_list.js +++ b/frontend_tests/node_tests/topic_list.js @@ -24,11 +24,13 @@ global.compile_template('topic_list_item'); var active_topic = "testing"; var max_topics = 5; - var recent_topics = {}; - recent_topics[stream_id] = [ - {name: "coding"}, - ]; - topic_data.populate_for_tests(recent_topics); + topic_data.reset(); + topic_data.add_message({ + stream_id: stream_id, + topic_name: 'coding', + message_id: 400, + }); + global.unread.num_unread_for_subject = function () { return 1; }; diff --git a/static/js/topic_data.js b/static/js/topic_data.js index a95fab658a..9ab62649cf 100644 --- a/static/js/topic_data.js +++ b/static/js/topic_data.js @@ -8,58 +8,136 @@ exports.stream_has_topics = function (stream_id) { return stream_dict.has(stream_id); }; -exports.process_message = function (message, remove_message) { - var current_timestamp = 0; - var count = 0; - var stream_id = message.stream_id; - var canon_topic = message.subject.toLowerCase(); +exports.topic_history = function () { + var topics = new Dict({fold_case: true}); - var recents = stream_dict.get(stream_id) || []; + var self = {}; - recents = _.filter(recents, function (item) { - var is_duplicate = ( - item.name.toLowerCase() === canon_topic); - if (is_duplicate) { - current_timestamp = item.timestamp; - count = item.count; + self.add_or_update = function (opts) { + var name = opts.name; + var message_id = opts.message_id || 0; + + message_id = parseInt(message_id, 10); + + var existing = topics.get(name); + + if (!existing) { + topics.set(opts.name, { + message_id: message_id, + pretty_name: name, + count: 1, + }); + return; } - return !is_duplicate; - }); - if (remove_message !== undefined) { - count = count - 1; - } else { - count = count + 1; + existing.count += 1; + if (message_id > existing.message_id) { + existing.message_id = message_id; + existing.pretty_name = name; + } + }; + + self.maybe_remove = function (topic_name) { + var existing = topics.get(topic_name); + + if (!existing) { + return; + } + + if (existing.count <= 1) { + topics.del(topic_name); + return; + } + + existing.count -= 1; + }; + + self.get_recent_names = function () { + var recents = topics.values(); + + recents.sort(function (a, b) { + return b.message_id - a.message_id; + }); + + var names = _.map(recents, function (obj) { + return obj.pretty_name; + }); + + return names; + }; + + return self; +}; + +exports.process_message = function (message, remove_message) { + if (remove_message) { + exports.remove_message({ + stream_id: message.stream_id, + topic_name: message.subject, + }); + return; } - if (count !== 0) { - recents.push({name: message.subject, - count: count, - timestamp: Math.max(message.timestamp, current_timestamp)}); + exports.add_message({ + stream_id: message.stream_id, + topic_name: message.subject, + message_id: message.id, + }); +}; + +exports.remove_message = function (opts) { + var stream_id = opts.stream_id; + var name = opts.topic_name; + var history = stream_dict.get(stream_id); + + // This is the special case of "removing" a message from + // a topic, which happens when we edit topics. + + if (!history) { + return; } - recents.sort(function (a, b) { - return b.timestamp - a.timestamp; - }); + // This is the normal case of an incoming message. + history.maybe_remove(name); +}; - stream_dict.set(stream_id, recents); +exports.find_or_create = function (stream_id) { + var history = stream_dict.get(stream_id); + + if (!history) { + history = exports.topic_history(); + stream_dict.set(stream_id, history); + } + + return history; +}; + +exports.add_message = function (opts) { + var stream_id = opts.stream_id; + var message_id = opts.message_id; + var name = opts.topic_name; + + var history = exports.find_or_create(stream_id); + + history.add_or_update({ + name: name, + message_id: message_id, + }); }; exports.get_recent_names = function (stream_id) { - var topic_objs = stream_dict.get(stream_id); + var history = stream_dict.get(stream_id); - if (!topic_objs) { + if (!history) { return []; } - return _.map(topic_objs, function (obj) { - return obj.name; - }); + return history.get_recent_names(); }; -exports.populate_for_tests = function (stream_map) { +exports.reset = function () { // This is only used by tests. - stream_dict = Dict.from(stream_map); + stream_dict = new Dict(); }; return exports;