filter: Implement has: filters on client side.

Prior to this commit has:link, has:attachment, has:image
filter couldn't be applied locally and deferred filtering to
web server. This commits make sure client filters all messages
it can instead of completely deferring to the server and hence
improve speed.

A tradeoff is also made to turn off local echo for has: narrows
as messages with link sent to has:link narrow were locally echoing
to another narrow and not appearing in the active has:link narrow.

Fixes: #6186.
This commit is contained in:
Mohit Gupta
2020-06-13 22:35:31 +05:30
committed by Tim Abbott
parent a4f5b0c635
commit 02ea52fc18
4 changed files with 92 additions and 7 deletions

View File

@@ -3,6 +3,9 @@ zrequire('unread');
zrequire('stream_data'); zrequire('stream_data');
zrequire('people'); zrequire('people');
set_global('Handlebars', global.make_handlebars()); set_global('Handlebars', global.make_handlebars());
global.stub_out_jquery();
set_global('$', global.make_zjquery());
zrequire('message_util', 'js/message_util');
zrequire('Filter', 'js/filter'); zrequire('Filter', 'js/filter');
set_global('message_store', {}); set_global('message_store', {});
@@ -121,7 +124,8 @@ run_test('basics', () => {
]; ];
filter = new Filter(operators); filter = new Filter(operators);
assert(filter.has_operator('has')); assert(filter.has_operator('has'));
assert(!filter.can_apply_locally()); assert(filter.can_apply_locally());
assert(!filter.can_apply_locally(true));
assert(!filter.includes_full_stream_history()); assert(!filter.includes_full_stream_history());
assert(!filter.can_mark_messages_read()); assert(!filter.can_mark_messages_read());
assert(!filter.is_personal_filter()); assert(!filter.is_personal_filter());
@@ -715,6 +719,62 @@ run_test('predicate_basics', () => {
display_recipient: [{id: steve.user_id}, {id: me.user_id}], display_recipient: [{id: steve.user_id}, {id: me.user_id}],
})); }));
assert(!predicate({type: 'stream'})); assert(!predicate({type: 'stream'}));
const img_msg = {
content: `<p><a href="/user_uploads/randompath/test.jpeg">test.jpeg</a></p><div class="message_inline_image"><a href="/user_uploads/randompath/test.jpeg" title="test.jpeg"><img src="/user_uploads/randompath/test.jpeg"></a></div>`,
};
const link_msg = {
content: `<p><a href="http://chat.zulip.org">chat.zulip.org</a></p>`,
};
const non_img_attachment_msg = {
content: `<p><a href="/user_uploads/randompath/attachment.ext">attachment.ext</a></p>`,
};
const no_has_filter_matching_msg = {
content: "<p>Testing</p>",
};
predicate = get_predicate([['has', 'non_valid_operand']]);
assert(!predicate(img_msg));
assert(!predicate(non_img_attachment_msg));
assert(!predicate(link_msg));
assert(!predicate(no_has_filter_matching_msg));
// HTML content of message is used to determine if image have link, image or attachment.
// We are using jquery to parse the html and find existence of relevant tags/elements.
// In tests we need to stub the calls to jquery so using zjquery's .set_find_results method.
const has_link = get_predicate([['has', 'link']]);
$(img_msg.content).set_find_results("a", [$("<a>")]);
assert(has_link(img_msg));
$(non_img_attachment_msg.content).set_find_results("a", [$("<a>")]);
assert(has_link(non_img_attachment_msg));
$(link_msg.content).set_find_results("a", [$("<a>")]);
assert(has_link(link_msg));
$(no_has_filter_matching_msg.content).set_find_results("a", false);
assert(!has_link(no_has_filter_matching_msg));
const has_attachment = get_predicate([['has', 'attachment']]);
$(img_msg.content).set_find_results("a[href^='/user_uploads']", [$("<a>")]);
assert(has_attachment(img_msg));
$(non_img_attachment_msg.content).set_find_results("a[href^='/user_uploads']", [$("<a>")]);
assert(has_attachment(non_img_attachment_msg));
$(link_msg.content).set_find_results("a[href^='/user_uploads']", false);
assert(!has_attachment(link_msg));
$(no_has_filter_matching_msg.content).set_find_results("a[href^='/user_uploads']", false);
assert(!has_attachment(no_has_filter_matching_msg));
const has_image = get_predicate([['has', 'image']]);
$(img_msg.content).set_find_results(".message_inline_image", [$("<img>")]);
assert(has_image(img_msg));
$(non_img_attachment_msg.content).set_find_results(".message_inline_image", false);
assert(!has_image(non_img_attachment_msg));
$(link_msg.content).set_find_results(".message_inline_image", false);
assert(!has_image(link_msg));
$(no_has_filter_matching_msg.content).set_find_results(".message_inline_image", false);
assert(!has_image(no_has_filter_matching_msg));
}); });
run_test('negated_predicates', () => { run_test('negated_predicates', () => {

View File

@@ -146,7 +146,7 @@ exports.try_deliver_locally = function (message_request) {
return; return;
} }
if (narrow_state.active() && !narrow_state.filter().can_apply_locally()) { if (narrow_state.active() && !narrow_state.filter().can_apply_locally(true)) {
return; return;
} }

View File

@@ -46,6 +46,15 @@ function message_in_home(message) {
function message_matches_search_term(message, operator, operand) { function message_matches_search_term(message, operator, operand) {
switch (operator) { switch (operator) {
case 'has':
if (operand === 'image') {
return message_util.message_has_image(message);
} else if (operand === 'link') {
return message_util.message_has_link(message);
} else if (operand === 'attachment') {
return message_util.message_has_attachment(message);
}
return false; // has:something_else returns false
case 'is': case 'is':
if (operand === 'private') { if (operand === 'private') {
return message.type === 'private'; return message.type === 'private';
@@ -620,7 +629,10 @@ Filter.prototype = {
return this.has_operand("is", "mentioned") || this.has_operand("is", "starred"); return this.has_operand("is", "mentioned") || this.has_operand("is", "starred");
}, },
can_apply_locally: function () { can_apply_locally: function (is_local_echo) {
// Since there can be multiple operators, each block should
// just return false here.
if (this.is_search()) { if (this.is_search()) {
// The semantics for matching keywords are implemented // The semantics for matching keywords are implemented
// by database plugins, and we don't have JS code for // by database plugins, and we don't have JS code for
@@ -629,10 +641,11 @@ Filter.prototype = {
return false; return false;
} }
if (this.has_operator('has')) { if (this.has_operator('has') && is_local_echo) {
// See #6186 to see why we currently punt on 'has:foo' // The has: operators can be applied locally for messages
// queries. This can be fixed, there are just some random // rendered by the backend; links, attachments, and images
// complications that make it non-trivial. // are not handled properly by the local echo markdown
// processor.
return false; return false;
} }

View File

@@ -16,6 +16,18 @@ function add_messages(messages, msg_list, opts) {
return render_info; return render_info;
} }
exports.message_has_link = function (message) {
return $(message.content).find(`a`).length > 0;
};
exports.message_has_image = function (message) {
return $(message.content).find(`.message_inline_image`).length > 0;
};
exports.message_has_attachment = function (message) {
return $(message.content).find(`a[href^='/user_uploads']`).length > 0;
};
exports.add_old_messages = function (messages, msg_list) { exports.add_old_messages = function (messages, msg_list) {
return add_messages(messages, msg_list, {messages_are_new: false}); return add_messages(messages, msg_list, {messages_are_new: false});
}; };