From 311f436c6836633db1233be17e622aa37a291c51 Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Wed, 19 Apr 2023 17:35:32 +0200 Subject: [PATCH] narrow: Add frontend support for `dm-including:` narrow. Adds support in the web app for `dm-including` operator. This will deprecate the `group-pm-with` operator, but any changes to that narrow operator will be in a separate commit since it returns a different message query. The `group-pm-with` operator only returned group direct messages, while the new `dm-including` operator returns both group and 1-on-1 direct messages. The general API changelog and documentation updates will be done in a final commit in the series of commits that adds support for the various new direct message narrows. --- web/src/filter.js | 27 ++++++++++- web/src/hash_util.js | 2 + web/src/message_fetch.js | 2 +- web/src/narrow_banner.js | 44 ++++++++++++++++++ web/src/search_suggestion.js | 29 ++++++++++-- web/tests/filter.test.js | 32 +++++++++++++ web/tests/narrow.test.js | 53 ++++++++++++++++++++++ web/tests/search_suggestion_future.test.js | 28 +++++++++++- web/tests/search_suggestion_now.test.js | 27 ++++++++++- 9 files changed, 235 insertions(+), 9 deletions(-) diff --git a/web/src/filter.js b/web/src/filter.js index 1ae46cb88e..79ecff6050 100644 --- a/web/src/filter.js +++ b/web/src/filter.js @@ -191,6 +191,18 @@ function message_matches_search_term(message, operator, operand) { return _.isEqual(operand_ids, user_ids); } + + case "dm-including": { + const operand_ids = people.pm_with_operand_ids(operand); + if (!operand_ids) { + return false; + } + const user_ids = people.all_user_ids_in_pm(message); + if (!user_ids) { + return false; + } + return user_ids.includes(operand_ids[0]); + } } return true; // unknown operators return true (effectively ignored) @@ -256,6 +268,7 @@ export class Filter { operand = people.my_current_email(); } break; + case "dm-including": case "group-pm-with": operand = operand.toString().toLowerCase(); break; @@ -299,7 +312,11 @@ export class Filter { static decodeOperand(encoded, operator) { encoded = encoded.replace(/"/g, ""); - if (["group-pm-with", "dm", "sender", "from", "pm-with"].includes(operator) === false) { + if ( + ["dm-including", "dm", "sender", "from", "pm-with", "group-pm-with"].includes( + operator, + ) === false + ) { encoded = encoded.replace(/\+/g, " "); } return util.robust_url_decode(encoded).trim(); @@ -470,6 +487,8 @@ export class Filter { "topic", "not-topic", "dm", + "dm-including", + "not-dm-including", "group-pm-with", "not-group-pm-with", "is-dm", @@ -752,6 +771,7 @@ export class Filter { return ( (this.has_operator("is") && this.operands("is")[0] === "dm") || this.has_operator("dm") || + this.has_operator("dm-including") || this.has_operator("group-pm-with") ); } @@ -882,6 +902,7 @@ export class Filter { update_email(user_id, new_email) { for (const term of this._operators) { switch (term.operator) { + case "dm-including": case "group-pm-with": case "dm": case "pm-with": @@ -941,6 +962,7 @@ export class Filter { "stream", "topic", "dm", + "dm-including", "group-pm-with", "sender", "near", @@ -1009,6 +1031,9 @@ export class Filter { case "dm": return verb + "direct messages with"; + case "dm-including": + return verb + "direct messages including"; + case "in": return verb + "messages in"; diff --git a/web/src/hash_util.js b/web/src/hash_util.js index 855eadd4ba..7d7cae9a57 100644 --- a/web/src/hash_util.js +++ b/web/src/hash_util.js @@ -40,6 +40,7 @@ export function build_reload_url() { export function encode_operand(operator, operand) { if ( operator === "group-pm-with" || + operator === "dm-including" || operator === "dm" || operator === "sender" || operator === "pm-with" @@ -68,6 +69,7 @@ export function encode_stream_name(operand) { export function decode_operand(operator, operand) { if ( operator === "group-pm-with" || + operator === "dm-including" || operator === "dm" || operator === "sender" || operator === "pm-with" diff --git a/web/src/message_fetch.js b/web/src/message_fetch.js index 8c688dd000..2509734c0d 100644 --- a/web/src/message_fetch.js +++ b/web/src/message_fetch.js @@ -133,7 +133,7 @@ function get_messages_success(data, opts) { // because doing so breaks the app in various modules that expect emails string. function handle_operators_supporting_id_based_api(data) { const operators_supporting_ids = new Set(["dm", "pm-with"]); - const operators_supporting_id = new Set(["sender", "group-pm-with", "stream"]); + const operators_supporting_id = new Set(["sender", "group-pm-with", "stream", "dm-including"]); if (data.narrow === undefined) { return data; diff --git a/web/src/narrow_banner.js b/web/src/narrow_banner.js index 172068e4ac..1c52decf73 100644 --- a/web/src/narrow_banner.js +++ b/web/src/narrow_banner.js @@ -381,6 +381,50 @@ function pick_empty_narrow_banner() { title: $t({defaultMessage: "This user does not exist!"}), }; } + case "dm-including": { + const person_in_dms = people.get_by_email(first_operand); + if (!person_in_dms) { + return { + title: $t({defaultMessage: "This user does not exist!"}), + }; + } + if ( + page_params.realm_private_message_policy === + settings_config.private_message_policy_values.disabled.code && + !person_in_dms.is_bot + ) { + return { + title: $t({ + defaultMessage: + "You are not allowed to send direct messages in this organization.", + }), + }; + } + if (people.is_current_user(first_operand)) { + return { + title: $t({ + defaultMessage: "You don't have any direct message conversations yet.", + }), + }; + } + return { + title: $t( + { + defaultMessage: "You have no direct messages including {person} yet.", + }, + {person: person_in_dms.full_name}, + ), + html: $t_html( + { + defaultMessage: "Why not start the conversation?", + }, + { + "z-link": (content_html) => + `${content_html}`, + }, + ), + }; + } case "group-pm-with": { const person_in_group_pm = people.get_by_email(first_operand); if (!person_in_group_pm) { diff --git a/web/src/search_suggestion.js b/web/src/search_suggestion.js index 820a022894..0954c562ea 100644 --- a/web/src/search_suggestion.js +++ b/web/src/search_suggestion.js @@ -102,6 +102,7 @@ function get_stream_suggestions(last, operators) { {operator: "streams"}, {operator: "is", operand: "dm"}, {operator: "dm"}, + {operator: "dm-including"}, {operator: "group-pm-with"}, ]; if (!check_validity(last, operators, valid, invalid)) { @@ -238,7 +239,7 @@ function make_people_getter(last) { }; } -// Possible args for autocomplete_operator: dm, pm-with, sender, from, group-pm-with +// Possible args for autocomplete_operator: dm, pm-with, sender, from, dm-including, group-pm-with function get_person_suggestions(people_getter, last, operators, autocomplete_operator) { if ((last.operator === "is" && last.operand === "dm") || last.operator === "pm-with") { // Interpret "is:dm" or "pm-with:" operator as equivalent to "dm:". @@ -256,6 +257,7 @@ function get_person_suggestions(people_getter, last, operators, autocomplete_ope let invalid; switch (autocomplete_operator) { + case "dm-including": case "group-pm-with": invalid = [{operator: "stream"}, {operator: "is", operand: "resolved"}]; break; @@ -359,6 +361,7 @@ function get_topic_suggestions(last, operators) { const invalid = [ {operator: "dm"}, {operator: "is", operand: "dm"}, + {operator: "dm-including"}, {operator: "group-pm-with"}, {operator: "topic"}, ]; @@ -516,6 +519,7 @@ function get_streams_filter_suggestions(last, operators) { {operator: "is", operand: "dm"}, {operator: "stream"}, {operator: "group-pm-with"}, + {operator: "dm-including"}, {operator: "dm"}, {operator: "in"}, {operator: "streams"}, @@ -564,6 +568,7 @@ function get_is_filter_suggestions(last, operators) { {operator: "is", operand: "resolved"}, {operator: "is", operand: "dm"}, {operator: "dm"}, + {operator: "dm-including"}, {operator: "group-pm-with"}, ], }, @@ -654,7 +659,17 @@ function get_operator_suggestions(last) { last_operand = last_operand.slice(1); } - let choices = ["stream", "topic", "dm", "sender", "near", "from", "group-pm-with", "pm-with"]; + let choices = [ + "stream", + "topic", + "dm", + "dm-including", + "sender", + "near", + "from", + "group-pm-with", + "pm-with", + ]; choices = choices.filter((choice) => common.phrase_match(last_operand, choice)); return choices.map((choice) => { @@ -731,7 +746,14 @@ export function get_search_result(base_query, query) { search_operators.push(last); } - const person_suggestion_ops = ["sender", "dm", "from", "group-pm-with", "pm-with"]; + const person_suggestion_ops = [ + "sender", + "dm", + "dm-including", + "from", + "group-pm-with", + "pm-with", + ]; // Handle spaces in person name in new suggestions only. Checks if the last operator is 'search' // and the second last operator in search_operators is one out of person_suggestion_ops. @@ -789,6 +811,7 @@ export function get_search_result(base_query, query) { get_stream_suggestions, get_people("sender"), get_people("dm"), + get_people("dm-including"), get_people("from"), get_people("group-pm-with"), get_group_suggestions, diff --git a/web/tests/filter.test.js b/web/tests/filter.test.js index 0f3197682e..20ae6992a4 100644 --- a/web/tests/filter.test.js +++ b/web/tests/filter.test.js @@ -810,6 +810,35 @@ test("predicate_basics", () => { }), ); + predicate = get_predicate([["dm-including", "nobody@example.com"]]); + assert.ok( + !predicate({ + type: "private", + display_recipient: [{id: joe.user_id}, {id: me.user_id}], + }), + ); + + predicate = get_predicate([["dm-including", "Joe@example.com"]]); + assert.ok( + predicate({ + type: "private", + display_recipient: [{id: joe.user_id}, {id: steve.user_id}, {id: me.user_id}], + }), + ); + assert.ok( + predicate({ + type: "private", + display_recipient: [{id: joe.user_id}, {id: me.user_id}], + }), + ); + assert.ok( + !predicate({ + type: "private", + display_recipient: [{id: steve.user_id}, {id: me.user_id}], + }), + ); + assert.ok(!predicate({type: "stream"})); + predicate = get_predicate([["group-pm-with", "nobody@example.com"]]); assert.ok( !predicate({ @@ -1783,6 +1812,9 @@ run_test("is_spectator_compatible", () => { ); assert.ok(Filter.is_spectator_compatible([{operator: "sender", operand: "hamlet@zulip.com"}])); assert.ok(!Filter.is_spectator_compatible([{operator: "dm", operand: "hamlet@zulip.com"}])); + assert.ok( + !Filter.is_spectator_compatible([{operator: "dm-including", operand: "hamlet@zulip.com"}]), + ); assert.ok( !Filter.is_spectator_compatible([{operator: "group-pm-with", operand: "hamlet@zulip.com"}]), ); diff --git a/web/tests/narrow.test.js b/web/tests/narrow.test.js index 77509af1ff..ebf0aa8c1b 100644 --- a/web/tests/narrow.test.js +++ b/web/tests/narrow.test.js @@ -424,6 +424,59 @@ run_test("show_empty_narrow_message", ({mock_template}) => { ), ); + // organization has disabled sending direct messages + page_params.realm_private_message_policy = + settings_config.private_message_policy_values.disabled.code; + + // prioritize information about invalid user in narrow/search + set_filter([["dm-including", ["Yo"]]]); + narrow_banner.show_empty_narrow_message(); + assert.equal( + $(".empty_feed_notice_main").html(), + empty_narrow_html("translated: This user does not exist!"), + ); + + set_filter([["dm-including", "alice@example.com"]]); + narrow_banner.show_empty_narrow_message(); + assert.equal( + $(".empty_feed_notice_main").html(), + empty_narrow_html( + "translated: You are not allowed to send direct messages in this organization.", + ), + ); + + // direct messages with a bot are possible even though + // the organization has disabled sending direct messages + set_filter([["dm-including", "bot@example.com"]]); + narrow_banner.show_empty_narrow_message(); + assert.equal( + $(".empty_feed_notice_main").html(), + empty_narrow_html( + "translated: You have no direct messages including Example Bot yet.", + 'translated HTML: Why not start the conversation?', + ), + ); + + // sending direct messages enabled + page_params.realm_private_message_policy = + settings_config.private_message_policy_values.by_anyone.code; + set_filter([["dm-including", "alice@example.com"]]); + narrow_banner.show_empty_narrow_message(); + assert.equal( + $(".empty_feed_notice_main").html(), + empty_narrow_html( + "translated: You have no direct messages including Alice Smith yet.", + 'translated HTML: Why not start the conversation?', + ), + ); + + set_filter([["dm-including", me.email]]); + narrow_banner.show_empty_narrow_message(); + assert.equal( + $(".empty_feed_notice_main").html(), + empty_narrow_html("translated: You don't have any direct message conversations yet."), + ); + // organization has disabled sending direct messages page_params.realm_private_message_policy = settings_config.private_message_policy_values.disabled.code; diff --git a/web/tests/search_suggestion_future.test.js b/web/tests/search_suggestion_future.test.js index daae701f86..133c9572ff 100644 --- a/web/tests/search_suggestion_future.test.js +++ b/web/tests/search_suggestion_future.test.js @@ -133,6 +133,7 @@ test("dm_suggestions", ({override}) => { "is:alerted", "sender:alice@zulip.com", "dm:alice@zulip.com", + "dm-including:alice@zulip.com", "group-pm-with:alice@zulip.com", ]; assert.deepEqual(suggestions.strings, expected); @@ -220,6 +221,7 @@ test("dm_suggestions", ({override}) => { "is:alerted", "sender:alice@zulip.com", "dm:alice@zulip.com", + "dm-including:alice@zulip.com", "group-pm-with:alice@zulip.com", ]; assert.deepEqual(suggestions.strings, expected); @@ -516,6 +518,7 @@ test("check_is_suggestions", ({override}) => { "is:resolved", "sender:alice@zulip.com", "dm:alice@zulip.com", + "dm-including:alice@zulip.com", "group-pm-with:alice@zulip.com", "has:image", ]; @@ -707,7 +710,13 @@ test("topic_suggestions", ({override}) => { stream_data.add_sub({stream_id: office_id, name: "office", subscribed: true}); suggestions = get_suggestions("", "te"); - expected = ["te", "sender:ted@zulip.com", "dm:ted@zulip.com", "group-pm-with:ted@zulip.com"]; + expected = [ + "te", + "sender:ted@zulip.com", + "dm:ted@zulip.com", + "dm-including:ted@zulip.com", + "group-pm-with:ted@zulip.com", + ]; assert.deepEqual(suggestions.strings, expected); stream_topic_history.add_message({ @@ -727,6 +736,7 @@ test("topic_suggestions", ({override}) => { "te", "sender:ted@zulip.com", "dm:ted@zulip.com", + "dm-including:ted@zulip.com", "group-pm-with:ted@zulip.com", "stream:office topic:team", "stream:office topic:test", @@ -882,6 +892,8 @@ test("people_suggestions", ({override}) => { "sender:ted@zulip.com", "dm:bob@zulip.com", // bob térry "dm:ted@zulip.com", + "dm-including:bob@zulip.com", + "dm-including:ted@zulip.com", "group-pm-with:bob@zulip.com", "group-pm-with:ted@zulip.com", ]; @@ -892,16 +904,19 @@ test("people_suggestions", ({override}) => { assert.equal(is_person("dm:ted@zulip.com"), true); assert.equal(is_person("sender:ted@zulip.com"), true); assert.equal(is_person("group-pm-with:ted@zulip.com"), true); + assert.equal(is_person("dm-including:ted@zulip.com"), true); const has_image = (q) => suggestions.lookup_table.get(q).user_pill_context.has_image; assert.equal(has_image("dm:bob@zulip.com"), true); assert.equal(has_image("sender:bob@zulip.com"), true); assert.equal(has_image("group-pm-with:bob@zulip.com"), true); + assert.equal(has_image("dm-including:bob@zulip.com"), true); const describe = (q) => suggestions.lookup_table.get(q).description_html; assert.equal(describe("dm:ted@zulip.com"), "Direct messages with"); assert.equal(describe("sender:ted@zulip.com"), "Sent by"); assert.equal(describe("group-pm-with:ted@zulip.com"), "Group direct messages including"); + assert.equal(describe("dm-including:ted@zulip.com"), "Direct messages including"); let expectedString = "Ted Smith"; @@ -910,6 +925,7 @@ test("people_suggestions", ({override}) => { assert.equal(get_full_name("sender:ted@zulip.com"), expectedString); assert.equal(get_full_name("dm:ted@zulip.com"), expectedString); assert.equal(get_full_name("group-pm-with:ted@zulip.com"), expectedString); + assert.equal(get_full_name("dm-including:ted@zulip.com"), expectedString); expectedString = `${example_avatar_url}?s=50`; @@ -917,9 +933,16 @@ test("people_suggestions", ({override}) => { assert.equal(get_avatar_url("dm:bob@zulip.com"), expectedString); assert.equal(get_avatar_url("sender:bob@zulip.com"), expectedString); assert.equal(get_avatar_url("group-pm-with:bob@zulip.com"), expectedString); + assert.equal(get_avatar_url("dm-including:bob@zulip.com"), expectedString); suggestions = get_suggestions("", "Ted "); // note space - expected = ["Ted", "sender:ted@zulip.com", "dm:ted@zulip.com", "group-pm-with:ted@zulip.com"]; + expected = [ + "Ted", + "sender:ted@zulip.com", + "dm:ted@zulip.com", + "dm-including:ted@zulip.com", + "group-pm-with:ted@zulip.com", + ]; assert.deepEqual(suggestions.strings, expected); @@ -1013,6 +1036,7 @@ test("multiple_operators_without_pills", () => { "is:dm is:alerted", "is:dm sender:alice@zulip.com", "is:dm dm:alice@zulip.com", + "is:dm dm-including:alice@zulip.com", "is:dm group-pm-with:alice@zulip.com", ]; assert.deepEqual(suggestions.strings, expected); diff --git a/web/tests/search_suggestion_now.test.js b/web/tests/search_suggestion_now.test.js index 3816b9baba..3a84f7a35b 100644 --- a/web/tests/search_suggestion_now.test.js +++ b/web/tests/search_suggestion_now.test.js @@ -134,6 +134,7 @@ test("dm_suggestions", ({override}) => { "is:dm is:alerted", "is:dm sender:alice@zulip.com", "is:dm dm:alice@zulip.com", + "is:dm dm-including:alice@zulip.com", "is:dm group-pm-with:alice@zulip.com", "is:dm", ]; @@ -217,6 +218,7 @@ test("dm_suggestions", ({override}) => { "is:starred has:link is:dm is:alerted", "is:starred has:link is:dm sender:alice@zulip.com", "is:starred has:link is:dm dm:alice@zulip.com", + "is:starred has:link is:dm dm-including:alice@zulip.com", "is:starred has:link is:dm group-pm-with:alice@zulip.com", "is:starred has:link is:dm", "is:starred has:link", @@ -524,6 +526,7 @@ test("check_is_suggestions", ({override}) => { "is:resolved", "sender:alice@zulip.com", "dm:alice@zulip.com", + "dm-including:alice@zulip.com", "group-pm-with:alice@zulip.com", "has:image", ]; @@ -674,7 +677,13 @@ test("topic_suggestions", ({override}) => { stream_data.add_sub({stream_id: office_id, name: "office", subscribed: true}); suggestions = get_suggestions("", "te"); - expected = ["te", "sender:ted@zulip.com", "dm:ted@zulip.com", "group-pm-with:ted@zulip.com"]; + expected = [ + "te", + "sender:ted@zulip.com", + "dm:ted@zulip.com", + "dm-including:ted@zulip.com", + "group-pm-with:ted@zulip.com", + ]; assert.deepEqual(suggestions.strings, expected); stream_topic_history.add_message({ @@ -694,6 +703,7 @@ test("topic_suggestions", ({override}) => { "te", "sender:ted@zulip.com", "dm:ted@zulip.com", + "dm-including:ted@zulip.com", "group-pm-with:ted@zulip.com", "stream:office topic:team", "stream:office topic:test", @@ -854,6 +864,8 @@ test("people_suggestions", ({override}) => { "sender:ted@zulip.com", "dm:bob@zulip.com", // bob térry "dm:ted@zulip.com", + "dm-including:bob@zulip.com", + "dm-including:ted@zulip.com", "group-pm-with:bob@zulip.com", "group-pm-with:ted@zulip.com", ]; @@ -866,6 +878,7 @@ test("people_suggestions", ({override}) => { assert.equal(is_person("dm:ted@zulip.com"), true); assert.equal(is_person("sender:ted@zulip.com"), true); assert.equal(is_person("group-pm-with:ted@zulip.com"), true); + assert.equal(is_person("dm-including:ted@zulip.com"), true); function has_image(q) { return suggestions.lookup_table.get(q).user_pill_context.has_image; @@ -873,6 +886,7 @@ test("people_suggestions", ({override}) => { assert.equal(has_image("dm:bob@zulip.com"), true); assert.equal(has_image("sender:bob@zulip.com"), true); assert.equal(has_image("group-pm-with:bob@zulip.com"), true); + assert.equal(has_image("dm-including:bob@zulip.com"), true); function describe(q) { return suggestions.lookup_table.get(q).description_html; @@ -880,6 +894,7 @@ test("people_suggestions", ({override}) => { assert.equal(describe("dm:ted@zulip.com"), "Direct messages with"); assert.equal(describe("sender:ted@zulip.com"), "Sent by"); assert.equal(describe("group-pm-with:ted@zulip.com"), "Group direct messages including"); + assert.equal(describe("dm-including:ted@zulip.com"), "Direct messages including"); let expectedString = "Ted Smith"; @@ -889,6 +904,7 @@ test("people_suggestions", ({override}) => { assert.equal(get_full_name("sender:ted@zulip.com"), expectedString); assert.equal(get_full_name("dm:ted@zulip.com"), expectedString); assert.equal(get_full_name("group-pm-with:ted@zulip.com"), expectedString); + assert.equal(get_full_name("dm-including:ted@zulip.com"), expectedString); expectedString = example_avatar_url + "?s=50"; @@ -898,10 +914,17 @@ test("people_suggestions", ({override}) => { assert.equal(get_avatar_url("dm:bob@zulip.com"), expectedString); assert.equal(get_avatar_url("sender:bob@zulip.com"), expectedString); assert.equal(get_avatar_url("group-pm-with:bob@zulip.com"), expectedString); + assert.equal(get_avatar_url("dm-including:bob@zulip.com"), expectedString); suggestions = get_suggestions("", "Ted "); // note space - expected = ["Ted", "sender:ted@zulip.com", "dm:ted@zulip.com", "group-pm-with:ted@zulip.com"]; + expected = [ + "Ted", + "sender:ted@zulip.com", + "dm:ted@zulip.com", + "dm-including:ted@zulip.com", + "group-pm-with:ted@zulip.com", + ]; assert.deepEqual(suggestions.strings, expected);