narrow: Don't advertise streams:public in is:starred.

We fix this by adding a more expressive data function, with tests, for
whether a filter is on UserMessage data, which would mean that
streams:public could never add additional matches.
This commit is contained in:
Tim Abbott
2020-04-08 11:25:18 -07:00
parent 218be002f1
commit 655993bf0f
3 changed files with 30 additions and 1 deletions

View File

@@ -72,6 +72,7 @@ run_test('basics', () => {
assert(!filter.allow_use_first_unread_when_narrowing()); assert(!filter.allow_use_first_unread_when_narrowing());
assert(filter.includes_full_stream_history()); assert(filter.includes_full_stream_history());
assert(filter.can_apply_locally()); assert(filter.can_apply_locally());
assert(!filter.is_personal_filter());
operators = [ operators = [
{operator: 'stream', operand: 'foo'}, {operator: 'stream', operand: 'foo'},
@@ -85,6 +86,7 @@ run_test('basics', () => {
assert(!filter.contains_only_private_messages()); assert(!filter.contains_only_private_messages());
assert(!filter.allow_use_first_unread_when_narrowing()); assert(!filter.allow_use_first_unread_when_narrowing());
assert(!filter.can_apply_locally()); assert(!filter.can_apply_locally());
assert(!filter.is_personal_filter());
assert(filter.can_bucket_by('stream')); assert(filter.can_bucket_by('stream'));
assert(filter.can_bucket_by('stream', 'topic')); assert(filter.can_bucket_by('stream', 'topic'));
@@ -98,6 +100,7 @@ run_test('basics', () => {
assert(!filter.contains_only_private_messages()); assert(!filter.contains_only_private_messages());
assert(!filter.has_operator('stream')); assert(!filter.has_operator('stream'));
assert(!filter.can_mark_messages_read()); assert(!filter.can_mark_messages_read());
assert(!filter.is_personal_filter());
// Negated searches are just like positive searches for our purposes, since // Negated searches are just like positive searches for our purposes, since
// the search logic happens on the back end and we need to have can_apply_locally() // the search logic happens on the back end and we need to have can_apply_locally()
@@ -110,6 +113,7 @@ run_test('basics', () => {
assert(filter.has_operator('search')); assert(filter.has_operator('search'));
assert(!filter.can_apply_locally()); assert(!filter.can_apply_locally());
assert(!filter.can_mark_messages_read()); assert(!filter.can_mark_messages_read());
assert(!filter.is_personal_filter());
// Similar logic applies to negated "has" searches. // Similar logic applies to negated "has" searches.
operators = [ operators = [
@@ -120,6 +124,7 @@ run_test('basics', () => {
assert(!filter.can_apply_locally()); assert(!filter.can_apply_locally());
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());
operators = [ operators = [
{operator: 'streams', operand: 'public', negated: true}, {operator: 'streams', operand: 'public', negated: true},
@@ -130,6 +135,7 @@ run_test('basics', () => {
assert(!filter.can_mark_messages_read()); assert(!filter.can_mark_messages_read());
assert(filter.has_negated_operand('streams', 'public')); assert(filter.has_negated_operand('streams', 'public'));
assert(!filter.can_apply_locally()); assert(!filter.can_apply_locally());
assert(!filter.is_personal_filter());
operators = [ operators = [
{operator: 'streams', operand: 'public'}, {operator: 'streams', operand: 'public'},
@@ -141,6 +147,7 @@ run_test('basics', () => {
assert(!filter.has_negated_operand('streams', 'public')); assert(!filter.has_negated_operand('streams', 'public'));
assert(!filter.can_apply_locally()); assert(!filter.can_apply_locally());
assert(filter.includes_full_stream_history()); assert(filter.includes_full_stream_history());
assert(!filter.is_personal_filter());
operators = [ operators = [
{operator: 'is', operand: 'private'}, {operator: 'is', operand: 'private'},
@@ -150,6 +157,7 @@ run_test('basics', () => {
assert(filter.can_mark_messages_read()); assert(filter.can_mark_messages_read());
assert(!filter.has_operator('search')); assert(!filter.has_operator('search'));
assert(filter.can_apply_locally()); assert(filter.can_apply_locally());
assert(!filter.is_personal_filter());
operators = [ operators = [
{operator: 'is', operand: 'mentioned'}, {operator: 'is', operand: 'mentioned'},
@@ -159,6 +167,17 @@ run_test('basics', () => {
assert(filter.can_mark_messages_read()); assert(filter.can_mark_messages_read());
assert(!filter.has_operator('search')); assert(!filter.has_operator('search'));
assert(filter.can_apply_locally()); assert(filter.can_apply_locally());
assert(filter.is_personal_filter());
operators = [
{operator: 'is', operand: 'starred'},
];
filter = new Filter(operators);
assert(!filter.contains_only_private_messages());
assert(!filter.can_mark_messages_read());
assert(!filter.has_operator('search'));
assert(filter.can_apply_locally());
assert(filter.is_personal_filter());
operators = [ operators = [
{operator: 'pm-with', operand: 'joe@example.com'}, {operator: 'pm-with', operand: 'joe@example.com'},
@@ -167,6 +186,7 @@ run_test('basics', () => {
assert(filter.contains_only_private_messages()); assert(filter.contains_only_private_messages());
assert(!filter.has_operator('search')); assert(!filter.has_operator('search'));
assert(filter.can_apply_locally()); assert(filter.can_apply_locally());
assert(!filter.is_personal_filter());
operators = [ operators = [
{operator: 'group-pm-with', operand: 'joe@example.com'}, {operator: 'group-pm-with', operand: 'joe@example.com'},

View File

@@ -446,6 +446,15 @@ Filter.prototype = {
return this.has_operator("stream") || this.has_operator("streams"); return this.has_operator("stream") || this.has_operator("streams");
}, },
is_personal_filter: function () {
// Whether the filter filters for user-specific data in the
// UserMessage table, such as stars or mentions.
//
// Such filters should not advertise "streams:public" as it
// will never add additional results.
return this.has_operand("is", "mentioned") || this.has_operand("is", "starred");
},
can_apply_locally: function () { can_apply_locally: function () {
if (this.is_search()) { if (this.is_search()) {
// The semantics for matching keywords are implemented // The semantics for matching keywords are implemented

View File

@@ -264,7 +264,7 @@ exports.activate = function (raw_operators, opts) {
// by a potential spammer user. // by a potential spammer user.
if (!filter.contains_only_private_messages() && if (!filter.contains_only_private_messages() &&
!filter.includes_full_stream_history() && !filter.includes_full_stream_history() &&
!filter.has_operand("is", "starred")) { !filter.is_personal_filter()) {
$(".all-messages-search-caution").show(); $(".all-messages-search-caution").show();
// Set the link to point to this search with streams:public added. // Set the link to point to this search with streams:public added.
// It's a bit hacky to use the href, but // It's a bit hacky to use the href, but