diff --git a/web/src/filter.ts b/web/src/filter.ts index 12a348f90c..173100d545 100644 --- a/web/src/filter.ts +++ b/web/src/filter.ts @@ -402,7 +402,8 @@ export class Filter { return orig_terms; } - const updated_terms = [...orig_terms]; + const updated_terms = orig_terms.filter((term: NarrowTerm) => term.operator !== "dm"); + let channel_term = updated_terms.find( (term: NarrowTerm) => Filter.canonicalize_operator(term.operator) === "channel", ); @@ -616,9 +617,9 @@ export class Filter { "channels-public", "channel", "topic", - "with", "dm", "dm-including", + "with", "sender", "near", "id", @@ -891,6 +892,8 @@ export class Filter { this.cached_sorted_terms_for_comparison = undefined; if (this.has_operator("channel")) { this._sub = stream_data.get_sub_by_name(this.operands("channel")[0]!); + } else { + this._sub = undefined; } } @@ -1078,6 +1081,10 @@ export class Filter { return true; } + if (_.isEqual(term_types, ["dm", "with"])) { + return true; + } + if (_.isEqual(term_types, ["dm"])) { return true; } @@ -1303,6 +1310,7 @@ export class Filter { } if ( (term_types.length === 2 && _.isEqual(term_types, ["dm", "near"])) || + (term_types.length === 2 && _.isEqual(term_types, ["dm", "with"])) || (term_types.length === 1 && _.isEqual(term_types, ["dm"])) ) { const emails = this.operands("dm")[0]!.split(","); @@ -1475,7 +1483,6 @@ export class Filter { fix_terms(terms: NarrowTerm[]): NarrowTerm[] { terms = this._canonicalize_terms(terms); terms = this._fix_redundant_is_private(terms); - terms = this._fix_redundant_with_dm(terms); return terms; } @@ -1488,16 +1495,6 @@ export class Filter { return terms.filter((term) => Filter.term_type(term) !== "is-dm"); } - _fix_redundant_with_dm(terms: NarrowTerm[]): NarrowTerm[] { - // Because DMs can't move, the `with` operator is a noop on a - // DM conversation. - if (terms.some((term) => Filter.term_type(term) === "dm")) { - return terms.filter((term) => Filter.term_type(term) !== "with"); - } - - return terms; - } - _canonicalize_terms(terms_mixed_case: NarrowTerm[]): NarrowTerm[] { return terms_mixed_case.map((term: NarrowTerm) => Filter.canonicalize_term(term)); } @@ -1613,6 +1610,7 @@ export class Filter { if ( _.isEqual(term_type, ["channel", "topic", "with"]) || _.isEqual(term_type, ["channel", "topic"]) || + _.isEqual(term_type, ["dm", "with"]) || _.isEqual(term_type, ["dm"]) ) { return true; diff --git a/web/src/narrow_state.ts b/web/src/narrow_state.ts index 7b60ea3c77..473b92480d 100644 --- a/web/src/narrow_state.ts +++ b/web/src/narrow_state.ts @@ -304,7 +304,7 @@ export function _possible_unread_message_ids( return unread.get_msg_ids_for_stream(sub.stream_id); } - if (current_filter.can_bucket_by("dm")) { + if (current_filter.can_bucket_by("dm", "with") || current_filter.can_bucket_by("dm")) { current_filter_pm_string = pm_ids_string(current_filter); if (current_filter_pm_string === undefined) { return []; @@ -349,7 +349,7 @@ export function narrowed_by_pm_reply(current_filter: Filter | undefined = filter if (current_filter === undefined) { return false; } - const terms = current_filter.terms(); + const terms = current_filter.terms().filter((term) => term.operator !== "with"); return terms.length === 1 && current_filter.has_operator("dm"); } diff --git a/web/tests/filter.test.js b/web/tests/filter.test.js index 192aa1aa4a..ef7fd88ffd 100644 --- a/web/tests/filter.test.js +++ b/web/tests/filter.test.js @@ -324,6 +324,19 @@ test("basics", () => { assert.ok(filter.is_conversation_view()); assert.ok(!filter.is_conversation_view_with_near()); + terms = [ + {operator: "dm", operand: "joe@example.com,jack@example.com"}, + {operator: "with", operand: "12"}, + ]; + filter = new Filter(terms); + assert.ok(filter.contains_only_private_messages()); + assert.ok(filter.can_mark_messages_read()); + assert.ok(filter.supports_collapsing_recipients()); + assert.ok(filter.can_apply_locally()); + assert.ok(!filter.is_personal_filter()); + assert.ok(filter.is_conversation_view()); + assert.ok(!filter.is_conversation_view_with_near()); + // "pm-with" was renamed to "dm" terms = [{operator: "pm-with", operand: "joe@example.com"}]; filter = new Filter(terms); @@ -742,13 +755,6 @@ test("redundancies", () => { ]; filter = new Filter(terms); assert.ok(filter.can_bucket_by("is-dm", "not-dm")); - - terms = [ - {operator: "dm", operand: "joe@example.com,"}, - {operator: "with", operand: "12"}, - ]; - filter = new Filter(terms); - assert.ok(filter.can_bucket_by("dm")); }); test("canonicalization", () => { @@ -1561,12 +1567,25 @@ test("can_bucket_by", () => { assert.equal(filter.can_bucket_by("channel"), false); assert.equal(filter.can_bucket_by("channel", "topic"), false); assert.equal(filter.can_bucket_by("dm"), false); + assert.equal(filter.can_bucket_by("dm", "with"), false); terms = [{operator: "dm", operand: "foo@example.com,bar@example.com"}]; filter = new Filter(terms); assert.equal(filter.can_bucket_by("channel"), false); assert.equal(filter.can_bucket_by("channel", "topic"), false); assert.equal(filter.can_bucket_by("dm"), true); + assert.equal(filter.can_bucket_by("dm", "with"), false); + assert.equal(filter.can_bucket_by("is-mentioned"), false); + assert.equal(filter.can_bucket_by("is-dm"), false); + + terms = [ + {operator: "dm", operand: "foo@example.com,bar@example.com"}, + {operator: "with", operand: "7"}, + ]; + filter = new Filter(terms); + assert.equal(filter.can_bucket_by("channel"), false); + assert.equal(filter.can_bucket_by("channel", "topic"), false); + assert.equal(filter.can_bucket_by("dm", "with"), true); assert.equal(filter.can_bucket_by("is-mentioned"), false); assert.equal(filter.can_bucket_by("is-dm"), false); @@ -1820,6 +1839,22 @@ test("try_adjusting_for_moved_with_target", ({override}) => { assert.deepEqual(filter.requires_adjustment_for_moved_with_target, false); assert.deepEqual(filter.terms(), [ {operator: "dm", operand: "user3@zulip.com", negated: false}, + {operator: "with", operand: "2", negated: false}, + ]); + + // When the narrow consists of `dm` operators, while the `with` + // operator corresponds to that of a channel topic message. + terms = [ + {operator: "dm", operand: "iago@foo.com"}, + {operator: "with", operand: "12"}, + ]; + filter = new Filter(terms); + filter.try_adjusting_for_moved_with_target(); + assert.deepEqual(filter.requires_adjustment_for_moved_with_target, false); + assert.deepEqual(filter.terms(), [ + {operator: "channel", operand: "Scotland", negated: false}, + {operator: "topic", operand: "Test 1", negated: false}, + {operator: "with", operand: "12", negated: false}, ]); // When message id attached to `with` operator is found locally, @@ -1991,6 +2026,10 @@ test("navbar_helpers", () => { {operator: "topic", operand: "pink"}, ]; const dm = [{operator: "dm", operand: "joe@example.com"}]; + const dm_with = [ + {operator: "dm", operand: "joe@example.com"}, + {operator: "with", operand: "12"}, + ]; const dm_group = [{operator: "dm", operand: "joe@example.com,STEVE@foo.com"}]; const dm_with_guest = [{operator: "dm", operand: "alice@example.com"}]; const dm_group_including_guest = [ @@ -2221,6 +2260,13 @@ test("navbar_helpers", () => { title: "Foo", redirect_url_with_search: "#", }, + { + terms: dm_with, + is_common_narrow: true, + zulip_icon: "user", + title: properly_separated_names([joe.full_name]), + redirect_url_with_search: "#", + }, ]; realm.realm_enable_guest_user_indicator = true;