From 191741a3828d42052339d0f996237bc59ddbc7a4 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Thu, 11 May 2017 14:22:46 -0700 Subject: [PATCH] Use stream ids to filter messages in client-side narrows. We now use stream ids to filter messages in narrowing situations, instead of doing stream name comparisons. This partially fixes certain stream-renaming scenarios, since we will be able to match the stream id for an out-of-date stream operand, but it doesn't fix some other stuff, such as the query that the server gets. --- frontend_tests/node_tests/filter.js | 31 ++++++++++++++++++++++++----- static/js/filter.js | 11 ++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/frontend_tests/node_tests/filter.js b/frontend_tests/node_tests/filter.js index 5cf0b67678..c80dc43177 100644 --- a/frontend_tests/node_tests/filter.js +++ b/frontend_tests/node_tests/filter.js @@ -212,6 +212,14 @@ function get_predicate(operators) { return new Filter(operators).predicate(); } +function make_sub(name, stream_id) { + var sub = { + name: name, + stream_id: stream_id, + }; + global.stream_data.add_sub(name, sub); +} + (function test_predicate_basics() { // Predicates are functions that accept a message object with the message // attributes (not content), and return true if the message belongs in a @@ -220,12 +228,22 @@ function get_predicate(operators) { // // To keep these tests simple, we only pass objects with a few relevant attributes // rather than full-fledged message objects. + + var stream_id = 42; + make_sub('Foo', stream_id); var predicate = get_predicate([['stream', 'Foo'], ['topic', 'Bar']]); - assert(predicate({type: 'stream', stream: 'foo', subject: 'bar'})); - assert(!predicate({type: 'stream', stream: 'foo', subject: 'whatever'})); - assert(!predicate({type: 'stream', stream: 'wrong'})); + + 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: 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'})); + predicate = get_predicate([['search', 'emoji']]); assert(predicate({})); @@ -299,12 +317,15 @@ function get_predicate(operators) { var predicate; var narrow; + var social_stream_id = 555; + make_sub('social', social_stream_id); + narrow = [ {operator: 'stream', operand: 'social', negated: true}, ]; predicate = new Filter(narrow).predicate(); - assert(predicate({type: 'stream', stream: 'devel'})); - assert(!predicate({type: 'stream', stream: 'social'})); + assert(predicate({type: 'stream', stream_id: 999999})); + assert(!predicate({type: 'stream', stream_id: social_stream_id})); }()); (function test_mit_exceptions() { diff --git a/static/js/filter.js b/static/js/filter.js index 3ea8a3a8a0..fe3b3cd618 100644 --- a/static/js/filter.js +++ b/static/js/filter.js @@ -82,6 +82,17 @@ function message_matches_search_term(message, operator, operand) { if (page_params.realm_is_zephyr_mirror_realm) { return zephyr_stream_name_match(message, operand); } + + // Try to match by stream_id if have a valid sub for + // the operand. + var stream_id = stream_data.get_stream_id(operand); + if (stream_id) { + return (message.stream_id === stream_id); + } + + // We need this fallback logic in case we have a message + // loaded for a stream that we are no longer + // subscribed to (or that was deleted). return (message.stream.toLowerCase() === operand); case 'topic':