From 37c78abe14e23f4897822109f39b09dd1bf19c24 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sun, 23 Dec 2018 15:49:14 +0000 Subject: [PATCH] frontend: Use topic on message. This seems like a small change (apart from all the test changes), but it fundamentally changes how the app finds "topic" on message objects. Now all code that used to set "subject" now sets "topic" on message-like objects. We convert incoming messages to have topic, and we write to "topic" all the way up to hitting the server (which now accepts "topic" on incoming endpoints). We fall back to subject as needed, but the code will emit a warning that should be heeded--the "subject" field is prone to becoming stale for things like topic changes. --- frontend_tests/node_tests/compose.js | 4 +-- frontend_tests/node_tests/compose_actions.js | 8 ++--- frontend_tests/node_tests/filter.js | 28 +++++++-------- frontend_tests/node_tests/general.js | 8 ++--- frontend_tests/node_tests/hash_util.js | 2 +- frontend_tests/node_tests/markdown.js | 36 +++++++++---------- frontend_tests/node_tests/message_events.js | 1 + frontend_tests/node_tests/message_list.js | 3 ++ .../node_tests/message_list_data.js | 33 ++++++++--------- .../node_tests/message_list_view.js | 6 ++-- frontend_tests/node_tests/narrow.js | 2 +- frontend_tests/node_tests/narrow_activate.js | 1 + frontend_tests/node_tests/narrow_local.js | 26 +++++++------- frontend_tests/node_tests/narrow_unread.js | 2 +- frontend_tests/node_tests/notifications.js | 18 +++++----- frontend_tests/node_tests/recent_senders.js | 16 ++++----- frontend_tests/node_tests/topic_data.js | 8 ++--- frontend_tests/node_tests/transmit.js | 4 +-- frontend_tests/node_tests/typeahead_helper.js | 10 +++--- frontend_tests/node_tests/unread.js | 22 ++++++------ frontend_tests/node_tests/util.js | 13 ++++--- static/js/message_store.js | 4 +++ static/js/util.js | 9 ++++- 23 files changed, 142 insertions(+), 122 deletions(-) diff --git a/frontend_tests/node_tests/compose.js b/frontend_tests/node_tests/compose.js index 9fb0720f16..21b4aca258 100644 --- a/frontend_tests/node_tests/compose.js +++ b/frontend_tests/node_tests/compose.js @@ -606,7 +606,7 @@ run_test('send_message', () => { sender_id: 101, queue_id: undefined, stream: '', - subject: '', + topic: '', to: '["alice@example.com"]', reply_to: 'alice@example.com', private_message_recipient: 'alice@example.com', @@ -1620,7 +1620,7 @@ run_test('create_message_object', () => { var message = compose.create_message_object(); assert.equal(message.to, 'social'); - assert.equal(message.subject, 'lunch'); + assert.equal(message.topic, 'lunch'); assert.equal(message.content, 'burrito'); global.compose_state.get_message_type = function () { diff --git a/frontend_tests/node_tests/compose_actions.js b/frontend_tests/node_tests/compose_actions.js index 9f675a0f93..f2a2577306 100644 --- a/frontend_tests/node_tests/compose_actions.js +++ b/frontend_tests/node_tests/compose_actions.js @@ -197,7 +197,7 @@ run_test('respond_to_message', () => { msg = { type: 'stream', stream: 'devel', - subject: 'python', + topic: 'python', reply_to: 'bob', // compose.start needs this for dubious reasons }; stub_selected_message(msg); @@ -213,7 +213,7 @@ run_test('reply_with_mention', () => { var msg = { type: 'stream', stream: 'devel', - subject: 'python', + topic: 'python', reply_to: 'bob', // compose.start needs this for dubious reasons sender_full_name: 'Bob Roberts', sender_id: 40, @@ -257,7 +257,7 @@ run_test('quote_and_reply', () => { var msg = { type: 'stream', stream: 'devel', - subject: 'python', + topic: 'python', reply_to: 'bob', sender_full_name: 'Bob Roberts', sender_id: 40, @@ -288,7 +288,7 @@ run_test('quote_and_reply', () => { return { type: 'stream', stream: 'devel', - subject: 'test', + topic: 'test', reply_to: 'bob', sender_full_name: 'Bob', sender_id: 40, diff --git a/frontend_tests/node_tests/filter.js b/frontend_tests/node_tests/filter.js index 9b1049017b..2528036dc8 100644 --- a/frontend_tests/node_tests/filter.js +++ b/frontend_tests/node_tests/filter.js @@ -234,16 +234,16 @@ run_test('predicate_basics', () => { make_sub('Foo', stream_id); var predicate = get_predicate([['stream', 'Foo'], ['topic', 'Bar']]); - assert(predicate({type: 'stream', stream_id: stream_id, subject: 'bar'})); - assert(!predicate({type: 'stream', stream_id: stream_id, subject: 'whatever'})); + assert(predicate({type: 'stream', stream_id: stream_id, topic: 'bar'})); + assert(!predicate({type: 'stream', stream_id: stream_id, topic: 'whatever'})); assert(!predicate({type: 'stream', stream_id: 9999999})); assert(!predicate({type: 'private'})); // For old streams that we are no longer subscribed to, we may not have // a sub, but these should still match by stream name. predicate = get_predicate([['stream', 'old-Stream'], ['topic', 'Bar']]); - assert(predicate({type: 'stream', stream: 'Old-stream', subject: 'bar'})); - assert(!predicate({type: 'stream', stream: 'no-match', subject: 'whatever'})); + assert(predicate({type: 'stream', stream: 'Old-stream', topic: 'bar'})); + assert(!predicate({type: 'stream', stream: 'no-match', topic: 'whatever'})); predicate = get_predicate([['search', 'emoji']]); assert(predicate({})); @@ -290,8 +290,8 @@ run_test('predicate_basics', () => { assert(!predicate({id: 6})); predicate = get_predicate([['id', 5], ['topic', 'lunch']]); - assert(predicate({type: 'stream', id: 5, subject: 'lunch'})); - assert(!predicate({type: 'stream', id: 5, subject: 'dinner'})); + assert(predicate({type: 'stream', id: 5, topic: 'lunch'})); + assert(!predicate({type: 'stream', id: 5, topic: 'dinner'})); predicate = get_predicate([['sender', 'Joe@example.com']]); assert(predicate({sender_id: joe.user_id})); @@ -372,15 +372,15 @@ run_test('mit_exceptions', () => { global.page_params.realm_is_zephyr_mirror_realm = true; var predicate = get_predicate([['stream', 'Foo'], ['topic', 'personal']]); - assert(predicate({type: 'stream', stream: 'foo', subject: 'personal'})); - assert(predicate({type: 'stream', stream: 'foo.d', subject: 'personal'})); - assert(predicate({type: 'stream', stream: 'foo.d', subject: ''})); + assert(predicate({type: 'stream', stream: 'foo', topic: 'personal'})); + assert(predicate({type: 'stream', stream: 'foo.d', topic: 'personal'})); + assert(predicate({type: 'stream', stream: 'foo.d', topic: ''})); assert(!predicate({type: 'stream', stream: 'wrong'})); - assert(!predicate({type: 'stream', stream: 'foo', subject: 'whatever'})); + assert(!predicate({type: 'stream', stream: 'foo', topic: 'whatever'})); assert(!predicate({type: 'private'})); predicate = get_predicate([['stream', 'Foo'], ['topic', 'bar']]); - assert(predicate({type: 'stream', stream: 'foo', subject: 'bar.d'})); + assert(predicate({type: 'stream', stream: 'foo', topic: 'bar.d'})); // Try to get the MIT regex to explode for an empty stream. var terms = [ @@ -388,7 +388,7 @@ run_test('mit_exceptions', () => { {operator: 'topic', operand: 'bar'}, ]; predicate = new Filter(terms).predicate(); - assert(!predicate({type: 'stream', stream: 'foo', subject: 'bar'})); + assert(!predicate({type: 'stream', stream: 'foo', topic: 'bar'})); // Try to get the MIT regex to explode for an empty topic. terms = [ @@ -396,7 +396,7 @@ run_test('mit_exceptions', () => { {operator: 'topic', operand: ''}, ]; predicate = new Filter(terms).predicate(); - assert(!predicate({type: 'stream', stream: 'foo', subject: 'bar'})); + assert(!predicate({type: 'stream', stream: 'foo', topic: 'bar'})); }); run_test('predicate_edge_cases', () => { @@ -426,7 +426,7 @@ run_test('predicate_edge_cases', () => { var filter = new Filter(terms); filter.predicate(); predicate = filter.predicate(); // get cached version - assert(predicate({type: 'stream', stream: 'foo', subject: 'bar'})); + assert(predicate({type: 'stream', stream: 'foo', topic: 'bar'})); }); diff --git a/frontend_tests/node_tests/general.js b/frontend_tests/node_tests/general.js index 906527c736..8c7bfb02e6 100644 --- a/frontend_tests/node_tests/general.js +++ b/frontend_tests/node_tests/general.js @@ -76,7 +76,7 @@ const messages = { stream_id: denmark_stream.stream_id, type: 'stream', flags: ['has_alert_word'], - subject: 'copenhagen', + topic: 'copenhagen', // note we don't have every field that a "real" message // would have, and that can be fine }, @@ -171,13 +171,13 @@ run_test('filter', () => { assert.equal(predicate({ type: 'stream', stream_id: denmark_stream.stream_id, - subject: 'does not match filter', + topic: 'does not match filter', }), false); assert.equal(predicate({ type: 'stream', stream_id: denmark_stream.stream_id, - subject: 'copenhagen', + topic: 'copenhagen', }), true); }); @@ -508,7 +508,7 @@ run_test('unread_ops', () => { id: 50, type: 'stream', stream_id: denmark_stream.stream_id, - subject: 'copenhagen', + topic: 'copenhagen', unread: true, }, ]; diff --git a/frontend_tests/node_tests/hash_util.js b/frontend_tests/node_tests/hash_util.js index 1fdd5964c0..22d703cf0d 100644 --- a/frontend_tests/node_tests/hash_util.js +++ b/frontend_tests/node_tests/hash_util.js @@ -133,7 +133,7 @@ run_test('test_by_conversation_and_time_uri', () => { var message = { type: 'stream', stream_id: frontend.stream_id, - subject: 'testing', + topic: 'testing', id: 42, }; diff --git a/frontend_tests/node_tests/markdown.js b/frontend_tests/node_tests/markdown.js index 12781481fe..196118313d 100644 --- a/frontend_tests/node_tests/markdown.js +++ b/frontend_tests/node_tests/markdown.js @@ -390,33 +390,33 @@ run_test('marked', () => { }); run_test('topic_links', () => { - var message = {type: 'stream', subject: "No links here"}; + var message = {type: 'stream', topic: "No links here"}; markdown.add_topic_links(message); assert.equal(util.get_topic_links(message).length, []); - message = {type: 'stream', subject: "One #123 link here"}; + message = {type: 'stream', topic: "One #123 link here"}; markdown.add_topic_links(message); assert.equal(util.get_topic_links(message).length, 1); assert.equal(util.get_topic_links(message)[0], "https://trac.zulip.net/ticket/123"); - message = {type: 'stream', subject: "Two #123 #456 link here"}; + message = {type: 'stream', topic: "Two #123 #456 link here"}; markdown.add_topic_links(message); assert.equal(util.get_topic_links(message).length, 2); assert.equal(util.get_topic_links(message)[0], "https://trac.zulip.net/ticket/123"); assert.equal(util.get_topic_links(message)[1], "https://trac.zulip.net/ticket/456"); - message = {type: 'stream', subject: "New ZBUG_123 link here"}; + message = {type: 'stream', topic: "New ZBUG_123 link here"}; markdown.add_topic_links(message); assert.equal(util.get_topic_links(message).length, 1); assert.equal(util.get_topic_links(message)[0], "https://trac2.zulip.net/ticket/123"); - message = {type: 'stream', subject: "New ZBUG_123 with #456 link here"}; + message = {type: 'stream', topic: "New ZBUG_123 with #456 link here"}; markdown.add_topic_links(message); assert.equal(util.get_topic_links(message).length, 2); assert(util.get_topic_links(message).indexOf("https://trac2.zulip.net/ticket/123") !== -1); assert(util.get_topic_links(message).indexOf("https://trac.zulip.net/ticket/456") !== -1); - message = {type: 'stream', subject: "One ZGROUP_123:45 link here"}; + message = {type: 'stream', topic: "One ZGROUP_123:45 link here"}; markdown.add_topic_links(message); assert.equal(util.get_topic_links(message).length, 1); assert.equal(util.get_topic_links(message)[0], "https://zone_45.zulip.net/ticket/123"); @@ -428,20 +428,20 @@ run_test('topic_links', () => { run_test('message_flags', () => { var input = "/me is testing this"; - var message = {subject: "No links here", raw_content: input}; + var message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.is_me_message, true); assert(!message.unread); input = "/me is testing\nthis"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.is_me_message, true); input = "testing this @**all** @**Cordelia Lear**"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.is_me_message, false); @@ -449,51 +449,51 @@ run_test('message_flags', () => { assert.equal(message.mentioned_me_directly, true); input = "test @**everyone**"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.is_me_message, false); assert.equal(message.mentioned, true); assert.equal(message.mentioned_me_directly, false); input = "test @**stream**"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.is_me_message, false); assert.equal(message.mentioned, true); assert.equal(message.mentioned_me_directly, false); input = "test @all"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.mentioned, false); input = "test @everyone"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.mentioned, false); input = "test @any"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.mentioned, false); input = "test @alleycat.com"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.mentioned, false); input = "test @*hamletcharacters*"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.mentioned, true); input = "test @*backend*"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.mentioned, false); input = "test @**invalid_user**"; - message = {subject: "No links here", raw_content: input}; + message = {topic: "No links here", raw_content: input}; markdown.apply_markdown(message); assert.equal(message.mentioned, false); }); diff --git a/frontend_tests/node_tests/message_events.js b/frontend_tests/node_tests/message_events.js index dca121c0f6..5f6eec2713 100644 --- a/frontend_tests/node_tests/message_events.js +++ b/frontend_tests/node_tests/message_events.js @@ -110,6 +110,7 @@ run_test('update_messages', () => { sender_full_name: 'Alice Patel', sender_id: 32, sent_by_me: false, + topic: undefined, }, ]); diff --git a/frontend_tests/node_tests/message_list.js b/frontend_tests/node_tests/message_list.js index 209c36813a..53cfefdbe0 100644 --- a/frontend_tests/node_tests/message_list.js +++ b/frontend_tests/node_tests/message_list.js @@ -404,11 +404,13 @@ run_test('unmuted_messages', () => { id: 50, stream_id: muted_stream_id, mentioned: true, // overrides mute + topic: 'whatever', }, { id: 60, stream_id: 42, mentioned: false, + topic: 'whatever', }, ]; var muted = [ @@ -416,6 +418,7 @@ run_test('unmuted_messages', () => { id: 70, stream_id: muted_stream_id, mentioned: false, + topic: 'whatever', }, ]; diff --git a/frontend_tests/node_tests/message_list_data.js b/frontend_tests/node_tests/message_list_data.js index af96955fb6..23282bed0e 100644 --- a/frontend_tests/node_tests/message_list_data.js +++ b/frontend_tests/node_tests/message_list_data.js @@ -17,6 +17,7 @@ function make_msg(msg_id) { return { id: msg_id, unread: true, + topic: 'whatever', }; } @@ -138,10 +139,10 @@ run_test('more muting', () => { }); const orig_messages = [ - {id: 3, subject: 'muted'}, - {id: 4}, - {id: 7, subject: 'muted'}, - {id: 8}, + {id: 3, topic: 'muted'}, + {id: 4, topic: 'whatever'}, + {id: 7, topic: 'muted'}, + {id: 8, topic: 'whatever'}, ]; const orig_info = mld.add_messages(orig_messages); @@ -150,8 +151,8 @@ run_test('more muting', () => { top_messages: [], interior_messages: [], bottom_messages: [ - {id: 4}, - {id: 8}, + {id: 4, topic: 'whatever'}, + {id: 8, topic: 'whatever'}, ], }); @@ -166,13 +167,13 @@ run_test('more muting', () => { ); const more_messages = [ - {id: 1, subject: 'muted'}, - {id: 2}, - {id: 3, subject: 'muted'}, // dup - {id: 5, subject: 'muted'}, - {id: 6}, - {id: 9, subject: 'muted'}, - {id: 10}, + {id: 1, topic: 'muted'}, + {id: 2, topic: 'whatever'}, + {id: 3, topic: 'muted'}, // dup + {id: 5, topic: 'muted'}, + {id: 6, topic: 'whatever'}, + {id: 9, topic: 'muted'}, + {id: 10, topic: 'whatever'}, ]; const more_info = mld.add_messages(more_messages); @@ -189,13 +190,13 @@ run_test('more muting', () => { assert.deepEqual(more_info, { top_messages: [ - {id: 2}, + {id: 2, topic: 'whatever'}, ], interior_messages: [ - {id: 6}, + {id: 6, topic: 'whatever'}, ], bottom_messages: [ - {id: 10}, + {id: 10, topic: 'whatever'}, ], }); diff --git a/frontend_tests/node_tests/message_list_view.js b/frontend_tests/node_tests/message_list_view.js index 910591df7f..4d6e9dc087 100644 --- a/frontend_tests/node_tests/message_list_view.js +++ b/frontend_tests/node_tests/message_list_view.js @@ -65,7 +65,7 @@ run_test('merge_message_groups', () => { status_message: false, type: 'stream', stream: 'Test Stream 1', - subject: 'Test Subject 1', + topic: 'Test Subject 1', sender_email: 'test@example.com', timestamp: _.uniqueId(), }); @@ -162,7 +162,7 @@ run_test('merge_message_groups', () => { message1, ]); - var message2 = build_message_context({subject: 'Test subject 2'}); + var message2 = build_message_context({topic: 'Test subject 2'}); var message_group2 = build_message_group([ message2, ]); @@ -293,7 +293,7 @@ run_test('merge_message_groups', () => { message1, ]); - var message2 = build_message_context({subject: 'Test Subject 2'}); + var message2 = build_message_context({topic: 'Test Subject 2'}); var message_group2 = build_message_group([ message2, ]); diff --git a/frontend_tests/node_tests/narrow.js b/frontend_tests/node_tests/narrow.js index c558e65df8..68d5f9da17 100644 --- a/frontend_tests/node_tests/narrow.js +++ b/frontend_tests/node_tests/narrow.js @@ -52,7 +52,7 @@ run_test('stream_topic', () => { global.current_msg_list.selected_message = function () { return { stream: 'Stream1', - subject: 'Topic1', + topic: 'Topic1', }; }; diff --git a/frontend_tests/node_tests/narrow_activate.js b/frontend_tests/node_tests/narrow_activate.js index 8b5380ca38..3ead5f0bec 100644 --- a/frontend_tests/node_tests/narrow_activate.js +++ b/frontend_tests/node_tests/narrow_activate.js @@ -142,6 +142,7 @@ run_test('basics', () => { id: selected_id, type: 'stream', stream_id: denmark.stream_id, + topic: 'whatever', }; var messages = [selected_message]; diff --git a/frontend_tests/node_tests/narrow_local.js b/frontend_tests/node_tests/narrow_local.js index 6e1436b0d5..acd5617237 100644 --- a/frontend_tests/node_tests/narrow_local.js +++ b/frontend_tests/node_tests/narrow_local.js @@ -81,9 +81,9 @@ run_test('near after unreads', () => { }, has_found_newest: false, all_messages: [ - {id: 37}, - {id: 42}, - {id: 44}, + {id: 37, topic: 'whatever'}, + {id: 42, topic: 'whatever'}, + {id: 44, topic: 'whatever'}, ], expected_id_info: { target_id: 42, @@ -108,8 +108,8 @@ run_test('near not in message list', () => { }, has_found_newest: false, all_messages: [ - {id: 45}, - {id: 46}, + {id: 45, topic: 'whatever'}, + {id: 46, topic: 'whatever'}, ], expected_id_info: { target_id: 42, @@ -134,9 +134,9 @@ run_test('near before unreads', () => { }, has_found_newest: false, all_messages: [ - {id: 42}, - {id: 43}, - {id: 44}, + {id: 42, topic: 'whatever'}, + {id: 43, topic: 'whatever'}, + {id: 44, topic: 'whatever'}, ], expected_id_info: { target_id: 42, @@ -301,8 +301,8 @@ run_test('is:alerted with no unreads and one match', () => { }, has_found_newest: true, all_messages: [ - {id: 55, alerted: true}, - {id: 57, alerted: false}, + {id: 55, topic: 'whatever', alerted: true}, + {id: 57, topic: 'whatever', alerted: false}, ], expected_id_info: { target_id: undefined, @@ -433,9 +433,9 @@ run_test('final corner case', () => { has_found_newest: true, empty: false, all_messages: [ - {id: 400}, - {id: 425, starred: true}, - {id: 500}, + {id: 400, topic: 'whatever'}, + {id: 425, topic: 'whatever', starred: true}, + {id: 500, topic: 'whatever'}, ], expected_id_info: { target_id: 450, diff --git a/frontend_tests/node_tests/narrow_unread.js b/frontend_tests/node_tests/narrow_unread.js index e5f8c84de4..cc88170e91 100644 --- a/frontend_tests/node_tests/narrow_unread.js +++ b/frontend_tests/node_tests/narrow_unread.js @@ -51,7 +51,7 @@ run_test('get_unread_ids', () => { id: 101, type: 'stream', stream_id: sub.stream_id, - subject: 'my topic', + topic: 'my topic', unread: true, mentioned: true, }; diff --git a/frontend_tests/node_tests/notifications.js b/frontend_tests/node_tests/notifications.js index 5537a1465a..21c761e231 100644 --- a/frontend_tests/node_tests/notifications.js +++ b/frontend_tests/node_tests/notifications.js @@ -56,7 +56,7 @@ run_test('message_is_notifiable', () => { type: 'stream', stream: 'general', stream_id: general.stream_id, - subject: 'whatever', + topic: 'whatever', }), false); // Case 2: If the user has already been sent a notificaton about this message, @@ -73,7 +73,7 @@ run_test('message_is_notifiable', () => { type: 'stream', stream: 'general', stream_id: general.stream_id, - subject: 'whatever', + topic: 'whatever', }), false); // Case 3: If a message mentions the user directly, @@ -88,7 +88,7 @@ run_test('message_is_notifiable', () => { type: 'stream', stream: 'muted', stream_id: muted.stream_id, - subject: 'topic_three', + topic: 'topic_three', }), true); // Case 4: @@ -102,7 +102,7 @@ run_test('message_is_notifiable', () => { type: 'stream', stream: 'general', stream_id: general.stream_id, - subject: 'vanilla', + topic: 'vanilla', }), true); // Case 5: If a message is in a muted stream @@ -117,7 +117,7 @@ run_test('message_is_notifiable', () => { type: 'stream', stream: 'muted', stream_id: muted.stream_id, - subject: 'whatever', + topic: 'whatever', }), false); // Case 6: If a message is in a muted topic @@ -132,7 +132,7 @@ run_test('message_is_notifiable', () => { type: 'stream', stream: 'general', stream_id: general.stream_id, - subject: 'muted topic', + topic: 'muted topic', }), false); // Case 7 @@ -149,7 +149,7 @@ run_test('message_is_notifiable', () => { type: 'stream', stream: 'general', stream_id: general.stream_id, - subject: 'whatever', + topic: 'whatever', }), true); }); @@ -190,7 +190,7 @@ run_test('basic_notifications', () => { type: 'stream', stream: 'general', stream_id: muted.stream_id, - subject: 'whatever', + topic: 'whatever', }; var message_2 = { @@ -204,7 +204,7 @@ run_test('basic_notifications', () => { type: 'stream', stream: 'general', stream_id: muted.stream_id, - subject: 'lunch', + topic: 'lunch', }; // Send notification. diff --git a/frontend_tests/node_tests/recent_senders.js b/frontend_tests/node_tests/recent_senders.js index a42d1cbcd3..a375c3fb21 100644 --- a/frontend_tests/node_tests/recent_senders.js +++ b/frontend_tests/node_tests/recent_senders.js @@ -18,13 +18,13 @@ run_test('process_message_for_senders', () => { var message1 = { stream_id: stream1, id: _.uniqueId(), - subject: topic1, + topic: topic1, sender_id: sender1, }; var message2 = { stream_id: stream2, id: _.uniqueId(), - subject: topic1, + topic: topic1, sender_id: sender2, }; rs.process_message_for_senders(message1); @@ -47,7 +47,7 @@ run_test('process_message_for_senders', () => { var message3 = { stream_id: stream1, id: _.uniqueId(), - subject: topic2, + topic: topic2, sender_id: sender3, }; rs.process_message_for_senders(message3); @@ -59,7 +59,7 @@ run_test('process_message_for_senders', () => { var message4 = { stream_id: stream1, id: _.uniqueId(), - subject: topic1, + topic: topic1, sender_id: sender2, }; rs.process_message_for_senders(message4); @@ -71,7 +71,7 @@ run_test('process_message_for_senders', () => { var message5 = { stream_id: stream1, id: _.uniqueId(), - subject: topic1, + topic: topic1, sender_id: sender1, }; rs.process_message_for_senders(message5); @@ -83,19 +83,19 @@ run_test('process_message_for_senders', () => { var message6 = { stream_id: stream3, id: _.uniqueId(), - subject: topic1, + topic: topic1, sender_id: sender1, }; var message7 = { stream_id: stream3, id: _.uniqueId(), - subject: topic2, + topic: topic2, sender_id: sender2, }; var message8 = { stream_id: stream3, id: _.uniqueId(), - subject: topic3, + topic: topic3, sender_id: sender3, }; diff --git a/frontend_tests/node_tests/topic_data.js b/frontend_tests/node_tests/topic_data.js index cb24b95619..38f08349d0 100644 --- a/frontend_tests/node_tests/topic_data.js +++ b/frontend_tests/node_tests/topic_data.js @@ -147,10 +147,10 @@ run_test('test_unread_logic', () => { assert.deepEqual(history, ['toPic1', 'topic2']); const msgs = [ - { id: 150, subject: 'TOPIC2' }, // will be ignored - { id: 61, subject: 'unread1' }, - { id: 60, subject: 'unread1' }, - { id: 20, subject: 'UNREAD2' }, + { id: 150, topic: 'TOPIC2' }, // will be ignored + { id: 61, topic: 'unread1' }, + { id: 60, topic: 'unread1' }, + { id: 20, topic: 'UNREAD2' }, ]; _.each(msgs, (msg) => { diff --git a/frontend_tests/node_tests/transmit.js b/frontend_tests/node_tests/transmit.js index 2e57a87d7b..be22840a94 100644 --- a/frontend_tests/node_tests/transmit.js +++ b/frontend_tests/node_tests/transmit.js @@ -178,7 +178,7 @@ run_test('reply_message_stream', () => { const stream_message = { type: 'stream', stream: 'social', - subject: 'lunch', + topic: 'lunch', sender_full_name: 'Alice', sender_id: 123, }; @@ -207,7 +207,7 @@ run_test('reply_message_stream', () => { type: 'stream', to: 'social', content: '@**Alice** hello', - subject: 'lunch', + topic: 'lunch', }); }); diff --git a/frontend_tests/node_tests/typeahead_helper.js b/frontend_tests/node_tests/typeahead_helper.js index b8cd8c01bf..8b16904d2b 100644 --- a/frontend_tests/node_tests/typeahead_helper.js +++ b/frontend_tests/node_tests/typeahead_helper.js @@ -212,19 +212,19 @@ run_test('sort_recipients', () => { global.recent_senders.process_message_for_senders({ sender_id: 7, stream_id: 1, - subject: "Dev Topic", + topic: "Dev Topic", id: _.uniqueId(), }); global.recent_senders.process_message_for_senders({ sender_id: 5, stream_id: 1, - subject: "Dev Topic", + topic: "Dev Topic", id: _.uniqueId(), }); global.recent_senders.process_message_for_senders({ sender_id: 6, stream_id: 1, - subject: "Dev Topic", + topic: "Dev Topic", id: _.uniqueId(), }); @@ -242,13 +242,13 @@ run_test('sort_recipients', () => { global.recent_senders.process_message_for_senders({ sender_id: 5, stream_id: 2, - subject: "Linux Topic", + topic: "Linux Topic", id: _.uniqueId(), }); global.recent_senders.process_message_for_senders({ sender_id: 7, stream_id: 2, - subject: "Linux Topic", + topic: "Linux Topic", id: _.uniqueId(), }); diff --git a/frontend_tests/node_tests/unread.js b/frontend_tests/node_tests/unread.js index b8e39b87ab..9dcc5a13c5 100644 --- a/frontend_tests/node_tests/unread.js +++ b/frontend_tests/node_tests/unread.js @@ -49,8 +49,8 @@ run_test('empty_counts_while_home', () => { assert.deepEqual(counts, zero_counts); }); -run_test('changing_subjects', () => { - // Summary: change the subject of a message from 'lunch' +run_test('changing_topics', () => { + // Summary: change the topic of a message from 'lunch' // to 'dinner' using update_unread_topics(). var count = unread.num_unread_for_topic('social', 'lunch'); assert.equal(count, 0); @@ -62,7 +62,7 @@ run_test('changing_subjects', () => { id: 15, type: 'stream', stream_id: stream_id, - subject: 'luNch', + topic: 'luNch', unread: true, }; @@ -70,7 +70,7 @@ run_test('changing_subjects', () => { id: 16, type: 'stream', stream_id: stream_id, - subject: 'lunCH', + topic: 'lunCH', unread: true, }; @@ -144,7 +144,7 @@ run_test('changing_subjects', () => { id: 17, type: 'stream', stream_id: stream_id, - subject: 'sticky', + topic: 'sticky', unread: true, }; @@ -198,7 +198,7 @@ run_test('muting', () => { id: 15, type: 'stream', stream_id: stream_id, - subject: 'test_muting', + topic: 'test_muting', unread: true, }; @@ -241,7 +241,7 @@ run_test('num_unread_for_topic', () => { var message = { type: 'stream', stream_id: stream_id, - subject: 'LuncH', + topic: 'LuncH', unread: true, }; @@ -319,7 +319,7 @@ run_test('home_messages', () => { id: 15, type: 'stream', stream_id: stream_id, - subject: 'lunch', + topic: 'lunch', unread: true, }; @@ -353,7 +353,7 @@ run_test('phantom_messages', () => { id: 999, type: 'stream', stream_id: 555, - subject: 'phantom', + topic: 'phantom', }; stream_data.get_sub_by_id = function () { return; }; @@ -448,7 +448,7 @@ run_test('mentions', () => { id: 15, type: 'stream', stream_id: 999, - subject: 'lunch', + topic: 'lunch', mentioned: true, unread: true, }; @@ -475,7 +475,7 @@ run_test('declare_bankruptcy', () => { id: 16, type: 'whatever', stream_id: 1999, - subject: 'whatever', + topic: 'whatever', mentioned: true, }; diff --git a/frontend_tests/node_tests/util.js b/frontend_tests/node_tests/util.js index 98fb5dc483..e4760a1696 100644 --- a/frontend_tests/node_tests/util.js +++ b/frontend_tests/node_tests/util.js @@ -1,4 +1,5 @@ set_global('$', global.make_zjquery()); +set_global('blueslip', global.make_zblueslip({})); set_global('document', {}); zrequire('util'); @@ -67,12 +68,12 @@ run_test('lower_bound', () => { run_test('same_recipient', () => { assert(util.same_recipient( - {type: 'stream', stream_id: 101, subject: 'Bar'}, - {type: 'stream', stream_id: 101, subject: 'bar'})); + {type: 'stream', stream_id: 101, topic: 'Bar'}, + {type: 'stream', stream_id: 101, topic: 'bar'})); assert(!util.same_recipient( - {type: 'stream', stream_id: 101, subject: 'Bar'}, - {type: 'stream', stream_id: 102, subject: 'whatever'})); + {type: 'stream', stream_id: 101, topic: 'Bar'}, + {type: 'stream', stream_id: 102, topic: 'whatever'})); assert(util.same_recipient( {type: 'private', to_user_ids: '101,102'}, @@ -83,7 +84,7 @@ run_test('same_recipient', () => { {type: 'private', to_user_ids: '103'})); assert(!util.same_recipient( - {type: 'stream', stream_id: 101, subject: 'Bar'}, + {type: 'stream', stream_id: 101, topic: 'Bar'}, {type: 'private'})); assert(!util.same_recipient( @@ -114,7 +115,9 @@ run_test('robust_uri_decode', () => { }); run_test('get_message_topic', () => { + blueslip.set_test_data('warn', 'programming error: message has no topic'); assert.equal(util.get_message_topic({subject: 'foo'}), 'foo'); + blueslip.clear_test_data(); assert.equal(util.get_message_topic({topic: 'bar'}), 'bar'); }); diff --git a/static/js/message_store.js b/static/js/message_store.js index 44c2459366..441d6b0b11 100644 --- a/static/js/message_store.js +++ b/static/js/message_store.js @@ -127,6 +127,10 @@ exports.add_message_metadata = function (message) { message.sender_email = sender.email; } + // Convert topic even for PMs, as legacy code + // wants the empty field. + util.convert_message_topic(message); + switch (message.type) { case 'stream': message.is_stream = true; diff --git a/static/js/util.js b/static/js/util.js index ef61c7035c..66a304e51c 100644 --- a/static/js/util.js +++ b/static/js/util.js @@ -311,11 +311,12 @@ exports.get_reload_topic = function (obj) { }; exports.set_message_topic = function (obj, topic) { - obj.subject = topic; + obj.topic = topic; }; exports.get_message_topic = function (obj) { if (obj.topic === undefined) { + blueslip.warn('programming error: message has no topic'); return obj.subject; } @@ -340,6 +341,12 @@ exports.is_topic_synonym = function (operator) { return operator === 'subject'; }; +exports.convert_message_topic = function (message) { + if (message.topic === undefined) { + message.topic = message.subject; + } +}; + return exports; }());